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

Update PAX conversion tracking service for compatibility with v1 #8693

Closed
2 tasks
aaemnnosttv opened this issue May 10, 2024 · 5 comments
Closed
2 tasks

Update PAX conversion tracking service for compatibility with v1 #8693

aaemnnosttv opened this issue May 10, 2024 · 5 comments
Labels
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

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented May 10, 2024

Feature Description

When we added the initial implementation of the conversionTrackingService, it was using a pre v1 definition which has since changed and needs some minor revisions.


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

Acceptance criteria

  • The PAX conversionTrackingService should be updated for compatibility with the current definition as defined in its type definition

Implementation Brief

  • Add the getSupportedConversionTrackingTypes method returning GetSupportedConversionTrackingTypesResponse ({ conversionTrackingTypes: string[] })
    • For now, this should only be a list with TYPE_PAGE_VIEW
  • Update the response for getPageViewConversionSetting to conform to the current GetPageViewConversionSettingResponse type by removing enablePageViewConversion from the response

Test Coverage

  • Update coverage for the service

QA Brief

The changes here should allow for the PAX campaign creation flow to complete successfully. At the same time, we should now see the page view conversion behavior for all conversion goal choices.

  • Set up the Ads module using the PAX app setup by enabling adsModule and adsPax feature flags.
  • Set up Site Kit and set up the Ads module.
  • There should be no errors during setup.
  • Conversion goal choices should show "page view conversions".

Changelog entry

  • Update PAX conversion tracking service code to improve compatibility with the new PAX version 1 API.
@aaemnnosttv aaemnnosttv added P0 High priority Type: Enhancement Improvement of an existing feature Module: Ads Google Ads module related issues labels May 10, 2024
@aaemnnosttv aaemnnosttv self-assigned this May 10, 2024
@aaemnnosttv aaemnnosttv removed their assignment May 10, 2024
@10upsimon 10upsimon self-assigned this May 13, 2024
@zutigrm zutigrm added the Squad 1 (Team S) Issues for Squad 1 label May 13, 2024
@10upsimon 10upsimon removed their assignment May 13, 2024
@tofumatt tofumatt assigned tofumatt and unassigned tofumatt May 13, 2024
@mohitwp mohitwp self-assigned this May 13, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented May 15, 2024

QA Update ❌

  • Tested on dev environment.
  • Verified Ads module set up when 'adspax' feature flag enabled.
  • Verified Ads module set up through pax app.

@aaemnnosttv Please find my observations below with couple of questions.

Q-1) QAB says - 'Conversion goal choices should show "page view conversions". Can you please tell me where this info will display? I found conversion goals under Ads dashboard and for me status is showing inactive.

image

Q-2) I haven't tested the design, responsiveness, and validation for the Pax campaign creation. Neither QAB nor AC mentions anything about design QA. Can you confirm if I don't need to test responsive and design issues as part of this ticket?

_Issue 1> On Pax app campaign creation 1st step console error is appearing._

image

Recording.953.mp4

_Issue 2> At 'What goal should your campaign be optimized to deliver?" step console error appears with red box if we remove single word from the selected page slug. This may be related to validation._

image

image

Recording.954.mp4

_Issue 3> If 'adspax' feature flag is enabled and Ads module is already set up then console error with red box appears on Dashboard after completing analytics setup._

Recording.956.mp4

image

image

image

@zutigrm
Copy link
Collaborator

zutigrm commented May 15, 2024

@mohitwp Thanks for your observations.

All error related questions are as I recall due to the proxy, it is coming from PAX app itself. Nothing that prevents PAX from working in current setup.

The last one - error on dashboard is missing dates service which is addressed in #8687 , you can ignore it for now until that issue is merged.

Regarding:

Q-1) QAB says - 'Conversion goal choices should show "page view conversions". Can you please tell me where this info will display? I found conversion goals under Ads dashboard and for me status is showing inactive.

I am not having enough info regarding this point maybe @10upsimon has more insights into this one

@aaemnnosttv
Copy link
Collaborator Author

Q-1) QAB says - 'Conversion goal choices should show "page view conversions". Can you please tell me where this info will display? I found conversion goals under Ads dashboard and for me status is showing inactive.

@mohitwp Conversion Goal choices are what you are prompted for on the step "What goal should your campaign be optimized to deliver?" i.e. what goal should your campaign be optimized for.

You should see the page view option for each choice but only when selected. This is shown by the page path select. This is then configured as a page view conversion – page views for the given URL are considered a conversion for the selected goal.
image

Q-2) I haven't tested the design, responsiveness, and validation for the Pax campaign creation. Neither QAB nor AC mentions anything about design QA. Can you confirm if I don't need to test responsive and design issues as part of this ticket?

Correct. This issue does not affect the UI but addresses some changes on our end to work with PAX. Regarding the UI though, it's important to understand what part is SK and what is PAX. Basically everything below the line and above the button here is PAX
image

We should still flag issues we find but we don't need to be testing this on issues that don't call for it.

Issue 1 On Pax app campaign creation 1st step console error is appearing.

This is a known issue that we've reported and it can be ignored for now.

Issue 3 If 'adspax' feature flag is enabled and Ads module is already set up then console error with red box appears on Dashboard after completing analytics setup.

I think this is related to the work in #8559.

Back to you @mohitwp !

@aaemnnosttv aaemnnosttv removed their assignment May 15, 2024
@wpdarren wpdarren assigned wpdarren and mohitwp and unassigned wpdarren May 16, 2024
@mohitwp
Copy link
Collaborator

mohitwp commented May 16, 2024

Blocked by the #8687. cc @wpdarren

@wpdarren wpdarren assigned wpdarren and unassigned mohitwp May 16, 2024
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • Set up Ads module and successfully created a campaign. See note.
  • Each conversion goal choice shows the page view option, but only when selected.

Note: There are a number of errors in the PAX application that we will feed back to the Ads team. These are not related to Site Kit but the application itself.

image

@wpdarren wpdarren removed their assignment May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

6 participants