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

22353 correction of C, CBEN, CCC, CUL #2943

Merged
merged 2 commits into from
Aug 26, 2024

Conversation

vysakh-menon-aot
Copy link
Collaborator

Issue #: /bcgov/entity#22353

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 lear license (Apache 2.0).

@vysakh-menon-aot vysakh-menon-aot self-assigned this Aug 26, 2024
@@ -133,7 +133,11 @@ def validate_roles(filing_dict: dict, legal_type: str, filing_type: str = 'incor
Business.LegalTypes.BCOMP.value: 1,
Business.LegalTypes.COMP.value: 1,
Business.LegalTypes.BC_ULC_COMPANY.value: 1,
Business.LegalTypes.BC_CCC.value: 3
Business.LegalTypes.BC_CCC.value: 3,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This func is used in correction

@@ -47,9 +47,7 @@ def process(correction_filing: Filing, filing: Dict, filing_meta: FilingMeta, bu
)

corrected_filing_type = filing['correction']['correctedFilingType']
# added CP, change of directors / change of address for CP is allowed
if business.legal_type in ['SP', 'GP', 'BC', 'BEN', 'CC', 'ULC', 'CP'] and \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

unnecessary if condition, this contains all legal types

Copy link
Collaborator

Choose a reason for hiding this comment

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

What if we add new legal types?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it should be handled in allowable actions

Copy link

sonarcloud bot commented Aug 26, 2024

@vysakh-menon-aot
Copy link
Collaborator Author

vysakh-menon-aot commented Aug 26, 2024

@argush3 I looked into the entity-emailer test failure, It seems to me that this issue is because of the legal name change in business.json() for SP's. Not sure if it's a real issue while emailing or just a unit test issue. If you don't have a ticket for this please create one

@argush3
Copy link
Collaborator

argush3 commented Aug 26, 2024

@argush3 I looked into the entity-emailer test failure, It seems to me that this issue is because of the legal name change in business.json() for SP's. Not sure if it's a real issue while emailing or just a unit test issue. If you don't have a ticket for this please create one

yeah, it's here https://app.zenhub.com/workspaces/entities---david-65af11a89e89f5043c2911f6/issues/gh/bcgov/entity/22367

@@ -422,66 +422,6 @@ async def test_process_filing_completed(app, session, mocker):
assert business.last_ar_date


async def test_correction_filing(app, session, mocker):
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not valid anymore. CP will not go to PENDING_CORRECTION status

Copy link
Collaborator

Choose a reason for hiding this comment

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

That status was for "local corrections", right?

Yes, local corrections are removed from UI code. The only possibility is something like a conversion correction (as above), but even that's not allowed, I think.

@vysakh-menon-aot vysakh-menon-aot merged commit 1c5c20d into bcgov:main Aug 26, 2024
14 of 15 checks passed
@vysakh-menon-aot vysakh-menon-aot deleted the feature/22353 branch August 26, 2024 20:02
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