Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhancement/8976 Refactor Gathering Data Notification #9126

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
84128f2
Remove default comment.
jimmymadon Jul 29, 2024
a51db65
Move gathering data notification check to notification registration.
jimmymadon Aug 1, 2024
0858770
Add a reusable constant for view only contexts.
jimmymadon Aug 1, 2024
2726f72
Check view only when computing gathering data logic.
jimmymadon Aug 1, 2024
d8efce3
Add a reusable hook to compute gathering and zero data for modules.
jimmymadon Aug 1, 2024
b1eaf71
Make gathering data notification component standalone & sufficient.
jimmymadon Aug 1, 2024
af3f159
Move the Gathering Data Notification as a standalone component.
jimmymadon Aug 2, 2024
b0f6e71
Register the new GatheringDataNotification standalone component.
jimmymadon Aug 4, 2024
cea797d
Make ZeroDataNotification standalone in its checks.
jimmymadon Aug 4, 2024
f9fa7d5
Add notifications component to render the first queued notification.
jimmymadon Aug 4, 2024
9ccd86a
Move the ZeroDataNotification as a standalone component.
jimmymadon Aug 4, 2024
3149f6f
Fix checkRequirements to return bools/promises instead of functions.
jimmymadon Aug 4, 2024
b303e65
Fix priority of ZeroDataNotification.
jimmymadon Aug 4, 2024
74b60a9
Add new story for GatheringDataNotification.
jimmymadon Aug 4, 2024
be62887
Add new story for ZeroDataNotification.
jimmymadon Aug 4, 2024
fb4a683
Remove ZeroDataNotifications combined component.
jimmymadon Aug 4, 2024
664c986
Remove old ZeroDataNotifications stories.
jimmymadon Aug 4, 2024
39f349e
Fix existing BannerNotifications tests.
jimmymadon Aug 4, 2024
b521c74
Merge branch 'develop' into enhancement/8976-refactor-gathering-data-…
jimmymadon Aug 6, 2024
f9251af
Ensure the ZeroDataNotification does not render in gathering data state.
jimmymadon Aug 6, 2024
00f3f12
Fix tests that were returning false positives for ZeroDataNotification.
jimmymadon Aug 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 23 additions & 3 deletions assets/js/components/notifications/BannerNotifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,20 @@ import ModuleRecoveryAlert from '../dashboard-sharing/ModuleRecoveryAlert';
import AdSenseAlerts from './AdSenseAlerts';
import EnhancedMeasurementActivationBanner from '../../modules/analytics-4/components/dashboard/EnhancedMeasurementActivationBanner';
import useViewOnly from '../../hooks/useViewOnly';
import ZeroDataStateNotifications from './ZeroDataStateNotifications';
import useViewContext from '../../hooks/useViewContext';
import ZeroDataNotification from './ZeroDataNotification';
import EnableAutoUpdateBannerNotification from './EnableAutoUpdateBannerNotification';
import GoogleTagIDMismatchNotification from './GoogleTagIDMismatchNotification';
import WebDataStreamNotAvailableNotification from './WebDataStreamNotAvailableNotification';
import AdBlockingRecoverySetupSuccessBannerNotification from './AdBlockingRecoverySetupSuccessBannerNotification';
import { CORE_UI } from '../../googlesitekit/datastore/ui/constants';
import { UI_KEY_KEY_METRICS_SETUP_CTA_RENDERED } from '../KeyMetrics/KeyMetricsSetupCTARenderedEffect';
import { NOTIFICATION_AREAS } from '../../googlesitekit/notifications/datastore/constants';
import Notifications from './Notifications';

export default function BannerNotifications() {
const viewOnly = useViewOnly();
const viewContext = useViewContext();

const isAuthenticated = useSelect( ( select ) =>
select( CORE_USER ).isAuthenticated()
Expand Down Expand Up @@ -90,7 +94,15 @@ export default function BannerNotifications() {
const [ slug ] = useQueryArg( 'slug' );

if ( viewOnly ) {
return <ZeroDataStateNotifications />;
return (
<Fragment>
<ZeroDataNotification />
<Notifications
viewContext={ viewContext }
areaSlug={ NOTIFICATION_AREAS.BANNERS_ABOVE_NAV }
/>
</Fragment>
);
}

return (
Expand All @@ -117,7 +129,15 @@ export default function BannerNotifications() {
<WebDataStreamNotAvailableNotification />
</Fragment>
) }
<ZeroDataStateNotifications />
<Notifications
viewContext={ viewContext }
areaSlug={ NOTIFICATION_AREAS.BANNERS_ABOVE_NAV }
/>
{ /* Temporary hack to give priority to the `GatheringDataNotification` component queued by `Notifications`.
This happens because `hasZeroData` selectors return true within `ZeroDataNotification` even if a module is
still gathering data. `ZeroDataNotification` should be refactored next which will make `Notifications`
the last component to be rendered. */ }
<ZeroDataNotification />
</Fragment>
);
}
45 changes: 45 additions & 0 deletions assets/js/components/notifications/BannerNotifications.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
* Internal dependencies
*/
import {
act,
createTestRegistry,
muteFetch,
provideModules,
Expand All @@ -37,6 +38,11 @@ import { mockLocation } from '../../../../tests/js/mock-browser-utils';
import BannerNotifications from './BannerNotifications';
import Header from '../Header';
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants';
import { VIEW_CONTEXT_MAIN_DASHBOARD } from '../../googlesitekit/constants';
import {
CORE_NOTIFICATIONS,
NOTIFICATION_AREAS,
} from '../../googlesitekit/notifications/datastore/constants';

describe( 'BannerNotifications', () => {
mockLocation();
Expand Down Expand Up @@ -94,6 +100,7 @@ describe( 'BannerNotifications', () => {
registry.dispatch( CORE_USER ).receiveGetSurvey( { survey: null } );
registry.dispatch( CORE_SITE ).receiveGetNotifications( [] );
registry.dispatch( CORE_USER ).receiveNonces( [] );
registry.dispatch( MODULES_ANALYTICS_4 ).receiveGetSettings( [] );
registry
.dispatch( MODULES_ANALYTICS_4 )
.receiveHasMismatchGoogleTagID( false );
Expand All @@ -107,6 +114,7 @@ describe( 'BannerNotifications', () => {
<BannerNotifications />,
{
registry,
viewContext: VIEW_CONTEXT_MAIN_DASHBOARD,
}
);

Expand All @@ -127,6 +135,7 @@ describe( 'BannerNotifications', () => {
<BannerNotifications />,
{
registry,
viewContext: VIEW_CONTEXT_MAIN_DASHBOARD,
}
);

Expand All @@ -138,6 +147,23 @@ describe( 'BannerNotifications', () => {
} );

it( 'prioritizes errors over alerts and regular notifications', async () => {
// Trigger the gathering data notification using the new registration process
act( () => {
registry
.dispatch( CORE_NOTIFICATIONS )
.registerNotification( 'gathering-data-notification', {
Component() {
return (
<div className="googlesitekit-publisher-win">
Test notification!
</div>
);
},
areaSlug: NOTIFICATION_AREAS.BANNERS_ABOVE_NAV,
viewContexts: [ VIEW_CONTEXT_MAIN_DASHBOARD ],
} );
} );

// Trigger the error notification.
provideUserAuthentication( registry, {
unsatisfiedScopes: [
Expand All @@ -157,6 +183,7 @@ describe( 'BannerNotifications', () => {
<Header subHeader={ <BannerNotifications /> } />,
{
registry,
viewContext: VIEW_CONTEXT_MAIN_DASHBOARD,
}
);

Expand All @@ -178,6 +205,23 @@ describe( 'BannerNotifications', () => {
} );

it( 'prioritizes alerts over regular notifications', async () => {
// Trigger the gathering data notification using the new registration process
act( () => {
registry
.dispatch( CORE_NOTIFICATIONS )
.registerNotification( 'gathering-data-notification', {
Component() {
return (
<div className="googlesitekit-publisher-win">
Test notification!
</div>
);
},
areaSlug: NOTIFICATION_AREAS.BANNERS_ABOVE_NAV,
viewContexts: [ VIEW_CONTEXT_MAIN_DASHBOARD ],
} );
} );

// Ensure setup completed notification is added.
global.location.href =
'http://example.com/wp-admin/admin.php?notification=authentication_success';
Expand All @@ -193,6 +237,7 @@ describe( 'BannerNotifications', () => {
<BannerNotifications />,
{
registry,
viewContext: VIEW_CONTEXT_MAIN_DASHBOARD,
}
);

Expand Down
17 changes: 15 additions & 2 deletions assets/js/components/notifications/EntityBannerNotifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,25 @@ import { Fragment } from '@wordpress/element';
/**
* Internal dependencies
*/
import ZeroDataNotifications from './ZeroDataStateNotifications';
import ZeroDataNotification from './ZeroDataNotification';
import Notifications from './Notifications';
import useViewContext from '../../hooks/useViewContext';
import { NOTIFICATION_AREAS } from '../../googlesitekit/notifications/datastore/constants';

export default function EntityBannerNotifications() {
const viewContext = useViewContext();

return (
<Fragment>
<ZeroDataNotifications />
<Notifications
viewContext={ viewContext }
areaSlug={ NOTIFICATION_AREAS.BANNERS_ABOVE_NAV }
/>
{ /* Temporary hack to give priority to the `GatheringDataNotification` component queued by `Notifications`.
This happens because `hasZeroData` selectors return true within `ZeroDataNotification` even if a module is
still gathering data. `ZeroDataNotification` should be refactored next which will make `Notifications`
the last component to be rendered. */ }
<ZeroDataNotification />
</Fragment>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,6 @@
* limitations under the License.
*/

/**
* External dependencies
*/
import PropTypes from 'prop-types';

/**
* WordPress dependencies
*/
Expand All @@ -31,16 +26,14 @@ import { useCallback } from '@wordpress/element';
* Internal dependencies
*/
import { useSelect } from 'googlesitekit-data';
import BannerNotification from '../BannerNotification';
import GatheringDataIcon from '../../../../svg/graphics/zero-state-red.svg';
import { CORE_SITE } from '../../../googlesitekit/datastore/site/constants';
import { DAY_IN_SECONDS, trackEvent } from '../../../util';
import useViewContext from '../../../hooks/useViewContext';
import BannerNotification from './BannerNotification';
import GatheringDataIcon from '../../../svg/graphics/zero-state-red.svg';
import { CORE_SITE } from '../../googlesitekit/datastore/site/constants';
import { DAY_IN_SECONDS, trackEvent } from '../../util';
import useViewContext from '../../hooks/useViewContext';
import useModuleGatheringZeroData from '../../hooks/useModuleGatheringZeroData';

export default function GatheringDataNotification( {
title,
gatheringDataWaitTimeInHours,
} ) {
export default function GatheringDataNotification() {
const viewContext = useViewContext();
const eventCategory = `${ viewContext }_gathering-data-notification`;
const handleOnView = useCallback( () => {
Expand All @@ -57,14 +50,40 @@ export default function GatheringDataNotification( {
select( CORE_SITE ).getAdminURL( 'googlesitekit-settings' )
);

const { analyticsGatheringData, searchConsoleGatheringData } =
useModuleGatheringZeroData();

let gatheringDataTitle;
// Analytics requires up to 72 hours to gather data.
let gatheringDataWaitTimeInHours = 72;
if ( analyticsGatheringData && searchConsoleGatheringData ) {
gatheringDataTitle = __(
'Search Console and Analytics are gathering data',
'google-site-kit'
);
} else if ( analyticsGatheringData ) {
gatheringDataTitle = __(
'Analytics is gathering data',
'google-site-kit'
);
} else if ( searchConsoleGatheringData ) {
// If only Search Console is gathering data, show a lower wait
// time, since it only requires 48 hours.
gatheringDataWaitTimeInHours = 48;
gatheringDataTitle = __(
'Search Console is gathering data',
'google-site-kit'
);
}

if ( ! gatheringDataWaitTimeInHours ) {
return null;
}

return (
<BannerNotification
id="gathering-data-notification"
title={ title }
title={ gatheringDataTitle }
description={ sprintf(
/* translators: %s: the number of hours the site can be in a gathering data state */
_n(
Expand All @@ -88,8 +107,3 @@ export default function GatheringDataNotification( {
/>
);
}

GatheringDataNotification.propTypes = {
title: PropTypes.string,
gatheringDataWaitTimeInHours: PropTypes.number,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* GatheringDataNotification Component Stories.
*
* Site Kit by Google, Copyright 2024 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Internal dependencies
*/
import { provideModules } from '../../../../tests/js/utils';
import WithRegistrySetup from '../../../../tests/js/WithRegistrySetup';
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants';
import { MODULES_SEARCH_CONSOLE } from '../../modules/search-console/datastore/constants';
import GatheringDataNotification from './GatheringDataNotification';

function Template( { setupRegistry } ) {
return (
<WithRegistrySetup func={ setupRegistry }>
<GatheringDataNotification />
</WithRegistrySetup>
);
}

export const AnalyticsGatheringData = Template.bind( {} );
AnalyticsGatheringData.storyName = 'Analytics Gathering Data';
AnalyticsGatheringData.args = {
setupRegistry: ( registry ) => {
registry
.dispatch( MODULES_SEARCH_CONSOLE )
.receiveIsGatheringData( false );
registry.dispatch( MODULES_ANALYTICS_4 ).receiveIsGatheringData( true );
},
};

export const SearchConsoleGatheringData = Template.bind( {} );
SearchConsoleGatheringData.storyName = 'Search Console Gathering Data';
SearchConsoleGatheringData.args = {
setupRegistry: ( registry ) => {
registry
.dispatch( MODULES_SEARCH_CONSOLE )
.receiveIsGatheringData( true );
registry
.dispatch( MODULES_ANALYTICS_4 )
.receiveIsGatheringData( false );
},
};

export const SearchConsoleAndAnalyticsGatheringData = Template.bind( {} );
SearchConsoleAndAnalyticsGatheringData.storyName =
'Search Console And Analytics Gathering Data';
SearchConsoleAndAnalyticsGatheringData.args = {
setupRegistry: ( registry ) => {
registry
.dispatch( MODULES_SEARCH_CONSOLE )
.receiveIsGatheringData( true );
registry.dispatch( MODULES_ANALYTICS_4 ).receiveIsGatheringData( true );
},
};

export default {
title: 'Components/Notifications/GatheringDataNotification',
decorators: [
( Story, { args } ) => {
const setupRegistry = ( registry ) => {
provideModules( registry, [
{
active: true,
connected: true,
slug: 'search-console',
},
{
active: true,
connected: true,
slug: 'analytics-4',
},
] );

// Call story-specific setup.
args.setupRegistry( registry );
};

return (
<WithRegistrySetup func={ setupRegistry }>
<Story />
</WithRegistrySetup>
);
},
],
};
Loading
Loading