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.

Sign up for GitHub

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

Jump to bottom

Handle error if the saved product ID is no longer available #10131

Closed
4 of 5 tasks
nfmohit opened this issue Jan 29, 2025 · 7 comments
Closed
4 of 5 tasks

Handle error if the saved product ID is no longer available #10131

nfmohit opened this issue Jan 29, 2025 · 7 comments
Labels
Good First Issue Good first issue for new engineers Module: RRM Reader Revenue Manager module related issues P0 High priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature

Comments

@nfmohit
Copy link
Collaborator

nfmohit commented Jan 29, 2025 •
edited by techanvil
Loading

Feature Description

The Reader Revenue Manager settings edit screen should be updated to handle an error if the saved product ID is no longer available in the publication.

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

Acceptance criteria

  • If the rrmModuleV2 feature flag is enabled:
    • The Reader Revenue Manager settings edit screen should display the following error at the top if a formerly selected product ID no longer exists in the publication, i.e. it was deleted:
      • "The previously selected product ID {PRODUCT_ID} was not found. Please select a new product ID."
    • The current user must have access to the module to see the above error.
    • The above error should not appear when the publication itself is no longer available, in which case, a different existing error will be displayed.

Implementation Brief

  • In assets/js/modules/reader-revenue-manager/components/settings/SettingsEdit.js:
    • At the very first, before rendering any errors or inputs, render the StoreErrorNotices component with the module slug and store name of Reader Revenue Manager.
    • After the existing error for unavailable publication, check if the rrmModuleV2 feature flag is enabled, the user has module access, the publication is available, and the saved product ID (within the productID module setting) no longer exists within the productIDs module setting.
      • If all the above conditions are met, render an ErrorText with a message according to the ACs.
  • Add Storybook story variant showing the said error.

Test Coverage

  • Add test coverage to verify the visibility of the above error.

QA Brief

  • Set up RRM using a publication which has a contribution or paywall CTA set up with one or more product IDs associated with it.
  • Navigate to the RRM settings edit screen.
  • Use the following snippet in the JS console to examine/verify the product IDs for the publication:
googlesitekit.data.select('modules/reader-revenue-manager').getProductIDs();
// Example output:
['CAownIG4DA:basic-product-id', 'CAownIG4DA:advanced-product-id']
  • Use the following snippet to select one of the existing product IDs:
googlesitekit.data.dispatch('modules/reader-revenue-manager').setProductID( /* Use a product ID from the actual list output from the command above */ "CAownIG4DA:basic-product-id");
googlesitekit.data.dispatch('modules/reader-revenue-manager').saveSettings();

  • The error message should not appear. Reload the page to test with the persisted settings, again the error message should not appear.

  • Use the snippet above but with a non-existent product ID to simulate the case where the selected product ID has been removed from settings.

googlesitekit.data.dispatch('modules/reader-revenue-manager').setProductID( /* Use a product ID that doesn't exist in the list output above */ "CAownIG4DA:some-non-existent-product-id");
googlesitekit.data.dispatch('modules/reader-revenue-manager').saveSettings();
  • The error message should now appear. Reload the page to test with the persisted settings, the error message should again appear.

Changelog entry

  • Handle the error case where the saved product ID is no longer available in the current Reader Revenue Manager publication.
@nfmohit nfmohit self-assigned this Jan 29, 2025
@nfmohit nfmohit added P0 High priority Type: Enhancement Improvement of an existing feature Team M Issues for Squad 2 Module: RRM Reader Revenue Manager module related issues labels Jan 29, 2025
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jan 29, 2025

Note to AC Reviewer

I also added the IB while working on the ACs. If they look good to you, please move this directly to EB. Thank you!

@nfmohit nfmohit removed their assignment Jan 29, 2025
@techanvil techanvil self-assigned this Jan 30, 2025
@techanvil
Copy link
Collaborator

Thanks @nfmohit, good catch!

These AC/IB are looking good, one question though - do you know what the state of the product ID dropdown will be in this scenario, will it show the old value or an empty one, or possibly revert to the default openaccess value?

@techanvil techanvil assigned nfmohit and unassigned techanvil Jan 30, 2025
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jan 31, 2025

Thank you for the review, @techanvil!

do you know what the state of the product ID dropdown will be in this scenario, will it show the old value or an empty one, or possibly revert to the default openaccess value?

This would basically act the same as the publication dropdown or the Analytics property dropdown in the situation where no publications or GA properties are available. In this case, it would just show the existing value of the productID module setting. WDYT? Thanks!

@nfmohit nfmohit assigned techanvil and unassigned nfmohit Jan 31, 2025
@techanvil
Copy link
Collaborator

Thanks @nfmohit, that SGTM!

IB ✅

@techanvil techanvil removed their assignment Feb 5, 2025
@nfmohit nfmohit added the Good First Issue Good first issue for new engineers label Feb 8, 2025
@juniovitorino juniovitorino self-assigned this Feb 12, 2025
@juniovitorino juniovitorino mentioned this issue Feb 13, 2025
Merged
19 tasks
@juniovitorino juniovitorino removed their assignment Feb 14, 2025
@techanvil techanvil removed their assignment Feb 21, 2025
@techanvil techanvil assigned juniovitorino and unassigned techanvil Feb 25, 2025
@techanvil techanvil assigned juniovitorino and unassigned techanvil Feb 25, 2025
@techanvil techanvil removed their assignment Feb 27, 2025
@mohitwp mohitwp self-assigned this Feb 27, 2025
@mohitwp
Copy link
Collaborator

mohitwp commented Feb 27, 2025 •
edited
Loading

QA Update ⚠

  • Tested on dev environment.

  • Verified that The Reader Revenue Manager settings edit screen display the following error at the top if a formerly selected product ID no longer exists in the publication, i.e. it was deleted:

    "The previously selected product ID {PRODUCT_ID} was not found. Please select a new product ID."

  • Verified that above error not appear when we change the publication.

@juniovitorino @techanvil

Currently, an error does not appear if the user does not have access to the RRM module which is expected.

However, I have a question: I logged in as Admin B and ran the code snippet mentioned in QAB to generate an error. As expected per the AC, the error did not appear because Admin B does not have access to the RRM setup, which was configured by Admin A.

But, despite using the code snippet as Admin B, I was able to change the Product ID. Is this expected behavior?

Steps:

  • Set up the SK and RRM module as Admin A.
  • Log in as Admin B.
  • Edit the RRM settings.
  • Run all three code snippets mentioned in QAB.
  • After running the third code snippet, no error appears for Admin B (expected).
  • Log in as Admin A and check the RRM settings.
  • Notice that the RRM Product ID has changed to the value entered by Admin B.

Admin B

Image

Admin A

Image

PASS CASES

Image

Error

Recording.1834.mp4

When user selects another publication error gets remove

Recording.1835.mp4

@techanvil
Copy link
Collaborator

Hi @mohitwp, thanks for checking in on this one.

Being able to change the module settings via the snippet for the second admin is as expected, at least as things stand, because we don't currently have any ownership-based restrictions on updating module settings. At the settings endpoint level any admin can update the settings for a module regardless of whether they are the owner.

However, in practise the user will be prevented from changing the publication by the UI, which disables the RRM setting dropdown when they don't have access to the module.

@mohitwp
Copy link
Collaborator

mohitwp commented Feb 28, 2025

QA Update ✅

Thank you @techanvil !

  • Tested on dev environment.

  • Verified that The Reader Revenue Manager settings edit screen display the following error at the top if a formerly selected product ID no longer exists in the publication, i.e. it was deleted:

    "The previously selected product ID {PRODUCT_ID} was not found. Please select a new product ID."

  • Verified that above error not appear when we change the publication.

Image

Error

Recording.1834.mp4

When user selects another publication error gets remove

Recording.1835.mp4

@mohitwp mohitwp removed their assignment Feb 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Good First Issue Good first issue for new engineers Module: RRM Reader Revenue Manager module related issues P0 High priority Team M Issues for Squad 2 Type: Enhancement Improvement of an existing feature
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@juniovitorino @tofumatt @techanvil @nfmohit @mohitwp