Merge pull request #5343 from ampproject/feature/5271-cache-error-ind…

…ex-counts Cache error index counts and URL count in "At a Glance" widget
ampproject · Sep 10, 2020 · a2c0d44 · a2c0d44
2 parents c39f672 + 4fe39ad
commit a2c0d44
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 31 deletions.
diff --git a/includes/validation/class-amp-validated-url-post-type.php b/includes/validation/class-amp-validated-url-post-type.php
@@ -223,6 +223,7 @@ static function () {
 		$handle_delete = static function ( $post_id ) {
 			if ( static::POST_TYPE_SLUG === get_post_type( $post_id ) ) {
 				delete_transient( static::NEW_VALIDATION_ERROR_URLS_COUNT_TRANSIENT );
+				delete_transient( AMP_Validation_Error_Taxonomy::TRANSIENT_KEY_ERROR_INDEX_COUNTS );
 			}
 		};
 		add_action( 'save_post_' . self::POST_TYPE_SLUG, $handle_delete );
@@ -482,17 +483,7 @@ public static function add_admin_menu_new_invalid_url_count() {
 			return;
 		}
 
-		$new_validation_error_urls = get_transient( static::NEW_VALIDATION_ERROR_URLS_COUNT_TRANSIENT );
-
-		if ( false === $new_validation_error_urls ) {
-			$new_validation_error_urls = static::get_validation_error_urls_count();
-			set_transient( static::NEW_VALIDATION_ERROR_URLS_COUNT_TRANSIENT, $new_validation_error_urls, DAY_IN_SECONDS );
-		} else {
-			// Handle case where integer stored in transient gets returned as string when persistent object cache is not
-			// used. This is due to wp_options.option_value being a string.
-			$new_validation_error_urls = (int) $new_validation_error_urls;
-		}
-
+		$new_validation_error_urls = static::get_validation_error_urls_count();
 		if ( 0 === $new_validation_error_urls ) {
 			return;
 		}
@@ -513,6 +504,13 @@ public static function add_admin_menu_new_invalid_url_count() {
 	 * @return int Count of new validation error URLs.
 	 */
 	protected static function get_validation_error_urls_count() {
+		$count = get_transient( static::NEW_VALIDATION_ERROR_URLS_COUNT_TRANSIENT );
+		if ( false !== $count ) {
+			// Handle case where integer stored in transient gets returned as string when persistent object cache is not
+			// used. This is due to wp_options.option_value being a string.
+			return (int) $count;
+		}
+
 		$query = new WP_Query(
 			[
 				'post_type'              => self::POST_TYPE_SLUG,
@@ -525,7 +523,11 @@ protected static function get_validation_error_urls_count() {
 			]
 		);
 
-		return $query->found_posts;
+		$count = $query->found_posts;
+
+		set_transient( static::NEW_VALIDATION_ERROR_URLS_COUNT_TRANSIENT, $count, DAY_IN_SECONDS );
+
+		return $count;
 	}
 
 	/**
@@ -2925,20 +2927,9 @@ public static function get_recheck_url( $url_or_post ) {
 	 * @return array Items.
 	 */
 	public static function filter_dashboard_glance_items( $items ) {
+		$count = self::get_validation_error_urls_count();
 
-		$query = new WP_Query(
-			[
-				'post_type'              => self::POST_TYPE_SLUG,
-				AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_STATUS_QUERY_VAR => [
-					AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_REJECTED_STATUS,
-					AMP_Validation_Error_Taxonomy::VALIDATION_ERROR_NEW_ACCEPTED_STATUS,
-				],
-				'update_post_meta_cache' => false,
-				'update_post_term_cache' => false,
-			]
-		);
-
-		if ( 0 !== $query->found_posts ) {
+		if ( 0 !== $count ) {
 			$items[] = sprintf(
 				'<a class="amp-validation-errors" href="%s">%s</a>',
 				esc_url(
@@ -2961,10 +2952,10 @@ public static function filter_dashboard_glance_items( $items ) {
 						_n(
 							'%s URL w/ new AMP errors',
 							'%s URLs w/ new AMP errors',
-							$query->found_posts,
+							$count,
 							'amp'
 						),
-						number_format_i18n( $query->found_posts )
+						number_format_i18n( $count )
 					)
 				)
 			);

diff --git a/includes/validation/class-amp-validation-error-taxonomy.php b/includes/validation/class-amp-validation-error-taxonomy.php
@@ -204,6 +204,13 @@ class AMP_Validation_Error_Taxonomy {
 	 */
 	const ERROR_STATUS = 'error_status';
 
+	/**
+	 * Key for the transient storing error index counts.
+	 *
+	 * @var string
+	 */
+	const TRANSIENT_KEY_ERROR_INDEX_COUNTS = 'amp_error_index_counts';
+
 	/**
 	 * Whether the terms_clauses filter should apply to a term query for validation errors to limit to a given status.
 	 *
@@ -279,6 +286,10 @@ public static function register() {
 		if ( is_admin() ) {
 			self::add_admin_hooks();
 		}
+
+		add_action( 'created_' . self::TAXONOMY_SLUG, [ __CLASS__, 'clear_cached_counts' ] );
+		add_action( 'edited_' . self::TAXONOMY_SLUG, [ __CLASS__, 'clear_cached_counts' ] );
+		add_action( 'delete_' . self::TAXONOMY_SLUG, [ __CLASS__, 'clear_cached_counts' ] );
 	}
 
 	/**
@@ -595,6 +606,16 @@ public static function get_validation_error_count( $args = [] ) {
 			$args
 		);
 
+		$cache_key     = wp_json_encode( $args );
+		$cached_counts = get_transient( self::TRANSIENT_KEY_ERROR_INDEX_COUNTS );
+		if ( empty( $cached_counts ) ) {
+			$cached_counts = [];
+		}
+
+		if ( isset( $cached_counts[ $cache_key ] ) ) {
+			return $cached_counts[ $cache_key ];
+		}
+
 		$groups = null;
 		if ( isset( $args['group'] ) ) {
 			$groups = self::sanitize_term_status( $args['group'], [ 'multiple' => true ] );
@@ -613,7 +634,13 @@ public static function get_validation_error_count( $args = [] ) {
 		if ( isset( $args['group'] ) ) {
 			remove_filter( 'terms_clauses', $filter );
 		}
-		return (int) $term_count;
+
+		$result = (int) $term_count;
+
+		$cached_counts[ $cache_key ] = $result;
+		set_transient( self::TRANSIENT_KEY_ERROR_INDEX_COUNTS, $cached_counts, DAY_IN_SECONDS );
+
+		return $result;
 	}
 
 	/**
@@ -3338,4 +3365,11 @@ public static function get_status_text_with_icon( $sanitization, $include_review
 
 		return sprintf( '<span class="status-text">%s %s</span>', $icon->to_html(), esc_html( $text ) );
 	}
+
+	/**
+	 * Deletes cached term counts.
+	 */
+	public static function clear_cached_counts() {
+		delete_transient( self::TRANSIENT_KEY_ERROR_INDEX_COUNTS );
+	}
 }
diff --git a/tests/e2e/specs/amp-onboarding/template-mode.js b/tests/e2e/specs/amp-onboarding/template-mode.js
@@ -88,12 +88,12 @@ describe( 'Stepper item modifications', () => {
 describe( 'Template mode recommendations with non-reader-theme active', () => {
 	beforeEach( async () => {
 		await cleanUpSettings();
-		await installTheme( 'astra' );
-		await activateTheme( 'astra' );
+		await installTheme( 'hestia' );
+		await activateTheme( 'hestia' );
 	} );
 
 	afterEach( async () => {
-		await deleteTheme( 'astra', 'twentytwenty' );
+		await deleteTheme( 'hestia', 'twentytwenty' );
 	} );
 
 	it( 'makes correct recommendations when user is not technical and the current theme is not a reader theme', async () => {