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

24920 - Redirect to new BRD based on FF and Query Param #3192

Merged
merged 5 commits into from
Jan 3, 2025

Conversation

JazzarKarim
Copy link
Collaborator

Issue #:
bcgov/entity#24920

Description of changes:

  • Redirect to the new BRD if the FF enable-business-registry-dashboard and if the Query Param newbrd is true.

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 self-assigned this Jan 2, 2025
@JazzarKarim JazzarKarim marked this pull request as ready for review January 2, 2025 18:26
@JazzarKarim
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-3192-2mpoe0is.web.app

@JazzarKarim
Copy link
Collaborator Author

/gcbrun

@bcregistry-sre
Copy link
Collaborator

Temporary Url for review: https://bcregistry-account-dev--pr-3192-2mpoe0is.web.app

@severinbeauvais
Copy link
Collaborator

/gcbrun

@bcregistry-sre
Copy link
Collaborator

bcregistry-sre commented Jan 3, 2025

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 3, 2025

While you're here, I noticed this console error:

image

I believe the new URL is gold, not silver. Please update the example, and your, env file.

@severinbeauvais
Copy link
Collaborator

severinbeauvais commented Jan 3, 2025

One last thing: when you merge this, please let developers know that from now on, they'll be using the new BRD (and tell them about the query parameter to not redirect, if they need it -- or point them to this PR).

@JazzarKarim
Copy link
Collaborator Author

One last thing: when you merge this, please let developers know that from now on, they'll be using the new BRD (and tell them about the query parameter to not redirect, if they need it -- or point them to this PR).

Deal, will do.

Copy link

sonarqubecloud bot commented Jan 3, 2025

@JazzarKarim JazzarKarim merged commit 85ff453 into bcgov:main Jan 3, 2025
10 of 12 checks passed
@@ -1,6 +1,6 @@
{
"name": "auth-web",
"version": "2.6.128",
"version": "2.7.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

WOOHOO!

@bcgov bcgov deleted a comment from JazzarKarim Jan 6, 2025
@bcgov bcgov deleted a comment from bcregistry-sre Jan 6, 2025
@@ -229,6 +230,13 @@ export function getRoutes (): RouteConfig[] {
{
path: '/account/:orgId',
name: 'account',
beforeEnter: (to, from, next) => {
if (LaunchDarklyService.getFlag(LDFlags.EnableBusinessRegistryDashboard) && !to.query.noRedirect) {
window.location.href = ConfigHelper.getNewBusinessRegistryDashboardUrl()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if this should return a new route (instead of hard-overwriting the URL like this)? That way, it could maybe retain the URL query parameters (eg, account id)...?

Ref: https://router.vuejs.org/guide/advanced/navigation-guards.html#Per-Route-Guard

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's the plan 👍

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