Skip to content

Commit 85dea50

Browse files
westonruterwestonruter
authored
Merge pull request #6777 from ampproject/fix/scannable-urls
Fix listing of Scannable URLs and re-fetching upon reaching Done step of Onboarding Wizard
2 parents 27a2104 + 878bebb commit 85dea50

File tree

8 files changed

+148
-42
lines changed

8 files changed

+148
-42
lines changed

‎assets/src/components/site-scan-context-provider/index.js‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@ export function siteScanReducer( state, action ) {
108108
...state,
109109
status: STATUS_REQUEST_SCANNABLE_URLS,
110110
forceStandardMode: action?.forceStandardMode ?? false,
111+
currentlyScannedUrlIndexes: [],
112+
urlIndexesPendingScan: [],
111113
};
112114
}
113115
case ACTION_SCANNABLE_URLS_RECEIVE: {
@@ -195,16 +197,18 @@ export function siteScanReducer( state, action ) {
195197
/**
196198
* Context provider for site scanning.
197199
*
198-
* @param {Object} props Component props.
199-
* @param {?any} props.children Component children.
200-
* @param {boolean} props.fetchCachedValidationErrors Whether to fetch cached validation errors on mount.
201-
* @param {boolean} props.resetOnOptionsChange Whether to reset scanner and refetch scannable URLs whenever AMP options are changed.
202-
* @param {string} props.scannableUrlsRestPath The REST path for interacting with the scannable URL resources.
203-
* @param {string} props.validateNonce The AMP validate nonce.
200+
* @param {Object} props Component props.
201+
* @param {?any} props.children Component children.
202+
* @param {boolean} props.fetchCachedValidationErrors Whether to fetch cached validation errors on mount.
203+
* @param {boolean} props.refetchPluginSuppressionOnScanComplete Whether to refetch plugin suppression data when site scan is complete.
204+
* @param {boolean} props.resetOnOptionsChange Whether to reset scanner and refetch scannable URLs whenever AMPoptions are changed.
205+
* @param {string} props.scannableUrlsRestPath The REST path for interacting with the scannable URL resources.
206+
* @param {string} props.validateNonce The AMP validate nonce.
204207
*/
205208
export function SiteScanContextProvider( {
206209
children,
207210
fetchCachedValidationErrors = false,
211+
refetchPluginSuppressionOnScanComplete = false,
208212
resetOnOptionsChange = false,
209213
scannableUrlsRestPath,
210214
validateNonce,
@@ -301,7 +305,6 @@ export function SiteScanContextProvider( {
301305
*/
302306
useEffect( () => {
303307
if ( resetOnOptionsChange && Object.keys( savedOptions ).length > 0 ) {
304-
dispatch( { type: ACTION_SCAN_CANCEL } );
305308
dispatch( { type: ACTION_SCANNABLE_URLS_REQUEST } );
306309
}
307310
}, [ resetOnOptionsChange, savedOptions ] );
@@ -316,6 +319,25 @@ export function SiteScanContextProvider( {
316319
}
317320
}, [ savedOptions?.suppressed_plugins, status ] );
318321

322+
/**
323+
* Once the site scan is complete, refetch the plugin suppression data so
324+
* that the suppressed table is updated with the latest validation errors.
325+
*/
326+
useEffect( () => {
327+
if ( status !== STATUS_REFETCHING_PLUGIN_SUPPRESSION ) {
328+
return;
329+
}
330+
331+
if ( refetchPluginSuppressionOnScanComplete ) {
332+
refetchPluginSuppression();
333+
}
334+
335+
dispatch( {
336+
type: ACTION_SET_STATUS,
337+
status: STATUS_COMPLETED,
338+
} );
339+
}, [ refetchPluginSuppression, refetchPluginSuppressionOnScanComplete, status ] );
340+
319341
/**
320342
* Delay concurrent validation requests.
321343
*/
@@ -344,20 +366,6 @@ export function SiteScanContextProvider( {
344366
};
345367
}, [ shouldDelayValidationRequest ] );
346368

347-
/**
348-
* Once the site scan is complete, refetch the plugin suppression data so
349-
* that the suppressed table is updated with the latest validation errors.
350-
*/
351-
useEffect( () => {
352-
if ( status === STATUS_REFETCHING_PLUGIN_SUPPRESSION ) {
353-
refetchPluginSuppression();
354-
dispatch( {
355-
type: ACTION_SET_STATUS,
356-
status: STATUS_COMPLETED,
357-
} );
358-
}
359-
}, [ refetchPluginSuppression, status ] );
360-
361369
/**
362370
* Fetch scannable URLs from the REST endpoint.
363371
*/
@@ -484,6 +492,7 @@ export function SiteScanContextProvider( {
484492
value={ {
485493
cancelSiteScan,
486494
fetchScannableUrls,
495+
forceStandardMode,
487496
hasSiteScanResults,
488497
isBusy: [ STATUS_IDLE, STATUS_IN_PROGRESS ].includes( status ),
489498
isCancelled: status === STATUS_CANCELLED,
@@ -511,6 +520,7 @@ export function SiteScanContextProvider( {
511520
SiteScanContextProvider.propTypes = {
512521
children: PropTypes.any,
513522
fetchCachedValidationErrors: PropTypes.bool,
523+
refetchPluginSuppressionOnScanComplete: PropTypes.bool,
514524
resetOnOptionsChange: PropTypes.bool,
515525
scannableUrlsRestPath: PropTypes.string,
516526
validateNonce: PropTypes.string,

‎assets/src/components/site-scan-context-provider/test/site-scan-reducer.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ describe( 'siteScanReducer', () => {
6969
} ) ).toStrictEqual( {
7070
status: STATUS_REQUEST_SCANNABLE_URLS,
7171
forceStandardMode: false,
72+
currentlyScannedUrlIndexes: [],
73+
urlIndexesPendingScan: [],
7274
} );
7375

7476
expect( siteScanReducer( {
@@ -79,6 +81,8 @@ describe( 'siteScanReducer', () => {
7981
} ) ).toStrictEqual( {
8082
status: STATUS_REQUEST_SCANNABLE_URLS,
8183
forceStandardMode: true,
84+
currentlyScannedUrlIndexes: [],
85+
urlIndexesPendingScan: [],
8286
} );
8387
} );
8488

‎assets/src/onboarding-wizard/index.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export function Providers( { children } ) {
8080
<ThemesContextProvider>
8181
<SiteScanContextProvider
8282
fetchCachedValidationErrors={ false }
83-
resetOnOptionsChange={ false }
83+
resetOnOptionsChange={ true }
8484
scannableUrlsRestPath={ SCANNABLE_URLS_REST_PATH }
8585
validateNonce={ VALIDATE_NONCE }
8686
>

‎assets/src/onboarding-wizard/pages/done/index.js‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export function Done() {
4848
const { didSaveDeveloperToolsOption, saveDeveloperToolsOption, savingDeveloperToolsOption } = useContext( User );
4949
const { canGoForward, setCanGoForward } = useContext( Navigation );
5050
const { downloadedTheme, downloadingTheme, downloadingThemeError } = useContext( ReaderThemes );
51-
const { isFetchingScannableUrls } = useContext( SiteScan );
51+
const { fetchScannableUrls, forceStandardMode, isFetchingScannableUrls } = useContext( SiteScan );
5252
const {
5353
hasPreview,
5454
isPreviewingAMP,
@@ -76,6 +76,15 @@ export function Done() {
7676
}
7777
}, [ didSaveOptions, saveOptions, savingOptions ] );
7878

79+
/**
80+
* Refetches the scannable URLs if they have been requested with Standard mode forced last time.
81+
*/
82+
useEffect( () => {
83+
if ( didSaveOptions && forceStandardMode ) {
84+
fetchScannableUrls();
85+
}
86+
}, [ didSaveOptions, fetchScannableUrls, forceStandardMode ] );
87+
7988
/**
8089
* Triggers saving of user options on arrival of this screen.
8190
*/

‎assets/src/onboarding-wizard/pages/done/use-preview.js‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
/**
22
* WordPress dependencies
33
*/
4-
import { useContext, useMemo, useState } from '@wordpress/element';
4+
import { useContext, useEffect, useMemo, useState } from '@wordpress/element';
55

66
/**
77
* Internal dependencies
@@ -14,13 +14,17 @@ export function usePreview() {
1414
const { scannableUrls } = useContext( SiteScan );
1515
const { editedOptions: { theme_support: themeSupport } } = useContext( Options );
1616

17-
const hasPreview = scannableUrls.length > 0;
1817
const [ isPreviewingAMP, setIsPreviewingAMP ] = useState( themeSupport !== STANDARD );
19-
const [ previewedPageType, setPreviewedPageType ] = useState( hasPreview ? scannableUrls[ 0 ].type : null );
18+
const [ previewedPageType, setPreviewedPageType ] = useState( scannableUrls.length > 0 ? scannableUrls[ 0 ].type : null );
2019

2120
const toggleIsPreviewingAMP = () => setIsPreviewingAMP( ( mode ) => ! mode );
2221
const setActivePreviewLink = ( link ) => setPreviewedPageType( link.type );
2322

23+
// Reset previewed page type whenever scannableUrls change.
24+
useEffect( () => {
25+
setPreviewedPageType( scannableUrls.length > 0 ? scannableUrls[ 0 ].type : null );
26+
}, [ scannableUrls ] );
27+
2428
const previewLinks = useMemo( () => scannableUrls.map( ( { url, amp_url: ampUrl, type, label } ) => ( {
2529
type,
2630
label,
@@ -31,7 +35,7 @@ export function usePreview() {
3135
const previewUrl = useMemo( () => previewLinks.find( ( link ) => link.isActive )?.url, [ previewLinks ] );
3236

3337
return {
34-
hasPreview,
38+
hasPreview: scannableUrls.length > 0,
3539
isPreviewingAMP,
3640
previewLinks,
3741
previewUrl,

‎assets/src/settings-page/index.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ function Providers( { children } ) {
9797
<ThemesContextProvider>
9898
<SiteScanContextProvider
9999
fetchCachedValidationErrors={ true }
100+
refetchPluginSuppressionOnScanComplete={ true }
100101
resetOnOptionsChange={ true }
101102
scannableUrlsRestPath={ SCANNABLE_URLS_REST_PATH }
102103
validateNonce={ VALIDATE_NONCE }

‎src/Validation/ScannableURLProvider.php‎

Lines changed: 49 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,14 @@ public function get_supportable_templates() {
7171
'is_singular',
7272
];
7373
if ( 'page' === get_option( 'show_on_front' ) ) {
74-
$allowed_templates[] = 'is_home';
75-
$allowed_templates[] = 'is_front_page';
74+
$page_for_posts = get_option( 'page_for_posts' );
75+
if ( $page_for_posts && amp_is_post_supported( $page_for_posts ) ) {
76+
$allowed_templates[] = 'is_home';
77+
}
78+
$page_on_front = get_option( 'page_on_front' );
79+
if ( $page_on_front && amp_is_post_supported( $page_on_front ) ) {
80+
$allowed_templates[] = 'is_front_page';
81+
}
7682
}
7783
foreach ( array_diff( array_keys( $supportable_templates ), $allowed_templates ) as $template ) {
7884
$supportable_templates[ $template ]['supported'] = false;
@@ -110,12 +116,41 @@ public function get_urls() {
110116
/*
111117
* If 'Your homepage displays' is set to 'Your latest posts', include the homepage.
112118
*/
113-
if ( 'posts' === get_option( 'show_on_front' ) && $this->is_template_supported( 'is_home' ) ) {
114-
$urls[] = [
115-
'url' => home_url( '/' ),
116-
'type' => 'is_home',
117-
'label' => __( 'Homepage', 'amp' ),
118-
];
119+
if ( 'posts' === get_option( 'show_on_front' ) ) {
120+
if ( $this->is_template_supported( 'is_home' ) ) {
121+
$urls[] = [
122+
'url' => home_url( '/' ),
123+
'type' => 'is_home',
124+
'label' => __( 'Homepage', 'amp' ),
125+
];
126+
}
127+
} elseif ( 'page' === get_option( 'show_on_front' ) ) {
128+
if (
129+
$this->is_template_supported( 'is_front_page' )
130+
&&
131+
get_option( 'page_on_front' )
132+
&&
133+
amp_is_post_supported( get_option( 'page_on_front' ) )
134+
) {
135+
$urls[] = [
136+
'url' => get_permalink( get_option( 'page_on_front' ) ),
137+
'type' => 'is_front_page',
138+
'label' => __( 'Homepage', 'amp' ),
139+
];
140+
}
141+
if (
142+
$this->is_template_supported( 'is_home' )
143+
&&
144+
get_option( 'page_for_posts' )
145+
&&
146+
amp_is_post_supported( get_option( 'page_for_posts' ) )
147+
) {
148+
$urls[] = [
149+
'url' => get_permalink( get_option( 'page_for_posts' ) ),
150+
'type' => 'is_home',
151+
'label' => __( 'Blog', 'amp' ),
152+
];
153+
}
119154
}
120155

121156
$amp_enabled_taxonomies = array_filter(
@@ -256,6 +291,12 @@ private function get_posts_by_type( $post_type, $offset = null ) {
256291
'order' => 'DESC',
257292
'fields' => 'ids',
258293
];
294+
if ( 'page' === get_option( 'show_on_front' ) ) {
295+
$args['post__not_in'] = [
296+
(int) get_option( 'page_for_posts' ),
297+
(int) get_option( 'page_on_front' ),
298+
];
299+
}
259300
if ( is_int( $offset ) ) {
260301
$args['offset'] = $offset;
261302
}

‎tests/php/src/Validation/ScannableURLProviderTest.php‎

Lines changed: 44 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ public function test__construct() {
4646
* @covers ::get_urls()
4747
* @covers ::get_supportable_templates()
4848
* @covers ::is_template_supported()
49+
* @covers ::get_posts_by_type()
4950
*/
5051
public function test_count_urls_to_validate_in_standard_mode() {
5152
$user = self::factory()->user->create();
@@ -103,24 +104,33 @@ public function test_count_urls_to_validate_in_standard_mode() {
103104
* @covers ::get_urls()
104105
* @covers ::get_supportable_templates()
105106
* @covers ::is_template_supported()
107+
* @covers ::get_posts_by_type()
106108
*/
107109
public function test_count_urls_to_validate_in_legacy_reader_mode() {
108-
$user_id = self::factory()->user->create();
109-
$term_id = self::factory()->term->create( [ 'taxonomy' => 'category' ] );
110-
$post_id = self::factory()->post->create(
110+
$this->scannable_url_provider->set_limit_per_type( 1 );
111+
112+
$user_id = self::factory()->user->create();
113+
$term_id = self::factory()->term->create( [ 'taxonomy' => 'category' ] );
114+
$post_id = self::factory()->post->create(
111115
[
112116
'post_type' => 'post',
113117
'post_author' => $user_id,
114118
'tax_input' => [ 'category' => $term_id ],
115119
]
116120
);
121+
$page1_id = self::factory()->post->create( [ 'post_type' => 'page' ] );
122+
$page2_id = self::factory()->post->create( [ 'post_type' => 'page' ] );
117123

118124
AMP_Options_Manager::update_option( Option::THEME_SUPPORT, AMP_Theme_Support::READER_MODE_SLUG );
119125
AMP_Options_Manager::update_option( Option::READER_THEME, ReaderThemes::DEFAULT_READER_THEME );
126+
$this->assertTrue( amp_is_legacy() );
120127

128+
// Test showing latest posts on front.
129+
update_option( 'show_on_front', 'posts' );
121130
$this->assertEqualSets(
122131
[
123132
get_permalink( $post_id ),
133+
get_permalink( $page2_id ),
124134
],
125135
wp_list_pluck( $this->scannable_url_provider->get_urls(), 'url' )
126136
);
@@ -136,20 +146,47 @@ static function ( $supportable_template ) {
136146
)
137147
);
138148

139-
$page1_id = self::factory()->post->create( [ 'page_type' => 'page' ] );
140-
$page2_id = self::factory()->post->create( [ 'page_type' => 'page' ] );
149+
// Test when there is a page_on_front and a page_on_front, but these pages do not have AMP enabled.
141150
update_option( 'show_on_front', 'page' );
142151
update_option( 'page_on_front', $page1_id );
143152
update_option( 'page_for_posts', $page2_id );
153+
$this->assertFalse( amp_is_post_supported( $page1_id ) );
154+
$this->assertFalse( amp_is_post_supported( $page2_id ) );
144155

145156
$this->assertEqualSets(
146157
[
147158
get_permalink( $post_id ),
148-
get_permalink( $page1_id ),
149-
get_permalink( $page2_id ),
150159
],
151160
wp_list_pluck( $this->scannable_url_provider->get_urls(), 'url' )
152161
);
162+
$this->assertEqualSets(
163+
[ 'is_singular' ],
164+
array_keys(
165+
array_filter(
166+
$this->scannable_url_provider->get_supportable_templates(),
167+
static function ( $supportable_template ) {
168+
return ! empty( $supportable_template['supported'] );
169+
}
170+
)
171+
)
172+
);
173+
174+
// Enable AMP for both page_on_front and page_on_front.
175+
update_post_meta( $page1_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY, AMP_Post_Meta_Box::ENABLED_STATUS );
176+
update_post_meta( $page2_id, AMP_Post_Meta_Box::STATUS_POST_META_KEY, AMP_Post_Meta_Box::ENABLED_STATUS );
177+
$this->assertTrue( amp_is_post_supported( $page1_id ) );
178+
$this->assertTrue( amp_is_post_supported( $page2_id ) );
179+
$this->assertEqualSets(
180+
array_unique(
181+
[
182+
get_permalink( $post_id ),
183+
get_permalink( $page1_id ),
184+
get_permalink( $page2_id ),
185+
home_url( '/' ), // Same as $page1_id since page_on_front.
186+
]
187+
),
188+
wp_list_pluck( $this->scannable_url_provider->get_urls(), 'url' )
189+
);
153190
$this->assertEqualSets(
154191
[ 'is_singular', 'is_home', 'is_front_page' ],
155192
array_keys(

0 commit comments

Comments
 (0)