Skip to content

Fix TwitterAds::Creative::PromotedTweet#save (#239)#240

Merged
tushdante merged 3 commits into
xdevplatform:masterfrom
bugcloud:fix-promoted-tweet-bug
Oct 9, 2020
Merged

Fix TwitterAds::Creative::PromotedTweet#save (#239)#240
tushdante merged 3 commits into
xdevplatform:masterfrom
bugcloud:fix-promoted-tweet-bug

Conversation

@bugcloud
Copy link
Copy Markdown
Contributor

@bugcloud bugcloud commented Oct 6, 2020

  • Fix invalid converting params

Issue Type: Bug

Fixes: #239

Changes Included:

  • Fix invalid converting params
  • Add test code of converting params

Check List:

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 6, 2020

CLA assistant check
All committers have signed the CLA.

@tushdante
Copy link
Copy Markdown
Contributor

Hey @bugcloud - Thanks for making this change. It looks like there's a couple linting errors that were detected. Could you resolve those and commit your changes?

@bugcloud
Copy link
Copy Markdown
Contributor Author

bugcloud commented Oct 7, 2020

@tushdante Thank you for replying. I fixed a lint error. f64ee61

Before

$ bundle exec rubocop spec/twitter-ads/creative/promoted_tweet_spec.rb
WARNING:  No private key present, creating unsigned gem.
Inspecting 1 file
C

Offenses:

spec/twitter-ads/creative/promoted_tweet_spec.rb:51:101: C: Line is too long. [165/100]
      expect(Request).to receive(:new).with(client, :post, "/#{TwitterAds::API_VERSION}/accounts/#{account.id}/promoted_tweets", expected_params).and_return(request)
                                                                                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

1 file inspected, 1 offense detected

After

$ bundle exec rubocop
WARNING:  No private key present, creating unsigned gem.
Inspecting 103 files
.......................................................................................................

103 files inspected, no offenses detected

Copy link
Copy Markdown
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 wanted to confirm the description for the test

expect { subject.save }.to raise_error(TwitterAds::ClientError)
end

it 'converts params[:tweet_id] to params[:tweet_ids]' do
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe this converts params[:tweet_ids] to params[:tweet_id]

Copy link
Copy Markdown
Contributor Author

@bugcloud bugcloud Oct 8, 2020

Choose a reason for hiding this comment

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

Oh really?
It seems that TwitterAds::Creative::PromotedTweet#save set params[:tweet_ids] from params[:tweet_id] .
(I followed this source code comment.)
https://github.com/twitterdev/twitter-ruby-ads-sdk/blob/master/lib/twitter-ads/creative/promoted_tweet.rb#L49

I have rewritten this description more clearly 🙇 eb4adb2

@tushdante tushdante merged commit 2e3d733 into xdevplatform:master Oct 9, 2020
@bugcloud bugcloud deleted the fix-promoted-tweet-bug branch October 10, 2020 06:32
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 this pull request may close these issues.

3 participants