fix: Resolve accountId from the route, initialize route-sync before the app is loaded (#10245)

On production on multiple instances it may happen that the UI is
rendered in correctly, with a lot of options in the sidebar not
available. On further investigation I found out that the feature flag
checks were disabling multiple of those, and also we could see many
correlated errors that pointed towards missing information.

So, there were two problems here

1. The `vuex-router-sync` was not very reliable in some cases
2. In `App.vue` the watch on `currentAccountId` didn't always trigger.

## Fix Tested on Staging

Basically tried to reload the page ~50 times with cache enabled,
disabled, throttling, navigating different pages.


https://www.loom.com/share/1bb27294aa364ac4acfb647780d6385a?sid=87e31330-8cb7-4ded-8616-5e95e2ae3516

<details><summary>

#### What I thought was the fix

</summary>
<p>

### My chain of actions

Replacing vuex-router-sync at first worked fine, but then I saw it was
still failing in some cases, I assumed (I was half-correct tho) that the
rendering of the `App.vue` and syncing of the route to the store was not
happening in a synchronous pattern. So I decided, let's not rely on the
store when the route is directly available in the App context.

Following this, I refactored `useAccount` composable to use `useRoute`
directly, instead of the store, and then replaced the getter inside
`App.vue`. What this did was surface the issue but more consistently 🤯

I saw the watcher, added some console logs, and turns out it was not
getting triggered in all those cases. So I added an `immediate` to it.
And viola, it works!

At the moment, this is deployed to staging and seems to be working
correctly. But we still need to verify it for sure, since how this issue
was surfaced is still a mystery. All we know is that it shows up when
the widget is also loaded alongside the app (if it loads before or after
the app, it works fine)

### What about the route in the store?

Well I have used the `route` usage there with fallback to the store
state. Since Vuex exists in the app context, the route should always be
available to it. But after today I have lost all trust in JavaScript and
will worship rails until end of my life, so I added that in a
`try-catch` block, logged the error to Sentry

</p>
</details> 

## Here's the real fix

If you read the explanation I wrote earlier, I thought I fixed the
issue, but then the chat list navigation completely broke. So I removed
the custom route sync implementation and added the original package
back. Turns out the vuex-router-sync earlier was placed after the app
was initalized, however for it to work, the vue app context is not
required. And it's best to run it before the app is even bootstrapped,
so I added it back and placed it correctly.

So the following changes fixes this problem

1. Hoisting the `sync` function call to before we call `createApp` this
ensures that the stores and route hooks are in place before even the app
is created
2. Ensuring the `initializeAccount` is run immediately when watching
`currentAccountId`
4. Source `currentAccountId` for critical top of the tree components
directly from the route instead of the store
This commit is contained in:
Shivam Mishra
2024-10-08 21:55:51 +05:30
committed by GitHub
parent a8c12ffb25
commit 3a78192e74
6 changed files with 32 additions and 21 deletions

View File

@@ -13,6 +13,7 @@ import { useStore } from 'dashboard/composables/store';
import WootSnackbarBox from './components/SnackbarContainer.vue';
import { setColorTheme } from './helper/themeHelper';
import { isOnOnboardingView } from 'v3/helpers/RouteHelper';
import { useAccount } from 'dashboard/composables/useAccount';
import {
registerSubscription,
verifyServiceWorkerExistence,
@@ -35,8 +36,9 @@ export default {
setup() {
const router = useRouter();
const store = useStore();
const { accountId } = useAccount();
return { router, store };
return { router, store, currentAccountId: accountId };
},
data() {
return {
@@ -52,7 +54,6 @@ export default {
currentUser: 'getCurrentUser',
authUIFlags: 'getAuthUIFlags',
accountUIFlags: 'accounts/getUIFlags',
currentAccountId: 'getCurrentAccountId',
}),
hasAccounts() {
const { accounts = [] } = this.currentUser || {};
@@ -69,10 +70,13 @@ export default {
this.showAddAccountModal = true;
}
},
currentAccountId() {
if (this.currentAccountId) {
this.initializeAccount();
}
currentAccountId: {
immediate: true,
handler() {
if (this.currentAccountId) {
this.initializeAccount();
}
},
},
},
mounted() {

View File

@@ -2,6 +2,7 @@
import { mapGetters } from 'vuex';
import { getSidebarItems } from './config/default-sidebar';
import { useKeyboardEvents } from 'dashboard/composables/useKeyboardEvents';
import { useAccount } from 'dashboard/composables/useAccount';
import { useRoute, useRouter } from 'vue-router';
import PrimarySidebar from './sidebarComponents/Primary.vue';
@@ -33,6 +34,7 @@ export default {
setup(props, { emit }) {
const route = useRoute();
const router = useRouter();
const { accountId } = useAccount();
const toggleKeyShortcutModal = () => {
emit('openKeyShortcutModal');
@@ -72,6 +74,7 @@ export default {
return {
toggleKeyShortcutModal,
accountId,
};
},
data() {
@@ -82,7 +85,6 @@ export default {
computed: {
...mapGetters({
accountId: 'getCurrentAccountId',
currentUser: 'getCurrentUser',
globalConfig: 'globalConfig/get',
inboxes: 'inboxes/getInboxes',

View File

@@ -1,14 +1,15 @@
import { ref } from 'vue';
import { describe, it, expect, vi } from 'vitest';
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { useAccount } from '../useAccount';
import { useStoreGetters } from 'dashboard/composables/store';
import { useRoute } from 'vue-router';
vi.mock('dashboard/composables/store');
vi.mock('vue-router');
describe('useAccount', () => {
beforeEach(() => {
useStoreGetters.mockReturnValue({
getCurrentAccountId: ref(123),
useRoute.mockReturnValue({
params: {
accountId: 123,
},
});
});

View File

@@ -1,18 +1,20 @@
import { computed } from 'vue';
import { useStoreGetters } from 'dashboard/composables/store';
import { useRoute } from 'vue-router';
/**
* Composable for account-related operations.
* @returns {Object} An object containing account-related properties and methods.
*/
export function useAccount() {
const getters = useStoreGetters();
/**
* Computed property for the current account ID.
* @type {import('vue').ComputedRef<number>}
*/
const accountId = computed(() => getters.getCurrentAccountId.value);
const route = useRoute();
const accountId = computed(() => {
return Number(route.params.accountId);
});
/**
* Generates an account-scoped URL.

View File

@@ -5,6 +5,7 @@ import { mapGetters } from 'vuex';
import { useAlert } from 'dashboard/composables';
import { useUISettings } from 'dashboard/composables/useUISettings';
import { useConfig } from 'dashboard/composables/useConfig';
import { useAccount } from 'dashboard/composables/useAccount';
import { FEATURE_FLAGS } from '../../../../featureFlags';
import semver from 'semver';
import { getLanguageDirection } from 'dashboard/components/widgets/conversation/advancedFilterItems/languages';
@@ -13,9 +14,10 @@ export default {
setup() {
const { updateUISettings } = useUISettings();
const { enabledLanguages } = useConfig();
const { accountId } = useAccount();
const v$ = useVuelidate();
return { updateUISettings, v$, enabledLanguages };
return { updateUISettings, v$, enabledLanguages, accountId };
},
data() {
return {
@@ -46,7 +48,6 @@ export default {
globalConfig: 'globalConfig/get',
getAccount: 'accounts/getAccount',
uiFlags: 'accounts/getUIFlags',
accountId: 'getCurrentAccountId',
isFeatureEnabledonAccount: 'accounts/isFeatureEnabledonAccount',
}),
showAutoResolutionConfig() {

View File

@@ -10,7 +10,6 @@ import Multiselect from 'vue-multiselect';
import { plugin, defaultConfig } from '@formkit/vue';
import WootSwitch from 'components/ui/Switch.vue';
import WootWizard from 'components/ui/Wizard.vue';
import { sync } from 'vuex-router-sync';
import FloatingVue from 'floating-vue';
import WootUiKit from 'dashboard/components';
import App from 'dashboard/App.vue';
@@ -18,6 +17,7 @@ import i18nMessages from 'dashboard/i18n';
import createAxios from 'dashboard/helper/APIHelper';
import commonHelpers, { isJSONValid } from 'dashboard/helper/commons';
import { sync } from 'vuex-router-sync';
import router, { initalizeRouter } from 'dashboard/routes';
import store from 'dashboard/store';
import constants from 'dashboard/constants/globals';
@@ -42,6 +42,8 @@ const i18n = createI18n({
messages: i18nMessages,
});
sync(store, router);
const app = createApp(App);
app.use(i18n);
app.use(store);
@@ -97,7 +99,6 @@ app.component('fluent-icon', FluentIcon);
app.directive('resize', vResizeObserver);
app.directive('on-clickaway', onClickaway);
sync(store, router);
// load common helpers into js
commonHelpers();
window.WOOT_STORE = store;