Implement `<PublicationSelect>` component

[nfmohit]

Feature Description

A <PublicationSelect> component should be added for the Reader Revenue Manager module that presents the user with a list of available publications and allows them to select one of them. The selected publication should be stored in the data store within the publicationID module setting, with its onboarding state stored within the publicationOnboardingState module setting.

Screenshot for reference

• See relevant section in the design doc.
• See Figma designs

---

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

Acceptance criteria

• A PublicationSelect component should be added for the Reader Revenue Manager module that presents the user with a dropdown of all available publications, according to the Figma designs.
• Its label should be "Publication".
• Upon selecting a publication from the dropdown, data store values of three module settings should be updated:
  • publicationID: Should contain the publication ID of the selected publication.
  • publicationOnboardingState: Should contain the onboarding state of the selected publication.
  • publicationOnboardingStateLastSyncedAtMs: Should contain the current time, in the form of the number of milliseconds elapsed since the epoch.

Implementation Brief

• [ ] The analytics-4 ProperySelect component can be used as a reference point here.
• [ ] Create a new component in assets/js/module/reader-revenue-manager/components/common/PublicationSelect.js. It should accept the following props.
  • [ ] isDisabled - Flag whether Publication select should be in disabled/enable state.
  • [ ] classNames - Class names for Select component from googlesitekit-components.
  • [ ] onChange - Function to call when value in dropdown changes.
  • [ ] hasModuleAccess - Flag to tell if the user has current module access.
• [ ] Use the getPublications selector implemented in #8794 to get the list of publications. getPublications selector will only be called when hasModuleAccess is not false and isDisabled is not falsy.

    hasModuleAccess !== false && ! isDisabled
  

  • [ ] Add a new function isValidPublicationID in assets/js/modules/analytics-4/utils/validation.js which should check if publication ID is valid. A publication ID should be string of only alphabets and digits. This should be similar to isValidMeasurementID function here.
  • [ ] Add the onPublicationChange using useCallback which would be passed to onEnhancedChange prop of Select component from googlesitekit-components.
  • [ ] onPublicationChange should set the publicationID, publicationOnboardingState using set method from the create-settings-store i.e. setPublicationID and setPublicationOnboardingState. These values should be received from getPublications selector.
  • [ ] Also set publicationOnboardingStateLastSyncedAtMs using same method, it should contain the current time, in the form of the number of milliseconds elapsed since the epoch.
  • [ ] Pass the label as Publication to Select component.
  • [ ] Render a Select component with no publications (other than if one is already selected) if hasModuleAccess is explicitly false, similar to the Analytics PropertySelect component.
  • [ ] We should display a loading ProgressBar component if the publications are loading. Take the reference of PropertySelect component here which does the same.
    • [ ] We can use the isResolving selector to check if the resolver for getPublications is still running.

Test Coverage

• [ ] Add tests for the component with different props and combinations.

QA Brief

Changelog entry

[nfmohit]

Thank you for the IB, @ankitrox ! Please take a look at my comments below:

• Update the getPublications selector to also retrieve onboardingState in publication object. This should already be received from GET:publications endpoint.

Let's remove this requirement as this property should already be a part of the getPublications selector. See this comment.

---

I think it might be beneficial to use the Analytics PropertySelect component as a starting point. See points below:

1. Let's mention the usage of the isDisabled and hasModuleAccess props. The getPublications selector should only be called if hasModuleAccess isn't explicitly false, and isDisabled isn't falsy, similar to the Analytics PropertySelect component.
2. Let's add validation for the publication ID, i.e. isValidPublicationID function. This should be similar to the Analytics isValidMeasurementID function which checks that the value is a string which only contains alphabets and numbers.
3. Let's mention that we want to render a Select component with no publications (other than if one is already selected) if hasModuleAccess is explicitly false, similar to the Analytics PropertySelect component.
4. Let's mention that we want to display a loading progress bar when the publications are being resolved, again, similar to the Analytics PropertySelect component.

---

For Storybook stories, I don't think it'll be very valuable to add a Storybook story for this component alone. This will likely be included in the Storybook stories for the module setup and settings screens, so an individual story may seem redundant.

---

Let me know what you think and if you have any questions, thank you!

[ankitrox]

Thanks @nfmohit for reviewing the IB.

I have updated the IB as per the suggested points. Re-assigning to you for review.

Thanks again!

[nfmohit]

Thank you for the updates, @ankitrox ! I've made a few changes to the IB to reduce the back and forth, namely:

1. I've mentioned using Analytics PropertySelect as a reference point.
2. I've removed the proposal to update the getPublications selector for onboarding state, as that should be no longer required.
3. I made the criteria for isValidPublicationID a little more specific and mentioned that it should only accept alphabets and digits (no symbols).
4. I've mentioned that we can use the isResolving selector for getPublications to determine the loading state, instead of proposing to create a new selector. Working on a new selector unless necessary also includes adding test coverage for it, thus requiring more time and effort.

Please let me know if you have any questions. IB LGTM 👍 ✅

[kelvinballoo]

QA Update ⚠️

Few items to raise: ITEM 1: Copy The text description is currently 'This is just added as a placeholder component to assist with testing.' Based on figma, it should be 'Select your preferred publication to connect with Site Kit'. Is this going to be tackled under a different issue?

Implementation:

Figma:

---

ITEM 2: Publication ID Missing and dropdown size The dropdown should have included the Publication ID as well. Currently, it doesn't. Moreover, the size of the dropdown should be 300px wide, not 200px.

Implementation: Dropdown is 200px wide

Figma size is 300px:

---

ITEM 3: Value for publicationOnboardingStateLastSyncedAtMs The "publicationOnboardingStateLastSyncedAtMs" is always 0 regardless of the publication selected. It's also 0 after some time that I've selected the publication at the dropdown. Are we expecting any values besides 0?

---

ITEM 4: Spacing From the icon to the description text, it's supposed to be around 45px. Currently, it's just 14px.

Figma:

Implementation:

[ankitrox]

ITEM 1: Copy

As you can see from the discussion on previous PR, we just added the component along with the message so that it is convenience to test. We will test it thoroughly in #8800

ITEM 2: Publication ID Missing and dropdown size

Sorry for not including this. It looks like this was missed during our previous round. I have created a new PR #9073 to address the issue. Also, addressed the width issue in same PR, although I believe that we could have kept the width as is to maintain consistency along with other select components. We can check if there is any feedback in CR for the same.

ITEM 3: Value for publicationOnboardingStateLastSyncedAtMs

This is will be handled in #8796 here . The reason, it can't be seen in action as of now is because we are yet to integrate everything in setup screen. We can test this in #8800

ITEM 4: Spacing

As per the discussion mentioned previously, we can test all the styling things in #8800

[techanvil]

Back to you for another pass, @kelvinballoo.

Please note that with regard to Item 2, the dropdown size is derived from content. As per this https://github.com/google/site-kit-wp/pull/9073#discussion_r1691287607 (which you may have already seen):

... the Select dropdown takes its width from its content, and this should be understood to be reflected in the Figma design rather than a fixed minimum width.

[kelvinballoo]

QA update ✅

Noted on Item 1, 3 and 4. Tested item 2 and the dropdown is now reflecting the Publication ID accordingly, similar to the Publisher Central.

Also tested the following:

• New dropdown's label is "Publication".

• Running googlesitekit.data.select('modules/reader-revenue-manager').getSettings(); will give 0 if nothing is selected at the dropdown.

  

• If there a publication is selected, the details related to that publication will appear when running the script.

  

Moving ticket to Approval.

[techanvil]

Moving this back to execution to fix the RRM SetupMain story that developed an error as a result of its first PR.

[aaemnnosttv]

Thanks @techanvil , merged 👍