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

Check for gathering data state in Search Console and Analytics before showing User Input notification. #7198

Merged
merged 25 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
dd207d9
Check for gathering data state in Search Console and Analytics before…
tofumatt Jun 23, 2023
cb98195
Merge branch 'develop' into fix/6607.
10upsimon Jun 29, 2023
cae57ed
Merge branch 'develop' of github.com:google/site-kit-wp into develop.
10upsimon Jul 5, 2023
c18a364
Merge branch 'develop' of github.com:google/site-kit-wp into develop.
10upsimon Jul 6, 2023
d5da3b7
Merge branch develop into fix/6607.
10upsimon Jul 6, 2023
4b4b670
Return null from KeyMetricsSetupCTAWidget when gathering data.
techanvil Jul 6, 2023
3147fd6
Reorder conditions for consistency.
techanvil Jul 6, 2023
90b011e
Fix E2E test, providing mock data to avoid "gathering data" state.
techanvil Jul 6, 2023
b04538f
Add an explanatory comment.
techanvil Jul 6, 2023
b6687c8
Tidy up.
techanvil Jul 6, 2023
023f282
Update paths for consistency.
techanvil Jul 7, 2023
0d7a974
Fix KeyMetricsSetupCTAWidget stories.
techanvil Jul 7, 2023
642b6f7
Prevent warnings in Storybook due to unhandled POST to data-available…
techanvil Jul 7, 2023
75fa2df
Update regexes for consistency.
techanvil Jul 7, 2023
6b1d719
Fix tests for KeyMetricsSetupCTAWidget.
techanvil Jul 7, 2023
43321a6
Update tests to use getWidgetComponentProps.
techanvil Jul 10, 2023
334d63b
Merge branch 'develop' into fix/6607.
techanvil Jul 10, 2023
efa0d9f
Add tests for when SC/GA4 are gathering data.
techanvil Jul 10, 2023
59cbdeb
Add provideGatheringDataState utility function.
techanvil Jul 10, 2023
13200cb
Refactor tests and story to use provideGatheringDataState helper.
techanvil Jul 10, 2023
3c52eea
Add JSDoc for new utility helpers.
techanvil Jul 10, 2023
405e91e
Remove unneeded calls to set reference date.
techanvil Jul 10, 2023
6658567
Rename variable.
techanvil Jul 10, 2023
0442858
Remove unnecessary setting of global data.
techanvil Jul 10, 2023
4bc8ad5
Rename variable.
techanvil Jul 10, 2023
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
17 changes: 15 additions & 2 deletions .storybook/fetch-mocks.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,21 @@ export function bootstrapFetchMocks() {
// Reset first to prevent errors when hot reloading.
fetchMock.reset();
fetchMockSaveSettings();
fetchMockSaveDataAvailable();
fetchMockGetModules();
fetchMockCatchAll();
}

export function fetchMockGetModules() {
fetchMock.get( /\/google-site-kit\/v1\/core\/modules\/data\/list/, {
fetchMock.get( new RegExp( '/google-site-kit/v1/core/modules/data/list' ), {
body: [],
status: 200,
} );
}

export function fetchMockSaveSettings() {
fetchMock.post(
/\/google-site-kit\/v1\/modules\/[\w-]+\/data\/settings/,
new RegExp( '/google-site-kit/v1/modules/[\\w-]+/data/settings' ),
( url, opts ) => {
const { data } = JSON.parse( opts.body );
return {
Expand All @@ -49,6 +50,18 @@ export function fetchMockSaveSettings() {
);
}

export function fetchMockSaveDataAvailable() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is added to prevent warnings in stories about POSTing to the data-available endpoint once the gathering data state has been determined.

fetchMock.post(
new RegExp( '/google-site-kit/v1/modules/[\\w-]+/data/data-available' ),
() => {
return {
status: 200,
body: true,
};
}
);
}

// Catch-all mock to log any other requests.
export function fetchMockCatchAll() {
fetchMock.catch( ( url, opts ) => {
Expand Down
16 changes: 15 additions & 1 deletion assets/js/components/KeyMetrics/KeyMetricsSetupCTAWidget.js
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note, UserInputSettings has been removed and the specified change needs to be applied to KeyMetricsSetupCTAWidget instead. See this PR and related comment for more details.

Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ import { Cell, Grid, Row } from '../../material-components';
import GhostCardsSVG from './GhostCards';
import { CORE_MODULES } from '../../googlesitekit/modules/datastore/constants';
import { CORE_USER } from '../../googlesitekit/datastore/user/constants';
import { MODULES_SEARCH_CONSOLE } from '../../modules/search-console/datastore/constants';
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants';
import { useFeature } from '../../hooks/useFeature';
import Link from '../Link';
import { CORE_SITE } from '../../googlesitekit/datastore/site/constants';
Expand All @@ -56,6 +58,16 @@ function KeyMetricsSetupCTAWidget( { Widget, WidgetNull } ) {
const ctaLink = useSelect( ( select ) =>
select( CORE_SITE ).getAdminURL( 'googlesitekit-user-input' )
);
const searchConsoleIsGatheringData = useSelect(
( select ) =>
searchConsoleModuleConnected &&
select( MODULES_SEARCH_CONSOLE ).isGatheringData()
);
const analyticsIsGatheringData = useSelect(
( select ) =>
analyticsModuleConnected &&
select( MODULES_ANALYTICS_4 ).isGatheringData()
);

const breakpoint = useBreakpoint();
const isMobileBreakpoint = breakpoint === BREAKPOINT_SMALL;
Expand All @@ -65,7 +77,9 @@ function KeyMetricsSetupCTAWidget( { Widget, WidgetNull } ) {
isUserInputCompleted === undefined ||
isUserInputCompleted ||
! analyticsModuleConnected ||
! searchConsoleModuleConnected
! searchConsoleModuleConnected ||
analyticsIsGatheringData !== false ||
searchConsoleIsGatheringData !== false
) {
return <WidgetNull />;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,17 @@
/**
* Internal dependencies
*/
import { provideModules } from '../../../../tests/js/utils';
import { withWidgetComponentProps } from '../../googlesitekit/widgets/util';
import {
provideModules,
provideUserAuthentication,
} from '../../../../tests/js/test-utils';
import WithRegistrySetup from '../../../../tests/js/WithRegistrySetup';
import { getSearchConsoleMockResponse } from '../../modules/search-console/util/data-mock';
import { getAnalytics4MockResponse } from '../../modules/analytics-4/utils/data-mock';
import { CORE_USER } from '../../googlesitekit/datastore/user/constants';
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants';
import { MODULES_SEARCH_CONSOLE } from '../../modules/search-console/datastore/constants';
import { withWidgetComponentProps } from '../../googlesitekit/widgets/util';
import KeyMetricsSetupCTAWidget from './KeyMetricsSetupCTAWidget';

const WidgetWithComponentProps = withWidgetComponentProps(
Expand All @@ -30,6 +38,19 @@ const WidgetWithComponentProps = withWidgetComponentProps(

const Template = () => <WidgetWithComponentProps />;

const searchConsoleReportOptions = {
dimensions: 'date',
startDate: '2020-07-14',
endDate: '2020-09-07',
};

const analytics4ReportOptions = {
dimensions: [ 'date' ],
metrics: [ { name: 'totalUsers' } ],
startDate: '2020-08-11',
endDate: '2020-09-07',
};

export const Default = Template.bind( {} );
Default.storyName = 'SetupCTAWidget';
Default.scenario = {
Expand All @@ -53,6 +74,29 @@ export default {
connected: true,
},
] );
provideUserAuthentication( registry );

registry.dispatch( CORE_USER ).setReferenceDate( '2020-09-08' );

// Provide reports to ensure "gathering data" is false for Analytics 4 and Search Console modules.
registry
.dispatch( MODULES_ANALYTICS_4 )
.receiveGetReport(
getAnalytics4MockResponse( analytics4ReportOptions ),
{
options: analytics4ReportOptions,
}
);
registry
.dispatch( MODULES_SEARCH_CONSOLE )
.receiveGetReport(
getSearchConsoleMockResponse(
searchConsoleReportOptions
),
{
options: searchConsoleReportOptions,
}
);
nfmohit marked this conversation as resolved.
Show resolved Hide resolved
};

return (
Expand Down
61 changes: 61 additions & 0 deletions assets/js/components/KeyMetrics/KeyMetricsSetupCTAWidget.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,34 @@
*/
import KeyMetricsSetupCTAWidget from './KeyMetricsSetupCTAWidget';
import { CORE_USER } from '../../googlesitekit/datastore/user/constants';
import { MODULES_ANALYTICS_4 } from '../../modules/analytics-4/datastore/constants';
import { MODULES_SEARCH_CONSOLE } from '../../modules/search-console/datastore/constants';
import { getAnalytics4MockResponse } from '../../modules/analytics-4/utils/data-mock';
import { getSearchConsoleMockResponse } from '../../modules/search-console/util/data-mock';
import {
render,
createTestRegistry,
provideModules,
provideSiteInfo,
muteFetch,
} from '../../../../tests/js/test-utils';

describe( 'KeyMetricsSetupCTAWidget', () => {
nfmohit marked this conversation as resolved.
Show resolved Hide resolved
let registry;

const searchConsoleReportOptions = {
dimensions: 'date',
startDate: '2020-07-14',
endDate: '2020-09-07',
};

const analytics4ReportOptions = {
dimensions: [ 'date' ],
metrics: [ { name: 'totalUsers' } ],
startDate: '2020-08-11',
endDate: '2020-09-07',
};

beforeEach( async () => {
registry = createTestRegistry();

Expand All @@ -39,12 +58,26 @@ describe( 'KeyMetricsSetupCTAWidget', () => {
.dispatch( CORE_USER )
.receiveIsUserInputCompleted( true );

registry.dispatch( CORE_USER ).setReferenceDate( '2020-09-08' );

fetchMock.getOnce(
new RegExp( '^/google-site-kit/v1/core/user/data/authentication' ),
{
authenticated: true,
}
);

muteFetch(
new RegExp(
'^/google-site-kit/v1/modules/analytics-4/data/data-available'
)
);

muteFetch(
new RegExp(
'^/google-site-kit/v1/modules/search-console/data/data-available'
)
);
} );

it( 'does not render when SC is not connected', async () => {
Expand Down Expand Up @@ -82,6 +115,16 @@ describe( 'KeyMetricsSetupCTAWidget', () => {
},
] );

// Provide reports to ensure "gathering data" is false for the Search Console module.
registry
.dispatch( MODULES_SEARCH_CONSOLE )
.receiveGetReport(
getSearchConsoleMockResponse( searchConsoleReportOptions ),
{
options: searchConsoleReportOptions,
}
);

const Widget = ( { children } ) => <div>{ children }</div>;
const WidgetNull = () => <div>NULL</div>;

Expand Down Expand Up @@ -118,6 +161,24 @@ describe( 'KeyMetricsSetupCTAWidget', () => {
},
] );

// Provide reports to ensure "gathering data" is false for Analytics 4 and Search Console modules.
registry
.dispatch( MODULES_ANALYTICS_4 )
.receiveGetReport(
getAnalytics4MockResponse( analytics4ReportOptions ),
{
options: analytics4ReportOptions,
}
);
registry
.dispatch( MODULES_SEARCH_CONSOLE )
.receiveGetReport(
getSearchConsoleMockResponse( searchConsoleReportOptions ),
{
options: searchConsoleReportOptions,
}
);

const Widget = ( { children } ) => <div>{ children }</div>;
const WidgetNull = () => <div>NULL</div>;

Expand Down
46 changes: 45 additions & 1 deletion tests/e2e/specs/user-input-questions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@
* limitations under the License.
*/

/**
* External dependencies
*/
import { set } from 'lodash';

/**
* WordPress dependencies
*/
Expand All @@ -37,6 +42,8 @@ import {
setupAnalytics,
setupAnalytics4,
} from '../utils';
import { getAnalytics4MockResponse } from '../../../assets/js/modules/analytics-4/utils/data-mock';
import { getSearchConsoleMockResponse } from '../../../assets/js/modules/search-console/util/data-mock';

describe( 'User Input Settings', () => {
async function fillInInputSettings() {
Expand Down Expand Up @@ -95,13 +102,50 @@ describe( 'User Input Settings', () => {
);
}

function getMultiDimensionalObjectFromParams( params ) {
return Object.entries( params ).reduce( ( acc, [ key, value ] ) => {
set( acc, key, value );
return acc;
}, {} );
}

beforeAll( async () => {
await page.setRequestInterception( true );

useRequestInterception( ( request ) => {
const url = request.url();

if ( url.match( '/google-site-kit/v1/modules' ) ) {
const paramsObject = Object.fromEntries(
new URL( url ).searchParams.entries()
);

// Provide mock data for Analytics 4 and Search Console requests to ensure they are not in the "gathering data" state.
if (
url.match(
'/google-site-kit/v1/modules/analytics-4/data/report?'
)
) {
request.respond( {
status: 200,
body: JSON.stringify(
getAnalytics4MockResponse(
// Some of the keys are nested paths e.g. `metrics[0][name]`, so we need to convert the search params to a multi-dimensional object.
getMultiDimensionalObjectFromParams( paramsObject )
)
),
Comment on lines +130 to +135
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏆 👍

} );
} else if (
url.match(
'/google-site-kit/v1/modules/search-console/data/searchanalytics?'
)
) {
request.respond( {
status: 200,
body: JSON.stringify(
getSearchConsoleMockResponse( paramsObject )
),
} );
} else if ( url.match( '/google-site-kit/v1/modules' ) ) {
request.respond( { status: 200 } );
} else {
request.continue();
Expand Down