Skip to content

Commit

Permalink
PSP-7907 Add representation of RETIRED properties on the map view and…
Browse files Browse the repository at this point in the history
… advance filter (#3853)

* Add retired map pin image

* Update map legend

* Add retired property toggle to advanced map filters

* Update backend filter queries

* Implement separate service call for retiring properties

* Map state-machine updates and frontend updates

* backend logic fix

* Update map clustering logic to not display retired properties

* Test corrections

* Update snapshots

* Properly exclude mockServiceWorker from any linting rules
  • Loading branch information
asanchezr authored Mar 9, 2024
1 parent b0dc090 commit 6c4d74a
Show file tree
Hide file tree
Showing 25 changed files with 191 additions and 42 deletions.
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)
{
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
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

0 comments on commit 6c4d74a

Please sign in to comment.