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

23092 - Cont in Business Only Alter to Cont in Type #590

Merged
merged 6 commits into from
Sep 23, 2024

Conversation

ArwenQin
Copy link
Collaborator

Issue #: /bcgov/entity#23092

Description of changes:

  • A C company can alter to a C Type (Cont In ULC, C, CCC, BEN)

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

@ArwenQin ArwenQin self-assigned this Sep 20, 2024
@ArwenQin
Copy link
Collaborator Author

/gcbrun

@ArwenQin
Copy link
Collaborator Author

Now a Cont in business can alter to other valid Cont in legal types successfully.
Test with:
Cont In ULC to Cont In Benefit or Limited
https://business-edit-dev--pr-590-mkdch8go.web.app/C9900774/alteration?accountid=3040

Cont In Limited to Cont In BEN, ULC, or CCC
https://business-edit-dev--pr-590-mkdch8go.web.app/C9900734/alteration?accountid=3040

@severinbeauvais
Copy link
Collaborator

I cannot test the preview links because Auth API is not returning settings or authorizations.

@ArwenQin
Copy link
Collaborator Author

ArwenQin commented Sep 20, 2024

I cannot test the preview links because Auth API is not returning settings or authorizations.

Oh yes I see... I tried, not working either

@severinbeauvais
Copy link
Collaborator

I cannot test the preview links because Auth API is not returning settings or authorizations.

Oh yes I see... I tried, not working either

I will checkout your code and try it locally. (gh pr checkout 590)

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Sep 20, 2024

OK, I tested locally the first business (C9900774) and the filing JSON looks correct (see below), but I still think there may be an easier solution.

image

Signed-off-by: Qin <Arwen.Qin@gov.bc.ca>
@ArwenQin
Copy link
Collaborator Author

OK, I tested locally the first business (C9900774) and the filing JSON looks correct (see below), but I still think there may be an easier solution.

image

Hi Sev, I just updated the solution. I also corrected the entityTypeOptions property for C, CBEN and CUL

@severinbeauvais
Copy link
Collaborator

Hi Sev, I just updated the solution. I also corrected the entityTypeOptions property for C, CBEN and CUL

OK, this "feels like" the solution I was expecting.

However, I have a done a bit of testing and there are issues (though maybe not caused by this code)...

  1. C9900774 is, at the moment, of type "C". But if try to create a name-change NR for it, business search (from Namerequest UI) says it's type CUL. Please check with Vysakh whether this should have been synced.

  2. Here is the draft alteration filing (where I changed type from C to CBEN). Do you know if filing.alteration.nameRequest.legalType should also be CBEN? (Maybe ask Vysakh again.)

image

Please test thoroughly, including with new NR to change the business name. Thanks.

Signed-off-by: Qin <Arwen.Qin@gov.bc.ca>
@ArwenQin
Copy link
Collaborator Author

/gcbrun

@ArwenQin
Copy link
Collaborator Author

Thanks Sev. I just updated the comments/order.
I will check with Vysakh

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-edit-dev--pr-590-mkdch8go.web.app

@ArwenQin
Copy link
Collaborator Author

@severinbeauvais
I just did thorough tests, the type alter works well. Only the Namerequest type pending Vysakh's confirmation.
By the way, I found another bug in the 3. Court Order and Plan of Arrangement section:
image
If I enter the court order number only, and don't check the POA box, there shows no error; but when we file it, there is a schema error.
If we don't enter the court order number, or enter court order number & check the box - all good.
Just to confirm, what is the requirement? Is court order number only + no check box allowed?
Shall I create a new ticket for this?

@severinbeauvais
Copy link
Collaborator

Just to confirm, what is the requirement? Is court order number only + no check box allowed? Shall I create a new ticket for this?

It should be possible to enter a court number and not check the POA box. This is probably a simple fix you can do in this ticket. According to the court order schema, effectOfOrder is a string but isn't nullable, so you probably need to remove that property if it's empty.

Please test a ChangeOfRegistration ("firm alteration") filing for the same bug, and fix that, too.

If you really want, you can create a new ticket for this. (And then just work on it today.)

Signed-off-by: Qin <Arwen.Qin@gov.bc.ca>
Signed-off-by: Qin <Arwen.Qin@gov.bc.ca>
@@ -331,7 +332,7 @@ export default class FilingTemplateMixin extends DateMixin {
if (this.getHasPlanOfArrangement || this.getFileNumber) {
filing.alteration.courtOrder = {
fileNumber: this.getFileNumber,
effectOfOrder: this.getHasPlanOfArrangement ? EffectOfOrders.PLAN_OF_ARRANGEMENT : null,
effectOfOrder: this.getHasPlanOfArrangement ? EffectOfOrders.PLAN_OF_ARRANGEMENT : undefined,
hasPlanOfArrangement: this.getHasPlanOfArrangement
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

corrected the EffectOfOrders schema for Court Order section

@ArwenQin
Copy link
Collaborator Author

/gcbrun

@bcgov bcgov deleted a comment from bcregistry-sre Sep 23, 2024
@bcgov bcgov deleted a comment from bcregistry-sre Sep 23, 2024
@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://business-edit-dev--pr-590-mkdch8go.web.app

@@ -209,7 +209,7 @@ export default class FilingTemplateMixin extends DateMixin {
if (this.getHasPlanOfArrangement || this.getFileNumber) {
filing.correction.courtOrder = {
fileNumber: this.getFileNumber,
effectOfOrder: this.getHasPlanOfArrangement ? EffectOfOrders.PLAN_OF_ARRANGEMENT : null,
effectOfOrder: this.getHasPlanOfArrangement ? EffectOfOrders.PLAN_OF_ARRANGEMENT : undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please verify by filing a correction with a court order number and with / without a POA.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tested ok with CP1002614

@@ -647,7 +648,7 @@ export default class FilingTemplateMixin extends DateMixin {
if (this.getHasPlanOfArrangement || this.getFileNumber) {
filing.changeOfRegistration.courtOrder = {
fileNumber: this.getFileNumber,
effectOfOrder: this.getHasPlanOfArrangement ? EffectOfOrders.PLAN_OF_ARRANGEMENT : null,
effectOfOrder: this.getHasPlanOfArrangement ? EffectOfOrders.PLAN_OF_ARRANGEMENT : undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please verify by filing a change of registration with a court order number and with / without a POA.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tested ok with FM1085277

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.

The code looks good but please ensure you have tested these scenarios before merging (and also add Test Notes in the ticket to let Riyaz know what you've verified locally):

  • BC to another type
  • BEN to another type
  • CC to another type
  • ULC to another type
  • C to another type
  • CBEN to another type
  • CCC to another type
  • CUL to another type
  • use name change NRs for the above wherever possible (if you find bugs related to this then create another ticket)

Thanks for the good work 👍

@ArwenQin ArwenQin merged commit 47671d9 into bcgov:main Sep 23, 2024
5 checks passed
@severinbeauvais
Copy link
Collaborator

/gcbrun

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.

3 participants