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

25066-Update-allowable-BEN-conversions #790

Conversation

meawong
Copy link
Collaborator

@meawong meawong commented Dec 31, 2024

Issue #: /bcgov/entity#25066

Description of changes:

  • update redirect logic for BENs according to these flows:
    1. Starting point in LEAR => resulting company in Lear
    2. Starting point in COLIN and resulting is not BEN => redirect to COLIN
    3. Starting point in COLIN and resulting is BEN => display the "Contact Us" text

Example 1 - LEAR
image

Example 2 - COLIN -> NOT BEN
image

Example 3 - COLIN -> BEN
image

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

@meawong
Copy link
Collaborator Author

meawong commented Dec 31, 2024

/gcbrun

@meawong meawong self-assigned this Dec 31, 2024
@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Dec 31, 2024

Temporary Url for review: https://namerequest-dev--pr-790-3i735qrh.web.app

LEAR ex: BC0884813
COLIN ex: BC0808036

@meawong
Copy link
Collaborator Author

meawong commented Jan 2, 2025

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Jan 2, 2025

Temporary Url for review: https://namerequest-dev--pr-790-3i735qrh.web.app

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

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

LGTM!

Thanks for the test screenshots. I also tested this and it looks great!

Copy link
Collaborator

@eve-git eve-git left a comment

Choose a reason for hiding this comment

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

We are preparing for next week's release for the NRO decommissioning. You might want to check with Omid to confirm if it's okay to merge the code into the main branch.

@meawong meawong merged commit 984098c into bcgov:main Jan 2, 2025
9 of 10 checks passed
@severinbeauvais
Copy link
Collaborator

We are preparing for next week's release for the NRO decommissioning. You might want to check with Omid to confirm if it's okay to merge the code into the main branch.

This code change should be OK as it is in Namerequest UI and has nothing to do with NRO decommissioning.

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 2, 2025

Please test this functionality while not logged in.

It looks to me like checkBusinessInLear uses fetchBusiness, and that function's header comment says you have to be logged in, ie:

image

It looks like this update doesn't work when not logged in:

image

Could this ticket be related? bcgov/entity#25118

@severinbeauvais
Copy link
Collaborator

I'm confused. In the console it shows this:

image

but in the Network tab it clearly shows the business call succeeded:

image

and

image

meawong added a commit that referenced this pull request Jan 2, 2025
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