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 6322 - lease and license refactor #3253

Merged
merged 6 commits into from
Jun 27, 2023

Conversation

devinleighsmith
Copy link
Collaborator

No description provided.

@devinleighsmith devinleighsmith added the enhancement New feature or request label Jun 15, 2023
@devinleighsmith devinleighsmith self-assigned this Jun 15, 2023
@devinleighsmith devinleighsmith changed the title Psp 6322 Psp 6322 - lease and license refactor Jun 15, 2023
@devinleighsmith devinleighsmith force-pushed the psp-6322 branch 2 times, most recently from 756871b to 59c2d7b Compare June 20, 2023 23:10
@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

/// Update the specified lease, and attached properties.
/// </summary>
/// <returns></returns>
[HttpPut("{id:long}")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: indentation seems off

/// <returns></returns>
[HttpGet("concept/{id:long}")]
[HasPermission(Permissions.LeaseView)]
[Produces("application/json")]
[ProducesResponseType(typeof(IEnumerable<Api.Models.Concepts.LeaseModel>), 200)]
[SwaggerOperation(Tags = new[] { "lease" })]
public IActionResult GetLeaseConcept(int id)
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: now that there is only the lease concept, maybe we can rename this method


return new JsonResult(_mapper.Map<IEnumerable<LeaseTenantModel>>(updatedLease));
}

/// <summary>
/// Update the specified tenants on the passed lease.
/// </summary>
/// <returns></returns>
[HttpPut("{leaseId:long}/tenants")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: a tad more restful would be to pass the tenant id as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

oh, nvm i thought it was for a single tenant

/// Update the specified term on the passed lease.
/// </summary>
/// <returns></returns>
[HttpGet("{leaseId:long}/term")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: most of our paths are plural

[HasPermission(Permissions.LeaseEdit)]
[Produces("application/json")]
[ProducesResponseType(typeof(IEnumerable<LeaseModel>), 200)]
[ProducesResponseType(typeof(void), 200)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: the other delete methods return a boolean.

@@ -76,23 +76,7 @@ public PimsLease Get(long id)

PimsLease lease = this.Context.PimsLeases.AsSplitQuery()
Copy link
Collaborator

Choose a reason for hiding this comment

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

thank you for cleaning this one up!

});
});

const leaseData: Api_Lease = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: imo the typed version is more maintainable

@@ -52,9 +52,10 @@ export const AddLeaseContainer: React.FunctionComponent<
formikHelpers: FormikHelpers<LeaseFormModel>,
userOverrideCodes: UserOverrideCode[],
) => {
const leaseApi = leaseFormModel.toApi();
const leaseApi = LeaseFormModel.toApi(leaseFormModel);
Copy link
Collaborator

Choose a reason for hiding this comment

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

q: given that the leaseFormModel has all the information for the transformation to the API version calling it as static is not necessary. What advantage do you see in it being static?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are some problems with non-static functions in classes that we were previously working around with any or as,
primarily it prevents some usages of ellipsis (...) which can be an issue for default* objects as well as test methods.

using static methods allows typescript to treat these models as if they were simple objects which makes transformations easier.

if (!!updatedLease?.id) {
setLease(updatedLease);
if (lease && lease.id) {
let request: Api_SecurityDepositReturn = returnDepositForm.toInterfaceModel();
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: toInterfaceModel -> toApi

@FuriousLlama FuriousLlama added the tech-debt Removing technical debt label Jun 21, 2023
@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@codecov
Copy link

codecov bot commented Jun 27, 2023

Codecov Report

Merging #3253 (1037e5d) into dev (65386a7) will decrease coverage by 0.90%.
The diff coverage is 55.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3253      +/-   ##
==========================================
- Coverage   73.47%   72.57%   -0.90%     
==========================================
  Files        1285     1291       +6     
  Lines       29940    29944       +4     
  Branches     5475     5527      +52     
==========================================
- Hits        21998    21733     -265     
- Misses       7701     7972     +271     
+ Partials      241      239       -2     
Flag Coverage Δ
unittests 72.57% <55.01%> (-0.90%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...pi/Areas/Leases/Controllers/InsuranceController.cs 0.00% <0.00%> (ø)
...nd/api/Areas/Leases/Controllers/LeaseController.cs 72.72% <ø> (+13.46%) ⬆️
...reas/Leases/Controllers/PropertyLeaseController.cs 0.00% <0.00%> (ø)
...as/Leases/Controllers/SecurityDepositController.cs 0.00% <0.00%> (ø)
...ses/Controllers/SecurityDepositReturnController.cs 0.00% <0.00%> (ø)
...backend/api/Areas/Leases/Models/Search/LeaseMap.cs 100.00% <ø> (ø)
...kend/api/Areas/Leases/Models/Search/PropertyMap.cs 100.00% <ø> (ø)
...d/api/Areas/Reports/Controllers/LeaseController.cs 87.14% <ø> (ø)
...ckend/api/Services/SecurityDepositReturnService.cs 0.00% <0.00%> (ø)
...rce/backend/api/Services/SecurityDepositService.cs 0.00% <0.00%> (ø)
... and 135 more

... and 2 files with indirect coverage changes

@github-actions
Copy link
Contributor

✅ No secrets were detected in the code.

@devinleighsmith devinleighsmith merged commit 0ace726 into bcgov:dev Jun 27, 2023
8 of 9 checks passed
devinleighsmith added a commit that referenced this pull request Jul 6, 2023
* Automation test - Stabilization of some automated features (#3285)

* Automation Projects

* Automation - Projects and Products automation

* Leases refactoring

* Leases refactoring

* Acquisition Files refactoring

* Automation fixes

* Automation - stabilization of some automation features

---------

Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>

* PSP-5111

* CI: Bump version to v3.2.0-56.18

* CI: Bump version to v3.2.0-56.19

* PSP-6286 : Restrict changing final compensation requisition status back to draft (#3281)

---------

Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* CI: Bump version to v3.2.0-56.20

* PSP-5715 : Sec3/Sec6 add forms to acquisition file (#3292)


Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* CI: Bump version to v3.2.0-56.21

* Psp 6322 - lease and license refactor (#3253)

* psp-6322 lease refactor.

* merge corrections.

* lint corrections.

* code review updates.

* merge corrections.

* test corrections.

* CI: Bump version to v3.2.0-56.22

* psp-5361: fix link name for financial codes in admin tools (#3291)

* CI: Bump version to v3.2.0-56.23

* Bump DEV version - IS57 (#3295)

* CI: Bump version to v3.2.0-57.1

* PSP-5951 refactor left hand file navigation - Acquisition ONLY (#3288)

* Update useQuery global hook

* Update shared tabbed forms

* Refactor property tabs to work with state OR routing

* Refactor acquisition left hand navigation to use routing

* Remove setContainerState callback from acquisition forms

* Test corrections

* Improve tab names for properties

* Set header title based on current tab route

* Fix routing issue with requisition compensations

* Update snapshots

* Fix failing tests

* Fix bug when removing property from File

* Add tests for routing in Acquisition View

* Redirect to default tabs when editing and path doesn't match

* Test corrections

* Fix dereference error when accessing non-existing property index

* Remove unused code

* Move router components to router folder

* Test corrections

* CI: Bump version to v3.2.0-57.2

* psp-5791: add assignee to tenant types (#3290)

* CI: Bump version to v3.2.0-57.3

* psp-6246 increase keycloak sync resiliancy via polly, also update deprecated calls from keycloak api. (#3294)

* CI: Bump version to v3.2.0-57.4

* PSP-5716, PSP-5717, PSP-5719: Expropriation forms 1, 5 and 9 (#3303)

* WIP

* Code refactor to accommodate expropriation forms

* Fix styling for ContactPicker

* WIP - Expr Form1

* ContactInputView fixes

* Refactor interest holder property table into generic file property table

* Expropriation Form1 - WIP

* Create generic wrapper around form items to provide validation error feedback

* Add impacted properties table to Expropriation Form 1

* Update snapshots

* Add remaining expropriation forms

* Make forms more testable

* Unit tests

* CI: Bump version to v3.2.0-57.5

* psp-5860 limit lease and license access by region. (#3298)

* psp-5860 limit lease and license access by region.

* PR updates.

* lint fixes.

* fix null vs undefined on project display.

* correct null vs undefined at model level.

* removed dead code.

* CI: Bump version to v3.2.0-57.6

* psp-6428 fix layers colors (#3304)

Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>
Co-authored-by: Alejandro Sanchez <emailforasr@gmail.com>

* CI: Bump version to v3.2.0-57.7

* Merge fixes

* CI: Bump version to v4.0.0-57.8

---------

Co-authored-by: Sue Tairaku <42981334+stairaku@users.noreply.github.com>
Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>
Co-authored-by: PryancaJSharma <99448336+PryancaJSharma@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Eduardo <eddherrera@users.noreply.github.com>
Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>
Co-authored-by: Aman Monga <53246811+buddy326@users.noreply.github.com>
devinleighsmith added a commit that referenced this pull request Jul 20, 2023
* psp-6425 ensure that a new property added to a lease shows up on the map without requiring manual refresh. (#3296)

* psp-6442 filter expired codes, correct code value displays. (#3300)

* HOTFIX: Psp 6408 - separate unmounted vs. error map search behaviour. (#3297)

* separate unmounted vs error behaviour on usemapsearch.

* test corrections.

* correct try/catch location.

* psp-6499 ensure access request region is being populated when accepting an AR. (#3301)

* financial activity code instead of id, concurrency control and update corrections. (#3305)

* bump version number for 4.0 (#3306)

* Fix issue where bad tenant data can cause an NPE. (#3309)

* TEST Release - IS57 (#3312)

* Automation test - Stabilization of some automated features (#3285)

* Automation Projects

* Automation - Projects and Products automation

* Leases refactoring

* Leases refactoring

* Acquisition Files refactoring

* Automation fixes

* Automation - stabilization of some automation features

---------

Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>

* PSP-5111

* CI: Bump version to v3.2.0-56.18

* CI: Bump version to v3.2.0-56.19

* PSP-6286 : Restrict changing final compensation requisition status back to draft (#3281)

---------

Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* CI: Bump version to v3.2.0-56.20

* PSP-5715 : Sec3/Sec6 add forms to acquisition file (#3292)


Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* CI: Bump version to v3.2.0-56.21

* Psp 6322 - lease and license refactor (#3253)

* psp-6322 lease refactor.

* merge corrections.

* lint corrections.

* code review updates.

* merge corrections.

* test corrections.

* CI: Bump version to v3.2.0-56.22

* psp-5361: fix link name for financial codes in admin tools (#3291)

* CI: Bump version to v3.2.0-56.23

* Bump DEV version - IS57 (#3295)

* CI: Bump version to v3.2.0-57.1

* PSP-5951 refactor left hand file navigation - Acquisition ONLY (#3288)

* Update useQuery global hook

* Update shared tabbed forms

* Refactor property tabs to work with state OR routing

* Refactor acquisition left hand navigation to use routing

* Remove setContainerState callback from acquisition forms

* Test corrections

* Improve tab names for properties

* Set header title based on current tab route

* Fix routing issue with requisition compensations

* Update snapshots

* Fix failing tests

* Fix bug when removing property from File

* Add tests for routing in Acquisition View

* Redirect to default tabs when editing and path doesn't match

* Test corrections

* Fix dereference error when accessing non-existing property index

* Remove unused code

* Move router components to router folder

* Test corrections

* CI: Bump version to v3.2.0-57.2

* psp-5791: add assignee to tenant types (#3290)

* CI: Bump version to v3.2.0-57.3

* psp-6246 increase keycloak sync resiliancy via polly, also update deprecated calls from keycloak api. (#3294)

* CI: Bump version to v3.2.0-57.4

* PSP-5716, PSP-5717, PSP-5719: Expropriation forms 1, 5 and 9 (#3303)

* WIP

* Code refactor to accommodate expropriation forms

* Fix styling for ContactPicker

* WIP - Expr Form1

* ContactInputView fixes

* Refactor interest holder property table into generic file property table

* Expropriation Form1 - WIP

* Create generic wrapper around form items to provide validation error feedback

* Add impacted properties table to Expropriation Form 1

* Update snapshots

* Add remaining expropriation forms

* Make forms more testable

* Unit tests

* CI: Bump version to v3.2.0-57.5

* psp-5860 limit lease and license access by region. (#3298)

* psp-5860 limit lease and license access by region.

* PR updates.

* lint fixes.

* fix null vs undefined on project display.

* correct null vs undefined at model level.

* removed dead code.

* CI: Bump version to v3.2.0-57.6

* psp-6428 fix layers colors (#3304)

Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>
Co-authored-by: Alejandro Sanchez <emailforasr@gmail.com>

* CI: Bump version to v3.2.0-57.7

* Merge fixes

* CI: Bump version to v4.0.0-57.8

---------

Co-authored-by: Sue Tairaku <42981334+stairaku@users.noreply.github.com>
Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>
Co-authored-by: PryancaJSharma <99448336+PryancaJSharma@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Eduardo <eddherrera@users.noreply.github.com>
Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>
Co-authored-by: Aman Monga <53246811+buddy326@users.noreply.github.com>

* CI: Bump version to v4.0.0-57.9

* psp-6495 Correct issue with object matching not working as expected. (#3310)

* HOTFIX: h0074 standardize agreement owner string other name. (#3315)

* h0074 standardize agreement owner string other name.

* unit test corrections.

* use financial code instead of id in generated h120. (#3316)

Co-authored-by: Alejandro Sanchez <emailforasr@gmail.com>

* Increment hotfix version in UAT (#3321)

* PSP-6522 : Display user friendly error when compensation req does not… (#3323)

Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* CI: Bump version to v4.0.0-57.10

* fix trufflehog install from failing (#3333) (#3335)

* correct credentials scan failure.

* force dependency versions.

---------

Co-authored-by: Smith <Devin.Smith@quartech.com>

* Fix for takes not showing properly on the update screen | psp-6572 (#3332)

* Fix for takes not showing properly on the update screen

* Removed debugger code

* PSP-6563 : HOTFIX: in trust is missing on H120 (#3325)

Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* PSP-6557 : HOTFIX - Missing values on Template H120 (#3326)

Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* Fixed business function not being displayed (#3330)

* Psp 6566 map search corrections (#3338)

* psp-6566

* correct order of operations for 0 pad.

* psp-6585 updates to allow etl to overwrite null expiry date on yearly financial codes. (#3340)

* bump release hotfix version. (#3341)

Co-authored-by: Smith <Devin.Smith@quartech.com>

* PSP-6527 : FT-REG: Lease & License - When a new license is created, the Created By and Last updated By dates are not displayed and the user is always USER (#3336)

Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>

* CI: Bump version to v4.0.0-57.11

* CI: Bump version to v4.0.3-57.51

* Update snapshots

---------

Co-authored-by: devinleighsmith <41091511+devinleighsmith@users.noreply.github.com>
Co-authored-by: Sue Tairaku <42981334+stairaku@users.noreply.github.com>
Co-authored-by: PryancaJSharma <99448336+PryancaJSharma@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Eduardo <eddherrera@users.noreply.github.com>
Co-authored-by: Eduardo Herrera <Eduardo.Herrera@quartech.com>
Co-authored-by: Aman Monga <53246811+buddy326@users.noreply.github.com>
Co-authored-by: Smith <Devin.Smith@quartech.com>
Co-authored-by: Manuel Rodriguez <marobej@gmail.com>
Co-authored-by: devinleighsmith <devinleighsmith@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tech-debt Removing technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants