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.

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

Add post meta setting to store post-level RRM settings #9955

Closed
3 tasks done
nfmohit opened this issue Jan 2, 2025 · 6 comments
Closed
3 tasks done

Add post meta setting to store post-level RRM settings #9955

nfmohit opened this issue Jan 2, 2025 · 6 comments
Labels
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 2, 2025

Feature Description

As part of RRM Phase 2, the WordPress post editor will allow users to override the snippet configuration at the post level. To facilitate this, a post meta setting should be added to store the post-level configuration.

A Modules\Reader_Revenue_Manager\Post_Product_ID class extending the Core\Storage\Post_Meta_Setting class should be created to register a post meta setting to store the post-level snippet options. It should hold an empty string as the value by default. The meta key can be googlesitekit_rrm_{Publication ID}:productID.


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

Acceptance criteria

  • When the rrmModuleV2 feature flag is enabled:
    • A new post meta setting should be registered using the existing post meta infrastructure that will store the per-post-level snippet configuration for Reader Revenue Manager.
    • The meta key should include the publication ID so that it can be uniquely identified when the connected publication changes.
    • The meta value by default should be an empty string, and it should only accept a string as the value.

Implementation Brief

  • In includes/Modules/Reader_Revenue_Manager.php

    • In register method, check if rrmModuleV2 feature is enabled using Feature_Flags::enabled( 'rrmModuleV2' ). If feature is enabled and module is connected, do the following.
      • Instantiate Core\Storage\Post_Meta class.
      • Instantiate Post_Product_ID and pass instance of Core\Storage\Post_Meta and the publication ID to it's constructor.
      • Call register method of the Post_Product_ID class.
  • In includes/Core/Storage/Post_Meta_Setting.php

    • Create a protected method get_meta_key which should return an empty string by default.
    • Remove META_KEY constant in the class and replace it's usage with the above method such as $this->get_meta_key().
  • Create a class Post_Product_ID inside includes/Modules/Reader_Revenue_Manager.

    • It should accept the instance of Core\Storage\Post_Meta and the publication ID in it's constructor. The publication ID can be assigned to a private class property which would then be utilised to form the meta key.
    • It extends Google\Site_Kit\Core\Storage\Post_Meta_Setting class.
    • Override the get_meta_key which should return googlesitekit_rrm_{publication_id}:productID. Note to replace the {Publication ID} with the value of publication Id property.
    • Override get_show_in_rest method which should return true. This is essential as this value needs to be set by post being edited using block editor.

Test Coverage

  1. Write tests for Post_Product_ID class.
  2. Fix any failing tests because of changes in Post_Meta_Setting class.

QA Brief

  1. Enable rrmModuleV2 feature.

  2. Enable the RRM module in Site Kit and connect to a publication. Note the publication ID which should be available in dropdown or it can be retrieved using the following command in SK dashboard.

  googlesitekit.data.select('modules/reader-revenue-manager').getPublicationID();
  1. Go to Posts > Add New post, open browser console and run following command.
  wp.data.select('core/editor').getEditedPostAttribute('meta')

There must be the googlesitekit_rrm_{publicationID}:productID key present in the list of meta keys, where {publicationID} is the one which we noted above.

Changelog entry

  • Add mechanism to store post-level Reader Revenue Manager settings.
@nfmohit nfmohit added P0 High priority Type: Enhancement Improvement of an existing feature Team M Issues for Squad 2 labels Jan 2, 2025
@ankitrox ankitrox self-assigned this Jan 6, 2025
@ivonac4 ivonac4 added the Next Up Issues to prioritize for definition label Jan 6, 2025
@ankitrox ankitrox removed their assignment Jan 6, 2025
@nfmohit nfmohit self-assigned this Jan 6, 2025
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jan 7, 2025

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

  • In includes/Core/Storage/Post_Meta_Setting.php

    • Create a protected method get_meta_key which should return static::META_KEY
    • Replace the use of static::META_KEY with the above method such as $this->get_meta_key().

I say, let's get rid of the META_KEY constant entirely and rely totally upon the get_meta_key method, which would, by default, return an empty string. What do you say?

  • Create a class Post_Product_ID inside includes/Modules/Reader_Revenue_Manager.

    • It should accept the instance of Reader_Revenue_Manager in it's constructor.
    • META_KEY constant should be set to googlesitekit_rrm_{publicatio_id}:productID
    • It extends Google\Site_Kit\Core\Storage\Post_Meta_Setting class.
    • Add a register method inside class, which should check if RRM module is connected using Reader_Revenue_Manager::is_connected method. If yes, call register method of parent class, else do nothing.
    • Override the get_meta_key which should replace {publicatio_id} with the one saved in setting. We can use Reader_Revenue_Manager instane to get that value via get_settings method.
    • Override get_show_in_rest method which should return true. This is essential as this value needs to be set by post being edited using block editor.
  • Instead of accepting the entire Reader_Revenue_Manager instance, let's just accept the publication ID when it is instantiated within Reader_Revenue_Manager.
  • If we remove Post_Meta_Setting::META_KEY, we don't need to set it.
  • Instead of checking if RRM is connected in this class, we should instead only call Post_Product_ID::register within Reader_Revenue_Manager if it is connected.
  • Instead of using a Reader_Revenue_Managerinstance, we should simply accept the publication ID, store it in a class property, and use it withinget_meta_key`.

In includes/Modules/Reader_Revenue_Manager.php

  • In register method, check if rrmModuleV2 feature is enabled using Feature_Flags::enabled( 'rrmModuleV2' ). If feature is enabled, instantiate Post_Product_ID class.
  • Call register method of the Post_Product_ID class.
  • Besides these, we also need to instantiate the Core\Storage\Post_Meta class and pass its instance to Post_Product_ID.
  • We also need to pass the publication ID as mentioned above.
  • We should only call the register method if the module is connected.

Test Coverage

  1. Write tests for Post_Product_ID class.

Any failing tests should also be fixed due to changes in the Post_Meta_Setting class.

I have updated the PoC to include most of these changes. Please let me know if you have any questions, thank you!

@nfmohit nfmohit assigned ankitrox and unassigned nfmohit Jan 7, 2025
@ankitrox
Copy link
Collaborator

ankitrox commented Jan 8, 2025

Thanks for reviewing the IB @nfmohit , I've made the suggested changes in the IB.

@ankitrox ankitrox assigned nfmohit and unassigned ankitrox Jan 8, 2025
@nfmohit
Copy link
Collaborator Author

nfmohit commented Jan 8, 2025

Thank you, @ankitrox ! IB ✅

@nfmohit nfmohit removed their assignment Jan 8, 2025
@ankitrox ankitrox self-assigned this Jan 9, 2025
@ankitrox ankitrox removed their assignment Jan 10, 2025
@benbowler benbowler assigned benbowler and unassigned benbowler Jan 10, 2025
@nfmohit nfmohit assigned nfmohit and ankitrox and unassigned nfmohit Jan 10, 2025
@ankitrox ankitrox assigned nfmohit and unassigned ankitrox Jan 13, 2025
@ivonac4 ivonac4 removed the Next Up Issues to prioritize for definition label Jan 13, 2025
@nfmohit nfmohit removed their assignment Jan 14, 2025
@nfmohit nfmohit added the Module: RRM Reader Revenue Manager module related issues label Jan 18, 2025
@mohitwp mohitwp self-assigned this Jan 20, 2025
@mohitwp
Copy link
Collaborator

mohitwp commented Jan 21, 2025

QA Update ⚠

@ankitrox On the "Add New Post" editor screen, I ran the command wp.data.select('core/editor').getEditedPostAttribute('meta') in the browser console. Upon running this command, a new meta key, "footnotes," was added to the wp_postmeta table with an empty string, which is the expected behavior according to the acceptance criteria (AC).

After this, I deleted the "footnotes" meta key from the database and reset the SK plugin. I then reconnected the plugin, set up RRM, and ran the same command again on the "Add New Post" screen. While I received the expected result in the console, the "footnotes" meta key was not added to the wp_postmeta table.

Is this behavior expected?

Image

PASS CASE

  • Tested on dev environment.
  • Tested after enabling rrmphase2 feature flag.
  • Tested after successfully set up of RRM module.
  • For test I ran the wp.data.select('core/editor').getEditedPostAttribute('meta') command in browser console.
  • Verified that googlesitekit_rrm_{publicationID}:productID key present in the list of meta keys, where {publicationID} is the one which selected under RRM.

Image

Recording.1697.mp4

@ankitrox
Copy link
Collaborator

@mohitwp As per our discussion on slack thread, the footnote meta is being set from somewhere else in the editor (maybe some plugin or theme) and we should only check for the mentioned meta key in QAB if it is being set correctly!

@ankitrox ankitrox removed their assignment Jan 22, 2025
@mohitwp
Copy link
Collaborator

mohitwp commented Jan 22, 2025

QA Update ✅

  • Tested on dev environment.
  • Tested after enabling rrmphase2 feature flag.
  • Tested after successfully set up of RRM module.
  • For test I ran the wp.data.select('core/editor').getEditedPostAttribute('meta') command in browser console.
  • Verified that googlesitekit_rrm_{publicationID}:productID key present in the list of meta keys, where {publicationID} is the one which selected under RRM.

Image

Recording.1697.mp4

@mohitwp mohitwp removed their assignment Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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
Development

No branches or pull requests

6 participants