-
Notifications
You must be signed in to change notification settings - Fork 52
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
15538 Don't set initial restoration type + misc cleanup #693
Conversation
- misc package updates - refactored restoration filing init (to support null type) - refactored restoration filing resume - deleted unneeded application types
@hfekete I added you as reviewer since this is Filings UI code and you might want to incorporate these changes in the new business dashboard. |
Temporary Url for review: https://business-filings-dev--pr-693-v4u2a128.web.app SB says, try this: https://business-filings-dev--pr-693-v4u2a128.web.app/BC0882895 |
* @returns the filing body | ||
*/ | ||
buildRestorationFiling (restorationType = FilingSubTypes.FULL_RESTORATION): RestorationFilingIF { | ||
buildRestorationFiling (restorationType: FilingSubTypes): RestorationFilingIF { |
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.
Just use what's passed in -- don't fall back to Full Restoration.
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 👍
- updated unit tests
Quality Gate passedIssues Measures |
} else if (restorationType === FilingSubTypes.LIMITED_RESTORATION_TO_FULL) { | ||
url = `${this.getEditUrl}${this.getIdentifier}/limitedRestorationToFull?restoration-id=${id}` | ||
} else { | ||
url = `${this.getCreateUrl}restoration-business-name?id=${this.getIdentifier}` |
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've added this direct route to step 1 of the restoration filing, which saves a default re-route and then another re-route for restorations.
Ditto in TodoList.
And also I updated the unit tests.
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.
@hfekete Here's why a route is specified for Create UI ^^.
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.
@patrickpeinanw FYI, and also see other changes in this PR. Thanks!
/gcbrun |
Temporary Url for review: https://business-filings-dev--pr-693-v4u2a128.web.app |
Issue #: bcgov/entity#15538
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).