Merge pull request #45 from github/add-message-id

Proposal to add an optional message id to the CRPC protocol

Author: mistydemeo

README.md

@@ -95,7 +95,7 @@ always be the login of the user typing the command, and `room_id` will be where
 it was typed.
 The optional `mention_slug` parameter will provide the name to use to refer to
 the user when sending a message; this may or may not be the same thing as the
-username, depending on the chat system being used.
+username, depending on the chat system being used. The optional `message_id` parameter will provide a reference to the message that invoked the rpc.
 
 You can return `jsonrpc_success` with a string to return text to chat. If you
 have an input validation or other handle-able error, you can use

docs/protocol-description.md

@@ -44,6 +44,7 @@ invocation. A method invocation is a JSON object with the following fields:
 
  * `user`: A slug username corresponding to to the command giver's GitHub login.
  * `mention_slug`: Optional. If provided, a string which should be used to mention the user when sending a message in response. For example, Slack requires that users be mentioned using user IDs instead of usernames.
+ * `message_id`: Optional. If provided, an id that uniquely identifies the message that generated the CRPC call. Useful for linking back to the original command to provide context.
  * `room_id`: A slug room name where the command originated.
  * `method`: The method name, without namespace, of the matching regex.
  * `params`: A mapping of parameter names to matches extracted from named capture groups in the command's regex. Parameters that are empty or null should not be passed.

lib/chatops/controller.rb

@@ -57,7 +57,7 @@ def setup_params!
 
       @jsonrpc_params = params.delete(:params) if params.has_key? :params
 
-      self.params = params.permit(:action, :chatop, :controller, :id, :mention_slug, :method, :room_id, :user)
+      self.params = params.permit(:action, :chatop, :controller, :id, :mention_slug, :message_id, :method, :room_id, :user)
     end
 
     def jsonrpc_params

lib/chatops/controller/test_case_helpers.rb

@@ -21,12 +21,14 @@ def chatop(method, params = {})
     user = args.delete :user
     room_id = args.delete :room_id
     mention_slug = args.delete :mention_slug
+    message_id = args.delete :message_id
 
     params = {
       :params => args,
       :room_id => room_id,
       :user => user,
       :mention_slug => mention_slug,
+      :message_id => message_id,
     }
 
     major_version = Rails.version.split('.')[0].to_i
@@ -37,7 +39,7 @@ def chatop(method, params = {})
     end
   end
 
-  def chat(message, user, room_id = "123")
+  def chat(message, user, room_id = "123", message_id = "456")
     get :list
     json_response = JSON.load(response.body)
     matchers = json_response["methods"].map { |name, metadata|
@@ -59,7 +61,7 @@ def chat(message, user, room_id = "123")
     matcher["params"].each do |param|
       jsonrpc_params[param] ||= match_data[param.to_sym]
     end
-    jsonrpc_params.merge!(user: user, room_id: room_id, mention_slug: user)
+    jsonrpc_params.merge!(user: user, room_id: room_id, mention_slug: user, message_id: message_id)
     chatop matcher["name"].to_sym, jsonrpc_params
   end
 

spec/lib/chatops/controller_spec.rb

@@ -25,6 +25,13 @@
       jsonrpc_success "You just foo and bar like it just don't matter"
     end
 
+    chatop :proxy_parameters,
+    /(?:proxy_parameters)/,
+    "proxy parameters back to test" do
+      response = { :params => params, :jsonrpc_params => jsonrpc_params }.to_json
+      jsonrpc_success response
+    end
+
     skip_before_action :ensure_method_exists, only: :non_chatop_method
     def non_chatop_method
       render :plain => "Why would you have something thats not a chatop?"
@@ -228,6 +235,12 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil)
             "regex" => /(?:how can i foo and bar all at once)?/.source,
             "params" => [],
             "path" => "foobar"
+          },
+          "proxy_parameters" => {
+            "help" => "proxy parameters back to test",
+            "regex" => /(?:proxy_parameters)/.source,
+            "params" => [],
+            "path" => "proxy_parameters"
           }
         },
         "version" => "3"
@@ -293,6 +306,26 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil)
       expect(response.status).to eq 200
     end
 
+    it "passes all expected paramters" do
+      rails_flexible_post :execute_chatop, {
+        :chatop => "proxy_parameters",
+        :user => "foo",
+        :mention_slug => "mention_slug_here",
+        :message_id => "message_id_here",
+        :room_id => "#someroom",
+        :unknown_key => "few" # This should get ignored
+      }, {
+       "app" => "foo" 
+      }
+      expect(json_response).to eq({
+        "jsonrpc" => "2.0",
+        "id" => nil,
+        "result" => "{\"params\":{\"action\":\"proxy_parameters\",\"chatop\":\"proxy_parameters\",\"controller\":\"anonymous\",\"mention_slug\":\"mention_slug_here\",\"message_id\":\"message_id_here\",\"room_id\":\"#someroom\",\"user\":\"foo\"},\"jsonrpc_params\":{\"app\":\"foo\"}}"
+      })
+      expect(response.status).to eq 200
+    end 
+
+
     it "uses typical controller fun like before_action" do
       rails_flexible_post :execute_chatop, :chatop => "wcid", :user => "foo"
       expect(json_response).to eq({
@@ -360,6 +393,15 @@ def rails_flexible_post(path, outer_params, jsonrpc_params = nil)
         expect(request.params["params"]["this-is-sparta"]).to eq "true"
       end
 
+      it "sends along all the parameters" do
+        chat "where can i deploy foobar", "my_username", "room_id_5", "message_id_6"
+        expect(request.params["action"]).to eq "execute_chatop"
+        expect(request.params["chatop"]).to eq "wcid"
+        expect(request.params["user"]).to eq "my_username"
+        expect(request.params["room_id"]).to eq "room_id_5"
+        expect(request.params["message_id"]).to eq "message_id_6"
+      end
+
       it "anchors regexes" do
         expect {
           chat "too bad that this message doesn't start with where can i deploy foobar", "bhuga"