Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
9 changes: 9 additions & 0 deletions app/controllers/application_chatops_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,13 @@ def verify_slack_hmac
rescue Slack::Events::Request::MissingSigningSecret, Slack::Events::Request::InvalidSignature, Slack::Events::Request::TimestampExpired
render plain: "Nope.", status: :unauthorized
end

def subcommand
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This returns the subcommand for the given request. For /doubleup list this would be "list"

Utils::ParsesCliStyleCommandArgs.new.call(text: params[:text]).subcommand
end

def command_params
Copy link
Copy Markdown
Collaborator Author

@cpruitt cpruitt Dec 15, 2021

Choose a reason for hiding this comment

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

The intent for this method is for arguments passed to a slash command to feel like typical rails params hash.

parsed = Utils::ParsesCliStyleCommandArgs.new.call(text: params[:text])
ActionController::Parameters.new(parsed.args)
end
end
24 changes: 24 additions & 0 deletions app/controllers/chatops/slack_slash_command_list_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
module Chatops
class SlackSlashCommandListController < ::ApplicationChatopsController
def handle
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This should be treated as a minimal first pass to be iterated on. Slack does not turn channels into links when the channel name is used. Instead we need to use the channel name to look up the channel ID and use that, like "<##{grouping.channel_id}>. I'm opting to leave this out of the current PR since we're planning to move to a DB backed model where we can cache the channel id.

config = Matchmaking.config
message = config.to_h.keys.sort.each_with_object([]) do |key, result|
grouping = config[key]
next unless include_grouping_in_list?(grouping)
result << "*#{key.to_s.titleize}*: Meets #{grouping.schedule} in groups of #{grouping.size} (Join: ##{grouping.channel})"
end.join("\n")

if config.to_h.keys.any?
render plain: message
else
render plain: "Sorry! There are no configured channels for groupings."
end
end

private

def include_grouping_in_list?(grouping)
grouping.active || command_params.key?(:all)
end
end
end
6 changes: 6 additions & 0 deletions app/lib/matchmaking.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module Matchmaking
def config
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Despite my growing distaste for Rails' config.x., that's not why I added this. 😂 I'd prefer to stub our own methods instead of stubbing something from the framework so this lets us to allow(Matchmaking).to receive(:config).and_return({}) which is a bit more tidy. It adds a layer of indirection but I don't think it's too bad.

Rails.application.config.x.matchmaking
end
module_function :config
end
12 changes: 12 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
class SlackSlashSubcommandConstraint
Copy link
Copy Markdown
Collaborator Author

@cpruitt cpruitt Dec 15, 2021

Choose a reason for hiding this comment

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

Constraint allows us to route different subcommands to different controllers instead of building up some big case statement based on subcommands. There are no specific tests for this but it should be covered by the Request specs that make sure the post works.

def initialize(matches_subcommand:)
@subcommand = matches_subcommand
end

def matches?(request)
parsed = Utils::ParsesCliStyleCommandArgs.new.call(text: request.params["text"])
parsed.subcommand == @subcommand
end
end

Rails.application.routes.draw do
if Rails.env.test?
TestOnlyRoutes = ActionDispatch::Routing::RouteSet.new unless defined?(::TestOnlyRoutes)
mount TestOnlyRoutes, at: "/"
end

post "/command/doubleup", to: "chatops/slack_slash_command_list#handle", constraints: SlackSlashSubcommandConstraint.new(matches_subcommand: "channels:list")
post "/command/doubleup", to: "chatops/slack_slash_command#handle"
end
161 changes: 161 additions & 0 deletions spec/requests/chatops/slack_slash_command_list_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
require "rails_helper"

RSpec.describe "SlackSlashCommandListController", type: :request do
scenario "responds with default message when no channels configured" do
allow(Matchmaking).to receive(:config).and_return({})

signed = signed_request_body(params: {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Proposing this as a simple helper to manage the params in tests (see private method in this file below). signed_request_body returns an object that gives us the params (body) as a string instead of a hash, a signature, and the timestamp used to produce the signature. These can then be used to make the test request.

If this patterns works then we can extract this into a test helper method and update the other tests.

"team_id" => "T02PF6RHYSY",
"team_domain" => "testdouble-hq",
"channel_id" => "C02NYBB3VPH",
"channel_name" => "some-channel",
"user_id" => "U02PRHH0XEV",
"user_name" => "cliff.pruitt",
"command" => "/doubleup",
"text" => "channels:list",
"api_app_id" => "A02PD0DUE03",
"is_enterprise_install" => "false",
"response_url" =>
"https://hooks.slack.com/commands/T02PF6RHYSY/2823421496992/0WC0HfWeGJpHetxmF8yUmawo",
"trigger_id" => "2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
})

request_headers = {
"X-Slack-Signature" => signed.signature,
"X-Slack-Request-Timestamp" => signed.timestamp
}

post "/command/doubleup",
params: signed.body,
headers: request_headers

expect(response).to have_http_status(:ok)
expect(response.body).to eq("Sorry! There are no configured channels for groupings.")
end

scenario "responds with list of only active channels" do
allow(Matchmaking).to receive(:config).and_return(OpenStruct.new({
active_one: OpenStruct.new({
active: true,
schedule: "weekly",
size: 2,
channel: "active-1"
}),
not_active_one: OpenStruct.new({
active: false,
schedule: "weekly",
size: 2,
channel: "not-active-1"
}),
active_two: OpenStruct.new({
active: true,
schedule: "weekly",
size: 2,
channel: "active-2"
})
}))

signed = signed_request_body(params: {
"team_id" => "T02PF6RHYSY",
"team_domain" => "testdouble-hq",
"channel_id" => "C02NYBB3VPH",
"channel_name" => "some-channel",
"user_id" => "U02PRHH0XEV",
"user_name" => "cliff.pruitt",
"command" => "/doubleup",
"text" => "channels:list",
"api_app_id" => "A02PD0DUE03",
"is_enterprise_install" => "false",
"response_url" =>
"https://hooks.slack.com/commands/T02PF6RHYSY/2823421496992/0WC0HfWeGJpHetxmF8yUmawo",
"trigger_id" => "2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
})

request_headers = {
"X-Slack-Signature" => signed.signature,
"X-Slack-Request-Timestamp" => signed.timestamp
}

post "/command/doubleup",
params: signed.body,
headers: request_headers

expect(response).to have_http_status(:ok)
expected_response = <<~MSG.chomp
*Active One*: Meets weekly in groups of 2 (Join: #active-1)
*Active Two*: Meets weekly in groups of 2 (Join: #active-2)
MSG
expect(response.body).to eq(expected_response)
end

scenario "responds with list of active and inactive channels with --all" do
allow(Matchmaking).to receive(:config).and_return(OpenStruct.new({
active_one: OpenStruct.new({
active: true,
schedule: "weekly",
size: 2,
channel: "active-1"
}),
not_active_one: OpenStruct.new({
active: false,
schedule: "weekly",
size: 2,
channel: "not-active-1"
}),
active_two: OpenStruct.new({
active: true,
schedule: "weekly",
size: 2,
channel: "active-2"
})
}))

signed = signed_request_body(params: {
"team_id" => "T02PF6RHYSY",
"team_domain" => "testdouble-hq",
"channel_id" => "C02NYBB3VPH",
"channel_name" => "some-channel",
"user_id" => "U02PRHH0XEV",
"user_name" => "cliff.pruitt",
"command" => "/doubleup",
"text" => "channels:list --all",
"api_app_id" => "A02PD0DUE03",
"is_enterprise_install" => "false",
"response_url" =>
"https://hooks.slack.com/commands/T02PF6RHYSY/2823421496992/0WC0HfWeGJpHetxmF8yUmawo",
"trigger_id" => "2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
})

request_headers = {
"X-Slack-Signature" => signed.signature,
"X-Slack-Request-Timestamp" => signed.timestamp
}

post "/command/doubleup",
params: signed.body,
headers: request_headers

expect(response).to have_http_status(:ok)
expected_response = <<~MSG.chomp
*Active One*: Meets weekly in groups of 2 (Join: #active-1)
*Active Two*: Meets weekly in groups of 2 (Join: #active-2)
*Not Active One*: Meets weekly in groups of 2 (Join: #not-active-1)
MSG
expect(response.body).to eq(expected_response)
end

private

def signed_request_body(params: {})
slack_signing_secret = Slack::Events.config.signing_secret
timestamp = Time.zone.now.to_i
request_body = params.to_param
data = ["v0", timestamp, request_body].join(":")
mac = OpenSSL::HMAC.hexdigest("SHA256", slack_signing_secret, data)
OpenStruct.new({
signature: "v0=#{mac}",
timestamp: timestamp,
body: request_body
})
end
end
95 changes: 95 additions & 0 deletions spec/requests/chatops_controller_command_params_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
require "rails_helper"

class CommandParamsChatopsController < ApplicationChatopsController
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Testing slash command params method in something like isolation.

def index
render plain: "Got command <#{subcommand}> with name <#{command_params[:name]}> and group size <#{command_params[:group_size]}>"
end
end

RSpec.describe "ApplicationChatopsController command params", type: :request do
before :all do
TestOnlyRoutes.draw do
post "/chatops/test-params", to: "command_params_chatops#index"
end
end

after :all do
TestOnlyRoutes.clear!
end

scenario "provides subcommand and command_params to actions" do
slack_signing_secret = Slack::Events.config.signing_secret
timestamp = Time.zone.now.to_i
request_body = "token=#{slack_signing_secret}&team_id=T02PF6RHYSY&team_domain=testdouble-hq&channel_id=C02NYBB3VPH&channel_name=some-channel&user_id=U02PRHH0XEV&user_name=cliff.pruitt&command=%2Fdoubleup&text=snazzy%3Acommand+--name%3Dblargh+--group-size%3D4&api_app_id=A02PD0DUE03&is_enterprise_install=false&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT02PF6RHYSY%2F2823421496992%2F0WC0HfWeGJpHetxmF8yUmawo&trigger_id=2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
data = ["v0", timestamp, request_body].join(":")
mac = OpenSSL::HMAC.hexdigest("SHA256", slack_signing_secret, data)
signature = "v0=#{mac}"

request_headers = {
"X-Slack-Signature" => signature,
"X-Slack-Request-Timestamp" => timestamp
}

request_params = {
"token" => slack_signing_secret,
"team_id" => "T02PF6RHYSY",
"team_domain" => "testdouble-hq",
"channel_id" => "C02NYBB3VPH",
"channel_name" => "some-channel",
"user_id" => "U02PRHH0XEV",
"user_name" => "cliff.pruitt",
"command" => "/doubleup",
"text" => "snazzy:command --name=blargh --group-size=4",
"api_app_id" => "A02PD0DUE03",
"is_enterprise_install" => "false",
"response_url" =>
"https://hooks.slack.com/commands/T02PF6RHYSY/2823421496992/0WC0HfWeGJpHetxmF8yUmawo",
"trigger_id" => "2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
}

post "/chatops/test-params",
params: request_params,
headers: request_headers

expect(response).to have_http_status(:ok)
expect(response.body).to eq("Got command <snazzy:command> with name <blargh> and group size <4>")
end

scenario "provides subcommand and command_params to actions for empty params" do
slack_signing_secret = Slack::Events.config.signing_secret
timestamp = Time.zone.now.to_i
request_body = "token=#{slack_signing_secret}&team_id=T02PF6RHYSY&team_domain=testdouble-hq&channel_id=C02NYBB3VPH&channel_name=some-channel&user_id=U02PRHH0XEV&user_name=cliff.pruitt&command=%2Fdoubleup&text=&api_app_id=A02PD0DUE03&is_enterprise_install=false&response_url=https%3A%2F%2Fhooks.slack.com%2Fcommands%2FT02PF6RHYSY%2F2823421496992%2F0WC0HfWeGJpHetxmF8yUmawo&trigger_id=2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
data = ["v0", timestamp, request_body].join(":")
mac = OpenSSL::HMAC.hexdigest("SHA256", slack_signing_secret, data)
signature = "v0=#{mac}"

request_headers = {
"X-Slack-Signature" => signature,
"X-Slack-Request-Timestamp" => timestamp
}

request_params = {
"token" => slack_signing_secret,
"team_id" => "T02PF6RHYSY",
"team_domain" => "testdouble-hq",
"channel_id" => "C02NYBB3VPH",
"channel_name" => "some-channel",
"user_id" => "U02PRHH0XEV",
"user_name" => "cliff.pruitt",
"command" => "/doubleup",
"text" => "",
"api_app_id" => "A02PD0DUE03",
"is_enterprise_install" => "false",
"response_url" =>
"https://hooks.slack.com/commands/T02PF6RHYSY/2823421496992/0WC0HfWeGJpHetxmF8yUmawo",
"trigger_id" => "2801968477828.2797229610916.883001cf5008d6d02fd58a3cf70f449e"
}

post "/chatops/test-params",
params: request_params,
headers: request_headers

expect(response).to have_http_status(:ok)
expect(response.body).to eq("Got command <> with name <> and group size <>")
end
end