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

20996 Fix MBR after legal name easy fix #2777

Merged
merged 6 commits into from
May 2, 2024

Conversation

JazzarKarim
Copy link
Collaborator

@JazzarKarim JazzarKarim commented Apr 29, 2024

Issue #:
bcgov/entity#20996

Description of changes:

  • Fixed the empty strings for firms
  • Grabbing the latest names (alternateNames array) from LEAR

Before:
before firms

After:
after firms

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-auth license (Apache 2.0).

@JazzarKarim JazzarKarim force-pushed the 20996-fix-mbr-after-legal-name branch from 06cd530 to 2e73f4f Compare April 29, 2024 22:49
@JazzarKarim JazzarKarim changed the base branch from main to release/2.5.18 April 29, 2024 22:51
@JazzarKarim JazzarKarim changed the base branch from release/2.5.18 to main April 29, 2024 22:51
export interface AffiliationResponse {
identifier?: string
draftType?: CorpTypes
legalType?: CorpTypes
businessNumber?: string
name?: string
legalName?: string
alternateNames?: AlternateNames[]
alternateNames?: AlternateNameIF[]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Using the latest/updated interface from the shared components repo now.

@@ -104,7 +97,6 @@ export interface NameRequestResponse {
natureOfBusiness?: string
expirationDate?: Date
nrNum?: string
requestActionCd?: string
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 is a duplicate of line 96 so I removed it.

@@ -209,6 +209,21 @@ export const useBusinessStore = defineStore('business', () => {
const entityResponse: AffiliationResponse[] = await OrgService.getAffiliatedEntities(currentOrganization.value.id)
let affiliatedEntities: Business[] = []

// do a lear search on firms to fetch the latest names (alternate names array)
// set that array to firm entries in the entityResponse array
for (const response of entityResponse) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Basically, for firms, we need to do a LEAR search to grab that alternateNames array that we need. It's needed to properly display the latest correct name.

Copy link
Collaborator

@seeker25 seeker25 Apr 30, 2024

Choose a reason for hiding this comment

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

This should be done in the API not on the frontend as a loop?

Or done in one API call not multiple?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, was thinking that as well. I'll be doing that.

Copy link
Collaborator

@seeker25 seeker25 Apr 30, 2024

Choose a reason for hiding this comment

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

Yeah I remember we spent quite a bit of time getting rid of having to call Namex and Legal-api on a per business basis.. it caused alot of performance issues when loading up a dashboard with lets say 100 businesses

Merging this PR would be going in the reverse direction

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds good, on it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed this entire block Travis, should be OK now.

Search call is now grabbing the alternateNames array (I've made changes in LEAR):
search call

MBR looks good:
MBR looks good

@@ -407,7 +422,7 @@ export const useBusinessStore = defineStore('business', () => {
return BusinessService.createBNRequest(request)
}

async function searchNRIndex (identifier: string): number {
async function searchNRIndex (identifier: string): Promise<number> {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The return type of async must be a Promise.

@JazzarKarim
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-2777-b68st5kz.web.app

@JazzarKarim JazzarKarim marked this pull request as ready for review April 29, 2024 23:12
@JazzarKarim JazzarKarim self-assigned this Apr 29, 2024
@@ -1,6 +1,6 @@
{
"name": "auth-web",
"version": "2.5.22",
"version": "2.5.23",
Copy link
Collaborator

Choose a reason for hiding this comment

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

2.6.0 ?

@@ -146,6 +147,6 @@ export interface LearBusiness {
identifier: string
legalName: string
legalType: string
alternateNames: AlternateNames[]
alternateNames: AlternateNameIF[]
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this properties (array) is required here but optional in AffiliationResponse interface?

Copy link
Collaborator

@seeker25 seeker25 left a comment

Choose a reason for hiding this comment

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

I have some concerns in the comments

@JazzarKarim JazzarKarim marked this pull request as draft April 30, 2024 19:10
@JazzarKarim JazzarKarim force-pushed the 20996-fix-mbr-after-legal-name branch from 988a48b to 7b3febd Compare May 2, 2024 17:40
@JazzarKarim JazzarKarim force-pushed the 20996-fix-mbr-after-legal-name branch from 7b3febd to 685234f Compare May 2, 2024 17:43
Copy link

sonarqubecloud bot commented May 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

@JazzarKarim JazzarKarim marked this pull request as ready for review May 2, 2024 17:44
@JazzarKarim
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-2777-b68st5kz.web.app

@seeker25 seeker25 merged commit 91e3190 into bcgov:main May 2, 2024
5 of 6 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.

5 participants