From 5690c5fee13b95496bb77de8740ad517e4e5933c Mon Sep 17 00:00:00 2001 From: Jen Huang Date: Fri, 10 May 2024 07:20:43 -0700 Subject: [PATCH] [UII] Use local storage for tours in Fleet and Integrations (#183102) ## Summary Resolves https://github.com/elastic/kibana/issues/180659. The original bug reported an error being throw when trying to dismiss the agent activity tour while logged in as a user with limited access. This was happening because the tour was backed by a `uiSetting` and attempted to write a new value to `/internal/kibana/settings` after dismissing the tour. `uiSettings` backs the global Kibana advanced settings used by all users in an instance. Therefore only users with access to Management > Advanced settings are allowed to write to it. The tours that we use in Fleet and Integrations is not the right use case for `uiSettings`. This PR changes the logic for the two tours (agent activity and add agent) so that: - If `uiSetting`'s `hideAnnouncements` is true, never show these tours - this is a global setting intended to surpress these kind of tours and messages, see https://github.com/elastic/kibana/pull/135030 - Otherwise read from and write to local storage (using Kibana's `storage` service) to determine whether to show these tours or not - Never attempt to write to `uiSettings` - Normalize the code pattern used by hooks related to the inactive agents tour (they were already using local storage but not from `useStartServices`) You can test this by: 1. Opening actions menu in an agent list row, add a new tag to the agent, and dismiss the Agent activity tour. Refresh the page, perform the action again, and tour should not show 2. From Integrations, add an integration to a new policy without adding an agent, after returning to the integration policies list, dismiss the Add agent activity tour. Refresh the page, perform the action again, and tour should not show You can clear these `fleet.*` local settings to trigger the tours again, or use another browser / incognito mode: image (cherry picked from commit 8cf79b71f26b324a27a45e6a3729cbcfb64643bb) --- .../fleet/cypress/e2e/agents/agent_list.cy.ts | 2 +- .../components/agent_activity_button.tsx | 18 +++++++++-- .../components/agent_status_filter.test.tsx | 32 +++++++++++-------- ...ctive_agents_callout_has_been_dismissed.ts | 24 ++++++++++---- .../use_last_seen_inactive_agents_count.ts | 9 ++++-- .../components/add_agent_help_popover.tsx | 29 +++++++++++++---- .../plugins/fleet/public/constants/index.ts | 18 +++++++++++ 7 files changed, 97 insertions(+), 35 deletions(-) diff --git a/x-pack/plugins/fleet/cypress/e2e/agents/agent_list.cy.ts b/x-pack/plugins/fleet/cypress/e2e/agents/agent_list.cy.ts index 27a1098fe8256..459fa5f53bfc5 100644 --- a/x-pack/plugins/fleet/cypress/e2e/agents/agent_list.cy.ts +++ b/x-pack/plugins/fleet/cypress/e2e/agents/agent_list.cy.ts @@ -91,7 +91,7 @@ describe('View agents list', () => { deleteAgentDocs(true); cleanupAgentPolicies(); setupFleetServer(); - setUISettings('hideAgentActivityTour', true); + setUISettings('hideAnnouncements', true); cy.getKibanaVersion().then((version) => { docs = createAgentDocs(version); diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_button.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_button.tsx index 7df5fc6ed0110..e8dad92b448d3 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_button.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_activity_button.tsx @@ -9,19 +9,31 @@ import { EuiButtonEmpty, EuiText, EuiTourStep } from '@elastic/eui'; import React, { useEffect, useState } from 'react'; import { FormattedMessage } from '@kbn/i18n-react'; +import type { TOUR_STORAGE_CONFIG } from '../../../../constants'; +import { TOUR_STORAGE_KEYS } from '../../../../constants'; import { useStartServices } from '../../../../hooks'; export const AgentActivityButton: React.FC<{ onClickAgentActivity: () => void; showAgentActivityTour: { isOpen: boolean }; }> = ({ onClickAgentActivity, showAgentActivityTour }) => { - const { uiSettings } = useStartServices(); + const { storage, uiSettings } = useStartServices(); const [agentActivityTourState, setAgentActivityTourState] = useState(showAgentActivityTour); - const isTourHidden = uiSettings.get('hideAgentActivityTour', false); + const isTourHidden = + uiSettings.get('hideAnnouncements', false) || + ( + storage.get(TOUR_STORAGE_KEYS.AGENT_ACTIVITY) as + | TOUR_STORAGE_CONFIG['AGENT_ACTIVITY'] + | undefined + )?.active === false; - const setTourAsHidden = () => uiSettings.set('hideAgentActivityTour', true); + const setTourAsHidden = () => { + storage.set(TOUR_STORAGE_KEYS.AGENT_ACTIVITY, { + active: false, + } as TOUR_STORAGE_CONFIG['AGENT_ACTIVITY']); + }; useEffect(() => { setAgentActivityTourState(showAgentActivityTour); diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_status_filter.test.tsx b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_status_filter.test.tsx index f8e3465557a81..d840ea4a74f87 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_status_filter.test.tsx +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/components/agent_status_filter.test.tsx @@ -11,6 +11,21 @@ import { __IntlProvider as IntlProvider } from '@kbn/i18n-react'; import { AgentStatusFilter } from './agent_status_filter'; const PARTIAL_TOUR_TEXT = 'Some agents have become inactive and have been hidden'; +const mockStorage: Record = {}; + +jest.mock('../../../../../../hooks', () => { + return { + useStartServices: jest.fn(() => ({ + uiSettings: { + get: jest.fn(() => false), + }, + storage: { + get: jest.fn((key) => mockStorage[key]), + set: jest.fn((key, val) => (mockStorage[key] = val)), + }, + })), + }; +}); const renderComponent = (props: React.ComponentProps) => { return render( @@ -20,18 +35,7 @@ const renderComponent = (props: React.ComponentProps) ); }; -const mockLocalStorage: Record = {}; describe('AgentStatusFilter', () => { - beforeEach(() => { - Object.defineProperty(window, 'localStorage', { - value: { - getItem: jest.fn((key) => mockLocalStorage[key]), - setItem: jest.fn((key, val) => (mockLocalStorage[key] = val)), - }, - writable: true, - }); - }); - it('Renders all statuses', () => { const { getByText } = renderComponent({ selectedStatus: [], @@ -70,12 +74,12 @@ describe('AgentStatusFilter', () => { expect(getByText('999')).toBeInTheDocument(); - expect(mockLocalStorage['fleet.inactiveAgentsCalloutHasBeenDismissed']).toBe('true'); + expect(mockStorage['fleet.inactiveAgentsTour']).toEqual({ active: false }); }); }); it('Should not show tour if previously been dismissed', async () => { - mockLocalStorage['fleet.inactiveAgentsCalloutHasBeenDismissed'] = 'true'; + mockStorage['fleet.inactiveAgentsTour'] = { active: false }; const { getByText } = renderComponent({ selectedStatus: [], @@ -89,7 +93,7 @@ describe('AgentStatusFilter', () => { }); it('Should should show difference between last seen inactive agents and total agents', async () => { - mockLocalStorage['fleet.lastSeenInactiveAgentsCount'] = '100'; + mockStorage['fleet.lastSeenInactiveAgentsCount'] = '100'; const { getByText, getByTestId } = renderComponent({ selectedStatus: [], diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_inactive_agents_callout_has_been_dismissed.ts b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_inactive_agents_callout_has_been_dismissed.ts index c9b3d1248a68b..2858b4be25f7c 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_inactive_agents_callout_has_been_dismissed.ts +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_inactive_agents_callout_has_been_dismissed.ts @@ -7,21 +7,31 @@ import { useState, useEffect } from 'react'; -const LOCAL_STORAGE_KEY = 'fleet.inactiveAgentsCalloutHasBeenDismissed'; +import type { TOUR_STORAGE_CONFIG } from '../../../../../../constants'; +import { TOUR_STORAGE_KEYS } from '../../../../../../constants'; +import { useStartServices } from '../../../../../../hooks'; export const useInactiveAgentsCalloutHasBeenDismissed = (): [boolean, (val: boolean) => void] => { + const { uiSettings, storage } = useStartServices(); + const [inactiveAgentsCalloutHasBeenDismissed, setInactiveAgentsCalloutHasBeenDismissed] = useState(false); useEffect(() => { - const storageValue = localStorage.getItem(LOCAL_STORAGE_KEY); - if (storageValue) { - setInactiveAgentsCalloutHasBeenDismissed(Boolean(storageValue)); - } - }, []); + setInactiveAgentsCalloutHasBeenDismissed( + uiSettings.get('hideAnnouncements', false) || + ( + storage.get(TOUR_STORAGE_KEYS.INACTIVE_AGENTS) as + | TOUR_STORAGE_CONFIG['INACTIVE_AGENTS'] + | undefined + )?.active === false + ); + }, [storage, uiSettings]); const updateInactiveAgentsCalloutHasBeenDismissed = (newValue: boolean) => { - localStorage.setItem(LOCAL_STORAGE_KEY, newValue.toString()); + storage.set(TOUR_STORAGE_KEYS.INACTIVE_AGENTS, { + active: false, + } as TOUR_STORAGE_CONFIG['INACTIVE_AGENTS']); setInactiveAgentsCalloutHasBeenDismissed(newValue); }; diff --git a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_last_seen_inactive_agents_count.ts b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_last_seen_inactive_agents_count.ts index e1baf8f277ff1..f875d093061f3 100644 --- a/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_last_seen_inactive_agents_count.ts +++ b/x-pack/plugins/fleet/public/applications/fleet/sections/agents/agent_list_page/hooks/use_last_seen_inactive_agents_count.ts @@ -7,19 +7,22 @@ import { useState, useEffect } from 'react'; +import { useStartServices } from '../../../../../../hooks'; + const LOCAL_STORAGE_KEY = 'fleet.lastSeenInactiveAgentsCount'; export const useLastSeenInactiveAgentsCount = (): [number, (val: number) => void] => { + const { storage } = useStartServices(); const [lastSeenInactiveAgentsCount, setLastSeenInactiveAgentsCount] = useState(0); useEffect(() => { - const storageValue = localStorage.getItem(LOCAL_STORAGE_KEY); + const storageValue = storage.get(LOCAL_STORAGE_KEY); if (storageValue) { setLastSeenInactiveAgentsCount(parseInt(storageValue, 10)); } - }, []); + }, [storage]); const updateLastSeenInactiveAgentsCount = (inactiveAgents: number) => { - localStorage.setItem(LOCAL_STORAGE_KEY, inactiveAgents.toString()); + storage.set(LOCAL_STORAGE_KEY, inactiveAgents.toString()); setLastSeenInactiveAgentsCount(inactiveAgents); }; diff --git a/x-pack/plugins/fleet/public/components/add_agent_help_popover.tsx b/x-pack/plugins/fleet/public/components/add_agent_help_popover.tsx index 7d48db2436cdc..51cc95ffe4620 100644 --- a/x-pack/plugins/fleet/public/components/add_agent_help_popover.tsx +++ b/x-pack/plugins/fleet/public/components/add_agent_help_popover.tsx @@ -15,6 +15,8 @@ import { useTheme } from 'styled-components'; import type { EuiTheme } from '@kbn/kibana-react-plugin/common'; +import type { TOUR_STORAGE_CONFIG } from '../constants'; +import { TOUR_STORAGE_KEYS } from '../constants'; import { useStartServices } from '../hooks'; export const AddAgentHelpPopover = ({ @@ -28,19 +30,31 @@ export const AddAgentHelpPopover = ({ offset?: number; closePopover: NoArgCallback; }) => { - const { docLinks, uiSettings } = useStartServices(); + const { docLinks, uiSettings, storage } = useStartServices(); const theme = useTheme() as EuiTheme; const optionalProps: { offset?: number } = {}; - const hideAnnouncements: boolean = useMemo( - () => uiSettings.get('hideAnnouncements'), - [uiSettings] - ); + const hideAddAgentTour: boolean = useMemo(() => { + return ( + uiSettings.get('hideAnnouncements', false) || + ( + storage.get(TOUR_STORAGE_KEYS.ADD_AGENT_POPOVER) as + | TOUR_STORAGE_CONFIG['ADD_AGENT_POPOVER'] + | undefined + )?.active === false + ); + }, [storage, uiSettings]); + + const onFinish = () => { + storage.set(TOUR_STORAGE_KEYS.ADD_AGENT_POPOVER, { + active: false, + } as TOUR_STORAGE_CONFIG['ADD_AGENT_POPOVER']); + }; if (offset !== undefined) { optionalProps.offset = offset; // offset being present in props sets it to 0 so only add if specified } - return hideAnnouncements ? ( + return hideAddAgentTour ? ( button ) : ( {}} + onFinish={onFinish} step={1} stepsTotal={1} title={ @@ -82,6 +96,7 @@ export const AddAgentHelpPopover = ({ footerAction={ { + onFinish(); closePopover(); }} > diff --git a/x-pack/plugins/fleet/public/constants/index.ts b/x-pack/plugins/fleet/public/constants/index.ts index cfcc38df4cf70..c76f74586ba34 100644 --- a/x-pack/plugins/fleet/public/constants/index.ts +++ b/x-pack/plugins/fleet/public/constants/index.ts @@ -41,3 +41,21 @@ export const DURATION_APM_SETTINGS_VARS = { TAIL_SAMPLING_INTERVAL: 'tail_sampling_interval', WRITE_TIMEOUT: 'write_timeout', }; + +export const TOUR_STORAGE_KEYS = { + AGENT_ACTIVITY: 'fleet.agentActivityTour', + ADD_AGENT_POPOVER: 'fleet.addAgentPopoverTour', + INACTIVE_AGENTS: 'fleet.inactiveAgentsTour', +}; + +export interface TOUR_STORAGE_CONFIG { + AGENT_ACTIVITY: { + active: boolean; + }; + ADD_AGENT_POPOVER: { + active: boolean; + }; + INACTIVE_AGENTS: { + active: boolean; + }; +}