Merge pull request #22072 from Yoast/446-implement-use-case-user-with…

…-wpseo_manage_options-capability-but-no-capability-to-install-plugins

fix error handling and capabilities for consent

packages/js/src/dashboard/widgets/top-queries-widget.js

@@ -1,9 +1,10 @@
 import { useCallback, useMemo } from "@wordpress/element";
 import { __ } from "@wordpress/i18n";
-import { Alert, SkeletonLoader } from "@yoast/ui-library";
+import { SkeletonLoader } from "@yoast/ui-library";
 import { WidgetTable } from "../components/widget-table";
 import { useRemoteData } from "../services/use-remote-data";
 import { Widget } from "./widget";
+import { ErrorAlert } from "../components/error-alert";
 
 /**
  * @type {import("../index").TopQueryData} TopQueryData
@@ -63,14 +64,15 @@ const TopQueriesTable = ( { data, children } ) => {
 /**
  * The content for TopQueriesWidget.
  *
- * @param {TopQueryData[]} [data]
- * @param {boolean} [isPending]
- * @param {Error} [error]
- * @param {number} [limit=5]
+ * @param {TopQueryData[]} [data] The data.
+ * @param {boolean} [isPending] Whether the data is pending.
+ * @param {Error} [error] The error.
+ * @param {number} [limit=5] The limit.
+ * @param {string} [supportLink] The support link.
  *
  * @returns {JSX.Element} The element.
  */
-export const TopQueriesWidgetContent = ( { data, isPending, error, limit = 5 } ) => {
+export const TopQueriesWidgetContent = ( { data, isPending, error, limit = 5, supportLink } ) => {
 	if ( isPending ) {
 		return (
 			<TopQueriesTable>
@@ -82,7 +84,7 @@ export const TopQueriesWidgetContent = ( { data, isPending, error, limit = 5 } )
 	}
 	if ( error ) {
 		return (
-			<Alert variant="error" className="yst-mt-4">{ error.message }</Alert>
+			<ErrorAlert error={ error } supportLink={ supportLink } className="yst-mt-4" />
 		);
 	}
 	if ( data.length === 0 ) {
@@ -128,6 +130,7 @@ export const TopQueriesWidget = ( { dataProvider, remoteDataProvider, dataFormat
 	}, [ dataProvider, limit ] );
 
 	const infoLink = dataProvider.getLink( "topQueriesInfoLearnMore" );
+	const supportLink = dataProvider.getLink( "errorSupport" );
 
 	/**
 	 * @type {function(?TopQueryData[]): TopQueryData[]} Function to format the top queries data.
@@ -150,6 +153,7 @@ export const TopQueriesWidget = ( { dataProvider, remoteDataProvider, dataFormat
 			isPending={ isPending }
 			limit={ limit }
 			error={ error }
+			supportLink={ supportLink }
 		/>
 	</Widget>;
 };

packages/js/tests/dashboard/widgets/top-pages-widget.test.js

@@ -84,7 +84,7 @@ describe( "TopPagesWidget", () => {
 		} );
 	} );
 
-	it( "should render the TopPagesWidget component with a pending state", async() => {
+	it( "should render the component with a pending state", async() => {
 		// Never resolving promise to ensure it keeps loading.
 		remoteDataProvider.fetchJson.mockImplementation( () => new Promise( () => {
 		} ) );
@@ -149,49 +149,32 @@ describe( "TopPagesWidget", () => {
 		} );
 	} );
 
-	it( "should render the TopPagesWidget component with an general error state", async() => {
-		// Mock the fetchJson method to throw an error.
-		const error = new Error( "Network Error" );
-		error.status = 500;
+	it.each( [
+		{
+			status: 408,
+			name: "Unauthorized",
+			message: "The request timed out. Try refreshing the page. If the problem persists, please check our Support page.",
+		},
+		{
+			status: 400,
+			name: "TimeoutError",
+			message: "The request timed out. Try refreshing the page. If the problem persists, please check our Support page.",
+		},
+		{
+			status: 403,
+			name: "Forbidden",
+			message: "You don’t have permission to access this resource. Please contact your admin for access. In case you need further help, please check our Support page.",
+		},
+		{
+			status: 500,
+			name: "Bad Request",
+			message: "Something went wrong. Try refreshing the page. If the problem persists, please check our Support page.",
+		},
+	] )( "should render the message for an error with status $status and error name $name.", async( { status, name, message } ) => {
+		const error = new Error( name );
+		error.status = status;
+		error.name = name;
 		remoteDataProvider.fetchJson.mockRejectedValue( error );
-
-		const { getByRole } = render( <TopPagesWidget
-			dataProvider={ dataProvider }
-			remoteDataProvider={ remoteDataProvider }
-			dataFormatter={ dataFormatter }
-		/> );
-
-		await waitFor( () => {
-			expect( getByRole( "status" ) )
-				.toHaveTextContent( "Something went wrong. Try refreshing the page. If the problem persists, please check our Support page." );
-			expect( getByRole( "link", { name: "Support page" } ) ).toHaveAttribute( "href", "https://example.com/error-support" );
-		} );
-	} );
-
-	it( "should render the TopPagesWidget component with an time out error state", async() => {
-		// Mock the fetchJson method to throw an error.
-		const error = new Error( "TimeoutError" );
-		error.status = 408;
-		remoteDataProvider.fetchJson.mockRejectedValue( error );
-
-		const { getByRole } = render( <TopPagesWidget
-			dataProvider={ dataProvider }
-			remoteDataProvider={ remoteDataProvider }
-			dataFormatter={ dataFormatter }
-		/> );
-
-		await waitFor( () => {
-			expect( getByRole( "status" ) )
-				.toHaveTextContent( "The request timed out. Try refreshing the page. If the problem persists, please check our Support page." );
-			expect( getByRole( "link", { name: "Support page" } ) ).toHaveAttribute( "href", "https://example.com/error-support" );
-		} );
-	} );
-
-	it( "should render the TopPagesWidget component with an no permission error state", async() => {
-		const error = new Error( "NoPermissionError" );
-		error.status = 403;
-		remoteDataProvider.fetchJson.mockRejectedValue( error );
-
 		const { getByRole } = render( <TopPagesWidget
 			dataProvider={ dataProvider }
 			remoteDataProvider={ remoteDataProvider }
@@ -200,7 +183,7 @@ describe( "TopPagesWidget", () => {
 
 		await waitFor( () => {
 			expect( getByRole( "status" ) )
-				.toHaveTextContent( "You don’t have permission to access this resource. Please contact your admin for access. In case you need further help, please check our Support page." );
+				.toHaveTextContent( message );
 			expect( getByRole( "link", { name: "Support page" } ) ).toHaveAttribute( "href", "https://example.com/error-support" );
 		} );
 	} );

packages/js/tests/dashboard/widgets/top-queries-widget.test.js

@@ -32,7 +32,7 @@ describe( "TopQueriesWidget", () => {
 		remoteDataProvider.fetchJson.mockClear();
 	} );
 
-	it( "should render the TopQueriesWidget component", async() => {
+	it( "should render the component", async() => {
 		remoteDataProvider.fetchJson.mockResolvedValue( data );
 		const { getByRole, getByText } = render( <TopQueriesWidget
 			dataProvider={ dataProvider }
@@ -57,7 +57,7 @@ describe( "TopQueriesWidget", () => {
 		} );
 	} );
 
-	it( "should render the TopQueriesWidget component without data", async() => {
+	it( "should render the component without data", async() => {
 		remoteDataProvider.fetchJson.mockResolvedValue( [] );
 		const { getByText } = render( <TopQueriesWidget
 			dataProvider={ dataProvider }
@@ -71,21 +71,46 @@ describe( "TopQueriesWidget", () => {
 		} );
 	} );
 
-	it( "should render the TopQueriesWidget component with an error", async() => {
-		const message = "An error occurred.";
-		remoteDataProvider.fetchJson.mockRejectedValue( new Error( message ) );
-		const { getByText } = render( <TopQueriesWidget
+	it.each( [
+		{
+			status: 408,
+			name: "Unauthorized",
+			message: "The request timed out. Try refreshing the page. If the problem persists, please check our Support page.",
+		},
+		{
+			status: 400,
+			name: "TimeoutError",
+			message: "The request timed out. Try refreshing the page. If the problem persists, please check our Support page.",
+		},
+		{
+			status: 403,
+			name: "Forbidden",
+			message: "You don’t have permission to access this resource. Please contact your admin for access. In case you need further help, please check our Support page.",
+		},
+		{
+			status: 500,
+			name: "Bad Request",
+			message: "Something went wrong. Try refreshing the page. If the problem persists, please check our Support page.",
+		},
+	] )( "should render the message for an error with status $status and error name $name.", async( { status, name, message } ) => {
+		const error = new Error( name );
+		error.status = status;
+		error.name = name;
+		remoteDataProvider.fetchJson.mockRejectedValue( error );
+		const { getByRole } = render( <TopQueriesWidget
 			dataProvider={ dataProvider }
 			remoteDataProvider={ remoteDataProvider }
 			dataFormatter={ dataFormatter }
 		/> );
 
 		await waitFor( () => {
-			expect( getByText( message ) ).toBeInTheDocument();
+			expect( getByRole( "status" ) )
+				.toHaveTextContent( message );
+			expect( getByRole( "link", { name: "Support page" } ) ).toHaveAttribute( "href", "https://example.com/error-support" );
 		} );
 	} );
 
-	it( "should render the TopQueriesWidget component with a pending state", async() => {
+	it( "should render the component with a pending state", async() => {
 		// Never resolving promise to ensure it keeps loading.
 		remoteDataProvider.fetchJson.mockImplementation( () => new Promise( () => {
 		} ) );

src/dashboard/user-interface/configuration/site-kit-configuration-dismissal-route.php

@@ -8,6 +8,7 @@
 use WP_REST_Response;
 use Yoast\WP\SEO\Conditionals\No_Conditionals;
 use Yoast\WP\SEO\Dashboard\Infrastructure\Configuration\Permanently_Dismissed_Site_Kit_Configuration_Repository_Interface;
+use Yoast\WP\SEO\Helpers\Capability_Helper;
 use Yoast\WP\SEO\Main;
 use Yoast\WP\SEO\Routes\Route_Interface;
 
@@ -43,15 +44,25 @@ class Site_Kit_Configuration_Dismissal_Route implements Route_Interface {
 	 */
 	private $permanently_dismissed_site_kit_configuration_repository;
 
+	/**
+	 * Holds the capabilit helper instance.
+	 *
+	 * @var Capability_Helper
+	 */
+	private $capability_helper;
+
 	/**
 	 * Constructs the class.
 	 *
 	 * @param Permanently_Dismissed_Site_Kit_Configuration_Repository_Interface $permanently_dismissed_site_kit_configuration_repository The repository.
+	 * @param Capability_Helper                                                 $capability_helper                                       The capability helper.
 	 */
 	public function __construct(
-		Permanently_Dismissed_Site_Kit_Configuration_Repository_Interface $permanently_dismissed_site_kit_configuration_repository
+		Permanently_Dismissed_Site_Kit_Configuration_Repository_Interface $permanently_dismissed_site_kit_configuration_repository,
+		Capability_Helper $capability_helper
 	) {
 		$this->permanently_dismissed_site_kit_configuration_repository = $permanently_dismissed_site_kit_configuration_repository;
+		$this->capability_helper                                       = $capability_helper;
 	}
 
 	/**
@@ -115,6 +126,6 @@ public function set_site_kit_configuration_permanent_dismissal( WP_REST_Request
 	 * @return bool
 	 */
 	public function check_capabilities() {
-		return \current_user_can( 'install_plugins' );
+		return $this->capability_helper->current_user_can( 'wpseo_manage_options' );
 	}
 }

src/dashboard/user-interface/configuration/site-kit-consent-management-route.php

@@ -8,6 +8,7 @@
 use WP_REST_Response;
 use Yoast\WP\SEO\Conditionals\No_Conditionals;
 use Yoast\WP\SEO\Dashboard\Infrastructure\Configuration\Site_Kit_Consent_Repository_Interface;
+use Yoast\WP\SEO\Helpers\Capability_Helper;
 use Yoast\WP\SEO\Main;
 use Yoast\WP\SEO\Routes\Route_Interface;
 
@@ -43,15 +44,25 @@ class Site_Kit_Consent_Management_Route implements Route_Interface {
 	 */
 	private $site_kit_consent_repository;
 
+	/**
+	 * Holds the capabilit helper instance.
+	 *
+	 * @var Capability_Helper
+	 */
+	private $capability_helper;
+
 	/**
 	 * Constructs the class.
 	 *
 	 * @param Site_Kit_Consent_Repository_Interface $site_kit_consent_repository The repository.
+	 * @param Capability_Helper                     $capability_helper           The capability helper.
 	 */
 	public function __construct(
-		Site_Kit_Consent_Repository_Interface $site_kit_consent_repository
+		Site_Kit_Consent_Repository_Interface $site_kit_consent_repository,
+		Capability_Helper $capability_helper
 	) {
 		$this->site_kit_consent_repository = $site_kit_consent_repository;
+		$this->capability_helper           = $capability_helper;
 	}
 
 	/**
@@ -115,6 +126,6 @@ public function set_site_kit_consent( WP_REST_Request $request ) {
 	 * @return bool
 	 */
 	public function check_capabilities() {
-		return \current_user_can( 'install_plugins' );
+		return $this->capability_helper->current_user_can( 'wpseo_manage_options' );
 	}
 }

tests/Unit/Dashboard/User_Interface/Configuration/Abstract_Site_Kit_Configuration_Permanent_Dismissal_Route_Test.php

@@ -5,6 +5,7 @@
 use Mockery;
 use WP_Error;
 use Yoast\WP\SEO\Dashboard\User_Interface\Configuration\Site_Kit_Configuration_Dismissal_Route;
+use Yoast\WP\SEO\Helpers\Capability_Helper;
 use Yoast\WP\SEO\Tests\Unit\Dashboard\Infrastructure\Configuration\Permanently_Dismissed_Site_Kit_Configuration_Repository_Fake;
 use Yoast\WP\SEO\Tests\Unit\TestCase;
 
@@ -24,6 +25,13 @@ abstract class Abstract_Site_Kit_Configuration_Permanent_Dismissal_Route_Test ex
 	 */
 	protected $instance;
 
+	/**
+	 * Holds the mock for the capability helper.
+	 *
+	 * @var Mockery\MockInterface|Capability_Helper
+	 */
+	protected $capability_helper;
+
 	/**
 	 * Sets up the test fixtures.
 	 *
@@ -33,9 +41,11 @@ protected function set_up() {
 		parent::set_up();
 
 		Mockery::mock( WP_Error::class );
+		$this->capability_helper = Mockery::mock( Capability_Helper::class );
 
 		$this->instance = new Site_Kit_Configuration_Dismissal_Route(
-			new Permanently_Dismissed_Site_Kit_Configuration_Repository_Fake()
+			new Permanently_Dismissed_Site_Kit_Configuration_Repository_Fake(),
+			$this->capability_helper
 		);
 	}
 }

tests/Unit/Dashboard/User_Interface/Configuration/Abstract_Site_Kit_Consent_Management_Route_Test.php

@@ -5,6 +5,7 @@
 use Mockery;
 use WP_Error;
 use Yoast\WP\SEO\Dashboard\User_Interface\Configuration\Site_Kit_Consent_Management_Route;
+use Yoast\WP\SEO\Helpers\Capability_Helper;
 use Yoast\WP\SEO\Tests\Unit\Dashboard\Infrastructure\Configuration\Site_Kit_Consent_Repository_Fake;
 use Yoast\WP\SEO\Tests\Unit\TestCase;
 
@@ -24,6 +25,13 @@ abstract class Abstract_Site_Kit_Consent_Management_Route_Test extends TestCase
 	 */
 	protected $instance;
 
+	/**
+	 * Holds the mock for the capability helper.
+	 *
+	 * @var Mockery\MockInterface|Capability_Helper
+	 */
+	protected $capability_helper;
+
 	/**
 	 * Sets up the test fixtures.
 	 *
@@ -33,9 +41,11 @@ protected function set_up() {
 		parent::set_up();
 
 		Mockery::mock( WP_Error::class );
+		$this->capability_helper = Mockery::mock( Capability_Helper::class );
 
 		$this->instance = new Site_Kit_Consent_Management_Route(
-			new Site_Kit_Consent_Repository_Fake()
+			new Site_Kit_Consent_Repository_Fake(),
+			$this->capability_helper
 		);
 	}
 }

tests/Unit/Dashboard/User_Interface/Configuration/Site_Kit_Configuration_Permanent_Dismissal_Route_Check_Capabilities_Test.php

@@ -2,8 +2,6 @@
 // phpcs:disable Yoast.NamingConventions.NamespaceName.TooLong -- Needed in the folder structure.
 namespace Yoast\WP\SEO\Tests\Unit\Dashboard\User_Interface\Configuration;
 
-use Brain\Monkey\Functions;
-
 /**
  * Abstract class for the Permanently Dismissed Site Kit Configuration Repository tests.
  *
@@ -21,9 +19,10 @@ final class Site_Kit_Configuration_Permanent_Dismissal_Route_Check_Capabilities_
 	 * @return void
 	 */
 	public function test_check_capabilities() {
-		Functions\expect( 'current_user_can' )
+		$this->capability_helper->expects( 'current_user_can' )
 			->once()
-			->with( 'install_plugins' )->andReturn( true );
+			->with( 'wpseo_manage_options' )
+			->andReturn( true );
 
 		$this->assertTrue( $this->instance->check_capabilities() );
 	}