Move back to message created_at ordering

When a message is approved later about where it should appear in the thread isn't clear but
given it was using created at we should just stick with it for now

This also adds some message loading via AJAX specs

app/assets/javascripts/thread_scroller.js

@@ -13,7 +13,7 @@ $(document).ready(function () {
     }
     var obs = new IntersectionObserver(function (entries) {
       if (lastViewedMessage && entries[0] && entries[0].isIntersecting) {
-        $.ajax({url: window.location.pathname, data: {initiallyLoadedFrom: document.querySelector('[data-initially-loaded-from]').dataset.initiallyLoadedFrom}, dataType: 'script'})
+        $.ajax({url: window.location.pathname, data: {initiallyLoadedFrom: initiallyLoadedFrom.dataset.initiallyLoadedFrom}, dataType: 'script'})
         obs.unobserve(lastViewedMessage)
       }
     }, options)

app/controllers/message_threads_controller.rb

@@ -38,7 +38,7 @@ def show
           @view_from = @messages.detect { |m| m.created_at >= last_viewed } || @messages.last
         end
 
-        @initially_loaded_from = @messages.first.updated_at
+        @initially_loaded_from = @messages.first&.created_at&.iso8601
 
         @library_items = Library::Item.find_by_tags_from(thread).limit(5)
         @tag_panel = TagPanelDecorator.new(thread, form_url: thread_tags_path(thread))
@@ -47,7 +47,7 @@ def show
       end
       format.js do
         messages = thread.messages.approved
-        initially_loaded_from = Time.zone.parse(params["initiallyLoadedFrom"])
+        initially_loaded_from = Time.zone.iso8601(params["initiallyLoadedFrom"])
         messages = messages.before_date(initially_loaded_from)
 
         @messages = messages.includes(

app/models/message.rb

@@ -56,8 +56,8 @@ class Message < ApplicationRecord
   scope :approved,   -> { where(status: [nil, "approved"]) }
   scope :mod_queued, -> { where(status: "mod_queued") }
   scope :in_group,   ->(group_id) { includes(:thread).where(message_threads: { group_id: group_id }).references(:thread) }
-  scope :after_date, ->(date) { where(arel_table[:updated_at].gteq(date)) }
-  scope :before_date, ->(date) { where(arel_table[:updated_at].lteq(date)) }
+  scope :after_date, ->(date) { where(arel_table[:created_at].gteq(date)) }
+  scope :before_date, ->(date) { where(arel_table[:created_at].lteq(date)) }
 
   validates :created_by, presence: true
   validates :body, presence: true, unless: :components?
@@ -92,7 +92,7 @@ class Message < ApplicationRecord
   end
 
   def self.after_date_with_n_before(after_date:, n_before:)
-    after_date(after_date).or(where(id: before_date(after_date).reorder(updated_at: :desc).limit(n_before + 1).ids))
+    after_date(after_date).or(where(id: before_date(after_date).reorder(created_at: :desc).limit(n_before + 1).ids))
   end
 
   def censor!

spec/controllers/message_threads_controller_spec.rb

@@ -6,9 +6,9 @@
   let(:thread) { create(:message_thread) }
 
   describe "thread views" do
-    let!(:message_a) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 4.days) }
-    let!(:message_b) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 3.days) }
-    let!(:message_c) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 2.days) }
+    let!(:message_4) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 4.days, created_by: message_2.created_by) }
+    let!(:message_3) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 3.days, created_by: message_2.created_by) }
+    let!(:message_2) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 2.days) }
 
     context "as a guest" do
       it "should not assign a message to view from" do
@@ -44,7 +44,33 @@
         it "should assign the first of the new messages" do
           create(:thread_view, thread: thread, user: user, viewed_at: Time.now.in_time_zone - 3.5.days)
           get :show, params: { id: thread.id }
-          expect(assigns(:view_from)).to eql(message_b)
+          expect(assigns(:view_from)).to eq(message_3)
+          expect(assigns(:initially_loaded_from)).to eq(message_4.created_at.in_time_zone("London").iso8601)
+        end
+      end
+
+      context "when there are more than 6 previously read messages" do
+        let!(:message_9) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 9.days, created_by: message_2.created_by) }
+        let!(:message_8) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 8.days, created_by: message_2.created_by) }
+        let!(:message_7) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 7.days, created_by: message_2.created_by) }
+        let!(:message_6) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 6.days, created_by: message_2.created_by) }
+        let!(:message_5) { create(:message, thread: thread, created_at: Time.now.in_time_zone - 5.days, created_by: message_2.created_by) }
+
+        context "with HTML" do
+          it "only shows the last 6" do
+            create(:thread_view, thread: thread, user: user, viewed_at: Time.now.in_time_zone)
+            get :show, params: { id: thread.id }
+            expect(assigns(:view_from)).to eq(message_2)
+            expect(assigns(:initially_loaded_from)).to eq(message_7.created_at.in_time_zone("London").iso8601)
+            expect(assigns(:messages)).to eq [message_7, message_6, message_5, message_4, message_3, message_2]
+          end
+        end
+
+        context "with JS (and initially_loaded_from)" do
+          it "shows messages before the initially_loaded_from" do
+            get :show, params: { id: thread.id, format: :js, initiallyLoadedFrom: message_7.created_at.in_time_zone("London").iso8601 }, xhr: true
+            expect(assigns(:messages)).to eq [message_9, message_8]
+          end
         end
       end
     end

spec/models/message_spec.rb

@@ -37,13 +37,13 @@
   end
 
   describe ".after_date_with_n_before" do
-    let!(:message_1) { create(:message, updated_at: 4.hours.ago) }
-    let!(:message_2) { create(:message, updated_at: 3.hours.ago, thread: message_1.thread, created_by: message_1.created_by) }
-    let!(:message_3) { create(:message, updated_at: 2.hours.ago, thread: message_1.thread, created_by: message_1.created_by) }
-    let!(:message_4) { create(:message, updated_at: 1.hour.ago, thread: message_1.thread, created_by: message_1.created_by) }
+    let!(:message_1) { create(:message, created_at: 4.hours.ago) }
+    let!(:message_2) { create(:message, created_at: 3.hours.ago, thread: message_1.thread, created_by: message_1.created_by) }
+    let!(:message_3) { create(:message, created_at: 2.hours.ago, thread: message_1.thread, created_by: message_1.created_by) }
+    let!(:message_4) { create(:message, created_at: 1.hour.ago, thread: message_1.thread, created_by: message_1.created_by) }
 
     it do
-      expect(message_1.thread.messages.approved.after_date_with_n_before(after_date: message_3.updated_at, n_before: 1)).to eq [
+      expect(message_1.thread.messages.approved.after_date_with_n_before(after_date: message_3.created_at, n_before: 1)).to eq [
         message_2, message_3, message_4
       ]
     end