Skip to content

DBC: Valid party roles restriction in legal-api#4390

Merged
loneil merged 3 commits into
bcgov:mainfrom
loneil:33427DBCLimitRoles
May 15, 2026
Merged

DBC: Valid party roles restriction in legal-api#4390
loneil merged 3 commits into
bcgov:mainfrom
loneil:33427DBCLimitRoles

Conversation

@loneil
Copy link
Copy Markdown
Collaborator

@loneil loneil commented May 13, 2026

Issue #: /bcgov/entity#33427

Description of changes:
Do the same valid party roles code as the python common implementation. (Soon these will be synced up and un-duplicated anyways)

Add some additional code simplification that was done in python/common as well to get ready to un-duplicate.

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>
Copilot AI review requested due to automatic review settings May 13, 2026 21:11
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

This PR aligns legal-api digital credential role handling with the shared python common implementation by restricting DBC party roles to the valid credential roles.

Changes:

  • Adds valid_role_types for proprietor, director, and partner.
  • Filters business and filing party roles against the valid role whitelist.
  • Updates related digital credential tests to use valid roles.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
legal-api/src/legal_api/services/digital_credentials_rules.py Adds valid role filtering for business and filing party roles.
legal-api/tests/unit/services/test_digital_credentials.py Updates credential helper expectations and setup data for valid roles.
legal-api/tests/unit/services/test_digital_credentials_rules.py Updates filing party role tests to use director roles with incorporation applications.
Comments suppressed due to low confidence (1)

legal-api/src/legal_api/services/digital_credentials_rules.py:163

  • The new business-party role whitelist is not directly tested in legal-api. Existing business-role tests only exercise matching valid roles, so an invalid business role like officer, secretary, or incorporator could start being returned without failing coverage.
        return list(
            filter(lambda role: role.role in self.valid_role_types and self.user_matches_party(user, role.party), roles)
        )

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

Comment on lines +151 to +153
return list(
filter(lambda role: role.role in self.valid_role_types and self.user_matches_party(user, role.party), roles)
)
Signed-off-by: Lucas <lucasoneil@gmail.com>
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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

legal-api/src/legal_api/services/digital_credentials_rules.py:71

  • Concatenating business and filing roles without de-duplicating can return duplicate preconditions when the same valid role exists in both sources (for example a director in both the business party roles and incorporation filing). That would surface repeated attestable roles to API consumers instead of a distinct role list.
            preconditions += self.user_business_party_roles(user, business)
            preconditions += self.user_filing_party_roles(user, business)

Comment thread legal-api/src/legal_api/services/digital_credentials_helpers.py
Comment thread legal-api/src/legal_api/services/digital_credentials_rules.py
Signed-off-by: Lucas <lucasoneil@gmail.com>
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
6.1% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

Copy link
Copy Markdown
Collaborator

@kialj876 kialj876 left a comment

Choose a reason for hiding this comment

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

Is there anything that should also be changed in the common dbc code?

@loneil loneil requested a review from meawong May 14, 2026 21:04
@loneil
Copy link
Copy Markdown
Collaborator Author

loneil commented May 14, 2026

Is there anything that should also be changed in the common dbc code?

No this is syncing the legal-api to match the common (so it's easier to remove the legal-api copy)

Copy link
Copy Markdown
Collaborator

@kialj876 kialj876 left a comment

Choose a reason for hiding this comment

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

Nice work!

@loneil loneil merged commit f5137af into bcgov:main May 15, 2026
7 of 8 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.

3 participants