General improvements to comment threads

[Oaphi]

Actually closes #1579, likely closes #544

This PR builds on the work done in #1584 by preventing the buttons for creating a new comment thread or replying in an existent one from showing up if the user can't comment (not only when it's impossible to comment [specifically, when the user is not a mod or an admin & comments on the post are disabled, or it is locked or deleted]).

Most importantly, it adds 2 new site settings for rate-limiting comments:

• RL_NewUserCommentsOwnPosts for new user comments on their own posts;
• RL_CommentsOwnPosts for unrestricted user comments on their own posts;

Combined with the existing settings, users will now be limited to the following number of comments per day (by default):

• new users: 0 comments on posts of others, 30 on their own posts & answers to them;
• unrestricted users: 50 comments on posts of others, 50 on their own posts & answers to them;

Open to discussion on whether to adjust the number of comments users can make on their own posts per day.

---

The PR also ensures that the add thread / reply buttons are always shown to users but disabled if they can't comment for one reason or the other. The exact reason as to why a user can't comment is moved to the button's title (instead of littering the page with huge notices):

https://github.com/user-attachments/assets/9b6c576e-3e45-464b-af9b-89b6d032399e

It also makes possible to take actions on inline comment threads (comment editing & deleting included):

https://github.com/user-attachments/assets/7b314983-34aa-4522-a945-c2e8ca378455

Plus, locked threads in post comment threads lists now show the lock icon (as do other "restricted" states):

We also no longer display the "Showing X of Y comments. See the whole thread." widget if X is equal to Y (it simply wastes space in such cases):

---

It also adds several scopes (and subsequently increases coverage across the project to make sure the new scopes don't break anything important):

model scopes

• AuditLog.of_type(name) (only include logs of a specific type by name);
• Comment.by(user) (only include ones made by the user);
• Flag.by(user) (same as Comment.by);
• Flag.declined (only include those that are handled as declined);
• Flag.helpful (only include those that are handled as helpful);
• ModWarning.active (only include warnings that are currently active);
• ModWarning.to(user) (only include warnings issued to the user);
• Post.bad (only include posts with score < 0.5);
• Post.by(user) (same as Comment.by);
• Post.good (only include posts with score > 0.5);
• Post.in(category) (only include posts made in the category);
• Post.on(community) (only include posts made on the community);
• Post.parent_by(user) (only include posts that have a parent made by the user);
• Post.problematic (only include posts with score < 0.25 or that are deleted);
• PostHistory.by (same as Comment.by);
• PostHistory.of_type(name) (same as AuditLog.of_type);
• PostHistory.on_undeleted (only include events made on undeleted posts);
• SuggestedEdit.approved (only include those that are decided as approved);
• SuggestedEdit.by(user) (same as Comment.by);
• SuggestedEdit.rejected (only include those that are decided as rejected);
• Vote.by(user) (same as Comment.by);
• Vote.for(user) (only include those where the target is the user);

concern (shared) scopes

• SoftDeletable.deleted (only include ones that are marked as deleted);
• SortDeletable.undeleted (the inverse of SoftDeletable.deleted);
• Timestamped.newest_first (order by created_at DESC);
• Timestamped.oldest_first (order by created_at ASC);
• Timestamped.recent (only include ones made in the last 24 hours);

as well as removes some existing scopes:

• CommunityUser.active (replaced by CommunityUser.undeleted to align with other soft-deletable models);
• User.active (replaced by User.undeleted to align with other soft-deletable models);

It also removes check_edits_limit! as it's been unused since 412b49096da9292eb0ed366df3be89a85ff8a3a8 (the check has been done inline for at least 4 years - we might want to abstract it into a similar helper).

Additionally, it adds a QoL []= class method to the SiteSetting model for us to be able to update site settings simply via SiteSetting[name] = value without having to explicitly handle value type conversion & cache updates.

Tangentially, it also adds two new methods:

• max_votes_per_day that functions identically to max_comments_per_day (proper logic encapsulation);
• recent_votes_count that functions similarly to recent_comments_count (unsure why we exclude self votes on parents but not votes on own posts there);

as well as renames some existing ones:

• User#can_push_to_network to can_push_to_network? (to align with the naming convention for boolean checks);
• User#can_update to can_update? (same as with can_push_to_network);
• User#can_see_deleted? to can_see_deleted_posts? (the check only applies to posts);
• User#has_ability_on to ability_on? (same as can_push_to_network);
• User#has_profile_on to profile_on? (same as can_push_to_network);
• User#is_moderator_on to moderator_on? (same as can_push_to_network);
• ModeratorController#verify_can_see_deleted to verify_can_see_deleted_posts (same as with can_see_deleted?);

and a couple of shared concerns:

• Inspectable that provides a generic inspect method that can be used on any model;
• SoftDeletable for logic shared by all models representing soft-deletable records;
• Timestamped for logic shared by all models representing timestamped records;
• UserRateLimits that houses methods related to various rate limits (f.e., comments & votes per day);
• UsernameValidations that encapsulates username-related validations (user model is too large);

Some extras:

• Layout/SpaceInsideArrayLiteralBrackets is now enabled to match our style guide for Ruby;
• assert_json_response_message(expected) test helper method for asserting that response body contains a given message;
• assert_redirected_to_sign_in test helper method for asserting that response has the :found HTTP status code & redirects the requestor to sign in;

Related meta issue: meta:294088

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 69.44%. Comparing base (7a2a1f6) to head (88c2345). Report is 241 commits behind head on develop.

:exclamation: Current head 88c2345 differs from pull request most recent head 4dcaf45

Please upload reports for the commit 4dcaf45 to get more accurate results.

Additional details and impacted files

Components	Coverage Δ
controllers	64.80% <100.00%> (+2.99%)	:arrow_up:
helpers	70.80% <100.00%> (+1.51%)	:arrow_up:
jobs	48.57% <ø> (ø)
models	85.17% <100.00%> (+3.24%)	:arrow_up:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Oaphi]

Leaving the coverage miss be as the logic there hasn't changed at all and both cases are covered already except for the audit log creation part, which definitely hasn't changed.

[Oaphi]

If a user can't comment (for the reasons in this PR), how is the message delivered to the user?

Because it isn't since the button will no longer be present if the user can't comment due to either lacking the privilege or being rate-limited in addition to the current reasons. Showing a notice as with the locked posts / posts with comments disabled will likely be a bit too noisy. If my memory serves me right, we decided to switch to always show the button but disable it instead with a tooltip explaining the reason - if that's the case, we can address the issue when we get to it.

Open to ideas for an interim solution until the PR is merged

[Oaphi]

Switching back to draft as we've decided to apply global rate limits regardless of whether the user is the post owner.

[cellio]

Because it isn't since the button will no longer be present if the user can't comment due to either lacking the privilege or being rate-limited in addition to the current reasons. Showing a notice as with the locked posts / posts with comments disabled will likely be a bit too noisy. If my memory serves me right, we decided to switch to always show the button but disable it instead with a tooltip explaining the reason - if that's the case, we can address the issue when we get to it.

Oh right -- thanks for the reminder. That's a fine end state (disabled button with tooltip), and I agree we should do that in a separate change. Is that a big change? If we can do that fairly soon then I'm not too worried about an interim solution, but if we think it'll take a while, we should think about what we can do in the meantime, else we'll get questions from puzzled users looking for the comment button.

[Oaphi]

. Is that a big change?

Not a big change, @cellio, just depends on my availability as usual 🙂. Will throw it in if I can spare some time

[cellio]

Does enabled inline actions on comments (editing, deletion) also address (or enable addressing) https://github.com/codidact/qpixel/issues/544 ?

[Oaphi]

also address (or enable addressing) https://github.com/codidact/qpixel/issues/544 ?

I believe so, @cellio - doesn't seem like there's currently any issue with the "flag" button working - I've just successfully tested comment flagging. It's likely that there are more issues that this PR ends up addressing, but I haven't even started looking through issues yet

[cellio]

The latest commit might address https://meta.codidact.com/posts/282342/282348#answer-282348 and https://meta.codidact.com/posts/282342/282382#answer-282382 (check in testing).

[Oaphi]

For reference as the description is way too long as it is - the "showing X of Y comments" widget will also take less space & come before the "reply to this thread button" section (as it doesn't make sense to put the widget after it). Not married to the centering but, in my view, it's at least better than the left-aligned version (a proper redesign is out of scope of this PR):

For comparison, this is how the sections are currently rendered together:

[Oaphi]

Note to folks stumbling on the PR: it's 99% done, feel free to review even though it's not marked as ready yet to avoid wasting time, the PR is massive. I only plan on adding a couple of tests and translation strings

[Oaphi]

Aaaand we are done, finally - have at it, folks!

Addendum: don't forget to run the seeds, there are two new site settings

[cellio]

Testing notes:

• Tested the own-comments limit for someone without Participate Everywhere and worked as advertised: upon reaching the limit buttons are grayed out with suitable message, and buttons on others' posts are grayed out at all times with suitable message.

• Tested editing, flagging, deleting, undeleting a comment from the in-page view -- beautiful!

• Confirmed that curators, who can lock posts, cannot comment on them (as expected), and that the button is grayed out and has a suitable tooltip.

• Also confirmed that moderators can comment on locked posts (as expected). Future (do not block on this!): I wonder if we should change the presentation for mods at all for the "most people can't comment but you can" cases; they get a tooltip, but people might not notice that especially on mobile.

• When clicking on the "reply" button, either in-page or in a thread, can the textbox get focus automatically instead of the user having to click into it?

• I love that the tools menu is available in-page. When I click on it from a comment thread way down on the page, though, it takes me to the top of the page (not sure if it's refreshing and defaulting to the top or what). As a user, this means I click on "tools" and then don't see it until I page down again.

• As mentioned in chat, when you add a comment from the in-page view, we take you to the thread. If the thread is already long this is the only way to see your new comment, but we probably don't want to change the redirect behavior based on whether your new comment would be visible. I think leaving you on the in-page view where you started is ok if we can somehow indicate that your comment was successfully posted, since you might not see it. I'm not sure of the best way to do that, both UX and code, so I think it's ok to redirect to the thread page until we figure that out. But we should remember to alert people to this when we announce the new comment features.

• As a mod, I locked a thread from the in-page view and it took me to the thread. There are no issues with showing new comments here, so let's leave you on the original page, adding the lock icon.

• ~~Minor layout thing: when there are deleted comments among the first 5, the indication of such (for those who can see deleted comments) has lost the padding it used to have:~~ -- Never mind; this was pre-existing behavior. (The spacing is better on the thread page and I think that's what I was remembering.)

[ArtOfCode-]

Approved from a code POV - haven't run locally to look at the UI/UX but looks like @cellio has covered that.

[cellio]

Approved from a code POV - haven't run locally to look at the UI/UX but looks like @cellio has covered that.

Yup. I haven't looked at the code, just the user-facing behavior.

[Oaphi]

Finish line TODO list:

• [x] add a widget before the newly created reply indicating that there are more comments in the thread than shown (if any);
• [x] display moderator exemption message inline with the new thread / reply to thread buttons;
  • [x] with the reply to thread button;
  • [x] with the new thread button;
• [x] autofocus the reply textarea when the reply to thread button is clicked;
• [x] preserve scroll state after opening tools:
  • [x] after clicking on the tools button;
  • [x] after clicking on one of the actions;
• [x] show the newly created comment inline if it's been created via the inline thread view;
  • [x] get rid of full page rendering & slicing off on the "THREAD STARTS ABOVE" / "THREAD ENDS BELOW" boundaries when requesting inline thread views;
  • [x] properly document parameters, instance variables, and variables used by comment thread views;
  • [x] expand thread by default after a redirect is made upon successfully replying to a comment thread;
• [x] do not redirect to the full view after locking / unlocking a thread from the inline view;
• [x] optional: improve styling of the "skipping deleted comment" thread section;
• [x] clicking on the "edit" link after entering comment edit mode shouldn't break the discard edit button;

[cellio]

• preserve scroll state after opening tools (might be better suited for Co-Design);

Or if that's too hard (or sends us into Co-Design), if the refresh went to the post link instead of the page link (e.g. the specific answer), you'd at least land nearby.

[Oaphi]

Before I forget, this is how the "skipping deleted comment" widget will look like - the padding is fixed & its styling is aligned the "showing X of Y" widget:

[Oaphi]

After successfully replying to a thread, we now redirect back to the post (unless the reply is made from the full view), but also ensure that the thread is expanded & that the newly created comment is shown even if it would be hidden due to the thread having 5 or more replies already (the count is adjusted accordingly):

For now, this is done via query parameters (which means that we can now do direct thread/comment links to inline views - that said, I haven't added any special styling to indicate that the comment is directly linked to - not yet at least). Ideally, this should be done as a new route posts/<post_id>/threads/<thread_id>/comments/<comment_id> (or something similar), but that's way too much work:

https://github.com/user-attachments/assets/a214555c-70b0-4b92-84bd-fbf694592ef8

[Oaphi]

And this is what I opted for to make moderator exemption message easier to find - a muted icon will be shown to the right of the button with the exemption text as its title displayed on hover:

https://github.com/user-attachments/assets/3f4ec9e1-8b0c-4559-a9e0-2bfa42ca5dbb

[cellio]

After successfully replying to a thread, we now redirect back to the post (unless the reply is made from the full view), but also ensure that the thread is expanded & that the newly created comment is shown even if it would be hidden due to the thread having 5 or more replies already (the count is adjusted accordingly):

Looks great! I was going to ask if, in that inline view after posting the comment, we could have some sort of "15 more comments" or "..." or something as a separator before the new comment, but the only person who will ever see that view is the author of that new comment, and if that person didn't look at the intervening comments before posting, I don't think a reminder now will change anything. (I'm leaving this comment to raise and then explain away that issue. :-) )

For now, this is done via query parameters (which means that we can now do direct thread/comment links to inline views - that said, I haven't added any special styling to indicate that the comment is directly linked to - not yet at least). Ideally, this should be done as a new route posts/<post_id>/threads/<thread_id>/comments/<comment_id> (or something similar), but that's way too much work:

Completely agreed -- if we want that (and I don't know if we do), that is a "future us" problem, not here!

[Oaphi]

if, in that inline view after posting the comment, we could have some sort of "15 more comments" or "..." or something as a separator before the new comment

Was already working on it 😇. This is what I intend to add (note that I also reduced size of all the "skipping M deleted", "skipping M more", and "showing M of N" widgets to save up on vertical space for long threads):

Later on, we can add a link that will load the skipped comments inline and subsequently get rid of the "showing M of N" widget.

[Oaphi]

Important note: /coverage directory is now mounted in dev mode (to generate the necessary assets, set the SimpleCov's formatter to SimpleCov::Formatter::HTMLFormatter or use the multi-formatter). If you run the tests locally, going to the /coverage path will serve the directory for ease of access ("thanks", CircleCI).

[Oaphi]

Probably won't move it from the draft status for a couple more days (want to improve coverage after the latest changes first), but I believe it's ready otherwise. Made re-review requests as there's been quite a few changes since the initial approvals (mostly what's listed in the finish line comment)

[cellio]

Probably won't move it from the draft status for a couple more days (want to improve coverage after the latest changes first), but I believe it's ready otherwise. Made re-review requests as there's been quite a few changes since the initial approvals (mostly what's listed in the finish line comment)

I'll test the things in the finish-line comment. I assume someone else will review the code changes.

[cellio]

Testing notes based on checklist:

• autofocus the reply textarea when the reply to thread button is clicked - I'm not getting auto-focus using Chrome or Firefox - I don't think we need to block on this; we can do as a followup

Confirmed working:

• display moderator exemption message inline with the new thread / reply to thread buttons
• preserve scroll state after opening tools (took me nearby, not exactly on that answer, but ok; I tested on a busy page)
• show the newly created comment inline if it's been created via the inline thread view ❤️
• do not redirect to the full view after locking / unlocking a thread from the inline view
• optional: improve styling of the "skipping deleted comment" thread section
• clicking on the "edit" link after entering comment edit mode shouldn't break the discard edit button

[cellio]

That fixed it, thanks! (Tested Chrome and Firefox, though it doesn't sound like this was browser-dependent anyway.)