Skip to content

Commit

Permalink
PSP-7929 : Backend error - For all files do not include a retired pro…
Browse files Browse the repository at this point in the history
…perty on selection (#3884)

Co-authored-by: Herrera <eduardo.herrera@quartech.com>
  • Loading branch information
eddherrera and Herrera authored Mar 21, 2024
1 parent c9b4c1f commit 6fe7f56
Show file tree
Hide file tree
Showing 15 changed files with 238 additions and 4 deletions.
3 changes: 2 additions & 1 deletion source/backend/api/Services/AcquisitionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,15 @@ public PimsAcquisitionFile Add(PimsAcquisitionFile acquisitionFile, IEnumerable<
ValidateStaff(acquisitionFile);
ValidateOrganizationStaff(acquisitionFile);

acquisitionFile.AcquisitionFileStatusTypeCode = "ACTIVE";
MatchProperties(acquisitionFile, userOverrides);
ValidatePropertyRegions(acquisitionFile);

PopulateAcquisitionChecklist(acquisitionFile);

acquisitionFile.AcquisitionFileStatusTypeCode = AcquisitionStatusTypes.ACTIVE.ToString();
var newAcqFile = _acqFileRepository.Add(acquisitionFile);
_acqFileRepository.CommitTransaction();

return newAcqFile;
}

Expand Down
1 change: 0 additions & 1 deletion source/backend/api/Services/DispositionFileService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ public PimsDispositionFile Add(PimsDispositionFile dispositionFile, IEnumerable<
ValidateStaff(dispositionFile);

MatchProperties(dispositionFile, userOverrides);

ValidatePropertyRegions(dispositionFile);

var newDispositionFile = _dispositionFileRepository.Add(dispositionFile);
Expand Down
12 changes: 11 additions & 1 deletion source/backend/api/Services/LeaseService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -185,6 +186,7 @@ public PimsLease Add(PimsLease lease, IEnumerable<UserOverrideCode> userOverride
pimsUser.ThrowInvalidAccessToLeaseFile(lease.RegionCode);

var leasesWithProperties = AssociatePropertyLeases(lease, userOverrides);

return _leaseRepository.Add(leasesWithProperties);
}

Expand All @@ -203,12 +205,20 @@ public PimsLease Update(PimsLease lease, IEnumerable<UserOverrideCode> 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)
{
Expand Down Expand Up @@ -237,7 +247,7 @@ public PimsLease Update(PimsLease lease, IEnumerable<UserOverrideCode> userOverr
List<PimsPropertyLease> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
{
Expand Down
6 changes: 6 additions & 0 deletions source/backend/dal/Repositories/AcquisitionFileRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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)
{
Expand Down
6 changes: 6 additions & 0 deletions source/backend/dal/Repositories/DispositionFileRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions source/backend/dal/Repositories/LeaseRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ public void Add_DuplicateTeam()
act.Should().Throw<BadRequestException>();
repository.Verify(x => x.Add(It.IsAny<PimsAcquisitionFile>()), Times.Never);
}

#endregion

#region GetById
Expand Down
39 changes: 38 additions & 1 deletion source/backend/tests/unit/api/Services/LeaseServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,13 @@
using System.Diagnostics.CodeAnalysis;
using System.Linq;
using FluentAssertions;
using Humanizer;
using MapsterMapper;
using Moq;
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;
Expand Down Expand Up @@ -193,19 +195,54 @@ public void Update_Properties_Success()
var propertyLeaseRepository = this._helper.GetService<Mock<IPropertyLeaseRepository>>();
var propertyRepository = this._helper.GetService<Mock<IPropertyRepository>>();
var userRepository = this._helper.GetService<Mock<IUserRepository>>();

propertyRepository.Setup(x => x.GetByPid(It.IsAny<int>())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property);
leaseRepository.Setup(x => x.GetNoTracking(It.IsAny<long>())).Returns(lease);
leaseRepository.Setup(x => x.Get(It.IsAny<long>())).Returns(EntityHelper.CreateLease(1));
userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny<Guid>())).Returns(new PimsUser());

// Act

var updatedLease = service.Update(lease, new List<UserOverrideCode>() { 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<Mock<ILeaseRepository>>();
var propertyLeaseRepository = this._helper.GetService<Mock<IPropertyLeaseRepository>>();
var propertyRepository = this._helper.GetService<Mock<IPropertyRepository>>();
var userRepository = this._helper.GetService<Mock<IUserRepository>>();

propertyRepository.Setup(x => x.GetByPid(It.IsAny<int>())).Returns(lease.PimsPropertyLeases.FirstOrDefault().Property);
leaseRepository.Setup(x => x.GetNoTracking(It.IsAny<long>())).Returns(lease);
leaseRepository.Setup(x => x.Get(It.IsAny<long>())).Returns(EntityHelper.CreateLease(1));
userRepository.Setup(x => x.GetByKeycloakUserId(It.IsAny<Guid>())).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>() { UserOverrideCode.AddLocationToProperty });

// Assert
var ex = act.Should().Throw<BusinessRuleViolationException>();
ex.WithMessage("Retired property can not be selected.");
}

[Fact]
public void Update_Properties_Delete_Success()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -140,6 +142,32 @@ public void Add_ThrowIfNull()
// Assert
act.Should().Throw<ArgumentNullException>();
}

[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<AcquisitionFilePropertyRepository>(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<BusinessRuleViolationException>();
ex.WithMessage("Retired property can not be selected.");
}

#endregion

#region Update
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,39 @@ public void Add_ThrowIfNull()
// Assert
act.Should().Throw<ArgumentNullException>();
}


[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<AcquisitionFileRepository>(user);

var acqFile = EntityHelper.CreateAcquisitionFile();
acqFile.PimsPropertyAcquisitionFiles = new List<PimsPropertyAcquisitionFile>()
{
new PimsPropertyAcquisitionFile()
{
PropertyId = 100,
Property = new PimsProperty()
{
IsRetired = true,
}
},
};

// Act
Action act = () => repository.Add(acqFile);

// Assert
var ex = act.Should().Throw<BusinessRuleViolationException>();
ex.WithMessage("Retired property can not be selected.");
}


#endregion

#region GetById
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -140,6 +141,38 @@ public void Add_ThrowIfNull()
// Assert
act.Should().Throw<ArgumentNullException>();
}

[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<DispositionFilePropertyRepository>(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<BusinessRuleViolationException>();
ex.WithMessage("Retired property can not be selected.");
}

#endregion

#region Update
Expand Down
Loading

0 comments on commit 6fe7f56

Please sign in to comment.