From 1cf82fa0a1a2499554aba699978787134051b5dc Mon Sep 17 00:00:00 2001 From: Herrera Date: Tue, 19 Mar 2024 15:01:14 -0700 Subject: [PATCH 1/3] - updates --- .../api/Services/AcquisitionFileService.cs | 7 +++- .../api/Services/DispositionFileService.cs | 5 ++- source/backend/api/Services/LeaseService.cs | 8 +++- .../AcquisitionFilePropertyRepository.cs | 6 +++ .../DispositionFilePropertyRepository.cs | 6 +++ .../Services/AcquisitionFileServiceTest.cs | 38 +++++++++++++++++++ .../Services/DispositionFileServiceTest.cs | 27 +++++++++++++ .../unit/api/Services/LeaseServiceTest.cs | 32 ++++++++++++++++ .../AcquisitionFilePropertyRepositoryTest.cs | 28 ++++++++++++++ .../DispositionFilePropertyRepositoryTest.cs | 33 ++++++++++++++++ 10 files changed, 187 insertions(+), 3 deletions(-) diff --git a/source/backend/api/Services/AcquisitionFileService.cs b/source/backend/api/Services/AcquisitionFileService.cs index 0fad555e95..9f7b7d7e8e 100644 --- a/source/backend/api/Services/AcquisitionFileService.cs +++ b/source/backend/api/Services/AcquisitionFileService.cs @@ -227,14 +227,19 @@ public PimsAcquisitionFile Add(PimsAcquisitionFile acquisitionFile, IEnumerable< ValidateStaff(acquisitionFile); ValidateOrganizationStaff(acquisitionFile); - acquisitionFile.AcquisitionFileStatusTypeCode = "ACTIVE"; + if (acquisitionFile.PimsPropertyAcquisitionFiles.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } MatchProperties(acquisitionFile, userOverrides); ValidatePropertyRegions(acquisitionFile); PopulateAcquisitionChecklist(acquisitionFile); + acquisitionFile.AcquisitionFileStatusTypeCode = AcquisitionStatusTypes.ACTIVE.ToString(); var newAcqFile = _acqFileRepository.Add(acquisitionFile); _acqFileRepository.CommitTransaction(); + return newAcqFile; } diff --git a/source/backend/api/Services/DispositionFileService.cs b/source/backend/api/Services/DispositionFileService.cs index 1dbfa131ef..e362e9ca65 100644 --- a/source/backend/api/Services/DispositionFileService.cs +++ b/source/backend/api/Services/DispositionFileService.cs @@ -77,8 +77,11 @@ public PimsDispositionFile Add(PimsDispositionFile dispositionFile, IEnumerable< ValidateStaff(dispositionFile); + if (dispositionFile.PimsDispositionFileProperties.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } MatchProperties(dispositionFile, userOverrides); - ValidatePropertyRegions(dispositionFile); var newDispositionFile = _dispositionFileRepository.Add(dispositionFile); diff --git a/source/backend/api/Services/LeaseService.cs b/source/backend/api/Services/LeaseService.cs index a63be14b1c..33f25b7ae3 100644 --- a/source/backend/api/Services/LeaseService.cs +++ b/source/backend/api/Services/LeaseService.cs @@ -5,6 +5,7 @@ using System.Security.Claims; using Microsoft.Extensions.Logging; using Pims.Api.Models.CodeTypes; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Dal.Entities; using Pims.Dal.Entities.Models; @@ -184,6 +185,11 @@ public PimsLease Add(PimsLease lease, IEnumerable userOverride var pimsUser = _userRepository.GetByKeycloakUserId(_user.GetUserKey()); pimsUser.ThrowInvalidAccessToLeaseFile(lease.RegionCode); + if (lease.PimsPropertyLeases.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + var leasesWithProperties = AssociatePropertyLeases(lease, userOverrides); return _leaseRepository.Add(leasesWithProperties); } @@ -237,7 +243,7 @@ public PimsLease Update(PimsLease lease, IEnumerable userOverr List differenceSet = currentProperties.Where(x => !lease.PimsPropertyLeases.Any(y => y.Internal_Id == x.Internal_Id)).ToList(); foreach (var deletedProperty in differenceSet) { - if (deletedProperty.Property.IsPropertyOfInterest == true) + if (deletedProperty.Property.IsPropertyOfInterest) { PimsProperty propertyWithAssociations = _propertyRepository.GetAllAssociationsById(deletedProperty.PropertyId); var leaseAssociationCount = propertyWithAssociations.PimsPropertyLeases.Count; diff --git a/source/backend/dal/Repositories/AcquisitionFilePropertyRepository.cs b/source/backend/dal/Repositories/AcquisitionFilePropertyRepository.cs index 995d6b554e..6ce4a42c48 100644 --- a/source/backend/dal/Repositories/AcquisitionFilePropertyRepository.cs +++ b/source/backend/dal/Repositories/AcquisitionFilePropertyRepository.cs @@ -3,6 +3,7 @@ using System.Security.Claims; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Dal.Entities; @@ -62,6 +63,11 @@ public PimsPropertyAcquisitionFile Add(PimsPropertyAcquisitionFile propertyAcqui { propertyAcquisitionFile.ThrowIfNull(nameof(propertyAcquisitionFile)); + if (propertyAcquisitionFile.Property.IsRetired.HasValue && propertyAcquisitionFile.Property.IsRetired.Value) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + // Mark the property not to be changed if it did not exist already. if (propertyAcquisitionFile.PropertyId != 0) { diff --git a/source/backend/dal/Repositories/DispositionFilePropertyRepository.cs b/source/backend/dal/Repositories/DispositionFilePropertyRepository.cs index 08118c6e88..3da73bf719 100644 --- a/source/backend/dal/Repositories/DispositionFilePropertyRepository.cs +++ b/source/backend/dal/Repositories/DispositionFilePropertyRepository.cs @@ -3,6 +3,7 @@ using System.Security.Claims; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Dal.Entities; @@ -60,6 +61,11 @@ public PimsDispositionFileProperty Add(PimsDispositionFileProperty propertyDispo { propertyDispositionFile.ThrowIfNull(nameof(propertyDispositionFile)); + if (propertyDispositionFile.Property.IsRetired.HasValue && propertyDispositionFile.Property.IsRetired.Value) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + // Mark the property not to be changed if it did not exist already. if (propertyDispositionFile.PropertyId != 0) { diff --git a/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs b/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs index 84cc5bfc1a..b09c04ca1a 100644 --- a/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs @@ -198,6 +198,44 @@ public void Add_DuplicateTeam() act.Should().Throw(); repository.Verify(x => x.Add(It.IsAny()), Times.Never); } + + [Fact] + public void Add_WithRetiredProperty_Should_Fail() + { + // Arrange + var service = this.CreateAcquisitionServiceWithPermissions(Permissions.AcquisitionFileAdd); + + var acqFile = EntityHelper.CreateAcquisitionFile(); + + acqFile.PimsPropertyAcquisitionFiles = new List() + { + new PimsPropertyAcquisitionFile() + { + PropertyId = 100, + Property = new PimsProperty() + { + IsRetired = true, + } + }, + }; + + var repository = this._helper.GetService>(); + repository.Setup(x => x.Add(It.IsAny())).Returns(acqFile); + + var lookupRepository = this._helper.GetService>(); + lookupRepository.Setup(x => x.GetAllRegions()).Returns(new List() { new PimsRegion() { Code = 4, RegionName = "Cannot determine" } }); + + var userRepository = this._helper.GetService>(); + userRepository.Setup(x => x.GetUserInfoByKeycloakUserId(It.IsAny())).Returns(EntityHelper.CreateUser(1, Guid.NewGuid(), "Test", regionCode: 1)); + + // Act + Action act = () => service.Add(acqFile, new List()); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + #endregion #region GetById diff --git a/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs b/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs index 5d19a12290..1a2c09fd12 100644 --- a/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs @@ -313,6 +313,33 @@ public void Add_Success_IsContractor_AssignedToTeam() repository.Verify(x => x.Add(It.IsAny()), Times.Once); } + [Fact] + public void Add_WithRetiredProperty_Should_Fail() + { + // Arrange + var service = this.CreateDispositionServiceWithPermissions(Permissions.DispositionAdd); + + var dispositionFile = EntityHelper.CreateDispositionFile(); + dispositionFile.PimsDispositionFileProperties = new List() + { + new PimsDispositionFileProperty() + { + PropertyId = 100, + Property = new PimsProperty() + { + IsRetired = true, + } + }, + }; + + // Act + Action act = () => service.Add(dispositionFile, new List()); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + #endregion #region Update diff --git a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs index 7c9f926bc6..7720a33da1 100644 --- a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs +++ b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs @@ -8,6 +8,7 @@ using Pims.Api.Constants; using Pims.Api.Models.Concepts; using Pims.Api.Services; +using Pims.Core.Exceptions; using Pims.Core.Test; using Pims.Dal; using Pims.Dal.Entities; @@ -113,6 +114,37 @@ public void Add_InvalidAccessToLeaseFile() leaseRepository.Verify(x => x.Add(It.IsAny()), Times.Never); } + //[Fact] + //public void Add_WithRetiredProperty_Should_Fail() + //{ + // // Arrange + // var lease = EntityHelper.CreateLease(1); + // lease.RegionCode = 1; + + // lease.PimsPropertyLeases + + // var user = new PimsUser(); + // user.PimsRegionUsers.Add(new PimsRegionUser() { RegionCode = lease.RegionCode.Value }); + + // var service = this.CreateLeaseService(Permissions.LeaseAdd); + + // var leaseRepository = this._helper.GetService>(); + // leaseRepository.Setup(x => x.Add(It.IsAny())).Returns(lease); + + // //var propertyRepository = this._helper.GetService>(); + // //propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); + + // //var userRepository = this._helper.GetService>(); + // //userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(user); + + // // Act + // Action act = () => service.Add(lease, new List()); + + // // Assert + // var ex = act.Should().Throw(); + // ex.WithMessage("Retired property can not be selected."); + //} + #endregion #region Update diff --git a/source/backend/tests/unit/dal/Repositories/AcquisitionFilePropertyRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/AcquisitionFilePropertyRepositoryTest.cs index 90f18f3215..bf554716c3 100644 --- a/source/backend/tests/unit/dal/Repositories/AcquisitionFilePropertyRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/AcquisitionFilePropertyRepositoryTest.cs @@ -3,8 +3,10 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using FluentAssertions; +using Pims.Core.Exceptions; using Pims.Core.Test; using Pims.Dal.Entities; +using Pims.Dal.Exceptions; using Pims.Dal.Repositories; using Pims.Dal.Security; using Xunit; @@ -140,6 +142,32 @@ public void Add_ThrowIfNull() // Assert act.Should().Throw(); } + + [Fact] + public void Add_WithRetiredProperty_Should_Fail() + { + // Arrange + var helper = new TestHelper(); + var user = PrincipalHelper.CreateForPermission(Permissions.AcquisitionFileAdd); + + var context = helper.CreatePimsContext(user, true); + var repository = helper.CreateRepository(user); + + var pimsPropertyAcquisitionFile = new PimsPropertyAcquisitionFile() + { + AcquisitionFileId = 1, + PropertyId = 1, + Property = new PimsProperty() { RegionCode = 1, PropertyId = 1, IsRetired = true } + }; + + // Act + Action act = () => repository.Add(pimsPropertyAcquisitionFile); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + #endregion #region Update diff --git a/source/backend/tests/unit/dal/Repositories/DispositionFilePropertyRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/DispositionFilePropertyRepositoryTest.cs index 7391e77a04..b7124a8937 100644 --- a/source/backend/tests/unit/dal/Repositories/DispositionFilePropertyRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/DispositionFilePropertyRepositoryTest.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using FluentAssertions; +using Pims.Core.Exceptions; using Pims.Core.Test; using Pims.Dal.Entities; using Pims.Dal.Repositories; @@ -140,6 +141,38 @@ public void Add_ThrowIfNull() // Assert act.Should().Throw(); } + + [Fact] + public void Add_WithRetiredProperty_Should_Fail() + { + // Arrange + var helper = new TestHelper(); + var user = PrincipalHelper.CreateForPermission(Permissions.DispositionAdd); + + var context = helper.CreatePimsContext(user, true); + var repository = helper.CreateRepository(user); + + var pimsDispositionFileProperty = new PimsDispositionFileProperty() + { + DispositionFileId = 1, + PropertyId = 1, + Property = new PimsProperty() + { + RegionCode = 1, + PropertyId = 1, + IsRetired = true + } + }; + + + // Act + Action act = () => repository.Add(pimsDispositionFileProperty); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + #endregion #region Update From 18d451ec144c1bb98e45e20def382a7edcb92414 Mon Sep 17 00:00:00 2001 From: Herrera Date: Tue, 19 Mar 2024 15:33:42 -0700 Subject: [PATCH 2/3] - test update --- .../unit/api/Services/LeaseServiceTest.cs | 53 +++++++++++-------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs index 7720a33da1..8a9782d85b 100644 --- a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs +++ b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs @@ -3,6 +3,7 @@ using System.Diagnostics.CodeAnalysis; using System.Linq; using FluentAssertions; +using Humanizer; using MapsterMapper; using Moq; using Pims.Api.Constants; @@ -114,36 +115,42 @@ public void Add_InvalidAccessToLeaseFile() leaseRepository.Verify(x => x.Add(It.IsAny()), Times.Never); } - //[Fact] - //public void Add_WithRetiredProperty_Should_Fail() - //{ - // // Arrange - // var lease = EntityHelper.CreateLease(1); - // lease.RegionCode = 1; - - // lease.PimsPropertyLeases + [Fact] + public void Add_WithRetiredProperty_Should_Fail() + { + // Arrange + var lease = EntityHelper.CreateLease(1); + lease.RegionCode = 1; - // var user = new PimsUser(); - // user.PimsRegionUsers.Add(new PimsRegionUser() { RegionCode = lease.RegionCode.Value }); + lease.PimsPropertyLeases = new List() { + new PimsPropertyLease() + { + PropertyId = 100, + Property = new PimsProperty() + { + IsRetired = true, + } + } + }; - // var service = this.CreateLeaseService(Permissions.LeaseAdd); + var user = new PimsUser(); + user.PimsRegionUsers.Add(new PimsRegionUser() { RegionCode = lease.RegionCode.Value }); - // var leaseRepository = this._helper.GetService>(); - // leaseRepository.Setup(x => x.Add(It.IsAny())).Returns(lease); + var service = this.CreateLeaseService(Permissions.LeaseAdd); - // //var propertyRepository = this._helper.GetService>(); - // //propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); + var leaseRepository = this._helper.GetService>(); + leaseRepository.Setup(x => x.Add(It.IsAny())).Returns(lease); - // //var userRepository = this._helper.GetService>(); - // //userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(user); + var userRepository = this._helper.GetService>(); + userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(user); - // // Act - // Action act = () => service.Add(lease, new List()); + // Act + Action act = () => service.Add(lease, new List()); - // // Assert - // var ex = act.Should().Throw(); - // ex.WithMessage("Retired property can not be selected."); - //} + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } #endregion From 2b817f946db67c2ff7fe379c68702dda9aeec300 Mon Sep 17 00:00:00 2001 From: Herrera Date: Wed, 20 Mar 2024 14:03:53 -0700 Subject: [PATCH 3/3] - updates --- .../api/Services/AcquisitionFileService.cs | 4 - .../api/Services/DispositionFileService.cs | 4 - source/backend/api/Services/LeaseService.cs | 14 ++-- .../Repositories/AcquisitionFileRepository.cs | 6 ++ .../Repositories/DispositionFileRepository.cs | 6 ++ .../dal/Repositories/LeaseRepository.cs | 6 ++ .../Services/AcquisitionFileServiceTest.cs | 37 ---------- .../Services/DispositionFileServiceTest.cs | 27 ------- .../unit/api/Services/LeaseServiceTest.cs | 74 +++++++++---------- .../Repositories/AcquisitionRepositoryTest.cs | 33 +++++++++ .../DispositionFileRepositoryTest.cs | 30 ++++++++ .../dal/Repositories/LeaseRepositoryTest.cs | 32 ++++++++ 12 files changed, 158 insertions(+), 115 deletions(-) diff --git a/source/backend/api/Services/AcquisitionFileService.cs b/source/backend/api/Services/AcquisitionFileService.cs index 719f1c1d49..6108f41013 100644 --- a/source/backend/api/Services/AcquisitionFileService.cs +++ b/source/backend/api/Services/AcquisitionFileService.cs @@ -227,10 +227,6 @@ public PimsAcquisitionFile Add(PimsAcquisitionFile acquisitionFile, IEnumerable< ValidateStaff(acquisitionFile); ValidateOrganizationStaff(acquisitionFile); - if (acquisitionFile.PimsPropertyAcquisitionFiles.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) - { - throw new BusinessRuleViolationException("Retired property can not be selected."); - } MatchProperties(acquisitionFile, userOverrides); ValidatePropertyRegions(acquisitionFile); diff --git a/source/backend/api/Services/DispositionFileService.cs b/source/backend/api/Services/DispositionFileService.cs index e362e9ca65..e4b13373ab 100644 --- a/source/backend/api/Services/DispositionFileService.cs +++ b/source/backend/api/Services/DispositionFileService.cs @@ -77,10 +77,6 @@ public PimsDispositionFile Add(PimsDispositionFile dispositionFile, IEnumerable< ValidateStaff(dispositionFile); - if (dispositionFile.PimsDispositionFileProperties.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) - { - throw new BusinessRuleViolationException("Retired property can not be selected."); - } MatchProperties(dispositionFile, userOverrides); ValidatePropertyRegions(dispositionFile); diff --git a/source/backend/api/Services/LeaseService.cs b/source/backend/api/Services/LeaseService.cs index 33f25b7ae3..beac175014 100644 --- a/source/backend/api/Services/LeaseService.cs +++ b/source/backend/api/Services/LeaseService.cs @@ -185,12 +185,8 @@ public PimsLease Add(PimsLease lease, IEnumerable userOverride var pimsUser = _userRepository.GetByKeycloakUserId(_user.GetUserKey()); pimsUser.ThrowInvalidAccessToLeaseFile(lease.RegionCode); - if (lease.PimsPropertyLeases.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) - { - throw new BusinessRuleViolationException("Retired property can not be selected."); - } - var leasesWithProperties = AssociatePropertyLeases(lease, userOverrides); + return _leaseRepository.Add(leasesWithProperties); } @@ -209,12 +205,20 @@ public PimsLease Update(PimsLease lease, IEnumerable userOverr { _logger.LogInformation("Updating lease {leaseId}", lease.LeaseId); _user.ThrowIfNotAuthorized(Permissions.LeaseEdit); + var pimsUser = _userRepository.GetByKeycloakUserId(_user.GetUserKey()); var currentLease = _leaseRepository.GetNoTracking(lease.LeaseId); + pimsUser.ThrowInvalidAccessToLeaseFile(currentLease.RegionCode); // need to check that the user is able to access the current lease as well as has the region for the updated lease. pimsUser.ThrowInvalidAccessToLeaseFile(lease.RegionCode); var currentProperties = _propertyLeaseRepository.GetAllByLeaseId(lease.LeaseId); + var newPropertiesAdded = lease.PimsPropertyLeases.Where(x => !currentProperties.Any(y => y.Internal_Id == x.Internal_Id)).ToList(); + + if(newPropertiesAdded.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } if (currentLease.LeaseStatusTypeCode != lease.LeaseStatusTypeCode) { diff --git a/source/backend/dal/Repositories/AcquisitionFileRepository.cs b/source/backend/dal/Repositories/AcquisitionFileRepository.cs index 1657f96cfd..e3893d1a52 100644 --- a/source/backend/dal/Repositories/AcquisitionFileRepository.cs +++ b/source/backend/dal/Repositories/AcquisitionFileRepository.cs @@ -5,6 +5,7 @@ using LinqKit; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Dal.Entities; using Pims.Dal.Entities.Models; @@ -656,6 +657,11 @@ public PimsAcquisitionFile Add(PimsAcquisitionFile acquisitionFile) using var scope = Logger.QueryScope(); acquisitionFile.ThrowIfNull(nameof(acquisitionFile)); + if (acquisitionFile.PimsPropertyAcquisitionFiles.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + // Existing properties should not be added. foreach (var acquisitionProperty in acquisitionFile.PimsPropertyAcquisitionFiles) { diff --git a/source/backend/dal/Repositories/DispositionFileRepository.cs b/source/backend/dal/Repositories/DispositionFileRepository.cs index 44506928b0..d98e4f2a2d 100644 --- a/source/backend/dal/Repositories/DispositionFileRepository.cs +++ b/source/backend/dal/Repositories/DispositionFileRepository.cs @@ -6,6 +6,7 @@ using LinqKit; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Dal.Entities; using Pims.Dal.Entities.Models; @@ -86,6 +87,11 @@ public PimsDispositionFile Add(PimsDispositionFile disposition) using var scope = Logger.QueryScope(); disposition.ThrowIfNull(nameof(disposition)); + if (disposition.PimsDispositionFileProperties.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + disposition.FileNumber = _sequenceRepository.GetNextSequenceValue(FILENUMBERSEQUENCETABLE).ToString(); Context.PimsDispositionFiles.Add(disposition); diff --git a/source/backend/dal/Repositories/LeaseRepository.cs b/source/backend/dal/Repositories/LeaseRepository.cs index 597cd94434..f16a28ea64 100644 --- a/source/backend/dal/Repositories/LeaseRepository.cs +++ b/source/backend/dal/Repositories/LeaseRepository.cs @@ -5,6 +5,7 @@ using System.Security.Claims; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.Logging; +using Pims.Core.Exceptions; using Pims.Core.Extensions; using Pims.Dal.Entities; using Pims.Dal.Entities.Models; @@ -761,6 +762,11 @@ public PimsLease Add(PimsLease lease) User.ThrowIfNotAuthorized(Permissions.LeaseAdd); + if (lease.PimsPropertyLeases.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + lease = GenerateLFileNo(lease); Context.PimsLeases.Add(lease); diff --git a/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs b/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs index b09c04ca1a..278ece2968 100644 --- a/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs @@ -199,43 +199,6 @@ public void Add_DuplicateTeam() repository.Verify(x => x.Add(It.IsAny()), Times.Never); } - [Fact] - public void Add_WithRetiredProperty_Should_Fail() - { - // Arrange - var service = this.CreateAcquisitionServiceWithPermissions(Permissions.AcquisitionFileAdd); - - var acqFile = EntityHelper.CreateAcquisitionFile(); - - acqFile.PimsPropertyAcquisitionFiles = new List() - { - new PimsPropertyAcquisitionFile() - { - PropertyId = 100, - Property = new PimsProperty() - { - IsRetired = true, - } - }, - }; - - var repository = this._helper.GetService>(); - repository.Setup(x => x.Add(It.IsAny())).Returns(acqFile); - - var lookupRepository = this._helper.GetService>(); - lookupRepository.Setup(x => x.GetAllRegions()).Returns(new List() { new PimsRegion() { Code = 4, RegionName = "Cannot determine" } }); - - var userRepository = this._helper.GetService>(); - userRepository.Setup(x => x.GetUserInfoByKeycloakUserId(It.IsAny())).Returns(EntityHelper.CreateUser(1, Guid.NewGuid(), "Test", regionCode: 1)); - - // Act - Action act = () => service.Add(acqFile, new List()); - - // Assert - var ex = act.Should().Throw(); - ex.WithMessage("Retired property can not be selected."); - } - #endregion #region GetById diff --git a/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs b/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs index 1a2c09fd12..5d19a12290 100644 --- a/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs @@ -313,33 +313,6 @@ public void Add_Success_IsContractor_AssignedToTeam() repository.Verify(x => x.Add(It.IsAny()), Times.Once); } - [Fact] - public void Add_WithRetiredProperty_Should_Fail() - { - // Arrange - var service = this.CreateDispositionServiceWithPermissions(Permissions.DispositionAdd); - - var dispositionFile = EntityHelper.CreateDispositionFile(); - dispositionFile.PimsDispositionFileProperties = new List() - { - new PimsDispositionFileProperty() - { - PropertyId = 100, - Property = new PimsProperty() - { - IsRetired = true, - } - }, - }; - - // Act - Action act = () => service.Add(dispositionFile, new List()); - - // Assert - var ex = act.Should().Throw(); - ex.WithMessage("Retired property can not be selected."); - } - #endregion #region Update diff --git a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs index 8a9782d85b..ad6dd27423 100644 --- a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs +++ b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs @@ -115,43 +115,6 @@ public void Add_InvalidAccessToLeaseFile() leaseRepository.Verify(x => x.Add(It.IsAny()), Times.Never); } - [Fact] - public void Add_WithRetiredProperty_Should_Fail() - { - // Arrange - var lease = EntityHelper.CreateLease(1); - lease.RegionCode = 1; - - lease.PimsPropertyLeases = new List() { - new PimsPropertyLease() - { - PropertyId = 100, - Property = new PimsProperty() - { - IsRetired = true, - } - } - }; - - var user = new PimsUser(); - user.PimsRegionUsers.Add(new PimsRegionUser() { RegionCode = lease.RegionCode.Value }); - - var service = this.CreateLeaseService(Permissions.LeaseAdd); - - var leaseRepository = this._helper.GetService>(); - leaseRepository.Setup(x => x.Add(It.IsAny())).Returns(lease); - - var userRepository = this._helper.GetService>(); - userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(user); - - // Act - Action act = () => service.Add(lease, new List()); - - // Assert - var ex = act.Should().Throw(); - ex.WithMessage("Retired property can not be selected."); - } - #endregion #region Update @@ -232,19 +195,54 @@ public void Update_Properties_Success() var propertyLeaseRepository = this._helper.GetService>(); var propertyRepository = this._helper.GetService>(); var userRepository = this._helper.GetService>(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); leaseRepository.Setup(x => x.GetNoTracking(It.IsAny())).Returns(lease); leaseRepository.Setup(x => x.Get(It.IsAny())).Returns(EntityHelper.CreateLease(1)); userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(new PimsUser()); // Act - var updatedLease = service.Update(lease, new List() { UserOverrideCode.AddLocationToProperty }); // Assert leaseRepository.Verify(x => x.Update(lease, false), Times.Once); } + [Fact] + public void Update_Properties_WithRetiredProperty_Should_Fail() + { + // Arrange + var lease = EntityHelper.CreateLease(1); + + var service = this.CreateLeaseService(Permissions.LeaseEdit, Permissions.LeaseView); + var leaseRepository = this._helper.GetService>(); + var propertyLeaseRepository = this._helper.GetService>(); + var propertyRepository = this._helper.GetService>(); + var userRepository = this._helper.GetService>(); + + propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); + leaseRepository.Setup(x => x.GetNoTracking(It.IsAny())).Returns(lease); + leaseRepository.Setup(x => x.Get(It.IsAny())).Returns(EntityHelper.CreateLease(1)); + userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(new PimsUser()); + + lease.PimsPropertyLeases.Add(new PimsPropertyLease() + { + PropertyId = 100, + Property = new PimsProperty() + { + Pid = 1, + IsRetired = true, + } + }); + + // Act + Action act = () => service.Update(lease, new List() { UserOverrideCode.AddLocationToProperty }); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + [Fact] public void Update_Properties_Delete_Success() { diff --git a/source/backend/tests/unit/dal/Repositories/AcquisitionRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/AcquisitionRepositoryTest.cs index b7369a2cdd..fa9797eb54 100644 --- a/source/backend/tests/unit/dal/Repositories/AcquisitionRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/AcquisitionRepositoryTest.cs @@ -220,6 +220,39 @@ public void Add_ThrowIfNull() // Assert act.Should().Throw(); } + + + [Fact] + public void Add_WithRetiredProperty_Should_Fail() + { + var helper = new TestHelper(); + var user = PrincipalHelper.CreateForPermission(Permissions.AcquisitionFileAdd); + helper.CreatePimsContext(user, true); + + var repository = helper.CreateRepository(user); + + var acqFile = EntityHelper.CreateAcquisitionFile(); + acqFile.PimsPropertyAcquisitionFiles = new List() + { + new PimsPropertyAcquisitionFile() + { + PropertyId = 100, + Property = new PimsProperty() + { + IsRetired = true, + } + }, + }; + + // Act + Action act = () => repository.Add(acqFile); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + + #endregion #region GetById diff --git a/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs index 0e3efba843..d17709d75d 100644 --- a/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs @@ -64,6 +64,36 @@ public void Add_Success() result.DispositionFileId.Should().Be(1); result.FileNumber.Equals("D-100"); } + + [Fact] + public void Add__WithRetiredProperty_Should_Fail() + { + // Arrange + var helper = new TestHelper(); + var user = PrincipalHelper.CreateForPermission(Permissions.DispositionAdd); + var dispositionFile = EntityHelper.CreateDispositionFile(); + + dispositionFile.PimsDispositionFileProperties = new List() + { + new PimsDispositionFileProperty() + { + PropertyId = 100, + Property = new PimsProperty() + { + IsRetired = true, + } + }, + }; + + var repository = helper.CreateRepository(user); + + // Act + Action act = () => repository.Add(dispositionFile); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } #endregion #region Update diff --git a/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs index e7d8a08c2f..084b5b7d29 100644 --- a/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs @@ -7,6 +7,7 @@ using Moq; using NSubstitute; using Pims.Api.Services; +using Pims.Core.Exceptions; using Pims.Core.Test; using Pims.Dal.Entities; using Pims.Dal.Entities.Models; @@ -218,6 +219,37 @@ public void Add_Lease_ThrowIfNull() // Assert act.Should().Throw(); } + + [Fact] + public void Add_Lease_WithRetiredProperty_Should_Fail() + { + var helper = new TestHelper(); + var user = PrincipalHelper.CreateForPermission(Permissions.LeaseAdd, Permissions.LeaseView); + helper.CreatePimsContext(user, true); + + var repository = helper.CreateRepository(user); + + var lease = EntityHelper.CreateLease(1); + lease.PimsPropertyLeases = new List() { + new PimsPropertyLease() + { + PropertyId = 100, + Property = new PimsProperty() + { + Pid = 1, + IsRetired = true, + } + } + }; + + // Act + Action act = () => repository.Add(lease); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + #endregion #region GetLastUpdateBy