Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
0f7784c
added Post#deleted_by_owner? predicate
Oaphi Sep 5, 2025
c7fc046
fixed server error when rendering posts/_expanded if deleter user is …
Oaphi Sep 5, 2025
a078946
fixed server error when rendering reactions from hard-deleted users
Oaphi Sep 5, 2025
037f560
fixed unexpected error when opening a comment thread lock modal
Oaphi Sep 5, 2025
595d9d1
added missing locked_at column to comment threads
Oaphi Sep 5, 2025
0dd8fa1
added Ability#post_score_percent_for(user) nil-safe method
Oaphi Sep 6, 2025
94076a2
added Ability#edit_score_percent_for(user) nil-safe method
Oaphi Sep 6, 2025
d32ae6b
added Ability#flag_score_percent_for(user) nil-safe method
Oaphi Sep 6, 2025
f89cf87
consider abilities with threshold set to 0 to be automatically reached
Oaphi Sep 6, 2025
4f8465b
added failure path tests for score_percent_for methods of Ability model
Oaphi Sep 6, 2025
5de8faa
added auto success path tests for score_percent_for methods of Abilit…
Oaphi Sep 6, 2025
d12c773
added test for edit_score_percent_for actual calculation
Oaphi Sep 6, 2025
aed68e6
added test for flag_score_percent_for actual calculation
Oaphi Sep 6, 2025
85797f1
added missing 'everyone' user ability to new user fixtures
Oaphi Sep 6, 2025
f67cb47
added missing 'unrestricted' user ability to new user fixtures
Oaphi Sep 6, 2025
4ad6032
made User#ability_on? tests more robust
Oaphi Sep 6, 2025
bbd8420
added test for post_score_percent_for actual calculation
Oaphi Sep 6, 2025
9968126
made ability score user fixtures clearer in intent
Oaphi Sep 6, 2025
ce1dcbb
moved comment thread locking into a proper lock_thread action with er…
Oaphi Sep 7, 2025
92014eb
don't forget to remove testing overrides
Oaphi Sep 7, 2025
5149019
split thread restrict actions into individual actions
Oaphi Sep 7, 2025
a191df6
split thread unrestrict actions into individual actions
Oaphi Sep 7, 2025
b22c736
disambiguated post_scorer_question fixture's meaning of 'positively s…
Oaphi Sep 8, 2025
bc6bacb
finished splitting up thread_restrict action
Oaphi Sep 9, 2025
3b974bf
restored follow comment thread client-side functionality
Oaphi Sep 9, 2025
027d186
fixed retrict actions on expanded thread view loading inline version …
Oaphi Sep 9, 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 app/assets/javascripts/comments.js
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ $(() => {
$modal.find('.js-follower-display').html(data);
});

$(document).on('click', '[class*=js--lock-thread] form', async (evt) => {
$(document).on('submit', '[class*=js--lock-thread] form', async (evt) => {
evt.preventDefault();

const $tgt = $(evt.target);
Expand Down
96 changes: 73 additions & 23 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,47 +219,85 @@ def thread_rename
redirect_to comment_thread_path(@comment_thread.id)
end

def archive_thread
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For other reviewers: the code in this block is copied from elsewhere, no changes to logic.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be more precise, these mehtods are more or less extractions of individual cases of thread_restrict and thread_unrestrict actions. It's intended that they'll become proper actions in the future (cc @trichoplax given that you are working on thread followers - there'll now be follow_thread and unfollow_thread methods)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a note for other reviewers - I went ahead and split the thread_restrict action, so archiving, deleting, locking, and following threads need to be tested.

Copy link
Member

@cellio cellio Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested archive/restore, delete/undelete, lock/unlock, and follow/unfollow, all from the in-page tools menu. All of that worked fine.

I then repeated the tests on the thread page. For lock, archive, delete, and follow, the action worked but when the page was refreshed, it showed the "collapse" and "show thread" controls (from the in-page view), which don't make sense here:

screenshot with extra controls highlighted

Reloading the page removes them, so this is only an issue on the immediate response to the action, which I guess is assuming context from the in-page view? (I.e. the header with the Tools menu etc can't tell where it is, maybe?) Not a big deal but a bit weird -- can be deferred if this isn't a trivial fix.

Interestingly, this does not happen for unlock, restore, undelete, and unfollow. I looked for a difference in the controller (since these used to be rolled up into two methods, "restrict" and "unrestrict"), but I'm not spotting it if that's where it's coming from. The comment_thread view isn't changed on this PR, but this behavior doesn't happen on the dev server as of 5c83ab90 (2025-09-05 19:17:57Z). That probably means it's in the Javascript, heaven help us.

Copy link
Member Author

@Oaphi Oaphi Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, that's likely just a side-effect of that we now replace the thread's content instead of reloading the whole page. It works as expected (this obviously doesn't happen for thread_unrestrict actions as those retain the old behavior for now unless I end up splitting those too), I suspect we also have the same issue when expanding threads from inline view. Good catch, let's fix it while we are at it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed now, @cellio

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working now, thanks!

status = @comment_thread.update(archived: true, archived_by: current_user)
restrict_thread_response(@comment_thread, status)
end

def delete_thread
status = @comment_thread.update(deleted: true, deleted_by: current_user)
restrict_thread_response(@comment_thread, status)
end

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

def lock_thread
lu = nil
unless params[:duration].blank?
lu = params[:duration].to_i.days.from_now
end

status = @comment_thread.update(locked: true, locked_by: current_user, locked_until: lu)
restrict_thread_response(@comment_thread, status)
end

def unarchive_thread
status = @comment_thread.update(archived: false, archived_by: nil, ever_archived_before: true)
restrict_thread_response(@comment_thread, status)
end

def undelete_thread
if @comment_thread.deleted_by.at_least_moderator? && !current_user.at_least_moderator?
render json: { status: 'error', message: I18n.t('comments.errors.mod_only_undelete') }
return
end
status = @comment_thread.update(deleted: false, deleted_by: nil)
restrict_thread_response(@comment_thread, status)
end

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

def unlock_thread
status = @comment_thread.update(locked: false, locked_by: nil, locked_until: nil)
restrict_thread_response(@comment_thread, status)
end

def thread_restrict
# TODO: remove this wrapper action entirely (callbacks need to be moved, routes assigned, etc)
case params[:type]
when 'lock'
lu = nil
unless params[:duration].blank?
lu = params[:duration].to_i.days.from_now
end
@comment_thread.update(locked: true, locked_by: current_user, locked_until: lu)
lock_thread
when 'archive'
@comment_thread.update(archived: true, archived_by: current_user)
archive_thread
when 'delete'
@comment_thread.update(deleted: true, deleted_by: current_user)
delete_thread
when 'follow'
ThreadFollower.create comment_thread: @comment_thread, user: current_user
follow_thread
else
return not_found!
not_found!
end

render json: { status: 'success' }
end

def thread_unrestrict
# TODO: remove this wrapper action entirely (callbacks need to be moved, routes assigned, etc)
case params[:type]
when 'lock'
@comment_thread.update(locked: false, locked_by: nil, locked_until: nil)
unlock_thread
when 'archive'
@comment_thread.update(archived: false, archived_by: nil, ever_archived_before: true)
unarchive_thread
when 'delete'
if @comment_thread.deleted_by.at_least_moderator? && !current_user.at_least_moderator?
render json: { status: 'error', message: I18n.t('comments.errors.mod_only_undelete') }
return
end
@comment_thread.update(deleted: false, deleted_by: nil)
undelete_thread
when 'follow'
tf = ThreadFollower.find_by(comment_thread: @comment_thread, user: current_user)
tf&.destroy
unfollow_thread
else
return not_found!
not_found!
end

render json: { status: 'success' }
end

def post
Expand Down Expand Up @@ -403,6 +441,18 @@ def check_for_pings(thread, content)
matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i)
end

# @param thread [CommentThread] thread to get response for
# @param status [Boolean] status of the restrict operation
def restrict_thread_response(thread, status)
if status
render json: { status: 'success', thread: thread }
else
render json: { status: 'failed',
message: thread.errors.full_messages.join(', ') },
status: :bad_request
end
end

# @param pings [Array<Integer>] list of pinged user ids
def apply_pings(pings)
pings.each do |p|
Expand Down
40 changes: 40 additions & 0 deletions app/models/ability.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,52 @@
class Ability < ApplicationRecord
include CommunityRelated
include AbilitiesHelper

validates :internal_id, uniqueness: { scope: [:community_id], case_sensitive: false }

def manual?
post_score_threshold.nil? && edit_score_threshold.nil? && flag_score_threshold.nil?
end

# Gets the edit score percent for a given user
# @param user [User, nil] user to get the percent for
# @return [Integer] edit score percent
def edit_score_percent_for(user)
return 0 if edit_score_threshold.nil? || user.nil?
return 100 if edit_score_threshold.zero?

linear_score = linearize_progress(user.community_user.edit_score)
linear_threshold = linearize_progress(edit_score_threshold)

(linear_score / linear_threshold * 100).to_i
end

# Gets the flag score percent for a given user
# @param user [User, nil] user to get the percent for
# @return [Integer] flag score percent
def flag_score_percent_for(user)
return 0 if flag_score_threshold.nil? || user.nil?
return 100 if flag_score_threshold.zero?

linear_score = linearize_progress(user.community_user.flag_score)
linear_threshold = linearize_progress(flag_score_threshold)

(linear_score / linear_threshold * 100).to_i
end

# Gets the post score percent for a given user
# @param user [User, nil] user to get the percent for
# @return [Integer] post score percent
def post_score_percent_for(user)
return 0 if post_score_threshold.nil? || user.nil?
return 100 if post_score_threshold.zero?

linear_score = linearize_progress(user.community_user.post_score)
linear_threshold = linearize_progress(post_score_threshold)

(linear_score / linear_threshold * 100).to_i
end

def self.on_user(user)
Ability.where(id: UserAbility.where(community_user: user.community_user).select(:ability_id).distinct)
end
Expand Down
6 changes: 6 additions & 0 deletions app/models/post.rb
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,12 @@ def closeable?
post_type.is_closeable
end

# Is the post deleted by the owner?
# @return [Boolean] check result
def deleted_by_owner?
deleted_by&.same_as?(user)
end

# @return [Boolean] whether there is a suggested edit pending for this post
def pending_suggested_edit?
SuggestedEdit.where(post_id: id, active: true).any?
Expand Down
42 changes: 21 additions & 21 deletions app/views/abilities/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,11 @@
<div class="widget--body">
<% if @user.id == current_user&.id %>
<p>You need to reach these thresholds to earn the ability:</p>
<% else %>
<% else %>
<p><%= @user.username %> needs to reach these thresholds to earn the ability:</p>
<% end %>
<% end %>
<% unless @ability.post_score_threshold.nil? %>
<% post_score_percent = (linearize_progress(@user.community_user.post_score) / linearize_progress(@ability.post_score_threshold) * 100).to_i %>
<% post_score_percent = @ability.post_score_percent_for(@user) %>

<p><strong>Post score threshold</strong></p>
<% if post_score_percent < 100 %>
Expand All @@ -74,7 +74,7 @@
<% end %>

<% unless @ability.edit_score_threshold.nil? %>
<% edit_score_percent = (linearize_progress(@user.community_user.edit_score) / linearize_progress(@ability.edit_score_threshold) * 100).to_i %>
<% edit_score_percent = @ability.edit_score_percent_for(@user) %>

<p><strong>Edit score threshold</strong></p>
<% if edit_score_percent < 100 %>
Expand All @@ -93,7 +93,7 @@
<% end %>

<% unless @ability.flag_score_threshold.nil? %>
<% flag_score_percent = (linearize_progress(@user.community_user.flag_score) / linearize_progress(@ability.flag_score_threshold) * 100).to_i %>
<% flag_score_percent = @ability.flag_score_percent_for(@user) %>

<p><strong>Flag score threshold</strong></p>
<% if flag_score_percent < 100 %>
Expand Down Expand Up @@ -124,21 +124,21 @@
<% end %>
</div>
<% if @your_ability&.is_suspended && (@user.id == current_user&.id || current_user&.at_least_moderator?) %>
<div class="widget--body">
<div class="notice is-danger">
<% if @your_ability.suspension_end.nil? %>
<p><strong>Your use of the <%= @ability.name %> ability has been suspended.</strong></p>
<% else %>
<p><strong>Your use of the <%= @ability.name %> ability has been temporarily suspended (ends <span title="<%= @your_ability.suspension_end.iso8601 %>">in <%= time_ago_in_words(@your_ability.suspension_end) %></span>)</strong>.</p>
<% end %>
<p><%= @your_ability.suspension_message %></p>
<p>If you have any questions regarding the site rules, you can ask them in the Meta category of this site or on <a href="https://meta.codidact.com/">meta.codidact.com</a>. If you have any questions about this suspension or would like to dispute it, use the Meta category or <a href="mailto:<%= SiteSetting['AdministratorContactEmail'] %>">contact us</a>.
</div>
<div class="widget--body">
<div class="notice is-danger">
<% if @your_ability.suspension_end.nil? %>
<p><strong>Your use of the <%= @ability.name %> ability has been suspended.</strong></p>
<% else %>
<p><strong>Your use of the <%= @ability.name %> ability has been temporarily suspended (ends <span title="<%= @your_ability.suspension_end.iso8601 %>">in <%= time_ago_in_words(@your_ability.suspension_end) %></span>)</strong>.</p>
<% end %>
<p><%= @your_ability.suspension_message %></p>
<p>If you have any questions regarding the site rules, you can ask them in the Meta category of this site or on <a href="https://meta.codidact.com/">meta.codidact.com</a>. If you have any questions about this suspension or would like to dispute it, use the Meta category or <a href="mailto:<%= SiteSetting['AdministratorContactEmail'] %>">contact us</a>.
</div>
</div>
<% end %>
</div>
<% end %>
</div>
<% end %>
<% end %>

<div class="h-p-2">
<%= raw @ability.description %>
</div>
<div class="h-p-2">
<%= raw @ability.description %>
</div>
12 changes: 7 additions & 5 deletions app/views/comments/_lock_thread_modal.html.erb
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
<%#
Helper for rendering comment thread lock action modal
"Helper for rendering comment thread lock action modal

Variables:
thread : Comment thread to create the modal for
%>
Variables:
thread : Comment thread to create the modal for
"%>

<div class="modal is-with-backdrop is-small js--lock-thread-<%= thread.id %>">
<%= form_tag restrict_comment_thread_path(thread.id), class: 'modal--container' do %>
<%= form_tag restrict_comment_thread_path(thread.id),
class: 'modal--container',
data: { thread: thread.id } do %>
<%= hidden_field_tag :type, 'lock' %>
<div class="modal--header">
<button type="button"
Expand Down
11 changes: 5 additions & 6 deletions app/views/posts/_expanded.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -131,24 +131,23 @@
</div>
<% end %>

<% if post.deleted %>
<% if post.deleted? %>
<div class="notice has-margin-2">
<p class="has-font-weight-normal">
<strong>Deleted</strong> by <%= user_link post.deleted_by %>
on <%= post.deleted_at.strftime('%b %e, %Y at %H:%M') %>
</p>
<hr>
<% if post.deleted_by.id == post.user_id %>
<% if post.deleted_by_owner? %>
<p>
The author of this post decided to delete it. It can only be seen by users with the Curate privilege.
The author of this post decided to delete it. It can only be seen by users with the Curate ability.
</p>
<% else %>
<p>
This post was deleted and can only be seen by users with the Curate privilege.
This post was deleted and can only be seen by users with the Curate ability.
</p>
<% end %>

<p>Users with the Curate privilege may vote to restore this post if it has been deleted incorrectly.</p>
<p>Users with the Curate ability may vote to restore this post if it has been deleted incorrectly.</p>
</div>
<% end %>

Expand Down
7 changes: 4 additions & 3 deletions app/views/reactions/_list.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
<% end %>
</div>

<%# TODO: eager load instead %>
<% host = post.community.host %>

<% post.reaction_list.each do |rt, rr| %>
<div class="modal is-with-backdrop reaction-info-modal"
id="reaction-info-<%= post.id %>-<%= rt.id %>">
Expand All @@ -43,9 +46,7 @@
</tr>
<% rr.each do |r| %>
<tr>
<td>
<%= link_to rtl_safe_username(r.user), user_path(r.user), dir: 'ltr'%>
</td>
<td><%= user_link(r.user, { host: host }) %></td>
<td class="comment-col">
<% if r.comment %>
<div class="muted-p">
Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20250905231140_add_locked_at_to_comment_threads.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddLockedAtToCommentThreads < ActiveRecord::Migration[7.2]
def change
add_column :comment_threads, :locked_at, :datetime, precision: nil
end
end
3 changes: 2 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_08_10_133540) do
ActiveRecord::Schema[7.2].define(version: 2025_09_05_231140) 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 @@ -175,6 +175,7 @@
t.datetime "created_at", precision: nil, null: false
t.datetime "updated_at", precision: nil, null: false
t.bigint "community_id", null: false
t.datetime "locked_at", precision: nil
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 Down
Loading