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

19191 Updated models in legal-api to use NAICS fields in AlternateName #2396

Merged
merged 4 commits into from
Jan 19, 2024

Conversation

chenhongjing
Copy link
Collaborator

@chenhongjing chenhongjing commented Jan 16, 2024

Issue #: /bcgov/entity#19191

Description of changes:

  • Updated models for legal-api

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: Hongjing Chen <Hongjing.Chen@gov.bc.ca>
Copy link
Collaborator

@thorwolpert thorwolpert left a comment

Choose a reason for hiding this comment

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

This doesn't look right to me.
Afaik a business can have a NAICS and its DBAs can.
Not every business will have a DBA or Assumed name, so that section wouldn't work.
Also, a business can have many DBAs (so a one-or-none would blow up).
We have some with many 10s of DBA names

Signed-off-by: Hongjing Chen <Hongjing.Chen@gov.bc.ca>
@chenhongjing chenhongjing marked this pull request as draft January 17, 2024 20:58
Signed-off-by: Hongjing Chen <Hongjing.Chen@gov.bc.ca>
Signed-off-by: Hongjing Chen <Hongjing.Chen@gov.bc.ca>
@chenhongjing chenhongjing changed the title 19191 Part1: Updated legal-api to use NAICS fields in AlternateName 19191 Updated models in legal-api to use NAICS fields in AlternateName Jan 18, 2024
@chenhongjing chenhongjing marked this pull request as ready for review January 18, 2024 17:25
Copy link

sonarcloud bot commented Jan 18, 2024

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@argush3
Copy link
Collaborator

argush3 commented Jan 18, 2024

@chenhongjing as per our conversation, we can just update the alternate_names model to have the new NAICS fields for this ticket/PR.

I will be creating some tickets to deal with integrating alternate name with rest of the code.

@@ -786,9 +786,22 @@ def find_by_legal_name(cls, legal_name: str = None):
@classmethod
def find_by_identifier(cls, identifier: str = None):
"""Return a Business by the id assigned by the Registrar."""
if not identifier or not cls.validate_identifier(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chenhongjing chenhongjing merged commit 50cf58e into bcgov:feature-legal-name Jan 19, 2024
4 of 5 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.

None yet

4 participants