-
Notifications
You must be signed in to change notification settings - Fork 543
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
Conversation
@@ -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? |
There was a problem hiding this comment.
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
There was a problem hiding this 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.
There was a problem hiding this 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
.
db/migrate/20250312233933_add_account_age_threshold_for_comment_spam_check_to_admin_settings.rb
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you!
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
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 |
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing
)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-6933
Purpose
Testing Instructions
See JIRA ticket
Credit
EchoEkhi