Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

PSP-7922 Completing disposition file - selected properties and errors #3885

Merged
merged 5 commits into from
Mar 21, 2024
Merged

PSP-7922 Completing disposition file - selected properties and errors #3885

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
138bdee
Refactor logic to validate disposition file upon closing file
asanchezr Mar 21, 2024
79cdeaa
Disposition form styling fixes
asanchezr Mar 21, 2024
80cb25b
Update snapshots
asanchezr Mar 21, 2024
c0d7fa1
Test corrections
asanchezr Mar 21, 2024
66be916
Merge branch 'dev' into psp-7922-disp-file-completion-warnings
asanchezr Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 57 additions & 35 deletions source/backend/api/Services/DispositionFileService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,35 +117,18 @@ public PimsDispositionFile Update(long id, PimsDispositionFile dispositionFile,
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.");
}

ValidateStaff(dispositionFile);
ValidateVersion(id, dispositionFile.ConcurrencyControlNumber);

dispositionFile.ThrowContractorRemovedFromTeam(_user, _userRepository);

var doNotAddToStatuses = new List<string>() { EnumDispositionFileStatusTypeCode.COMPLETE.ToString(), EnumDispositionFileStatusTypeCode.ARCHIVED.ToString(), EnumDispositionFileStatusTypeCode.CANCELLED.ToString(), };
// validate disposition file state before proceeding with any database updates
var currentDispositionFile = _dispositionFileRepository.GetById(id);
ValidateFileBeforeUpdate(dispositionFile, currentDispositionFile, userOverrides);

if (!userOverrides.Contains(UserOverrideCode.DispositionFileFinalStatus) && !doNotAddToStatuses.Contains(currentDispositionFile.DispositionFileStatusTypeCode)
&& doNotAddToStatuses.Contains(dispositionFile.DispositionFileStatusTypeCode))
{
throw new UserOverrideException(UserOverrideCode.DispositionFileFinalStatus, "You are changing this file to a non-editable state. (Only system administrators can edit the file when set to Archived, Cancelled or Completed state). Do you wish to continue?");
}
else if (currentDispositionFile.DispositionFileStatusTypeCode != EnumDispositionFileStatusTypeCode.COMPLETE.ToString()
&& dispositionFile.DispositionFileStatusTypeCode == EnumDispositionFileStatusTypeCode.COMPLETE.ToString())
{
if (currentDispositionFile?.PimsDispositionSales?.FirstOrDefault()?.SaleFinalAmt == null)
{
throw new BusinessRuleViolationException("You have not added a Sales Price. Please add a Sales Price before completion.");
}
else if (currentDispositionFile.PimsDispositionFileProperties.Count > 0)
{
DisposeOfProperties(dispositionFile, userOverrides);
}
}
var isFileClosing = currentDispositionFile.DispositionFileStatusTypeCode != EnumDispositionFileStatusTypeCode.COMPLETE.ToString() &&
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could move this assigment bebore calling private "ValidateFileBeforeUpdate" method, and pass the value to the method as parameter, that way you would make the assigment once.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fair- still I feel this is a simple check and perhaps should be kept so the "Validate" doesn't have to trust someone to do the check for itself

dispositionFile.DispositionFileStatusTypeCode == EnumDispositionFileStatusTypeCode.COMPLETE.ToString();

if (!userOverrides.Contains(UserOverrideCode.UpdateRegion))
if (isFileClosing && currentDispositionFile.PimsDispositionFileProperties?.Count > 0)
{
ValidateMinistryRegion(id, dispositionFile.RegionCode);
DisposeOfProperties(dispositionFile);
}

_dispositionFileRepository.Update(id, dispositionFile);
Expand Down Expand Up @@ -560,30 +543,69 @@ public PimsDispositionFile UpdateProperties(PimsDispositionFile dispositionFile,
return _dispositionFileRepository.GetById(dispositionFile.Internal_Id);
}

/// <summary>
/// Attempt to dispose of any properties if all business rules are met.
/// </summary>
/// <param name="dispositionFile"></param>
private void DisposeOfProperties(PimsDispositionFile dispositionFile, IEnumerable<UserOverrideCode> userOverrides)
private void ValidateFileBeforeUpdate(PimsDispositionFile incomingDispositionFile, PimsDispositionFile currentDispositionFile, IEnumerable<UserOverrideCode> userOverrides)
{
var currentProperties = _dispositionFilePropertyRepository.GetPropertiesByDispositionFileId(dispositionFile.Internal_Id);
if (currentProperties.Any(p => p.Property.IsOwned) && !userOverrides.Contains(UserOverrideCode.DisposeOfProperties))
// Implement file validation logic before proceeding to update. This includes file closing validation.
// The order of validation checks is important as it has been requested by business users.
var isFileClosing = currentDispositionFile.DispositionFileStatusTypeCode != EnumDispositionFileStatusTypeCode.COMPLETE.ToString() &&
incomingDispositionFile.DispositionFileStatusTypeCode == EnumDispositionFileStatusTypeCode.COMPLETE.ToString();

var currentProperties = _dispositionFilePropertyRepository.GetPropertiesByDispositionFileId(incomingDispositionFile.Internal_Id);

// The following checks result in hard STOP errors
if (isFileClosing)
{
throw new UserOverrideException(UserOverrideCode.DisposeOfProperties, "You are completing this Disposition File with owned PIMS inventory properties. All properties will be removed from the PIMS inventory (any Other Interests will remain). Do you wish to proceed?");
if (currentProperties.Any(p => p.Property.IsPropertyOfInterest))
{
throw new BusinessRuleViolationException("You have one or more properties attached to this Disposition file that is NOT in the \"Core Inventory\" (i.e. owned by BCTFA and/or HMK). To complete this file you must either, remove these non \"Non-Core Inventory\" properties, OR make sure the property is added to the PIMS inventory first.");
}

if (currentDispositionFile.PimsDispositionSales?.FirstOrDefault()?.SaleFinalAmt == null)
{
throw new BusinessRuleViolationException("You have not added a Sales Price. Please add a Sales Price before completion.");
}
}
else if (currentProperties.Any(p => p.Property.IsPropertyOfInterest))

ValidateStaff(incomingDispositionFile);
incomingDispositionFile.ThrowContractorRemovedFromTeam(_user, _userRepository);

// From here on - these checks result in warnings that require user confirmation
if (!userOverrides.Contains(UserOverrideCode.UpdateRegion))
{
throw new BusinessRuleViolationException("You have one or more properties attached to this Disposition file that is NOT in the \"Core Inventory\" (i.e. owned by BCTFA and/or HMK). To complete this file you must either, remove these non \"Non-Core Inventory\" properties, OR make sure the property is added to the PIMS inventory first.");
// confirm user action - file region was changed
ValidateMinistryRegion(incomingDispositionFile.Internal_Id, incomingDispositionFile.RegionCode);
}

// Get the current properties in the research file
var nonEditableStatuses = new List<string>() { EnumDispositionFileStatusTypeCode.COMPLETE.ToString(), EnumDispositionFileStatusTypeCode.ARCHIVED.ToString(), EnumDispositionFileStatusTypeCode.CANCELLED.ToString(), };
var isFileChangingToNonEditableState = !nonEditableStatuses.Contains(currentDispositionFile.DispositionFileStatusTypeCode) && nonEditableStatuses.Contains(incomingDispositionFile.DispositionFileStatusTypeCode);

// confirm user action - file is changing to non-editable state
if (!userOverrides.Contains(UserOverrideCode.DispositionFileFinalStatus) && isFileChangingToNonEditableState)
{
throw new UserOverrideException(UserOverrideCode.DispositionFileFinalStatus, "You are changing this file to a non-editable state. (Only system administrators can edit the file when set to Archived, Cancelled or Completed state). Do you wish to continue?");
}

if (isFileClosing && currentProperties.Any(p => p.Property.IsOwned) && !userOverrides.Contains(UserOverrideCode.DisposeOfProperties))
{
throw new UserOverrideException(UserOverrideCode.DisposeOfProperties, "You are completing this Disposition File with owned PIMS inventory properties. All properties will be removed from the PIMS inventory (any Other Interests will remain). Do you wish to proceed?");
}
}

/// <summary>
/// Attempt to dispose of any properties if all business rules are met.
/// </summary>
/// <param name="dispositionFile">The disposition file entity.</param>
private void DisposeOfProperties(PimsDispositionFile dispositionFile)
{
// Get the current properties in the disposition file
var currentProperties = _dispositionFilePropertyRepository.GetPropertiesByDispositionFileId(dispositionFile.Internal_Id);
var ownedProperties = currentProperties.Where(p => p.Property.IsOwned);

// PSP-7275 Business rule: Transfer properties of interest to disposed when disposition file is completed
foreach (var dispositionProperty in ownedProperties)
{
var property = dispositionProperty.Property;
_propertyRepository.TransferFileProperty(property, new Dal.Models.PropertyOwnershipState() { isDisposed = true, isPropertyOfInterest = false, isOtherInterest = false, isOwned = false});
_propertyRepository.TransferFileProperty(property, new Dal.Models.PropertyOwnershipState() { isDisposed = true, isPropertyOfInterest = false, isOtherInterest = false, isOwned = false });
}
}

Expand Down
5 changes: 5 additions & 0 deletions source/backend/tests/unit/api/Services/DispositionFileServiceTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ public void Update_Final_Validation_UserOverride(EnumDispositionFileStatusTypeCo
// Arrange
var service = this.CreateDispositionServiceWithPermissions(Permissions.DispositionEdit);
var repository = this._helper.GetService<Mock<IDispositionFileRepository>>();
var dispositionFilePropertyRepository = this._helper.GetService<Mock<IDispositionFilePropertyRepository>>();

var dispFile = EntityHelper.CreateDispositionFile(1);

Expand All @@ -429,6 +430,8 @@ public void Update_Final_Validation_UserOverride(EnumDispositionFileStatusTypeCo
repository.Setup(x => x.Update(It.IsAny<long>(), It.IsAny<PimsDispositionFile>())).Returns(dispFile);
repository.Setup(x => x.GetById(It.IsAny<long>())).Returns(dispFile);

dispositionFilePropertyRepository.Setup(x => x.GetPropertiesByDispositionFileId(It.IsAny<long>())).Returns(new List<PimsDispositionFileProperty>() { });

// Act
Action act = () => service.Update(2, updateDispFile, new List<UserOverrideCode>() { UserOverrideCode.DispositionFileFinalStatus });

Expand All @@ -443,6 +446,7 @@ public void Update_Should_Fail_Complete_NoSale()
// Arrange
var service = this.CreateDispositionServiceWithPermissions(Permissions.DispositionEdit);
var repository = this._helper.GetService<Mock<IDispositionFileRepository>>();
var dispositionFilePropertyRepository = this._helper.GetService<Mock<IDispositionFilePropertyRepository>>();

var statusMock = this._helper.GetService<Mock<IDispositionStatusSolver>>();
statusMock.Setup(x => x.CanEditDetails(It.IsAny<DispositionStatusTypes>())).Returns(true);
Expand All @@ -456,6 +460,7 @@ public void Update_Should_Fail_Complete_NoSale()
repository.Setup(x => x.GetRegion(It.IsAny<long>())).Returns(1);
repository.Setup(x => x.Update(It.IsAny<long>(), It.IsAny<PimsDispositionFile>())).Returns(dispFile);
repository.Setup(x => x.GetById(It.IsAny<long>())).Returns(dispFile);
dispositionFilePropertyRepository.Setup(x => x.GetPropertiesByDispositionFileId(It.IsAny<long>())).Returns(new List<PimsDispositionFileProperty>() { });

// Act
Action act = () => service.Update(2, updateDispFile, new List<UserOverrideCode>() { UserOverrideCode.DispositionFileFinalStatus });
Expand Down
8 changes: 3 additions & 5 deletions .../features/mapSideBar/disposition/tabs/fileDetails/detail/update/UpdateDispositionForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ const UpdateDispositionForm: React.FC<IUpdateDispositionFormProps> = ({
<LoadingBackdrop show={loading} parentScreen />

<Container>
<Section header="">
<Section>
<SectionField label="Status" required>
<Select
field="fileStatusTypeCode"
Expand Down Expand Up @@ -244,10 +244,8 @@ const UpdateDispositionForm: React.FC<IUpdateDispositionFormProps> = ({
export default UpdateDispositionForm;

const Container = styled.div`
.form-section {
margin: 0;
padding-left: 0;
}
background-color: ${props => props.theme.css.filterBackgroundColor};
padding-top: 1rem;
Comment on lines +247 to +248
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixing the styling of the Update Disposition form. Was missing the grey background and padding


.tab-pane {
.form-section {
Expand Down
Loading
Loading