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.

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

PSP-7907 Add representation of RETIRED properties on the map view and advance filter #3853

Merged
merged 11 commits into from
Mar 9, 2024
2 changes: 2 additions & 0 deletions source/backend/api/Services/IPropertyService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ public interface IPropertyService

PimsProperty Update(PimsProperty property, bool commitTransaction = true);

PimsProperty RetireProperty(PimsProperty property, bool commitTransaction = true);

IList<PimsPropertyContact> GetContacts(long propertyId);

PimsPropertyContact GetContact(long propertyId, long contactId);
Expand Down
18 changes: 9 additions & 9 deletions source/backend/api/Services/PropertyOperationService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ public IEnumerable<PimsPropertyOperation> SubdivideProperty(IEnumerable<PimsProp

if (operations.Any(op => op.SourcePropertyId != operations.FirstOrDefault().SourcePropertyId))
{
throw new BusinessRuleViolationException("All property operations must have the same PIMS parent property.");
throw new BusinessRuleViolationException("All property operations must have the same PIMS parent property.");
}

if (operations.Select(o => o.DestinationProperty).Count() < 2)
{
throw new BusinessRuleViolationException("Subdivisions must contain at least two child properties.");
throw new BusinessRuleViolationException("Subdivisions must contain at least two child properties.");
}

foreach (var operation in operations)
Expand All @@ -75,15 +75,15 @@ public IEnumerable<PimsPropertyOperation> SubdivideProperty(IEnumerable<PimsProp
_propertyService.GetByPid(operation.DestinationProperty.Pid.ToString());
throw new BusinessRuleViolationException("Subdivision children may not already be in the PIMS inventory.");

} catch (KeyNotFoundException)
}
catch (KeyNotFoundException)
{
// ignore exception, the pid should not exist.
}
}

// retire the source property
dbSourceProperty.IsRetired = true;
_propertyService.Update(dbSourceProperty, false);
_propertyService.RetireProperty(dbSourceProperty, false);

foreach (var operation in operations)
{
Expand All @@ -109,7 +109,7 @@ public IEnumerable<PimsPropertyOperation> ConsolidateProperty(IEnumerable<PimsPr
IEnumerable<PimsProperty> dbSourceProperties = _propertyService.GetMultipleById(sourceProperties.Select(sp => sp.PropertyId).ToList());

CommonPropertyOperationValidation(operations);
if(destinationProperty?.Pid == null)
if (destinationProperty?.Pid == null)
{
throw new BusinessRuleViolationException("Consolidation child must have a property with a valid PID.");
}
Expand Down Expand Up @@ -153,13 +153,13 @@ public IEnumerable<PimsPropertyOperation> ConsolidateProperty(IEnumerable<PimsPr
// retire the source properties
foreach (var sp in dbSourceProperties)
{
sp.IsRetired = true;
_propertyService.Update(sp, false);
_propertyService.RetireProperty(sp, false);
}

destinationProperty.PropertyId = 0; // in the case this property already exists, this will force it to be recreated.
var newProperty = _propertyService.PopulateNewProperty(destinationProperty, isOwned: true, isPropertyOfInterest: false);
operations.ForEach(op => {
operations.ForEach(op =>
{
op.DestinationProperty = newProperty;
op.DestinationPropertyId = newProperty.PropertyId;
op.SourceProperty = null; // do not allow the property operation to modify the source in the add range operation.
Expand Down
14 changes: 13 additions & 1 deletion source/backend/api/Services/PropertyService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
using Pims.Api.Helpers.Exceptions;
using Pims.Api.Models.CodeTypes;
using Pims.Api.Models.Concepts.Property;
using Pims.Core.Exceptions;
using Pims.Core.Extensions;
using Pims.Dal.Entities;
using Pims.Dal.Exceptions;
Expand Down Expand Up @@ -121,6 +120,19 @@ public PimsProperty Update(PimsProperty property, bool commitTransaction = true)
return GetById(newProperty.Internal_Id);
}

public PimsProperty RetireProperty(PimsProperty property, bool commitTransaction = true)
{
_logger.LogInformation("Retiring property with id {id}", property.Internal_Id);
_user.ThrowIfNotAuthorized(Permissions.PropertyEdit);

var retiredProperty = _propertyRepository.RetireProperty(property);
if (commitTransaction)
{
_propertyRepository.CommitTransaction();
}
return GetById(retiredProperty.Internal_Id); ;
}

public IList<PimsPropertyContact> GetContacts(long propertyId)
{
_logger.LogInformation("Retrieving property contacts...");
Expand Down
5 changes: 5 additions & 0 deletions source/backend/dal/Models/PropertyFilterCriteria.cs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ public class PropertyFilterCriteria
/// </summary>
public bool IsDisposed { get; set; } = false;

/// <summary>
/// get/set - Whether or not to show retired properties.
/// </summary>
public bool IsRetired { get; set; } = false;

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ public interface IPropertyRepository : IRepository<PimsProperty>

PimsProperty TransferFileProperty(PimsProperty property, PropertyOwnershipState state);

PimsProperty RetireProperty(PimsProperty property);

HashSet<long> GetMatchingIds(PropertyFilterCriteria filter);

short GetPropertyRegion(long id);
Expand Down
42 changes: 36 additions & 6 deletions source/backend/dal/Repositories/PropertyRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ public Paged<PimsProperty> GetPage(PropertyFilter filter)
/// <returns></returns>
public PimsProperty GetById(long id)
{
this.User.ThrowIfNotAllAuthorized(Permissions.PropertyView);
User.ThrowIfNotAllAuthorized(Permissions.PropertyView);

var property = this.Context.PimsProperties
var property = Context.PimsProperties.AsNoTracking()
.Include(p => p.DistrictCodeNavigation)
.Include(p => p.RegionCodeNavigation)
.Include(p => p.PropertyTypeCodeNavigation)
Expand Down Expand Up @@ -421,6 +421,17 @@ public PimsProperty TransferFileProperty(PimsProperty property, PropertyOwnershi
return existingProperty;
}

public PimsProperty RetireProperty(PimsProperty property)
{
property.ThrowIfNull(nameof(property));

var existingProperty = Context.PimsProperties
.FirstOrDefault(p => p.PropertyId == property.Internal_Id) ?? throw new KeyNotFoundException();

existingProperty.IsRetired = true;
return existingProperty;
}

public HashSet<long> GetMatchingIds(PropertyFilterCriteria filter)
{
var predicate = PredicateBuilder.New<PimsProperty>(p => true);
Expand Down Expand Up @@ -493,10 +504,29 @@ public HashSet<long> GetMatchingIds(PropertyFilterCriteria filter)
}

// Property ownership filters
predicate.And(p => (p.IsOwned && filter.IsCoreInventory) ||
(p.IsPropertyOfInterest && filter.IsPropertyOfInterest) ||
(p.IsOtherInterest && filter.IsOtherInterest) ||
(p.IsDisposed && filter.IsDisposed));
var ownershipBuilder = PredicateBuilder.New<PimsProperty>(p => false);
if (filter.IsCoreInventory)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I really liked this logic more explicit

{
ownershipBuilder.Or(p => p.IsOwned);
}
if (filter.IsPropertyOfInterest)
{
ownershipBuilder.Or(p => p.IsPropertyOfInterest);
}
if (filter.IsOtherInterest)
{
ownershipBuilder.Or(p => p.IsOtherInterest);
}
if (filter.IsDisposed)
{
ownershipBuilder.Or(p => p.IsDisposed);
}
if (filter.IsRetired)
{
ownershipBuilder.Or(p => p.IsRetired.HasValue && p.IsRetired.Value);
}

predicate.And(ownershipBuilder);

return Context.PimsProperties.AsNoTracking()
.Where(predicate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ public void Subdivide_Success()
var sameProperty = EntityHelper.CreateProperty(3);
propertyService.Setup(x => x.GetById(It.IsAny<long>())).Returns(sameProperty);
propertyService.Setup(x => x.GetByPid(It.IsAny<string>())).Throws(new KeyNotFoundException());
propertyService.Setup(x => x.Update(It.IsAny<PimsProperty>(), false)).Returns(sameProperty);
propertyService.Setup(x => x.RetireProperty(It.IsAny<PimsProperty>(), false)).Returns(sameProperty);
propertyService.Setup(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false)).Returns(sameProperty);

var operations = new List<PimsPropertyOperation>() { EntityHelper.CreatePropertyOperation(), EntityHelper.CreatePropertyOperation() };
Expand All @@ -204,7 +204,7 @@ public void Subdivide_Success()

// Assert
repository.Verify(x => x.AddRange(It.IsAny<List<PimsPropertyOperation>>()), Times.Once);
propertyService.Verify(x => x.Update(It.IsAny<PimsProperty>(), false), Times.Once);
propertyService.Verify(x => x.RetireProperty(It.IsAny<PimsProperty>(), false), Times.Once);
propertyService.Verify(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false), Times.Exactly(2));
}

Expand All @@ -217,7 +217,7 @@ public void Subdivide_Success_SameSourceDestinationPid()
var sameProperty = EntityHelper.CreateProperty(3);
propertyService.Setup(x => x.GetById(It.IsAny<long>())).Returns(sameProperty);
propertyService.Setup(x => x.GetByPid(It.IsAny<string>())).Returns(sameProperty);
propertyService.Setup(x => x.Update(It.IsAny<PimsProperty>(), false)).Returns(sameProperty);
propertyService.Setup(x => x.RetireProperty(It.IsAny<PimsProperty>(), false)).Returns(sameProperty);
propertyService.Setup(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false)).Returns(sameProperty);

var operationWithSameDestSource = EntityHelper.CreatePropertyOperation();
Expand All @@ -233,7 +233,7 @@ public void Subdivide_Success_SameSourceDestinationPid()

// Assert
repository.Verify(x => x.AddRange(It.IsAny<List<PimsPropertyOperation>>()), Times.Once);
propertyService.Verify(x => x.Update(It.IsAny<PimsProperty>(), false), Times.Once);
propertyService.Verify(x => x.RetireProperty(It.IsAny<PimsProperty>(), false), Times.Once);
propertyService.Verify(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false), Times.Exactly(2));
}

Expand Down Expand Up @@ -434,7 +434,7 @@ public void Consolidate_Success()
var otherProperty = EntityHelper.CreateProperty(4);
propertyService.Setup(x => x.GetMultipleById(It.IsAny<List<long>>())).Returns(new List<PimsProperty> { sameProperty, otherProperty });
propertyService.Setup(x => x.GetByPid(It.IsAny<string>())).Throws(new KeyNotFoundException());
propertyService.Setup(x => x.Update(It.IsAny<PimsProperty>(), false)).Returns(sameProperty);
propertyService.Setup(x => x.RetireProperty(It.IsAny<PimsProperty>(), false)).Returns((PimsProperty p, bool b) => p);
propertyService.Setup(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false)).Returns(sameProperty);

var operationOne = EntityHelper.CreatePropertyOperation();
Expand All @@ -451,7 +451,7 @@ public void Consolidate_Success()

// Assert
repository.Verify(x => x.AddRange(It.IsAny<List<PimsPropertyOperation>>()), Times.Once);
propertyService.Verify(x => x.Update(It.IsAny<PimsProperty>(), false), Times.Exactly(2));
propertyService.Verify(x => x.RetireProperty(It.IsAny<PimsProperty>(), false), Times.Exactly(2));
propertyService.Verify(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false), Times.Once);
}

Expand All @@ -465,7 +465,7 @@ public void Consolidate_Success_SameSourceDestinationPid()
var otherProperty = EntityHelper.CreateProperty(4);
propertyService.Setup(x => x.GetMultipleById(It.IsAny<List<long>>())).Returns(new List<PimsProperty> { sameProperty, otherProperty });
propertyService.Setup(x => x.GetByPid(It.IsAny<string>())).Returns(sameProperty);
propertyService.Setup(x => x.Update(It.IsAny<PimsProperty>(), false)).Returns(sameProperty);
propertyService.Setup(x => x.RetireProperty(It.IsAny<PimsProperty>(), false)).Returns((PimsProperty p, bool b) => p);
propertyService.Setup(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false)).Returns(sameProperty);

var operationWithSameDestSource = EntityHelper.CreatePropertyOperation();
Expand All @@ -484,7 +484,7 @@ public void Consolidate_Success_SameSourceDestinationPid()

// Assert
repository.Verify(x => x.AddRange(It.IsAny<List<PimsPropertyOperation>>()), Times.Once);
propertyService.Verify(x => x.Update(It.IsAny<PimsProperty>(), false), Times.Exactly(2));
propertyService.Verify(x => x.RetireProperty(It.IsAny<PimsProperty>(), false), Times.Exactly(2));
propertyService.Verify(x => x.PopulateNewProperty(It.IsAny<PimsProperty>(), true, false), Times.Once);
}

Expand Down
1 change: 1 addition & 0 deletions source/frontend/.eslintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
src/serviceWorker.ts
mockServiceWorker.js
node_modules
2 changes: 1 addition & 1 deletion source/frontend/.prettierignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ node_modules
build
*.yaml
*.yml
mockServiceWork.js
mockServiceWorker.js
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface IMapStateMachineContext {
isShowingMapLayers: boolean;
activePimsPropertyIds: number[];
showDisposed: boolean;
showRetired: boolean;

requestFlyToLocation: (latlng: LatLngLiteral) => void;
requestFlyToBounds: (bounds: LatLngBounds) => void;
Expand All @@ -61,6 +62,7 @@ export interface IMapStateMachineContext {

setVisiblePimsProperties: (propertyIds: number[]) => void;
setShowDisposed: (show: boolean) => void;
setShowRetired: (show: boolean) => void;
}

const MapStateMachineContext = React.createContext<IMapStateMachineContext>(
Expand Down Expand Up @@ -265,6 +267,13 @@ export const MapStateMachineProvider: React.FC<React.PropsWithChildren<unknown>>
[serviceSend],
);

const setShowRetired = useCallback(
(show: boolean) => {
serviceSend({ type: 'SET_SHOW_RETIRED', show });
},
[serviceSend],
);

const toggleMapFilter = useCallback(() => {
serviceSend({ type: 'TOGGLE_FILTER' });
}, [serviceSend]);
Expand Down Expand Up @@ -316,6 +325,7 @@ export const MapStateMachineProvider: React.FC<React.PropsWithChildren<unknown>>
isShowingMapLayers: isShowingMapLayers,
activePimsPropertyIds: state.context.activePimsPropertyIds,
showDisposed: state.context.showDisposed,
showRetired: state.context.showRetired,

setMapSearchCriteria,
refreshMapProperties,
Expand All @@ -336,6 +346,7 @@ export const MapStateMachineProvider: React.FC<React.PropsWithChildren<unknown>>
setFilePropertyLocations,
setVisiblePimsProperties,
setShowDisposed,
setShowRetired,
}}
>
{children}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ const featureViewStates = {
on: {
TOGGLE_FILTER: {
target: 'browsing',
actions: assign({ showDisposed: () => false }),
actions: [assign({ showDisposed: () => false }), assign({ showRetired: () => false })],
},
TOGGLE_LAYERS: {
target: 'layerControl',
Expand All @@ -64,6 +64,9 @@ const featureViewStates = {
SET_SHOW_DISPOSED: {
actions: assign({ showDisposed: (_, event: any) => event.show }),
},
SET_SHOW_RETIRED: {
actions: assign({ showRetired: (_, event: any) => event.show }),
},
},
},
},
Expand Down Expand Up @@ -334,6 +337,7 @@ export const mapMachine = createMachine<MachineContext>({
filePropertyLocations: [],
activePimsPropertyIds: [],
showDisposed: false,
showRetired: false,
},

// State definitions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export type MachineContext = {
filePropertyLocations: LatLngLiteral[];
activePimsPropertyIds: number[];
showDisposed: boolean;
showRetired: boolean;
};

// Possible state machine states
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export const FilterContentContainer: React.FC<
> = ({ View }) => {
const mapMachine = useMapStateMachine();

const { setVisiblePimsProperties, setShowDisposed } = mapMachine;
const { setVisiblePimsProperties, setShowDisposed, setShowRetired } = mapMachine;

const { getMatchingProperties } = usePimsPropertyRepository();

Expand All @@ -37,8 +37,9 @@ export const FilterContentContainer: React.FC<
(model: PropertyFilterFormModel) => {
filterProperties(model.toApi());
setShowDisposed(model.isDisposed);
setShowRetired(model.isRetired);
},
[filterProperties, setShowDisposed],
[filterProperties, setShowDisposed, setShowRetired],
);

return <View onChange={onChange} isLoading={getMatchingProperties.loading} />;
Expand Down
Loading
Loading