feat: Allow users to report comments #3016

arkirchner · 2025-07-14T14:17:33Z

Comments can be used to potentially harass users who request comments. A button was added to report a message as spam.

Resolves #2715

arkirchner · 2025-07-14T14:31:10Z

app/controllers/request_for_comments_controller.rb

@@ -168,7 +168,7 @@ def report

    ReportMailer.with(reported_content: @request_for_comment).report_content.deliver_later

-    redirect_to @request_for_comment, notice: t('.report.reported'), status: :see_other
+    redirect_to @request_for_comment, notice: t('.reported'), status: :see_other


Through unrelated changes in the locals, i18_task asked me to adjust the path. The message localization is unchanged.

codecov · 2025-07-14T14:35:11Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.23%. Comparing base (d40557c) to head (def5481).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3016      +/-   ##
==========================================
+ Coverage   70.08%   70.23%   +0.14%     
==========================================
  Files         215      216       +1     
  Lines        6850     6870      +20     
==========================================
+ Hits         4801     4825      +24     
+ Misses       2049     2045       -4

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

arkirchner · 2025-07-14T14:53:40Z

app/models/spam_report.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+class SpamReport


I extracted this object out of the mailer because the mailer logic has become quite complex.

I have done this for the following reasons:

I wanted to test the delegation between Comment and RfC in isolation (without rendering email).

The email feature would become too complex with this much logic (single responsibility).

Putting this into the model is a bit debatable. I think in smaller Rails this is a valid strategy.

Another approach could have been to create separate mailers for comments and RfCs. I decided against this because I think this approach keeps more of the business logic out of the mailer.

arkirchner · 2025-07-14T14:56:46Z

app/assets/javascripts/request_for_comments.js

@@ -116,6 +116,9 @@ $(document).on('turbo-migration:load', function () {
            <button class="action-edit btn btn-sm btn-warning">' + I18n.t('shared.edit') + '</button> \
            <button class="action-delete btn btn-sm btn-danger">' + I18n.t('shared.destroy') + '</button> \
          </div> \
+          <div class="text-warning' + (comment.reportable ? '' : ' d-none') + '"> \


Originally I wanted to add this feature by utilizing the Turbo framework. However, the changes seemed a bit large, and I want to release this soon. Therefore, I thought it was not a good time for the Turbo migration.

arkirchner · 2025-07-14T15:03:37Z

app/views/comments/index.json.jbuilder

@@ -1,6 +1,11 @@
 # frozen_string_literal: true

 json.array!(@comments) do |comment|
-  json.extract! comment, :id, :user_id, :file_id, :row, :column, :text, :username, :date, :updated, :editable
+  json.extract! comment, :id, :user_id, :file_id, :row, :column, :text


Please allow me this small refactor with this PR. I think this view-related field should be set in the template and not in the Controller and Model.

kkoehn

I left a few comments and suggestions.

What do you think about moving the button to the right? It looks kind of lost down there..

config/locales/de/comment.yml

config/locales/en/comment.yml

kkoehn · 2025-07-16T19:47:45Z

app/models/spam_report.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+class SpamReport


I think it's a good idea to extract that logic, but I would propose a different name. SpamReport sounds very specific and as far as I understand, we not really trying to catch spam here but inappropriate content. Wouldn't Report be sufficient?

Additionally I'm not sure if models/ is the right folder for this class. AFAIK we don't have this kind of class yet, so we would have to create a new place for this.

I have renamed SpamReport to UserContentReport. I think it makes it more clear that any kind of user is reported not only spam.

Furthermore, I investigated where to put these kind objects. I considered turning it into a form object to stick it into app/forms. But this is not a form and just would make it more difficult. I also considered if this model is a parameter object, but I think it is not because of the business logic.

I think it should remain in models for the following reasons:

I think extracted objects that are not common patterns like Forms, Parameters, or Policies should remain in /app/models. The model folder holds objects with business logic. In my opinion it is not necessary for these objects to have a persistence layer.

For this kind of logic objects we have no meaningful suffix. For other patterns, the suffix like UserPolicy, UserController or SignUpForm tells you where this object is placed. Any object without that should live in /app/models/ 🤔

In this example of extracting a class from a Rails controller the models folder was used as well.
https://thoughtbot.com/ruby-science/extract-class.html

I am not perfectly happy with this as well, but I don't know how to solve this in a better way.

arkirchner · 2025-07-16T21:49:00Z

I have only looked at this feature from the users views. I am not sure if a teacher/admin should be able to report content? 🤔

For now I will try to adjust the view.

arkirchner · 2025-07-17T12:44:57Z

View as a learner who commented on the RfC.

View as a teacher/admin.

Comments can be used to potentially harass users who request comments. A button was added to report a message as spam. Resolves #2715

Co-authored-by: kkoehn <[email protected]>

Not only Spam will be reported. Any user content can be reported. Mailer name was adjusted to fit the naming convention.

arkirchner self-assigned this Jul 14, 2025

arkirchner commented Jul 14, 2025

View reviewed changes

arkirchner added the enhancement label Jul 14, 2025

arkirchner changed the base branch from main to ak/test_comment_policy July 14, 2025 14:59

arkirchner force-pushed the ak/report_comments branch from 0836383 to 1136443 Compare July 14, 2025 15:00

arkirchner commented Jul 14, 2025

View reviewed changes

arkirchner marked this pull request as ready for review July 14, 2025 15:04

arkirchner requested review from MrSerth and kkoehn July 15, 2025 07:52

kkoehn requested changes Jul 16, 2025

View reviewed changes

Base automatically changed from ak/test_comment_policy to main July 17, 2025 13:35

arkirchner and others added 6 commits July 17, 2025 15:41

feat: Allow users to report comments

ee65b1f

Comments can be used to potentially harass users who request comments. A button was added to report a message as spam. Resolves #2715

Update config/locales/de/comment.yml

119b15c

Co-authored-by: kkoehn <[email protected]>

Update config/locales/en/comment.yml

5c17c73

Co-authored-by: kkoehn <[email protected]>

Report button next to edit/delete buttons

63d4882

Fix comment grammar to align with other specs

702d7f8

Rename SpamReport to UserContentReport

def5481

Not only Spam will be reported. Any user content can be reported. Mailer name was adjusted to fit the naming convention.

arkirchner force-pushed the ak/report_comments branch from 3f74f8f to def5481 Compare July 17, 2025 14:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: Allow users to report comments #3016

feat: Allow users to report comments #3016

Uh oh!

arkirchner commented Jul 14, 2025

Uh oh!

arkirchner Jul 14, 2025

Uh oh!

codecov bot commented Jul 14, 2025 •

edited

Loading

Uh oh!

arkirchner Jul 14, 2025

Uh oh!

arkirchner Jul 14, 2025

Uh oh!

arkirchner Jul 14, 2025

Uh oh!

kkoehn left a comment

Uh oh!

Uh oh!

Uh oh!

kkoehn Jul 16, 2025

Uh oh!

kkoehn Jul 16, 2025

Uh oh!

arkirchner Jul 17, 2025

Uh oh!

arkirchner commented Jul 16, 2025

Uh oh!

arkirchner commented Jul 17, 2025

Uh oh!

Uh oh!

		@@ -0,0 +1,51 @@
		# frozen_string_literal: true

		class SpamReport

feat: Allow users to report comments #3016

Are you sure you want to change the base?

feat: Allow users to report comments #3016

Uh oh!

Conversation

arkirchner commented Jul 14, 2025

Uh oh!

arkirchner Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

arkirchner Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

arkirchner Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

arkirchner Jul 14, 2025

Choose a reason for hiding this comment

Uh oh!

kkoehn left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kkoehn Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

kkoehn Jul 16, 2025

Choose a reason for hiding this comment

Uh oh!

arkirchner Jul 17, 2025

Choose a reason for hiding this comment

Uh oh!

arkirchner commented Jul 16, 2025

Uh oh!

arkirchner commented Jul 17, 2025

Uh oh!

Uh oh!

codecov bot commented Jul 14, 2025 •

edited

Loading