PSP-7886 : Prevent users from adding new takes, and/or deleting compl…

…eted takes from a non-active (e.g. draft or active) acquisition file (#3923) Co-authored-by: Herrera <[email protected]>
bcgov · Apr 4, 2024 · affbf03 · affbf03
1 parent afce13a
commit affbf03
Show file tree

Hide file tree

Showing 13 changed files with 262 additions and 120 deletions.
diff --git a/source/backend/api/Services/TakeService.cs b/source/backend/api/Services/TakeService.cs
@@ -1,8 +1,8 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Security.Claims;
 using Microsoft.Extensions.Logging;
-using Pims.Api.Constants;
 using Pims.Api.Models.CodeTypes;
 using Pims.Core.Exceptions;
 using Pims.Dal.Entities;
@@ -61,11 +61,35 @@ public IEnumerable<PimsTake> UpdateAcquisitionPropertyTakes(long acquisitionFile
             _user.ThrowIfNotAuthorized(Permissions.PropertyView, Permissions.AcquisitionFileView);
 
             var currentAcquistionFile = _acqFileRepository.GetByAcquisitionFilePropertyId(acquisitionFilePropertyId);
+            var currentFilePropertyTakes = _takeRepository.GetAllByPropertyAcquisitionFileId(acquisitionFilePropertyId);
 
             var currentAcquisitionStatus = Enum.Parse<AcquisitionStatusTypes>(currentAcquistionFile.AcquisitionFileStatusTypeCode);
-            if (!_statusSolver.CanEditTakes(currentAcquisitionStatus) && !_user.HasPermission(Permissions.SystemAdmin))
+
+            // No user can update the takes when the File is not Active/Draft
+            if (!_statusSolver.CanEditTakes(currentAcquisitionStatus))
+            {
+                throw new BusinessRuleViolationException("Retired records are referenced for historical purposes only and cannot be edited or deleted. If the take has been added in error, contact your system administrator to re-open the file, which will allow take deletion.");
+            }
+            else
             {
-                throw new BusinessRuleViolationException("The file you are editing is not active or draft, so you cannot save changes. Refresh your browser to see file state.");
+                // Complete Takes can only be deleted or set to InProgress by Admins when File is Active/Draft
+                if (!_user.HasPermission(Permissions.SystemAdmin))
+                {
+                    var currentCompleteTakes = currentFilePropertyTakes
+                        .Where(x => x.TakeStatusTypeCode == AcquisitionTakeStatusTypes.COMPLETE.ToString()).ToList();
+
+                    if (currentCompleteTakes.Count > 0)
+                    {
+                        foreach (var completeTake in currentCompleteTakes)
+                        {
+                            var updatedTake = takes.FirstOrDefault(x => x.TakeId == completeTake.TakeId);
+                            if (updatedTake is null || updatedTake?.TakeStatusTypeCode != completeTake.TakeStatusTypeCode)
+                            {
+                                throw new BusinessRuleViolationException("Retired records are referenced for historical purposes only and cannot be edited or deleted. If the take has been added in error, contact your system administrator to re-open the file, which will allow take deletion.");
+                            }
+                        }
+                    }
+                }
             }
 
             _takeRepository.UpdateAcquisitionPropertyTakes(acquisitionFilePropertyId, takes);

diff --git a/source/backend/tests/unit/api/Services/TakeServiceTest.cs b/source/backend/tests/unit/api/Services/TakeServiceTest.cs
@@ -157,7 +157,7 @@ public void Update_NoPermission()
         }
 
         [Fact]
-        public void Update_InvalidStatus()
+        public void Update_InvalidStatus_AcquisitionFile_Complete()
         {
             // Arrange
             var service = this.CreateWithPermissions(Permissions.PropertyView, Permissions.AcquisitionFileView);
@@ -175,7 +175,64 @@ public void Update_InvalidStatus()
             Action act = () => service.UpdateAcquisitionPropertyTakes(1, new List<PimsTake>());
 
             // Assert
-            act.Should().Throw<BusinessRuleViolationException>();
+            act.Should().Throw<BusinessRuleViolationException>().WithMessage("Retired records are referenced for historical purposes only and cannot be edited or deleted. If the take has been added in error, contact your system administrator to re-open the file, which will allow take deletion.");
+        }
+
+        [Fact]
+        public void Update_InvalidStatus_AcquisitionFile_Active_DeleteCompleteTake_NotAdmin()
+        {
+            // Arrange
+            var service = this.CreateWithPermissions(Permissions.PropertyView, Permissions.AcquisitionFileView);
+
+            var acqRepository = this._helper.GetService<Mock<IAcquisitionFileRepository>>();
+            acqRepository.Setup(x => x.GetByAcquisitionFilePropertyId(It.IsAny<long>())).Returns(new PimsAcquisitionFile() { AcquisitionFileStatusTypeCode = AcquisitionStatusTypes.ACTIVE.ToString() });
+
+            PimsTake completedTake = new()
+            {
+                TakeId = 100,
+                TakeStatusTypeCode = AcquisitionTakeStatusTypes.COMPLETE.ToString(),
+            };
+
+            var takeRepository = this._helper.GetService<Mock<ITakeRepository>>();
+            takeRepository.Setup(x => x.GetAllByPropertyAcquisitionFileId(It.IsAny<long>())).Returns(new List<PimsTake>() { completedTake });
+
+            var solver = this._helper.GetService<Mock<IAcquisitionStatusSolver>>();
+            solver.Setup(x => x.CanEditTakes(It.IsAny<AcquisitionStatusTypes?>())).Returns(true);
+
+            // Act
+            Action act = () => service.UpdateAcquisitionPropertyTakes(1, new List<PimsTake>());
+
+            // Assert
+            act.Should().Throw<BusinessRuleViolationException>().WithMessage("Retired records are referenced for historical purposes only and cannot be edited or deleted. If the take has been added in error, contact your system administrator to re-open the file, which will allow take deletion.");
+        }
+
+        [Fact]
+        public void Update_AcquisitionFile_Active_DeleteCompleteTake_Admin_Success()
+        {
+            // Arrange
+            var service = this.CreateWithPermissions(Permissions.SystemAdmin, Permissions.PropertyView, Permissions.AcquisitionFileView);
+
+            var acqRepository = this._helper.GetService<Mock<IAcquisitionFileRepository>>();
+            acqRepository.Setup(x => x.GetByAcquisitionFilePropertyId(It.IsAny<long>())).Returns(new PimsAcquisitionFile() { AcquisitionFileStatusTypeCode = AcquisitionStatusTypes.ACTIVE.ToString() });
+
+            PimsTake completedTake = new()
+            {
+                TakeId = 100,
+                TakeStatusTypeCode = AcquisitionTakeStatusTypes.COMPLETE.ToString(),
+            };
+
+            var takeRepository = this._helper.GetService<Mock<ITakeRepository>>();
+            takeRepository.Setup(x => x.GetAllByPropertyAcquisitionFileId(It.IsAny<long>())).Returns(new List<PimsTake>() { completedTake });
+
+            var solver = this._helper.GetService<Mock<IAcquisitionStatusSolver>>();
+            solver.Setup(x => x.CanEditTakes(It.IsAny<AcquisitionStatusTypes?>())).Returns(true);
+
+            // Act
+            var result = service.UpdateAcquisitionPropertyTakes(1, new List<PimsTake>());
+
+            // Assert
+            Assert.NotNull(result);
+            takeRepository.Verify(x => x.UpdateAcquisitionPropertyTakes(1, new List<PimsTake>()), Times.Once);
         }
     }
 }
diff --git a/source/frontend/src/constants/acquisitionFileStatus.ts b/source/frontend/src/constants/acquisitionFileStatus.ts
diff --git a/source/frontend/src/features/mapSideBar/acquisition/common/AcquisitionMenu.test.tsx b/source/frontend/src/features/mapSideBar/acquisition/common/AcquisitionMenu.test.tsx
@@ -1,10 +1,10 @@
-import { AcquisitionStatus } from '@/constants/acquisitionFileStatus';
 import { Claims, Roles } from '@/constants/index';
 import { mockAcquisitionFileResponse } from '@/mocks/acquisitionFiles.mock';
 import { toTypeCode } from '@/utils/formUtils';
 import { act, render, RenderOptions, userEvent } from '@/utils/test-utils';
 
 import AcquisitionMenu, { IAcquisitionMenuProps } from './AcquisitionMenu';
+import { ApiGen_CodeTypes_AcquisitionStatusTypes } from '@/models/api/generated/ApiGen_CodeTypes_AcquisitionStatusTypes';
 
 // mock auth library
 jest.mock('@react-keycloak/web');
@@ -129,7 +129,7 @@ describe('AcquisitionMenu component', () => {
       {
         acquisitionFile: {
           ...mockAcquisitionFileResponse(),
-          fileStatusTypeCode: toTypeCode(AcquisitionStatus.Complete),
+          fileStatusTypeCode: toTypeCode(ApiGen_CodeTypes_AcquisitionStatusTypes.COMPLT),
         },
         items: testData,
         selectedIndex: 1,
@@ -148,7 +148,7 @@ describe('AcquisitionMenu component', () => {
       {
         acquisitionFile: {
           ...mockAcquisitionFileResponse(),
-          fileStatusTypeCode: toTypeCode(AcquisitionStatus.Complete),
+          fileStatusTypeCode: toTypeCode(ApiGen_CodeTypes_AcquisitionStatusTypes.COMPLT),
         },
         items: testData,
         selectedIndex: 1,

diff --git a/...apSideBar/acquisition/tabs/compensation/detail/CompensationRequisitionDetailView.test.tsx b/...apSideBar/acquisition/tabs/compensation/detail/CompensationRequisitionDetailView.test.tsx
@@ -1,6 +1,5 @@
 import { createMemoryHistory } from 'history';
 
-import { AcquisitionStatus } from '@/constants/acquisitionFileStatus';
 import Claims from '@/constants/claims';
 import { Roles } from '@/constants/index';
 import { mockAcquisitionFileResponse } from '@/mocks/acquisitionFiles.mock';
@@ -15,6 +14,7 @@ import { act, render, RenderOptions, userEvent, waitFor } from '@/utils/test-uti
 import CompensationRequisitionDetailView, {
   CompensationRequisitionDetailViewProps,
 } from './CompensationRequisitionDetailView';
+import { ApiGen_CodeTypes_AcquisitionStatusTypes } from '@/models/api/generated/ApiGen_CodeTypes_AcquisitionStatusTypes';
 
 const setEditMode = jest.fn();
 
@@ -128,7 +128,7 @@ describe('Compensation Detail View Component', () => {
       props: {
         acquisitionFile: {
           ...acquistionFile,
-          fileStatusTypeCode: toTypeCodeNullable(AcquisitionStatus.Active),
+          fileStatusTypeCode: toTypeCodeNullable(ApiGen_CodeTypes_AcquisitionStatusTypes.ACTIVE),
         },
         compensation: { ...mockFinalCompensation, isDraft: true },
       },
@@ -164,7 +164,7 @@ describe('Compensation Detail View Component', () => {
       props: {
         acquisitionFile: {
           ...acquistionFile,
-          fileStatusTypeCode: toTypeCodeNullable(AcquisitionStatus.Complete),
+          fileStatusTypeCode: toTypeCodeNullable(ApiGen_CodeTypes_AcquisitionStatusTypes.COMPLT),
         },
         compensation: { ...mockFinalCompensation, isDraft: false },
       },

diff --git a/.../src/features/mapSideBar/acquisition/tabs/compensation/list/CompensationListView.test.tsx b/.../src/features/mapSideBar/acquisition/tabs/compensation/list/CompensationListView.test.tsx
@@ -1,6 +1,5 @@
 import { createMemoryHistory } from 'history';
 
-import { AcquisitionStatus } from '@/constants/acquisitionFileStatus';
 import Claims from '@/constants/claims';
 import Roles from '@/constants/roles';
 import {
@@ -15,6 +14,7 @@ import { toTypeCode, toTypeCodeNullable } from '@/utils/formUtils';
 import { act, render, RenderOptions, userEvent, waitFor } from '@/utils/test-utils';
 
 import CompensationListView, { ICompensationListViewProps } from './CompensationListView';
+import { ApiGen_CodeTypes_AcquisitionStatusTypes } from '@/models/api/generated/ApiGen_CodeTypes_AcquisitionStatusTypes';
 
 const storeState = {
   [lookupCodesSlice.name]: { lookupCodes: mockLookups },
@@ -118,7 +118,7 @@ describe('compensation list view', () => {
     const { findAllByTitle } = setup({
       acquisitionFile: {
         ...mockAcquisitionFileResponse(),
-        fileStatusTypeCode: toTypeCodeNullable(AcquisitionStatus.Active),
+        fileStatusTypeCode: toTypeCodeNullable(ApiGen_CodeTypes_AcquisitionStatusTypes.ACTIVE),
       },
       compensations: compensations,
       claims: [Claims.COMPENSATION_REQUISITION_DELETE],
@@ -136,7 +136,7 @@ describe('compensation list view', () => {
     const { queryByTestId } = setup({
       acquisitionFile: {
         ...mockAcquisitionFileResponse(),
-        fileStatusTypeCode: toTypeCode(AcquisitionStatus.Active),
+        fileStatusTypeCode: toTypeCode(ApiGen_CodeTypes_AcquisitionStatusTypes.ACTIVE),
       },
       compensations: compensations,
       claims: [Claims.COMPENSATION_REQUISITION_DELETE],
@@ -151,7 +151,7 @@ describe('compensation list view', () => {
     const { queryByTestId } = setup({
       acquisitionFile: {
         ...mockAcquisitionFileResponse(),
-        fileStatusTypeCode: toTypeCode(AcquisitionStatus.Active),
+        fileStatusTypeCode: toTypeCode(ApiGen_CodeTypes_AcquisitionStatusTypes.ACTIVE),
       },
       compensations: compensations,
       claims: [Claims.COMPENSATION_REQUISITION_DELETE],