Lazy loading: load messages in batches

Instead of rendering all messages with the initial page, we only render
skeletons and eager load the messages in batches of 50

Author: dutow

app/assets/stylesheets/application.postcss.css

@@ -15,6 +15,7 @@
 @import "components/avatars.css";
 @import "components/participants.css";
 @import "components/messages.css";
+@import "components/message-skeleton.css";
 @import "components/notes.css";
 @import "components/activities.css";
 @import "components/profile.css";

app/assets/stylesheets/components/message-skeleton.css

@@ -0,0 +1,32 @@
+.message-batch-skeleton {
+  padding: var(--spacing-5);
+}
+
+.skeleton-line {
+  height: 1em;
+  background: var(--color-bg-hover);
+  border-radius: var(--border-radius-sm);
+  margin-bottom: var(--spacing-3);
+  animation: skeleton-pulse 1.5s ease-in-out infinite;
+}
+
+.skeleton-line:last-child {
+  margin-bottom: 0;
+}
+
+.skeleton-line-short {
+  width: 40%;
+}
+
+.skeleton-line-medium {
+  width: 70%;
+}
+
+.skeleton-line-long {
+  width: 90%;
+}
+
+@keyframes skeleton-pulse {
+  0%, 100% { opacity: 0.4; }
+  50% { opacity: 0.7; }
+}

app/controllers/topics_controller.rb

@@ -1,6 +1,6 @@
 class TopicsController < ApplicationController
-  before_action :set_topic, only: [ :show, :aware, :read_all, :star, :unstar, :latest_patchset ]
-  before_action :require_authentication, only: [ :aware, :aware_bulk, :aware_all, :read_all, :star, :unstar ]
+  before_action :set_topic, only: [ :show, :message_batch, :aware, :read_all, :unread_all, :star, :unstar, :latest_patchset ]
+  before_action :require_authentication, only: [ :aware, :aware_bulk, :aware_all, :read_all, :unread_all, :star, :unstar ]
 
   def index
     @search_query = nil
@@ -51,25 +51,22 @@ def new_topics_count
   end
 
   def show
+    message_columns = (Message.column_names - %w[body body_tsv]).map { |c| "messages.#{c}" }
     messages_scope = @topic.messages
-      .eager_load(
+      .select(message_columns)
+      .preload(
         :sender,
         :sender_person,
         { sender_person: :default_alias },
-        {
-          reply_to: [
-            :sender,
-            :sender_person,
-            { sender_person: :default_alias }
-          ]
-        }
+        :attachments
       )
-      .preload(:attachments)
 
     @messages = messages_scope.order(created_at: :asc)
     @message_numbers = @messages.each_with_index.to_h { |msg, idx| [ msg.id, idx + 1 ] }
+    wire_reply_to_from_loaded_messages!
     preload_read_state!
     auto_mark_aware!
+    load_inline_message_bodies!
 
     build_participants_sidebar_data(messages_scope)
     build_thread_outline(@messages)
@@ -95,6 +92,20 @@ def show
     end
   end
 
+  def message_batch
+    ids = params[:ids].to_s.split(",").map(&:to_i).reject(&:zero?).first(50)
+    return head :bad_request if ids.empty?
+
+    @messages = @topic.messages
+      .where(id: ids)
+      .eager_load(:sender, :sender_person, { sender_person: :default_alias })
+      .preload(:attachments)
+      .order(:created_at)
+
+    expires_in 1.year, public: true
+    render layout: false
+  end
+
   def aware
     last_id = params[:up_to_message_id].presence&.to_i || @topic.messages.maximum(:id)
     ThreadAwareness.mark_until(user: current_user, topic: @topic, until_message_id: last_id) if last_id
@@ -140,6 +151,15 @@ def read_all
     end
   end
 
+  def unread_all
+    MessageReadRange.where(user: current_user, topic: @topic).delete_all
+
+    respond_to do |format|
+      format.json { render json: { status: "ok" } }
+      format.html { redirect_to topic_path(@topic) }
+    end
+  end
+
   def star
     TopicStar.create!(user: current_user, topic: @topic)
     respond_to do |format|
@@ -413,6 +433,39 @@ def build_thread_outline(messages)
     end
   end
 
+  def wire_reply_to_from_loaded_messages!
+    messages_by_id = @messages.index_by(&:id)
+    @messages.each do |msg|
+      target = msg.reply_to_id ? messages_by_id[msg.reply_to_id] : nil
+      msg.association(:reply_to).target = target
+    end
+  end
+
+  def load_inline_message_bodies!
+    inline_ids = compute_inline_message_ids
+    return if inline_ids.empty?
+
+    full_messages = Message
+      .where(id: inline_ids)
+      .eager_load(:sender, :sender_person, { sender_person: :default_alias },
+                  { reply_to: [ :sender, :sender_person, { sender_person: :default_alias } ] })
+      .preload(:attachments)
+      .index_by(&:id)
+
+    @messages = @messages.map { |msg| full_messages[msg.id] || msg }
+  end
+
+  def compute_inline_message_ids
+    ids = []
+    @messages.each do |msg|
+      is_read = user_signed_in? && @read_message_ids&.dig(msg.id)
+      next if is_read
+      ids << msg.id
+      break if ids.size >= 20
+    end
+    ids
+  end
+
   def preload_read_state!
     return unless user_signed_in?
 

app/javascript/controllers/batch_loader_controller.js

@@ -0,0 +1,80 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  static values = {
+    url: String,
+    batchSize: { type: Number, default: 50 },
+    signedIn: { type: Boolean, default: false }
+  }
+
+  static targets = ["skeleton"]
+
+  connect() {
+    this.loadAllBatches()
+  }
+
+  async loadAllBatches() {
+    const skeletons = this.skeletonTargets
+    if (skeletons.length === 0) return
+
+    const allIds = skeletons.map(el => {
+      return el.closest(".message-card").dataset.messageId
+    })
+
+    for (let i = 0; i < allIds.length; i += this.batchSizeValue) {
+      const batchIds = allIds.slice(i, i + this.batchSizeValue)
+      await this.fetchAndInjectBatch(batchIds)
+    }
+  }
+
+  async fetchAndInjectBatch(ids) {
+    const url = `${this.urlValue}?ids=${ids.join(",")}`
+    try {
+      const response = await fetch(url)
+      if (!response.ok) return
+      const html = await response.text()
+      this.injectBatch(html)
+    } catch (e) {
+      console.warn("batch load failed", e)
+    }
+  }
+
+  injectBatch(html) {
+    const template = document.createElement("template")
+    template.innerHTML = html
+
+    template.content.querySelectorAll("[data-message-id]").forEach(batchItem => {
+      const messageId = batchItem.dataset.messageId
+      const card = this.element.querySelector(
+        `.message-card[data-message-id="${messageId}"]`
+      )
+      if (!card) return
+
+      const skeleton = card.querySelector(".message-batch-skeleton")
+      if (!skeleton) return // already rendered inline, skip
+
+      const isRead = card.dataset.read === "true"
+
+      // The batchItem contains a div[data-message-id] wrapping .message-content
+      // Extract the .message-content element
+      const content = batchItem.firstElementChild
+      if (!content) return
+
+      if (isRead) {
+        content.classList.add("is-read")
+      }
+
+      skeleton.replaceWith(content)
+
+      if (this.signedInValue) {
+        const messageContent = card.querySelector(".message-content")
+        if (messageContent) {
+          messageContent.dataset.controller = "read-status"
+          messageContent.dataset.readStatusMessageIdValue = messageId
+          messageContent.dataset.readStatusReadUrlValue = `/messages/${messageId}/read.json`
+          messageContent.dataset.readStatusDelaySecondsValue = "5"
+        }
+      }
+    })
+  }
+}

app/javascript/controllers/read_status_controller.js

@@ -8,6 +8,10 @@ export default class extends Controller {
   }
 
   connect() {
+    if (this.element.classList.contains("is-read")) {
+      this.marked = true
+      return
+    }
     this.marked = false
     this.timer = null
     this.pollInterval = null

app/javascript/controllers/thread_actions_controller.js

@@ -4,6 +4,7 @@ export default class extends Controller {
   static values = {
     topicId: Number,
     readAllUrl: String,
+    unreadAllUrl: String,
   }
 
   markAllRead(event) {
@@ -18,6 +19,39 @@ export default class extends Controller {
     this.post(this.readAllUrlValue)
   }
 
+  markAllUnread(event) {
+    event.preventDefault()
+    document.querySelectorAll(".message-content").forEach(el => el.classList.remove("is-read"))
+    document.querySelectorAll(".message-card").forEach(card => {
+      card.dataset.read = "false"
+      const controller = this.application.getControllerForElementAndIdentifier(card, "message-collapse")
+      if (controller) {
+        controller.collapsedValue = false
+      }
+    })
+    this.post(this.unreadAllUrlValue)
+  }
+
+  collapseAll(event) {
+    event.preventDefault()
+    document.querySelectorAll(".message-card").forEach(card => {
+      const controller = this.application.getControllerForElementAndIdentifier(card, "message-collapse")
+      if (controller) {
+        controller.collapsedValue = true
+      }
+    })
+  }
+
+  expandAll(event) {
+    event.preventDefault()
+    document.querySelectorAll(".message-card").forEach(card => {
+      const controller = this.application.getControllerForElementAndIdentifier(card, "message-collapse")
+      if (controller) {
+        controller.collapsedValue = false
+      }
+    })
+  }
+
   post(url, onSuccess) {
     fetch(url, {
       method: "POST",

app/views/topics/_message.html.slim

@@ -1,7 +1,8 @@
 - message_classes = [("reply-message" if message.reply_to_id), ("message-branch-#{local_assigns[:branch_index]}" unless local_assigns[:branch_index].nil?)].compact.join(" ")
 - render_mode = local_assigns.fetch(:render_mode, :inline)
-- is_collapsed = render_mode != :inline
-.message-card id=message_dom_id(message) class=message_classes data-controller="message-collapse" data-message-collapse-collapsed-value=is_collapsed
+- is_read = local_assigns.fetch(:is_read, false)
+- is_collapsed = (render_mode == :collapsed) || (render_mode == :shell && is_read)
+.message-card id=message_dom_id(message) class=message_classes data-controller="message-collapse" data-message-collapse-collapsed-value=is_collapsed data-message-id=message.id data-read=(is_read ? "true" : "false")
   - if (mid_anchor = message_id_anchor(message))
     a.message-id-anchor id=mid_anchor aria-hidden="true"
   - if local_assigns[:is_first_unread]
@@ -56,3 +57,12 @@
       = render "topics/message_body", message: message
     - elsif render_mode == :collapsed
       = turbo_frame_tag "message-body-#{message.id}", src: message_content_path(message)
+    - elsif render_mode == :shell
+      .message-batch-skeleton data-batch-loader-target="skeleton"
+        .skeleton-line.skeleton-line-long
+        .skeleton-line.skeleton-line-medium
+        .skeleton-line.skeleton-line-long
+        .skeleton-line.skeleton-line-short
+    - if user_signed_in?
+      - notes_for_message = @notes_by_message&.[](message.id) || []
+      = render "notes/note_stack", topic: @topic, message: message, notes: notes_for_message

app/views/topics/_message_body.html.slim

@@ -1,5 +1,6 @@
-- is_read = defined?(@read_message_ids) && @read_message_ids[message.id]
-- if user_signed_in?
+- include_read_status = local_assigns.fetch(:include_read_status, true)
+- is_read = include_read_status && defined?(@read_message_ids) && @read_message_ids[message.id]
+- if include_read_status && user_signed_in?
   - read_data = { controller: "read-status", "read-status-message-id-value": message.id, "read-status-read-url-value": read_message_path(message, format: :json), "read-status-delay-seconds-value": read_visibility_seconds }
   - read_classes = ["message-content", ("is-read" if is_read)].compact
 - else
@@ -42,7 +43,3 @@
     details
       summary Import Notes
       pre.import-log = message.import_log
-
-- notes_for_message = @notes_by_message&.[](message.id) || []
-- if user_signed_in?
-  = render "notes/note_stack", topic: @topic, message: message, notes: notes_for_message

app/views/topics/message_batch.html.slim

@@ -0,0 +1,3 @@
+- @messages.each do |message|
+  div data-message-id=message.id
+    = render "topics/message_body", message: message, include_read_status: false

app/views/topics/show.html.slim

@@ -197,26 +197,36 @@
   .thread-notes-container
     = render "notes/note_stack", topic: @topic, message: nil, notes: thread_notes
 
-.messages-container.flat.asc
+.messages-container.flat.asc data-controller="batch-loader" data-batch-loader-url-value=message_batch_topic_path(@topic) data-batch-loader-signed-in-value=(user_signed_in? ? true : false)
   - last_message = @messages.last
   - if last_message.present?
-    .thread-actions data-controller="thread-actions" data-thread-actions-topic-id-value=@topic.id data-thread-actions-read-all-url-value=read_all_topic_path(@topic, format: :json)
+    .thread-actions data-controller="thread-actions" data-thread-actions-topic-id-value=@topic.id data-thread-actions-read-all-url-value=read_all_topic_path(@topic, format: :json) data-thread-actions-unread-all-url-value=unread_all_topic_path(@topic, format: :json)
       - if user_signed_in?
-        button.button.button-secondary.button-small data-action="click->thread-actions#markAllRead" Mark all messages read
+        button.button.button-secondary.button-small data-action="click->thread-actions#markAllRead" Mark all read
+        button.button.button-secondary.button-small data-action="click->thread-actions#markAllUnread" Mark all unread
+      button.button.button-secondary.button-small data-action="click->thread-actions#collapseAll" Collapse all
+      button.button.button-secondary.button-small data-action="click->thread-actions#expandAll" Show all
       = link_to "Jump to latest", "#message-#{last_message.id}", class: "button button-secondary button-small", title: "Jump to latest message", "aria-label": "Jump to latest message"
       - if user_signed_in?
         = link_to "Jump to unread", "#first-unread", class: "button button-secondary button-small", title: "Jump to first unread message", "aria-label": "Jump to first unread message"
 
   - first_unread_found = false
+  - unread_inline_count = 0
   - @messages.each do |message|
     - is_read = user_signed_in? && @read_message_ids&.dig(message.id)
     - is_first_unread = false
     - if user_signed_in? && !is_read && !first_unread_found
       - is_first_unread = true
       - first_unread_found = true
-    - render_mode = (user_signed_in? && is_read) ? :collapsed : :inline
+    - if is_read
+      - render_mode = :shell
+    - elsif unread_inline_count < 20
+      - render_mode = :inline
+      - unread_inline_count += 1
+    - else
+      - render_mode = :shell
     - branch_index = @message_branch_index&.dig(message.id)
-    = render 'message', message: message, number: @message_numbers[message.id], is_first_unread: is_first_unread, branch_index: branch_index, render_mode: render_mode
+    = render 'message', message: message, number: @message_numbers[message.id], is_first_unread: is_first_unread, branch_index: branch_index, render_mode: render_mode, is_read: !!is_read
 
 .topic-navigation
   = link_to "← Back to Topics", topics_path, class: "back-link"