From c553ec6aa063c5f8c53559b218d622dec6474ca1 Mon Sep 17 00:00:00 2001 From: Will Martin Date: Thu, 4 Jan 2024 20:56:59 -0500 Subject: [PATCH] [AC-1389] [AC-1919] Only require CanManage permission when admins cannot access all items (#3530) * move this error behind the Flexible Collections v1 flag instead of MVP * only enforce this requirement if organization.allowAdminAccessToAllCollectionItems is false --------- Co-authored-by: Thomas Rittson Co-authored-by: Thomas Rittson <31796059+eliykat@users.noreply.github.com> --- .../Services/Implementations/CollectionService.cs | 12 ++++++------ test/Core.Test/Services/CollectionServiceTests.cs | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/Core/Services/Implementations/CollectionService.cs b/src/Core/Services/Implementations/CollectionService.cs index 17ae6068d..27c933826 100644 --- a/src/Core/Services/Implementations/CollectionService.cs +++ b/src/Core/Services/Implementations/CollectionService.cs @@ -43,9 +43,6 @@ public class CollectionService : ICollectionService _featureService = featureService; } - private bool UseFlexibleCollections => - _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext); - public async Task SaveAsync(Collection collection, IEnumerable groups = null, IEnumerable users = null) { @@ -59,11 +56,11 @@ public class CollectionService : ICollectionService var usersList = users?.ToList(); // If using Flexible Collections - a collection should always have someone with Can Manage permissions - if (UseFlexibleCollections) + if (_featureService.IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, _currentContext)) { var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false; var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false; - if (!groupHasManageAccess && !userHasManageAccess) + if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems) { throw new BadRequestException( "At least one member or group must have can manage permission."); @@ -125,7 +122,10 @@ public class CollectionService : ICollectionService } else { - var collections = await _collectionRepository.GetManyByUserIdAsync(_currentContext.UserId.Value, UseFlexibleCollections); + var collections = await _collectionRepository.GetManyByUserIdAsync( + _currentContext.UserId.Value, + _featureService.IsEnabled(FeatureFlagKeys.FlexibleCollections, _currentContext) + ); orgCollections = collections.Where(c => c.OrganizationId == organizationId); } diff --git a/test/Core.Test/Services/CollectionServiceTests.cs b/test/Core.Test/Services/CollectionServiceTests.cs index ec592f3dd..e9c3acb48 100644 --- a/test/Core.Test/Services/CollectionServiceTests.cs +++ b/test/Core.Test/Services/CollectionServiceTests.cs @@ -114,8 +114,9 @@ public class CollectionServiceTest collection.Id = default; sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); sutProvider.GetDependency() - .IsEnabled(FeatureFlagKeys.FlexibleCollections, Arg.Any(), Arg.Any()) + .IsEnabled(FeatureFlagKeys.FlexibleCollectionsV1, Arg.Any(), Arg.Any()) .Returns(true); + organization.AllowAdminAccessToAllCollectionItems = false; var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.SaveAsync(collection, null, users)); Assert.Contains("At least one member or group must have can manage permission.", ex.Message);