From f3b80f77a6ca38492407cde157b4c6847f16b662 Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Mon, 22 Apr 2024 13:31:27 -0400 Subject: [PATCH] Remove global state access from components/root (#26751) * root part 1 - rudder * root part 2 - luxon * root part 3 - recent emojis * root part 4 - redirect to onboarding * root part 5 - login logout handler * Fix indentation --- webapp/channels/src/actions/views/root.ts | 11 +- .../root/__snapshots__/root.test.tsx.snap | 1 + .../channels/src/components/root/actions.ts | 91 +++++++++++++++ .../channels/src/components/root/effects.ts | 26 ----- webapp/channels/src/components/root/index.ts | 6 +- .../src/components/root/luxon_controller.tsx | 25 +++++ .../src/components/root/root.test.tsx | 92 +++++++-------- webapp/channels/src/components/root/root.tsx | 106 +++--------------- .../mattermost-redux/src/actions/general.ts | 3 +- 9 files changed, 197 insertions(+), 164 deletions(-) create mode 100644 webapp/channels/src/components/root/actions.ts delete mode 100644 webapp/channels/src/components/root/effects.ts create mode 100644 webapp/channels/src/components/root/luxon_controller.tsx diff --git a/webapp/channels/src/actions/views/root.ts b/webapp/channels/src/actions/views/root.ts index 3e0d6b9641..d3e98e6baa 100644 --- a/webapp/channels/src/actions/views/root.ts +++ b/webapp/channels/src/actions/views/root.ts @@ -1,6 +1,8 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. +import type {ClientConfig} from '@mattermost/types/config'; + import {getClientConfig, getLicenseConfig} from 'mattermost-redux/actions/general'; import {loadMe} from 'mattermost-redux/actions/users'; import {Client4} from 'mattermost-redux/client'; @@ -18,9 +20,9 @@ const pluginTranslationSources: Record = {}; export type TranslationPluginFunction = (locale: string) => Translations -export function loadConfigAndMe(): ActionFuncAsync { +export function loadConfigAndMe(): ThunkActionFunc> { return async (dispatch) => { - await Promise.all([ + const results = await Promise.all([ dispatch(getClientConfig()), dispatch(getLicenseConfig()), ]); @@ -31,7 +33,10 @@ export function loadConfigAndMe(): ActionFuncAsync { isMeLoaded = dataFromLoadMe?.data ?? false; } - return {data: isMeLoaded}; + return { + config: results[0].data, + isMeLoaded, + }; }; } diff --git a/webapp/channels/src/components/root/__snapshots__/root.test.tsx.snap b/webapp/channels/src/components/root/__snapshots__/root.test.tsx.snap index 2528a58fb8..a9c5aaf9d4 100644 --- a/webapp/channels/src/components/root/__snapshots__/root.test.tsx.snap +++ b/webapp/channels/src/components/root/__snapshots__/root.test.tsx.snap @@ -3,6 +3,7 @@ exports[`components/Root Routes Should mount public product routes 1`] = ` + { + return async (dispatch, getState) => { + const state = getState(); + const isUserAdmin = isCurrentUserSystemAdmin(state); + if (!isUserAdmin) { + GlobalActions.redirectUserToDefaultTeam(); + return; + } + + const teams = getActiveTeamsList(state); + + const onboardingFlowEnabled = getIsOnboardingFlowEnabled(state); + + if (teams.length > 0 || !onboardingFlowEnabled) { + GlobalActions.redirectUserToDefaultTeam(); + return; + } + + const firstAdminSetupComplete = await dispatch(getFirstAdminSetupComplete()); + if (firstAdminSetupComplete?.data) { + GlobalActions.redirectUserToDefaultTeam(); + return; + } + + const profilesResult = await dispatch(getProfiles(0, General.PROFILE_CHUNK_SIZE, {roles: General.SYSTEM_ADMIN_ROLE})); + if (profilesResult.error) { + GlobalActions.redirectUserToDefaultTeam(); + return; + } + const currentUser = getCurrentUser(getState()); + const adminProfiles = profilesResult.data?.reduce( + (acc: Record, curr: UserProfile) => { + acc[curr.id] = curr; + return acc; + }, + {}, + ); + if (adminProfiles && checkIsFirstAdmin(currentUser, adminProfiles)) { + history.push('/preparing-workspace'); + return; + } + + GlobalActions.redirectUserToDefaultTeam(); + }; +} + +export function handleLoginLogoutSignal(e: StorageEvent): ThunkActionFunc { + return (dispatch, getState) => { + // when one tab on a browser logs out, it sets __logout__ in localStorage to trigger other tabs to log out + const isNewLocalStorageEvent = (event: StorageEvent) => event.storageArea === localStorage && event.newValue; + + if (e.key === StoragePrefixes.LOGOUT && isNewLocalStorageEvent(e)) { + console.log('detected logout from a different tab'); //eslint-disable-line no-console + GlobalActions.emitUserLoggedOutEvent('/', false, false); + } + if (e.key === StoragePrefixes.LOGIN && isNewLocalStorageEvent(e)) { + const isLoggedIn = getCurrentUser(getState()); + + // make sure this is not the same tab which sent login signal + // because another tabs will also send login signal after reloading + if (isLoggedIn) { + return; + } + + // detected login from a different tab + function reloadOnFocus() { + location.reload(); + } + window.addEventListener('focus', reloadOnFocus); + } + }; +} diff --git a/webapp/channels/src/components/root/effects.ts b/webapp/channels/src/components/root/effects.ts deleted file mode 100644 index cf4465f220..0000000000 --- a/webapp/channels/src/components/root/effects.ts +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. -// See LICENSE.txt for license information. - -import {Settings} from 'luxon'; - -import {getCurrentTimezone} from 'mattermost-redux/selectors/entities/timezone'; - -import {getCurrentLocale} from 'selectors/i18n'; - -import type {GlobalState} from 'types/store'; - -let prevTimezone: string | undefined; -let prevLocale: string | undefined; -export function applyLuxonDefaults(state: GlobalState) { - const locale = getCurrentLocale(state); - if (locale !== prevLocale) { - prevLocale = locale; - Settings.defaultLocale = locale; - } - - const tz = getCurrentTimezone(state); - if (tz !== prevTimezone) { - prevTimezone = tz; - Settings.defaultZone = tz ?? 'system'; - } -} diff --git a/webapp/channels/src/components/root/index.ts b/webapp/channels/src/components/root/index.ts index 18cb3cca70..55df272a87 100644 --- a/webapp/channels/src/components/root/index.ts +++ b/webapp/channels/src/components/root/index.ts @@ -13,7 +13,7 @@ import {getTheme} from 'mattermost-redux/selectors/entities/preferences'; import {getTeam} from 'mattermost-redux/selectors/entities/teams'; import {shouldShowTermsOfService, getCurrentUserId} from 'mattermost-redux/selectors/entities/users'; -import {migrateRecentEmojis} from 'actions/emoji_actions'; +import {loadRecentlyUsedCustomEmojis, migrateRecentEmojis} from 'actions/emoji_actions'; import {loadConfigAndMe, registerCustomPostRenderer} from 'actions/views/root'; import {getShowLaunchingWorkspace} from 'selectors/onboarding'; import {shouldShowAppBar} from 'selectors/plugins'; @@ -28,6 +28,7 @@ import {initializeProducts} from 'plugins/products'; import type {GlobalState} from 'types/store/index'; +import {handleLoginLogoutSignal, redirectToOnboardingOrDefaultTeam} from './actions'; import Root from './root'; function mapStateToProps(state: GlobalState) { @@ -67,9 +68,12 @@ function mapDispatchToProps(dispatch: Dispatch) { loadConfigAndMe, getFirstAdminSetupComplete, getProfiles, + loadRecentlyUsedCustomEmojis, migrateRecentEmojis, registerCustomPostRenderer, initializeProducts, + handleLoginLogoutSignal, + redirectToOnboardingOrDefaultTeam, }, dispatch), }; } diff --git a/webapp/channels/src/components/root/luxon_controller.tsx b/webapp/channels/src/components/root/luxon_controller.tsx new file mode 100644 index 0000000000..3af0cef334 --- /dev/null +++ b/webapp/channels/src/components/root/luxon_controller.tsx @@ -0,0 +1,25 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {Settings} from 'luxon'; +import {useEffect} from 'react'; +import {useSelector} from 'react-redux'; + +import {getCurrentTimezone} from 'mattermost-redux/selectors/entities/timezone'; + +import {getCurrentLocale} from 'selectors/i18n'; + +export default function LuxonController() { + const locale = useSelector(getCurrentLocale); + + useEffect(() => { + Settings.defaultLocale = locale; + }, [locale]); + + const tz = useSelector(getCurrentTimezone); + useEffect(() => { + Settings.defaultZone = tz ?? 'system'; + }, [tz]); + + return null; +} diff --git a/webapp/channels/src/components/root/root.test.tsx b/webapp/channels/src/components/root/root.test.tsx index 4d4c5e5b08..461a111e68 100644 --- a/webapp/channels/src/components/root/root.test.tsx +++ b/webapp/channels/src/components/root/root.test.tsx @@ -4,23 +4,25 @@ import {shallow} from 'enzyme'; import React from 'react'; import type {RouteComponentProps} from 'react-router-dom'; +import {bindActionCreators} from 'redux'; import rudderAnalytics from 'rudder-sdk-js'; import {ServiceEnvironment} from '@mattermost/types/config'; -import {GeneralTypes} from 'mattermost-redux/action_types'; import {Client4} from 'mattermost-redux/client'; import type {Theme} from 'mattermost-redux/selectors/entities/preferences'; import * as GlobalActions from 'actions/global_actions'; -import store from 'stores/redux_store'; import Root from 'components/root/root'; +import testConfigureStore from 'packages/mattermost-redux/test/test_store'; import {StoragePrefixes} from 'utils/constants'; import type {ProductComponent} from 'types/store/plugins'; +import {handleLoginLogoutSignal, redirectToOnboardingOrDefaultTeam} from './actions'; + jest.mock('rudder-sdk-js', () => ({ identify: jest.fn(), load: jest.fn(), @@ -43,15 +45,20 @@ jest.mock('utils/utils', () => { localizeMessage: () => {}, applyTheme: jest.fn(), makeIsEligibleForClick: jest.fn(), - }; }); jest.mock('mattermost-redux/actions/general', () => ({ + getFirstAdminSetupComplete: jest.fn(() => Promise.resolve({ + type: 'FIRST_ADMIN_COMPLETE_SETUP_RECEIVED', + data: true, + })), setUrl: () => {}, })); describe('components/Root', () => { + const store = testConfigureStore(); + const baseProps = { telemetryEnabled: true, telemetryId: '1234ab', @@ -61,18 +68,20 @@ describe('components/Root', () => { actions: { loadConfigAndMe: jest.fn().mockImplementation(() => { return Promise.resolve({ - data: false, + config: {}, + isMeLoaded: false, }); }), - getFirstAdminSetupComplete: jest.fn(() => Promise.resolve({ - type: GeneralTypes.FIRST_ADMIN_COMPLETE_SETUP_RECEIVED, - data: true, - })), getProfiles: jest.fn(), + loadRecentlyUsedCustomEmojis: jest.fn(), migrateRecentEmojis: jest.fn(), savePreferences: jest.fn(), registerCustomPostRenderer: jest.fn(), initializeProducts: jest.fn(), + ...bindActionCreators({ + handleLoginLogoutSignal, + redirectToOnboardingOrDefaultTeam, + }, store.dispatch), }, permalinkRedirectTeamName: 'myTeam', showLaunchingWorkspace: false, @@ -98,9 +107,9 @@ describe('components/Root', () => { } as unknown as RouteComponentProps['history'], }; - const wrapper = shallow(); + const wrapper = shallow(); - (wrapper.instance() as any).onConfigLoaded(); + wrapper.instance().onConfigLoaded({}); expect(props.history.push).toHaveBeenCalledWith('/signup_user_complete'); wrapper.unmount(); }); @@ -114,7 +123,10 @@ describe('components/Root', () => { actions: { ...baseProps.actions, loadConfigAndMe: jest.fn().mockImplementation(() => { - return Promise.resolve({data: true}); + return Promise.resolve({ + config: {}, + isMeLoaded: true, + }); }), }, }; @@ -145,7 +157,10 @@ describe('components/Root', () => { actions: { ...baseProps.actions, loadConfigAndMe: jest.fn().mockImplementation(() => { - return Promise.resolve({data: true}); + return Promise.resolve({ + config: {}, + isMeLoaded: true, + }); }), }, }; @@ -172,7 +187,7 @@ describe('components/Root', () => { push: jest.fn(), } as unknown as RouteComponentProps['history'], }; - const wrapper = shallow(); + const wrapper = shallow(); expect(props.history.push).not.toHaveBeenCalled(); const props2 = { noAccounts: true, @@ -188,7 +203,7 @@ describe('components/Root', () => { writable: true, }); window.location.reload = jest.fn(); - const wrapper = shallow(); + const wrapper = shallow(); const loginSignal = new StorageEvent('storage', { key: StoragePrefixes.LOGIN, newValue: String(Math.random()), @@ -207,14 +222,11 @@ describe('components/Root', () => { }); test('should not set a TelemetryHandler when onConfigLoaded is called if Rudder is not configured', () => { - store.dispatch({ - type: GeneralTypes.CLIENT_CONFIG_RECEIVED, - data: { - ServiceEnvironment: ServiceEnvironment.DEV, - }, - }); + const wrapper = shallow(); - const wrapper = shallow(); + wrapper.instance().onConfigLoaded({ + ServiceEnvironment: ServiceEnvironment.DEV, + }); Client4.trackEvent('category', 'event'); @@ -224,17 +236,12 @@ describe('components/Root', () => { }); test('should set a TelemetryHandler when onConfigLoaded is called if Rudder is configured', () => { - store.dispatch({ - type: GeneralTypes.CLIENT_CONFIG_RECEIVED, - data: { - ServiceEnvironment: ServiceEnvironment.TEST, - }, + const wrapper = shallow(); + + wrapper.instance().onConfigLoaded({ + ServiceEnvironment: ServiceEnvironment.TEST, }); - const wrapper = shallow(); - - (wrapper.instance() as any).onConfigLoaded(); - Client4.trackEvent('category', 'event'); expect(Client4.telemetryHandler).toBeDefined(); @@ -247,17 +254,12 @@ describe('components/Root', () => { // Simulate an error occurring and the callback not getting called }); - store.dispatch({ - type: GeneralTypes.CLIENT_CONFIG_RECEIVED, - data: { - ServiceEnvironment: ServiceEnvironment.TEST, - }, + const wrapper = shallow(); + + wrapper.instance().onConfigLoaded({ + ServiceEnvironment: ServiceEnvironment.PRODUCTION, }); - const wrapper = shallow(); - - (wrapper.instance() as any).onConfigLoaded(); - Client4.trackEvent('category', 'event'); expect(Client4.telemetryHandler).not.toBeDefined(); @@ -287,9 +289,9 @@ describe('components/Root', () => { } as unknown as ProductComponent], }; - const wrapper = shallow(); + const wrapper = shallow(); - (wrapper.instance() as any).setState({configLoaded: true}); + wrapper.instance().setState({configLoaded: true}); expect(wrapper).toMatchSnapshot(); wrapper.unmount(); }); @@ -313,8 +315,8 @@ describe('components/Root', () => { }; test('should show for normal cases', () => { - const wrapper = shallow(); - (wrapper.instance() as any).onConfigLoaded(); + const wrapper = shallow(); + wrapper.instance().onConfigLoaded({}); expect(landingProps.history.push).toHaveBeenCalledWith('/landing#/'); wrapper.unmount(); }); @@ -328,8 +330,8 @@ describe('components/Root', () => { }, } as RouteComponentProps, }; - const wrapper = shallow(); - (wrapper.instance() as any).onConfigLoaded(); + const wrapper = shallow(); + wrapper.instance().onConfigLoaded({}); expect(props.history.push).not.toHaveBeenCalled(); wrapper.unmount(); }); diff --git a/webapp/channels/src/components/root/root.tsx b/webapp/channels/src/components/root/root.tsx index e27f300665..d96e156459 100644 --- a/webapp/channels/src/components/root/root.tsx +++ b/webapp/channels/src/components/root/root.tsx @@ -3,30 +3,23 @@ import classNames from 'classnames'; import deepEqual from 'fast-deep-equal'; +import type {History} from 'history'; import React from 'react'; import {Route, Switch, Redirect} from 'react-router-dom'; import type {RouteComponentProps} from 'react-router-dom'; +import type {ClientConfig} from '@mattermost/types/config'; import {ServiceEnvironment} from '@mattermost/types/config'; -import type {UserProfile} from '@mattermost/types/users'; import {setSystemEmojis} from 'mattermost-redux/actions/emojis'; import {setUrl} from 'mattermost-redux/actions/general'; import {Client4} from 'mattermost-redux/client'; import {rudderAnalytics, RudderTelemetryHandler} from 'mattermost-redux/client/rudder'; -import {General} from 'mattermost-redux/constants'; -import {getConfig} from 'mattermost-redux/selectors/entities/general'; -import {getIsOnboardingFlowEnabled} from 'mattermost-redux/selectors/entities/preferences'; import type {Theme} from 'mattermost-redux/selectors/entities/preferences'; -import {getActiveTeamsList} from 'mattermost-redux/selectors/entities/teams'; -import {getCurrentUser, isCurrentUserSystemAdmin, checkIsFirstAdmin} from 'mattermost-redux/selectors/entities/users'; import type {ActionResult} from 'mattermost-redux/types/actions'; -import {loadRecentlyUsedCustomEmojis} from 'actions/emoji_actions'; -import * as GlobalActions from 'actions/global_actions'; import {measurePageLoadTelemetry, temporarilySetPageLoadContext, trackEvent, trackSelectorMetrics} from 'actions/telemetry_actions.jsx'; import BrowserStore from 'stores/browser_store'; -import store from 'stores/redux_store'; import AccessProblem from 'components/access_problem'; import AnnouncementBarController from 'components/announcement_bar'; @@ -53,7 +46,7 @@ import webSocketClient from 'client/web_websocket_client'; import {initializePlugins} from 'plugins'; import Pluggable from 'plugins/pluggable'; import A11yController from 'utils/a11y_controller'; -import {PageLoadContext, StoragePrefixes} from 'utils/constants'; +import {PageLoadContext} from 'utils/constants'; import {EmojiIndicesByAlias} from 'utils/emoji'; import {TEAM_NAME_PATH_PATTERN} from 'utils/path'; import {getSiteURL} from 'utils/url'; @@ -62,7 +55,7 @@ import * as Utils from 'utils/utils'; import type {ProductComponent, PluginComponent} from 'types/store/plugins'; -import {applyLuxonDefaults} from './effects'; +import LuxonController from './luxon_controller'; import RootProvider from './root_provider'; import RootRedirect from './root_redirect'; @@ -134,12 +127,14 @@ function LoggedInRoute(props: LoggedInRouteProps) { const noop = () => {}; export type Actions = { - getFirstAdminSetupComplete: () => Promise; getProfiles: (page?: number, pageSize?: number, options?: Record) => Promise; + loadRecentlyUsedCustomEmojis: () => Promise; migrateRecentEmojis: () => void; - loadConfigAndMe: () => Promise; + loadConfigAndMe: () => Promise<{config?: Partial; isMeLoaded: boolean}>; registerCustomPostRenderer: (type: string, component: any, id: string) => Promise; initializeProducts: () => Promise; + handleLoginLogoutSignal: (e: StorageEvent) => unknown; + redirectToOnboardingOrDefaultTeam: (history: History) => unknown; } type Props = { @@ -208,12 +203,9 @@ export default class Root extends React.PureComponent { }; this.a11yController = new A11yController(); - - store.subscribe(() => applyLuxonDefaults(store.getState())); } - onConfigLoaded = () => { - const config = getConfig(store.getState()); + onConfigLoaded = (config: Partial) => { const telemetryId = this.props.telemetryId; const rudderUrl = 'https://pdat.matterlytics.com'; @@ -231,7 +223,7 @@ export default class Root extends React.PureComponent { if (rudderKey !== '' && this.props.telemetryEnabled) { const rudderCfg: {setCookieDomain?: string} = {}; - const siteURL = getConfig(store.getState()).SiteURL; + const siteURL = config.SiteURL; if (siteURL !== '') { try { rudderCfg.setCookieDomain = new URL(siteURL || '').hostname; @@ -293,7 +285,7 @@ export default class Root extends React.PureComponent { }); this.props.actions.migrateRecentEmojis(); - store.dispatch(loadRecentlyUsedCustomEmojis()); + this.props.actions.loadRecentlyUsedCustomEmojis(); this.showLandingPageIfNecessary(); @@ -376,50 +368,6 @@ export default class Root extends React.PureComponent { } } - async redirectToOnboardingOrDefaultTeam() { - const storeState = store.getState(); - const isUserAdmin = isCurrentUserSystemAdmin(storeState); - if (!isUserAdmin) { - GlobalActions.redirectUserToDefaultTeam(); - return; - } - - const teams = getActiveTeamsList(storeState); - - const onboardingFlowEnabled = getIsOnboardingFlowEnabled(storeState); - - if (teams.length > 0 || !onboardingFlowEnabled) { - GlobalActions.redirectUserToDefaultTeam(); - return; - } - - const firstAdminSetupComplete = await this.props.actions.getFirstAdminSetupComplete(); - if (firstAdminSetupComplete?.data) { - GlobalActions.redirectUserToDefaultTeam(); - return; - } - - const profilesResult = await this.props.actions.getProfiles(0, General.PROFILE_CHUNK_SIZE, {roles: General.SYSTEM_ADMIN_ROLE}); - if (profilesResult.error) { - GlobalActions.redirectUserToDefaultTeam(); - return; - } - const currentUser = getCurrentUser(store.getState()); - const adminProfiles = profilesResult.data.reduce( - (acc: Record, curr: UserProfile) => { - acc[curr.id] = curr; - return acc; - }, - {}, - ); - if (checkIsFirstAdmin(currentUser, adminProfiles)) { - this.props.history.push('/preparing-workspace'); - return; - } - - GlobalActions.redirectUserToDefaultTeam(); - } - captureUTMParams() { const qs = new URLSearchParams(window.location.search); @@ -445,13 +393,15 @@ export default class Root extends React.PureComponent { } initiateMeRequests = async () => { - const {data: isMeLoaded} = await this.props.actions.loadConfigAndMe(); + const {config, isMeLoaded} = await this.props.actions.loadConfigAndMe(); if (isMeLoaded && this.props.location.pathname === '/') { - this.redirectToOnboardingOrDefaultTeam(); + this.props.actions.redirectToOnboardingOrDefaultTeam(this.props.history); } - this.onConfigLoaded(); + if (config) { + this.onConfigLoaded(config); + } }; componentDidMount() { @@ -475,28 +425,7 @@ export default class Root extends React.PureComponent { } handleLogoutLoginSignal = (e: StorageEvent) => { - // when one tab on a browser logs out, it sets __logout__ in localStorage to trigger other tabs to log out - const isNewLocalStorageEvent = (event: StorageEvent) => event.storageArea === localStorage && event.newValue; - - if (e.key === StoragePrefixes.LOGOUT && isNewLocalStorageEvent(e)) { - console.log('detected logout from a different tab'); //eslint-disable-line no-console - GlobalActions.emitUserLoggedOutEvent('/', false, false); - } - if (e.key === StoragePrefixes.LOGIN && isNewLocalStorageEvent(e)) { - const isLoggedIn = getCurrentUser(store.getState()); - - // make sure this is not the same tab which sent login signal - // because another tabs will also send login signal after reloading - if (isLoggedIn) { - return; - } - - // detected login from a different tab - function reloadOnFocus() { - location.reload(); - } - window.addEventListener('focus', reloadOnFocus); - } + this.props.actions.handleLoginLogoutSignal(e); }; setRootMeta = () => { @@ -519,6 +448,7 @@ export default class Root extends React.PureComponent { return ( + { return async (dispatch, getState) => { let data; try {