Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AO3-6933 Add spam checking to user comments #5099

Merged
merged 14 commits into from
Mar 26, 2025

Conversation

EchoEkhi
Copy link
Contributor

@EchoEkhi EchoEkhi commented Mar 13, 2025

Pull Request Checklist

Issue

https://otwarchive.atlassian.net/browse/AO3-6933

Purpose

  • Adds a new admin setting called account_age_threshold_for_comment_spam_check (self-explanatory)
  • When an account is too new, its comments are spam-checked
  • When an account gets spam-banned by an admin, all of its comments are sent to Akismet for training

Testing Instructions

See JIRA ticket

Credit

EchoEkhi

@github-actions github-actions bot added Has Migrations Contains migrations and therefore needs special attention when deploying Awaiting Review labels Mar 13, 2025
@@ -65,7 +65,7 @@ def by_anonymous_creator?
validate :check_for_spam, on: :create

def check_for_spam
errors.add(:base, ts("This comment looks like spam to our system, sorry! Please try again, or create an account to comment.")) unless check_for_spam?
errors.add(:base, ts("This comment looks like spam to our system, sorry! Please try again.")) unless check_for_spam?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll leave the i18n to a later ticket

@EchoEkhi EchoEkhi marked this pull request as ready for review March 14, 2025 02:27
@EchoEkhi EchoEkhi requested a review from sarken March 16, 2025 02:45
@EchoEkhi EchoEkhi requested a review from sarken March 17, 2025 14:18
Copy link
Collaborator

@sarken sarken left a comment

Choose a reason for hiding this comment

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

The removal of the role attribute looks good, thank you! I think that just leaves Bilka's comment.

@EchoEkhi EchoEkhi requested a review from Bilka2 March 18, 2025 03:04
Copy link
Contributor

@Bilka2 Bilka2 left a comment

Choose a reason for hiding this comment

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

Thank you for working on this! In addition to addressing my comments, please also update the PUT #update test in settings_controller_spec.rb for the new setting.

As a sidenote, you can use comment.user instead of comment.pseud.user if you prefer; user is delegated to pseud.

Copy link
Contributor

@Bilka2 Bilka2 left a comment

Choose a reason for hiding this comment

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

Thank you!

@brianjaustin brianjaustin merged commit 4e7b35f into otwcode:master Mar 26, 2025
47 of 49 checks passed
Copy link

sentry-io bot commented Mar 27, 2025

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ ArgumentError: comparison of Integer with nil failed (ArgumentError) CommentsController#create View Issue

Did you find this useful? React with a 👍 or 👎

@brianjaustin
Copy link
Member

Looks like the setting got cached with the wrong (old) value on staging. I will make a note that the migration needs to be applied before the production deploy to prevent this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Migrations Contains migrations and therefore needs special attention when deploying Priority: High Reviewed: Ready to Merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants