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

Effective Date and Time for Unit Notes #1415

Merged
merged 7 commits into from
Jul 12, 2023

Conversation

dimak1
Copy link
Collaborator

@dimak1 dimak1 commented Jul 7, 2023

Issue #:

Description of changes:

  • Add Effective Date component
  • Set effective and expiry dates
  • Add unit tests

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the PPR license (Apache 2.0).

@dimak1
Copy link
Collaborator Author

dimak1 commented Jul 7, 2023

/gcbrun

@pwei1018
Copy link
Collaborator

pwei1018 commented Jul 7, 2023

Temporary Url for review: https://bcregistry-assets-dev--pr-1415-gura6rd7.web.app

Comment on lines 11 to 25
/**
* Creates and mounts a component, so that it can be tested.
*
* @returns a Wrapper<Any> object with the given parameters.
*/
function createComponent (propsData: any): Wrapper<any> {
const localVue = createLocalVue()
localVue.use(Vuetify)

return mount((EffectiveDateTime as any), {
localVue,
propsData,
vuetify
})
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe there is a common function from the test utils folder we can use.
Trying to get away from having this function in every test file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated.

Comment on lines +160 to +161
const expiryDateTime = new Date(new Date(val).getTime() + 90 * 24 * 60 * 60 * 1000).toISOString()
setMhrUnitNote({ key: 'expiryDateTime', value: expiryDateTime })
Copy link
Collaborator

Choose a reason for hiding this comment

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

The UI creates/sets the expiry date? I assumed it just passed the effectiveDateTime to the api 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

CAUC can submit an expiry date, CAUE must submit an expiry date.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there implications if the user modifies there local system time here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@cameron-eyds I thought so too, but then I looked at API spec again, and there was the expiry date that we need to submit, so I added it here. There may be implications if local time on machine changes.
Maybe we can pass a flag (and not the date/time string) that the effective date is immediate? Then based on that server generates effective and exp date? For past date, we can just pass a date/string in past. The only problem I see is that the user will not see the (server time) summary note about the Notice at the bottom of the component.

@@ -0,0 +1,236 @@
<template>
<div id="contact-info-container">
Copy link
Collaborator

Choose a reason for hiding this comment

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

copy pasta carry over i think

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

Copy link
Collaborator

@doug-lovett doug-lovett left a comment

Choose a reason for hiding this comment

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

LGTM

expect(EffectiveDateTimeComponent.find(getTestId('time-picker-fields')).exists()).toBeTruthy()
expect(EffectiveDateTimeComponent.find(getTestId('footnote-label')).exists()).toBeFalsy()
})
})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to add another test or two, to prove the time selected and/or format?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added more tests.

Copy link
Collaborator

@cameron-eyds cameron-eyds left a comment

Choose a reason for hiding this comment

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

Looks good, just a few comments/questions

Copy link
Contributor

@orelbn orelbn left a comment

Choose a reason for hiding this comment

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

Amazing job looks awesome!

@dimak1 dimak1 force-pushed the feat/unit-note-effective-date branch from 012da73 to e1a5465 Compare July 8, 2023 00:26
@codecov
Copy link

codecov bot commented Jul 8, 2023

Codecov Report

Merging #1415 (ab6ff87) into main (9255e77) will increase coverage by 4.89%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1415      +/-   ##
==========================================
+ Coverage   72.35%   77.24%   +4.89%     
==========================================
  Files         307      332      +25     
  Lines       12767     6123    -6644     
  Branches     2630      963    -1667     
==========================================
- Hits         9237     4730    -4507     
+ Misses       3518     1361    -2157     
- Partials       12       32      +20     
Flag Coverage Δ
pprui 77.24% <ø> (+4.89%) ⬆️

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

Impacted Files Coverage Δ
ppr-ui/src/App.vue 100.00% <ø> (+56.78%) ⬆️
ppr-ui/src/components/collateral/Collateral.vue 100.00% <ø> (+10.97%) ⬆️
...nents/collateral/generalCollateral/GenColAmend.vue 100.00% <ø> (+18.00%) ⬆️
...onents/collateral/generalCollateral/GenColEdit.vue 100.00% <ø> (+15.15%) ⬆️
...nts/collateral/generalCollateral/GenColSummary.vue 100.00% <ø> (+8.00%) ⬆️
...collateral/generalCollateral/GeneralCollateral.vue 100.00% <ø> (+6.66%) ⬆️
...ts/collateral/vehicleCollateral/EditCollateral.vue 100.00% <ø> (+22.58%) ⬆️
...collateral/vehicleCollateral/VehicleCollateral.vue 100.00% <ø> (+19.41%) ⬆️
...cleCollateral/factories/useCollateralValidation.ts 71.42% <ø> (ø)
...llateral/vehicleCollateral/factories/useVehicle.ts 70.14% <ø> (-0.91%) ⬇️
... and 141 more

... and 102 files with indirect coverage changes

@dimak1
Copy link
Collaborator Author

dimak1 commented Jul 8, 2023

/gcbrun

@pwei1018
Copy link
Collaborator

pwei1018 commented Jul 8, 2023

Temporary Url for review: https://bcregistry-assets-dev--pr-1415-gura6rd7.web.app

@dimak1
Copy link
Collaborator Author

dimak1 commented Jul 9, 2023

Pending UXA feedback. Will merge after.

@dimak1 dimak1 force-pushed the feat/unit-note-effective-date branch from e1a5465 to 8a6ed20 Compare July 11, 2023 23:57
@dimak1
Copy link
Collaborator Author

dimak1 commented Jul 11, 2023

/gcbrun

@pwei1018
Copy link
Collaborator

Temporary Url for review: https://bcregistry-assets-dev--pr-1415-gura6rd7.web.app

@dimak1
Copy link
Collaborator Author

dimak1 commented Jul 12, 2023

/gcbrun

@pwei1018
Copy link
Collaborator

Temporary Url for review: https://bcregistry-assets-dev--pr-1415-gura6rd7.web.app

@dimak1
Copy link
Collaborator Author

dimak1 commented Jul 12, 2023

Pending another round of UXA.

@dimak1 dimak1 merged commit a6788e4 into bcgov:main Jul 12, 2023
7 checks passed
@dimak1 dimak1 deleted the feat/unit-note-effective-date branch November 20, 2023 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants