fix: avoid routes to be overwritten when set in the store

Sometimes the module are not shown in the primary bar even if they are loaded right. Logging the draft of the addRoute producer, it seems that the order of execution of the addRoute, plus the direct set made by the default views, makes the primary views array lenght change in an unpredictable way. Sometimes happens that the length in the execution n is greater then the lenght in the exectution n+1. This fix aims to avoid this kind of race condition, by registering all the routes (so even the default ones) through the setters provided by the store. In the setters, the curried produce of immer is used to update the store through a direct modification of the draftState. By doing so, there should not present anymore the situation where the state is entirely replaced by an older version of the draft, since the draft is always incrementally updated with the push (to add) and the splice (to remove), and nevere replaced with a direct assignation. refs: SHELL-152 (#314)
zextras · Aug 23, 2023 · af5f0d8 · af5f0d8
1 parent d9d881a
commit af5f0d8
Show file tree

Hide file tree

Showing 7 changed files with 173 additions and 235 deletions.
diff --git a/src/boot/app/default-views.ts b/src/boot/app/default-views.ts
@@ -6,10 +6,9 @@
 /* eslint-disable no-param-reassign */
 
 import type { TFunction } from 'i18next';
-import { produce } from 'immer';
 import { size } from 'lodash';
 
-import type { AppState, PrimaryBarView, SettingsView } from '../../../types';
+import type { AppRouteDescriptor, SettingsView } from '../../../types';
 import { SEARCH_APP_ID, SETTINGS_APP_ID, SHELL_APP_ID } from '../../constants';
 import { SearchAppView } from '../../search/search-app-view';
 import { AccountsSettings } from '../../settings/accounts-settings';
@@ -20,37 +19,6 @@ import { SettingsSidebar } from '../../settings/settings-sidebar';
 import { useAccountStore } from '../../store/account';
 import { useAppStore } from '../../store/app';
 
-const settingsRoute = {
-	route: SETTINGS_APP_ID,
-	id: SETTINGS_APP_ID,
-	app: SETTINGS_APP_ID
-};
-
-const settingsPrimaryBar = (t: TFunction): PrimaryBarView => ({
-	id: SETTINGS_APP_ID,
-	app: SETTINGS_APP_ID,
-	route: SETTINGS_APP_ID,
-	component: 'SettingsModOutline',
-	position: 16,
-	visible: true,
-	label: t('settings.app', 'Settings'),
-	badge: {
-		show: false
-	}
-});
-const settingsSecondaryBar = {
-	id: SETTINGS_APP_ID,
-	app: SETTINGS_APP_ID,
-	route: SETTINGS_APP_ID,
-	component: SettingsSidebar
-};
-
-const settingsAppView = {
-	id: SETTINGS_APP_ID,
-	app: SETTINGS_APP_ID,
-	route: SETTINGS_APP_ID,
-	component: SettingsAppView
-};
 const settingsGeneralView = (t: TFunction): SettingsView => ({
 	id: 'general',
 	route: 'general',
@@ -72,50 +40,41 @@ const settingsAccountsView = (t: TFunction): SettingsView => ({
 	position: 1
 });
 
-const searchRoute = {
-	route: SEARCH_APP_ID,
-	id: SEARCH_APP_ID,
-	app: SEARCH_APP_ID
-};
-const searchPrimaryBar = (t: TFunction): PrimaryBarView => ({
+const searchRouteDescriptor = (t: TFunction): AppRouteDescriptor => ({
 	id: SEARCH_APP_ID,
 	app: SEARCH_APP_ID,
 	route: SEARCH_APP_ID,
-	component: 'SearchModOutline',
+	appView: SearchAppView,
+	badge: {
+		show: false
+	},
+	label: t('search.app', 'Search'),
 	position: 15,
 	visible: true,
-	label: t('search.app', 'Search'),
+	primaryBar: 'SearchModOutline'
+});
+
+const settingsRouteDescriptor = (t: TFunction): AppRouteDescriptor => ({
+	id: SETTINGS_APP_ID,
+	app: SETTINGS_APP_ID,
+	route: SETTINGS_APP_ID,
+	appView: SettingsAppView,
 	badge: {
 		show: false
-	}
+	},
+	label: t('settings.app', 'Settings'),
+	position: 16,
+	visible: true,
+	primaryBar: 'SettingsModOutline',
+	secondaryBar: SettingsSidebar
 });
-const searchAppView = {
-	id: SEARCH_APP_ID,
-	app: SEARCH_APP_ID,
-	route: SEARCH_APP_ID,
-	component: SearchAppView
-};
 
 export const registerDefaultViews = (t: TFunction): void => {
-	useAppStore.setState(
-		produce((s: AppState) => {
-			const { attrs } = useAccountStore.getState().settings;
-			if (size(attrs) === 0 || attrs.zimbraFeatureOptionsEnabled === 'FALSE') {
-				s.routes = {
-					[SEARCH_APP_ID]: searchRoute
-				};
-				s.views.primaryBar = [searchPrimaryBar(t)];
-				s.views.appView = [searchAppView];
-			} else {
-				s.routes = {
-					[SEARCH_APP_ID]: searchRoute,
-					[SETTINGS_APP_ID]: settingsRoute
-				};
-				s.views.primaryBar = [searchPrimaryBar(t), settingsPrimaryBar(t)];
-				s.views.secondaryBar = [settingsSecondaryBar];
-				s.views.appView = [searchAppView, settingsAppView];
-				s.views.settings = [settingsGeneralView(t), settingsAccountsView(t)];
-			}
-		})
-	);
+	const { attrs } = useAccountStore.getState().settings;
+	if (size(attrs) > 0 && attrs.zimbraFeatureOptionsEnabled !== 'FALSE') {
+		useAppStore.getState().setters.addRoute(settingsRouteDescriptor(t));
+		useAppStore.getState().setters.addSettingsView(settingsGeneralView(t));
+		useAppStore.getState().setters.addSettingsView(settingsAccountsView(t));
+	}
+	useAppStore.getState().setters.addRoute(searchRouteDescriptor(t));
 };
diff --git a/src/network/get-components.ts b/src/network/get-components.ts
@@ -20,5 +20,5 @@ export const getComponents = (): Promise<void> =>
 		.then(({ components }: { components: Array<CarbonioModule> }) => {
 			useAppStore
 				.getState()
-				.setters.addApps(filter(components, ({ type }) => type === 'shell' || type === 'carbonio'));
+				.setters.setApps(filter(components, ({ type }) => type === 'shell' || type === 'carbonio'));
 		});
diff --git a/src/shell/shell-primary-bar.test.tsx b/src/shell/shell-primary-bar.test.tsx
@@ -135,7 +135,7 @@ describe('Shell primary bar', () => {
 		const { getByRoleWithIcon, user } = setup(<ShellWrapper />);
 
 		act(() => {
-			useAppStore.getState().setters.addApps([
+			useAppStore.getState().setters.setApps([
 				{
 					commit: '',
 					description: 'Mails module',
@@ -215,7 +215,7 @@ describe('Shell primary bar', () => {
 		const { getByRoleWithIcon, user } = setup(<ShellWrapper />);
 
 		act(() => {
-			useAppStore.getState().setters.addApps([
+			useAppStore.getState().setters.setApps([
 				{
 					commit: '',
 					description: 'Mails module',
@@ -314,7 +314,7 @@ describe('Shell primary bar', () => {
 		const { getByRoleWithIcon, user } = setup(<ShellWrapper />);
 
 		act(() => {
-			useAppStore.getState().setters.addApps([
+			useAppStore.getState().setters.setApps([
 				{
 					commit: '',
 					description: 'Mails module',

diff --git a/src/shell/shell-primary-bar.tsx b/src/shell/shell-primary-bar.tsx
@@ -12,7 +12,7 @@ import { useHistory, useLocation } from 'react-router-dom';
 import styled from 'styled-components';
 
 import BadgeWrap from './badge-wrap';
-import { AppRoute, PrimaryAccessoryView, PrimaryBarView } from '../../types';
+import { PrimaryAccessoryView, PrimaryBarView } from '../../types';
 import AppContextProvider from '../boot/app/app-context-provider';
 import {
 	BOARD_CONTAINER_ZINDEX,
@@ -98,12 +98,8 @@ const OverlayRow = styled(Row)`
 	overflow-y: overlay;
 `;
 
-interface ShellPrimaryBarComponentProps {
-	activeRoute: AppRoute | undefined;
-}
-const ShellPrimaryBarComponent = ({
-	activeRoute
-}: ShellPrimaryBarComponentProps): JSX.Element | null => {
+const ShellPrimaryBar = (): JSX.Element | null => {
+	const activeRoute = useCurrentRoute();
 	const primaryBarViews = useAppStore((s) => s.views.primaryBar);
 	const { push } = useHistory();
 
@@ -126,7 +122,8 @@ const ShellPrimaryBarComponent = ({
 				[activeRoute.id]: `${trim(pathname, '/')}${search}`
 			};
 		}
-	}, [activeRoute, pathname, search, primaryBarViews]);
+	}, [activeRoute, pathname, search]);
+
 	const primaryBarAccessoryViews = useAppStore((s) => s.views.primaryBarAccessories);
 
 	const accessoryViews = useMemo(
@@ -189,11 +186,4 @@ const ShellPrimaryBarComponent = ({
 	);
 };
 
-const MemoShellPrimaryBarComponent = React.memo(ShellPrimaryBarComponent);
-
-const ShellPrimaryBar = (): JSX.Element => {
-	const activeRoute = useCurrentRoute();
-	return <MemoShellPrimaryBarComponent activeRoute={activeRoute} />;
-};
-
 export default ShellPrimaryBar;