Skip to content

Commit

Permalink
viewPermissions: add array of can_request_help_to in $.[*].permission…
Browse files Browse the repository at this point in the history
…s for group_membership, item_unlocking, self and other

Also changed the structure of a can_request_help_to permission: id, is_all_users_group and name are now always present. It improves consistency.
Not implemented for computed yet
@see #1004
  • Loading branch information
GeoffreyHuck committed Sep 8, 2023
1 parent dfe77ed commit b08cb57
Show file tree
Hide file tree
Showing 3 changed files with 212 additions and 41 deletions.
180 changes: 150 additions & 30 deletions app/api/groups/get_permissions.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,36 @@ import (

"github.com/go-chi/render"

"github.com/France-ioi/AlgoreaBackend/app/database"
"github.com/France-ioi/AlgoreaBackend/app/domain"
"github.com/France-ioi/AlgoreaBackend/app/service"
"github.com/France-ioi/AlgoreaBackend/app/structures"
)

const (
OriginGroupMembership = "group_membership"
OriginItemUnlocking = "item_unlocking"
OriginSelf = "self"
OriginOther = "other"
)

type permissionsStruct struct {
structures.ItemPermissions
// required: true
CanMakeSessionOfficial bool `json:"can_make_session_official"`
}

// The group which can be asked for help.
// either (ID, Name) is set, or IsAllUsersGroup is set and equal to true.
type canRequestHelpTo struct {
ID int64 `json:"id,string,omitempty"`
Name string `json:"name,omitempty"`
// Whether the group is the "all-users" group
IsAllUsersGroup bool `json:"is_all_users_group,omitempty"`
// required: true
ID int64 `json:"id,string"`
// The name is only set if the group is visible to the current user.
// Nullable
// required: true
Name *string `json:"name"`
// Whether the group is the "all-users" group.
// required: true
IsAllUsersGroup bool `json:"is_all_users_group"`
}

// Permissions granted directly to the group via `origin` = 'group_membership' and `source_group_id` = `{source_group_id}`.
Expand Down Expand Up @@ -92,6 +104,14 @@ type permissionsViewResponse struct {
GrantedViaOther permissionsGrantedViaOther `json:"granted_via_other"`
}

type canRequestHelpToPermissionsRaw struct {
Origin string
SourceGroupID int64
PermissionGroupID int64
GroupID int64
GroupName string
}

// swagger:operation GET /groups/{source_group_id}/permissions/{group_id}/{item_id} groups permissionsView
//
// ---
Expand Down Expand Up @@ -202,7 +222,7 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
Where("group_id = ?", groupID).
Where("item_id = ?", itemID).
Where("source_group_id = ?", sourceGroupID).
Where("origin = 'group_membership'").
Where("origin = ?", OriginGroupMembership).
Select(`
can_view_value,
can_grant_view_value,
Expand Down Expand Up @@ -263,11 +283,11 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
aggregatedPermissions := ancestorPermissions.Select(canEnterFromColumn + ", " + canMakeSessionOfficialColumn)

grantedPermissionsGroupMembership := grantedPermissionsWithAncestors.
Where("origin = 'group_membership'").
Where("origin = ?", OriginGroupMembership).
Where("NOT (group_id = ? AND source_group_id = ?)", groupID, sourceGroupID)
grantedPermissionsItemUnlocking := grantedPermissionsWithAncestors.Where("origin = 'item_unlocking'")
grantedPermissionsSelf := grantedPermissionsWithAncestors.Where("origin = 'self'")
grantedPermissionsOther := grantedPermissionsWithAncestors.Where("origin = 'other'")
grantedPermissionsItemUnlocking := grantedPermissionsWithAncestors.Where("origin = ?", OriginItemUnlocking)
grantedPermissionsSelf := grantedPermissionsWithAncestors.Where("origin = ?", OriginSelf)
grantedPermissionsOther := grantedPermissionsWithAncestors.Where("origin = ?", OriginOther)

err = store.
Raw(`
Expand Down Expand Up @@ -320,26 +340,45 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
ScanIntoSliceOfMaps(&permissions).Error()
service.MustNotBeError(err)

var canRequestHelpToPermissions []canRequestHelpToPermissionsRaw
err = ancestorPermissions.
Joins("JOIN `groups` AS can_request_help_to_group ON can_request_help_to_group.id = permissions_granted.can_request_help_to").
Where("source_group_id = ?", sourceGroupID).
Select(`
permissions_granted.origin AS origin,
permissions_granted.source_group_id AS source_group_id,
permissions_granted.group_id AS permission_group_id,
can_request_help_to_group.id AS group_id,
can_request_help_to_group.name AS group_name
`).
Scan(&canRequestHelpToPermissions).
Error()
service.MustNotBeError(err)

permissionsRow := permissions[0]
permissionsGrantedStore := store.PermissionsGranted()

allUsersGroupID := domain.ConfigFromContext(r.Context()).AllUsersGroupID

var canRequestHelpToPermission *canRequestHelpTo

if permissionsRow["granted_directly_can_request_help_to__id"] != nil {
canRequestHelpToGroupID := permissionsRow["granted_directly_can_request_help_to__id"].(int64)
if domain.ConfigFromContext(r.Context()).AllUsersGroupID == canRequestHelpToGroupID {
canRequestHelpToPermission = &canRequestHelpTo{
IsAllUsersGroup: true,
}
groupName := permissionsRow["granted_directly_can_request_help_to__name"].(string)

canRequestHelpToPermission = &canRequestHelpTo{
ID: canRequestHelpToGroupID,
}

if allUsersGroupID == canRequestHelpToGroupID {
canRequestHelpToPermission.IsAllUsersGroup = true
canRequestHelpToPermission.Name = &groupName
} else if store.Groups().IsVisibleFor(canRequestHelpToGroupID, user) {
canRequestHelpToPermission = &canRequestHelpTo{
ID: canRequestHelpToGroupID,
Name: permissionsRow["granted_directly_can_request_help_to__name"].(string),
}
canRequestHelpToPermission.Name = &groupName
}
}

render.Respond(w, r, &permissionsViewResponse{
response := permissionsViewResponse{
Granted: grantedPermissionsStruct{
permissionsStruct: permissionsStruct{
ItemPermissions: structures.ItemPermissions{
Expand All @@ -366,8 +405,16 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
},
CanMakeSessionOfficial: permissionsRow["generated_can_make_session_official"].(int64) == 1,
},
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["generated_can_enter_from"]),
CanRequestHelpTo: make([]canRequestHelpTo, 0),
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["generated_can_enter_from"]),
CanRequestHelpTo: filterCanRequestHelpTo(
canRequestHelpToPermissions,
"computed",
groupID,
sourceGroupID,
store,
user.GroupID,
allUsersGroupID,
),
}},
GrantedViaGroupMembership: permissionsGrantedViaGroupMembership{aggregatedPermissionsWithCanEnterFromStruct{
permissionsStruct: permissionsStruct{
Expand All @@ -380,8 +427,16 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
},
CanMakeSessionOfficial: permissionsRow["granted_anc_membership_can_make_session_official"].(int64) == 1,
},
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_membership_can_enter_from"]),
CanRequestHelpTo: make([]canRequestHelpTo, 0),
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_membership_can_enter_from"]),
CanRequestHelpTo: filterCanRequestHelpTo(
canRequestHelpToPermissions,
OriginGroupMembership,
groupID,
sourceGroupID,
store,
user.GroupID,
allUsersGroupID,
),
}},
GrantedViaItemUnlocking: permissionsGrantedViaItemUnlocking{aggregatedPermissionsWithCanEnterFromStruct{
permissionsStruct: permissionsStruct{
Expand All @@ -394,8 +449,16 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
},
CanMakeSessionOfficial: permissionsRow["granted_anc_unlocking_can_make_session_official"].(int64) == 1,
},
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_unlocking_can_enter_from"]),
CanRequestHelpTo: make([]canRequestHelpTo, 0),
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_unlocking_can_enter_from"]),
CanRequestHelpTo: filterCanRequestHelpTo(
canRequestHelpToPermissions,
OriginItemUnlocking,
groupID,
sourceGroupID,
store,
user.GroupID,
allUsersGroupID,
),
}},
GrantedViaSelf: permissionsGrantedViaSelf{aggregatedPermissionsWithCanEnterFromStruct{
permissionsStruct: permissionsStruct{
Expand All @@ -408,8 +471,16 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
},
CanMakeSessionOfficial: permissionsRow["granted_anc_self_can_make_session_official"].(int64) == 1,
},
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_self_can_enter_from"]),
CanRequestHelpTo: make([]canRequestHelpTo, 0),
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_self_can_enter_from"]),
CanRequestHelpTo: filterCanRequestHelpTo(
canRequestHelpToPermissions,
OriginSelf,
groupID,
sourceGroupID,
store,
user.GroupID,
allUsersGroupID,
),
}},
GrantedViaOther: permissionsGrantedViaOther{aggregatedPermissionsWithCanEnterFromStruct{
permissionsStruct: permissionsStruct{
Expand All @@ -422,9 +493,58 @@ func (srv *Service) getPermissions(w http.ResponseWriter, r *http.Request) servi
},
CanMakeSessionOfficial: permissionsRow["granted_anc_other_can_make_session_official"].(int64) == 1,
},
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_other_can_enter_from"]),
CanRequestHelpTo: make([]canRequestHelpTo, 0),
CanEnterFrom: service.ConvertDBTimeToJSONTime(permissionsRow["granted_anc_other_can_enter_from"]),
CanRequestHelpTo: filterCanRequestHelpTo(
canRequestHelpToPermissions,
OriginOther,
groupID,
sourceGroupID,
store,
user.GroupID,
allUsersGroupID,
),
}},
})
}

render.Respond(w, r, &response)
return service.NoError
}

func filterCanRequestHelpTo(
permissions []canRequestHelpToPermissionsRaw,
origin string,
groupID int64,
sourceGroupID int64,
store *database.DataStore,
visibleGroupID int64,
allUsersGroupID int64,
) []canRequestHelpTo {
results := make([]canRequestHelpTo, 0)

for i, canRequestHelpToPermission := range permissions {
// The canRequestHelpToPermission matching "group_membership" origin as well as GroupID and SourceGroupID
// is a special case that goes into "granted".
if canRequestHelpToPermission.Origin == OriginGroupMembership &&
canRequestHelpToPermission.PermissionGroupID == groupID &&
canRequestHelpToPermission.SourceGroupID == sourceGroupID {
continue
}

if canRequestHelpToPermission.Origin == origin {
curCanRequestHelpTo := canRequestHelpTo{
ID: canRequestHelpToPermission.GroupID,
}

if allUsersGroupID == canRequestHelpToPermission.GroupID {
curCanRequestHelpTo.IsAllUsersGroup = true
curCanRequestHelpTo.Name = &permissions[i].GroupName
} else if store.Groups().IsVisibleForGroup(canRequestHelpToPermission.GroupID, visibleGroupID) {
curCanRequestHelpTo.Name = &permissions[i].GroupName
}

results = append(results, curCanRequestHelpTo)
}
}

return results
}
71 changes: 61 additions & 10 deletions app/api/groups/get_permissions_can_request_help_to.feature
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ Feature: Get permissions can_request_help_to for a group
Background:
Given allUsersGroup is defined as the group @AllUsers
And there are the following groups:
| group | parent | members |
| @AllUsers | | |
| @School | | @Teacher |
| @Class | | |
| group | parent | members |
| @AllUsers | | |
| @School | | @Teacher |
| @ClassParentParent | | |
| @ClassParent | @ClassParentParent | |
| @Class | @ClassParent | |
And @Teacher is a manager of the group @Class and can grant group access
And there are the following tasks:
| item |
Expand All @@ -34,18 +36,20 @@ Feature: Get permissions can_request_help_to for a group
When I send a GET request to "/groups/@Class/permissions/@Class/@Item"
Then the response code should be 200
And the response at $.granted.can_request_help_to should be:
| id | name |
| @HelperGroup | Group HelperGroup |
| id | name | is_all_users_group |
| @HelperGroup | Group HelperGroup | false |

Scenario: Should not return helper group when set and not visible by the current user
Scenario: Should return helper group without the name when set and not visible by the current user
Given I am @Teacher
And there is a group @HelperGroup
And there are the following item permissions:
| item | group | is_owner | can_request_help_to |
| @Item | @Class | false | @HelperGroup |
When I send a GET request to "/groups/@Class/permissions/@Class/@Item"
Then the response code should be 200
And the response at $.granted.can_request_help_to should be "null"
And the response at $.granted.can_request_help_to should be:
| id | name | is_all_users_group |
| @HelperGroup | | false |

Scenario: Should return helper group as "AllUsers" group when set to its value
Given I am @Teacher
Expand All @@ -55,5 +59,52 @@ Feature: Get permissions can_request_help_to for a group
When I send a GET request to "/groups/@Class/permissions/@Class/@Item"
Then the response code should be 200
And the response at $.granted.can_request_help_to should be:
| is_all_users_group |
| true |
| id | name | is_all_users_group |
| @AllUsers | AllUsers | true |

Scenario: Should return can_request_help_to arrays when permissions with specific origins are defined
Given I am @Teacher
And there are the following item permissions:
| item | group | source_group | origin | is_owner | can_request_help_to | comment |
| @Item | @Class | @Class | self | false | @HelperGroupSelf1 | |
| @Item | @ClassParent | @Class | self | false | @AllUsers | |
| @Item | @ClassParentParent | @Class | self | false | | check we don't get empty groups |
| @Item | @Class | @Class | group_membership | false | @HelperGroupGroupMembership1 | granted |
| @Item | @ClassParent | @Class | group_membership | false | @HelperGroupGroupMembership2 | group_membership but not granted |
| @Item | @ClassParentParent | @Class | group_membership | false | @AllUsers | group_membership but not granted |
| @Item | @ClassParent | @Class | item_unlocking | false | @HelperGroupItemUnlocking | |
| @Item | @Class | @Class | item_unlocking | false | @AllUsers | |
| @Item | @ClassParentParent | @Class | item_unlocking | false | @HelperGroupNotVisible | not visible |
| @Item | @Class | @Class | other | false | @AllUsers | |
| @Item | @ClassParentParent | @Class | other | false | @HelperGroupOther1 | |
| @Item | @ClassParent | @Class | other | false | @HelperGroupOther2 | |
# The following lines are to make the groups visible by @Teacher
And the group @Teacher is a descendant of the group @HelperGroupSelf1
And the group @Teacher is a descendant of the group @HelperGroupGroupMembership1
And the group @Teacher is a descendant of the group @HelperGroupGroupMembership2
And the group @Teacher is a descendant of the group @HelperGroupItemUnlocking
And the group @Teacher is a descendant of the group @HelperGroupOther1
And the group @Teacher is a descendant of the group @HelperGroupOther2
When I send a GET request to "/groups/@Class/permissions/@Class/@Item"
Then the response code should be 200
And the response at $.granted_via_self.can_request_help_to[*] should be:
| id | name | is_all_users_group |
| @AllUsers | AllUsers | true |
| @HelperGroupSelf1 | Group HelperGroupSelf1 | false |
And the response at $.granted.can_request_help_to should be:
| id | name |
| @HelperGroupGroupMembership1 | Group HelperGroupGroupMembership1 |
And the response at $.granted_via_group_membership.can_request_help_to[*] should be:
| id | name | is_all_users_group |
| @HelperGroupGroupMembership2 | Group HelperGroupGroupMembership2 | false |
| @AllUsers | AllUsers | true |
And the response at $.granted_via_item_unlocking.can_request_help_to[*] should be:
| id | name | is_all_users_group |
| @HelperGroupItemUnlocking | Group HelperGroupItemUnlocking | false |
| @AllUsers | AllUsers | true |
| @HelperGroupNotVisible | | false |
And the response at $.granted_via_other.can_request_help_to[*] should be:
| id | name | is_all_users_group |
| @AllUsers | AllUsers | true |
| @HelperGroupOther1 | Group HelperGroupOther1 | false |
| @HelperGroupOther2 | Group HelperGroupOther2 | false |
2 changes: 1 addition & 1 deletion testhelpers/steps_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func jsonPathResultMatchesValue(jsonPathRes interface{}, value string) bool {
case interface{}:
}

q if jsonPathRes == nil && (value == "null" || value == "") {
if jsonPathRes == nil && (value == "null" || value == "") {
return true
}

Expand Down

0 comments on commit b08cb57

Please sign in to comment.