From 8228044880dcfd4b751c33f744bb297206bb2e67 Mon Sep 17 00:00:00 2001 From: Herrera Date: Sun, 24 Mar 2024 18:48:42 -0700 Subject: [PATCH 1/3] PSP-8051 : FT:User is able to add the retired property record to the Acquisition&lease/license files --- .../api/Services/AcquisitionFileService.cs | 12 +- .../api/Services/DispositionFileService.cs | 16 ++- source/backend/api/Services/LeaseService.cs | 26 +++-- .../DispositionFilePropertyModel.cs | 2 - .../Repositories/AcquisitionFileRepository.cs | 2 +- .../Repositories/DispositionFileRepository.cs | 2 +- .../Interfaces/IPropertyRepository.cs | 2 +- .../dal/Repositories/LeaseRepository.cs | 2 +- .../dal/Repositories/PropertyRepository.cs | 65 ++++++----- .../Services/AcquisitionFileServiceTest.cs | 107 ++++++++++++++++-- .../Services/DispositionFileServiceTest.cs | 95 ++++++++++++++-- .../unit/api/Services/LeaseServiceTest.cs | 67 ++++++++--- .../api/Services/ResearchFileServiceTest.cs | 4 +- .../DispositionFileRepositoryTest.cs | 2 +- .../dal/Repositories/LeaseRepositoryTest.cs | 6 +- 15 files changed, 321 insertions(+), 89 deletions(-) diff --git a/source/backend/api/Services/AcquisitionFileService.cs b/source/backend/api/Services/AcquisitionFileService.cs index 6108f41013..61b06767e7 100644 --- a/source/backend/api/Services/AcquisitionFileService.cs +++ b/source/backend/api/Services/AcquisitionFileService.cs @@ -626,7 +626,12 @@ private void MatchProperties(PimsAcquisitionFile acquisitionFile, IEnumerable userOverr 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) { - _entityNoteRepository.Add( + _entityNoteRepository.Add( new PimsLeaseNote() { LeaseId = currentLease.LeaseId, @@ -300,6 +292,7 @@ private PimsLease AssociatePropertyLeases(PimsLease lease, IEnumerable p.LeaseId != lease.Internal_Id); var isPropertyOnThisLease = existingPropertyLeases.Any(p => p.LeaseId == lease.Internal_Id); + if (isPropertyOnOtherLease && !isPropertyOnThisLease && !userOverrides.Contains(UserOverrideCode.AddPropertyToInventory)) { var genericOverrideErrorMsg = $"is attached to L-File # {existingPropertyLeases.FirstOrDefault().Lease.LFileNo}"; @@ -313,6 +306,7 @@ private PimsLease AssociatePropertyLeases(PimsLease lease, IEnumerable user var pid = leaseProperty.Property.Pid.Value; try { - var foundProperty = _propertyRepository.GetByPid(pid); + var foundProperty = _propertyRepository.GetByPid(pid, true); + if (foundProperty.IsRetired.HasValue && foundProperty.IsRetired.Value) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + leaseProperty.PropertyId = foundProperty.Internal_Id; UpdateLocation(leaseProperty.Property, ref foundProperty, userOverrides); leaseProperty.Property = foundProperty; @@ -383,6 +382,11 @@ private void MatchProperties(PimsLease lease, IEnumerable user try { var foundProperty = _propertyRepository.GetByPin(pin); + if (foundProperty.IsRetired.HasValue && foundProperty.IsRetired.Value) + { + throw new BusinessRuleViolationException("Retired property can not be selected."); + } + leaseProperty.PropertyId = foundProperty.Internal_Id; UpdateLocation(leaseProperty.Property, ref foundProperty, userOverrides); leaseProperty.Property = foundProperty; diff --git a/source/backend/apimodels/Models/Concepts/DispositionFile/DispositionFilePropertyModel.cs b/source/backend/apimodels/Models/Concepts/DispositionFile/DispositionFilePropertyModel.cs index f647b93c6b..1c6b86b04d 100644 --- a/source/backend/apimodels/Models/Concepts/DispositionFile/DispositionFilePropertyModel.cs +++ b/source/backend/apimodels/Models/Concepts/DispositionFile/DispositionFilePropertyModel.cs @@ -1,6 +1,4 @@ using Pims.Api.Models.Concepts.File; -using Pims.Api.Models.Base; -using Pims.Api.Models.Concepts.Property; namespace Pims.Api.Models.Concepts.DispositionFile { diff --git a/source/backend/dal/Repositories/AcquisitionFileRepository.cs b/source/backend/dal/Repositories/AcquisitionFileRepository.cs index e3893d1a52..818a017d2c 100644 --- a/source/backend/dal/Repositories/AcquisitionFileRepository.cs +++ b/source/backend/dal/Repositories/AcquisitionFileRepository.cs @@ -657,7 +657,7 @@ 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)) + if (acquisitionFile.PimsPropertyAcquisitionFiles.Any(x =>x.Property != null && x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) { throw new BusinessRuleViolationException("Retired property can not be selected."); } diff --git a/source/backend/dal/Repositories/DispositionFileRepository.cs b/source/backend/dal/Repositories/DispositionFileRepository.cs index d98e4f2a2d..7c272b6fa6 100644 --- a/source/backend/dal/Repositories/DispositionFileRepository.cs +++ b/source/backend/dal/Repositories/DispositionFileRepository.cs @@ -87,7 +87,7 @@ 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)) + if (disposition.PimsDispositionFileProperties.Any(x => x.Property != null && x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) { throw new BusinessRuleViolationException("Retired property can not be selected."); } diff --git a/source/backend/dal/Repositories/Interfaces/IPropertyRepository.cs b/source/backend/dal/Repositories/Interfaces/IPropertyRepository.cs index 1eacb25cf7..acd8b9430a 100644 --- a/source/backend/dal/Repositories/Interfaces/IPropertyRepository.cs +++ b/source/backend/dal/Repositories/Interfaces/IPropertyRepository.cs @@ -20,7 +20,7 @@ public interface IPropertyRepository : IRepository PimsProperty GetByPid(string pid); - PimsProperty GetByPid(int pid); + PimsProperty GetByPid(int pid, bool includeRetired = false); PimsProperty GetByPin(int pin); diff --git a/source/backend/dal/Repositories/LeaseRepository.cs b/source/backend/dal/Repositories/LeaseRepository.cs index f16a28ea64..fe37a66c16 100644 --- a/source/backend/dal/Repositories/LeaseRepository.cs +++ b/source/backend/dal/Repositories/LeaseRepository.cs @@ -762,7 +762,7 @@ public PimsLease Add(PimsLease lease) User.ThrowIfNotAuthorized(Permissions.LeaseAdd); - if (lease.PimsPropertyLeases.Any(x => x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) + if (lease.PimsPropertyLeases.Any(x => x.Property != null && x.Property.IsRetired.HasValue && x.Property.IsRetired.Value)) { throw new BusinessRuleViolationException("Retired property can not be selected."); } diff --git a/source/backend/dal/Repositories/PropertyRepository.cs b/source/backend/dal/Repositories/PropertyRepository.cs index 52b42feeb5..a1e319d669 100644 --- a/source/backend/dal/Repositories/PropertyRepository.cs +++ b/source/backend/dal/Repositories/PropertyRepository.cs @@ -185,38 +185,47 @@ public PimsProperty GetByPid(string pid) /// Get the property for the specified PID value. /// /// + /// /// - public PimsProperty GetByPid(int pid) + public PimsProperty GetByPid(int pid, bool includeRetired = false) { this.User.ThrowIfNotAllAuthorized(Permissions.PropertyView); - var property = this.Context.PimsProperties.AsNoTracking() - .Include(p => p.DistrictCodeNavigation) - .Include(p => p.RegionCodeNavigation) - .Include(p => p.PropertyTypeCodeNavigation) - .Include(p => p.PropertyStatusTypeCodeNavigation) - .Include(p => p.PropertyDataSourceTypeCodeNavigation) - .Include(p => p.PropertyClassificationTypeCodeNavigation) - .Include(p => p.PimsPropPropAnomalyTypes) - .ThenInclude(t => t.PropertyAnomalyTypeCodeNavigation) - .Include(p => p.PimsPropPropRoadTypes) - .ThenInclude(t => t.PropertyRoadTypeCodeNavigation) - .Include(p => p.PimsPropPropTenureTypes) - .ThenInclude(t => t.PropertyTenureTypeCodeNavigation) - .Include(p => p.PropertyAreaUnitTypeCodeNavigation) - .Include(p => p.VolumetricTypeCodeNavigation) - .Include(p => p.VolumeUnitTypeCodeNavigation) - .Include(p => p.Address) - .ThenInclude(a => a.RegionCodeNavigation) - .Include(p => p.Address) - .ThenInclude(a => a.DistrictCodeNavigation) - .Include(p => p.Address) - .ThenInclude(a => a.ProvinceState) - .Include(p => p.Address) - .ThenInclude(a => a.Country) - .OrderByDescending(p => p.PropertyId) - .FirstOrDefault(p => p.Pid == pid && p.IsRetired != true) ?? throw new KeyNotFoundException(); - return property; + var query = Context.PimsProperties.AsNoTracking(); + + if(includeRetired) + { + query = query.Where(r => r.IsRetired.HasValue && r.IsRetired.Value); + } + else + { + query = query.Where(r => !r.IsRetired.HasValue || (r.IsRetired.HasValue && !r.IsRetired.Value)); + } + + return query.Include(p => p.DistrictCodeNavigation) + .Include(p => p.RegionCodeNavigation) + .Include(p => p.PropertyTypeCodeNavigation) + .Include(p => p.PropertyStatusTypeCodeNavigation) + .Include(p => p.PropertyDataSourceTypeCodeNavigation) + .Include(p => p.PropertyClassificationTypeCodeNavigation) + .Include(p => p.PimsPropPropAnomalyTypes) + .ThenInclude(t => t.PropertyAnomalyTypeCodeNavigation) + .Include(p => p.PimsPropPropRoadTypes) + .ThenInclude(t => t.PropertyRoadTypeCodeNavigation) + .Include(p => p.PimsPropPropTenureTypes) + .ThenInclude(t => t.PropertyTenureTypeCodeNavigation) + .Include(p => p.PropertyAreaUnitTypeCodeNavigation) + .Include(p => p.VolumetricTypeCodeNavigation) + .Include(p => p.VolumeUnitTypeCodeNavigation) + .Include(p => p.Address) + .ThenInclude(a => a.RegionCodeNavigation) + .Include(p => p.Address) + .ThenInclude(a => a.DistrictCodeNavigation) + .Include(p => p.Address) + .ThenInclude(a => a.ProvinceState) + .Include(p => p.Address) + .ThenInclude(a => a.Country) + .OrderByDescending(p => p.PropertyId).FirstOrDefault(p => p.Pid == pid) ?? throw new KeyNotFoundException(); } /// diff --git a/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs b/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs index 278ece2968..fa09ec97a6 100644 --- a/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/AcquisitionFileServiceTest.cs @@ -199,6 +199,45 @@ 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(); + PimsProperty property = new PimsProperty() + { + PropertyId = 100, + Pid = 1000, + IsRetired = true, + }; + + acqFile.PimsPropertyAcquisitionFiles.Add(new PimsPropertyAcquisitionFile() + { + Property = property, + }); + + 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)); + + var propertyRepository = this._helper.GetService>(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); + + // 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 @@ -1311,7 +1350,7 @@ public void UpdateProperties_Takes_Violation() repository.Setup(x => x.GetById(It.IsAny())).Returns(acqFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(property); var filePropertyRepository = this._helper.GetService>(); filePropertyRepository.Setup(x => x.GetPropertiesByAcquisitionFileId(It.IsAny())).Returns(acqFile.PimsPropertyAcquisitionFiles.ToList()); @@ -1343,7 +1382,7 @@ public void UpdateProperties_MatchProperties_Success() repository.Setup(x => x.GetById(It.IsAny())).Returns(acqFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var filePropertyRepository = this._helper.GetService>(); @@ -1384,7 +1423,7 @@ public void UpdateProperties_MatchProperties_NewProperty_Success() filePropertyRepository.Setup(x => x.Add(It.IsAny())).Callback(x => updatedAcquisitionFileProperty = x).Returns(acqFile.PimsPropertyAcquisitionFiles.FirstOrDefault()); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Throws(); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var coordinateService = this._helper.GetService>(); @@ -1446,7 +1485,7 @@ public void UpdateProperties_UpdatePropertyFile_Success() filePropertyRepository.Setup(x => x.GetPropertiesByAcquisitionFileId(It.IsAny())).Returns(new List() { new PimsPropertyAcquisitionFile() { Internal_Id = 1, Property = property, PropertyName = "updated" } }); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Throws(); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var userRepository = this._helper.GetService>(); @@ -1481,7 +1520,7 @@ public void UpdateProperties_RemovePropertyFile_Success() filePropertyRepository.Setup(x => x.GetPropertiesByAcquisitionFileId(It.IsAny())).Returns(new List() { new PimsPropertyAcquisitionFile() { Property = property } }); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); var userRepository = this._helper.GetService>(); userRepository.Setup(x => x.GetUserInfoByKeycloakUserId(It.IsAny())).Returns(EntityHelper.CreateUser("Test")); @@ -1520,7 +1559,7 @@ public void UpdateProperties_RemoveProperty_Success() filePropertyRepository.Setup(x => x.GetAcquisitionFilePropertyRelatedCount(It.IsAny())).Returns(1); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); propertyRepository.Setup(x => x.GetAllAssociationsById(It.IsAny())).Returns(property); var userRepository = this._helper.GetService>(); @@ -1604,7 +1643,7 @@ public void UpdateProperties_ExistingTakes() filePropertyRepository.Setup(x => x.GetAcquisitionFilePropertyRelatedCount(It.IsAny())).Returns(1); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); propertyRepository.Setup(x => x.GetAllAssociationsById(It.IsAny())).Returns(property); var userRepository = this._helper.GetService>(); @@ -1638,7 +1677,7 @@ public void UpdateProperties_ExistingInterestHolders() filePropertyRepository.Setup(x => x.GetAcquisitionFilePropertyRelatedCount(It.IsAny())).Returns(1); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); propertyRepository.Setup(x => x.GetAllAssociationsById(It.IsAny())).Returns(property); var userRepository = this._helper.GetService>(); @@ -1696,7 +1735,7 @@ public void UpdateProperties_ValidatePropertyRegions_Success() repository.Setup(x => x.GetById(It.IsAny())).Returns(acqFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var filePropertyRepository = this._helper.GetService>(); @@ -1732,7 +1771,7 @@ public void UpdateProperties_ValidatePropertyRegions_Error() repository.Setup(x => x.GetById(It.IsAny())).Returns(acqFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(3); var filePropertyRepository = this._helper.GetService>(); @@ -1748,6 +1787,54 @@ public void UpdateProperties_ValidatePropertyRegions_Error() var exception = act.Should().Throw(); exception.WithMessage("You cannot add a property that is outside of your user account region(s)*"); // partial match on the error message - as documented by FluentAssertions } + + + [Fact] + public void UpdateProperties_WithRetiredProperty_Should_Fail() + { + // Arrange + var service = this.CreateAcquisitionServiceWithPermissions(Permissions.AcquisitionFileEdit, Permissions.PropertyAdd, Permissions.PropertyView); + + var acqFile = EntityHelper.CreateAcquisitionFile(); + acqFile.ConcurrencyControlNumber = 1; + + PimsProperty property = EntityHelper.CreateProperty(12345, regionCode: 3); + PimsProperty retiredProperty = new PimsProperty() + { + PropertyId = 100, + Pid = 1000, + IsRetired = true, + }; + + acqFile.PimsPropertyAcquisitionFiles = new List() { + new() + { + Property = retiredProperty + } + }; + + + var repository = this._helper.GetService>(); + repository.Setup(x => x.GetRowVersion(It.IsAny())).Returns(1); + repository.Setup(x => x.GetById(It.IsAny())).Returns(acqFile); + + var propertyRepository = this._helper.GetService>(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(retiredProperty); + propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(3); + + var filePropertyRepository = this._helper.GetService>(); + filePropertyRepository.Setup(x => x.GetPropertiesByAcquisitionFileId(It.IsAny())).Returns(acqFile.PimsPropertyAcquisitionFiles.ToList()); + + 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.UpdateProperties(acqFile, new List()); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } #endregion #region Checklist diff --git a/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs b/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs index 874444e501..38cbe05ff0 100644 --- a/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs @@ -313,6 +313,42 @@ 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(); + var pimsProperty = EntityHelper.CreateProperty(1000, isRetired: true); + + dispositionFile.PimsDispositionFileProperties = new List() + { + new PimsDispositionFileProperty() + { + DispositionFilePropertyId = 0, + Property = pimsProperty, + }, + }; + + var repository = this._helper.GetService>(); + repository.Setup(x => x.Add(It.IsAny())).Returns(dispositionFile); + + var lookupRepository = this._helper.GetService>(); + lookupRepository.Setup(x => x.GetAllRegions()).Returns(new List() { new PimsRegion() { Code = 4, RegionName = "Cannot determine" } }); + + var propertyRepository = this._helper.GetService>(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(pimsProperty); + + // 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 @@ -865,7 +901,7 @@ public void UpdateProperties_MatchProperties_Success() repository.Setup(x => x.GetById(It.IsAny())).Returns(dspFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var filePropertyRepository = this._helper.GetService>(); @@ -903,7 +939,7 @@ public void UpdateProperties_MatchProperties_NewProperty_UserOverride() filePropertyRepository.Setup(x => x.Add(It.IsAny())).Callback(x => updatedDispositionFileProperty = x).Returns(dspFile.PimsDispositionFileProperties.FirstOrDefault()); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Throws(); var coordinateService = this._helper.GetService>(); coordinateService.Setup(x => x.TransformCoordinates(It.IsAny(), It.IsAny(), It.IsAny())).Returns(new Coordinate(924046.3314288399, 1088892.9140135897)); @@ -954,7 +990,7 @@ public void UpdateProperties_MatchProperties_NewProperty_Success() filePropertyRepository.Setup(x => x.Add(It.IsAny())).Callback(x => updatedDispositionFileProperty = x).Returns(dspFile.PimsDispositionFileProperties.FirstOrDefault()); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Throws(); var coordinateService = this._helper.GetService>(); coordinateService.Setup(x => x.TransformCoordinates(It.IsAny(), It.IsAny(), It.IsAny())).Returns(new Coordinate(924046.3314288399, 1088892.9140135897)); @@ -1012,7 +1048,7 @@ public void UpdateProperties_UpdatePropertyFile_Success() filePropertyRepository.Setup(x => x.GetPropertiesByDispositionFileId(It.IsAny())).Returns(new List() { new PimsDispositionFileProperty() { Internal_Id = 1, Property = property, PropertyName = "updated" } }); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Throws(); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var userRepository = this._helper.GetService>(); @@ -1044,7 +1080,7 @@ public void UpdateProperties_RemovePropertyFile_Success() filePropertyRepository.Setup(x => x.GetPropertiesByDispositionFileId(It.IsAny())).Returns(new List() { new PimsDispositionFileProperty() { Property = property } }); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); var userRepository = this._helper.GetService>(); userRepository.Setup(x => x.GetUserInfoByKeycloakUserId(It.IsAny())).Returns(EntityHelper.CreateUser("Test")); @@ -1080,7 +1116,7 @@ public void UpdateProperties_RemoveProperty_Success() filePropertyRepository.Setup(x => x.GetDispositionFilePropertyRelatedCount(It.IsAny())).Returns(1); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); propertyRepository.Setup(x => x.GetAllAssociationsById(It.IsAny())).Returns(property); var userRepository = this._helper.GetService>(); @@ -1157,7 +1193,7 @@ public void UpdateProperties_ValidatePropertyRegions_Success() repository.Setup(x => x.GetById(It.IsAny())).Returns(dspFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(1); var filePropertyRepository = this._helper.GetService>(); @@ -1190,7 +1226,7 @@ public void UpdateProperties_ValidatePropertyRegions_Error() repository.Setup(x => x.GetById(It.IsAny())).Returns(dspFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(property); propertyRepository.Setup(x => x.GetPropertyRegion(It.IsAny())).Returns(3); var filePropertyRepository = this._helper.GetService>(); @@ -1206,6 +1242,49 @@ public void UpdateProperties_ValidatePropertyRegions_Error() var exception = act.Should().Throw(); exception.WithMessage("You cannot add a property that is outside of your user account region(s). Either select a different property, or get your system administrator to add the required region to your user account settings."); } + + [Fact] + public void UpdateProperties_WithRetiredProperty_Should_Fail() + { + // Arrange + var service = this.CreateDispositionServiceWithPermissions(Permissions.DispositionEdit, Permissions.PropertyAdd, Permissions.PropertyView); + + var dspFile = EntityHelper.CreateDispositionFile(); + dspFile.ConcurrencyControlNumber = 1; + + PimsProperty retiredProperty = new PimsProperty() + { + PropertyId = 100, + Pid = 1000, + IsRetired = true, + }; + + dspFile.PimsDispositionFileProperties.Add(new PimsDispositionFileProperty() + { + Property = retiredProperty, + }); + + var repository = this._helper.GetService>(); + repository.Setup(x => x.GetRowVersion(It.IsAny())).Returns(1); + repository.Setup(x => x.GetById(It.IsAny())).Returns(dspFile); + + var filePropertyRepository = this._helper.GetService>(); + filePropertyRepository.Setup(x => x.GetPropertiesByDispositionFileId(It.IsAny())).Returns(dspFile.PimsDispositionFileProperties.ToList()); + + var userRepository = this._helper.GetService>(); + userRepository.Setup(x => x.GetUserInfoByKeycloakUserId(It.IsAny())).Returns(EntityHelper.CreateUser("Test")); + + var propertyRepository = this._helper.GetService>(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(retiredProperty); + + // Act + Action act = () => service.UpdateProperties(dspFile, new List()); + + // Assert + var ex = act.Should().Throw(); + ex.WithMessage("Retired property can not be selected."); + } + #endregion #region GetTeamMembers diff --git a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs index ad6dd27423..6638a4035c 100644 --- a/source/backend/tests/unit/api/Services/LeaseServiceTest.cs +++ b/source/backend/tests/unit/api/Services/LeaseServiceTest.cs @@ -58,7 +58,7 @@ public void Add_Success() 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); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); var userRepository = this._helper.GetService>(); userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny())).Returns(user); @@ -115,6 +115,46 @@ 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; + var user = new PimsUser(); + user.PimsRegionUsers.Add(new PimsRegionUser() { RegionCode = lease.RegionCode.Value }); + + PimsProperty retiredProperty = new PimsProperty() + { + PropertyId = 100, + Pid = 1000, + IsRetired = true, + }; + + lease.PimsPropertyLeases.Add(new PimsPropertyLease() + { + Property = retiredProperty, + }); + + 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(), true)).Returns(retiredProperty); + + 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 @@ -196,7 +236,7 @@ public void Update_Properties_Success() var propertyRepository = this._helper.GetService>(); var userRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).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()); @@ -220,21 +260,18 @@ public void Update_Properties_WithRetiredProperty_Should_Fail() var propertyRepository = this._helper.GetService>(); var userRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); + PimsProperty property = new PimsProperty() + { + PropertyId = 100, + Pid = 1, + IsRetired = true, + }; + + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(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 }); @@ -256,7 +293,7 @@ public void Update_Properties_Delete_Success() var propertyRepository = this._helper.GetService>(); var userRepository = this._helper.GetService>(); propertyLeaseRepository.Setup(x => x.GetAllByLeaseId(It.IsAny())).Returns(lease.PimsPropertyLeases); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).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()); @@ -284,7 +321,7 @@ public void Update_Properties_Delete_POI_Success() var userRepository = this._helper.GetService>(); propertyLeaseRepository.Setup(x => x.GetAllByLeaseId(It.IsAny())).Returns(lease.PimsPropertyLeases); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(deletedProperty); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(deletedProperty); propertyRepository.Setup(x => x.GetAllAssociationsById(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)); diff --git a/source/backend/tests/unit/api/Services/ResearchFileServiceTest.cs b/source/backend/tests/unit/api/Services/ResearchFileServiceTest.cs index 6047df68ad..c94e8b5ac6 100644 --- a/source/backend/tests/unit/api/Services/ResearchFileServiceTest.cs +++ b/source/backend/tests/unit/api/Services/ResearchFileServiceTest.cs @@ -116,7 +116,7 @@ public void UpdateProperties_MatchProperties_PID_Success() repository.Setup(x => x.GetById(It.IsAny())).Returns(researchFile); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(property); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(property); var filePropertyRepository = this._helper.GetService>(); filePropertyRepository.Setup(x => x.GetAllByResearchFileId(It.IsAny())).Returns(researchFile.PimsPropertyResearchFiles.ToList()); @@ -181,7 +181,7 @@ public void UpdateProperties_MatchProperties_PID_NewProperty_Success() filePropertyRepository.Setup(x => x.Add(It.IsAny())).Callback(x => updatedResearchFileProperty = x).Returns(researchFile.PimsPropertyResearchFiles.FirstOrDefault()); var propertyRepository = this._helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Throws(); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Throws(); var coordinateService = this._helper.GetService>(); coordinateService.Setup(x => x.TransformCoordinates(It.IsAny(), It.IsAny(), It.IsAny())).Returns(new Coordinate(924046.3314288399, 1088892.9140135897)); diff --git a/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs index d17709d75d..7664599a05 100644 --- a/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/DispositionFileRepositoryTest.cs @@ -66,7 +66,7 @@ public void Add_Success() } [Fact] - public void Add__WithRetiredProperty_Should_Fail() + public void Add_WithRetiredProperty_Should_Fail() { // Arrange var helper = new TestHelper(); diff --git a/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs index 084b5b7d29..a037bb32d2 100644 --- a/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs @@ -608,7 +608,7 @@ public void Update_Lease_Properties_AddPropertyInLease() helper.SaveChanges(); var propertyRepository = helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(propertyOne); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(propertyOne); var repository = helper.GetService>(); repository.Setup(x => x.GetAllByPropertyId(It.IsAny())).Returns(propertyOne.PimsPropertyLeases); @@ -642,7 +642,7 @@ public void Update_Lease_Properties_AddPropertyInLeaseOverride() propertyOne.PimsPropertyLeases = new List() { new Dal.Entities.PimsPropertyLease() { LeaseId = leaseTwo.LeaseId, PropertyId = propertyOne.PropertyId, Lease = leaseTwo } }; var propertyRepository = helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(propertyOne); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(propertyOne); var repository = helper.GetService>(); repository.Setup(x => x.GetAllByPropertyId(It.IsAny())).Returns(propertyOne.PimsPropertyLeases); @@ -680,7 +680,7 @@ public void Update_Lease_Properties_Update() helper.SaveChanges(); var propertyRepository = helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny())).Returns(propertyOne); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(propertyOne); var repository = helper.GetService>(); repository.Setup(x => x.GetAllByPropertyId(It.IsAny())).Returns(new List()); From deef01c2b1e7d1e0eaa0440e837511169a747e1a Mon Sep 17 00:00:00 2001 From: Herrera Date: Mon, 25 Mar 2024 09:19:07 -0700 Subject: [PATCH 2/3] - test updates --- .../tests/unit/dal/Repositories/LeaseRepositoryTest.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs b/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs index a037bb32d2..ab773e17f5 100644 --- a/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs +++ b/source/backend/tests/unit/dal/Repositories/LeaseRepositoryTest.cs @@ -601,14 +601,16 @@ public void Update_Lease_Properties_AddPropertyInLease() var propertyOne = EntityHelper.CreateProperty(1); var context = helper.CreatePimsContext(user, true); context.AddRange(propertyOne, lease); + var service = helper.Create(user); helper.SaveChanges(); + var leaseTwo = context.CreateLease(2, addProperty: false); propertyOne.PimsPropertyLeases = new List() { new Dal.Entities.PimsPropertyLease() { LeaseId = leaseTwo.LeaseId, Lease = leaseTwo, PropertyId = propertyOne.PropertyId } }; helper.SaveChanges(); var propertyRepository = helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(propertyOne); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(propertyOne); var repository = helper.GetService>(); repository.Setup(x => x.GetAllByPropertyId(It.IsAny())).Returns(propertyOne.PimsPropertyLeases); @@ -642,7 +644,7 @@ public void Update_Lease_Properties_AddPropertyInLeaseOverride() propertyOne.PimsPropertyLeases = new List() { new Dal.Entities.PimsPropertyLease() { LeaseId = leaseTwo.LeaseId, PropertyId = propertyOne.PropertyId, Lease = leaseTwo } }; var propertyRepository = helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(propertyOne); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(propertyOne); var repository = helper.GetService>(); repository.Setup(x => x.GetAllByPropertyId(It.IsAny())).Returns(propertyOne.PimsPropertyLeases); @@ -680,7 +682,7 @@ public void Update_Lease_Properties_Update() helper.SaveChanges(); var propertyRepository = helper.GetService>(); - propertyRepository.Setup(x => x.GetByPid(It.IsAny(), false)).Returns(propertyOne); + propertyRepository.Setup(x => x.GetByPid(It.IsAny(), true)).Returns(propertyOne); var repository = helper.GetService>(); repository.Setup(x => x.GetAllByPropertyId(It.IsAny())).Returns(new List()); From aaf4ab6882a067f0e926d894e43d7ce6af51198c Mon Sep 17 00:00:00 2001 From: Herrera Date: Mon, 25 Mar 2024 13:22:52 -0700 Subject: [PATCH 3/3] - linting --- source/backend/api/Services/LeaseService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/source/backend/api/Services/LeaseService.cs b/source/backend/api/Services/LeaseService.cs index bda365ec2f..f82313fdd7 100644 --- a/source/backend/api/Services/LeaseService.cs +++ b/source/backend/api/Services/LeaseService.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Configuration; using System.Globalization; using System.Linq; using System.Security.Claims;