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-8771 & psp-8683 | Lease expiry using renewal information #4189

Merged
merged 4 commits into from
Jul 13, 2024

Conversation

FuriousLlama
Copy link
Collaborator

Updated lease expiry search to use renewal information. Updated expiry definition for leases

@FuriousLlama FuriousLlama added the enhancement New feature or request label Jul 12, 2024
@FuriousLlama FuriousLlama self-assigned this Jul 12, 2024
Copy link
Contributor

✅ No secrets were detected in the code.

Copy link
Contributor

✅ No secrets were detected in the code.

{
dest.LFileNo = src.lease.LFileNo;
dest.MotiRegion = src.lease.RegionCodeNavigation?.RegionName;
dest.StartDate = src.lease.OrigStartDate.FilterSqlMinDate().ToNullableDateOnly();
dest.EndDate = src.lease.OrigExpiryDate?.FilterSqlMinDate().ToNullableDateOnly();
dest.EndDate = src.expiryDate.ToNullableDateOnly();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to filter the sql min date here? previously this would cause exceptions converting from an sql min date to a date time.

@@ -36,7 +36,7 @@ private static void MapLease((Entity.PimsLeasePeriod period, Entity.PimsLease le
dest.InspectionNotes = src.lease.InspectionNotes;
dest.InspectionDate = src.lease.InspectionDate?.FilterSqlMinDate();
dest.LeaseNotes = src.lease.LeaseNotes;
dest.IsExpired = (src.lease.GetExpiryDate() < DateTime.Now).BoolToYesNo();
dest.IsExpired = (src.expiryDate < DateTime.Now).BoolToYesNo();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to include renewals in this calculation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not 100%. I dont think this field is actually used in the frontend. But I will clean it up as tech debt

const excercisedRenewalDates = renewals.filter(r => r.isExercised).flatMap(fr => fr.expiryDt);

let calculatedExpiry: string | null = lease?.expiryDate ?? '';
for (let i = 0; i < excercisedRenewalDates.length; i++) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

could also do with a sort? this seems fine though.

@devinleighsmith
Copy link
Collaborator

@FuriousLlama here is the only area I'm seeing a mismatch so far:
image
(the latest renewal is July 31st on this lease, shows up correctly on list view and export but not here).

Copy link
Collaborator

@devinleighsmith devinleighsmith left a comment

Choose a reason for hiding this comment

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

approved, only found one instance in application where no expiry not working as intended.

Copy link
Contributor

✅ No secrets were detected in the code.

@FuriousLlama FuriousLlama merged commit 8c468a8 into bcgov:dev Jul 13, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants