-
Notifications
You must be signed in to change notification settings - Fork 54
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
25307 Redirect FF and code cleanup #713
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -147,14 +147,9 @@ import { | |
NameRequestInvalidDialog, | ||
NotInGoodStandingDialog | ||
} from '@/components/dialogs' | ||
import { | ||
ConfigJson, | ||
getDashboardBreadcrumb, | ||
getMyBusinessRegistryBreadcrumb, | ||
getRegistryDashboardBreadcrumb, | ||
getStaffDashboardBreadcrumb | ||
} from '@/resources' | ||
import { CommonMixin, DateMixin, DirectorMixin, FilingMixin, NameRequestMixin } from '@/mixins' | ||
import { ConfigJson } from '@/resources' | ||
import { BreadcrumbMixin, CommonMixin, DateMixin, DirectorMixin, FilingMixin, NameRequestMixin } | ||
from '@/mixins' | ||
import { AuthServices, EnumUtilities, LegalServices } from '@/services/' | ||
import { | ||
ApiFilingIF, | ||
|
@@ -189,6 +184,7 @@ import { useBusinessStore, useConfigurationStore, useFilingHistoryListStore, use | |
} | ||
}) | ||
export default class App extends Mixins( | ||
BreadcrumbMixin, | ||
CommonMixin, | ||
DateMixin, | ||
DirectorMixin, | ||
|
@@ -223,22 +219,22 @@ export default class App extends Mixins( | |
CorpTypeCd.ULC_CONTINUE_IN | ||
] | ||
// business store references | ||
@Getter(useBusinessStore) getEntityName!: string | ||
// @Getter(useBusinessStore) getLegalType!: CorpTypeCd | ||
// @Getter(useBusinessStore) getIdentifier!: string | ||
@Getter(useBusinessStore) isEntitySoleProp!: boolean | ||
|
||
// configuration store references | ||
@Getter(useConfigurationStore) getAuthApiUrl!: string | ||
@Getter(useConfigurationStore) getBusinessesUrl!: string | ||
// @Getter(useConfigurationStore) getBusinessDashUrl!: string | ||
// @Getter(useConfigurationStore) getBusinessesUrl!: string | ||
@Getter(useConfigurationStore) getCreateUrl!: string | ||
@Getter(useConfigurationStore) getRegHomeUrl!: string | ||
|
||
// root store references | ||
@Getter(useRootStore) getKeycloakRoles!: Array<string> | ||
@Getter(useRootStore) isBootstrapFiling!: boolean | ||
@Getter(useRootStore) isBootstrapPending!: boolean | ||
@Getter(useRootStore) isBootstrapTodo!: boolean | ||
// @Getter(useRootStore) isNoRedirect!: boolean | ||
@Getter(useRootStore) isRoleStaff!: boolean | ||
@Getter(useRootStore) showFetchingDataSpinner!: boolean | ||
@Getter(useRootStore) showStartingAmalgamationSpinner!: boolean | ||
|
@@ -312,20 +308,20 @@ export default class App extends Mixins( | |
get breadcrumbs (): Array<BreadcrumbIF> { | ||
const breadcrumbs = this.$route?.meta?.breadcrumb | ||
const crumbs: Array<BreadcrumbIF> = [ | ||
getDashboardBreadcrumb(this.getEntityName, this.getBusinessDashUrl, this.getIdentifier), | ||
this.getDashboardBreadcrumb(), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this function is now in a mixin, it has full access to the store and this simplifies things. Ditto below. |
||
...(breadcrumbs || []) | ||
] | ||
|
||
// Set base crumbs based on user role | ||
// Staff don't want the home landing page and they can't access the Manage Business Dashboard | ||
if (this.isRoleStaff) { | ||
// If staff, set StaffDashboard as home crumb | ||
crumbs.unshift(getStaffDashboardBreadcrumb(this.getBusinessesUrl)) | ||
crumbs.unshift(this.getStaffDashboardBreadcrumb()) | ||
} else { | ||
// For non-staff, set Home and Dashboard crumbs | ||
crumbs.unshift( | ||
getRegistryDashboardBreadcrumb(this.getRegHomeUrl), | ||
getMyBusinessRegistryBreadcrumb(this.getBusinessesUrl)) | ||
this.getBcRegistriesDashboardBreadcrumb(), | ||
this.getMyBusinessRegistryBreadcrumb()) | ||
} | ||
return crumbs | ||
} | ||
|
@@ -392,6 +388,7 @@ export default class App extends Mixins( | |
@Action(useRootStore) setFetchingDataSpinner!: (x: boolean) => void | ||
@Action(useRootStore) setKeycloakRoles!: (x: Array<string>) => void | ||
@Action(useRootStore) setNameRequest!: (x: any) => void | ||
@Action(useRootStore) setNoRedirect!: (x: boolean) => void | ||
@Action(useRootStore) setParties!: (x: Array<PartyIF>) => void | ||
@Action(useRootStore) setPendingsList!: (x: Array<any>) => void | ||
@Action(useRootStore) setRecordsAddress!: (x: OfficeAddressIF) => void | ||
|
@@ -450,9 +447,9 @@ export default class App extends Mixins( | |
console.log('Error fetching user info or updating Launch Darkly =', error) | ||
} | ||
|
||
// now that LaunchDarkly has been updated (ie, in case of user targeting), | ||
// check whether to use this Entity Dashboard or the new Business Dashboard | ||
if (GetFeatureFlag('use-business-dashboard') && (this.$route.name === Routes.DASHBOARD)) { | ||
// check whether to redirect to the new Business Dashboard | ||
if (this.$route.query.noRedirect !== undefined) this.setNoRedirect(true) | ||
if (!this.isNoRedirect && (this.$route.name === Routes.DASHBOARD)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This allows devs to use the old Filings UI dashboard. The FF is now obsolete. |
||
const identifier = (this.businessId || this.tempRegNumber) | ||
const dashboardUrl = `${this.getBusinessDashUrl}${identifier}${this.$route.fullPath}` | ||
navigate(dashboardUrl) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,7 +66,7 @@ | |
id="return-to-company-dashboard-button" | ||
color="primary" | ||
class="mt-6" | ||
@click="handleGoToCredentialsDashboard()" | ||
@click="handleGoToCompanyDashboard()" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed as per button text. |
||
> | ||
Return to Company Dashboard | ||
<v-icon>mdi-chevron-right</v-icon> | ||
|
@@ -79,8 +79,8 @@ import { Component, Emit, Vue } from 'vue-property-decorator' | |
|
||
@Component({}) | ||
export default class CredentialSimpleSteps extends Vue { | ||
@Emit('onGoToCredentialsDashboard') | ||
handleGoToCredentialsDashboard (): void { | ||
@Emit('onGoToCompanyDashboard') | ||
handleGoToCompanyDashboard (): void { | ||
return undefined | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { Component, Vue } from 'vue-property-decorator' | ||
import { Getter } from 'pinia-class' | ||
import { BreadcrumbIF } from '@bcrs-shared-components/interfaces' | ||
import { CurrentAccountIF } from '@/interfaces' | ||
import { Routes } from '@/enums' | ||
import { useAuthenticationStore, useBusinessStore, useConfigurationStore, useRootStore } from '@/stores' | ||
|
||
/** | ||
* Mixin that provides some useful Name Request utilities. | ||
*/ | ||
@Component({}) | ||
export default class BreadcrumbMixin extends Vue { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I changed this to a mixin so that it could directly access the store getters. I know it's sometimes possible to access the store externally, ie:
However, I found that I was not seeing the latest store value that I cared about (isNoRedirect) -- it was set to True but I was getting False. So, I'm using a mixing and the Getter mechanism. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PS This mixin is used exclusively by App.vue so using a mixin isn't polluting |
||
@Getter(useAuthenticationStore) getCurrentAccount!: CurrentAccountIF | ||
|
||
@Getter(useBusinessStore) getEntityName!: string | ||
@Getter(useBusinessStore) getIdentifier!: string | ||
|
||
@Getter(useConfigurationStore) getBusinessesUrl!: string | ||
@Getter(useConfigurationStore) getDashboardUrl!: string | ||
@Getter(useConfigurationStore) getRegHomeUrl!: string | ||
|
||
@Getter(useRootStore) isNoRedirect!: boolean | ||
|
||
/** Returns the breadcrumb to the BC Registries Dashboard. */ | ||
getBcRegistriesDashboardBreadcrumb (): BreadcrumbIF { | ||
const accountId = this.getCurrentAccount?.id || 0 | ||
const params = accountId ? `?accountid=${accountId}` : '' | ||
return { | ||
text: 'BC Registries Dashboard', | ||
href: `${this.getRegHomeUrl}dashboard/${params}` | ||
} | ||
} | ||
|
||
/** Returns the breadcrumb to the My Business Registry page. */ | ||
getMyBusinessRegistryBreadcrumb (): BreadcrumbIF { | ||
const accountId = this.getCurrentAccount?.id || 0 | ||
return { | ||
text: 'My Business Registry', | ||
href: `${this.getBusinessesUrl}account/${accountId}/business` | ||
} | ||
} | ||
|
||
/** Returns the breadcrumb to the Staff Dashboard. */ | ||
getStaffDashboardBreadcrumb (): BreadcrumbIF { | ||
return { | ||
text: 'Staff Dashboard', | ||
href: `${this.getBusinessesUrl}staff/dashboard/active` | ||
} | ||
} | ||
|
||
/** Returns the breadcrumb to the Business Dashboard. */ | ||
getDashboardBreadcrumb (): BreadcrumbIF { | ||
if (!this.isNoRedirect) { | ||
// redirect to new Business Dashboard | ||
return { | ||
text: this.getEntityName || 'Unknown Name', | ||
href: `${this.getDashboardUrl}${this.getIdentifier}` | ||
} | ||
} else { | ||
// route to old Entity Dashboard | ||
return { | ||
text: this.getEntityName || 'Unknown Name', | ||
to: { name: Routes.DASHBOARD } | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,9 @@ | ||
import { Component, Vue } from 'vue-property-decorator' | ||
import { isEqual, omit } from 'lodash' | ||
import { Getter } from 'pinia-class' | ||
import { useConfigurationStore } from '@/stores' | ||
import { GetFeatureFlag, navigate } from '@/utils' | ||
import { RawLocation } from 'vue-router' | ||
import { useBusinessStore, useConfigurationStore, useRootStore } from '@/stores' | ||
import { navigate } from '@/utils' | ||
import { Routes } from '@/enums' | ||
|
||
/** | ||
|
@@ -11,6 +12,8 @@ import { Routes } from '@/enums' | |
@Component({}) | ||
export default class CommonMixin extends Vue { | ||
@Getter(useConfigurationStore) getBusinessDashUrl!: string | ||
@Getter(useBusinessStore) getIdentifier!: string | ||
@Getter(useRootStore) isNoRedirect!: boolean | ||
|
||
/** True if Vitest is running the code. */ | ||
get isVitestRunning (): boolean { | ||
|
@@ -76,22 +79,24 @@ export default class CommonMixin extends Vue { | |
|
||
/** | ||
* Navigates to the dashboard page, optionally with a filing ID. | ||
* @param identifier The identifier to include in the dashboard URL | ||
* @param filingId The filing ID to include in the dashboard URL (optional, defaults to null) | ||
* @param identifier the identifier to include in the dashboard URL (optional) | ||
* @param filingId the filing ID to include in the dashboard URL (optional) | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The main change, below, is that I swapped the "then" and "else" conditions to avoid a double negative condition (ie, "if !isNoRedirect"). I also provide a default value for identifier and a proper "no value" default for filing id (NaN). |
||
protected navigateToBusinessDashboard (identifier: string, filingId: number = null): void { | ||
if (GetFeatureFlag('use-business-dashboard')) { | ||
protected navigateToBusinessDashboard (identifier = this.getIdentifier, filingId = NaN): void { | ||
if (!this.isNoRedirect) { | ||
// redirect to the new Business Dashboard | ||
let dashboardUrl = `${this.getBusinessDashUrl}/${identifier}` | ||
if (filingId !== null) { | ||
if (!isNaN(filingId)) { | ||
dashboardUrl += `?filing_id=${filingId.toString()}` | ||
} | ||
navigate(dashboardUrl) | ||
} else { | ||
const route: any = { name: Routes.DASHBOARD } | ||
if (filingId !== null) { | ||
// stay in this UI | ||
const route: RawLocation = { name: Routes.DASHBOARD } | ||
if (!isNaN(filingId)) { | ||
route.query = { filing_id: filingId.toString() } | ||
} | ||
this.$router.push(route).catch(() => {}) // Ignore potential navigation abort errors | ||
this.$router.push(route).catch(() => {}) // ignore potential navigation abort errors | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This getter is used in this module but if I uncomment this then I get a warning that it's already imported in a mixin. Still, it's nice to see, here, where this is imported from.