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

Add ability to publish media on Twitter #2161

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 6 additions & 3 deletions app/models/agents/twitter_publish_agent.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,11 @@ def receive(incoming_events)
incoming_events = incoming_events.first(20)
end
incoming_events.each do |event|
tweet_text = interpolated(event)['message']
result = interpolated(event)
tweet_text = result['message']
tweet_media = result['media_url']
begin
tweet = publish_tweet tweet_text
tweet = publish_tweet tweet_text, tweet_media
create_event :payload => {
'success' => true,
'published_tweet' => tweet_text,
Expand All @@ -59,7 +61,8 @@ def receive(incoming_events)
end
end

def publish_tweet(text)
def publish_tweet(text, media)
return twitter.update_with_media(text, File.new(open(media))) unless media.blank?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume we are requiring open-uri somewhere and this also works with URLs?

Copy link
Author

Choose a reason for hiding this comment

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

I presume it is required by rails.
I am not entirely satisfied with the test coverage to be honest. I would have liked to refactor the test to stub the twitter API instead of the publish_tweet method so the internals of this method would have been covered. Maybe when I have more time.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We do require it in a few files, I think it would make sense to either move the requires to an initializer or add it to this Agent as well.

I would have liked to refactor the test to stub the twitter API instead of the publish_tweet method so the internals of this method would have been covered. Maybe when I have more time.

Good idea!

twitter.update(text)
end
end
Expand Down
19 changes: 14 additions & 5 deletions spec/models/agents/twitter_publish_agent_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
:consumer_secret => "---",
:oauth_token => "---",
:oauth_token_secret => "---",
:message => "{{text}}"
:message => "{{text}}",
:media_url => "{{media_url}}"
}

@checker = Agents::TwitterPublishAgent.new(:name => "HuginnBot", :options => @opts)
Expand All @@ -23,8 +24,10 @@
@event.save!

@sent_messages = []
stub.any_instance_of(Agents::TwitterPublishAgent).publish_tweet { |message|
@media_items = []
stub.any_instance_of(Agents::TwitterPublishAgent).publish_tweet { |message, media_url|
@sent_messages << message
@media_items << media_url unless media_url.blank?
OpenStruct.new(:id => 454209588376502272)
}
end
Expand All @@ -41,9 +44,15 @@
event2.payload = { :text => 'More payload' }
event2.save!

Agents::TwitterPublishAgent.async_receive(@checker.id, [event1.id, event2.id])
expect(@sent_messages.count).to eq(2)
expect(@checker.events.count).to eq(2)
event3 = Event.new
event3.agent = agents(:bob_weather_agent)
event3.payload = { :text => 'Payload with media', media_url: 'http://images.com/hello.png' }
event3.save!

Agents::TwitterPublishAgent.async_receive(@checker.id, [event1.id, event2.id, event3.id])
expect(@sent_messages.count).to eq(3)
expect(@checker.events.count).to eq(3)
expect(@media_items.count).to eq(1)
end
end

Expand Down