Adjust to the in-house style

- Use jbuilder instead of hashes
- Use resource instead of direct HTTP verbs
    Verbs only make sense if you have one or two routes, if there are
    multiple that emulate what resource does then it's better to use resource.
- Paginate using link headers
- Cache responses

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

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!(boost_params)
-    @boost = @message.boosts.create!(content: read_body)
 
     broadcast_create
-    head :created
+    render :show, status: :created
-  rescue ActiveRecord::RecordNotFound
-    head :not_found
   end
 
   private
     def set_message
-      @room = Current.user.rooms.find(params[:room_id])
+      if room = Current.user.rooms.find_by(id: params[:room_id])
-      @message = @room.messages.find(params[:message_id])
+        @message = room.messages.find_by(id: params[:message_id])
+      end
+
+      head :not_found unless @message
     end
 
-    def read_body
+    def ensure_content_present
-      request.body.rewind
+      if raw_request_body.blank?
-      request.body.read.force_encoding("UTF-8")
+        head :unprocessable_content
-    ensure
+      end
-      request.body.rewind
     end
 
-    def broadcast_create
+    def boost_params
-      @boost.broadcast_append_to @boost.message.room, :messages,
+      { content: raw_request_body }
-        target: "boosts_message_#{@boost.message.client_message_id}",
-        partial: "messages/boosts/boost",
-        attributes: { maintain_scroll: true }
     end
 end

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)
+    set_pagination_headers
-  rescue ActiveRecord::RecordNotFound
-    head :not_found
   end
 
   def create
-    set_room
+    super
-    @message = @room.messages.create_with_attachment!(message_params)
-    @message.broadcast_create
-    deliver_webhooks_to_bots
     head :created, location: message_url(@message)
-  rescue ActiveRecord::RecordNotFound
-    head :not_found
   end
 
   private
-    def messages_as_json(messages)
+    def set_room
-      {
+      @room = Current.user.rooms.find_by(id: params[:room_id])
-        room: {
+
-          id: @room.id,
+      head :not_found unless @room
-          name: @room.name
-        },
-        messages: messages.map { |m| message_as_json(m) },
-        pagination: pagination_info(messages)
-      }
     end
 
-    def message_as_json(message)
+    def ensure_body_or_attachment_present
-      {
+      if params[:attachment].blank? && raw_request_body.blank?
-        id: message.id,
+        head :unprocessable_content
-        body: {
+      end
-          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?
-        }
-      }
     end
 
-    def pagination_info(messages)
+    def set_pagination_headers
-      return {} if messages.empty?
+      headers["X-Total-Count"] = @room.messages.count.to_s
-      {
+
-        oldest_id: messages.last.id,
+      if next_page = next_page_params
-        newest_id: messages.first.id,
+        headers["Link"] = %(<#{room_bot_messages_url(@room, params[:bot_key], **next_page)}>; rel="next")
-        has_more: messages.size == Message::PAGE_SIZE
+      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

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

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

app/views/messages/boosts/by_bots/show.json.jbuilder

@@ -0,0 +1 @@
+json.partial! "messages/boosts/boost", boost: @boost

app/views/messages/by_bots/index.json.jbuilder

@@ -0,0 +1 @@
+json.array! @messages, partial: "messages/message", as: :message

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

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
+    nested do
-    get ":bot_key/messages", to: "messages/by_bots#index", as: :bot_messages_index
+      scope path: ":bot_key", as: :bot, defaults: { format: :json } do
-    post ":bot_key/messages", to: "messages/by_bots#create", as: :bot_messages
+        resources :messages, controller: "messages/by_bots", only: %i[ index create ] do
-    post ":bot_key/messages/:message_id/boosts", to: "messages/boosts/by_bots#create", as: :bot_message_boosts
+          resources :boosts, controller: "messages/boosts/by_bots", only: :create
+        end
+      end
+    end
 
     scope module: "rooms" do
       resource :refresh, only: :show

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
+  test "create with text content" 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
     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
+  test "create is not found for a room the bot is not a member of" do
-    room_without_bot = rooms(:designers)
-    message_in_other_room = messages(:first)  # Message in designers room
-
     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
+  test "create is not found for a message outside the room" do
-    message_in_other_room = messages(:first)  # Message in designers room, not watercooler
-
     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
+  test "create can't be abused to post boosts as a regular user" do
-    user = users(:kevin)
+    bot_key = "#{users(:kevin).id}-"
-    bot_key = "#{user.id}-"
 
     assert_no_difference -> { Boost.count } do
       post room_bot_message_boosts_url(@room, bot_key, @message), params: +"👀"

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
+  test "index returns the room's messages in the order they were sent" do
-    get room_bot_messages_index_url(@room, users(:bender).bot_key)
+    get room_bot_messages_url(@room, users(:bender).bot_key)
     assert_response :success
 
     json = JSON.parse(response.body)
-    assert json["room"]["id"].present?
+    assert_equal @room.messages.ordered.map(&:id), json.map { it["id"] }
-    assert json["room"]["name"].present?
-    assert json["messages"].is_a?(Array)
-    assert json["pagination"].present?
   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: +"Hello from Bender!"
-    post room_bot_messages_url(@room, users(:bender).bot_key), params: +"Test message for index"
 
-    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_equal Message::PAGE_SIZE, json.size
-    assert message.present?, "Expected to find the test message"
+    assert_equal "41", response.headers["X-Total-Count"]
-    assert message["id"].present?
+    assert_not_includes json.map { it["id"] }, messages(:fourth).id
-    assert message["created_at"].present?
-    assert message["creator"]["id"].present?
-    assert message["creator"]["name"].present?
-  end
 
-  test "index supports pagination with before parameter" do
+    get response.headers["Link"][/<(.*)>/, 1]
-    # 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)
     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
+  test "index pages newer messages with after" do
-    # Use a message from the watercooler room (where bender is a member)
+    get room_bot_messages_url(@room, users(:bender).bot_key, after: messages(:tenth).id)
-    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)
     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
+  test "index in a room with no messages" do
-    get room_bot_messages_index_url(@room, "invalid-bot-key")
+    get room_bot_messages_url(rooms(:bender_and_kevin), users(:bender).bot_key)
-    assert_response :redirect  # Redirects to login
+    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
+  test "index requires a valid bot key" do
-    # bender bot is NOT a member of the designers room
+    get room_bot_messages_url(@room, "invalid-bot-key")
-    room_without_bot = rooms(:designers)
+    assert_response :redirect
-    get room_bot_messages_index_url(room_without_bot, users(:bender).bot_key)
+  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
+  test "create is not found for a room the bot is not 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)
     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
+  test "regular messages index remains denied for bots" do
-    # The standard messages endpoint (not the bot-specific one) should still be forbidden
     get room_messages_url(@room, bot_key: users(:bender).bot_key)
     assert_response :forbidden
   end