From 54095f4077f69d6eb642860a285a193fbba5ab82 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Thu, 16 Jun 2022 15:09:42 -0400 Subject: [PATCH 01/12] Add Module::is_recoverable method. --- includes/Core/Modules/Module.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/includes/Core/Modules/Module.php b/includes/Core/Modules/Module.php index e5278537753..749cdb1d1bb 100644 --- a/includes/Core/Modules/Module.php +++ b/includes/Core/Modules/Module.php @@ -836,4 +836,22 @@ public function is_shareable() { return false; } + + /** + * Checks whether the module is recoverable. + * + * @since n.e.x.t + * + * @return bool + */ + public function is_recoverable() { + /** + * Filters the recoverable status of the module. + * + * @since n.e.x.t + * @param bool $_ Whether or not the module is recoverable. Default: false + * @param string $slug Module slug. + */ + return (bool) apply_filters( 'googlesitekit_is_module_recoverable', false, $this->slug ); + } } From e152f2e6085e275475dc1357d3edf1af34b77919 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Thu, 16 Jun 2022 15:09:59 -0400 Subject: [PATCH 02/12] Filter module recoverability from Modules. --- includes/Core/Modules/Modules.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/includes/Core/Modules/Modules.php b/includes/Core/Modules/Modules.php index f59158959c9..99b6bf1571b 100644 --- a/includes/Core/Modules/Modules.php +++ b/includes/Core/Modules/Modules.php @@ -364,6 +364,13 @@ function ( $data ) { } ); + add_filter( + 'googlesitekit_is_module_recoverable', + function ( $slug ) { + return $this->is_module_recoverable( $slug ); + } + ); + add_filter( 'option_' . Module_Sharing_Settings::OPTION, $this->get_method_proxy( 'filter_shared_ownership_module_settings' ) ); add_filter( 'default_option_' . Module_Sharing_Settings::OPTION, $this->get_method_proxy( 'filter_shared_ownership_module_settings' ), 20 ); From da48e8f724c7b254617b93dc9c71f8a2260399fe Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Thu, 16 Jun 2022 15:11:39 -0400 Subject: [PATCH 03/12] Update oauth client qualification logic. --- includes/Core/Modules/Module.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/includes/Core/Modules/Module.php b/includes/Core/Modules/Module.php index 749cdb1d1bb..c4ad2b66a38 100644 --- a/includes/Core/Modules/Module.php +++ b/includes/Core/Modules/Module.php @@ -577,9 +577,17 @@ private function get_oauth_client_for_datapoint( Datapoint $datapoint ) { && $this->is_shareable() && $datapoint->is_shareable() && $this->get_owner_id() !== get_current_user_id() + && ! $this->is_recoverable() && current_user_can( Permissions::READ_SHARED_MODULE_DATA, $this->slug ) ) { - return $this->get_owner_oauth_client(); + $oauth_client = $this->get_owner_oauth_client(); + + try { + $this->validate_base_scopes( $oauth_client ); + return $oauth_client; + } catch ( Exception $exception ) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedCatch + // Fallthrough to default oauth client if scopes are unsatisfied. + } } return $this->authentication->get_oauth_client(); From c35a8b7f9baa78a8824d6dcff127b2b4b3135b9d Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Fri, 17 Jun 2022 13:40:40 -0400 Subject: [PATCH 04/12] Add test for is_recoverable. --- .../integration/Core/Modules/ModuleTest.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/phpunit/integration/Core/Modules/ModuleTest.php b/tests/phpunit/integration/Core/Modules/ModuleTest.php index 0342d49f2ec..73554c1511e 100644 --- a/tests/phpunit/integration/Core/Modules/ModuleTest.php +++ b/tests/phpunit/integration/Core/Modules/ModuleTest.php @@ -349,6 +349,27 @@ public function test_is_shareable() { $this->assertFalse( $module->is_shareable() ); } + public function test_is_recoverable() { + remove_all_filters( 'googlesitekit_is_module_recoverable' ); + $module = new FakeModule( new Context( GOOGLESITEKIT_PLUGIN_MAIN_FILE ) ); + $invocations = array(); + $spy = function ( ...$args ) use ( &$invocations ) { + $invocations[] = $args; + return $args[0]; + }; + + // is_recoverable is a proxy through this filter which is handled by + // Modules::is_module_recoverable. @see \Google\Site_Kit\Tests\Core\Modules\ModulesTest::test_is_module_recoverable + add_filter( 'googlesitekit_is_module_recoverable', $spy, 10, 2 ); + + $module->is_recoverable(); + + $this->assertCount( 1, $invocations ); + list ( $given, $slug ) = $invocations[0]; + $this->assertFalse( $given ); + $this->assertEquals( $module->slug, $slug ); + } + /** * Determine the difference between the expected and the returned date. * From 9564ac269e45b41cf56a6df0898e366d5a9aea17 Mon Sep 17 00:00:00 2001 From: Evan Mattson Date: Fri, 17 Jun 2022 13:41:05 -0400 Subject: [PATCH 05/12] Update filter in Modules. --- includes/Core/Modules/Modules.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/includes/Core/Modules/Modules.php b/includes/Core/Modules/Modules.php index 99b6bf1571b..da661eed4e3 100644 --- a/includes/Core/Modules/Modules.php +++ b/includes/Core/Modules/Modules.php @@ -366,9 +366,11 @@ function ( $data ) { add_filter( 'googlesitekit_is_module_recoverable', - function ( $slug ) { + function ( $recoverable, $slug ) { return $this->is_module_recoverable( $slug ); - } + }, + 10, + 2 ); add_filter( 'option_' . Module_Sharing_Settings::OPTION, $this->get_method_proxy( 'filter_shared_ownership_module_settings' ) ); From 108614d9e01f03e0e94920165f2f8fa44babe3f1 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Sun, 19 Jun 2022 15:25:54 +0600 Subject: [PATCH 06/12] Change 'All Admins' with 'Any admin signed in with Google'. --- .../dashboard-sharing/DashboardSharingSettings/Module.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js index 52fc3767fdd..32dd1935af9 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js @@ -65,7 +65,7 @@ const viewAccessOptions = [ }, { value: 'all_admins', - label: __( 'All Admins', 'google-site-kit' ), + label: __( 'Any admin signed in with Google', 'google-site-kit' ), }, ]; From b088c96fbd342691c521cf9050f9998b1572b1bb Mon Sep 17 00:00:00 2001 From: nfmohit Date: Sun, 19 Jun 2022 15:41:08 +0600 Subject: [PATCH 07/12] Update notice to reflect on the wording change. --- .../dashboard-sharing/DashboardSharingSettings/Notice.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js index f802c8993fb..f7f3e5866f9 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js @@ -51,7 +51,7 @@ export default function Notice() { isEditingManagement && createInterpolateElement( __( - 'By clicking Apply, you are giving other admins permission to manage view-only access on your behalf to data from the Google services you selected “All Admins” for.', + 'By clicking Apply, you are giving other admins permission to manage view-only access on your behalf to data from the Google services you selected “Any admin signed in with Google” for.', 'google-site-kit' ), { From 71e9c7d9ed1c6e5cb3dae7c296903095b7c5a977 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Sun, 19 Jun 2022 15:42:35 +0600 Subject: [PATCH 08/12] Update test copy to reflect on this change. --- .../dashboard-sharing/DashboardSharingSettings/index.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js index 9593d464a56..60d17edd47c 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/index.test.js @@ -184,7 +184,7 @@ describe( 'DashboardSharingSettings', () => { ).toBeInTheDocument(); } ); - it( 'should set sharing management to `All Admins` and disabled if a module has shared ownership', () => { + it( 'should set sharing management to `Any admin signed in with Google` and disabled if a module has shared ownership', () => { provideModules( registry, modules ); provideModuleRegistrations( registry ); provideSiteConnection( registry, { From 65e3ce22e8587c6cc013c48ad140e1e37f3cf4ed Mon Sep 17 00:00:00 2001 From: nfmohit Date: Sun, 19 Jun 2022 15:53:23 +0600 Subject: [PATCH 09/12] Revert notice update as it is to be handled later with #5371. This reverts commit b088c96fbd342691c521cf9050f9998b1572b1bb. --- .../dashboard-sharing/DashboardSharingSettings/Notice.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js index f7f3e5866f9..f802c8993fb 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Notice.js @@ -51,7 +51,7 @@ export default function Notice() { isEditingManagement && createInterpolateElement( __( - 'By clicking Apply, you are giving other admins permission to manage view-only access on your behalf to data from the Google services you selected “Any admin signed in with Google” for.', + 'By clicking Apply, you are giving other admins permission to manage view-only access on your behalf to data from the Google services you selected “All Admins” for.', 'google-site-kit' ), { From b887e0f94e3e0bc99c119c9eaa4f910b2a7b0269 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Tue, 21 Jun 2022 14:55:16 +0600 Subject: [PATCH 10/12] Fix DS view-only information alignment. --- assets/js/components/ViewOnlyMenu/Service.js | 4 +++- .../components/global/_googlesitekit-view-only-menu.scss | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/assets/js/components/ViewOnlyMenu/Service.js b/assets/js/components/ViewOnlyMenu/Service.js index 92ecd69d39d..196a0723c97 100644 --- a/assets/js/components/ViewOnlyMenu/Service.js +++ b/assets/js/components/ViewOnlyMenu/Service.js @@ -53,7 +53,9 @@ export default function Service( { module } ) { return (
  • - + + + { name } diff --git a/assets/sass/components/global/_googlesitekit-view-only-menu.scss b/assets/sass/components/global/_googlesitekit-view-only-menu.scss index 61cb59bf3dd..9deb219d762 100644 --- a/assets/sass/components/global/_googlesitekit-view-only-menu.scss +++ b/assets/sass/components/global/_googlesitekit-view-only-menu.scss @@ -93,23 +93,28 @@ .googlesitekit-view-only-menu__service { display: flex; + gap: 8px; margin-bottom: 10px; svg { - margin-right: 8px; padding: 2px; width: 24px; } } + .googlesitekit-view-only-menu__service--icon { + flex: 0 0 24px; + } + .googlesitekit-view-only-menu__service--name { - flex: 1; + flex: 0 0 100px; font-size: 14px; margin-right: 8px; } .googlesitekit-view-only-menu__service--owner { color: $c-primary; + flex: 1; font-size: 12px; margin-left: auto; } From 488fdcda68be326f70924c1df4427adf59658233 Mon Sep 17 00:00:00 2001 From: nfmohit Date: Tue, 21 Jun 2022 16:22:00 +0600 Subject: [PATCH 11/12] Change 'Only Me' to 'Only me'. --- .../dashboard-sharing/DashboardSharingSettings/Module.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js index 32dd1935af9..3a16fdae70e 100644 --- a/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js +++ b/assets/js/components/dashboard-sharing/DashboardSharingSettings/Module.js @@ -61,7 +61,7 @@ const { useSelect, useDispatch } = Data; const viewAccessOptions = [ { value: 'owner', - label: __( 'Only Me', 'google-site-kit' ), + label: __( 'Only me', 'google-site-kit' ), }, { value: 'all_admins', From 4132310e423b3fc865155f96af87077ca8a41ce6 Mon Sep 17 00:00:00 2001 From: Maciej Date: Tue, 21 Jun 2022 12:40:09 +0200 Subject: [PATCH 12/12] Default the MDC Menu quickOpen attribute to true. --- assets/js/components/Menu.js | 1 + 1 file changed, 1 insertion(+) diff --git a/assets/js/components/Menu.js b/assets/js/components/Menu.js index 7f7ae2aefeb..c0f003856d5 100644 --- a/assets/js/components/Menu.js +++ b/assets/js/components/Menu.js @@ -75,6 +75,7 @@ const Menu = forwardRef( const menuComponent = new MDCMenu( menuRef.current ); menuComponent.listen( 'MDCMenu:selected', handleMenuSelected ); + menuComponent.quickOpen = true; setMenu( menuComponent );