code review updates.

bcgov · Jun 27, 2023 · de87970 · de87970
1 parent 5c747ba
commit de87970
Show file tree

Hide file tree

Showing 38 changed files with 298 additions and 118 deletions.
diff --git a/source/backend/api/Areas/Leases/Controllers/LeaseController.cs b/source/backend/api/Areas/Leases/Controllers/LeaseController.cs
@@ -53,7 +53,7 @@ public LeaseController(ILeaseService leaseService, IMapper mapper)
         [Produces("application/json")]
         [ProducesResponseType(typeof(IEnumerable<Api.Models.Concepts.LeaseModel>), 200)]
         [SwaggerOperation(Tags = new[] { "lease" })]
-        public IActionResult GetLeaseConcept(int id)
+        public IActionResult GetLease(int id)
         {
             var lease = _leaseService.GetById(id);
             var mapped = _mapper.Map<Api.Models.Concepts.LeaseModel>(lease);
@@ -78,11 +78,11 @@ public IActionResult AddLease(Api.Models.Concepts.LeaseModel leaseModel, [FromQu
             return new JsonResult(_mapper.Map<Api.Models.Concepts.LeaseModel>(lease));
         }
 
-    /// <summary>
-    /// Update the specified lease, and attached properties.
-    /// </summary>
-    /// <returns></returns>
-    [HttpPut("{id:long}")]
+        /// <summary>
+        /// Update the specified lease, and attached properties.
+        /// </summary>
+        /// <returns></returns>
+        [HttpPut("{id:long}")]
         [HasPermission(Permissions.LeaseEdit)]
         [Produces("application/json")]
         [ProducesResponseType(typeof(IEnumerable<Api.Models.Concepts.LeaseModel>), 200)]

diff --git a/source/backend/api/Areas/Leases/Controllers/LeaseTermController.cs b/source/backend/api/Areas/Leases/Controllers/LeaseTermController.cs
@@ -55,7 +55,7 @@ public LeaseTermController(ILeaseTermService leaseTermService, IMapper mapper, I
         /// Update the specified term on the passed lease.
         /// </summary>
         /// <returns></returns>
-        [HttpGet("{leaseId:long}/term")]
+        [HttpGet("{leaseId:long}/terms")]
         [HasPermission(Permissions.LeaseView)]
         [Produces("application/json")]
         [ProducesResponseType(typeof(IEnumerable<LeaseTermModel>), 200)]
@@ -79,7 +79,7 @@ public IActionResult GetTerms(long leaseId)
         /// Update the specified term on the passed lease.
         /// </summary>
         /// <returns></returns>
-        [HttpPost("{leaseId:long}/term")]
+        [HttpPost("{leaseId:long}/terms")]
         [HasPermission(Permissions.LeaseAdd)]
         [Produces("application/json")]
         [ProducesResponseType(typeof(LeaseTermModel), 200)]
@@ -104,7 +104,7 @@ public IActionResult AddTerm(long leaseId, [FromBody] LeaseTermModel termModel)
         /// Update the specified term on the passed lease.
         /// </summary>
         /// <returns></returns>
-        [HttpPut("{leaseId:long}/term/{termId:long}")]
+        [HttpPut("{leaseId:long}/terms/{termId:long}")]
         [HasPermission(Permissions.LeaseEdit)]
         [Produces("application/json")]
         [ProducesResponseType(typeof(LeaseTermModel), 200)]
@@ -129,7 +129,7 @@ public IActionResult UpdateTerm(long leaseId, long termId, [FromBody] LeaseTermM
         /// Delete the specified term on the passed lease.
         /// </summary>
         /// <returns></returns>
-        [HttpDelete("{leaseId:long}/term")]
+        [HttpDelete("{leaseId:long}/terms")]
         [HasPermission(Permissions.LeaseEdit)]
         [Produces("application/json")]
         [ProducesResponseType(typeof(LeaseTermModel), 200)]

diff --git a/source/backend/api/Areas/Leases/Controllers/SecurityDepositController.cs b/source/backend/api/Areas/Leases/Controllers/SecurityDepositController.cs
@@ -151,10 +151,10 @@ public IActionResult UpdateDeposit(long leaseId, long depositId, Pims.Api.Models
         [HttpDelete("{leaseId:long}/deposits/{depositId:long}")]
         [HasPermission(Permissions.LeaseEdit)]
         [Produces("application/json")]
-        [ProducesResponseType(typeof(void), 200)]
+        [ProducesResponseType(typeof(bool), 200)]
         [SwaggerOperation(Tags = new[] { "lease" })]
         [TypeFilter(typeof(NullJsonResultFilter))]
-        public void DeleteDeposit(long leaseId, long depositId, [FromBody] Pims.Api.Models.Concepts.SecurityDepositModel deleteRequest)
+        public bool DeleteDeposit(long leaseId, long depositId, [FromBody] Pims.Api.Models.Concepts.SecurityDepositModel deleteRequest)
         {
             _logger.LogInformation(
                 "Request received by Controller: {Controller}, Action: {ControllerAction}, User: {User}, DateTime: {DateTime}",
@@ -173,7 +173,7 @@ public void DeleteDeposit(long leaseId, long depositId, [FromBody] Pims.Api.Mode
                 throw new BadRequestException($"Invalid Security Deposit Id");
             }
             var depositEntity = _mapper.Map<PimsSecurityDeposit>(deleteRequest);
-            _securityDepositService.DeleteLeaseDeposit(depositEntity);
+            return _securityDepositService.DeleteLeaseDeposit(depositEntity);
         }
 
         /// <summary>

diff --git a/source/backend/api/Areas/Leases/Models/Search/LeaseMap.cs b/source/backend/api/Areas/Leases/Models/Search/LeaseMap.cs
@@ -0,0 +1,23 @@
+using System.Linq;
+using Mapster;
+using Pims.Dal.Helpers.Extensions;
+using Entity = Pims.Dal.Entities;
+using Model = Pims.Api.Areas.Lease.Models.Search;
+
+namespace Pims.Api.Areas.Lease.Mapping.Search
+{
+    public class LeaseMap : IRegister
+    {
+        public void Register(TypeAdapterConfig config)
+        {
+            config.NewConfig<Entity.PimsLease, Model.LeaseModel>()
+                .Map(dest => dest.Id, src => src.LeaseId)
+                .Map(dest => dest.LFileNo, src => src.LFileNo)
+                .Map(dest => dest.ExpiryDate, src => src.GetExpiryDate())
+                .Map(dest => dest.ProgramName, src => src.GetProgramName())
+                .Map(dest => dest.TenantNames, src => src.PimsLeaseTenants.Select(t => t.GetTenantName()))
+                .Map(dest => dest.Properties, src => src.GetProperties())
+                .Map(dest => dest.StatusType, src => src.LeaseStatusTypeCodeNavigation);
+        }
+    }
+}
diff --git a/source/backend/api/Areas/Leases/Models/Search/PropertyMap.cs b/source/backend/api/Areas/Leases/Models/Search/PropertyMap.cs
@@ -0,0 +1,18 @@
+using Mapster;
+using Entity = Pims.Dal.Entities;
+using Model = Pims.Api.Areas.Lease.Models.Search;
+
+namespace Pims.Api.Areas.Lease.Mapping.Search
+{
+    public class PropertyMap : IRegister
+    {
+        public void Register(TypeAdapterConfig config)
+        {
+            config.NewConfig<Entity.PimsProperty, Model.PropertyModel>()
+                .Map(dest => dest.Id, src => src.PropertyId)
+                .Map(dest => dest.Pin, src => src.Pin)
+                .Map(dest => dest.Pid, src => src.Pid)
+                .Map(dest => dest.Address, src => src.Address);
+        }
+    }
+}
diff --git a/source/backend/api/Services/ISecurityDepositService.cs b/source/backend/api/Services/ISecurityDepositService.cs
@@ -13,6 +13,6 @@ public interface ISecurityDepositService
 
         void UpdateLeaseDepositNote(long leaseId, string note);
 
-        void DeleteLeaseDeposit(PimsSecurityDeposit deposit);
+        bool DeleteLeaseDeposit(PimsSecurityDeposit deposit);
     }
 }
diff --git a/source/backend/api/Services/SecurityDepositService.cs b/source/backend/api/Services/SecurityDepositService.cs
@@ -70,14 +70,15 @@ public void UpdateLeaseDepositNote(long leaseId, string note)
             _leaseRepository.CommitTransaction();
         }
 
-        public void DeleteLeaseDeposit(PimsSecurityDeposit deposit)
+        public bool DeleteLeaseDeposit(PimsSecurityDeposit deposit)
         {
             _logger.LogInformation("Deleting lease deposit for lease id {leaseId}, deposit id {depositId}", deposit.LeaseId, deposit.SecurityDepositId);
             _user.ThrowIfNotAuthorized(Permissions.LeaseEdit);
             ValidateDeletionRules(deposit);
 
-            _securityDepositRepository.Delete(deposit.SecurityDepositId);
+            bool deleted = _securityDepositRepository.Delete(deposit.SecurityDepositId);
             _securityDepositRepository.CommitTransaction();
+            return deleted;
         }
 
         private void ValidateDeletionRules(PimsSecurityDeposit deposit)

diff --git a/source/backend/dal/Repositories/Interfaces/ISecurityDepositRepository.cs b/source/backend/dal/Repositories/Interfaces/ISecurityDepositRepository.cs
@@ -16,6 +16,6 @@ public interface ISecurityDepositRepository : IRepository<PimsSecurityDeposit>
 
         PimsSecurityDeposit Update(PimsSecurityDeposit securityDeposit);
 
-        void Delete(long id);
+        bool Delete(long id);
     }
 }
diff --git a/source/backend/dal/Repositories/SecurityDepositRepository.cs b/source/backend/dal/Repositories/SecurityDepositRepository.cs
@@ -67,7 +67,7 @@ public PimsSecurityDeposit Update(PimsSecurityDeposit securityDeposit)
             return updatedSecurityDeposit.Entity;
         }
 
-        public void Delete(long id)
+        public bool Delete(long id)
         {
             PimsSecurityDeposit deposit = this.Context.PimsSecurityDeposits
                 .Where(x => x.SecurityDepositId == id)
@@ -77,6 +77,8 @@ public void Delete(long id)
                 this.Context.Remove(deposit.PimsSecurityDepositHolder);
             }
             this.Context.Remove(deposit);
+
+            return true;
         }
     }
 }
diff --git a/source/frontend/src/features/admin/access-request/models.ts b/source/frontend/src/features/admin/access-request/models.ts
@@ -3,7 +3,7 @@ import { UserTypes } from '@/constants/index';
 import { Api_AccessRequest } from '@/models/api/AccessRequest';
 import { NumberFieldValue } from '@/typings/NumberFieldValue';
 import { getPreferredContactMethodValue } from '@/utils/contactMethodUtil';
-import { fromTypeCode, stringToNull, toTypeCode } from '@/utils/formUtils';
+import { fromTypeCode, stringToUndefined, toTypeCode } from '@/utils/formUtils';
 
 export class FormAccessRequest {
   public id: NumberFieldValue;
@@ -56,9 +56,9 @@ export class FormAccessRequest {
 
   public toApi(): Api_AccessRequest {
     return {
-      id: stringToNull(this.id),
-      userId: stringToNull(this.userId),
-      roleId: stringToNull(this.roleId),
+      id: stringToUndefined(this.id),
+      userId: stringToUndefined(this.userId),
+      roleId: stringToUndefined(this.roleId),
       note: this.note,
       accessRequestStatusTypeCode: toTypeCode(this.accessRequestStatusTypeCodeId),
       regionCode: this.regionCodeId ? toTypeCode<number>(this.regionCodeId) : undefined,

diff --git a/source/frontend/src/features/admin/financial-codes/models.ts b/source/frontend/src/features/admin/financial-codes/models.ts
@@ -1,7 +1,7 @@
 import moment from 'moment';
 
 import { Api_FinancialCode } from '@/models/api/FinancialCode';
-import { stringToNull } from '@/utils/formUtils';
+import { stringToUndefined } from '@/utils/formUtils';
 
 export class FinancialCodeForm {
   id?: number;
@@ -22,7 +22,7 @@ export class FinancialCodeForm {
       description: this.description,
       displayOrder: this.displayOrder !== undefined ? Number(this.displayOrder) : undefined,
       effectiveDate: this.effectiveDate,
-      expiryDate: stringToNull(this.expiryDate),
+      expiryDate: stringToUndefined(this.expiryDate),
     };
   }
 

diff --git a/source/frontend/src/features/contacts/contactUtils.ts b/source/frontend/src/features/contacts/contactUtils.ts
@@ -19,7 +19,7 @@ import {
 } from '@/interfaces/editable-contact';
 import { IContactPerson } from '@/interfaces/IContact';
 import { Api_Organization, Api_OrganizationPerson } from '@/models/api/Organization';
-import { fromTypeCode, stringToBoolean, stringToNull, toTypeCode } from '@/utils/formUtils';
+import { fromTypeCode, stringToBoolean, stringToUndefined, toTypeCode } from '@/utils/formUtils';
 import { formatFullName } from '@/utils/personUtils';
 
 import { Api_Address } from './../../models/api/Address';
@@ -228,7 +228,7 @@ export function apiAddressToFormAddress(address?: IBaseAddress) {
 function formContactMethodToApiContactMethod(formContactMethod: IEditableContactMethodForm) {
   return {
     ...formContactMethod,
-    value: stringToNull(formContactMethod.value),
+    value: stringToUndefined(formContactMethod.value),
     contactMethodTypeCode: toTypeCode(formContactMethod.contactMethodTypeCode),
   } as IEditableContactMethod;
 }

diff --git a/source/frontend/src/features/leases/add/AddLeaseContainer.test.tsx b/source/frontend/src/features/leases/add/AddLeaseContainer.test.tsx
@@ -6,6 +6,7 @@ import { createMemoryHistory } from 'history';
 import { noop } from 'lodash';
 
 import { mockLookups } from '@/mocks/lookups.mock';
+import { Api_Lease } from '@/models/api/Lease';
 import { UserOverrideCode } from '@/models/api/UserOverrideCode';
 import { lookupCodesSlice } from '@/store/slices/lookupCodes';
 import { act, fillInput, renderAsync, RenderOptions, screen, waitFor } from '@/utils/test-utils';
@@ -97,10 +98,10 @@ describe('AddLeaseContainer component', () => {
       userEvent.click(getByText(/Save/i));
     });
     await waitFor(() => {
-      expect(mockAxios.history.post[0].data).toEqual(leaseData);
+      expect(JSON.parse(mockAxios.history.post[0].data)).toEqual(leaseData);
     });
 
-    expect(mockAxios.history.post[0].data).toEqual(leaseData);
+    expect(JSON.parse(mockAxios.history.post[0].data)).toEqual(leaseData);
   });
 
   it('triggers the confirm popup', async () => {
@@ -146,7 +147,7 @@ describe('AddLeaseContainer component', () => {
       .reply(409, { error: 'test message', errorCode: UserOverrideCode.ADD_LOCATION_TO_PROPERTY });
     act(() => userEvent.click(getByText(/Save/i)));
     await waitFor(() => {
-      expect(mockAxios.history.post[0].data).toEqual(leaseData);
+      expect(JSON.parse(mockAxios.history.post[0].data)).toEqual(leaseData);
     });
 
     const popup = await screen.findByText(/test message/i);
@@ -156,9 +157,91 @@ describe('AddLeaseContainer component', () => {
       userEvent.click(await screen.findByText('Acknowledge & Continue'));
     });
 
-    expect(mockAxios.history.post[0].data).toEqual(leaseData);
+    expect(JSON.parse(mockAxios.history.post[0].data)).toEqual(leaseData);
   });
 });
-
-const leaseData =
-  '{"expiryDate":"2020-01-02","startDate":"2020-01-01","amount":0,"paymentReceivableType":{"id":"RCVBL"},"categoryType":null,"purposeType":{"id":"BCFERRIES"},"responsibilityType":null,"initiatorType":null,"statusType":{"id":"DRAFT"},"type":{"id":"LICONSTRC"},"region":{"id":1},"programType":{"id":"BCFERRIES"},"returnNotes":"","motiName":"","properties":[],"isResidential":false,"isCommercialBuilding":false,"isOtherImprovement":false,"consultations":[{"id":0,"consultationType":{"id":"1STNATION"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null},{"id":0,"consultationType":{"id":"STRATRE"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null},{"id":0,"consultationType":{"id":"REGPLANG"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null},{"id":0,"consultationType":{"id":"REGPRPSVC"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null},{"id":0,"consultationType":{"id":"DISTRICT"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null},{"id":0,"consultationType":{"id":"HQ"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null},{"id":0,"consultationType":{"id":"OTHER"},"consultationStatusType":{"id":"UNKNOWN"},"parentLeaseId":0,"otherDescription":null}],"tenants":[],"terms":[],"insurances":[]}';
+const leaseData: Api_Lease = {
+  startDate: '2020-01-01',
+  amount: 0,
+  paymentReceivableType: { id: 'RCVBL' },
+  purposeType: { id: 'BCFERRIES' },
+  statusType: { id: 'DRAFT' },
+  type: { id: 'LICONSTRC' },
+  region: { id: 1 },
+  programType: { id: 'BCFERRIES' },
+  returnNotes: '',
+  motiName: '',
+  properties: [],
+  isResidential: false,
+  isCommercialBuilding: false,
+  isOtherImprovement: false,
+  responsibilityType: null,
+  categoryType: null,
+  initiatorType: null,
+  otherType: null,
+  otherCategoryType: null,
+  otherProgramType: null,
+  otherPurposeType: null,
+  tfaFileNumber: null,
+  responsibilityEffectiveDate: null,
+  psFileNo: null,
+  note: null,
+  lFileNo: null,
+  description: null,
+  documentationReference: null,
+  expiryDate: '2020-01-02',
+  tenants: [],
+  terms: [],
+  insurances: [],
+  consultations: [
+    {
+      id: 0,
+      consultationType: { id: '1STNATION' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+    {
+      id: 0,
+      consultationType: { id: 'STRATRE' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+    {
+      id: 0,
+      consultationType: { id: 'REGPLANG' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+    {
+      id: 0,
+      consultationType: { id: 'REGPRPSVC' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+    {
+      id: 0,
+      consultationType: { id: 'DISTRICT' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+    {
+      id: 0,
+      consultationType: { id: 'HQ' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+    {
+      id: 0,
+      consultationType: { id: 'OTHER' },
+      consultationStatusType: { id: 'UNKNOWN' },
+      parentLeaseId: 0,
+      otherDescription: null,
+    },
+  ],
+};
diff --git a/source/frontend/src/features/leases/detail/LeasePages/deposits/DepositsContainer.test.tsx b/source/frontend/src/features/leases/detail/LeasePages/deposits/DepositsContainer.test.tsx
@@ -19,8 +19,8 @@ import {
   waitFor,
 } from '@/utils/test-utils';
 
+import DepositsContainer from './DepositsContainer';
 import { FormLeaseDeposit } from './models/FormLeaseDeposit';
-import { DepositsContainer } from './styles';
 
 const mockAxios = new MockAdapter(axios);
 jest.mock('@react-keycloak/web');