google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.22k stars 277 forks source link

Scaffold RRM module setup flow #8800

Open nfmohit opened 1 month ago

nfmohit commented 1 month ago

Feature Description

The Reader Revenue Manager module setup flow has different states based on the number of publications the user has. This issue should handle setting the module setup screen and placeholder components for each state.


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

  1. Add tests no publication, one publication and multiple publication.
  2. Add tests for scenario by setting the form value.
  3. Add stories for component for all the scenarios.

QA Brief

Changelog entry

nfmohit commented 1 week ago

Note: Added a new point in the ACs:

  • This change in the view component should be permanent, i.e. once the user is shown the <PublicationCreate> component, they shouldn't be shown the content below even though Site Kit detects that publications exist later. This is crucial to ensure that the two steps in the <PublicationCreate> component that appear based on the availability of publications function correctly. This can be done by using a new form key in the core/forms data store.

Back to IB again.

nfmohit commented 6 days ago

Excellent IB, thanks @ankitrox ! Please take a look at my comments below:


  • Add constant READER_REVENUE_MANAGER_FORM with value readerRevenueManagerFormData in assets/js/modules/reader-revenue-manager/datastore/constants.js.
  1. Let's propose this constant to be READER_REVENUE_MANAGER_SETUP_FORM.
  2. Let's add this as a top-level item.
  • If there are no publications available, set the form value using setValues function from core/forms store. Pass form name as READER_REVENUE_MANAGER_FORM and value as an object as following
  {
    publicationCreateRendered: true
  }

Let's change the object key to be something more meaningful, such as showPublicationCreateForm which is added as the value for a new constant SHOW_PUBLICATION_CREATE_FORM in assets/js/modules/reader-revenue-manager/datastore/constants.js.

  • If there are no publications available assign PublicationCreate component to viewComponent.

Instead of using the number of publications to show PublicationCreate, let's simplify this and only rely on the CORE_FORM datastore value, i.e. "_Assign PublicationCreate component to viewComponent if SHOW_PUBLICATION_CREATE_FORM value is true_".

  • Display the "Create Publication" CTA should use Button component with href prop passed to it. The link should be be passed using getServiceURL selector

The "Create publication" CTA inside PublicationCreate will be added as part of #8836, we do not need to include it here.

However, the PublicationCreate component will need an onCompleteSetup prop (see the IB for #8836), which is a callback that should dispatch the submitChanges action and call the finishSetup function.

If there are publications available, but value of publicationCreateRendered is true in CORE_FORMS store for READER_REVENUE_MANAGER_FORM form name, assign PublicationCreate component to viewComponent. This will be useful to display the second step of PublicationCreate component.

  • [ ] Clicking on the Complete setup should call the saveSettings on RRM module store and call the finishSetup function.

This bit is not necessary, as mentioned above, we should always render PublicationCreate when SHOW_PUBLICATION_CREATE_FORM value is true.

  • Make use findMatchedPublication action to find the suitable publication in case there is no publication ID available already and multiple publications are available. If the publication is returned by the action, set it using setPublicationID action so that PublicationSelect component can retrieve it and display it as selected one.

Let's also set the publicationOnboardingState and publicationOnboardingStateLastSyncedAtMs module settings similar to PublicationSelect.

  • Clicking on the Complete setup should call the saveSettings on RRM module store and call the finishSetup function.

In order to save settings in the setup/settings screens, we should ideally use the submitChanges action, which checks if any changes have been actually made first.


Please let me know if you have any questions on the above, thank you!

ankitrox commented 3 days ago

Thanks for reviewing the IB @nfmohit and your valuable feedback.

I've made the changes as per the suggestions. Over to you for re-review.

Thanks again!

nfmohit commented 2 days ago

Thank you for the iteration here, @ankitrox ! This looks good to me. Note: I've removed the requirement to set publicationOnboardingStateLastSyncedAtMs as it will be set anyway almost immediately as part of maybeSyncPublicationOnboardingState action.

IB ✅