Skip to content

32211: Currency validation#4235

Merged
loneil merged 4 commits intobcgov:mainfrom
loneil:32211currencyValidation
Apr 13, 2026
Merged

32211: Currency validation#4235
loneil merged 4 commits intobcgov:mainfrom
loneil:32211currencyValidation

Conversation

@loneil
Copy link
Copy Markdown
Collaborator

@loneil loneil commented Mar 30, 2026

Issue #: /bcgov/entity#32211

Description of changes:

Adds support for the legacy OTHER currency type on share classes migrated from COLIN and validates that currency values match recognized currency codes.

  • Additional to OTHER type handling, currency validation was previously just done on UI and missing from API (tracked in previous FE/BE validation mismatch analysis and left to this point to handle)

Model

  • Return currencyAdditional in ShareClass.json so UIs can display the OTHER currency text

Validation (validate_share_currency)

-Validate that share class currency is a recognized currency code (matching the standard currency list used by the UI dropdowns), only when hasParValue is true
-Grandfathered OTHER share classes pass through when the incoming share class ID matches an existing OTHER record in the DB — this handles the round-trip scenario where an alteration payload includes unchanged OTHER classes alongside modified non-OTHER classes
-New series under grandfathered OTHER share classes are blocked

Filing coverage

  • Alteration, Correction, Transition: pass business for ID matching (allows existing OTHER pass-through)
  • Incorporation Application, Continuation In, Amalgamation (regular): no business context, all currencies must be valid

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

Signed-off-by: Lucas <lucasoneil@gmail.com>
@loneil loneil force-pushed the 32211currencyValidation branch from 38037ff to 9c64bd5 Compare April 7, 2026 06:19
Signed-off-by: Lucas <lucasoneil@gmail.com>
@loneil loneil force-pushed the 32211currencyValidation branch from d4a5104 to e5055ee Compare April 7, 2026 06:35
@loneil loneil changed the title Currecny validation 32211: Currency validation Apr 7, 2026
loneil added 2 commits April 6, 2026 23:46
Signed-off-by: Lucas <lucasoneil@gmail.com>
Signed-off-by: Lucas <lucasoneil@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented Apr 7, 2026

@loneil loneil marked this pull request as ready for review April 7, 2026 07:48
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds backend enforcement for share class currency validation (ISO 4217) and supports round-tripping legacy OTHER currency share classes migrated from COLIN by exposing currencyAdditional and grandfathering existing OTHER records in certain filing contexts.

Changes:

  • Add currencyAdditional to ShareClass.json for API consumers (UI) to display legacy OTHER currency text.
  • Introduce validate_share_currency and wire it into multiple filing validation flows (IA, Continuation In, Amalgamation Application, Alteration, Correction, Transition).
  • Add unit tests covering ISO validation, OTHER grandfathering rules, and series-blocking under OTHER.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
legal-api/src/legal_api/services/filings/validations/common_validations.py Adds validate_share_currency plus helper functions for ISO validation and legacy OTHER grandfathering.
legal-api/src/legal_api/services/filings/validations/incorporation_application.py Invokes share currency validation for corps during IA validation.
legal-api/src/legal_api/services/filings/validations/continuation_in.py Invokes share currency validation for continuation-in filings.
legal-api/src/legal_api/services/filings/validations/amalgamation_application.py Invokes share currency validation for regular amalgamations.
legal-api/src/legal_api/services/filings/validations/alteration.py Invokes share currency validation in alteration share structure validation flow (with business context).
legal-api/src/legal_api/services/filings/validations/correction.py Invokes share currency validation for corp corrections (with business context).
legal-api/src/legal_api/services/filings/validations/transition.py Invokes share currency validation for transition filings (with business context).
legal-api/src/legal_api/models/share_class.py Extends ShareClass.json to include currencyAdditional.
legal-api/tests/unit/services/filings/validations/test_common_validations.py Adds direct unit tests for validate_share_currency (ISO, OTHER behavior, series rules).
legal-api/tests/unit/services/filings/validations/test_incorporation_application.py Adds a test asserting IA runs share currency validation through the full validate() path.
legal-api/tests/unit/models/test_share_class.py Updates/extends model json tests for currencyAdditional.
legal-api/tests/unit/models/test_share_series.py Updates expected share class json payload in series-related model tests to include currencyAdditional.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@loneil loneil requested a review from ketaki-deodhar April 7, 2026 18:37
Copy link
Copy Markdown
Collaborator

@Rajandeep98 Rajandeep98 left a comment

Choose a reason for hiding this comment

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

lgtm

@loneil loneil merged commit d270508 into bcgov:main Apr 13, 2026
12 checks passed
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