-
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
Fix conditions for showing the publication approved overlay notification #9089
Comments
AC ✅ Thank you @techanvil ! |
The IB looks good to me, @ankitrox. Would you agree that it would be wise to increase the estimate to a |
Hi @nfmohit Thanks for flagging this. I had been conservative, but this exceeds the estimated time considering the fact that we may have to adjust test cases and may encounter some unforeseen scenarios. I have updated estimate to 7 as it sounds correct. Thanks again. |
Thank you @ankitrox ! IB ✅ |
QA Update: ✅Verified: Onboarding state setting to something other than ONBOARDING_COMPLETE.
Onboarding state setting to ONBOARDING_COMPLETE.
Refreshing the page and running the above script again.
Besides the above, I also performed a smoke test using the QAB for #8796 and #8797. I will be creating a more thorough smoke test of the entire RRM functionality as approaching the completion of the bug bash instructions. |
Feature Description
When the onboarding state for the current publication is synced, the publication approved overlay notification should appear, but only when the onboarding state has changed from another non-empty state.
At present the notification will appear even if the publication was already in the onboarding state prior to the sync. See https://github.com/google/site-kit-wp/pull/9017/files#r1693880442.
Do not alter or remove anything below. The following sections will be managed by moderators only.
Acceptance criteria
Implementation Brief
assets/js/modules/reader-revenue-manager/datastore/publications.js
, update following conditionsite-kit-wp/assets/js/modules/reader-revenue-manager/datastore/publications.js
Lines 131 to 134 in e72c216
add following check along with existing one
Test coverage
QA Brief
rrmModule
feature flag, and change the "_Force Reader Revenue Manager publication onboarding state_" setting to something other than
ONBOARDING_COMPLETE
.ONBOARDING_COMPLETE
.syncPublicationOnboardingState()
action #8796 and Implement RRMmaybeSyncPublicationOnboardingState()
action #8797, as this includes a few improvements regarding those areas.Changelog entry
The text was updated successfully, but these errors were encountered: