From b8c6ff88b84cee9116025801546c179eb651fc33 Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Tue, 14 Dec 2021 16:13:48 +0100 Subject: [PATCH 1/2] Ensure site is not scanned multiple times in Onboarding Wizard --- .../site-scan-context-provider/index.js | 27 ++++++++++++++++--- assets/src/onboarding-wizard/index.js | 1 + 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/assets/src/components/site-scan-context-provider/index.js b/assets/src/components/site-scan-context-provider/index.js index b05c133c4be..cfe365517a8 100644 --- a/assets/src/components/site-scan-context-provider/index.js +++ b/assets/src/components/site-scan-context-provider/index.js @@ -64,7 +64,9 @@ const INITIAL_STATE = { currentlyScannedUrlIndexes: [], forceStandardMode: false, scannableUrls: [], + scanOnce: false, status: '', + scansCount: 0, urlIndexesPendingScan: [], }; @@ -89,6 +91,7 @@ const CONCURRENT_VALIDATION_REQUESTS_WAIT_MS = 500; * @param {Object} action Action to call. * @return {Object} New state. */ +//eslint-disable-next-line complexity export function siteScanReducer( state, action ) { // Bail out early if Site Scan is skipped, i.e. if there is no validation nonce provided meaning the current user // does not have capabilities for running AMP validation. @@ -113,9 +116,16 @@ export function siteScanReducer( state, action ) { }; } case ACTION_SCANNABLE_URLS_RECEIVE: { + if ( ! action.scannableUrls || action.scannableUrls.length === 0 ) { + return { + ...state, + status: STATUS_COMPLETED, + }; + } + return { ...state, - status: action.scannableUrls?.length > 0 ? STATUS_READY : STATUS_COMPLETED, + status: state.scanOnce && state.scansCount > 0 ? STATUS_COMPLETED : STATUS_READY, scannableUrls: action.scannableUrls, }; } @@ -124,10 +134,18 @@ export function siteScanReducer( state, action ) { return state; } + if ( state.scanOnce && state.scansCount > 0 ) { + return { + ...state, + status: STATUS_COMPLETED, + }; + } + return { ...state, status: STATUS_IDLE, currentlyScannedUrlIndexes: [], + scansCount: state.scansCount + 1, urlIndexesPendingScan: state.scannableUrls.map( ( url, index ) => index ), }; } @@ -201,8 +219,9 @@ export function siteScanReducer( state, action ) { * @param {?any} props.children Component children. * @param {boolean} props.fetchCachedValidationErrors Whether to fetch cached validation errors on mount. * @param {boolean} props.refetchPluginSuppressionOnScanComplete Whether to refetch plugin suppression data when site scan is complete. - * @param {boolean} props.resetOnOptionsChange Whether to reset scanner and refetch scannable URLs whenever AMPoptions are changed. + * @param {boolean} props.resetOnOptionsChange Whether to reset scanner and refetch scannable URLs whenever AMP options are changed. * @param {string} props.scannableUrlsRestPath The REST path for interacting with the scannable URL resources. + * @param {boolean} props.scanOnce Whether to scan only once. * @param {string} props.validateNonce The AMP validate nonce. */ export function SiteScanContextProvider( { @@ -211,6 +230,7 @@ export function SiteScanContextProvider( { refetchPluginSuppressionOnScanComplete = false, resetOnOptionsChange = false, scannableUrlsRestPath, + scanOnce = false, validateNonce, } ) { const { @@ -221,7 +241,7 @@ export function SiteScanContextProvider( { refetchPluginSuppression, } = useContext( Options ); const { setAsyncError } = useAsyncError(); - const [ state, dispatch ] = useReducer( siteScanReducer, INITIAL_STATE ); + const [ state, dispatch ] = useReducer( siteScanReducer, { ...INITIAL_STATE, scanOnce } ); const { currentlyScannedUrlIndexes, forceStandardMode, @@ -523,5 +543,6 @@ SiteScanContextProvider.propTypes = { refetchPluginSuppressionOnScanComplete: PropTypes.bool, resetOnOptionsChange: PropTypes.bool, scannableUrlsRestPath: PropTypes.string, + scanOnce: PropTypes.bool, validateNonce: PropTypes.string, }; diff --git a/assets/src/onboarding-wizard/index.js b/assets/src/onboarding-wizard/index.js index 437ca21ea0d..b02f7339867 100644 --- a/assets/src/onboarding-wizard/index.js +++ b/assets/src/onboarding-wizard/index.js @@ -82,6 +82,7 @@ export function Providers( { children } ) { fetchCachedValidationErrors={ false } resetOnOptionsChange={ true } scannableUrlsRestPath={ SCANNABLE_URLS_REST_PATH } + scanOnce={ true } validateNonce={ VALIDATE_NONCE } > From 8d49e62c6b6141353f5e92a4de492c8b49abf92a Mon Sep 17 00:00:00 2001 From: Piotr Delawski Date: Tue, 14 Dec 2021 16:35:00 +0100 Subject: [PATCH 2/2] Update unit tests --- .../site-scan-context-provider/index.js | 2 +- .../test/site-scan-reducer.js | 46 +++++++++++++++++-- 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/assets/src/components/site-scan-context-provider/index.js b/assets/src/components/site-scan-context-provider/index.js index cfe365517a8..7a9480315a9 100644 --- a/assets/src/components/site-scan-context-provider/index.js +++ b/assets/src/components/site-scan-context-provider/index.js @@ -116,7 +116,7 @@ export function siteScanReducer( state, action ) { }; } case ACTION_SCANNABLE_URLS_RECEIVE: { - if ( ! action.scannableUrls || action.scannableUrls.length === 0 ) { + if ( ! action.scannableUrls || action.scannableUrls?.length === 0 ) { return { ...state, status: STATUS_COMPLETED, diff --git a/assets/src/components/site-scan-context-provider/test/site-scan-reducer.js b/assets/src/components/site-scan-context-provider/test/site-scan-reducer.js index a327637da85..42c632ebaa8 100644 --- a/assets/src/components/site-scan-context-provider/test/site-scan-reducer.js +++ b/assets/src/components/site-scan-context-provider/test/site-scan-reducer.js @@ -90,20 +90,33 @@ describe( 'siteScanReducer', () => { * ACTION_SCANNABLE_URLS_RECEIVE */ it( 'returns correct state for ACTION_SCANNABLE_URLS_RECEIVE', () => { - expect( siteScanReducer( {}, { + expect( siteScanReducer( { scanOnce: false, scansCount: 0 }, { type: ACTION_SCANNABLE_URLS_RECEIVE, scannableUrls: [], } ) ).toStrictEqual( { status: STATUS_COMPLETED, - scannableUrls: [], + scanOnce: false, + scansCount: 0, } ); - expect( siteScanReducer( {}, { + expect( siteScanReducer( { scanOnce: false, scansCount: 2 }, { type: ACTION_SCANNABLE_URLS_RECEIVE, scannableUrls: [ 'foo', 'bar' ], } ) ).toStrictEqual( { status: STATUS_READY, scannableUrls: [ 'foo', 'bar' ], + scanOnce: false, + scansCount: 2, + } ); + + expect( siteScanReducer( { scanOnce: true, scansCount: 1 }, { + type: ACTION_SCANNABLE_URLS_RECEIVE, + scannableUrls: [ 'foo', 'bar' ], + } ) ).toStrictEqual( { + status: STATUS_COMPLETED, + scannableUrls: [ 'foo', 'bar' ], + scanOnce: true, + scansCount: 1, } ); } ); @@ -129,6 +142,8 @@ describe( 'siteScanReducer', () => { ] )( 'returns correct state for ACTION_SCAN_INITIALIZE when initial status is %s', ( status ) => { expect( siteScanReducer( { status, + scanOnce: false, + scansCount: 0, scannableUrls: [ 'foo', 'bar' ], urlIndexesPendingScan: [], }, { @@ -136,11 +151,36 @@ describe( 'siteScanReducer', () => { } ) ).toStrictEqual( { status: STATUS_IDLE, currentlyScannedUrlIndexes: [], + scanOnce: false, + scansCount: 1, scannableUrls: [ 'foo', 'bar' ], urlIndexesPendingScan: [ 0, 1 ], } ); } ); + it.each( [ + STATUS_CANCELLED, + STATUS_COMPLETED, + STATUS_FAILED, + STATUS_READY, + ] )( 'returns correct state for ACTION_SCAN_INITIALIZE when initial status is %s and scan should be done just once', ( status ) => { + expect( siteScanReducer( { + status, + scanOnce: true, + scansCount: 1, + scannableUrls: [ 'foo', 'bar' ], + urlIndexesPendingScan: [], + }, { + type: ACTION_SCAN_INITIALIZE, + } ) ).toStrictEqual( { + status: STATUS_COMPLETED, + scanOnce: true, + scansCount: 1, + scannableUrls: [ 'foo', 'bar' ], + urlIndexesPendingScan: [], + } ); + } ); + /** * ACTION_SCAN_URL */