Tailored Audience for v4 #180

esakai · 2018-12-20T10:00:39Z

This is an implementation for Twitter Ads API v4 endpoints for tailored audiences.

POST accounts/:account_id/tailored_audiences
POST accounts/:account_id/tailored_audiences/:tailored_audience_id/users

Tests are not available yet.

esakai · 2018-12-20T10:15:09Z

@jbabichjapan
It would be great if you could review this pull request. Thank you in advance.

jbabichjapan · 2018-12-21T06:46:09Z

Thanks so much for this - @tushdante will be reviewing this and also has been preparing the rest of the support for v4 - I think it would be good to align the checkin so that the other functionality doesn't break when we upgrade the API_VERSION from 3 to 4

tushdante · 2019-01-03T22:00:13Z

lib/twitter-ads/audiences/tailored_audience.rb

+      # @since 4.0
+      #
+      # @return [TailoredAudience] The newly created tailored audience instance.
+      def create_instance(account, name)


Given that v4 will no longer support TON based audiences, I'd suggest removing this method in favor of editing the existing create method instead

@tushdante

There is a concern. The signatures of these two methods are totally different.

create(account, file_path, name, list_type) create_instance(account, name)

If I made a method like create(account, name), the backward compatibility would be broken. Is this OK with you? If so, I would simply drop the current create method and rename create_instance to create.

Good point! I think it should be fine to just drop the current create method and rename the create_instance to create, given that the current create method will no longer be supported as of v4.

@tushdante Thank you for your comment. I understand. I will modify the code as you mentioned.

tushdante · 2019-01-03T22:02:35Z

lib/twitter-ads/audiences/tailored_audience.rb

+    # @since 4.0
+    #
+    # @return success_count, total_count
+    def users(params)


Please add @param params [JSON object] A hash containing the list of users to be added/removed/updated as a comment to describe the input format

@tushdante
Thank you for your comment. I will add it.

tushdante · 2019-01-03T22:03:16Z

lib/twitter-ads/client.rb

@@ -3,7 +3,7 @@

 module TwitterAds

-  API_VERSION = '3'.freeze
+  API_VERSION = '4'.freeze


Lets leave this as 3 for now, given that this change will break all deprecated endpoints. We'll make the change once all the other v4 changes have been merged.

tushdante · 2019-01-03T22:09:39Z

Left a couple comments @esakai ! Let me know if there's any additional questions. The changes should be simple enough and once we've merged this PR we can make the change to v4.

eccyan · 2019-01-30T06:59:21Z

How's this PR going?

esakai · 2019-01-31T01:40:37Z

I have already implemented all parts I was requested to change. I am waiting for @tushdante 's review.

treby · 2019-03-04T07:42:17Z

It seems the code conflicts with master.
@esakai do you plan to resolve that?

esakai · 2019-03-04T08:14:56Z

@treby

Thank you for your notice.
I have just rebased this branch on the master branch so that the conflict can be resolved.

There is one more suggestion.
I think that TwitterAds::TailoredAudience.memberships is no longer needed.
Do you want me to delete the method?
If you wish to do so, you can delete it yourself.
Please let me know your thoughts.

treby · 2019-03-04T10:25:51Z

I think that TwitterAds::TailoredAudience.memberships is no longer needed.

I think so too.
According to twitter, version 3 of the Ads API reached end of life on February 28, 2019.

The document for the API is no longer available, too.
https://developer.twitter.com/en/docs/ads/audiences/api-reference/real-time

I can't decide the scope of this PR (Maybe @jbabichjapan or @tushdante - gem maintainers can), but just in my opinion both of the way (below) are ok.

option 1: First merge this PR, remove outdated feature on other PR
option 2: Including outdated feature removal on this PR, and review & merge

To maintainers, what do you think?

smaeda-ks · 2019-03-04T10:45:43Z

@treby @esakai Thanks. As you noticed, some of the old Audience endpoints are no longer work in v5 and there are even more lines to be removed. We will take care that part in another PR, so let's freeze this PR for now and merge it as it's been a while since created last year. Though, thank you for your patience :)

cc: @tushdante

treby · 2019-03-14T02:56:50Z

@esakai I think this PR is ready to merge (Perhaps you need to follow master branch :) ).
Do you have any works for this PR before merging it?

The current create method will no longer be supported as of v4.

esakai · 2019-03-14T06:25:53Z

@treby
Thank you for your reminder.
I have just rebased this branch on twitterdev:master branch.
I have already done everything.
You can merge this branch to the master branch now.

esakai · 2019-04-09T06:40:07Z

I think that this pull request should be merged because all the problems are solved now. I am wondering why this pull request is still open.

bugcloud · 2019-05-13T09:22:25Z

What is the blocker of this PR??

smaeda-ks · 2019-05-17T10:06:21Z

@esakai @treby @bugcloud This is now merged. Again, thanks for your contribution!

* Support for media library, media creative and account media endpoints (#185) * Bump version to v4 (#186) * make error handling image be shown (#177) * update twitter-ads version to 4.0.1 (#187) * Fix Rubocop Rules (#189) * update twitter-ads version to 4.0.1 * update rubocop.yml and tailored_audience.rb * Update to v5 (#191) * Added support for MEDIA_CREATIVE stats * added support for card_uris * updated example for v5 * updated TC line_item_id to line_item_ids * added support for with_draft * remove preview_url from all cards endpoints * Tailored Audience for v4 (#180) * Add a comment to TailoredAudience.users * Rename create_instance to create The current create method will no longer be supported as of v4. * Delele unused methods * Add (start|end)_time property to LineItem class #178 (#193) * Remove TONUpload class (#192) * Update SDK to support Ruby 2.3+ (#190) * Remove CI guards * Start upgrade to min 2.3. Rubocop auto fixes * Disable and update a handful of Rubocop warnings Disabled Linting warnings on using Void and URI:Escape * Update copyright * Test ruby 2.3+ on Travis CI * Add Ruby 2.6 to build matrix and correct copyright lint * Minor fix to copyright * Add rubocop as separate step to display errors * Make rubocop verbose to help debugging * Remove erroneous newline * Fix merge conflicts * Revert "Remove CI guards" This reverts commit 85db938. * Update supported Ruby versions and bump license year * Update LICENSE * Fix merge conflicts * Add ActiveEntities support (#195) * Add ActiveEntities support * Update style * Add new preview endpoint support (#194) * Minor bug fixes (#197) * Fix promoted_tweet example * Fix TypeError > no implicit conversion of nil into Hash (TypeError) when with_deleted is false * Improve Travis CI Tests (#196) * Update .travis.yml * Update README We no longer test JRuby since #133 * Set min version to 2.4 * Allow bundler 2 * Update .rubocop.yml * Update twitter-ads version to 5.1.0 (#198) * [docs] updating v5.1.0 documentation

* Add a comment to TailoredAudience.users * Rename create_instance to create The current create method will no longer be supported as of v4. * Delele unused methods

tushdante requested changes Jan 3, 2019

View reviewed changes

tushdante mentioned this pull request Jan 3, 2019

Update to version 4 #181

Closed

tushdante approved these changes Feb 8, 2019

View reviewed changes

esakai force-pushed the tailored-audience-for-v4 branch from 6f46aec to 9b0ffd1 Compare March 4, 2019 08:10

esakai added 3 commits March 14, 2019 15:21

Add a comment to TailoredAudience.users

c561462

Rename create_instance to create

bf272b7

The current create method will no longer be supported as of v4.

Delele unused methods

a5bff88

esakai force-pushed the tailored-audience-for-v4 branch from 005ca25 to a5bff88 Compare March 14, 2019 06:22

smaeda-ks merged commit f15d7fa into xdevplatform:master May 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tailored Audience for v4 #180

Tailored Audience for v4 #180

esakai commented Dec 20, 2018

esakai commented Dec 20, 2018

jbabichjapan commented Dec 21, 2018

tushdante Jan 3, 2019

esakai Jan 7, 2019

tushdante Jan 7, 2019

esakai Jan 8, 2019

tushdante Jan 3, 2019

esakai Jan 7, 2019

tushdante Jan 3, 2019

tushdante commented Jan 3, 2019

eccyan commented Jan 30, 2019

esakai commented Jan 31, 2019

treby commented Mar 4, 2019

esakai commented Mar 4, 2019

treby commented Mar 4, 2019

smaeda-ks commented Mar 4, 2019

treby commented Mar 14, 2019

esakai commented Mar 14, 2019

esakai commented Apr 9, 2019

bugcloud commented May 13, 2019

smaeda-ks commented May 17, 2019

Tailored Audience for v4 #180

Tailored Audience for v4 #180

Conversation

esakai commented Dec 20, 2018

esakai commented Dec 20, 2018

jbabichjapan commented Dec 21, 2018

tushdante Jan 3, 2019

Choose a reason for hiding this comment

esakai Jan 7, 2019

Choose a reason for hiding this comment

tushdante Jan 7, 2019

Choose a reason for hiding this comment

esakai Jan 8, 2019

Choose a reason for hiding this comment

tushdante Jan 3, 2019

Choose a reason for hiding this comment

esakai Jan 7, 2019

Choose a reason for hiding this comment

tushdante Jan 3, 2019

Choose a reason for hiding this comment

tushdante commented Jan 3, 2019

eccyan commented Jan 30, 2019

esakai commented Jan 31, 2019

treby commented Mar 4, 2019

esakai commented Mar 4, 2019

treby commented Mar 4, 2019

smaeda-ks commented Mar 4, 2019

treby commented Mar 14, 2019

esakai commented Mar 14, 2019

esakai commented Apr 9, 2019

bugcloud commented May 13, 2019

smaeda-ks commented May 17, 2019