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

Conversation

qwertme
Copy link

@qwertme qwertme commented Nov 12, 2017

Just a quick fix to allow posting images to Twitter.

@qwertme qwertme changed the title Add ability too publish media on Twitter Add ability to publish media on Twitter Nov 12, 2017
@@ -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!

@whtsky
Copy link
Contributor

whtsky commented Mar 1, 2020

Tried and it works on my instance. Is the open-uri require the only roadblock?

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.

None yet

3 participants