Show the latest page when a refresh misses more than one

app/controllers/rooms/refreshes_controller.rb

@@ -6,6 +6,9 @@ class Rooms::RefreshesController < ApplicationController
   def show
     @new_messages = @room.messages.with_presentation.page_created_since(@last_updated_at)
     @updated_messages = @room.messages.without(@new_messages).with_presentation.page_updated_since(@last_updated_at)
+    # More was posted than one page holds. The page is told so, and loads the rest as it does when scrolling down,
+    # instead of counting itself up to date after the first page.
+    @more_messages = @room.messages.created_since(@last_updated_at).paged?
   end
 
   private

app/javascript/controllers/messages_controller.js

@@ -14,6 +14,7 @@ export default class extends Controller {
   #paginator
   #formatter
   #scrollManager
+  #heldMessages = []
 
   // Lifecycle
 
@@ -64,6 +65,12 @@ export default class extends Controller {
       const upToDate = this.#paginator.upToDate
 
       if (upToDate) {
+        // A refresh that missed more than it sends: the rest loads in order as the end of the list comes into view.
+        if (event.detail.newStream.hasAttribute("incomplete")) {
+          this.#paginator.upToDate = false
+          this.latestTarget.hidden = false
+        }
+
         event.detail.render = async (streamElement) => {
           const didScroll = await this.#scrollManager.autoscroll(false, async () => {
             await render(streamElement)
@@ -78,6 +85,10 @@ export default class extends Controller {
           }
         }
       } else {
+        // The end of the list isn't the latest message, so one appended there would leave a gap above it. It waits
+        // until the list has caught up.
+        this.#heldMessages.push(event.detail.newStream.templateContent)
+        event.detail.render = () => {}
         this.latestTarget.hidden = false
       }
     }
@@ -121,6 +132,7 @@ export default class extends Controller {
 
   #allContentViewed() {
     this.latestTarget.hidden = true
+    this.#appendHeldMessages()
   }
 
 
@@ -148,6 +160,22 @@ export default class extends Controller {
   async #ensureUpToDate() {
     if (!this.#paginator.upToDate) {
       await this.#paginator.resetToLastPage()
+      this.#appendHeldMessages()
+    }
+  }
+
+  // Messages that arrived while the list was catching up, once it has. Only those past its end: the others it fetched on
+  // the way, or they sit above the latest page and load on scrolling up.
+  #appendHeldMessages() {
+    let end = Math.max(0, ...Array.from(this.messagesTarget.children, (element) => this.#sortValue(element)))
+
+    for (const fragment of this.#heldMessages.splice(0)) {
+      for (const message of fragment.querySelectorAll(".message")) {
+        if (this.#sortValue(message) > end) {
+          this.messagesTarget.append(message)
+          end = this.#sortValue(message)
+        }
+      }
     }
   }
 

app/javascript/models/message_paginator.js

@@ -162,6 +162,11 @@ export default class MessagePaginator {
       const lastNewElement = page.lastElementChild
 
       keepScroll(this.#container, top, () => {
+        // A message appended live just before a refresh sits above the ones the refresh missed. Keep the copy in order.
+        for (const message of page.querySelectorAll(".message")) {
+          this.#container.querySelector(`#${CSS.escape(message.id)}`)?.remove()
+        }
+
         insertHTMLFragment(page, this.#container, top)
 
         // Ensure formatting is correct over page boundaries

app/models/message/pagination.rb

@@ -35,7 +35,8 @@ def preload_associations(records)
     scope :page_before, ->(message) { before(message).last_page }
     scope :page_after, ->(message) { after(message).first_page }
 
-    scope :page_created_since, ->(time) { where("created_at > ?", time).first_page }
+    scope :created_since, ->(time) { where("created_at > ?", time) }
+    scope :page_created_since, ->(time) { created_since(time).first_page }
     # Sorting on +created_at, which no index covers, has SQLite find the few messages updated since
     # through (room_id, updated_at) and sort just those, instead of walking the whole room by
     # (room_id, created_at) looking for them.

app/views/rooms/refreshes/show.turbo_stream.erb

@@ -1,6 +1,7 @@
-<%= turbo_stream.append dom_id(@room, :messages) do %>
-  <%= render partial: "messages/message", collection: @new_messages, cached: ->(message) { message_fragment_cache_key(message) } %>
-<% end if @new_messages.any? %>
+<% if @new_messages.any? %>
+  <%= turbo_stream_action_tag :append, target: dom_id(@room, :messages), incomplete: (true if @more_messages),
+        template: render(partial: "messages/message", formats: :html, collection: @new_messages, cached: ->(message) { message_fragment_cache_key(message) }) %>
+<% end %>
 
 <% @updated_messages.each do |message| %>
   <%= turbo_stream.replace dom_id(message), partial: "messages/message", locals: { message: message } %>

test/controllers/rooms/refreshes_controller_test.rb

@@ -29,4 +29,39 @@ class Rooms::RefreshesControllerTest < ActionDispatch::IntegrationTest
       assert_select "template", count: 1
     end
   end
+
+  test "refresh appends a whole page of new messages as all there is" do
+    missed = post_messages(Message::PAGE_SIZE)
+
+    get room_refresh_url(rooms(:hq), format: :turbo_stream), params: { since: 10.minutes.ago.to_fs(:epoch) }
+
+    assert_response :success
+    assert_select "turbo-stream[action='append'][incomplete]", count: 0
+    assert_select "turbo-stream[action='append'][target='#{dom_id(rooms(:hq), :messages)}']" do
+      missed.each { |message| assert_select "#" + dom_id(message) }
+    end
+  end
+
+  test "refresh says when more was posted than the page it appends" do
+    missed = post_messages(Message::PAGE_SIZE + 5)
+
+    get room_refresh_url(rooms(:hq), format: :turbo_stream), params: { since: 10.minutes.ago.to_fs(:epoch) }
+
+    assert_response :success
+    assert_select "turbo-stream[action='append'][incomplete][target='#{dom_id(rooms(:hq), :messages)}']" do
+      assert_select "[data-message-id]", count: Message::PAGE_SIZE
+      missed.first(Message::PAGE_SIZE).each { |message| assert_select "#" + dom_id(message) }
+    end
+  end
+
+  private
+    def post_messages(count)
+      base = 5.minutes.ago
+
+      count.times.map do |i|
+        travel_to(base + i.seconds) do
+          rooms(:hq).messages.create!(creator: users(:jason), body: "Missed #{i}", client_message_id: "missed-#{i}")
+        end
+      end
+    end
 end

test/system/catching_up_test.rb

@@ -0,0 +1,88 @@
+require "application_system_test_case"
+
+class CatchingUpTest < ApplicationSystemTestCase
+  setup do
+    sign_in "jz@37signals.com"
+  end
+
+  test "after missing more than a page, the room shows every missed message once, in order" do
+    join_room rooms(:designers)
+    missed = miss_messages(45)
+
+    # A copy of one of them, appended live just before the refresh, out of order.
+    append_to_messages <<~HTML
+      <div class="message message--formatted" id="#{dom_id(missed[42])}" data-message-id="#{missed[42].id}" data-sort-value="#{missed[42].created_at.to_fs(:epoch)}"><div class="message__body">Early copy</div></div>
+    HTML
+    reconnect
+
+    assert_message_text "Missed 44", wait: 30
+    assert_equal (0..44).to_a, missed_messages_shown
+    assert_no_message_text "Early copy"
+  end
+
+  test "catching up leaves a message being edited as it is" do
+    join_room rooms(:designers)
+    within_message messages(:third) do
+      reveal_message_actions
+      find(".message__edit-btn").click
+    end
+    assert_selector "##{dom_id(messages(:third))} .message__body-content--editing"
+
+    miss_messages 45
+    reconnect
+
+    assert_message_text "Missed 44", wait: 30
+    assert_selector "##{dom_id(messages(:third))} .message__body-content--editing"
+  end
+
+  test "a message that arrives while the list isn't at its end waits, then shows in order" do
+    # Enough history after the message that the end of the page around it stays below the fold.
+    earlier = miss_messages(60, label: "Earlier")
+    visit room_at_message_url(rooms(:designers), messages(:first))
+    wait_for_cable_connection
+    dismiss_pwa_install_prompt
+
+    # One older than the latest page, as if it had been held for long, and one just now.
+    Turbo::StreamsChannel.broadcast_append_to rooms(:designers), :messages, target: dom_id(rooms(:designers), :messages), html: <<~HTML
+      <div class="message message--formatted" id="message_999998" data-message-id="999998" data-sort-value="#{earlier[5].created_at.to_fs(:epoch)}"><div class="message__body">Held for long</div></div>
+      <div class="message message--formatted" id="message_999999" data-message-id="999999" data-sort-value="#{Time.current.to_fs(:epoch)}"><div class="message__body">Arrived live</div></div>
+    HTML
+
+    assert_selector ".message-area__return-to-latest", visible: true, wait: 10
+    assert_no_message_text "Arrived live"
+    assert_selector ".search-highlight", text: messages(:first).plain_text_body
+
+    # Sending one brings the list to its end first, and the held message comes before ours.
+    send_message "Mine"
+
+    last_two = all("##{dom_id(rooms(:designers), :messages)} > .message").last(2).map(&:text)
+    assert_equal "Arrived live", last_two.first
+    assert_match "Mine", last_two.last
+    assert_no_message_text "Held for long"
+  end
+
+  private
+    # Created without the controller, so none of them is broadcast: this browser misses them all live.
+    def miss_messages(count, label: "Missed")
+      count.times.map do |i|
+        rooms(:designers).messages.create!(creator: users(:kevin), body: "#{label} #{i}", client_message_id: "#{label.downcase}-#{i}")
+      end
+    end
+
+    # Without a prompt reconnect, Action Cable's monitor notices the dropped socket only after its stale threshold.
+    def reconnect
+      ActionCable.server.remote_connections.where(current_user: users(:jz)).disconnect(reconnect: true)
+    end
+
+    def missed_messages_shown
+      all(".message__body", text: /Missed \d+/).map { |body| body.text[/Missed (\d+)/, 1].to_i }
+    end
+
+    def append_to_messages(html)
+      page.execute_script("document.getElementById(arguments[0]).insertAdjacentHTML('beforeend', arguments[1])", dom_id(rooms(:designers), :messages), html)
+    end
+
+    def assert_no_message_text(text)
+      assert_no_selector ".message__body", text: text
+    end
+end