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

Improve publication onboarding state synchronization. #9469

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ describe( 'PublicationOnboardingStateNotice', () => {
.receiveGetSettings( {
publicationID: 'ABCDEFGH',
publicationOnboardingState: ONBOARDING_COMPLETE,
publicationOnboardingStateLastSyncedAtMs: 0,
} );

const { container } = render( <PublicationOnboardingStateNotice />, {
Expand Down Expand Up @@ -99,7 +98,6 @@ describe( 'PublicationOnboardingStateNotice', () => {
.receiveGetSettings( {
publicationID: 'ABCDEFGH',
publicationOnboardingState: publicationState,
publicationOnboardingStateLastSyncedAtMs: 0,
} );

const { container, getByText, waitForRegistry } = render(
Expand Down Expand Up @@ -168,7 +166,7 @@ describe( 'PublicationOnboardingStateNotice', () => {
.receiveGetSettings( {
publicationID: 'QRSTUVWX',
publicationOnboardingState: ONBOARDING_ACTION_REQUIRED,
publicationOnboardingStateLastSyncedAtMs: 0,
publicationOnboardingStateChanged: false,
} );

fetchMock.getOnce( publicationsEndpoint, {
Expand Down Expand Up @@ -214,7 +212,7 @@ describe( 'PublicationOnboardingStateNotice', () => {
data: {
publicationID: 'QRSTUVWX',
publicationOnboardingState: ONBOARDING_COMPLETE,
publicationOnboardingStateLastSyncedAtMs: Date.now(),
publicationOnboardingStateChanged: false,
},
},
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,6 @@ describe( 'PublicationSelect', () => {

await waitForRegistry();

// Verify that the onboarding state last synced timestamp is
// not set at this point.
expect(
registry
.select( MODULES_READER_REVENUE_MANAGER )
.getPublicationOnboardingStateLastSyncedAtMs()
).toBeUndefined();

// Click the label to expose the elements in the menu.
fireEvent.click( container.querySelector( '.mdc-floating-label' ) );
// Click this element to select it and fire the onChange event.
Expand All @@ -152,14 +144,6 @@ describe( 'PublicationSelect', () => {

expect( publicationID ).toEqual( newPublicationID );
expect( onboardingState ).toEqual( newOnboardingState );

// Verify that the onboarding state last synced timestamp is
// set to a positive integer after selection.
expect(
registry
.select( MODULES_READER_REVENUE_MANAGER )
.getPublicationOnboardingStateLastSyncedAtMs()
).toBeGreaterThan( 0 );
}
);
} );
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
/**
* WordPress dependencies
*/
import { useEffect, useRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

/**
Expand All @@ -40,15 +41,35 @@ import { VIEW_CONTEXT_MAIN_DASHBOARD } from '../../../../googlesitekit/constants
import {
MODULES_READER_REVENUE_MANAGER,
UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION,
PUBLICATION_ONBOARDING_STATES,
} from '../../datastore/constants';

const { ONBOARDING_COMPLETE } = PUBLICATION_ONBOARDING_STATES;

export const RRM_PUBLICATION_APPROVED_OVERLAY_NOTIFICATION =
'rrmPublicationApprovedOverlayNotification';

export default function PublicationApprovedOverlayNotification() {
const viewContext = useViewContext();
const isViewOnly = useViewOnly();
const dashboardType = useDashboardType();
const { saveSettings, setPublicationOnboardingStateChanged } = useDispatch(
MODULES_READER_REVENUE_MANAGER
);
const { publicationOnboardingState, publicationOnboardingStateChanged } =
useSelect(
( select ) =>
select( MODULES_READER_REVENUE_MANAGER ).getSettings() || {}
);

// Check if settings has been resolved.
const hasSettingsResolved = useSelect( ( select ) =>
select( MODULES_READER_REVENUE_MANAGER ).hasFinishedResolution(
'getSettings'
)
);

const initialPublicationOnboardingStateChanged = useRef( undefined );

const isDismissed = useSelect( ( select ) =>
select( CORE_USER ).isItemDismissed(
Expand All @@ -73,12 +94,16 @@ export default function PublicationApprovedOverlayNotification() {
* - The notification UI is enabled.
* - The user is not in view-only mode.
* - The user is on the main dashboard.
* - The publication onboarding state has changed.
* - The publication onboarding state is complete.
*/
const shouldShowNotification =
isDismissed === false &&
showApprovedNotificationUI === true &&
! isViewOnly &&
dashboardType === VIEW_CONTEXT_MAIN_DASHBOARD;
dashboardType === VIEW_CONTEXT_MAIN_DASHBOARD &&
( showApprovedNotificationUI === true ||
( initialPublicationOnboardingStateChanged.current === true &&
publicationOnboardingState === ONBOARDING_COMPLETE ) );

const isDismissing = useSelect( ( select ) =>
select( CORE_USER ).isDismissingItem(
Expand All @@ -103,6 +128,24 @@ export default function PublicationApprovedOverlayNotification() {
} );
};

// In useEffect, set publicationOnboardingStateChanged to false using setPublicationOnboardingStateChanged method and save the setting using saveSettings action. This effect should be run only once when component is mounted.
useEffect( () => {
if ( initialPublicationOnboardingStateChanged.current === undefined ) {
initialPublicationOnboardingStateChanged.current =
publicationOnboardingStateChanged;

if ( publicationOnboardingStateChanged === true ) {
setPublicationOnboardingStateChanged( false );
saveSettings();
}
}
}, [
hasSettingsResolved,
publicationOnboardingStateChanged,
saveSettings,
setPublicationOnboardingStateChanged,
] );

return (
<OverlayNotification
className="googlesitekit-reader-revenue-manager-publication-approved-notification"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@
import PublicationApprovedOverlayNotification from './PublicationApprovedOverlayNotification';
import WithRegistrySetup from '../../../../../../tests/js/WithRegistrySetup';
import { CORE_UI } from '../../../../googlesitekit/datastore/ui/constants';
import { UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION } from '../../datastore/constants';
import {
MODULES_READER_REVENUE_MANAGER,
UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION,
} from '../../datastore/constants';
import { VIEW_CONTEXT_MAIN_DASHBOARD } from '../../../../googlesitekit/constants';
import { Provider as ViewContextProvider } from '../../../../components/Root/ViewContextContext';

Expand All @@ -45,6 +48,12 @@ export default {
decorators: [
( Story, { args } ) => {
const setupRegistry = ( registry ) => {
registry
.dispatch( MODULES_READER_REVENUE_MANAGER )
.receiveGetSettings( {
publicationOnboardingState: 'ONBOARDING_COMPLETE',
publicationOnboardingStateChanged: true,
} );
// Set the UI key to true to show the overlay notification.
registry
.dispatch( CORE_UI )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ import {
} from '../../../../googlesitekit/constants';
import * as tracking from '../../../../util/tracking';
import { Provider as ViewContextProvider } from '../../../../components/Root/ViewContextContext';
import { UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION } from '../../datastore/constants';
import {
MODULES_READER_REVENUE_MANAGER,
UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION,
} from '../../datastore/constants';
import { CORE_USER } from '../../../../googlesitekit/datastore/user/constants';

const mockTrackEvent = jest.spyOn( tracking, 'trackEvent' );
Expand All @@ -49,6 +52,10 @@ describe( 'PublicationApprovedOverlayNotification', () => {
'^/google-site-kit/v1/core/user/data/dismiss-item'
);

const settingsEndpoint = new RegExp(
'^/google-site-kit/v1/modules/reader-revenue-manager/data/settings'
);

beforeEach( () => {
mockTrackEvent.mockClear();
registry = createTestRegistry();
Expand All @@ -58,6 +65,20 @@ describe( 'PublicationApprovedOverlayNotification', () => {
UI_KEY_READER_REVENUE_MANAGER_SHOW_PUBLICATION_APPROVED_NOTIFICATION,
true
);

registry
.dispatch( MODULES_READER_REVENUE_MANAGER )
.receiveGetSettings( {
publicationOnboardingState: 'ONBOARDING_COMPLETE',
publicationOnboardingStateChanged: true,
} );

fetchMock.postOnce( settingsEndpoint, ( _url, opts ) => {
const { data } = JSON.parse( opts.body );

// Return the same settings passed to the API.
return { body: data, status: 200 };
} );
} );

it( 'should render the component with correct title and description', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -214,20 +214,14 @@ describe( 'RRMSetupSuccessSubtleNotification', () => {
);

it( 'should sync onboarding state when the window is refocused 15 seconds after clicking the CTA', async () => {
const originalDateNow = Date.now;

// Mock the date to be an arbitrary time.
const mockNow = new Date( '2020-01-01 12:30:00' ).getTime();
Date.now = jest.fn( () => mockNow );

jest.useFakeTimers();

registry
.dispatch( MODULES_READER_REVENUE_MANAGER )
.receiveGetSettings( {
publicationID: 'QRSTUVWX',
publicationOnboardingState: ONBOARDING_ACTION_REQUIRED,
publicationOnboardingStateLastSyncedAtMs: 0,
publicationOnboardingStateChanged: false,
} );

fetchMock.getOnce( publicationsEndpoint, {
Expand Down Expand Up @@ -289,7 +283,7 @@ describe( 'RRMSetupSuccessSubtleNotification', () => {
data: {
publicationID: 'QRSTUVWX',
publicationOnboardingState: ONBOARDING_COMPLETE,
publicationOnboardingStateLastSyncedAtMs: Date.now(),
publicationOnboardingStateChanged: false,
},
},
} );
Expand Down Expand Up @@ -317,8 +311,5 @@ describe( 'RRMSetupSuccessSubtleNotification', () => {
'Your Reader Revenue Manager account was successfully set up, but your publication still requires further setup in Reader Revenue Manager.'
)
).not.toBeInTheDocument();

// Restore Date.now method.
Date.now = originalDateNow;
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,6 @@ export default {
publicationID: publications[ 0 ].publicationId,
publicationOnboardingState:
publications[ 0 ].onboardingState,
publicationOnboardingStateLastSyncedAtMs: 0,
};

registry
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ describe( 'SettingsEdit', () => {
.receiveGetSettings( {
publicationID,
publicationOnboardingState,
publicationOnboardingStateLastSyncedAtMs: 0,
ownerID: 1,
} );
} );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ export default {
ownerID: 1,
publicationID: 'ABCDEFGH',
publicationOnboardingState: '',
publicationOnboardingStateLastSyncedAtMs: 0,
};

registry
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ describe( 'SettingsView', () => {
ownerID: 1,
publicationID,
publicationOnboardingState: publicationState,
publicationOnboardingStateLastSyncedAtMs: 0,
} );

const { getByText, waitForRegistry } = render( <SettingsView />, {
Expand All @@ -128,7 +127,6 @@ describe( 'SettingsView', () => {
ownerID: 2,
publicationID,
publicationOnboardingState: ONBOARDING_ACTION_REQUIRED,
publicationOnboardingStateLastSyncedAtMs: 0,
} );

registry
Expand Down
2 changes: 1 addition & 1 deletion assets/js/modules/reader-revenue-manager/datastore/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ export default Modules.createModuleStore( 'reader-revenue-manager', {
'ownerID',
'publicationID',
'publicationOnboardingState',
'publicationOnboardingStateLastSyncedAtMs',
'publicationOnboardingStateChanged',
],
} );
Loading
Loading