Continuous Release v1.9.2

.vscode/settings.json

@@ -79,6 +79,12 @@
         }
     ],
     //////////////////////////////////////
+    // Git
+    //////////////////////////////////////
+    "git.inputValidation": "warn",
+    "git.inputValidationSubjectLength": 50,
+    "git.inputValidationLength": 72,
+    //////////////////////////////////////
     // Spell Checker
     //////////////////////////////////////
     "cSpell.words": [

app/controllers/commontator/comments_controller.rb

@@ -71,6 +71,7 @@ def create
             Commontator::Subscription.comment_created(@comment)
             # The next line constitutes a customization of the original controller
             update_unread_status
+            activate_unread_comments_icon_if_necessary
 
             @commontator_page = @commontator_thread.new_comment_page(
               @comment.parent_id, @commontator_show_all
@@ -194,9 +195,14 @@ def subscribe_mentioned
         end
       end
 
-      # This method ensures that the unread_comments flag is updated
-      # for users affected by the creation of a newly created comment
-      # It constitues a customization
+      # Updates the unread_comments flag for users subscribed to the current thread.
+      # This method should only be called when a new comment was created.
+      #
+      # The originator of the comment does not get the flag set since that user
+      # already knows about the comment; that user has just created it after all.
+      #
+      # (This is a customization of the original controller provided
+      # by the commontator gem.)
       def update_unread_status
         medium = @commontator_thread.commontable
         return unless medium.released.in?(["all", "users", "subscribers"])
@@ -205,22 +211,35 @@ def update_unread_status
         relevant_users.where.not(id: current_user.id)
                       .where(unread_comments: false)
                       .update(unread_comments: true)
-
-        # make sure that the thread associated to this comment is marked as read
-        # by the comment creator (unless some other user posted a comment in it
-        # that has not yet been read)
-        @reader = Reader.find_or_create_by(user: current_user,
-                                           thread: @commontator_thread)
-        if unseen_comments?
-          @update_icon = true
-          return
-        end
-        @reader.touch
       end
 
-      def unseen_comments?
+      # Might activate the flag used in the view to indicate unread comments.
+      # This method should only be called when a new comment was created.
+      # The flag is activated if the current user has not seen all comments
+      # in the thread in which the new comment was created.
+      #
+      # The flag might only be activated, not deactivated since the checks
+      # performed here are not sufficient to determine whether a user has
+      # any unread comments (including those in possibly other threads).
+      #
+      # This method was introduced for one specific edge case:
+      # When the current user A has just created a new comment in a thread,
+      # but in the meantime, another user B has created a comment in the same
+      # thread. User A will not be informed immediately about the new comment
+      # by B since we don't have websockets implemented. Instead, A will be
+      # informed by a visual indicator as soon as A has posted their own comment.
+      #
+      # (This is a customization of the original controller provided
+      # by the commontator gem.)
+      def activate_unread_comments_icon_if_necessary
+        reader = Reader.find_by(user: current_user, thread: @commontator_thread)
+        @update_icon = true if unseen_comments_in_current_thread?(reader)
+      end
+
+      def unseen_comments_in_current_thread?(reader = nil)
         @commontator_thread.comments.any? do |c|
-          c.creator != current_user && c.created_at > @reader.updated_at
+          not_marked_as_read_in_reader = reader.nil? || c.created_at > reader.updated_at
+          c.creator != current_user && not_marked_as_read_in_reader
         end
       end
   end

app/controllers/main_controller.rb

@@ -26,7 +26,7 @@ def sponsors
   end
 
   def comments
-    @media_comments = current_user.media_latest_comments
+    @media_comments = current_user.subscribed_media_with_latest_comments_not_by_creator
     @media_comments.select! do |m|
       (Reader.find_by(user: current_user, thread: m[:thread])
             &.updated_at || 1000.years.ago) < m[:latest_comment].created_at &&

app/controllers/readers_controller.rb

@@ -9,7 +9,7 @@ def update
     @reader = Reader.find_or_create_by(user: current_user,
                                        thread: @thread)
     @reader.touch
-    @anything_left = current_user.media_latest_comments.any? do |m|
+    @anything_left = current_user.subscribed_media_with_latest_comments_not_by_creator.any? do |m|
       (Reader.find_by(user: current_user, thread: m[:thread])
             &.updated_at || 1000.years.ago) < m[:latest_comment].created_at
     end

app/models/user.rb

@@ -549,14 +549,38 @@ def subscribed_commentable_media_with_comments
       .select { |m| m.commontator_thread.comments.any? }
   end
 
-  def media_latest_comments
-    media = subscribed_commentable_media_with_comments
-            .map do |m|
-      { medium: m,
-        thread: m.commontator_thread,
-        latest_comment: m.commontator_thread
-                         .comments.max_by(&:created_at) }
+  # Returns the media that the user has subscribed to and that have been
+  # commented on by somebody else (not by the current user). The order is
+  # given by the time of the latest comment by somebody else.
+  #
+  # Media that have not been commented on by somebody else than the current user,
+  # are not returned (!).
+  #
+  # For each medium, the following information is stored:
+  # - the medium itself
+  # - the thread of the medium
+  # - the latest comment by somebody else than the current user
+  # - the latest comment by any user (which might include the current user)
+  def subscribed_media_with_latest_comments_not_by_creator
+    media = []
+
+    subscribed_commentable_media_with_comments.each do |m|
+      thread = m.commontator_thread
+      comments = thread.comments
+      next if comments.blank?
+
+      comments_not_by_creator = comments.reject { |c| c.creator == self }
+      next if comments_not_by_creator.blank?
+
+      latest_comment = comments_not_by_creator.max_by(&:created_at)
+      latest_comment_by_any_user = comments.max_by(&:created_at)
+
+      media << { medium: m,
+                 thread: thread,
+                 latest_comment: latest_comment,
+                 latest_comment_by_any_user: latest_comment_by_any_user }
     end
+
     media.sort_by { |x| x[:latest_comment].created_at }.reverse
   end
 

app/views/main/comments.html.erb

@@ -68,8 +68,8 @@
             </div>
             <div class="col-4">
               <%= t('comments.who_and_when',
-                    when: time_ago_in_words(m[:latest_comment].created_at),
-                    who: m[:latest_comment].creator.name) %>
+                    when: time_ago_in_words(m[:latest_comment_by_any_user].created_at),
+                    who: m[:latest_comment_by_any_user].creator.name) %>
             </div>
             <div class="col-2 d-flex justify-content-center">
               <div clas="row">

db/migrate/20240116180000_fix_unread_comments_inconsistencies.rb

@@ -0,0 +1,56 @@
+# Fixes the unread_comments flag for all users. Unintended behavior was
+# introduced in pull request #515. Migration introduced in #587.
+# Behavior fixed in #585.
+#
+# This migration is generally *not* idempotent since users might have interacted
+# with the website since the migration was run and thus they will probably have
+# different unread comments flags as the ones at the time of the migration.
+#
+# This migration is not reversible as we don't store the previous state of
+# the unread_comments flag.
+class FixUnreadCommentsInconsistencies < ActiveRecord::Migration[7.0]
+  def up
+    num_fixed_users = 0
+
+    User.find_each do |user|
+      had_user_unread_comments = user.unread_comments # boolean
+      has_user_unread_comments = user_unread_comments?(user)
+
+      has_flag_changed = (had_user_unread_comments != has_user_unread_comments)
+      user.update(unread_comments: has_user_unread_comments) if has_flag_changed
+      num_fixed_users += 1 if has_flag_changed
+    end
+
+    Rails.logger.warn { "Ran through #{User.count} users (unread comments flag)" }
+    Rails.logger.warn { "Fixed #{num_fixed_users} users (unread comments flag)" }
+  end
+
+  # Checks and returns whether the user has unread comments.
+  def user_unread_comments?(user)
+    # Check for unread comments -- directly via Reader
+    readers = Reader.where(user: user)
+    readers.each do |reader|
+      thread = Commontator::Thread.find_by(id: reader.thread_id)
+      next if thread.nil?
+
+      latest_thread_comment_by_any_user = thread.comments.max_by(&:created_at)
+      next if latest_thread_comment_by_any_user.blank?
+
+      latest_thread_comment_time = latest_thread_comment_by_any_user.created_at
+      has_user_unread_comments = reader.updated_at < latest_thread_comment_time
+
+      return true if has_user_unread_comments
+    end
+
+    # User might still have unread comments but no related Reader objects
+    # -> Check for unread comments -- via Media
+    unseen_media = user.subscribed_media_with_latest_comments_not_by_creator.select do |m|
+      m[:medium].visible_for_user?(user)
+    end
+    unseen_media.present?
+  end
+
+  def down
+    raise ActiveRecord::IrreversibleMigration
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.0].define(version: 2023_11_01_100015) do
+ActiveRecord::Schema[7.0].define(version: 2024_01_16_180000) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"