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

Campaign Creation: Remove Ads Audience field #2501

Open
2 tasks
Tracked by #2459
joemcgill opened this issue Aug 6, 2024 · 3 comments · Fixed by #2551
Open
2 tasks
Tracked by #2459

Campaign Creation: Remove Ads Audience field #2501

joemcgill opened this issue Aug 6, 2024 · 3 comments · Fixed by #2551

Comments

@joemcgill
Copy link
Collaborator

joemcgill commented Aug 6, 2024

Part of #2459

Image

Today we show “Ads audience” (see screenshot below) but instead, we can remove it and target all countries selected during product feed configuration (step 2).

Acceptance Criteria

  • The campaign creation step no longer shows the audience section during onboarding.
  • When a new campaign is created, the values selected during step 2 are used for the campaign.

Implementation Brief

The AudienceSection component can be removed from the PaidAdsSetupSections component. Update the PaidAdsSetupSections component so that it initializes the paidAds state with targetAudience data that was saved in step 2. We can probably use the getTargetAudience selector to get this data, but will need to make sure that it then gets formatted in a way that is expected when the form is submitted.

Test Coverage

Update the E2E tests for tests/e2e/specs/setup-mc/step-4-complete-campaign.test.js to remove tests related to the audience field.

@joemcgill joemcgill changed the title Remove Ads Audience field Campaign Creation: Remove Ads Audience field Aug 6, 2024
@joemcgill joemcgill assigned joemcgill, eason9487 and mikkamp and unassigned joemcgill Aug 6, 2024
@eason9487
Copy link
Member

  1. Are there any side effects that we would need to be aware of when using target audience data from MC for the Ads campaign?

I think there shouldn't be any side effects because the target audiences from MC are originally the default value when the starting campaign creation.

@eclarke1 eclarke1 removed the Rollover label Sep 5, 2024
@asvinb asvinb assigned joemcgill and unassigned asvinb Sep 5, 2024
@asvinb asvinb assigned eason9487 and unassigned asvinb Sep 9, 2024
@eason9487 eason9487 assigned asvinb and unassigned eason9487 Sep 10, 2024
@asvinb asvinb assigned eason9487 and unassigned asvinb Sep 10, 2024
@eason9487 eason9487 assigned asvinb and unassigned eason9487 Sep 11, 2024
@eason9487
Copy link
Member

There is a code conflict so I moved this ticket to the In Progress status. I believe the PR won't need another round of code reviews after resolving the code conflict.

@asvinb
Copy link
Collaborator

asvinb commented Sep 11, 2024

Thanks @eason9487 . Merge conflicts fixed. As per your comment, assigning task to @fblascogarma for UAT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants