-
Notifications
You must be signed in to change notification settings - Fork 51
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
21977 Refactored Entity Status to support more bootstrap filing states cleanly #682
Conversation
- refactored Entity Status into Bootstrap Filing Status and Bootstrap Filing Type - refactored getters as above - deleted obsolete file entityStatus.ts - deleted obsolete file state.ts - updated unit tests
@@ -240,7 +234,8 @@ export default class App extends Mixins( | |||
// root store references | |||
@Getter(useRootStore) getKeycloakRoles!: Array<string> | |||
@Getter(useRootStore) isBootstrapFiling!: boolean | |||
@Getter(useRootStore) isBootstrapTask!: boolean | |||
@Getter(useRootStore) isBootstrapPending!: boolean | |||
@Getter(useRootStore) isBootstrapTodo!: boolean |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are now 3 getters for the bootstrap filings:
- does it belong in Filing History List?
- does it belong in Pending List?
- does it belong in Todo List?
else if (isIncorporationApplication) entityStatus = EntityStatus.FILED_INCORP_APP | ||
else if (isRegistration) entityStatus = EntityStatus.FILED_REGISTRATION | ||
else throw new Error(`Invalid status ${status} for ${filingName} filing`) | ||
break |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved all of this ^^ into the root store as getters.
} | ||
|
||
// store business info | ||
this.setEntityStatus(entityStatus) | ||
this.setBootstrapFilingStatus(status) | ||
this.setBootstrapFilingType(filingName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we save individual status and type instead of a combined type value (which was getting messy).
src/enums/entityStatus.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the combined status/type enum that I deleted in favour of separate values.
state.bootstrapFilingStatus === FilingStatus.COMPLETED || | ||
state.bootstrapFilingStatus === FilingStatus.PAID | ||
) | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this ^^ is the new logic. It's more verbose but I think it's easier to understand and it's easier to extend if needed.
src/stores/state.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file hasn't been used for a while. It dates back to when we use VueX for the store. Now, with Pinia, the state is an object at the top of the store file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/gcbrun |
Temporary Url for review: https://business-filings-dev--pr-682-gtb4b6a0.web.app SB says, try these:
(*) the view for awaiting review continuation applications is still WIP |
/** Stores bootstrap filing in the Filing History List. */ | ||
/** Stores bootstrap item in the Pending List. */ | ||
storeBootstrapPending (response: any): void { | ||
this.storeBootstrapFiling(response) // *** TODO: implement this |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Temporary code while I continue implementing this feature.
Issue #: bcgov/entity#21977
This is another "small" PR to do some refactoring in preparation for future changes. This change should not affect current functionality.
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the business-filings-ui license (Apache 2.0).