diff --git a/projects/plugins/protect/changelog/update-protect-fixer-ui-to-handle-long-running-fixers b/projects/plugins/protect/changelog/update-protect-fixer-ui-to-handle-long-running-fixers new file mode 100644 index 0000000000000..d8f2fd52d4ce2 --- /dev/null +++ b/projects/plugins/protect/changelog/update-protect-fixer-ui-to-handle-long-running-fixers @@ -0,0 +1,4 @@ +Significance: minor +Type: added + +Adds handling for long running fixers diff --git a/projects/plugins/protect/src/js/components/firewall-header/index.jsx b/projects/plugins/protect/src/js/components/firewall-header/index.jsx index c4a65bab7c63e..0cd9e69b7ba35 100644 --- a/projects/plugins/protect/src/js/components/firewall-header/index.jsx +++ b/projects/plugins/protect/src/js/components/firewall-header/index.jsx @@ -7,13 +7,14 @@ import { Button, Status, } from '@automattic/jetpack-components'; -import { Spinner, Popover } from '@wordpress/components'; +import { Spinner } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; -import { Icon, help } from '@wordpress/icons'; -import React, { useState, useCallback } from 'react'; +import { help } from '@wordpress/icons'; +import React, { useCallback } from 'react'; import useAnalyticsTracks from '../../hooks/use-analytics-tracks'; import usePlan from '../../hooks/use-plan'; import useWafData from '../../hooks/use-waf-data'; +import IconTooltip from '../icon-tooltip'; import styles from './styles.module.scss'; const UpgradePrompt = () => { @@ -44,51 +45,21 @@ const UpgradePrompt = () => { ); }; -const FirewallSubheadingPopover = ( { - children = __( - 'The free version of the firewall does not receive updates to automatic security rules.', - 'jetpack-protect' - ), -} ) => { - const [ showPopover, setShowPopover ] = useState( false ); - - const handleEnter = useCallback( () => { - setShowPopover( true ); - }, [] ); - - const handleOut = useCallback( () => { - setShowPopover( false ); - }, [] ); - - return ( -
- - { showPopover && ( - - - { children } - - - ) } -
- ); -}; - -const FirewallSubheadingContent = ( { className, text = '', popover = false, children } ) => { +const FirewallSubheadingContent = ( { className, text = '', popover = false } ) => { return (
{ text } - { popover && } + { popover && ( + + ) }
); }; diff --git a/projects/plugins/protect/src/js/components/firewall-header/styles.module.scss b/projects/plugins/protect/src/js/components/firewall-header/styles.module.scss index 385ae494760dd..25885b45f95a7 100644 --- a/projects/plugins/protect/src/js/components/firewall-header/styles.module.scss +++ b/projects/plugins/protect/src/js/components/firewall-header/styles.module.scss @@ -17,12 +17,6 @@ svg.spinner { &__content { display: flex; - - > .icon-popover { - display: flex; - margin-left: calc( var( --spacing-base ) / 2 ); // 4px - fill: var( --jp-gray-20 ); - } } } @@ -47,11 +41,6 @@ svg.spinner { } } -.popover-text { - width: 250px; - padding: calc( var( --spacing-base ) * 2 ); // 16px -} - .loading-text { font-size: 18px; } diff --git a/projects/plugins/protect/src/js/components/fix-all-threats-modal/index.jsx b/projects/plugins/protect/src/js/components/fix-all-threats-modal/index.jsx index b9fc6c2f10cc0..68ef496a610ff 100644 --- a/projects/plugins/protect/src/js/components/fix-all-threats-modal/index.jsx +++ b/projects/plugins/protect/src/js/components/fix-all-threats-modal/index.jsx @@ -14,21 +14,23 @@ const FixAllThreatsModal = ( { threatList = [] } ) => { const [ threatIds, setThreatIds ] = useState( threatList.map( ( { id } ) => parseInt( id ) ) ); - const handleCancelClick = useCallback( () => { - return event => { + const handleCancelClick = useCallback( + event => { event.preventDefault(); setModal( { type: null } ); - }; - }, [ setModal ] ); + }, + [ setModal ] + ); - const handleFixClick = useCallback( () => { - return async event => { + const handleFixClick = useCallback( + async event => { event.preventDefault(); await fixThreats( threatIds ); setModal( { type: null } ); - }; - }, [ fixThreats, setModal, threatIds ] ); + }, + [ fixThreats, setModal, threatIds ] + ); const handleCheckboxClick = useCallback( ( checked, threat ) => { @@ -63,12 +65,12 @@ const FixAllThreatsModal = ( { threatList = [] } ) => {
-
- { ! hideAutoFixColumn && ( + { ! hideAutoFixColumn && fixable && (
- { fixable && ( - <> - { fixInProgressThreatIds.includes( id ) ? ( - - ) : ( - - ) } - { isSmall && { __( 'Auto-fix', 'jetpack-protect' ) } } - - ) } + { renderFixerStatus( isThreatFixInProgress( id ), isThreatFixStale( id ) ) } + { isSmall && { __( 'Auto-fix', 'jetpack-protect' ) } }
) }
- +
-
+
{ children }
diff --git a/projects/plugins/protect/src/js/components/paid-accordion/styles.module.scss b/projects/plugins/protect/src/js/components/paid-accordion/styles.module.scss index ec6049085fe23..3e88a5f9f7ccb 100644 --- a/projects/plugins/protect/src/js/components/paid-accordion/styles.module.scss +++ b/projects/plugins/protect/src/js/components/paid-accordion/styles.module.scss @@ -103,6 +103,10 @@ fill: var( --jp-green-40 ); } +.icon-info { + fill: var( --jp-red ); +} + .status-badge { border-radius: 32px; flex-shrink: 0; @@ -131,6 +135,16 @@ color: #008a20; } +.support-link { + color: inherit; + + &:focus, + &:hover { + color: inherit; + box-shadow: none; + } +} + @media ( max-width: 599px ) { .accordion-header { display: grid; diff --git a/projects/plugins/protect/src/js/components/threats-list/index.jsx b/projects/plugins/protect/src/js/components/threats-list/index.jsx index e8e9d3aee4c6d..4b190d53ea6c1 100644 --- a/projects/plugins/protect/src/js/components/threats-list/index.jsx +++ b/projects/plugins/protect/src/js/components/threats-list/index.jsx @@ -7,7 +7,8 @@ import { Text, } from '@automattic/jetpack-components'; import { __, sprintf } from '@wordpress/i18n'; -import React, { useCallback, useState } from 'react'; +import React, { useCallback, useMemo, useState } from 'react'; +import useFixers from '../../hooks/use-fixers'; import useModal from '../../hooks/use-modal'; import usePlan from '../../hooks/use-plan'; import OnboardingPopover from '../onboarding-popover'; @@ -22,8 +23,18 @@ import useThreatsList from './use-threats-list'; const ThreatsList = () => { const { hasPlan } = usePlan(); const { item, list, selected, setSelected } = useThreatsList(); - const fixableList = list.filter( obj => obj.fixable ); const [ isSm ] = useBreakpointMatch( 'sm' ); + const { isThreatFixInProgress, isThreatFixStale } = useFixers(); + + // List of fixable threats that do not have a fix in progress + const fixableList = useMemo( () => { + return list.filter( threat => { + const threatId = parseInt( threat.id ); + return ( + threat.fixable && ! isThreatFixInProgress( threatId ) && ! isThreatFixStale( threatId ) + ); + } ); + }, [ list, isThreatFixInProgress, isThreatFixStale ] ); // Popover anchors const [ yourScanResultsPopoverAnchor, setYourScanResultsPopoverAnchor ] = useState( null ); @@ -31,11 +42,11 @@ const ThreatsList = () => { const { setModal } = useModal(); - const [ fixAllThreatsPopoverAnchor, setFixAllThreatsPopoverAnchor ] = useState( null ); + const [ showAutoFixersPopoverAnchor, setShowAutoFixersPopoverAnchor ] = useState( null ); const [ dailyAndManualScansPopoverAnchor, setDailyAndManualScansPopoverAnchor ] = useState( null ); - const handleFixAllThreatsClick = threatList => { + const handleShowAutoFixersClick = threatList => { return event => { event.preventDefault(); setModal( { @@ -111,9 +122,9 @@ const ThreatsList = () => { { fixableList.length > 0 && ( <> { fixable && ( - ) } diff --git a/projects/plugins/protect/src/js/data/scan/use-fixers-query.ts b/projects/plugins/protect/src/js/data/scan/use-fixers-query.ts index 2f5e52d748294..f860359de8e29 100644 --- a/projects/plugins/protect/src/js/data/scan/use-fixers-query.ts +++ b/projects/plugins/protect/src/js/data/scan/use-fixers-query.ts @@ -1,12 +1,18 @@ import { useConnection } from '@automattic/jetpack-connection'; import { useQuery, useQueryClient, type UseQueryResult } from '@tanstack/react-query'; -import { __ } from '@wordpress/i18n'; -import { useEffect, useMemo } from 'react'; +import { __, _n, sprintf } from '@wordpress/i18n'; +import { useCallback, useEffect } from 'react'; import API from '../../api'; import { QUERY_FIXERS_KEY, QUERY_HISTORY_KEY, QUERY_SCAN_STATUS_KEY } from '../../constants'; +import { fixerTimestampIsStale } from '../../hooks/use-fixers'; import useNotices from '../../hooks/use-notices'; import { FixersStatus } from '../../types/fixers'; +const initialData: FixersStatus = window.jetpackProtectInitialState?.fixerStatus || { + ok: true, + threats: {}, +}; + /** * Fixers Query Hook * @@ -32,16 +38,33 @@ export default function useFixersQuery( { skipUserConnection: true, } ); - // Memoize initialData to prevent recalculating on every render - const initialData: FixersStatus = useMemo( - () => - window.jetpackProtectInitialState?.fixerStatus || { - ok: true, - threats: {}, - }, - [] + // Helper to show success or failure notices + const showBulkNotices = useCallback( + ( failures: string[], successes: string[] ) => { + if ( failures.length > 0 ) { + // Translators: %d is the number of threats, and %s is a list of threat IDs. + const failureMessage = _n( + 'A threat could not be fixed.', + '%d threats could not be fixed.', + failures.length, + 'jetpack-protect' + ); + showErrorNotice( sprintf( failureMessage, failures.length ) ); + } else if ( successes.length > 0 ) { + // Translators: %d is the number of threats, and %s is a list of threat IDs. + const successMessage = _n( + 'Threat fixed successfully.', + '%d threats fixed successfully.', + successes.length, + 'jetpack-protect' + ); + showSuccessNotice( sprintf( successMessage, successes.length ) ); + } + }, + [ showErrorNotice, showSuccessNotice ] ); + // Main query function to fetch fixer status const fixersQuery = useQuery( { queryKey: [ QUERY_FIXERS_KEY ], queryFn: async () => { @@ -51,30 +74,40 @@ export default function useFixersQuery( { | FixersStatus | undefined; - // Check if any fixers have completed, by comparing the latest data against the cache. - Object.keys( data?.threats ).forEach( ( threatId: string ) => { - // Find the specific threat in the cached data. - const threat = data?.threats[ threatId ]; + const successes: string[] = []; + const failures: string[] = []; + + Object.keys( data?.threats || {} ).forEach( threatId => { + const threat = data.threats[ threatId ]; const cachedThreat = cachedData?.threats?.[ threatId ]; - if ( - cachedThreat && - cachedThreat.status === 'in_progress' && - threat.status !== 'in_progress' - ) { - // Invalidate related queries when a fixer has completed. - queryClient.invalidateQueries( { queryKey: [ QUERY_SCAN_STATUS_KEY ] } ); - queryClient.invalidateQueries( { queryKey: [ QUERY_HISTORY_KEY ] } ); - - // Show a relevant notice. - if ( threat.status === 'fixed' ) { - showSuccessNotice( __( 'Threat fixed successfully.', 'jetpack-protect' ) ); - } else { - showErrorNotice( __( 'Threat could not be fixed.', 'jetpack-protect' ) ); + if ( cachedThreat?.status === 'in_progress' ) { + // If still in progress + if ( threat.status === 'in_progress' ) { + if ( + ! fixerTimestampIsStale( cachedThreat.last_updated ) && + fixerTimestampIsStale( threat.last_updated ) + ) { + failures.push( threatId ); + } + } + + // Handle completion of fixers + if ( threat.status !== 'in_progress' ) { + queryClient.invalidateQueries( { queryKey: [ QUERY_SCAN_STATUS_KEY ] } ); + queryClient.invalidateQueries( { queryKey: [ QUERY_HISTORY_KEY ] } ); + + if ( threat.status === 'fixed' ) { + successes.push( threatId ); + } else { + failures.push( threatId ); + } } } } ); + showBulkNotices( failures, successes ); + // Return the fetched data so the query resolves return data; }, @@ -84,14 +117,15 @@ export default function useFixersQuery( { return false; } - // Refetch while any threats are still in progress. - if ( - Object.values( query.state.data?.threats ).some( - ( threat: { status: string } ) => threat.status === 'in_progress' - ) - ) { + const inProgressNotStale = Object.values( query.state.data.threats ).some( + ( threat: { status: string; last_updated: string } ) => + threat.status === 'in_progress' && ! fixerTimestampIsStale( threat.last_updated ) + ); + + // Refetch while any threats are still in progress and not stale. + if ( inProgressNotStale ) { // Refetch on a shorter interval first, then slow down if it is taking a while. - return query.state.dataUpdateCount < 5 ? 5_000 : 15_000; + return query.state.dataUpdateCount < 5 ? 5000 : 15000; } return false; @@ -103,13 +137,11 @@ export default function useFixersQuery( { // Handle error if present in the query result useEffect( () => { if ( fixersQuery.isError && fixersQuery.error ) { - // Reset the query data to initial state + // Reset the query data to the initial state queryClient.setQueryData( [ QUERY_FIXERS_KEY ], initialData ); - - // Show an error notice showErrorNotice( __( 'An error occurred while fetching fixers status.', 'jetpack-protect' ) ); } - }, [ fixersQuery.isError, fixersQuery.error, queryClient, initialData, showErrorNotice ] ); + }, [ fixersQuery.isError, fixersQuery.error, queryClient, showErrorNotice ] ); return fixersQuery; } diff --git a/projects/plugins/protect/src/js/hooks/use-fixers.ts b/projects/plugins/protect/src/js/hooks/use-fixers.ts index ba4f6ca0ea2dd..2d31b65bb35cf 100644 --- a/projects/plugins/protect/src/js/hooks/use-fixers.ts +++ b/projects/plugins/protect/src/js/hooks/use-fixers.ts @@ -1,15 +1,28 @@ -import { useMemo } from 'react'; +import { useCallback } from 'react'; import useFixersMutation from '../data/scan/use-fixers-mutation'; import useFixersQuery from '../data/scan/use-fixers-query'; import useScanStatusQuery from '../data/scan/use-scan-status-query'; -import { FixersStatus } from '../types/fixers'; +import { FixersStatus, ThreatFixStatus } from '../types/fixers'; + +const FIXER_IS_STALE_THRESHOLD = 1000 * 60 * 60 * 24; // 24 hours + +export const fixerTimestampIsStale = ( lastUpdatedTimestamp: string ) => { + const now = new Date(); + const lastUpdated = new Date( lastUpdatedTimestamp ); + return now.getTime() - lastUpdated.getTime() >= FIXER_IS_STALE_THRESHOLD; +}; + +export const fixerStatusIsStale = ( fixerStatus: ThreatFixStatus ) => { + return fixerStatus.status === 'in_progress' && fixerTimestampIsStale( fixerStatus.last_updated ); +}; type UseFixersResult = { fixableThreatIds: number[]; - fixInProgressThreatIds: number[]; fixersStatus: FixersStatus; fixThreats: ( threatIds: number[] ) => Promise< unknown >; isLoading: boolean; + isThreatFixInProgress: ( threatId: number ) => boolean; + isThreatFixStale: ( threatId: number ) => boolean; }; /** @@ -25,22 +38,27 @@ export default function useFixers(): UseFixersResult { usePolling: true, } ); - // List of threat IDs that are currently being fixed. - const fixInProgressThreatIds = useMemo( - () => - Object.entries( fixersStatus?.threats || {} ) - .filter( - ( [ , threat ]: [ string, { status?: string } ] ) => threat.status === 'in_progress' - ) - .map( ( [ id ] ) => parseInt( id ) ), + const isThreatFixInProgress = useCallback( + ( threatId: number ) => { + return fixersStatus?.threats?.[ threatId ]?.status === 'in_progress'; + }, + [ fixersStatus ] + ); + + const isThreatFixStale = useCallback( + ( threatId: number ) => { + const threatFixStatus = fixersStatus?.threats?.[ threatId ]; + return threatFixStatus ? fixerStatusIsStale( threatFixStatus ) : false; + }, [ fixersStatus ] ); return { fixableThreatIds: status.fixableThreatIds, - fixInProgressThreatIds, fixersStatus, fixThreats: fixersMutation.mutateAsync, isLoading: fixersMutation.isPending, + isThreatFixInProgress, + isThreatFixStale, }; } diff --git a/projects/plugins/protect/src/js/types/fixers.ts b/projects/plugins/protect/src/js/types/fixers.ts index fc43d944a5830..c732af6311b63 100644 --- a/projects/plugins/protect/src/js/types/fixers.ts +++ b/projects/plugins/protect/src/js/types/fixers.ts @@ -1,9 +1,13 @@ export type FixerStatus = 'not_started' | 'in_progress' | 'fixed' | 'not_fixed'; export type FixersStatus = { + ok: boolean; threats: { - [ key: number ]: { - status: FixerStatus; - }; + [ key: number ]: ThreatFixStatus; }; }; + +export type ThreatFixStatus = { + status: FixerStatus; + last_updated: string; +};