-
Notifications
You must be signed in to change notification settings - Fork 21
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
Enable labeling of Ads campaigns #2485
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2485 +/- ##
===========================================
+ Coverage 64.5% 64.9% +0.4%
- Complexity 4564 4575 +11
===========================================
Files 795 475 -320
Lines 22844 17856 -4988
Branches 1220 0 -1220
===========================================
- Hits 14739 11590 -3149
+ Misses 7938 6266 -1672
+ Partials 167 0 -167
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @jorgemd24, thanks for adding the label functionality 🙌
Confirmed that the label is created and assigned to the campaign.
I left one question but there aren't any issues so LGTM ✅
src/API/Google/AdsCampaignLabel.php
Outdated
$label_id = null; | ||
|
||
foreach ( $label_results->iterateAllElements() as $row ) { | ||
return $row->getLabel()->getId(); | ||
} | ||
|
||
return $label_id; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why you decided to create the $label_id
variable instead of just returning null
after the loop?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep that's true, it is unnecessary, I removed it here d30851d
Changes proposed in this Pull Request:
Part of pcTzPl-2nS-p2
We would like to track where campaigns are created from by labeling them with their source of creation. For example, use
wc-gla
for the web version, and for the mobile versions, something likewc-gla-android
orwc-gla-ios
.This PR allows the creation of a new campaign with a specific label. Note that I haven't updated the frontend to send the label query parameter yet, as I would first like to define the label names we'll be using.
Screenshots:
Detailed test instructions:
POST wp-json/wc/gla/ads/campaigns
with this body:You should see the campaign that you just created.
6. Alternatively, you can customize the campaigns table to display the labels:
Additional details:
Changelog entry