17042 Fixed draft restoration bug + fixed validation flags error #563
Conversation
- added check before restoring approval type from state filing - misc cleanup/comments - fixed some Action declarations - fixed some filing data typing - set state filing before parsing draft restoration (which depends on it)
- fixed scrollToTop typing - added validation flags comments - fixed incorrect validation flag order in state model - fixed mockImplementation variable issue
|
/gcbrun |
|
Temporary Url for review: https://business-edit-dev--pr-563-ucr8v2i7.web.app |
| @@ -220,7 +220,7 @@ export default class ViewWrapper extends Mixins(CommonMixin, FilingTemplateMixin | |||
| switch (action) { | |||
| case FeeSummaryActions.BACK: | |||
| this.setSummaryMode(false) | |||
| await this.scrollToTop(document.getElementById('app')) | |||
| this.scrollToTop(document.getElementById('app')) | |||
There was a problem hiding this comment.
scrollToTop() is no longer async. There are a few changes like this in here.
|
|
||
| // Evaluate valid flags. Scroll to invalid components or continue to review. | ||
| if (await this.validateAndScroll(this.getFlagsCompanyInfo, ComponentsCompanyInfo)) { | ||
| if (this.validateAndScroll(this.getFlagsCompanyInfo, ComponentsCompanyInfo)) { |
There was a problem hiding this comment.
validateAndScroll() is no longer async.
| @@ -967,7 +967,7 @@ export default class OfficeAddresses extends Mixins(CommonMixin) { | |||
| /** | |||
| * When Done is clicked, stores updated addresses. | |||
| */ | |||
| protected async acceptChanges (): Promise<void> { | |||
There was a problem hiding this comment.
I removed "protected" because VS Code was complaining that this method wasn't visible in the template (even though the code worked).
| // don't call window.scrollTo during Vitest tests because jsdom doesn't implement it | ||
| if (!this.isVitestRunning) await element.scrollIntoView({ behavior: 'smooth' }) | ||
| if (!this.isVitestRunning) element.scrollIntoView({ behavior: 'smooth' }) |
There was a problem hiding this comment.
Element.scrollIntoView() is not async.
Thus, scrollToTop() doesn't need to be async, nor its callers, etc.
| @@ -57,6 +56,7 @@ export const stateModel: StateModelIF = { | |||
| isValidSpecialResolution: true, | |||
| isValidSpecialResolutionSignature: true, | |||
| isValidApprovalType: true, | |||
| isValidRelationship: true, | |||
There was a problem hiding this comment.
I found this bug while testing my other changes: console errors due to an element id not found (null de-reference).
The order here is now correct. I also updated some comments above.
This is also how I found out that Element.scrollIntoView() is not async and so I cleaned that up, too.
| this.setRestorationApprovalType(this.getStateFilingRestoration?.approvalType) | ||
| } else if (this.getStateFilingRestoration?.approvalType) { | ||
| // get approval type from state filing | ||
| this.setRestorationApprovalType(this.getStateFilingRestoration.approvalType) |
There was a problem hiding this comment.
This is a safety check only (because the previous code could have stored "undefined" as the restoration approval type, which could cause reactivity issues).
This is not the fix for the bug in the subject ticket/PR.
| @@ -1247,7 +1247,7 @@ export const useStore = defineStore('store', { | |||
| * Only applicable to limited restoration extension filing. | |||
| */ | |||
| getRestorationExpiryDate (): string { | |||
| return this.stateModel.restoration?.expiry | |||
| return this.getRestoration.expiry | |||
There was a problem hiding this comment.
Just using an existing getter here to simplify the code.
| @@ -285,7 +292,7 @@ export default class LimitedRestorationExtension extends Mixins( | |||
| this.setResource(this.restorationResource) | |||
|
|
|||
| // initialize Fee Summary data | |||
| this.setFilingData([this.restorationResource.filingData]) | |||
| this.setFilingData([this.restorationResource.filingData as unknown as FilingDataIF]) | |||
| @@ -176,7 +177,7 @@ export default class LimitedRestorationExtension extends Mixins( | |||
| @Action(useStore) setFilingId!: (x: number) => void | |||
| @Action(useStore) setHaveUnsavedChanges!: (x: boolean) => void | |||
| @Action(useStore) setResource!: (x: ResourceIF) => void | |||
| @Action(useStore) setStateFilingRestoration!: (x: Promise<any>) => void | |||
| @Action(useStore) setStateFilingRestoration!: () => Promise<void> | |||
There was a problem hiding this comment.
Type fixes.
| @@ -46,7 +46,7 @@ | |||
| :isCourtOrderOnly="isCourtOrderOnly" | |||
| :isCourtOrderRadio="showCourtOrderRadio" | |||
| :invalidSection="!getApprovalTypeValid" | |||
| @courtNumberChange="setRestorationCourtOrder({ 'fileNumber': $event })" | |||
| @courtNumberChange="setRestorationCourtOrder({ fileNumber: $event })" | |||
There was a problem hiding this comment.
Not a bug. Just cleanup.
| // set the previously filed limited restoration in the store | ||
| // (will throw on error) | ||
| await this.setStateFilingRestoration() | ||
|
|
||
| // parse draft restoration filing into store | ||
| this.parseRestorationFiling(restorationFiling) | ||
|
|
There was a problem hiding this comment.
THIS is the bug fix -- we need to set the previous limited restoration in the store (line 303) before we parse the draft filing into the store (since the parse code needs the state filing).
Issue #: bcgov/entity#17042
Description of changes:
- app version = 4.7.17
- added check before restoring approval type from state filing
- misc cleanup/comments
- fixed some Action declarations
- fixed some filing data typing
- set state filing before parsing draft restoration (which depends on it)
Collateral fixes:
- fixed scrollToTop async (and callers)
- fixed scrollToTop typing
- added validation flags comments
- fixed incorrect validation flag order in state model
- fixed mockImplementation variable issue
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).