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

Implement PAX Config Support for contentConfig.partnerAdsExperienceConfig.reportingStyle Property #8637

Closed
1 task done
10upsimon opened this issue Apr 29, 2024 · 3 comments
Labels
javascript Pull requests that update Javascript code Module: Ads Google Ads module related issues P0 High priority Squad 1 (Team S) Issues for Squad 1 Type: Enhancement Improvement of an existing feature

Comments

@10upsimon
Copy link
Collaborator

10upsimon commented Apr 29, 2024

Feature Description

As an extension to the PAX main component and scaffolding work done in #8556 and #8557, there is a need to implement the newly added PAX config property contentConfig.partnerAdsExperienceConfig.reportingStyle, which will be abstracted by the PAX component displayMode prop.

Applicable values of this config property are:

  • REPORTING_STYLE_FULL - Will render the full PAX experience, and should be the default mode
  • REPORTING_STYLE_MINI - Will render the reporting widget only

As indicated in #8556 and #8557, support for this display mode was made available via a displayMode prop on the main PAX component, but it was not clear at the time what config value(s) these properties would interface with.

Component and config work implemented thus far should be extended so that values passed to displayMode prop are forwarded and abstracted as PAX config properties applicable to contentConfig.partnerAdsExperienceConfig.reportingStyle. The following values were earmaked for use as values passed to the displayMode prop:

  • 'setup' - Indicates the full PAX experience is desired, and should result in the contentConfig.partnerAdsExperienceConfig.reportingStyle config prop having a value of REPORTING_STYLE_FULL
  • 'reporting' - Indicates the widget reporting view for PAX, and should result in the contentConfig.partnerAdsExperienceConfig.reportingStyle config prop having a value of REPORTING_STYLE_MINI
  • 'default' (or null/omitted) - Indicates the full PAX experience is desired, and should result in the contentConfig.partnerAdsExperienceConfig.reportingStyle config prop having a value of REPORTING_STYLE_FULL

Implementation of the existing PAX config logic should be extended to cater for the above scenario.


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

Acceptance criteria

  • The existing PAX Component as defined in Create PAX component #8557 should be extended/enhanced to support the PAX config property contentConfig.partnerAdsExperienceConfig.reportingStyle
  • The 3 values passed to the PAX component displayMode prop should be abstracted to result in the applicable contentConfig.partnerAdsExperienceConfig.reportingStyle values of REPORTING_STYLE_FULL when the value of the displayMode prop is 'setup' or 'default', or REPORTING_STYLE_MINI when the value of the displayMode prop is 'reporting'

Implementation Brief

  • Update assets/js/modules/ads/components/PAXEmbeddedApp.js
    • Expand existing paxConfig constant value, by including new property in the object: contentConfig.partnerAdsExperienceConfig.reportingStyle (dots annotate nesting)
      • For value, check if passed prop - displayMode has value of reporting to use REPORTING_STYLE_MINI
      • Otherwise assign it a value of REPORTING_STYLE_FULL by default

Test Coverage

  • No updates needed

QA Brief

  • As the PAX component is currently not surfaced to users, this must be tested by an engineer hence the QA:Eng tag - Confirm the AC is correctly implemented and the config passed to the PAX config is updated correctly based on the displayMode.

Changelog entry

  • Add support for the Partner Ads Experience reportingStyle config.
@10upsimon 10upsimon added javascript Pull requests that update Javascript code Module: Ads Google Ads module related issues Squad 1 (Team S) Issues for Squad 1 Type: Enhancement Improvement of an existing feature labels Apr 29, 2024
@10upsimon 10upsimon assigned 10upsimon and unassigned 10upsimon Apr 29, 2024
@10upsimon 10upsimon added the P0 High priority label Apr 29, 2024
@eclarke1 eclarke1 added the Next Up Issues to prioritize for definition label Apr 30, 2024
@tofumatt tofumatt self-assigned this Apr 30, 2024
@tofumatt
Copy link
Collaborator

I think this sounds good, moving to IB 👍🏻

Might be easier to do the IB after #8624 is merged, but not required 🙂

@tofumatt tofumatt assigned 10upsimon and unassigned tofumatt and 10upsimon Apr 30, 2024
@zutigrm zutigrm assigned zutigrm and unassigned zutigrm May 1, 2024
@10upsimon
Copy link
Collaborator Author

IB ✅

@bethanylang bethanylang removed the Next Up Issues to prioritize for definition label May 6, 2024
@benbowler benbowler self-assigned this May 7, 2024
@benbowler benbowler added the QA: Eng Requires specialized QA by an engineer label May 7, 2024
@benbowler benbowler removed their assignment May 7, 2024
@jimmymadon jimmymadon assigned jimmymadon and unassigned jimmymadon May 7, 2024
@tofumatt tofumatt self-assigned this May 7, 2024
@tofumatt tofumatt removed the QA: Eng Requires specialized QA by an engineer label May 8, 2024
@tofumatt tofumatt removed their assignment May 8, 2024
@tofumatt
Copy link
Collaborator

tofumatt commented May 8, 2024

There are no QA steps needed for this issue, moving directly to Approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code Module: Ads Google Ads module related issues P0 High priority Squad 1 (Team S) Issues for Squad 1 Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

8 participants