-
Notifications
You must be signed in to change notification settings - Fork 286
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
Implement settings edit screen for RRM #8841
Comments
Thank you for the IB, @ankitrox ! Please take a look at my comments below:
Besides when changes are being submitted, the
The settings form should also be shown even if the user doesn't have access. The "no access" scenario will be handled by the Please let me know if you have any questions on the above, thank you! |
Thanks @nfmohit for reviewing the IB. I've updated IB as per suggestion. Over to you for re-review. Thanks again! |
Thank you for the iteration here, @ankitrox ! This looks good to me. Note: I have also included a point to add Storybook stories and VRT references for the IB ✅ |
QA Update
|
QA Update ✅Finally, per the comment from @kuasha420 here: #8797 (comment) , it's now working. I was able to verify this ticket and it's looking good
Moving ticket to approval. RRM.works.mov |
Feature Description
The module settings edit screen should be added for the Reader Revenue Manager module. This should include a
<PublicationSelect>
component and<PublicationOnboardingStateNotice>
component.Screenshot for reference
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
<PublicationSelect>
component being implemented by #8837), followed by a notice that conveys the publication onboarding state (via the<PublicationOnboardingStateNotice>
component being implemented by #8838).Implementation Brief
assets/js/modules/reader-revenue-manager/components/settings/SettingsEdit.js
isDoingSubmitChanges
selector from theMODULES_READER_REVENUE_MANAGER
store to check this.hasModuleOwnershipOrAccess
fromCORE_MODULES
store. Passreader-revenue-manager
as parameter tohasModuleOwnershipOrAccess
. Take the reference ofSettingsEdit
component from analytics-4 module from here to check the access. Pass result ofhasModuleOwnershipOrAccess
tohasModuleAccess
prop ofPublicationSelect
.hasModuleOwnershipOrAccess
has not resolved yet, return theProgressBar
component.hasModuleOwnershipOrAccess
has resolved, return thediv
with class namegooglesitekit-setup-module
. It should wrapPublicationSelect
andPublicationOnboardingStateNotice
components as mentioned in AC.SettingsEdit
component.Test Coverage
SettingsEdit
component.QA Brief
Note 1: Ensure that you have publications setup in publisher center. Refer QAB of this issue on how to setup publications. Also, there should be at least two publications setup for testing this.
Note 2: Ensure that "Custom Site URL" is NOT set in "Tester Settings" (Site Kit by Google Tester plugin).
Activate the module.
Run the following command in browser console. Remember to replace publicationID value with your publication ID in publisher center. It can be found in publisher center URL.
Refresh the page.
Go to
Site Kit > Settings > Reader Revenue Manager
, click on "Edit".Initially there should be a progress bar which should be displayed until the publications have loaded. Once publications are loaded, there should be a dropdown for selecting publications, and a notice below it if the selected publication is in the "pending verification" or "action required" state (
PENDING_VERIFICATION
orONBOARDING_ACTION_REQUIRED
).Change the publication from the dropdown and the CTA should change from "Save" to "Confirm changes".
Click on "Confirm changes". Refresh the page and make sure that settings have been saved correctly and publication stays selected.
Changelog entry
The text was updated successfully, but these errors were encountered: