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-7869 Indicate "retired" on property information details #3840

Merged
merged 11 commits into from
Mar 7, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public IActionResult GetMultipleConceptPropertyWithId([FromQuery] long[] ids)
[TypeFilter(typeof(NullJsonResultFilter))]
public IActionResult UpdateConceptProperty([FromBody] PropertyModel propertyModel)
{
var propertyEntity = _mapper.Map<Pims.Dal.Entities.PimsProperty>(propertyModel);
var propertyEntity = _mapper.Map<Dal.Entities.PimsProperty>(propertyModel);
var updatedProperty = _propertyService.Update(propertyEntity);

return new JsonResult(_mapper.Map<PropertyModel>(updatedProperty));
Expand Down
3 changes: 2 additions & 1 deletion source/backend/api/Services/PropertyService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
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 @@ -100,7 +101,7 @@ public PimsProperty GetByPid(string pid)

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

// convert spatial location from lat/long (4326) to BC Albers (3005) for database storage
Expand Down
14 changes: 11 additions & 3 deletions source/backend/dal/Repositories/PropertyRepository.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.Core.Helpers;
using Pims.Dal.Entities;
Expand Down Expand Up @@ -283,17 +284,24 @@ public PimsProperty GetAllAssociationsById(long id)
/// <summary>
/// Update the passed property in the database assuming the user has the required claims.
/// </summary>
/// <param name="property"></param>
/// <returns></returns>
/// <param name="property">The property to update.</param>
/// <param name="overrideLocation">Whether to update the property spatial location with the incoming value. Defaults to false.</param>
/// <returns>The updated property.</returns>
public PimsProperty Update(PimsProperty property, bool overrideLocation = false)
{
property.ThrowIfNull(nameof(property));

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

// prevent editing on retired properties
if (existingProperty.IsRetired.HasValue && existingProperty.IsRetired.Value)
{
throw new BusinessRuleViolationException("Retired records are referenced for historical purposes only and cannot be edited or deleted.");
}

// ignore a number of properties that we don't the frontend to override - for now
property.Boundary = existingProperty.Boundary;
if (!overrideLocation)
Expand Down
39 changes: 24 additions & 15 deletions source/backend/tests/core/Entities/PropertyHelper.cs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
using System;
using System.Linq;
using NetTopologySuite.Geometries;
using Pims.Api.Models.CodeTypes;
using Pims.Dal;
using Pims.Dal.Entities;
using Entity = Pims.Dal.Entities;

namespace Pims.Core.Test
{
Expand All @@ -23,27 +23,30 @@ public static partial class EntityHelper
/// <param name="areaUnit"></param>
/// <param name="dataSource"></param>
/// <returns></returns>
public static Entity.PimsProperty CreateProperty(int pid, int? pin = null, Entity.PimsPropertyType type = null, Entity.PimsPropertyClassificationType classification = null, Entity.PimsAddress address = null, Entity.PimsPropertyTenureType tenure = null, Entity.PimsAreaUnitType areaUnit = null, Entity.PimsDataSourceType dataSource = null, Entity.PimsPropertyStatusType status = null, Entity.PimsLease lease = null, short? regionCode = null, bool? isCoreInventory = null, bool? isPointOfInterest = null, bool? isOtherInterest = null, bool? isDisposed = null)
public static PimsProperty CreateProperty(int pid, int? pin = null, PimsPropertyType type = null, PimsPropertyClassificationType classification = null, PimsAddress address = null, PimsPropertyTenureType tenure = null, PimsAreaUnitType areaUnit = null, PimsDataSourceType dataSource = null, PimsPropertyStatusType status = null, PimsLease lease = null, short? regionCode = null, bool? isCoreInventory = null, bool? isPointOfInterest = null, bool? isOtherInterest = null, bool? isDisposed = null, bool? isRetired = null)
{
type ??= EntityHelper.CreatePropertyType($"Land-{pid}");
classification ??= EntityHelper.CreatePropertyClassificationType($"Class-{pid}");
address ??= EntityHelper.CreateAddress(pid);
tenure ??= EntityHelper.CreatePropertyTenureType($"Tenure-{pid}");
type ??= CreatePropertyType($"Land-{pid}");
classification ??= CreatePropertyClassificationType($"Class-{pid}");
address ??= CreateAddress(pid);
tenure ??= CreatePropertyTenureType($"Tenure-{pid}");

areaUnit ??= EntityHelper.CreatePropertyAreaUnitType($"Sqft-{pid}");
dataSource ??= EntityHelper.CreateDataSourceType($"LIS-{pid}");
status ??= EntityHelper.CreatePropertyStatusType($"Status-{pid}");
var property = new Entity.PimsProperty(pid, type, classification, address, new Entity.PimsPropPropTenureType { PropertyTenureTypeCodeNavigation = tenure }, areaUnit, dataSource, DateTime.UtcNow, status)
areaUnit ??= CreatePropertyAreaUnitType($"Sqft-{pid}");
dataSource ??= CreateDataSourceType($"LIS-{pid}");
status ??= CreatePropertyStatusType($"Status-{pid}");

var property = new PimsProperty(pid, type, classification, address, new PimsPropPropTenureType { PropertyTenureTypeCodeNavigation = tenure }, areaUnit, dataSource, DateTime.UtcNow, status)
{
PropertyId = pid,
Pin = pin,
ConcurrencyControlNumber = 1,
Location = new NetTopologySuite.Geometries.Point(0, 0),
Location = new NetTopologySuite.Geometries.Point(0, 0) { SRID = SpatialReference.BCALBERS },
SurplusDeclarationTypeCode = "SURPLUS",
IsRetired = false,
};

if (lease != null)
{
lease.PimsPropertyLeases.Add(new Entity.PimsPropertyLease() { Property = property, Lease = lease });
lease.PimsPropertyLeases.Add(new PimsPropertyLease() { Property = property, Lease = lease });
}
if (regionCode.HasValue)
{
Expand All @@ -65,6 +68,11 @@ public static Entity.PimsProperty CreateProperty(int pid, int? pin = null, Entit
{
property.IsDisposed = isDisposed.Value;
}
if (isRetired.HasValue)
{
property.IsRetired = isRetired.Value;
}

return property;
}

Expand All @@ -81,7 +89,7 @@ public static Entity.PimsProperty CreateProperty(int pid, int? pin = null, Entit
/// <param name="areaUnit"></param>
/// <param name="dataSource"></param>
/// <returns></returns>
public static Entity.PimsProperty CreateProperty(this PimsContext context, int pid, int? pin = null, Entity.PimsPropertyType type = null, Entity.PimsPropertyClassificationType classification = null, Entity.PimsAddress address = null, Entity.PimsPropertyTenureType tenure = null, Entity.PimsAreaUnitType areaUnit = null, Entity.PimsDataSourceType dataSource = null, Entity.PimsPropertyStatusType status = null, Geometry location = null)
public static PimsProperty CreateProperty(this PimsContext context, int pid, int? pin = null, PimsPropertyType type = null, PimsPropertyClassificationType classification = null, PimsAddress address = null, PimsPropertyTenureType tenure = null, PimsAreaUnitType areaUnit = null, PimsDataSourceType dataSource = null, PimsPropertyStatusType status = null, Geometry location = null, bool isRetired = false)
{
type ??= context.PimsPropertyTypes.FirstOrDefault() ?? throw new InvalidOperationException("Unable to find a property type.");
classification ??= context.PimsPropertyClassificationTypes.FirstOrDefault() ?? throw new InvalidOperationException("Unable to find a property classification type.");
Expand All @@ -90,9 +98,10 @@ public static Entity.PimsProperty CreateProperty(this PimsContext context, int p
areaUnit ??= context.PimsAreaUnitTypes.FirstOrDefault() ?? throw new InvalidOperationException("Unable to find a property area unit type.");
dataSource ??= context.PimsDataSourceTypes.FirstOrDefault() ?? throw new InvalidOperationException("Unable to find a property data source type.");
status ??= context.PimsPropertyStatusTypes.FirstOrDefault() ?? throw new InvalidOperationException("Unable to find a property status type.");
var lease = context.PimsLeases.FirstOrDefault() ?? EntityHelper.CreateLease(pid);
var property = EntityHelper.CreateProperty(pid, pin, type, classification, address, tenure, areaUnit, dataSource, status);
var lease = context.PimsLeases.FirstOrDefault() ?? CreateLease(pid);
var property = CreateProperty(pid, pin, type, classification, address, tenure, areaUnit, dataSource, status);
property.Location = location;
property.IsRetired = isRetired;
context.PimsProperties.Add(property);
return property;
}
Expand Down
9 changes: 6 additions & 3 deletions source/backend/tests/unit/api/Services/PropertyServiceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Pims.Api.Helpers.Exceptions;
using Pims.Api.Models.CodeTypes;
using Pims.Api.Services;
using Pims.Core.Exceptions;
using Pims.Core.Extensions;
using Pims.Core.Test;
using Pims.Dal.Constants;
Expand Down Expand Up @@ -120,7 +121,7 @@ public void Update_Property_No_Reprojection_Success()
var service = this.CreatePropertyServiceWithPermissions(Permissions.PropertyView, Permissions.PropertyEdit);
var repository = this._helper.GetService<Mock<IPropertyRepository>>();
repository.Setup(x => x.Update(It.IsAny<PimsProperty>(), It.IsAny<bool>())).Returns(property);

repository.Setup(x => x.GetById(It.IsAny<long>())).Returns(property);
var coordinateService = this._helper.GetService<Mock<ICoordinateTransformService>>();
coordinateService.Setup(x => x.TransformCoordinates(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Coordinate>()));

Expand All @@ -135,7 +136,7 @@ public void Update_Property_No_Reprojection_Success()

// Assert
repository.Verify(x => x.Update(It.IsAny<PimsProperty>(), It.IsAny<bool>()), Times.Once);
coordinateService.Verify(x => x.TransformCoordinates(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Coordinate>()), Times.Never);
coordinateService.Verify(x => x.TransformCoordinates(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Coordinate>()));
}

[Fact]
Expand All @@ -147,6 +148,7 @@ public void Update_Property_With_Reprojection_Success()
var service = this.CreatePropertyServiceWithPermissions(Permissions.PropertyView, Permissions.PropertyEdit);
var repository = this._helper.GetService<Mock<IPropertyRepository>>();
repository.Setup(x => x.Update(It.IsAny<PimsProperty>(), It.IsAny<bool>())).Returns(property);
repository.Setup(x => x.GetById(It.IsAny<long>())).Returns(property);

var projected = new Coordinate(14000, 9200);
var coordinateService = this._helper.GetService<Mock<ICoordinateTransformService>>();
Expand All @@ -162,7 +164,7 @@ public void Update_Property_With_Reprojection_Success()
var updatedProperty = service.Update(newValues);

// Assert
coordinateService.Verify(x => x.TransformCoordinates(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Coordinate>()), Times.Once);
coordinateService.Verify(x => x.TransformCoordinates(It.IsAny<int>(), It.IsAny<int>(), It.IsAny<Coordinate>()));
repository.Verify(x => x.Update(It.Is<PimsProperty>(p => p.Location.Coordinate.Equals(projected)), It.IsAny<bool>()), Times.Once);
}

Expand All @@ -177,6 +179,7 @@ public void Update_Property_KeyNotFound()

var repository = this._helper.GetService<Mock<IPropertyRepository>>();
repository.Setup(x => x.Update(property, It.IsAny<bool>())).Throws<KeyNotFoundException>();
repository.Setup(x => x.GetById(It.IsAny<long>())).Throws<KeyNotFoundException>();

// Assert
Assert.Throws<KeyNotFoundException>(() => service.Update(property));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using FluentAssertions.Extensions;
using Moq;
using NetTopologySuite.Geometries;
using Pims.Api.Models.CodeTypes;
using Pims.Api.Services;
using Pims.Core.Test;
using Pims.Dal.Entities;
Expand Down Expand Up @@ -167,6 +168,7 @@ public void UpdateProperties_MatchProperties_PID_NewProperty_Success()
researchFile.ConcurrencyControlNumber = 1;

var property = EntityHelper.CreateProperty(12345);
property.Location.SRID = SpatialReference.WGS84;
researchFile.PimsPropertyResearchFiles = new List<PimsPropertyResearchFile>() { new PimsPropertyResearchFile() { Property = property } };

var repository = this._helper.GetService<Mock<IResearchFileRepository>>();
Expand Down Expand Up @@ -211,6 +213,7 @@ public void UpdateProperties_MatchProperties_PIN_NewProperty_Success()
researchFile.ConcurrencyControlNumber = 1;

var property = EntityHelper.CreateProperty(12345, 54321);
property.Location.SRID = SpatialReference.WGS84;
property.Pid = null;
researchFile.PimsPropertyResearchFiles = new List<PimsPropertyResearchFile>() { new PimsPropertyResearchFile() { Property = property } };

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Drawing;
using System.Linq;
using FluentAssertions;
using Pims.Core.Exceptions;
using Pims.Core.Extensions;
using Pims.Core.Test;
using Pims.Dal.Entities;
Expand Down Expand Up @@ -501,12 +502,35 @@ public void GetByPin_Success()
#endregion

#region Update
[Theory]
[InlineData("test", 200, null)]
[InlineData("test", 200, false)]
public void Update_Property_Success_Not_Retired(string propertyDescription, int pid, bool? isRetired)
{
// Arrange
var repository = CreateRepositoryWithPermissions(Permissions.PropertyView, Permissions.PropertyEdit);
var property = EntityHelper.CreateProperty(1, isRetired: isRetired);
_helper.AddAndSaveChanges(property);

var newValues = new Entity.PimsProperty();
property.CopyValues(newValues);
newValues.Description = propertyDescription;
newValues.Pid = pid;

// Act
var updatedProperty = repository.Update(newValues);

// Assert
updatedProperty.Description.Should().Be(propertyDescription);
updatedProperty.Pid.Should().Be(pid);
}

[Fact]
public void Update_Property_Success()
public void Update_Property_Retired_Violation()
{
// Arrange
var repository = CreateRepositoryWithPermissions(Permissions.PropertyView, Permissions.PropertyEdit);
var property = EntityHelper.CreateProperty(1);
var property = EntityHelper.CreateProperty(1, isRetired: true);
_helper.AddAndSaveChanges(property);

var newValues = new Entity.PimsProperty();
Expand All @@ -515,11 +539,11 @@ public void Update_Property_Success()
newValues.Pid = 200;

// Act
var updatedProperty = repository.Update(newValues);
Action act = () => repository.Update(newValues);

// Assert
updatedProperty.Description.Should().Be("test");
updatedProperty.Pid.Should().Be(200);
var exception = act.Should().Throw<BusinessRuleViolationException>();
exception.WithMessage("Retired records are referenced for historical purposes only and cannot be edited or deleted.");
}

[Fact]
Expand Down Expand Up @@ -582,7 +606,7 @@ public void TransferFileProperties_Success_Owned()


// Act
var transferredProperty = repository.TransferFileProperty(property, new Models.PropertyOwnershipState() { isPropertyOfInterest = true, isOwned = true});
var transferredProperty = repository.TransferFileProperty(property, new Models.PropertyOwnershipState() { isPropertyOfInterest = true, isOwned = true });
context.CommitTransaction();

// Assert
Expand Down
2 changes: 2 additions & 0 deletions source/frontend/src/assets/scss/_variables.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,6 @@
summaryBorderColor: $summary-border-color;
summaryColor: $summary-color;
textColor: $text-color;
expiredColor: $expired-color;
expiredBackgroundColor: $expired-background-color;
}
2 changes: 2 additions & 0 deletions source/frontend/src/assets/scss/colors.scss
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ $subtle-color: #aaaaaa;
$summary-color: #f9f1c6;
$summary-border-color: #fcba19;
$font-warning-color: #6c4a00;
$expired-color: #6c4a00;
$expired-background-color: #f9f1c6;
// table ui
$table-hover-color: #dee2e6;
$table-header-text-color: #494949;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import { ApiGen_Concepts_AcquisitionFileProperty } from '@/models/api/generated/

const generateFn = jest.fn();
const getAcquisitionFileFn = jest.fn<ApiGen_Concepts_AcquisitionFile | undefined, any[]>();
const getAcquisitionFilePropertiesFn = jest.fn<ApiGen_Concepts_AcquisitionFileProperty | undefined, any[]>();
const getAcquisitionFilePropertiesFn = jest.fn<
ApiGen_Concepts_AcquisitionFileProperty | undefined,
any[]
>();
const getPersonConceptFn = jest.fn();
const getOrganizationConceptFn = jest.fn();

Expand All @@ -38,7 +41,7 @@ jest.mock('@/hooks/pims-api/useApiContacts');

let currentStore: MockStoreEnhanced<any, {}>;
const mockStore = configureMockStore([thunk]);
const getStore = (values?: any) => {
const getStore = (values?: any) => {
currentStore = mockStore(values ?? {});
return currentStore;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,26 @@ describe('MotiInventoryHeader component', () => {
},
isLoading: false,
});
// PID is shown
// land parcel type is shown
expect(result.getByText(testProperty?.propertyType?.description as string)).toBeVisible();
});

it(`shows "retired" indicator for retired properties`, async () => {
const testProperty: ApiGen_Concepts_Property = {
...getEmptyProperty(),
isRetired: true,
};
const result = setup({
composedProperty: {
...defaultComposedProperty,
pimsProperty: testProperty,
},
isLoading: false,
});
// "retired" indicator is shown
expect(result.getByText(/expired/i)).toBeVisible();
});

it('allows the active property to be zoomed in', async () => {
const testProperty: ApiGen_Concepts_Property = {} as any;

Expand Down
Loading
Loading