Improve edit message api (#46)

* EditMessage indicates room/message id are immutable

* Update CHANGELOG for pending 1.9.0

CHANGELOG.md

@@ -6,12 +6,14 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased](https://github.com/pusher/chatkit-server-ruby/compare/1.7.1...HEAD)]
 
-## [1.8.0](https://github.com/pusher/chatkit-server-ruby/compare/1.7.1...1.8.0)
+## [1.9.0](https://github.com/pusher/chatkit-server-ruby/compare/1.7.1...1.8.0)
 
 ### Additions
 
 - Adds message editing via `edit{_simple,_multipart,}_message`.
 
+## 1.8.0 Yanked
+
 ## [1.7.1](https://github.com/pusher/chatkit-server-ruby/compare/1.7.0...1.7.1)
 
 ### Fixes

lib/chatkit/client.rb

@@ -531,24 +531,32 @@ def delete_message(options)
       })
     end
 
-    def edit_simple_message(options)
+    def edit_simple_message(room_id, message_id, options)
       verify({text: "You must provide some text for the message",
              }, options)
 
       options[:parts] = [{type: "text/plain",
                           content: options[:text]
                          }]
 
-      edit_multipart_message(options)
+      edit_multipart_message(room_id, message_id, options)
     end
 
-    def edit_multipart_message(options)
+    def edit_multipart_message(room_id, message_id, options)
       verify({
-        room_id: "You must provide the ID of the room in which the message to edit belongs",
-        message_id: "You must provide the ID of the message to edit",
         sender_id: "You must provide the ID of the user editing the message",
         parts: "You must provide a parts array",
       }, options)
+      verify({
+        room_id: "You must provide the ID of the room in which the message to edit belongs",
+        message_id: "You must provide the ID of the message to edit",
+      }, {room_id: room_id, message_id: message_id})
+      if !options['room_id'].nil?
+        raise Chatkit::UnexpectedParameterError.new("a messages room id cannot be edited")
+      end
+      if !options['message_id'].nil?
+        raise Chatkit::UnexpectedParameterError.new("a messages id cannot be edited")
+      end
 
       if not options[:parts].length > 0
         raise Chatkit::MissingParameterError.new("parts array must have at least one item")
@@ -571,7 +579,7 @@ def edit_multipart_message(options)
             url: part[:url]
           }
         elsif !part[:file].nil?
-          attachment_id = upload_attachment(token, options[:room_id], part)
+          attachment_id = upload_attachment(token, room_id, part)
           {
             type: part[:type],
             attachment: {id: attachment_id},
@@ -583,18 +591,18 @@ def edit_multipart_message(options)
 
       api_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages/#{options[:message_id]}",
+        path: "/rooms/#{CGI::escape room_id}/messages/#{message_id}",
         body: {parts: request_parts},
         jwt: token
       })
     end
 
-    def edit_message(options)
-      if options[:room_id].nil?
+    def edit_message(room_id, message_id, options)
+      if room_id.nil?
         raise Chatkit::MissingParameterError.new("You must provide the ID of the room in which the message to edit belongs")
       end
 
-      if options[:message_id].nil?
+      if message_id.nil?
         raise Chatkit::MissingParameterError.new("You must provide the ID of the message to edit")
       end
 
@@ -606,6 +614,14 @@ def edit_message(options)
         raise Chatkit::MissingParameterError.new("You must provide some text for the message")
       end
 
+      if !options['room_id'].nil?
+        raise Chatkit::UnexpectedParameterError.new("a messages room id cannot be edited")
+      end
+      if !options['message_id'].nil?
+        raise Chatkit::UnexpectedParameterError.new("a messages id cannot be edited")
+      end
+
+
       attachment = options[:attachment]
 
       unless attachment.nil?
@@ -629,7 +645,7 @@ def edit_message(options)
 
       api_v2_request({
         method: "PUT",
-        path: "/rooms/#{CGI::escape options[:room_id]}/messages/#{options[:message_id]}",
+        path: "/rooms/#{CGI::escape room_id}/messages/#{message_id}",
         body: payload,
         jwt: generate_su_token({ user_id: options[:sender_id] })[:token]
       })

lib/chatkit/unexpected_parameter_error.rb

@@ -0,0 +1,16 @@
+require_relative './error'
+
+module Chatkit
+  class UnexpectedParameterError < Chatkit::Error
+    attr_accessor :message
+
+    def initialize(message)
+      @message = message
+    end
+
+    def to_s
+      "Chatkit::UnexpectedParameterError - #{@message}"
+    end
+
+  end
+end

spec/client_spec.rb

@@ -1448,34 +1448,32 @@
     describe "should raise a MissingParameterError if" do
       it "no room_id is provided" do
         expect {
-          @chatkit.edit_message({ text: 'hi', sender_id: 'ham', message_id: 42 })
+          @chatkit.edit_message(nil, 42, { text: 'hi', sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no message_id is provided" do
         expect {
-          @chatkit.edit_message({ text: 'hi', sender_id: 'ham', room_id: "123" })
+          @chatkit.edit_message("123", nil, { text: 'hi', sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no sender_id is provided" do
         expect {
-          @chatkit.edit_message({ text: 'hi', room_id: "123", message_id: 42 })
+          @chatkit.edit_message("123", 42, { text: 'hi' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no text is provided" do
         expect {
-          @chatkit.edit_message({ sender_id: 'ham', room_id: "123", message_id: 42 })
+          @chatkit.edit_message("123", 42, { sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no resource_link is provided for a message with an attachment" do
         expect {
-          @chatkit.edit_message({
+          @chatkit.edit_message("123", 42, {
             sender_id: 'ham',
-            room_id: "123",
-            message_id: 42,
             text: 'test',
             attachment: {
               type: 'image'
@@ -1486,9 +1484,8 @@
 
       it "no type is provided for a message with an attachment" do
         expect {
-          @chatkit.edit_message({
+          @chatkit.edit_message("123", 42, {
             sender_id: 'ham',
-            room_id: "123",
             text: 'test',
             attachment: {
               resource_link: 'https://placekitten.com/200/300'
@@ -1499,10 +1496,8 @@
 
       it "an invalid type is provided for a message with an attachment" do
         expect {
-          @chatkit.edit_message({
+          @chatkit.edit_message("123", 42, {
             sender_id: 'ham',
-            room_id: "123",
-            message_id: 42,
             text: 'test',
             attachment: {
               resource_link: 'https://placekitten.com/200/300',
@@ -1530,9 +1525,7 @@
         expect(send_message_res[:status]).to eq 201
         expect(send_message_res[:body]).to have_key :message_id
 
-        edit_message_res = @chatkit.edit_message({
-          room_id: room_res[:body][:id],
-          message_id: send_message_res[:body][:message_id],
+        edit_message_res = @chatkit.edit_message(room_res[:body][:id], send_message_res[:body][:message_id], {
           sender_id: user_id,
           text: 'hi 1 edited'
         })
@@ -1559,9 +1552,7 @@
         expect(send_message_res[:status]).to eq 201
         expect(send_message_res[:body]).to have_key :message_id
 
-        edit_message_res = @chatkit.edit_message({
-          room_id: room_res[:body][:id],
-          message_id: send_message_res[:body][:message_id],
+        edit_message_res = @chatkit.edit_message(room_res[:body][:id], send_message_res[:body][:message_id],{
           sender_id: user_id,
           text: 'hi 1',
           attachment: {
@@ -1578,25 +1569,25 @@
     describe "should raise a MissingParameterError if" do
       it "no room_id is provided" do
         expect {
-          @chatkit.edit_simple_message({ text: 'hi', sender_id: 'ham', message_id: 42 })
+          @chatkit.edit_simple_message(nil, 42, { text: 'hi', sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no message_id is provided" do
         expect {
-          @chatkit.edit_simple_message({ text: 'hi', room_id: "123", sender_id: 'ham' })
+          @chatkit.edit_simple_message("123", nil, { text: 'hi', sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no sender_id is provided" do
         expect {
-          @chatkit.edit_simple_message({ text: 'hi', room_id: "123", message_id: 42 })
+          @chatkit.edit_simple_message("123", 42, { text: 'hi' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no text is provided" do
         expect {
-          @chatkit.edit_simple_message({ sender_id: 'ham', room_id: "123", message_id: 42 })
+          @chatkit.edit_simple_message("123", 42, { sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
     end
@@ -1618,9 +1609,7 @@
         expect(send_message_res[:status]).to eq 201
         expect(send_message_res[:body]).to have_key :message_id
 
-        edit_message_res = @chatkit.edit_simple_message({
-          room_id: room_res[:body][:id],
-          message_id: send_message_res[:body][:message_id],
+        edit_message_res = @chatkit.edit_simple_message(room_res[:body][:id], send_message_res[:body][:message_id], {
           sender_id: user_id,
           text: 'hi 1 - edited'
         })
@@ -1651,45 +1640,41 @@
     describe "should raise a MissingParameterError if" do
       it "no room_id is provided" do
         expect {
-          @chatkit.edit_multipart_message({ sender_id: 'ham', message_id: 42, parts:  good_parts })
+          @chatkit.edit_multipart_message(nil, 42, { sender_id: 'ham', parts:  good_parts })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no message_id is provided" do
         expect {
-          @chatkit.edit_multipart_message({ sender_id: 'ham', room_id: "123", parts:  good_parts })
+          @chatkit.edit_multipart_message("123", nil, { sender_id: 'ham', parts:  good_parts })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no sender_id is provided" do
         expect {
-          @chatkit.edit_multipart_message({ room_id: "123", message_id: 42, parts: good_parts })
+          @chatkit.edit_multipart_message("123", 42, { room_id: "123", parts: good_parts })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no parts are provided" do
         expect {
-          @chatkit.edit_multipart_message({ sender_id: 'ham', room_id: "123", message_id: 42 })
+          @chatkit.edit_multipart_message("123", 42, { sender_id: 'ham' })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "no type is provided for a part" do
         expect {
-          @chatkit.edit_multipart_message(
+          @chatkit.edit_multipart_message( "123", 42,
             {sender_id: 'ham',
-             room_id: "123",
-             message_id: 42,
              parts: [{ content: 'test' }]
             })
         }.to raise_error Chatkit::MissingParameterError
       end
 
       it "only type is provided for a part" do
         expect {
-          @chatkit.edit_multipart_message(
+          @chatkit.edit_multipart_message("123", 42,
             {sender_id: 'ham',
-             room_id: "123",
-             message_id: 42,
              parts: [{ type: 'text/plain' }]
             })
         }.to raise_error Chatkit::MissingParameterError
@@ -1713,9 +1698,7 @@
         expect(send_message_res[:status]).to eq 201
         expect(send_message_res[:body]).to have_key :message_id
 
-        edit_message_res = @chatkit.edit_multipart_message({
-          room_id: room_res[:body][:id],
-          message_id: send_message_res[:body][:message_id],
+        edit_message_res = @chatkit.edit_multipart_message(room_res[:body][:id], send_message_res[:body][:message_id], {
           sender_id: user_id,
           parts: good_edit_parts
         })