-
Notifications
You must be signed in to change notification settings - Fork 44
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
20133 Convert entity types in numbered Continuation In drafts #764
Conversation
- added missing entity type (CS) to enum - added comments to mapping table - adding missing entity type to common mixin methods - updated createNumberedBusiness to convert "entity type" to "continuation in entity type"
/gcbrun |
specifier: 3.0.8 | ||
version: 3.0.8 | ||
specifier: 3.0.12 | ||
version: 3.0.12 |
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'm not sure why the old lock file specified v3.0.8, because the previous package file specified v3.0.12. Anyways, looks correct now. 🤷♂️
@@ -11,6 +11,7 @@ export enum EntityTypes { | |||
CCC = CorpTypeCd.CCC_CONTINUE_IN, | |||
CP = CorpTypeCd.COOP, | |||
CR = CorpTypeCd.CORPORATION, | |||
CS = CorpTypeCd.CONT_IN_SOCIETY, |
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 added this for completeness, since this code is not used as there's a feature flag that redirects users to Societies Online.
Temporary Url for review: https://namerequest-dev--pr-764-hllvd3sa.web.app |
case EntityTypes.CBEN: return 'Benefit Company (Continuation In)' | ||
case EntityTypes.CCC: return 'BC Community Contribution Company (Continuation In)' | ||
case EntityTypes.CS: return 'BC Social Enterprise (Continuation In)' | ||
case EntityTypes.CUL: return 'BC Unlimited Liability Company (Continuation In)' |
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.
case CorpTypeCd.BC_ULC_COMPANY: legalType = CorpTypeCd.ULC_CONTINUE_IN; break | ||
case CorpTypeCd.SOCIETY: legalType = CorpTypeCd.CONT_IN_SOCIETY; 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.
So here's the gist of it. This is right where we create the draft filing in Legal API.
As far as Namerequest UI is concerned, up to this point, the entity type / corp type was CR/BC/CC/UL/SO.
Maybe you're wondering how this works for NRs? It's similar -- Namerequest UI uses CR/BC/CC/UL/SO, but then Namex API returns the new NR with a converted entityType
(C/CBEN/CCC/CUL/CS).
@@ -531,7 +531,7 @@ export const getEntityTypesBC = (state: StateIF): EntityI[] => { | |||
return EntityTypesBcData | |||
} catch (err) { | |||
console.error('entityTypesBC() =', err) // eslint-disable-line no-console | |||
return EntityTypesBcData | |||
return [] |
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 carried over this change from my previous (closed) PR.
The old code masked any errors by returning what looks like legitimate data. I think it's better to return "nothing" as that will stand out during testing and we can fix any problems.
} else if ([EntityTypes.FR, EntityTypes.GP, EntityTypes.UL].includes(x.value)) { | ||
x.shortlist = null | ||
x.rank = null | ||
} |
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.
Yes, I left the else-if block in here, even though I'm pretty sure it does nothing. This is the "don't make waves" pattern. If you've worked in Namerequest UI then you'll understand 😁
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.
Looks good Sev!
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
Issue #: bcgov/entity#20133
(I abandoned my previous PR because that solution was fairly messy and still not working correctly 🙄)
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 namerequest license (Apache 2.0).