feat(collections): collections block feedback

includes/collections/class-collection-meta.php

@@ -245,6 +245,16 @@ public static function register_rest_fields() {
 	 * @see Query_Helper::get_ctas()
 	 */
 	public static function get_collection_ctas_for_rest( $post ) {
-		return Query_Helper::get_ctas( $post['id'] );
+		return array_map(
+			function ( $cta ) {
+				return [
+					'label' => esc_html( $cta['label'] ?? '' ),
+					'url'   => esc_url( $cta['url'] ?? '' ),
+					'type'  => esc_attr( $cta['type'] ?? '' ),
+					'class' => esc_attr( $cta['class'] ?? '' ),
+				];
+			},
+			Query_Helper::get_ctas( $post['id'] )
+		);
 	}
 }

includes/collections/class-template-helper.php

@@ -563,7 +563,6 @@ public static function render_collections_grid( $collections ) {
 			'selectedCollections' => $collections,
 			'columns'             => 6,
 			'showCategory'        => false,
-			'showSeeAllLink'      => false,
 		];
 
 		/**
@@ -604,7 +603,6 @@ public static function render_collections_intro( $post, $args = [] ) {
 				'showExcerpt'         => true,
 				'showCategory'        => false,
 				'numberOfCTAs'        => -1,
-				'showSeeAllLink'      => false,
 				'headingText'         => '',
 				'noPermalinks'        => false,
 			]

src/blocks/collections/block.json

@@ -103,14 +103,6 @@
 		"specificCTAs": {
 			"type": "string",
 			"default": ""
-		},
-		"showSeeAllLink": {
-			"type": "boolean",
-			"default": true
-		},
-		"seeAllLinkText": {
-			"type": "string",
-			"default": ""
 		}
 	},
 	"example": {
@@ -125,9 +117,7 @@
 			"showVolume": true,
 			"showNumber": true,
 			"showCTAs": true,
-			"numberOfCTAs": 1,
-			"showSeeAllLink": true,
-			"seeAllLinkText": "See all collections"
+			"numberOfCTAs": 1
 		}
 	},
 	"textdomain": "newspack-plugin",

src/blocks/collections/class-collections-block.php

@@ -54,8 +54,6 @@ final class Collections_Block {
 		'showCTAs'            => true,
 		'numberOfCTAs'        => 1,
 		'specificCTAs'        => '',
-		'showSeeAllLink'      => true,
-		'seeAllLinkText'      => '',
 		'headingText'         => '',
 		'noPermalinks'        => false,
 	];
@@ -91,14 +89,8 @@ public static function register_block() {
 	 * @return string The block HTML.
 	 */
 	public static function render_block( array $attributes ) {
-
-		$attributes = wp_parse_args( $attributes, self::DEFAULT_ATTRIBUTES );
-
-		// Sanitize and normalize attributes that are used in queries/output.
-		$attributes['numberOfItems'] = max( 1, absint( $attributes['numberOfItems'] ) );
-		$attributes['offset']        = max( 0, absint( $attributes['offset'] ) );
-		$attributes['columns']       = max( 1, absint( $attributes['columns'] ) );
-		$attributes['numberOfCTAs']  = ( -1 === (int) $attributes['numberOfCTAs'] ) ? -1 : max( 1, absint( $attributes['numberOfCTAs'] ) );
+		// Sanitize and normalize attributes.
+		$attributes = self::sanitize_attributes( wp_parse_args( $attributes, self::DEFAULT_ATTRIBUTES ) );
 
 		// Normalize selectedCollections to determine if we have post objects or IDs.
 		$normalized_posts = Template_Helper::normalize_post_list( (array) $attributes['selectedCollections'] );
@@ -136,25 +128,36 @@ public static function render_block( array $attributes ) {
 		?>
 		<div <?php echo $wrapper_attributes; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?>>
 			<?php self::render_collections( $collections, $attributes ); ?>
-
-			<?php if ( $attributes['showSeeAllLink'] ) : ?>
-				<div class="wp-block-newspack-collections__see-all">
-					<a class="wp-block-button__link" href="<?php echo esc_url( get_post_type_archive_link( Post_Type::get_post_type() ) ); ?>">
-						<?php
-						if ( ! empty( $attributes['seeAllLinkText'] ) ) {
-							echo esc_html( $attributes['seeAllLinkText'] );
-						} else {
-							esc_html_e( 'See all', 'newspack-plugin' );
-						}
-						?>
-					</a>
-				</div>
-			<?php endif; ?>
 		</div>
 		<?php
 		return ob_get_clean();
 	}
 
+	/**
+	 * Sanitize and normalize attributes that are used in queries/output.
+	 *
+	 * @param array $attributes The block attributes.
+	 * @return array Sanitized attributes.
+	 */
+	public static function sanitize_attributes( array $attributes ) {
+		foreach ( [ 'numberOfItems', 'offset', 'columns', 'numberOfCTAs' ] as $attr ) {
+			if ( ! isset( $attributes[ $attr ] ) ) {
+				continue;
+			}
+
+			if ( 'numberOfCTAs' === $attr && -1 === (int) $attributes[ $attr ] ) {
+				$attributes[ $attr ] = -1;
+			} else {
+				$value               = absint( $attributes[ $attr ] );
+				$attributes[ $attr ] = $value > 0
+					? $value
+					: self::DEFAULT_ATTRIBUTES[ $attr ];
+			}
+		}
+
+		return $attributes;
+	}
+
 	/**
 	 * Get CSS classes for the block wrapper.
 	 *

src/blocks/collections/components/InspectorPanel.jsx

@@ -44,8 +44,6 @@ const InspectorPanel = ( { attributes, setAttributes } ) => {
 		showCTAs,
 		numberOfCTAs,
 		specificCTAs,
-		showSeeAllLink,
-		seeAllLinkText,
 	} = attributes;
 
 	// Category suggestions.
@@ -133,25 +131,6 @@ const InspectorPanel = ( { attributes, setAttributes } ) => {
 						/>
 					</>
 				) }
-
-				<ToggleControl
-					label={ __( 'Show "See all" link', 'newspack-plugin' ) }
-					checked={ showSeeAllLink }
-					onChange={ value => setAttributes( { showSeeAllLink: value } ) }
-				/>
-
-				{ showSeeAllLink && (
-					<BaseControl id="see-all-text-control" aria-label={ __( '"See all" link text', 'newspack-plugin' ) }>
-						<input
-							type="text"
-							value={ seeAllLinkText ? seeAllLinkText : __( 'See all', 'newspack-plugin' ) }
-							onChange={ e => setAttributes( { seeAllLinkText: e.target.value } ) }
-							placeholder={ __( 'See all', 'newspack-plugin' ) }
-							className="components-text-control__input"
-							style={ { height: '40px' } }
-						/>
-					</BaseControl>
-				) }
 			</PanelBody>
 
 			{ layout === 'grid' && (

src/blocks/collections/edit.jsx

@@ -1,19 +1,16 @@
 import { __ } from '@wordpress/i18n';
-import { useBlockProps, BlockControls, RichText } from '@wordpress/block-editor';
+import { useBlockProps, BlockControls } from '@wordpress/block-editor';
 import { ToolbarGroup, ToolbarButton, Placeholder, Spinner } from '@wordpress/components';
 import { caution, list, grid, pullLeft, pullRight, postFeaturedImage } from '@wordpress/icons';
 import { useMemo } from '@wordpress/element';
 import classnames from 'classnames';
 
 import { useCollections } from './hooks/useCollections';
 import CollectionItem from './components/CollectionItem';
-import usePreventNav from './hooks/usePreventNav';
 import InspectorPanel from './components/InspectorPanel';
 
 const Edit = ( { attributes, setAttributes } ) => {
-	const { layout, columns, imageAlignment, imageSize, showFeaturedImage, showSeeAllLink, seeAllLinkText } = attributes;
-
-	const preventNav = usePreventNav();
+	const { layout, columns, imageAlignment, imageSize, showFeaturedImage } = attributes;
 
 	// Fetch collections data.
 	const { collections, isLoading, hasCollections } = useCollections( attributes );
@@ -131,22 +128,6 @@ const Edit = ( { attributes, setAttributes } ) => {
 						) ) }
 					</>
 				) }
-
-				{ /* See all link */ }
-				{ showSeeAllLink && hasCollections && (
-					<div className="wp-block-newspack-collections__see-all">
-						<RichText
-							tagName="a"
-							className="wp-block-button__link"
-							value={ seeAllLinkText || __( 'See all', 'newspack-plugin' ) }
-							onChange={ value => setAttributes( { seeAllLinkText: value } ) }
-							href="#"
-							onClick={ preventNav }
-							allowedFormats={ [] }
-							placeholder={ __( 'See all', 'newspack-plugin' ) }
-						/>
-					</div>
-				) }
 			</div>
 		</>
 	);

src/blocks/collections/styles/_ctas.scss

@@ -9,16 +9,4 @@
 			width: 100%;
 		}
 	}
-
-	// See all collections link.
-	.wp-block-newspack-collections__see-all {
-		text-align: center;
-		grid-column: 1 / -1;
-
-		a:hover {
-			color: var(--newspack-theme-color-against-secondary);
-			background-color: var(--newspack-theme-color-bg-button-hover);
-		}
-	}
 }
-

tests/unit-tests/collections/class-test-collection-meta.php

@@ -262,4 +262,60 @@ public function test_get_collection_ctas_for_rest_empty() {
 		$this->assertIsArray( $result, 'Result should be an array.' );
 		$this->assertEmpty( $result, 'Should return empty array when no CTAs.' );
 	}
+
+	/**
+	 * Test that get_collection_ctas_for_rest escapes malicious values.
+	 *
+	 * @covers \Newspack\Collections\Collection_Meta::get_collection_ctas_for_rest
+	 */
+	public function test_get_collection_ctas_for_rest_escaping() {
+		$ctas_data = [
+			[
+				'type'  => 'link',
+				'label' => '<script>alert("xss")</script>Malicious Label',
+				'url'   => 'javascript:alert("malicious")',
+			],
+			[
+				'type'  => 'link',
+				'label' => 'Safe & Good Label',
+				'url'   => 'https://example.com/page?param=value&other=test',
+			],
+			[
+				'type'  => 'link"',
+				'label' => 'Test',
+				'url'   => 'https://example.com',
+				'class' => 'btn" onclick="alert(\'xss\')" class="fake',
+			],
+		];
+
+		// Test all fields are properly escaped.
+		$expected = [
+			[
+				'type'  => 'link',
+				'label' => '&lt;script&gt;alert(&quot;xss&quot;)&lt;/script&gt;Malicious Label',
+				'url'   => '',
+				'class' => '',
+			],
+			[
+				'type'  => 'link',
+				'label' => 'Safe &amp; Good Label',
+				'url'   => 'https://example.com/page?param=value&#038;other=test',
+				'class' => '',
+			],
+			[
+				'type'  => 'link&quot;',
+				'label' => 'Test',
+				'url'   => 'https://example.com',
+				'class' => 'btn&quot; onclick=&quot;alert(&#039;xss&#039;)&quot; class=&quot;fake',
+			],
+		];
+
+		$collection_id = $this->create_test_collection();
+		Collection_Meta::set( $collection_id, 'ctas', $ctas_data );
+		$result = Collection_Meta::get_collection_ctas_for_rest( [ 'id' => $collection_id ] );
+
+		$this->assertIsArray( $result, 'Result should be an array.' );
+		$this->assertCount( 3, $result, 'Should return 3 CTAs.' );
+		$this->assertEquals( $expected, $result, 'All CTA fields should be properly escaped.' );
+	}
 }

tests/unit-tests/collections/class-test-collections-block.php

@@ -115,35 +115,6 @@ public function test_render_block_with_collections() {
 		$this->assertStringNotContainsString( 'No collections found.', $output, 'Should not show no collections message' );
 	}
 
-	/**
-	 * Test render_block with see all link.
-	 *
-	 * @covers \Newspack\Blocks\Collections\Collections_Block::render_block
-	 */
-	public function test_render_block_with_custom_see_all_link() {
-		$this->create_test_collection();
-
-		$attributes = [
-			'seeAllLinkText' => 'View All Collections',
-		];
-		$output     = $this->render_collections_block( $attributes );
-
-		$this->assertStringContainsString( 'wp-block-newspack-collections__see-all', $output, 'Should contain see all wrapper' );
-		$this->assertStringContainsString( 'View All Collections', $output, 'Should contain custom see all text' );
-	}
-
-	/**
-	 * Test render_block with default see all text.
-	 *
-	 * @covers \Newspack\Blocks\Collections\Collections_Block::render_block
-	 */
-	public function test_render_block_with_default_see_all_link() {
-		$this->create_test_collection();
-		$output = $this->render_collections_block();
-
-		$this->assertStringContainsString( 'See all', $output, 'Should contain default see all text' );
-	}
-
 	/**
 	 * Test numberOfCTAs attribute handles -1 correctly for showing all CTAs.
 	 *
@@ -170,7 +141,6 @@ public function test_render_block_with_all_ctas() {
 			'selectedCollections' => [ $collection_id ],
 			'numberOfCTAs'        => -1,
 			'showCTAs'            => true,
-			'showSeeAllLink'      => false,
 		];
 
 		$output = $this->render_collections_block( $attributes );
@@ -538,23 +508,44 @@ public function test_render_collection_ctas_with_limit() {
 	/**
 	 * Test attribute sanitization.
 	 *
-	 * @covers \Newspack\Blocks\Collections\Collections_Block::render_block
+	 * @covers \Newspack\Blocks\Collections\Collections_Block::sanitize_attributes
 	 */
 	public function test_attribute_sanitization() {
 		$post_title    = 'Test Collection';
 		$collection_id = $this->create_test_collection( [ 'post_title' => $post_title ] );
 
 		// Test with negative and invalid values.
-		$attributes = [
-			'numberOfItems'       => -5,
+		$invalid_attributes = [
+			'numberOfItems'       => 'one',
 			'columns'             => 0,
 			'numberOfCTAs'        => -1,
+			'offset'              => -0.5,
 			'selectedCollections' => [ 'invalid', '123', $collection_id ],
 		];
 
-		$output = $this->render_collections_block( $attributes );
+		$sanitized = Collections_Block::sanitize_attributes( $invalid_attributes );
+
+		$this->assertEquals( Collections_Block::DEFAULT_ATTRIBUTES['numberOfItems'], $sanitized['numberOfItems'], 'Invalid numberOfItems should use default' );
+		$this->assertEquals( Collections_Block::DEFAULT_ATTRIBUTES['columns'], $sanitized['columns'], 'Invalid columns should use default' );
+		$this->assertEquals( Collections_Block::DEFAULT_ATTRIBUTES['offset'], $sanitized['offset'], 'Invalid offset should use default' );
+		$this->assertEquals( -1, $sanitized['numberOfCTAs'], 'Special value -1 for numberOfCTAs should be preserved' );
+
+		// Test with valid values that should be preserved (even if less than defaults).
+		$valid_attributes = [
+			'numberOfItems' => 2,
+			'columns'       => 1,
+			'numberOfCTAs'  => 3,
+		];
+
+		$sanitized_valid = Collections_Block::sanitize_attributes( $valid_attributes );
+
+		$this->assertEquals( 2, $sanitized_valid['numberOfItems'], 'Valid numberOfItems should be preserved' );
+		$this->assertEquals( 1, $sanitized_valid['columns'], 'Valid columns should be preserved' );
+		$this->assertEquals( 3, $sanitized_valid['numberOfCTAs'], 'Valid numberOfCTAs should be preserved' );
+
+		// Test rendering still works with invalid attributes.
+		$output = $this->render_collections_block( $invalid_attributes );
 
-		// Should still render successfully due to sanitization.
 		$this->assertStringContainsString( 'wp-block-newspack-collections', $output, 'Should contain block wrapper despite invalid attributes' );
 		$this->assertStringContainsString( $post_title, $output, 'Should contain collection title' );
 	}