Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a994cf8
switched comments last_activity_at to a column
Oaphi Dec 21, 2025
940aaa1
switched comment threads last_activity_at to a column
Oaphi Dec 21, 2025
0594fdd
added last_activity wrapper method to Comment (max of created, update…
Oaphi Dec 22, 2025
253a35b
renamed last_activity_at CommentThread method to last_activity & ensu…
Oaphi Dec 22, 2025
178e300
made bump_last_activity_at methods public to be able to bump by unrel…
Oaphi Dec 22, 2025
cbde601
switched 'thread_content' freshness check to last_activity & added th…
Oaphi Dec 22, 2025
3f1b69e
creating/destroying ThreadFollowers should bump their threads last ac…
Oaphi Dec 22, 2025
dff7c9f
renamed all *_last_activity_at methods to *_last_activity
Oaphi Dec 22, 2025
66e035e
[unrelated] fixed test description for the pings method
Oaphi Dec 22, 2025
c73eb26
[unrelated] fixed User#anonymize confusing inverted persist_changes p…
Oaphi Dec 22, 2025
c12ef0c
added tests for Comment#last_activity method
Oaphi Dec 22, 2025
bbf383a
reworked CommentThread#last_activity_at test into the new CommentThre…
Oaphi Dec 22, 2025
aa48e58
added CommentThread#add_follower QoL method & expanded last_activity …
Oaphi Dec 22, 2025
0f32bf4
added CommentThread#remove_follower QoL method & expanded last_activi…
Oaphi Dec 22, 2025
be1f952
CommentThread#bump_last_activity should have the option to persist ch…
Oaphi Dec 22, 2025
18142a4
Comment#bump_last_activity should have the option to persist changes
Oaphi Dec 22, 2025
74eaa4c
CommentThread#remove_follower should remove all followers
Oaphi Dec 22, 2025
d06c750
add_follower should prevent duplicate thread follower records
Oaphi Dec 23, 2025
a4d6b95
moved unfollowing coment threads from unrestrict_thread to its own un…
Oaphi Dec 23, 2025
8f5b894
let's not fail if add_follower / remove_follower is a noop
Oaphi Dec 23, 2025
76f5655
added job for cleaning up duplicate thread followers
Oaphi Dec 23, 2025
b6d0a52
even if there's nothing to do, bump thread's last activity to improve…
Oaphi Dec 23, 2025
d2f2a81
added tests covering CommentThread#remove_follower last activity bumping
Oaphi Dec 23, 2025
8a6e63b
thread wrapper can be absent at the time of handling follow/unfollow …
Oaphi Dec 23, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ Naming/BlockForwarding:
EnforcedStyle: explicit
Naming/PredicateMethod:
Mode: conservative
AllowedPatterns: ['verify_.*', 'check_.*', 'enforce_.*', 'setup_.*']
AllowedPatterns: ['verify_.*', 'check_.*', 'enforce_.*', 'setup_.*','add_.*', 'remove_.*']
AllowedMethods:
- post_sign_in # returns a boolean but is a necessary after-sign-in method for both normal and SAML flows
- comment_rate_limited? # returns a tuple with the boolean as the first value
Expand Down
25 changes: 23 additions & 2 deletions app/assets/javascripts/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,11 +250,32 @@ $(() => {

QPixel.handleJSONResponse(data, () => {
const wrapper = getCommentThreadWrapper($tgt);
const inline = isInlineCommentThread(wrapper);
openThread(wrapper, threadID, { inline });

if (wrapper) {
const inline = isInlineCommentThread(wrapper);
openThread(wrapper, threadID, { inline });
}
});
});

$(document).on('click', '.js--unfollow-thread', async (ev) => {
ev.preventDefault();

const $tgt = $(ev.target);
const threadID = $tgt.data('thread');

const data = await QPixel.unfollowThread(threadID);

QPixel.handleJSONResponse(data, () => {
const wrapper = getCommentThreadWrapper($tgt);

if (wrapper) {
const inline = isInlineCommentThread(wrapper);
openThread(wrapper, threadID, { inline });
}
});
})

/**
* @param {Element} target
*/
Expand Down
8 changes: 8 additions & 0 deletions app/assets/javascripts/qpixel_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,14 @@ window.QPixel = {
return QPixel.parseJSONResponse(resp, 'Failed to follow thread');
},

unfollowThread: async (id) => {
const resp = await QPixel.fetchJSON(`/comments/thread/${id}/unfollow`, {}, {
headers: { 'Accept': 'application/json' },
});

return QPixel.parseJSONResponse(resp, 'Failed to unfollow thread');
},

lockThread: async (id, duration) => {
const resp = await QPixel.fetchJSON(`/comments/thread/${id}/lock`, {
duration,
Expand Down
17 changes: 9 additions & 8 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ class CommentsController < ApplicationController
:archive_thread,
:delete_thread,
:follow_thread,
:unfollow_thread,
:lock_thread,
:thread_unrestrict,
:thread_followers]
Expand Down Expand Up @@ -188,14 +189,16 @@ def show
end

def thread
respond_to do |format|
format.html { render 'comments/thread' }
format.json { render json: @comment_thread }
if stale?(last_modified: @comment_thread.last_activity.utc)
respond_to do |format|
format.html { render 'comments/thread' }
format.json { render json: @comment_thread }
end
end
end

def thread_content
if stale?(last_modified: @comment_thread.last_activity_at.utc)
if stale?(last_modified: @comment_thread.last_activity.utc)
render partial: 'comment_threads/expanded',
locals: { inline: params[:inline] == 'true',
show_deleted: params[:show_deleted_comments] == '1',
Expand Down Expand Up @@ -264,7 +267,7 @@ def delete_thread
end

def follow_thread
status = ThreadFollower.create(comment_thread: @comment_thread, user: current_user)
status = @comment_thread.add_follower(current_user)
restrict_thread_response(@comment_thread, status)
end

Expand Down Expand Up @@ -293,7 +296,7 @@ def undelete_thread
end

def unfollow_thread
status = ThreadFollower.find_by(comment_thread: @comment_thread, user: current_user)&.destroy
status = @comment_thread.remove_follower(current_user)
restrict_thread_response(@comment_thread, status)
end

Expand All @@ -311,8 +314,6 @@ def thread_unrestrict
unarchive_thread
when 'delete'
undelete_thread
when 'follow'
unfollow_thread
else
not_found!
end
Expand Down
24 changes: 24 additions & 0 deletions app/jobs/clean_up_thread_followers_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
class CleanUpThreadFollowersJob < ApplicationJob
queue_as :default

def perform
sql = File.read(Rails.root.join('db/scripts/threads_with_duplicate_followers.sql'))
threads = ActiveRecord::Base.connection.execute(sql).to_a

threads.each do |thread|
user_id, thread_id = thread

followers = ThreadFollower.where(comment_thread_id: thread_id, user_id: user_id)

next unless followers.many?

duplicate = followers.first
result = duplicate.destroy

unless result
puts "failed to destroy thread follower duplicate \"#{duplicate.id}\""
duplicate.errors.each { |e| puts e.full_message }
end
end
end
end
24 changes: 18 additions & 6 deletions app/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,12 @@ class Comment < ApplicationRecord
after_create :create_follower
after_update :delete_thread

before_save :bump_last_activity

counter_culture :comment_thread, column_name: proc { |model| model.deleted? ? nil : 'reply_count' }, touch: true

validate :content_length

# Gets last activity date and time on the comment
# @return [DateTime] last activity date and time
def last_activity_at
[created_at, updated_at].compact.max
end

def root
# If parent_question is nil, the comment is already on a question, so we can just return post.
parent_question || post
Expand All @@ -41,12 +37,28 @@ def content_length
end
end

# Gets last activity date and time on the comment
# @return [DateTime] last activity date and time
def last_activity
[created_at, updated_at, last_activity_at].compact.max
end

def pings
pingable = comment_thread.pingable
matches = content.scan(USER_PING_REG_EXP)
matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i)
end

# Directly bumps the comment's last activity date & time
# @param persist_changes [Boolean] if set to +true+, will persist the changes
def bump_last_activity(persist_changes: false)
self.last_activity_at = DateTime.now

if persist_changes
save
end
end

private

def create_follower
Expand Down
42 changes: 39 additions & 3 deletions app/models/comment_thread.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ class CommentThread < ApplicationRecord

after_create :create_follower

before_save :bump_last_activity

# Gets threads appropriately scoped for a given user & post
# @param user [User, nil] user to check
# @para post [Post] post to check
Expand Down Expand Up @@ -51,9 +53,9 @@ def can_access?(user)

# Gets last activity date and time on the thread
# @return [DateTime] last activity date and time
def last_activity_at
last_comment_activity_at = comments.map(&:last_activity_at).max
[created_at, locked_at, updated_at, last_comment_activity_at].compact.max
def last_activity
last_comment_activity = comments.map(&:last_activity).compact.max
[created_at, updated_at, last_activity_at, last_comment_activity].compact.max
end

# Gets a list of user IDs who should be pingable in the thread.
Expand Down Expand Up @@ -86,6 +88,40 @@ def maximum_title_length
end
end

# Registers a given user as a follower of the thread
# @param user [User] user to register as a follower
# @return [Boolean] status of the operation
def add_follower(user)
if ThreadFollower.where(comment_thread: self, user: user).any?
bump_last_activity(persist_changes: true)
return true
end

ThreadFollower.create(comment_thread: self, user: user)
end

# Directly bumps the thread's last activity date & time
# @param persist_changes [Boolean] if set to +true+, will persist the changes
def bump_last_activity(persist_changes: false)
self.last_activity_at = DateTime.now

if persist_changes
save
end
end

# Removes a given user from the thread's followers
# @param user [User] user to remove from followers
# @return [Boolean] status of the operation
def remove_follower(user)
if ThreadFollower.where(comment_thread: self, user: user).none?
bump_last_activity(persist_changes: true)
return true
end

ThreadFollower.where(comment_thread: self, user: user).destroy_all.any?
end

private

# Comment author and post author are automatically followed to the thread. Question author is NOT
Expand Down
7 changes: 7 additions & 0 deletions app/models/thread_follower.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,17 @@ class ThreadFollower < ApplicationRecord
belongs_to :post, optional: true
belongs_to :user

after_create :bump_thread_last_activity
before_destroy :bump_thread_last_activity

validate :thread_or_post

private

def bump_thread_last_activity
comment_thread&.bump_last_activity(persist_changes: true)
end

def thread_or_post
if comment_thread.nil? && post.nil?
errors.add(:base, 'Must refer to either a comment thread or a post.')
Expand Down
6 changes: 3 additions & 3 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,13 @@ def active_flags(post)
end

# Anonymizes the user (f.e., for the purpose of soft deletion)
# @param persist_changes [Boolean] if set to +false+, will persist the changes
# @param persist_changes [Boolean] if set to +true+, will persist the changes
def anonymize(persist_changes: false)
assign_attributes(username: "user#{id}",
email: "#{id}@deleted.localhost",
password: SecureRandom.hex(32))

unless persist_changes
if persist_changes
skip_reconfirmation!
save
end
Expand All @@ -505,7 +505,7 @@ def soft_delete(attribute_to)
deleted_by_id: attribute_to.id,
deleted_at: DateTime.now)

anonymize(persist_changes: true)
anonymize(persist_changes: false)

skip_reconfirmation!
save
Expand Down
3 changes: 1 addition & 2 deletions app/views/comments/_thread_follow_link.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@

<% if thread.followed_by?(user) %>
<a href="#"
class="widget--header-link js--unrestrict-thread"
data-action="follow"
class="widget--header-link js--unfollow-thread"
data-thread="<%= thread.id %>"
title="You are following this thread and will be notified of every response. You can unfollow at any time."
role="button"
Expand Down
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@
post 'thread/:id/archive', to: 'comments#archive_thread', as: :archive_comment_thread
post 'thread/:id/delete', to: 'comments#delete_thread', as: :delete_comment_thread
post 'thread/:id/follow', to: 'comments#follow_thread', as: :follow_comment_thread
post 'thread/:id/unfollow', to: 'comments#unfollow_thread', as: :unfollow_comment_thread
post 'thread/:id/lock', to: 'comments#lock_thread', as: :lock_comment_thread

post 'thread/:id/unrestrict', to: 'comments#thread_unrestrict', as: :unrestrict_comment_thread
Expand Down
4 changes: 4 additions & 0 deletions config/schedule.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@
runner 'scripts/run_complaints_closure.rb'
end

every 7.days, at: '02:35' do
runner 'scripts/run_thread_followers_cleanup.rb'
end

every 6.hours do
runner 'scripts/recalc_abilities.rb'
end
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20251221140930_add_last_activity_at_to_comments.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddLastActivityAtToComments < ActiveRecord::Migration[7.2]
def change
add_column :comments, :last_activity_at, :datetime
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddLastActivityAtToCommentThreads < ActiveRecord::Migration[7.2]
def change
add_column :comment_threads, :last_activity_at, :datetime
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.2].define(version: 2025_10_15_121326) do
ActiveRecord::Schema[7.2].define(version: 2025_12_21_142105) do
create_table "abilities", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t|
t.bigint "community_id"
t.string "name"
Expand Down Expand Up @@ -176,6 +176,7 @@
t.datetime "updated_at", precision: nil, null: false
t.bigint "community_id", null: false
t.datetime "locked_at", precision: nil
t.datetime "last_activity_at"
t.index ["archived_by_id"], name: "index_comment_threads_on_archived_by_id"
t.index ["community_id"], name: "index_comment_threads_on_community_id"
t.index ["deleted_by_id"], name: "index_comment_threads_on_deleted_by_id"
Expand All @@ -195,6 +196,7 @@
t.boolean "has_reference", default: false, null: false
t.text "reference_text"
t.bigint "references_comment_id"
t.datetime "last_activity_at"
t.index ["comment_thread_id"], name: "index_comments_on_comment_thread_id"
t.index ["community_id"], name: "index_comments_on_community_id"
t.index ["post_id"], name: "index_comments_on_post_type_and_post_id"
Expand Down
13 changes: 13 additions & 0 deletions db/scripts/threads_with_duplicate_followers.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
select
user_id,
comment_thread_id,
count(*) as count
from
thread_followers
where
post_id is null
group by
user_id,
comment_thread_id
having
count > 1;
7 changes: 7 additions & 0 deletions global.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,13 @@ interface QPixel {
*/
followThread?: (id: string) => Promise<QPixelResponseJSON>

/**
* Attempts to unfollow a comment thread
* @param id id of the thread to unfollow
* @returns result of the operation
*/
unfollowThread?: (id: string) => Promise<QPixelResponseJSON>

/**
* Attempts to start following comments on a given post
* @param postId id of the post to follow comments on
Expand Down
1 change: 1 addition & 0 deletions scripts/run_thread_followers_cleanup.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
CleanUpThreadFollowersJob.perform_later
2 changes: 1 addition & 1 deletion test/comments_test_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def try_follow_thread(thread)
# Attempts to unfollow a given comment thread
# @param thread [CommentThread] thread to unfollow
def try_unfollow_thread(thread)
post :thread_unrestrict, params: { id: thread.id, type: 'follow' }
post :unfollow_thread, params: { id: thread.id }
end

# Attempts to follow new threads on a given post
Expand Down
Loading