diff --git a/app/database/group_group_store.go b/app/database/group_group_store.go index 139691e39..bd1a2447b 100644 --- a/app/database/group_group_store.go +++ b/app/database/group_group_store.go @@ -102,7 +102,7 @@ func (s *GroupGroupStore) DeleteRelation(parentGroupID, childGroupID int64, shou if shouldDeleteChildGroup { s.GroupGroups().deleteGroupAndOrphanedDescendants(childGroupID) } else { - // delete the relation we are asked to delete (triggers will delete a lot from groups_ancestors and mark relations for propagation) + // delete the relation we are asked to delete (triggers will mark relations for propagation) mustNotBeError(s.GroupGroups().Delete("parent_group_id = ? AND child_group_id = ?", parentGroupID, childGroupID).Error()) permissionsResult := s.PermissionsGranted(). @@ -134,10 +134,12 @@ func (s *GroupGroupStore) deleteGroupAndOrphanedDescendants(groupID int64) { Pluck("groups.id", &candidatesForDeletion).Error()) // we delete groups_groups linked to groupID here in order to recalculate new ancestors correctly - mustNotBeError(s.deleteObjectsLinkedToGroups([]int64{groupID}).Error()) + groupRelationsDeleted, permissionsDeleted := s.deleteObjectsLinkedToGroups([]int64{groupID}) // recalculate relations - s.GroupGroups().createNewAncestors() + if groupRelationsDeleted > 0 { + s.GroupGroups().createNewAncestors() + } var idsToDelete []int64 // besides the group with id = groupID, we also want to delete its descendants @@ -162,9 +164,9 @@ func (s *GroupGroupStore) deleteGroupAndOrphanedDescendants(groupID int64) { Pluck("groups.id", &idsToDelete).Error()) if len(idsToDelete) > 0 { - deleteResult := s.deleteObjectsLinkedToGroups(idsToDelete) - mustNotBeError(deleteResult.Error()) - if deleteResult.RowsAffected() > 0 { + groupRelationsDeleted, permissionsDeletedForDescendants := s.deleteObjectsLinkedToGroups(idsToDelete) + permissionsDeleted += permissionsDeletedForDescendants + if groupRelationsDeleted > 0 { s.GroupGroups().createNewAncestors() } } @@ -174,20 +176,23 @@ func (s *GroupGroupStore) deleteGroupAndOrphanedDescendants(groupID int64) { // cascading deletes from many tables including groups_ancestors, groups_propagate, permission_granted & permissions_generated mustNotBeError(s.Groups().Delete("id IN (?)", idsToDelete).Error()) - s.SchedulePermissionsPropagation() + if permissionsDeleted > 0 { + s.SchedulePermissionsPropagation() + } } -func (s *GroupGroupStore) deleteObjectsLinkedToGroups(groupIDs []int64) *DB { - return s.Exec(` - DELETE group_children, group_parents, filters - FROM `+"`groups`"+` - LEFT JOIN groups_groups AS group_children - ON group_children.parent_group_id = groups.id - LEFT JOIN groups_groups AS group_parents - ON group_parents.child_group_id = groups.id - LEFT JOIN filters - ON filters.group_id = groups.id - WHERE groups.id IN(?)`, groupIDs) +func (s *GroupGroupStore) deleteObjectsLinkedToGroups(groupIDs []int64) (groupRelationsDeleted, permissionsDeleted int64) { + result := s.GroupGroups().Delete("parent_group_id IN(?)", groupIDs) + mustNotBeError(result.Error()) + groupRelationsDeleted += result.RowsAffected() + result = s.GroupGroups().Delete("child_group_id IN(?)", groupIDs) + mustNotBeError(result.Error()) + groupRelationsDeleted += result.RowsAffected() + mustNotBeError(s.Table("filters").Delete("group_id IN(?)", groupIDs).Error()) + result = s.PermissionsGranted().Delete("source_group_id IN(?)", groupIDs) + mustNotBeError(result.Error()) + permissionsDeleted = result.RowsAffected() + return groupRelationsDeleted, permissionsDeleted } // CreateNewAncestors creates ancestors for groups marked as 'todo' in `groups_propagate`. diff --git a/app/database/group_store_integration_test.go b/app/database/group_store_integration_test.go index e5a0eb8fd..986dd419f 100644 --- a/app/database/group_store_integration_test.go +++ b/app/database/group_store_integration_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/France-ioi/AlgoreaBackend/v2/app/database" "github.com/France-ioi/AlgoreaBackend/v2/testhelpers" @@ -357,7 +358,7 @@ func TestGroupStore_CheckIfEntryConditionsStillSatisfiedForAllActiveParticipatio } } -func Test_GroupStore_DeleteGroup(t *testing.T) { +func TestGroupStore_DeleteGroup(t *testing.T) { testoutput.SuppressIfPasses(t) db := testhelpers.SetupDBWithFixtureString(`groups: [{id: 1234}]`) @@ -373,6 +374,52 @@ func Test_GroupStore_DeleteGroup(t *testing.T) { assert.Empty(t, ids) } +func TestGroupStore_DeleteGroup_RecomputesAccess(t *testing.T) { + testoutput.SuppressIfPasses(t) + + db := testhelpers.SetupDBWithFixtureString(` + groups: [{id: 1234}, {id: 1235}] + items: [{id: 10, default_language_tag: fr}] + permissions_granted: + - {group_id: 1234, item_id: 10, source_group_id: 1235, can_view: content} + - {group_id: 1234, item_id: 10, source_group_id: 1234, can_view: info} + permissions_generated: [{group_id: 1234, item_id: 10, can_view_generated: content}]`) + defer func() { _ = db.Close() }() + + dataStore := database.NewDataStore(db).Groups() + require.NoError(t, dataStore.InTransaction(func(store *database.DataStore) error { + return store.Groups().DeleteGroup(1235) + })) + var newPermission string + require.NoError(t, dataStore.Permissions().Where("group_id = 1234 AND item_id = 10"). + PluckFirst("can_view_generated", &newPermission).Error()) + assert.Equal(t, "info", newPermission) +} + +func TestGroupStore_DeleteGroup_RecomputesAccessForOrphanedSourceGroups(t *testing.T) { + testoutput.SuppressIfPasses(t) + + db := testhelpers.SetupDBWithFixtureString(` + groups: [{id: 1234}, {id: 1235}, {id: 1236}] + groups_groups: [{parent_group_id: 1235, child_group_id: 1236}] + groups_ancestors: [{ancestor_group_id: 1235, child_group_id: 1236}] + items: [{id: 10, default_language_tag: fr}] + permissions_granted: + - {group_id: 1234, item_id: 10, source_group_id: 1236, can_view: content} + - {group_id: 1234, item_id: 10, source_group_id: 1234, can_view: info} + permissions_generated: [{group_id: 1234, item_id: 10, can_view_generated: content}]`) + defer func() { _ = db.Close() }() + + dataStore := database.NewDataStore(db).Groups() + require.NoError(t, dataStore.InTransaction(func(store *database.DataStore) error { + return store.Groups().DeleteGroup(1235) + })) + var newPermission string + require.NoError(t, dataStore.Permissions().Where("group_id = 1234 AND item_id = 10"). + PluckFirst("can_view_generated", &newPermission).Error()) + assert.Equal(t, "info", newPermission) +} + func TestGroupStore_TriggerBeforeUpdate_RefusesToModifyType(t *testing.T) { const expectedErrorMessage = "Error 1644: Unable to change groups.type from/to User/Team"