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.

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

13550 verify entity type for sp/gp change and conversion filing #507

Conversation

bennypc
Copy link
Contributor

@bennypc bennypc commented Jun 14, 2023

Issue #: /bcgov/entity#13550

Description of changes:
Created a switch statement to check for the entity type within Change and Conversion UI, checks if entity is a benefit company

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the bcrs-entities-create-ui license (Apache 2.0).

Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

Can you post a screenshot of the console when you try to do a change for BEN for example?

@codecov
Copy link

codecov bot commented Jun 15, 2023

Codecov Report

Merging #507 (7f5c501) into main (ae09408) will decrease coverage by 1.20%.
The diff coverage is 82.87%.

@@            Coverage Diff             @@
##             main     #507      +/-   ##
==========================================
- Coverage   86.96%   85.77%   -1.20%     
==========================================
  Files         195      202       +7     
  Lines        3645     3753     +108     
  Branches      444      471      +27     
==========================================
+ Hits         3170     3219      +49     
- Misses        462      519      +57     
- Partials       13       15       +2     
Impacted Files Coverage Δ
src/components/common/Detail.vue 100.00% <ø> (ø)
src/components/common/EffectiveDateTime.vue 100.00% <ø> (ø)
...omponents/common/PeopleAndRoles/PeopleAndRoles.vue 100.00% <ø> (ø)
src/components/common/YourCompany/index.ts 100.00% <ø> (ø)
src/dialogs/FetchErrorDialog.vue 100.00% <ø> (ø)
src/dialogs/FileAndPayInvalidNameRequestDialog.vue 100.00% <ø> (ø)
src/dialogs/PaymentErrorDialog.vue 100.00% <ø> (ø)
src/dialogs/SaveErrorDialog.vue 100.00% <ø> (ø)
...erfaces/state-interfaces/name-request-interface.ts 100.00% <ø> (ø)
src/resources/LimitedRestorationExtension/BC.ts 100.00% <ø> (ø)
... and 89 more

... and 2 files with indirect coverage 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.

I think you should also add checks to the other filings. For example:

  • Special Resolutions are for coops only
  • Alterations are for corps (BC/BEN/CCC/ULC) only
  • Limited Restoration Extensions are for corps only
  • Limited Restoration to Full are for corps only
  • Corp Corrections are for corps only
  • Firm Corrections are for firms (SP/GP) only

There will soon be a Special Resolution Correction (or maybe Coop Correction) for coops only -- check with Jia Xu and Travis Semple in RocketChat to see if they can add the entity type check to their code.

@severinbeauvais
Copy link
Collaborator

Re:
image

Codecov complains when the coverage "goes down". I put that in quotes because the coverage can "go down" even if you make no code changes, so something's not right there. I think we're fine as long as code coverage doesn't drop a lot or go below 80%.

@bennypc
Copy link
Contributor Author

bennypc commented Jun 15, 2023

Can you post a screenshot of the console when you try to do a change for BEN for example?

Screenshot 2023-06-15 at 2 06 39 PM

@severinbeauvais
Copy link
Collaborator

Screenshot 2023-06-15 at 2 06 39 PM

Is there an error dialog as well?

Also you should get one of our UX people (Ethan, Yui or Janis) to approve the error message.

@bennypc
Copy link
Contributor Author

bennypc commented Jun 20, 2023

I think you should also add checks to the other filings. For example:

  • Special Resolutions are for coops only
  • Alterations are for corps (BC/BEN/CCC/ULC) only
  • Limited Restoration Extensions are for corps only
  • Limited Restoration to Full are for corps only
  • Corp Corrections are for corps only
  • Firm Corrections are for firms (SP/GP) only

There will soon be a Special Resolution Correction (or maybe Coop Correction) for coops only -- check with Jia Xu and Travis Semple in RocketChat to see if they can add the entity type check to their code.

It seems like the other filings already have a way to check this (except for Alterations). For example in Limited Restoration Extension theres

get restorationResource (): ResourceIF { switch (true) { case this.isBcCompany: return BcRestorationResource case this.isBenefitCompany: return BenRestorationResource case this.isBcCcc: return CccRestorationResource case this.isBcUlcCompany: return UlcRestorationResource } return null // should never happen }

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.

LGTM, but please wait another review or 2 before merging. Thx.

Copy link
Collaborator

@chenhongjing chenhongjing left a comment

Choose a reason for hiding this comment

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

LGTM 😺

Copy link
Collaborator

@JazzarKarim JazzarKarim left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@severinbeauvais severinbeauvais merged commit 828912e into bcgov:main Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants