Skip to content

Line item field name changes #277

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.

Sign up for GitHub

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

Jump to bottom
Merged
juanshishido merged 4 commits into from
Mar 27, 2021
Merged

Line item field name changes #277

juanshishido merged 4 commits into from
Mar 27, 2021

Conversation

@juanshishido
Copy link
Contributor

@juanshishido juanshishido commented Mar 25, 2021 •
edited
Loading

The following are based on changes in Ads API v9:

  • Add goal
  • Remove optimization and bid_unit
  • Rename:
    • bid_type -> bid_strategy
    • charge_by -> pay_by

@juanshishido juanshishido requested a review from a team March 25, 2021 23:35
tushdante
tushdante approved these changes Mar 26, 2021
View reviewed changes
Copy link
Contributor

@tushdante tushdante left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just want to confirm the ENUM changes prior to merging

twitter_ads/enum.py
PREROLL_STARTS='PREROLL_STARTS',
REACH_WITH_ENGAGEMENT='REACH_WITH_ENGAGEMENT',
VIDEO_VIEW='VIDEO_VIEW',
VIEW_3S_100PCT='VIEW_3S_100PCT',
Copy link
Contributor

@tushdante tushdante Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't realize these are valid goals too (VIEW_3S_100PCT, VIEW_6S and VIEW_15S). These should be pay_by if I'm not mistaken

Copy link
Contributor Author

@juanshishido juanshishido Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the callout. Let me double check on this.

Copy link
Contributor Author

@juanshishido juanshishido Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed that VIEW_3S_100PCT works when creating a line item. Also checked the field values internally. Thanks again for flagging!

@juanshishido
Copy link
Contributor Author

juanshishido commented Mar 26, 2021

@tushdante Should I update API_VERSION in this file with this diff? Or should that happen separately?

@juanshishido
Copy link
Contributor Author

juanshishido commented Mar 27, 2021

@tushdante Should I update API_VERSION in this file with this diff? Or should that happen separately?

Will do this in a separate diff.

@juanshishido juanshishido merged commit ebb8202 into xdevplatform:master Mar 27, 2021
@juanshishido juanshishido deleted the line-item-field-name-changes branch March 27, 2021 02:30
robi-ng added a commit to nugit/twitter-python-ads-sdk that referenced this pull request May 19, 2021
@robi-ng
Revert "Line item field name changes (xdevplatform#277)"
27cc214 
This reverts commit ebb8202.
@ChanYiLin
Copy link

ChanYiLin commented Sep 28, 2021 •
edited
Loading

Hi @juanshishido , there is an issue for this change
While line item has removed 'bid_type' property, the save function in LineItem still checks the bid_type and causes property not found issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@tushdante tushdante tushdante approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@juanshishido @ChanYiLin @tushdante