diff --git a/app/controllers/concerns/raw_request_body.rb b/app/controllers/concerns/raw_request_body.rb new file mode 100644 index 0000000..0a35b8b --- /dev/null +++ b/app/controllers/concerns/raw_request_body.rb @@ -0,0 +1,12 @@ +module RawRequestBody + extend ActiveSupport::Concern + + private + + def raw_request_body + request.body.rewind + request.body.read.force_encoding("UTF-8") + ensure + request.body.rewind + end +end diff --git a/app/controllers/messages/boosts/by_bots_controller.rb b/app/controllers/messages/boosts/by_bots_controller.rb index 85f93ae..10ee80a 100644 --- a/app/controllers/messages/boosts/by_bots_controller.rb +++ b/app/controllers/messages/boosts/by_bots_controller.rb @@ -1,33 +1,33 @@ -class Messages::Boosts::ByBotsController < ApplicationController +class Messages::Boosts::ByBotsController < Messages::BoostsController + include RawRequestBody + allow_bot_access only: :create + before_action :ensure_content_present + def create - set_message - @boost = @message.boosts.create!(content: read_body) + @boost = @message.boosts.create!(boost_params) broadcast_create - head :created - rescue ActiveRecord::RecordNotFound - head :not_found + render :show, status: :created end private def set_message - @room = Current.user.rooms.find(params[:room_id]) - @message = @room.messages.find(params[:message_id]) + if room = Current.user.rooms.find_by(id: params[:room_id]) + @message = room.messages.find_by(id: params[:message_id]) + end + + head :not_found unless @message end - def read_body - request.body.rewind - request.body.read.force_encoding("UTF-8") - ensure - request.body.rewind + def ensure_content_present + if raw_request_body.blank? + head :unprocessable_content + end end - def broadcast_create - @boost.broadcast_append_to @boost.message.room, :messages, - target: "boosts_message_#{@boost.message.client_message_id}", - partial: "messages/boosts/boost", - attributes: { maintain_scroll: true } + def boost_params + { content: raw_request_body } end end diff --git a/app/controllers/messages/by_bots_controller.rb b/app/controllers/messages/by_bots_controller.rb index ff398ca..f225e51 100644 --- a/app/controllers/messages/by_bots_controller.rb +++ b/app/controllers/messages/by_bots_controller.rb @@ -1,73 +1,57 @@ class Messages::ByBotsController < MessagesController + include RawRequestBody + allow_bot_access only: %i[ index create ] + before_action :set_room + before_action :ensure_body_or_attachment_present, only: :create + def index - set_room @messages = find_paged_messages - render json: messages_as_json(@messages) - rescue ActiveRecord::RecordNotFound - head :not_found + set_pagination_headers end def create - set_room - @message = @room.messages.create_with_attachment!(message_params) - @message.broadcast_create - deliver_webhooks_to_bots + super head :created, location: message_url(@message) - rescue ActiveRecord::RecordNotFound - head :not_found end private - def messages_as_json(messages) - { - room: { - id: @room.id, - name: @room.name - }, - messages: messages.map { |m| message_as_json(m) }, - pagination: pagination_info(messages) - } + def set_room + @room = Current.user.rooms.find_by(id: params[:room_id]) + + head :not_found unless @room end - def message_as_json(message) - { - id: message.id, - body: { - plain: message.plain_text_body, - html: message.body&.body&.to_s - }, - created_at: message.created_at.iso8601, - creator: { - id: message.creator.id, - name: message.creator.name, - is_bot: message.creator.bot? - } - } + def ensure_body_or_attachment_present + if params[:attachment].blank? && raw_request_body.blank? + head :unprocessable_content + end end - def pagination_info(messages) - return {} if messages.empty? - { - oldest_id: messages.last.id, - newest_id: messages.first.id, - has_more: messages.size == Message::PAGE_SIZE - } + def set_pagination_headers + headers["X-Total-Count"] = @room.messages.count.to_s + + if next_page = next_page_params + headers["Link"] = %(<#{room_bot_messages_url(@room, params[:bot_key], **next_page)}>; rel="next") + end + end + + def next_page_params + if @messages.any? + if params[:after].present? + { after: @messages.last.id } if @room.messages.after(@messages.last).exists? + else + { before: @messages.first.id } if @room.messages.before(@messages.first).exists? + end + end end def message_params if params[:attachment] params.permit(:attachment) else - reading(request.body) { |body| { body: body } } + { body: raw_request_body } end end - - def reading(io) - io.rewind - yield io.read.force_encoding("UTF-8") - ensure - io.rewind - end end diff --git a/app/views/messages/_message.json.jbuilder b/app/views/messages/_message.json.jbuilder new file mode 100644 index 0000000..e8abfa6 --- /dev/null +++ b/app/views/messages/_message.json.jbuilder @@ -0,0 +1,18 @@ +json.cache! message do + json.(message, :id) + + json.created_at message.created_at.utc + + json.body do + json.plain_text message.plain_text_body + json.html message.body.to_s + end + + json.creator message.creator, partial: "users/user", as: :user + + json.room do + json.id message.room_id + end + + json.url room_message_url(message.room, message) +end diff --git a/app/views/messages/boosts/_boost.json.jbuilder b/app/views/messages/boosts/_boost.json.jbuilder new file mode 100644 index 0000000..c3392b6 --- /dev/null +++ b/app/views/messages/boosts/_boost.json.jbuilder @@ -0,0 +1,12 @@ +json.cache! boost do + json.(boost, :id, :content) + + json.created_at boost.created_at.utc + + json.booster boost.booster, partial: "users/user", as: :user + + json.message do + json.id boost.message_id + json.url room_message_url(boost.message.room, boost.message) + end +end diff --git a/app/views/messages/boosts/by_bots/show.json.jbuilder b/app/views/messages/boosts/by_bots/show.json.jbuilder new file mode 100644 index 0000000..a3acc9e --- /dev/null +++ b/app/views/messages/boosts/by_bots/show.json.jbuilder @@ -0,0 +1 @@ +json.partial! "messages/boosts/boost", boost: @boost diff --git a/app/views/messages/by_bots/index.json.jbuilder b/app/views/messages/by_bots/index.json.jbuilder new file mode 100644 index 0000000..27db314 --- /dev/null +++ b/app/views/messages/by_bots/index.json.jbuilder @@ -0,0 +1 @@ +json.array! @messages, partial: "messages/message", as: :message diff --git a/app/views/users/_user.json.jbuilder b/app/views/users/_user.json.jbuilder new file mode 100644 index 0000000..4425302 --- /dev/null +++ b/app/views/users/_user.json.jbuilder @@ -0,0 +1,5 @@ +json.cache! user do + json.(user, :id, :name, :role) + + json.avatar_url fresh_user_avatar_url(user) +end diff --git a/config/routes.rb b/config/routes.rb index 26afbcb..03466a1 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -62,10 +62,13 @@ Rails.application.routes.draw do resources :rooms do resources :messages - # Bot API endpoints - authenticated via bot_key in URL - get ":bot_key/messages", to: "messages/by_bots#index", as: :bot_messages_index - post ":bot_key/messages", to: "messages/by_bots#create", as: :bot_messages - post ":bot_key/messages/:message_id/boosts", to: "messages/boosts/by_bots#create", as: :bot_message_boosts + nested do + scope path: ":bot_key", as: :bot, defaults: { format: :json } do + resources :messages, controller: "messages/by_bots", only: %i[ index create ] do + resources :boosts, controller: "messages/boosts/by_bots", only: :create + end + end + end scope module: "rooms" do resource :refresh, only: :show diff --git a/test/controllers/messages/boosts/by_bots_controller_test.rb b/test/controllers/messages/boosts/by_bots_controller_test.rb index 09dfd86..a2487e5 100644 --- a/test/controllers/messages/boosts/by_bots_controller_test.rb +++ b/test/controllers/messages/boosts/by_bots_controller_test.rb @@ -3,27 +3,29 @@ require "test_helper" class Messages::Boosts::ByBotsControllerTest < ActionDispatch::IntegrationTest setup do @room = rooms(:watercooler) - @message = messages(:fourth) # Message in watercooler room where bender bot is a member + @message = messages(:fourth) @bot = users(:bender) end - test "create adds a boost to the message" do + test "create adds a boost to the message and returns it" do assert_difference -> { @message.boosts.count }, +1 do post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"👀" assert_response :created end - assert_equal "👀", @message.boosts.last.content + boost = @message.boosts.last + assert_equal "👀", boost.content + assert_equal @bot, boost.booster + + json = JSON.parse(response.body) + assert_equal boost.id, json["id"] + assert_equal "👀", json["content"] + assert_equal @bot.id, json["booster"]["id"] + assert_equal @message.id, json["message"]["id"] + assert_equal room_message_url(@room, @message), json["message"]["url"] end - test "create with emoji reaction" do - assert_difference -> { Boost.count }, +1 do - post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"🎉" - assert_response :created - end - end - - test "create with text reaction" do + test "create with text content" do assert_difference -> { Boost.count }, +1 do post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +"Nice!" assert_response :created @@ -38,35 +40,39 @@ class Messages::Boosts::ByBotsControllerTest < ActionDispatch::IntegrationTest end end - test "create requires valid bot key" do + test "create without content" do + assert_no_difference -> { Boost.count } do + post room_bot_message_boosts_url(@room, @bot.bot_key, @message) + assert_response :unprocessable_content + + post room_bot_message_boosts_url(@room, @bot.bot_key, @message), params: +" " + assert_response :unprocessable_content + end + end + + test "create requires a valid bot key" do assert_no_difference -> { Boost.count } do post room_bot_message_boosts_url(@room, "invalid-bot-key", @message), params: +"👀" end - assert_response :redirect # Redirects to login + assert_response :redirect end - test "create returns not_found for room bot is not a member of" do - room_without_bot = rooms(:designers) - message_in_other_room = messages(:first) # Message in designers room - + test "create is not found for a room the bot is not a member of" do assert_no_difference -> { Boost.count } do - post room_bot_message_boosts_url(room_without_bot, @bot.bot_key, message_in_other_room), params: +"👀" + post room_bot_message_boosts_url(rooms(:designers), @bot.bot_key, messages(:first)), params: +"👀" end assert_response :not_found end - test "create returns not_found for message not in the room" do - message_in_other_room = messages(:first) # Message in designers room, not watercooler - + test "create is not found for a message outside the room" do assert_no_difference -> { Boost.count } do - post room_bot_message_boosts_url(@room, @bot.bot_key, message_in_other_room), params: +"👀" + post room_bot_message_boosts_url(@room, @bot.bot_key, messages(:first)), params: +"👀" end assert_response :not_found end - test "create can't be abused to post boosts as regular user" do - user = users(:kevin) - bot_key = "#{user.id}-" + test "create can't be abused to post boosts as a regular user" do + bot_key = "#{users(:kevin).id}-" assert_no_difference -> { Boost.count } do post room_bot_message_boosts_url(@room, bot_key, @message), params: +"👀" diff --git a/test/controllers/messages/by_bots_controller_test.rb b/test/controllers/messages/by_bots_controller_test.rb index f1122da..0c145e7 100644 --- a/test/controllers/messages/by_bots_controller_test.rb +++ b/test/controllers/messages/by_bots_controller_test.rb @@ -1,6 +1,6 @@ require "test_helper" -class Messages::ByBotsControlleTest < ActionDispatch::IntegrationTest +class Messages::ByBotsControllerTest < ActionDispatch::IntegrationTest setup do @room = rooms(:watercooler) end @@ -40,6 +40,16 @@ class Messages::ByBotsControlleTest < ActionDispatch::IntegrationTest end end + test "create without a body or attachment" do + assert_no_difference -> { Message.count } do + post room_bot_messages_url(@room, users(:bender).bot_key) + assert_response :unprocessable_content + + post room_bot_messages_url(@room, users(:bender).bot_key), params: +" " + assert_response :unprocessable_content + end + end + test "create can't be abused to post messages as any user" do user = users(:kevin) bot_key = "#{user.id}-" @@ -51,77 +61,90 @@ class Messages::ByBotsControlleTest < ActionDispatch::IntegrationTest assert_response :redirect end - test "index returns messages as JSON" do - get room_bot_messages_index_url(@room, users(:bender).bot_key) + test "index returns the room's messages in the order they were sent" do + get room_bot_messages_url(@room, users(:bender).bot_key) assert_response :success json = JSON.parse(response.body) - assert json["room"]["id"].present? - assert json["room"]["name"].present? - assert json["messages"].is_a?(Array) - assert json["pagination"].present? + assert_equal @room.messages.ordered.map(&:id), json.map { it["id"] } end test "index includes message details" do - # Create a message in the room first - post room_bot_messages_url(@room, users(:bender).bot_key), params: +"Test message for index" + post room_bot_messages_url(@room, users(:bender).bot_key), params: +"Hello from Bender!" - get room_bot_messages_index_url(@room, users(:bender).bot_key) + get room_bot_messages_url(@room, users(:bender).bot_key) + assert_response :success + + message = Message.last + json_message = JSON.parse(response.body).last + assert_equal message.id, json_message["id"] + assert_equal "Hello from Bender!", json_message["body"]["plain_text"] + assert_includes json_message["body"]["html"], "Hello from Bender!" + assert_equal message.created_at.utc.iso8601(3), json_message["created_at"] + assert_equal users(:bender).id, json_message["creator"]["id"] + assert_equal "Bender Bot", json_message["creator"]["name"] + assert_equal "bot", json_message["creator"]["role"] + assert_equal @room.id, json_message["room"]["id"] + assert_equal room_message_url(@room, message), json_message["url"] + end + + test "index pages through older messages with the Link header" do + (Message::PAGE_SIZE - @room.messages.count + 1).times do |i| + @room.messages.create!(body: "Filler #{i}", creator: users(:jason), client_message_id: "filler-#{i}") + end + + get room_bot_messages_url(@room, users(:bender).bot_key) assert_response :success json = JSON.parse(response.body) - message = json["messages"].find { |m| m["body"]["plain"] == "Test message for index" } - assert message.present?, "Expected to find the test message" - assert message["id"].present? - assert message["created_at"].present? - assert message["creator"]["id"].present? - assert message["creator"]["name"].present? - end + assert_equal Message::PAGE_SIZE, json.size + assert_equal "41", response.headers["X-Total-Count"] + assert_not_includes json.map { it["id"] }, messages(:fourth).id - test "index supports pagination with before parameter" do - # Use a message from the watercooler room (where bender is a member) - message_in_room = messages(:thirteenth) # Latest message in watercooler - get room_bot_messages_index_url(@room, users(:bender).bot_key, before: message_in_room.id) + get response.headers["Link"][/<(.*)>/, 1] assert_response :success + + json = JSON.parse(response.body) + assert_equal [ messages(:fourth).id ], json.map { it["id"] } + assert_nil response.headers["Link"] end - test "index supports pagination with after parameter" do - # Use a message from the watercooler room (where bender is a member) - message_in_room = messages(:fourth) # First message in watercooler - get room_bot_messages_index_url(@room, users(:bender).bot_key, after: message_in_room.id) + test "index pages newer messages with after" do + get room_bot_messages_url(@room, users(:bender).bot_key, after: messages(:tenth).id) assert_response :success + + json = JSON.parse(response.body) + assert_equal %i[ eleventh twelfth thirteenth ].map { messages(it).id }, json.map { it["id"] } + assert_nil response.headers["Link"] end - test "index requires valid bot key" do - get room_bot_messages_index_url(@room, "invalid-bot-key") - assert_response :redirect # Redirects to login + test "index in a room with no messages" do + get room_bot_messages_url(rooms(:bender_and_kevin), users(:bender).bot_key) + assert_response :success + + assert_equal [], JSON.parse(response.body) + assert_equal "0", response.headers["X-Total-Count"] + assert_nil response.headers["Link"] end - test "index returns not_found for room bot is not a member of" do - # bender bot is NOT a member of the designers room - room_without_bot = rooms(:designers) - get room_bot_messages_index_url(room_without_bot, users(:bender).bot_key) + test "index requires a valid bot key" do + get room_bot_messages_url(@room, "invalid-bot-key") + assert_response :redirect + end + + test "index is not found for a room the bot is not a member of" do + get room_bot_messages_url(rooms(:designers), users(:bender).bot_key) assert_response :not_found end - test "index works for room bot IS a member of" do - # bender bot IS a member of watercooler - room_with_bot = rooms(:watercooler) - get room_bot_messages_index_url(room_with_bot, users(:bender).bot_key) - assert_response :success - end - - test "create returns not_found for room bot is not a member of" do - # bender bot is NOT a member of the designers room - verify create matches index behavior - room_without_bot = rooms(:designers) + test "create is not found for a room the bot is not a member of" do assert_no_difference -> { Message.count } do - post room_bot_messages_url(room_without_bot, users(:bender).bot_key), params: +"Hello!" + post room_bot_messages_url(rooms(:designers), users(:bender).bot_key), params: +"Hello!" end assert_response :not_found end - test "regular messages index still denied for bots" do - # The standard messages endpoint (not the bot-specific one) should still be forbidden + test "regular messages index remains denied for bots" do get room_messages_url(@room, bot_key: users(:bender).bot_key) assert_response :forbidden end