Incorporate now for named companies after NR is approved #627
Conversation
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-627-58y7qy7d.web.app |
| * The alternate codes for entity types. | ||
| * Alternate codes are used in Entities UIs. | ||
| */ | ||
| entityTypeAlternateCode (entityType: EntityType): CorpTypeCd { |
There was a problem hiding this comment.
Moving this function to CommonMixin. It makes sense since it'll come in handy for even further work later on.
|
|
||
| <!-- incorporate button --> | ||
| <div class="mt-5 text-center" v-if="showIncorporateButton"> | ||
| <v-btn id="INCORPORATE-btn" @click="handleButtonClick(NrAction.INCORPORATE)"> |
There was a problem hiding this comment.
This button is now in the NRApprovedGrayBoy component above. I'm following the UXPin. It has similar behavior as the Register Your Business button
| this.isSupportedEntity(this.nr) && | ||
| this.nr.request_action_cd && | ||
| this.nr.request_action_cd === NrRequestActionCodes.NEW_BUSINESS && | ||
| NrState.APPROVED === this.nr.state |
There was a problem hiding this comment.
We'll be showing the button if the current entity type is supported (from FF), new business, and if the NR has been approved. The isSupportedEntity function is in the CommonMixin for usage as we like. It'll take the place of isBenefitCompany.
There was a problem hiding this comment.
I think you can remove line 416 since line 417 will cover it.
Where did you copy the "if approved" logic from? It may be complicated that this (ie, conditional approval). Not certain but asking if you've looked into it.
There was a problem hiding this comment.
The logic here is almost similar to the showRegisterButton on line 422 with the difference being instead of isFirm, it's isSupportedEntity.
I do agree with you on the first point. I was just using the similar condition to the existing one (showRegisterButton).
There was a problem hiding this comment.
Please call out for some extra testing on acceptable NR states to incorporate from (and/or confirm what's done in Auth Web for IA NRs).
Otherwise, my concerns are satisfied here.
There was a problem hiding this comment.
I don't think so. I took a look and this should be OK.
Keep in mind that the component takes a prop that disables the button when the NR was approved but still in processing (5 minutes or so).
src/mixins/nr-affiliation-mixin.ts
Outdated
| const name = this.isBenefitCompany(nr) ? 'incorporationApplication' : 'registration' | ||
| const legalType = this.isBenefitCompany(nr) ? 'BEN' : nr.legalType | ||
| const name = this.isSupportedEntity(nr) ? 'incorporationApplication' : 'registration' | ||
| const legalType = this.isSupportedEntity(nr) ? this.entityTypeAlternateCode(nr.entity_type_cd) : nr.legalType |
There was a problem hiding this comment.
This'll behave the same as before but now instead of a hard-coded 'BEN', we're checking for FF values.
There was a problem hiding this comment.
Since the "if" is evaluated in both statements, would it look cleaner to combine these into a regular if-then-else? Not sure, let me know.
There was a problem hiding this comment.
Your new code looks cleaner, for sure 👍
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-627-58y7qy7d.web.app |
JazzarKarim
requested review from
severinbeauvais,
bennypc,
chenhongjing and
leodube-aot
|
/gcbrun |
|
Temporary Url for review: https://namerequest-dev--pr-627-58y7qy7d.web.app |
|
@severinbeauvais Sev, can you approve please? I need an approval from someone with write access to this repo to merge 😁 |
* Package update * Implemented incorporate now for named companies after NR submission * Update in response to Sev's PR comments
* Package update * Implemented incorporate now for named companies after NR submission * Update in response to Sev's PR comments
* Package update * Implemented incorporate now for named companies after NR submission * Update in response to Sev's PR comments
* Package update * Implemented incorporate now for named companies after NR submission * Update in response to Sev's PR comments
Issue #: /bcgov/entity#16942
Description of changes:
I tested all the possible scenarios (BC, BEN, CC, ULC). I tried implementing the button as clean as possible with minimal code additions by building upon previous work.
UXPin: https://preview.uxpin.com/a86dccdd20ac62828f965c5eea10f81b828a9853#/pages/163136223/simulate/sitemap
Example for CC:
After NR is Approved:
Redirect:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the namerequest license (Apache 2.0).