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

Drop giphy dependency #89

Merged
merged 4 commits into from
Jul 31, 2016
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@ rvm:
- 2.3.0
env:
- CONCURRENCY=celluloid-io
- CONCURRENCY=celluloid-io WITH_GIPHY=true
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd just call it GIPHY to be consistent with CONCURRENCY, nbd though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought about that as well, but since CONCURRENCY is a requirement and giphy is optional, I think the WITH_ prefix makes it's purpose a bit more clear.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense!

- CONCURRENCY=faye-websocket

- CONCURRENCY=faye-websocket WITH_GIPHY=true
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### 0.8.3 (Next)

* Your contribution here.
* [#89](https://github.com/dblock/slack-ruby-bot/pull/89): Drop giphy dependency - [@tmsrjs](https://github.com/tmsrjs).

### 0.8.2 (7/10/2016)

Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,4 @@ source 'http://rubygems.org'
gemspec

gem ENV['CONCURRENCY'], require: false if ENV.key?('CONCURRENCY')
gem 'giphy', require: false if ENV.key?('WITH_GIPHY')
35 changes: 22 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,30 @@ class Phone < SlackRubyBot::Commands::Base
end
```

The `SlackRubyBot::Client` implementation comes with GIF support.
### Animated GIFs

#### say(channel: ..., text: ..., gif: ...)
The `SlackRubyBot::Client` implementation comes with GIF support.
To enable it simply add `gem 'giphy'` to your **Gemfile**.
**Note:** Bots send animated GIFs in default commands and errors.

Sends text and/or a random GIF that matches a keyword using a RealTime client to a channel.
```ruby
class Phone < SlackRubyBot::Commands::Base
command 'call'

def self.call(client, data, match)
client.say(channel: data.channel, text: 'called', gif: 'phone')
# Sends the text 'called' and a random GIF that matches the keyword 'phone'.
end
end
```

If you use giphy for something else but don't want your bots to send GIFs you can set `ENV['SLACK_RUBY_BOT_SEND_GIFS']` or `SlackRubyBot::Config.send_gifs` to `false`. The latter takes precedence.

```ruby
SlackRubyBot.configure do |config|
config.send_gifs = false
end
```

### Built-In Commands

Expand Down Expand Up @@ -291,16 +310,6 @@ These will get pushed into the hook set on initialization.

Either by configuration, explicit assignment or hook blocks, multiple handlers can exist for the same event type.

### Disable Animated GIFs

By default bots send animated GIFs in default commands and errors. To disable animated GIFs set `send_gifs` or `ENV['SLACK_RUBY_BOT_SEND_GIFS']` to `false`.

```ruby
SlackRubyBot.configure do |config|
config.send_gifs = false
end
```

### Message Loop Protection

By default bots do not respond to their own messages. If you wish to change that behavior, set `allow_message_loops` to `true`.
Expand Down
7 changes: 7 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
Upgrading SlackRubyBot
======================

### Upgrading to >= 0.8.3

#### Add giphy to your Gemfile for GIF support

The dependency on giphy was dropped, if you want GIF support you need to add `gem 'giphy'` to your **Gemfile**.
You can ignore this if you had already disabled GIFs for your bot.

### Upgrading to >= 0.8.0

#### Require a concurrency library
Expand Down
1 change: 0 additions & 1 deletion lib/config/boot.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,3 @@
require 'active_support/core_ext'
require 'hashie'
require 'slack'
require 'giphy'
11 changes: 7 additions & 4 deletions lib/initializers/giphy.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
require 'giphy'

Giphy::Configuration.configure do |config|
config.api_key = ENV['GIPHY_API_KEY'] || 'dc6zaTOxFJmzC' # from https://github.com/Giphy/GiphyAPI
begin
require 'giphy'
rescue LoadError
else
Giphy::Configuration.configure do |config|
config.api_key = ENV['GIPHY_API_KEY'] || 'dc6zaTOxFJmzC' # from https://github.com/Giphy/GiphyAPI
end
end
7 changes: 4 additions & 3 deletions lib/slack-ruby-bot/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class Client < Slack::RealTime::Client
def initialize(attrs = {})
super(attrs)
@aliases = attrs[:aliases]
@send_gifs = attrs.key?(:send_gifs) ? !!attrs[:send_gifs] : true
@send_gifs = attrs[:send_gifs]
end

def names
Expand All @@ -30,7 +30,8 @@ def name?(name)
end

def send_gifs?
send_gifs
return false unless defined?(Giphy)
send_gifs.nil? ? SlackRubyBot::Config.send_gifs? : send_gifs
end

def name
Expand All @@ -52,7 +53,7 @@ def say(options = {})
rescue StandardError => e
logger.warn "Giphy.random: #{e.message}"
nil
end if SlackRubyBot::Config.send_gifs? && send_gifs? && keywords
end if keywords && send_gifs?
text = [text, gif && gif.image_url.to_s].compact.join("\n")
message({ text: text }.merge(options))
end
Expand Down
3 changes: 2 additions & 1 deletion lib/slack-ruby-bot/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@ def allow_message_loops?
end

def send_gifs?
return false unless defined?(Giphy)
v = boolean_from_env('SLACK_RUBY_BOT_SEND_GIFS')
v.nil? ? (send_gifs.nil? || send_gifs) : v
send_gifs.nil? ? (v.nil? || v) : send_gifs
end

def reset!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

RSpec::Matchers.define :respond_with_error do |error, error_message|
match do |actual|
allow(Giphy).to receive(:random)

client = if respond_to?(:client)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is going to break downstream users upgrading the library. I think this needs to be put back and have a if defined?(Giphy) or one of those.

send(:client)
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
message_command = SlackRubyBot::Hooks::Message.new

channel, user, message = parse(actual)
allow(Giphy).to receive(:random)

expect(client).to receive(:message).with(channel: channel, text: expected)
message_command.call(client, Hashie::Mash.new(text: message, channel: channel, user: user))
Expand Down
6 changes: 6 additions & 0 deletions lib/slack-ruby-bot/rspec/support/slack_ruby_bot_configure.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
RSpec.configure do |config|
config.before :each do
allow(Giphy).to receive(:random) if ENV.key?('WITH_GIPHY')
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you fix the above allow this won't be necessary.

SlackRubyBot.configure do |c|
c.token = 'testtoken'
c.user = 'rubybot'
c.user_id = 'DEADBEEF'
end
end

config.after :each do
ENV.delete 'SLACK_RUBY_BOT_SEND_GIFS'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This doesn't look right. If we're not setting it before in this context we shouldn't be deleting it after.

SlackRubyBot::Config.reset!
end
end
2 changes: 1 addition & 1 deletion lib/slack-ruby-bot/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Server
def initialize(options = {})
@token = options[:token]
@aliases = options[:aliases]
@send_gifs = options.key?(:send_gifs) ? !!options[:send_gifs] : true
@send_gifs = options[:send_gifs]

# Hook Handling
flush_hook_blocks
Expand Down
1 change: 0 additions & 1 deletion slack-ruby-bot.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ Gem::Specification.new do |s|
s.summary = 'The easiest way to write a Slack bot in Ruby.'
s.add_dependency 'hashie'
s.add_dependency 'slack-ruby-client', '>= 0.6.0'
s.add_dependency 'giphy', '~> 2.0.2'
s.add_development_dependency 'rake'
s.add_development_dependency 'rspec'
s.add_development_dependency 'rack-test'
Expand Down
41 changes: 41 additions & 0 deletions spec/slack-ruby-bot/client_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
require 'spec_helper'

describe SlackRubyBot::Client do
describe '#send_gifs?' do
context 'without giphy is false', unless: ENV.key?('WITH_GIPHY') do
it 'by default' do
expect(subject.send_gifs?).to be false
end

it 'when set to true' do
subject.send_gifs = true
expect(subject.send_gifs?).to be false
end

it 'when set to true via config' do
SlackRubyBot::Config.send_gifs = true
expect(SlackRubyBot::Config.send_gifs?).to be false
end
end

context 'with giphy', if: ENV.key?('WITH_GIPHY') do
it 'default is true' do
expect(subject.send_gifs?).to be true
end

it 'defaults to SlackRubyBot::Config.send_gifs? if set' do
SlackRubyBot::Config.send_gifs = false
expect(subject.send_gifs?).to be false
end

it 'client setting takes precedence' do
SlackRubyBot::Config.send_gifs = true
subject.send_gifs = false
expect(subject.send_gifs?).to be false
SlackRubyBot::Config.send_gifs = false
subject.send_gifs = true
expect(subject.send_gifs?).to be true
end
end
end
end
1 change: 0 additions & 1 deletion spec/slack-ruby-bot/commands/empty_text_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ def self.call(client, data, _match)
end
end
it 'sends default text' do
allow(Giphy).to receive(:random)
expect(message: "#{SlackRubyBot.config.user} empty_text", channel: 'channel', user: 'user').to respond_with_slack_message('')
end
end
10 changes: 1 addition & 9 deletions spec/slack-ruby-bot/commands/message_loop_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ def app

before do
allow(client).to receive(:self).and_return(Hashie::Mash.new('id' => 'UDEADBEEF'))
allow(Giphy).to receive(:random)
end

context 'default' do
Expand All @@ -21,14 +20,7 @@ def app
end
context 'with allow_message_loops=true' do
before do
SlackRubyBot.configure do |config|
config.allow_message_loops = true
end
end
after do
SlackRubyBot.configure do |config|
config.allow_message_loops = nil
end
SlackRubyBot::Config.allow_message_loops = true
end
it 'responds to self' do
expect(client).to receive(:message)
Expand Down
1 change: 0 additions & 1 deletion spec/slack-ruby-bot/commands/nil_message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ def self.call(_client, data, _match)
let(:message_hook) { SlackRubyBot::Hooks::Message.new }

it 'ignores nil messages' do
allow(Giphy).to receive(:random)
expect(client).to_not receive(:message)
message_hook.call(client, Hashie::Mash.new(text: nil, channel: 'channel', user: 'user'))
end
Expand Down
4 changes: 3 additions & 1 deletion spec/slack-ruby-bot/commands/send_gif_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
require 'spec_helper'

describe SlackRubyBot::Commands do
describe SlackRubyBot::Commands, if: ENV.key?('WITH_GIPHY') do
let! :command do
SlackRubyBot::Config.send_gifs = true
Class.new(SlackRubyBot::Commands::Base) do
command 'send_gif_spec' do |client, data, _match|
client.say(channel: data.channel, gif: 'dummy')
Expand All @@ -10,6 +11,7 @@
end

let(:gif_image_url) { 'http://media2.giphy.com/media/pzOijFsdDrsS4/giphy.gif' }

it 'sends a gif' do
gif = Giphy::RandomGif.new('image_url' => gif_image_url)
expect(Giphy).to receive(:random).and_return(gif)
Expand Down
46 changes: 8 additions & 38 deletions spec/slack-ruby-bot/commands/send_message_with_gif_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
require 'spec_helper'

describe SlackRubyBot::Commands do
describe SlackRubyBot::Commands, if: ENV.key?('WITH_GIPHY') do
let! :command do
Class.new(SlackRubyBot::Commands::Base) do
command 'send_message_with_gif_spec' do |client, data, match|
Expand All @@ -16,7 +16,7 @@
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message("message\n#{gif_image_url}")
end

it 'eats up the error' do
it 'eats up errors' do
expect(Giphy).to receive(:random) { raise 'oh no!' }
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message('message')
end
Expand All @@ -26,44 +26,14 @@
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message('message')
end

context 'send_gifs' do
context 'set to false via client' do
let(:client) { SlackRubyBot::Client.new }

before do
client.send_gifs = false
end

it 'does not send a gif' do
expect(Giphy).to_not receive(:random)
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message('message')
end
end

context 'set to false via config' do
before do
SlackRubyBot::Config.send_gifs = false
end
it 'does not send a gif' do
expect(Giphy).to_not receive(:random)
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message('message')
end
after do
SlackRubyBot::Config.reset!
end
context 'when client.send_gifs is false' do
let :client do
SlackRubyBot::Client.new.tap { |c| c.send_gifs = false }
end

context 'set to false via SLACK_RUBY_BOT_SEND_GIFS' do
before do
ENV['SLACK_RUBY_BOT_SEND_GIFS'] = 'false'
end
it 'does not send a gif' do
expect(Giphy).to_not receive(:random)
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message('message')
end
after do
ENV.delete 'SLACK_RUBY_BOT_SEND_GIFS'
end
it 'does not send a gif' do
expect(Giphy).to_not receive(:random)
expect(message: "#{SlackRubyBot.config.user} send_message_with_gif_spec message").to respond_with_slack_message('message')
end
end
end
Loading