From 15538a874904b55126cd5988337c2dc852ba42a6 Mon Sep 17 00:00:00 2001 From: Nate Weller Date: Tue, 6 Aug 2024 14:52:43 -0600 Subject: [PATCH] Do not show scan error when optimistically scanning (#38703) --- .../interstitial-page/stories/broken/mock.js | 4 +- .../src/js/components/pricing-table/index.jsx | 29 ++++--- projects/plugins/protect/src/js/constants.js | 14 ++++ .../protect/src/js/routes/scan/index.jsx | 45 +++++------ .../src/js/routes/scan/use-status-polling.js | 11 ++- .../plugins/protect/src/js/state/actions.js | 5 +- .../plugins/protect/src/js/state/reducers.js | 3 +- .../plugins/protect/src/js/state/selectors.js | 75 +++++++++++++++++++ 8 files changed, 144 insertions(+), 42 deletions(-) diff --git a/projects/plugins/protect/src/js/components/interstitial-page/stories/broken/mock.js b/projects/plugins/protect/src/js/components/interstitial-page/stories/broken/mock.js index 8c96f115927f0..f21cac9a0fb83 100644 --- a/projects/plugins/protect/src/js/components/interstitial-page/stories/broken/mock.js +++ b/projects/plugins/protect/src/js/components/interstitial-page/stories/broken/mock.js @@ -1,3 +1,5 @@ +import { SCAN_STATUS_SCHEDULED } from '../../../../constants'; + export const jetpackProtectInitialState = { apiRoot: 'http://localhost/wp-json/', apiNonce: 'f2d2d42e2a', @@ -7,7 +9,7 @@ export const jetpackProtectInitialState = { num_threats: 6, num_plugins_threats: 3, num_themes_threats: 3, - status: 'scheduled', + status: SCAN_STATUS_SCHEDULED, wordpress: { version: '5.9.3', threats: [], diff --git a/projects/plugins/protect/src/js/components/pricing-table/index.jsx b/projects/plugins/protect/src/js/components/pricing-table/index.jsx index d305703f90735..5e245f31d3840 100644 --- a/projects/plugins/protect/src/js/components/pricing-table/index.jsx +++ b/projects/plugins/protect/src/js/components/pricing-table/index.jsx @@ -30,7 +30,7 @@ const ConnectedPricingTable = ( { onScanAdd } ) => { skipUserConnection: true, } ); - const { refreshPlan, refreshStatus } = useDispatch( STORE_ID ); + const { refreshPlan, refreshStatus, startScanOptimistically } = useDispatch( STORE_ID ); const [ getProtectFreeButtonIsLoading, setGetProtectFreeButtonIsLoading ] = useState( false ); const [ getScanButtonIsLoading, setGetScanButtonIsLoading ] = useState( false ); @@ -54,17 +54,26 @@ const ConnectedPricingTable = ( { onScanAdd } ) => { onScanAdd(); } ); - const getProtectFree = useCallback( () => { + const getProtectFree = useCallback( async () => { recordEvent( 'jetpack_protect_connected_product_activated' ); setGetProtectFreeButtonIsLoading( true ); - handleRegisterSite() - .then( () => setGetProtectFreeButtonIsLoading( false ) ) - .then( () => { - refreshPlan(); - refreshWaf(); - refreshStatus( true ); - } ); - }, [ handleRegisterSite, recordEvent, refreshWaf, refreshPlan, refreshStatus ] ); + try { + await handleRegisterSite(); + startScanOptimistically(); + await refreshPlan(); + await refreshWaf(); + await refreshStatus( true ); + } finally { + setGetProtectFreeButtonIsLoading( false ); + } + }, [ + handleRegisterSite, + recordEvent, + refreshWaf, + refreshPlan, + refreshStatus, + startScanOptimistically, + ] ); const args = { title: __( 'Stay one step ahead of threats', 'jetpack-protect' ), diff --git a/projects/plugins/protect/src/js/constants.js b/projects/plugins/protect/src/js/constants.js index 5b5540d3b68b6..a82cc01aa3a4e 100644 --- a/projects/plugins/protect/src/js/constants.js +++ b/projects/plugins/protect/src/js/constants.js @@ -3,3 +3,17 @@ export const FREE_PLUGIN_SUPPORT_URL = 'https://wordpress.org/support/plugin/jet export const PAID_PLUGIN_SUPPORT_URL = 'https://jetpack.com/contact-support/?rel=support'; export const JETPACK_SCAN_SLUG = 'jetpack_scan'; + +/** + * Scan Status Constants + */ +export const SCAN_STATUS_SCHEDULED = 'scheduled'; +export const SCAN_STATUS_SCANNING = 'scanning'; +export const SCAN_STATUS_OPTIMISTICALLY_SCANNING = 'optimistically_scanning'; +export const SCAN_STATUS_IDLE = 'idle'; +export const SCAN_STATUS_UNAVAILABLE = 'unavailable'; +export const SCAN_IN_PROGRESS_STATUSES = [ + SCAN_STATUS_SCHEDULED, + SCAN_STATUS_SCANNING, + SCAN_STATUS_OPTIMISTICALLY_SCANNING, +]; diff --git a/projects/plugins/protect/src/js/routes/scan/index.jsx b/projects/plugins/protect/src/js/routes/scan/index.jsx index 3c646a8b3d8e5..1de1cb5448eb4 100644 --- a/projects/plugins/protect/src/js/routes/scan/index.jsx +++ b/projects/plugins/protect/src/js/routes/scan/index.jsx @@ -12,6 +12,7 @@ import ScanFooter from '../../components/scan-footer'; import SeventyFiveLayout from '../../components/seventy-five-layout'; import Summary from '../../components/summary'; import ThreatsList from '../../components/threats-list'; +import { SCAN_STATUS_UNAVAILABLE } from '../../constants'; import useAnalyticsTracks from '../../hooks/use-analytics-tracks'; import { OnboardingContext } from '../../hooks/use-onboarding'; import useProtectData from '../../hooks/use-protect-data'; @@ -72,6 +73,7 @@ const ErrorSection = ( { errorMessage, errorCode } ) => { }; const ScanningSection = ( { currentProgress } ) => { + const { hasRequiredPlan } = useProtectData(); const { stats } = useWafData(); const { globalStats } = stats; const totalVulnerabilities = parseInt( globalStats?.totalVulnerabilities ); @@ -105,7 +107,7 @@ const ScanningSection = ( { currentProgress } ) => {

{ __( 'Your results will be ready soon', 'jetpack-protect' ) }

- { currentProgress !== null && currentProgress >= 0 && ( + { hasRequiredPlan && currentProgress !== null && currentProgress >= 0 && ( ) } @@ -152,16 +154,20 @@ const DefaultSection = () => { }; const ScanPage = () => { - const { lastChecked, error, errorCode, errorMessage, hasRequiredPlan } = useProtectData(); + const { lastChecked, hasRequiredPlan } = useProtectData(); const { refreshStatus } = useDispatch( STORE_ID ); - const { statusIsFetching, scanIsUnavailable, status } = useSelect( select => ( { - statusIsFetching: select( STORE_ID ).getStatusIsFetching(), - scanIsUnavailable: select( STORE_ID ).getScanIsUnavailable(), - status: select( STORE_ID ).getStatus(), - } ) ); + const { scanInProgress, statusIsFetching, scanIsUnavailable, status, scanError } = useSelect( + select => ( { + scanError: select( STORE_ID ).scanError(), + scanInProgress: select( STORE_ID ).scanInProgress(), + scanIsUnavailable: select( STORE_ID ).getScanIsUnavailable(), + status: select( STORE_ID ).getStatus(), + statusIsFetching: select( STORE_ID ).getStatusIsFetching(), + } ) + ); let currentScanStatus; - if ( status.error || scanIsUnavailable ) { + if ( scanError ) { currentScanStatus = 'error'; } else if ( ! lastChecked ) { currentScanStatus = 'in_progress'; @@ -183,33 +189,22 @@ const ScanPage = () => { // retry fetching status if it is not available useEffect( () => { - if ( ! statusIsFetching && 'unavailable' === status.status && ! scanIsUnavailable ) { + if ( ! statusIsFetching && SCAN_STATUS_UNAVAILABLE === status.status && ! scanIsUnavailable ) { refreshStatus( true ); } }, [ statusIsFetching, status.status, refreshStatus, scanIsUnavailable ] ); const renderSection = useMemo( () => { - // Error - if ( error || scanIsUnavailable ) { - return ; + if ( scanInProgress ) { + return ; } - // Scanning - const scanningStatuses = new Set( [ 'scheduled', 'scanning', 'optimistically_scanning' ] ); - if ( scanningStatuses.has( status.status ) || ! lastChecked ) { - return ; + if ( scanError ) { + return ; } return ; - }, [ - error, - errorMessage, - errorCode, - scanIsUnavailable, - status.status, - status.currentProgress, - lastChecked, - ] ); + }, [ scanInProgress, status.currentProgress, scanError ] ); return ( diff --git a/projects/plugins/protect/src/js/routes/scan/use-status-polling.js b/projects/plugins/protect/src/js/routes/scan/use-status-polling.js index 71c7f3db7eb5b..ad662bc0ca4b4 100644 --- a/projects/plugins/protect/src/js/routes/scan/use-status-polling.js +++ b/projects/plugins/protect/src/js/routes/scan/use-status-polling.js @@ -2,6 +2,11 @@ import apiFetch from '@wordpress/api-fetch'; import { useDispatch, useSelect } from '@wordpress/data'; import camelize from 'camelize'; import { useEffect } from 'react'; +import { + SCAN_IN_PROGRESS_STATUSES, + SCAN_STATUS_IDLE, + SCAN_STATUS_UNAVAILABLE, +} from '../../constants'; import useAnalyticsTracks from '../../hooks/use-analytics-tracks'; import { STORE_ID } from '../../state/store'; @@ -20,11 +25,11 @@ const useStatusPolling = () => { const pollDuration = 10000; const statusIsInProgress = currentStatus => - [ 'scheduled', 'scanning' ].indexOf( currentStatus ) >= 0; + SCAN_IN_PROGRESS_STATUSES.indexOf( currentStatus ) >= 0; // if there has never been a scan, and the scan status is idle, then we must still be getting set up const scanIsInitializing = ( currentStatus, lastChecked ) => - ! lastChecked && currentStatus === 'idle'; + ! lastChecked && SCAN_STATUS_IDLE === currentStatus; const pollStatus = () => { return new Promise( ( resolve, reject ) => { @@ -74,7 +79,7 @@ const useStatusPolling = () => { setStatusIsFetching( true ); pollStatus() .then( newStatus => { - setScanIsUnavailable( 'unavailable' === newStatus.status ); + setScanIsUnavailable( SCAN_STATUS_UNAVAILABLE === newStatus.status ); setStatus( camelize( newStatus ) ); recordEvent( 'jetpack_protect_scan_completed', { scan_status: newStatus.status, diff --git a/projects/plugins/protect/src/js/state/actions.js b/projects/plugins/protect/src/js/state/actions.js index 67d0893d6046f..ca9fefa87fdd3 100644 --- a/projects/plugins/protect/src/js/state/actions.js +++ b/projects/plugins/protect/src/js/state/actions.js @@ -2,6 +2,7 @@ import apiFetch from '@wordpress/api-fetch'; import { sprintf, _n, __ } from '@wordpress/i18n'; import camelize from 'camelize'; import API from '../api'; +import { SCAN_STATUS_UNAVAILABLE } from '../constants'; const SET_CREDENTIALS_STATE_IS_FETCHING = 'SET_CREDENTIALS_STATE_IS_FETCHING'; const SET_CREDENTIALS_STATE = 'SET_CREDENTIALS_STATE'; @@ -84,7 +85,7 @@ const refreshStatus = return fetchStatus( hardRefresh ) .then( checkStatus ) .then( status => { - dispatch( setScanIsUnavailable( 'unavailable' === status.status ) ); + dispatch( setScanIsUnavailable( SCAN_STATUS_UNAVAILABLE === status.status ) ); dispatch( setStatus( camelize( status ) ) ); resolve( status ); } ) @@ -120,7 +121,7 @@ const refreshScanHistory = () => { */ const checkStatus = ( currentStatus, attempts = 0 ) => { return new Promise( ( resolve, reject ) => { - if ( 'unavailable' === currentStatus.status && attempts < 3 ) { + if ( SCAN_STATUS_UNAVAILABLE === currentStatus.status && attempts < 3 ) { fetchStatus( true ) .then( newStatus => { setTimeout( () => { diff --git a/projects/plugins/protect/src/js/state/reducers.js b/projects/plugins/protect/src/js/state/reducers.js index 8d4e8bb39a6d4..ebf4208200ea6 100644 --- a/projects/plugins/protect/src/js/state/reducers.js +++ b/projects/plugins/protect/src/js/state/reducers.js @@ -1,5 +1,6 @@ import { combineReducers } from '@wordpress/data'; import camelize from 'camelize'; +import { SCAN_STATUS_OPTIMISTICALLY_SCANNING } from '../constants'; import { SET_CREDENTIALS_STATE, SET_CREDENTIALS_STATE_IS_FETCHING, @@ -61,7 +62,7 @@ const status = ( state = {}, action ) => { case SET_STATUS_PROGRESS: return { ...state, currentProgress: action.currentProgress }; case START_SCAN_OPTIMISTICALLY: - return { ...state, currentProgress: 0, status: 'optimistically_scanning' }; + return { ...state, currentProgress: 0, status: SCAN_STATUS_OPTIMISTICALLY_SCANNING }; } return state; }; diff --git a/projects/plugins/protect/src/js/state/selectors.js b/projects/plugins/protect/src/js/state/selectors.js index 9a8e818f4e82c..a438eb48cf69d 100644 --- a/projects/plugins/protect/src/js/state/selectors.js +++ b/projects/plugins/protect/src/js/state/selectors.js @@ -1,3 +1,76 @@ +import { __ } from '@wordpress/i18n'; +import { SCAN_IN_PROGRESS_STATUSES, SCAN_STATUS_OPTIMISTICALLY_SCANNING } from '../constants'; + +/** + * Scan in progress selector. + * + * @param {object} state - The current state. + * @returns {boolean} Whether a scan is in progress. + */ +const scanInProgress = state => { + const { status, error, lastChecked } = selectors.getStatus( state ); + const unavailable = selectors.getScanIsUnavailable( state ); + + // When "optimistically" scanning, ignore any other status or error. + if ( SCAN_STATUS_OPTIMISTICALLY_SCANNING === status ) { + return true; + } + + // If there is an error or the scan is unavailable, scanning is not in progress. + if ( error || unavailable ) { + return false; + } + + // If the status is one of the scanning statuses, or if we have never checked, we are scanning. + if ( SCAN_IN_PROGRESS_STATUSES.includes( status.status ) || ! lastChecked ) { + return true; + } + + return false; +}; + +/** + * Scan error selector. + * + * @param {object} state - The current state. + * + * @typedef {object} ScanError + * @property {string} code - The code identifying the type of error. + * @property {string} message - A message describing the error. + * + * @returns {ScanError|null} The error object or null. + */ +const scanError = state => { + const { status, error, errorCode, errorMessage } = selectors.getStatus( state ); + const unavailable = selectors.getScanIsUnavailable( state ); + const isFetching = selectors.getStatusIsFetching( state ); + + // When "optimistically" scanning, ignore any errors. + if ( SCAN_STATUS_OPTIMISTICALLY_SCANNING === status ) { + return null; + } + + // While still fetching the status, ignore any errors. + if ( isFetching ) { + return null; + } + + // If the scan results include an error, return it. + if ( error ) { + return { code: errorCode, message: errorMessage }; + } + + // If the scan is unavailable, return an error. + if ( unavailable ) { + return { + code: 'scan_unavailable', + message: __( 'We are having problems scanning your site.', 'jetpack-protect' ), + }; + } + + return null; +}; + const selectors = { getCredentials: state => state.credentials || null, getCredentialsIsFetching: state => state.credentialsIsFetching || false, @@ -8,6 +81,8 @@ const selectors = { getStatusIsFetching: state => state.statusIsFetching || false, getScanIsUnavailable: state => state.scanIsUnavailable || false, getScanIsEnqueuing: state => state.scanIsEnqueuing || false, + scanInProgress, + scanError, getWpVersion: state => state.wpVersion || '', getJetpackScan: state => state.jetpackScan || {}, getThreatsUpdating: state => state.threatsUpdating || {},