Merge branch 'trunk' into epic/ras-acc

CHANGELOG.md

@@ -1,3 +1,10 @@
+## [4.2.2](https://github.com/Automattic/newspack-blocks/compare/v4.2.1...v4.2.2) (2024-10-11)
+
+
+### Bug Fixes
+
+* **block-caching:** only serve from cache if all blocks are cached ([#1899](https://github.com/Automattic/newspack-blocks/issues/1899)) ([fb64d0c](https://github.com/Automattic/newspack-blocks/commit/fb64d0ca4ffc2f16870bcc51db83f090054a5d26))
+
 ## [4.2.1](https://github.com/Automattic/newspack-blocks/compare/v4.2.0...v4.2.1) (2024-10-09)
 
 

includes/class-newspack-blocks-caching.php

@@ -12,11 +12,31 @@ class Newspack_Blocks_Caching {
 
 	const CACHE_GROUP = 'newspack_blocks';
 
+	/**
+	 * Store the cache status for all blocks for this request.
+	 *
+	 * @var bool
+	 */
+	private static $can_serve_all_blocks_from_cache = true;
+
+	/**
+	 * Store the current block index. This will be incremented with each cache reading,
+	 * in order to add specificity to the cache key. The cache key consists of the
+	 * hashed block attributes – which may be duplicated on a page – and a unique index.
+	 * With index only, replacing the block would *not* invalidate cache, which is undesired.
+	 * With hashed block attributes only, duplicated block configurations would result in
+	 * duplication of rendered posts.
+	 *
+	 * @var int
+	 */
+	private static $current_block_index = 0;
+
 	/**
 	 * Add hooks and filters.
 	 */
 	public static function init() {
 		if ( defined( 'NEWSPACK_BLOCKS_CACHE_BLOCKS' ) && NEWSPACK_BLOCKS_CACHE_BLOCKS ) {
+			add_action( 'template_redirect', [ __CLASS__, 'check_all_blocks_cache_status' ] );
 			add_filter( 'pre_render_block', [ __CLASS__, 'maybe_serve_cached_block' ], 10, 2 );
 			add_filter( 'render_block', [ __CLASS__, 'maybe_cache_block' ], 9999, 2 );
 
@@ -27,18 +47,65 @@ public static function init() {
 	}
 
 	/**
-	 * Determine whether a block should be cached.
+	 * Traverse all blocks on a page to check their cache status.
+	 * This has to happen before any blocks are rendered.
+	 */
+	public static function check_all_blocks_cache_status() {
+		if ( is_singular() ) {
+			$post = get_post();
+			if ( $post && property_exists( $post, 'post_content' ) ) {
+				self::check_block_cache_status( parse_blocks( $post->post_content ) );
+				// Reset the index after initial checks.
+				self::$current_block_index = 0;
+			}
+		}
+	}
+
+	/**
+	 * Check if a block can be cached, recursively.
 	 *
-	 * @param array $block_data Parsed block data.
-	 * @return bool True if block should be cached. False otherwise.
+	 * @param array $blocks Array of block data.
 	 */
-	protected static function should_cache_block( $block_data ) {
+	private static function check_block_cache_status( $blocks ) {
+		$cacheable_block_names = self::get_cacheable_blocks_names();
+		foreach ( $blocks as $block_data ) {
+			// Special treatment for reusable blocks, which are blocks stored in the posts table.
+			if ( $block_data['blockName'] === 'core/block' ) {
+				$reusable_block_post = get_post( $block_data['attrs']['ref'] );
+				if ( $reusable_block_post && property_exists( $reusable_block_post, 'post_content' ) ) {
+					self::check_block_cache_status( parse_blocks( $reusable_block_post->post_content ) );
+				}
+			}
+			if ( in_array( $block_data['blockName'], $cacheable_block_names, true ) ) {
+				if ( ! self::get_cached_block_data( $block_data ) ) {
+					self::$can_serve_all_blocks_from_cache = false;
+				}
+			}
+			if ( ! empty( $block_data['innerBlocks'] ) ) {
+				self::check_block_cache_status( $block_data['innerBlocks'] );
+			}
+		}
+	}
+
+	/**
+	 * Get cacheable blocks' names.
+	 */
+	public static function get_cacheable_blocks_names() {
 		$cacheable_blocks = [
 			'newspack-blocks/homepage-articles',
 			'newspack-blocks/carousel',
 		];
-		$cacheable_blocks = apply_filters( 'newspack_blocks_cacheable_blocks', $cacheable_blocks );
-		return in_array( $block_data['blockName'], $cacheable_blocks, true );
+		return apply_filters( 'newspack_blocks_cacheable_blocks', $cacheable_blocks );
+	}
+
+	/**
+	 * Determine whether a block should be cached.
+	 *
+	 * @param array $block_data Parsed block data.
+	 * @return bool True if block should be cached. False otherwise.
+	 */
+	protected static function should_cache_block( $block_data ) {
+		return in_array( $block_data['blockName'], self::get_cacheable_blocks_names(), true );
 	}
 
 	/**
@@ -49,7 +116,8 @@ protected static function should_cache_block( $block_data ) {
 	 */
 	protected static function get_cache_key( $block_data ) {
 		$block_attributes = $block_data['attrs'];
-		$cache_key        = 'np_cached_block_' . md5( wp_json_encode( $block_attributes ) );
+		$cache_key        = 'np_cached_block_' . md5( wp_json_encode( $block_attributes ) ) . '_' . self::$current_block_index;
+		self::$current_block_index++;
 		return $cache_key;
 	}
 
@@ -92,15 +160,13 @@ protected static function debug_log( $message ) {
 	}
 
 	/**
-	 * Serve a cached block if a valid one exists.
+	 * Is the block available in the cache?
 	 *
-	 * @param string|null $block_html Block HTML. If you return something non-null here it will short-circuit block rendering.
-	 * @param array       $block_data Parsed block data.
-	 * @return string|null Block markup if served from cache. Default (usually null), otherwise.
+	 * @param array $block_data Parsed block data.
 	 */
-	public static function maybe_serve_cached_block( $block_html, $block_data ) {
+	public static function get_cached_block_data( $block_data ) {
 		if ( ! self::should_cache_block( $block_data ) ) {
-			return $block_html;
+			return false;
 		}
 
 		$cache_key   = self::get_cache_key( $block_data );
@@ -110,7 +176,7 @@ public static function maybe_serve_cached_block( $block_html, $block_data ) {
 		$cached_data = wp_cache_get( $cache_key, $cache_group );
 		if ( ! is_array( $cached_data ) || ! isset( $cached_data['timestamp_generated'], $cached_data['cached_content'] ) || empty( $cached_data['cached_content'] ) ) {
 			self::debug_log( sprintf( 'Cached data not found for item %s in group %s', $cache_key, $cache_group ) );
-			return $block_html;
+			return false;
 		}
 
 		// Double-check to make sure cached data is still valid.
@@ -119,10 +185,30 @@ public static function maybe_serve_cached_block( $block_html, $block_data ) {
 				Newspack\Logger::log( sprintf( 'Flushing cache for item %s in group %s because it expired', $cache_key, $cache_group ) );
 			}
 			wp_cache_delete( $cache_key, $cache_group );
-			return $block_html;
+			return false;
 		}
+		self::debug_log( sprintf( 'Found cached block: item %s in group %s', $cache_key, $cache_group ) );
+		return $cached_data;
+	}
 
-		self::debug_log( sprintf( 'Serving cached block: item %s in group %s', $cache_key, $cache_group ) );
+	/**
+	 * Serve a cached block if a valid one exists.
+	 *
+	 * @param string|null $block_html Block HTML. If you return something non-null here it will short-circuit block rendering.
+	 * @param array       $block_data Parsed block data.
+	 * @return string|null Block markup if served from cache. Default (usually null), otherwise.
+	 */
+	public static function maybe_serve_cached_block( $block_html, $block_data ) {
+		if ( ! self::should_cache_block( $block_data ) ) {
+			return $block_html;
+		}
+		if ( ! self::$can_serve_all_blocks_from_cache ) {
+			return $block_html;
+		}
+		$cached_data = self::get_cached_block_data( $block_data );
+		if ( ! $cached_data ) {
+			return $block_html;
+		}
 
 		Newspack_Blocks::enqueue_view_assets( 'homepage-articles' );
 

includes/class-newspack-blocks.php

@@ -241,6 +241,7 @@ public static function enqueue_block_editor_assets() {
 				'custom_taxonomies'          => self::get_custom_taxonomies(),
 				'can_use_name_your_price'    => self::can_use_name_your_price(),
 				'tier_amounts_template'      => self::get_formatted_amount(),
+				'can_use_video_playlist'     => self::can_use_video_playlist_block(),
 			];
 
 			if ( class_exists( 'WP_REST_Newspack_Author_List_Controller' ) ) {
@@ -1667,5 +1668,29 @@ public static function get_image_caption( $attachment_id = null, $include_captio
 
 		return $combined_caption;
 	}
+
+	/**
+	 * Check if the current site can use the Video Playlist block.
+	 * If the block doesn't already exist in site content, it won't be registered.
+	 */
+	public static function can_use_video_playlist_block() {
+		// Check if the block exists in any content on the site.
+		$existing_blocks = new WP_Query(
+			[
+				'fields'         => 'ids',
+				'post_type'      => 'any',
+				'post_status'    => 'publish',
+				's'              => 'newspack-blocks/youtube-video-playlist',
+				'posts_per_page' => 1,
+			]
+		);
+
+		// Don't register the block if it's not already on the site.
+		if ( 0 < $existing_blocks->found_posts ) {
+			return true;
+		}
+
+		return false;
+	}
 }
 Newspack_Blocks::init();

newspack-blocks.php

@@ -7,15 +7,15 @@
  * Author URI:      https://newspack.com/
  * Text Domain:     newspack-blocks
  * Domain Path:     /languages
- * Version:         4.2.1
+ * Version:         4.2.2
  *
  * @package         Newspack_Blocks
  */
 
 define( 'NEWSPACK_BLOCKS__PLUGIN_FILE', __FILE__ );
 define( 'NEWSPACK_BLOCKS__BLOCKS_DIRECTORY', 'dist/' );
 define( 'NEWSPACK_BLOCKS__PLUGIN_DIR', plugin_dir_path( NEWSPACK_BLOCKS__PLUGIN_FILE ) );
-define( 'NEWSPACK_BLOCKS__VERSION', '4.2.1' );
+define( 'NEWSPACK_BLOCKS__VERSION', '4.2.2' );
 
 require_once NEWSPACK_BLOCKS__PLUGIN_DIR . 'includes/class-newspack-blocks.php';
 require_once NEWSPACK_BLOCKS__PLUGIN_DIR . 'includes/class-newspack-blocks-api.php';

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@automattic/newspack-blocks",
-	"version": "4.2.1",
+	"version": "4.2.2",
 	"author": "Automattic",
 	"devDependencies": {
 		"@rushstack/eslint-patch": "^1.10.4",

src/blocks/video-playlist/edit.js

@@ -1,19 +1,14 @@
 /**
  * WordPress dependencies
  */
-import { __ } from '@wordpress/i18n';
+import { __, sprintf } from '@wordpress/i18n';
 import apiFetch from '@wordpress/api-fetch';
 import { Component, Fragment } from '@wordpress/element';
-import { Placeholder, Spinner, PanelBody, RangeControl } from '@wordpress/components';
+import { Notice, Placeholder, Spinner, PanelBody } from '@wordpress/components';
 import { InspectorControls } from '@wordpress/block-editor';
 import { addQueryArgs } from '@wordpress/url';
 import { decodeEntities } from '@wordpress/html-entities';
 
-/**
- * Internal dependencies.
- */
-import AutocompleteTokenField from '../../components/autocomplete-tokenfield';
-
 class Edit extends Component {
 	constructor( props ) {
 		super( props );
@@ -175,55 +170,46 @@ class Edit extends Component {
 		} );
 	};
 
-	/**
-	 * Hide the overlay so users can play the videos.
-	 */
-	hideOverlay() {
-		this.setState( { interactive: true } );
-	}
-
 	/**
 	 * Render.
 	 */
 	render() {
-		const { attributes, setAttributes } = this.props;
-		const { categories, videosToShow } = attributes;
-		const { embed, isLoading, interactive } = this.state;
+		const { embed, isLoading } = this.state;
+
+		const Warning = () => (
+			<>
+				<h2>{ __( 'The YouTube Video Playlist block is deprecated', 'newspack-plugin' ) }</h2>
+				<p dangerouslySetInnerHTML={ {
+					__html: sprintf(
+						// translators: %1$s is the link to Google's help doc on creating YouTube playlists. %2$s is the link to the help doc on embedding playlists.
+						__( 'Consider using <a href="%1$s">YouTube Playlists</a> instead, which can be <a href="%2$s">embedded</a> into post or page content.', 'newspack-blocks' ),
+						'https://support.google.com/youtube/answer/57792',
+						'https://support.google.com/youtube/answer/171780'
+					),
+				} } />
+			</>
+		)
 
 		return (
 			<Fragment>
-				{ ( isLoading || '' === embed ) && this.renderPlaceholder() }
-				{ ! ( isLoading || '' === embed ) && (
-					<div className="wpbnbvp-preview">
-						<div dangerouslySetInnerHTML={ { __html: embed } } />
-						{ ! interactive && (
-							// eslint-disable-next-line jsx-a11y/no-static-element-interactions
-							<div className="wpbnbvp__overlay" onMouseUp={ () => this.hideOverlay() } />
-						) }
-					</div>
-				) }
+				<div>
+					{ ( isLoading || '' === embed ) && this.renderPlaceholder() }
+					{ ! ( isLoading || '' === embed ) && (
+						<div className="wpbnbvp-preview">
+							<div dangerouslySetInnerHTML={ { __html: embed } } />
+						</div>
+					) }
+					{ ! isLoading && (
+						<div className="wpbnbvp__overlay">
+							<Warning />
+						</div>
+					) }
+				</div>
 				<InspectorControls>
 					<PanelBody title={ __( 'Settings', 'newspack-blocks' ) } initialOpen={ true }>
-						<Fragment>
-							<RangeControl
-								className="videosToShow"
-								label={ __( 'Videos', 'newspack-blocks' ) }
-								help={ __( 'The maximum number of videos to pull from posts.', 'newspack-blocks' ) }
-								value={ videosToShow }
-								onChange={ _videosToShow => setAttributes( { videosToShow: _videosToShow } ) }
-								min={ 1 }
-								max={ 30 }
-								required
-							/>
-							<AutocompleteTokenField
-								key="categories"
-								tokens={ categories || [] }
-								onChange={ _categories => setAttributes( { categories: _categories } ) }
-								fetchSuggestions={ this.fetchCategorySuggestions }
-								fetchSavedInfo={ this.fetchSavedCategories }
-								label={ __( 'Categories', 'newspack-blocks' ) }
-							/>
-						</Fragment>
+						<Notice status="warning" isDismissible={ false }>
+							<Warning />
+						</Notice>
 					</PanelBody>
 				</InspectorControls>
 			</Fragment>

src/blocks/video-playlist/editor.js

@@ -4,4 +4,6 @@
 import { registerBlockType } from '@wordpress/blocks';
 import { name, settings } from '.';
 
-registerBlockType( `newspack-blocks/${ name }`, settings );
+if ( window.newspack_blocks_data?.can_use_video_playlist ) {
+	registerBlockType( `newspack-blocks/${ name }`, settings );
+}