Skip to content
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.

Sign up for GitHub

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

Jump to bottom

24190 Fix BEN to CCC Alteration and Enable Continued-In Alterations #782

Merged
merged 5 commits into from
Nov 14, 2024
Merged

24190 Fix BEN to CCC Alteration and Enable Continued-In Alterations #782

merged 5 commits into from
Nov 14, 2024

Conversation

meawong
Copy link
Collaborator

@meawong meawong commented Nov 8, 2024 •
edited
Loading

Issue #: /bcgov/entity#24190

Description of changes:
Changes as per Severin's WIP PR (#780)

  • app version = 5.5.11
  • imported latest shared enums
  • updated conversion enum keys
  • updated BEN conversion logic (WIP)
  • sorted Conversion Types list data
  • added BEN -> CCC conversion
  • updated online alteration logic

Since added:

  • removed conditional logic for BEN companies (which assumes only one alteration is allowed i.e. BEN -> BC)
  • added continued-in business alterations to Conversion Types list data
  • updated allowable alteration logic
  • updated logic which sets the entity type for conversions

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).

Severin Beauvais and others added 3 commits November 6, 2024 11:53
- app version = 5.5.11
f56dec3 
- imported latest shared enums
- updated conversion enum keys
- updated BEN conversion logic (WIP)
- sorted Conversion Types list data
- added BEN -> CCC conversion
- updated online alteration logic
@meawong
remove conditional logic for BEN alteration
ee66086
@meawong
add conversion types/allow alerations for continued businesses
a607267
@meawong
Copy link
Collaborator Author

meawong commented Nov 8, 2024

/gcbrun

1 similar comment
@meawong
Copy link
Collaborator Author

meawong commented Nov 8, 2024

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Nov 8, 2024 •
edited by meawong
Loading

Temporary Url for review: https://namerequest-dev--pr-782-jvlgs0r3.web.app

For testing:

BEN: BC0883657
C: C9900820
CBEN: C9900826
CUL: C9900824

@meawong meawong self-assigned this Nov 8, 2024
@meawong meawong marked this pull request as ready for review November 8, 2024 21:41
@severinbeauvais severinbeauvais mentioned this pull request Nov 8, 2024
Closed
@severinbeauvais
Copy link
Collaborator

For testing:

BEN: BC0883657
C: C9900820
CBEN: C9900826
CUL: C9900824

Awesome, thanks! I tried all of these :)

You were not able to submit any of these NRs due the Namex not being ready yet, right?

meawong
meawong commented Nov 12, 2024
View reviewed changes
app/src/mixins/search-mixin.ts
const entityType = ConversionTypes.find(conv =>
conv.value === nrRequestType && conv.origin_entity_type_cd === this.origin_entity_type_cd
)?.entity_type_cd || null

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated this logic to also check for the originEntityType since more than one ConversionType now maps back to the same NrRequestTypeCode (with the added entries for Continued-In business).

@meawong
Copy link
Collaborator Author

meawong commented Nov 12, 2024

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://namerequest-dev--pr-782-jvlgs0r3.web.app

@meawong
Copy link
Collaborator Author

meawong commented Nov 12, 2024

For testing:
BEN: BC0883657
C: C9900820
CBEN: C9900826
CUL: C9900824

Awesome, thanks! I tried all of these :)

You were not able to submit any of these NRs due the Namex not being ready yet, right?

correct, you won't be able to submit the NR
image

severinbeauvais
severinbeauvais approved these changes Nov 12, 2024
View reviewed changes
Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the extra fix. Looks good to me.

@eve-git , could you please review this PR and/or ping other devs who can? Thanks!

eve-git
eve-git approved these changes Nov 14, 2024
View reviewed changes
Copy link
Collaborator

@eve-git eve-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@meawong meawong merged commit 4612e40 into bcgov:main Nov 14, 2024
9 of 10 checks passed
@meawong meawong deleted the 24190 branch November 14, 2024 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eve-git eve-git eve-git approved these changes

@severinbeauvais severinbeauvais severinbeauvais approved these changes

@shaangill025 shaangill025 Awaiting requested review from shaangill025 shaangill025 is a code owner

@stevenc987 stevenc987 Awaiting requested review from stevenc987 stevenc987 is a code owner

@mengdong19 mengdong19 Awaiting requested review from mengdong19 mengdong19 is a code owner

@ozamani9gh ozamani9gh Awaiting requested review from ozamani9gh ozamani9gh is a code owner

@rarmitag rarmitag Awaiting requested review from rarmitag rarmitag is a code owner

@flutistar flutistar Awaiting requested review from flutistar flutistar is a code owner

@EPortman EPortman Awaiting requested review from EPortman EPortman is a code owner

Assignees

@severinbeauvais severinbeauvais

@meawong meawong

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@meawong @bcregistry-sre @severinbeauvais @eve-git