diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index be54549c63..90faf32524 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -53,14 +53,15 @@ def akismet_attributes scope :by_date, -> { order('created_at DESC') } - # Standardize the format of work, chapter, and profile URLs to get it ready - # for the url_is_not_over_reported validation. + # Standardize the format of work, chapter, comment, and profile URLs + # to get it ready for the url_is_not_over_reported validation. # Work URLs: "works/123" # Chapter URLs: "chapters/123" + # Comment URLs: "comments/123" # Profile URLs: "users/username" before_validation :standardize_url, on: :create def standardize_url - return unless url =~ %r{((chapters|works)/\d+)} || url =~ %r{(users\/\w+)} + return unless url =~ %r{((chapters|works|comments)/\d+)} || url =~ %r{(users/\w+)} self.url = add_scheme_to_url(url) self.url = clean_url(url) @@ -208,35 +209,36 @@ def attach_work_download(ticket_id) ReportAttachmentJob.perform_later(ticket_id, work) if work end - # if the URL clearly belongs to a work (i.e. contains "/works/123") - # or a user profile (i.e. contains "/users/username") - # make sure it isn't reported more than ABUSE_REPORTS_PER_WORK_MAX - # or ABUSE_REPORTS_PER_USER_MAX times per month + # if the URL clearly belongs to a specific piece of content + # make sure it isn't reported more than ABUSE_REPORTS_PER__MAX + # times within the configured time period def url_is_not_over_reported - message = ts('This page has already been reported. Our volunteers only - need one report in order to investigate and resolve an issue, - so please be patient and do not submit another report.') - if url =~ /\/works\/\d+/ + case url + when %r{/comments/\d+} + comment_params_only = url.match(%r{/comments/\d+/}).to_s + comment_report_period = ArchiveConfig.ABUSE_REPORTS_PER_COMMENT_PERIOD.days.ago + existing_reports_total = AbuseReport.where('created_at > ? AND + url LIKE ?', + comment_report_period, + "%#{comment_params_only}%").count + errors.add(:base, :over_reported_comment) if existing_reports_total >= ArchiveConfig.ABUSE_REPORTS_PER_COMMENT_MAX + when %r{/works/\d+} # use "/works/123/" to avoid matching chapter or external work ids - work_params_only = url.match(/\/works\/\d+\//).to_s + work_params_only = url.match(%r{/works/\d+/}).to_s work_report_period = ArchiveConfig.ABUSE_REPORTS_PER_WORK_PERIOD.days.ago existing_reports_total = AbuseReport.where('created_at > ? AND url LIKE ?', work_report_period, "%#{work_params_only}%").count - if existing_reports_total >= ArchiveConfig.ABUSE_REPORTS_PER_WORK_MAX - errors.add(:base, message) - end - elsif url =~ /\/users\/\w+/ - user_params_only = url.match(/\/users\/\w+\//).to_s + errors.add(:base, :over_reported) if existing_reports_total >= ArchiveConfig.ABUSE_REPORTS_PER_WORK_MAX + when %r{/users/\w+} + user_params_only = url.match(%r{/users/\w+/}).to_s user_report_period = ArchiveConfig.ABUSE_REPORTS_PER_USER_PERIOD.days.ago existing_reports_total = AbuseReport.where('created_at > ? AND url LIKE ?', user_report_period, "%#{user_params_only}%").count - if existing_reports_total >= ArchiveConfig.ABUSE_REPORTS_PER_USER_MAX - errors.add(:base, message) - end + errors.add(:base, :over_reported) if existing_reports_total >= ArchiveConfig.ABUSE_REPORTS_PER_USER_MAX end end diff --git a/config/config.yml b/config/config.yml index 2c04187451..f44c877682 100644 --- a/config/config.yml +++ b/config/config.yml @@ -173,6 +173,10 @@ ABUSE_REPORTS_PER_USER_PERIOD: 31 ABUSE_REPORTS_PER_WORK_MAX: 5 # time period, in days, used to total number of reports for a given work URL ABUSE_REPORTS_PER_WORK_PERIOD: 31 +# max number of abuse reports to accept for a given comment URL +ABUSE_REPORTS_PER_COMMENT_MAX: 3 +# time period, in days, used to total number of reports for a given comment URL +ABUSE_REPORTS_PER_COMMENT_PERIOD: 30 # max number of abuse reports to accept from a given email ABUSE_REPORTS_PER_EMAIL_MAX: 5 diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 834f877264..e6f87c57e5 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -86,6 +86,9 @@ en: models: abuse_report: attributes: + base: + over_reported: This page has already been reported. Our volunteers only need one report in order to investigate and resolve an issue, so please be patient and do not submit another report. + over_reported_comment: This comment has already been reported. Our volunteers only need one report in order to investigate and resolve an issue, so please be patient and do not submit another report. url: not_on_archive: does not appear to be on this site. admin_post: diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 79db070062..a029a4e938 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -145,6 +145,14 @@ end end + shared_examples "enough comment reports" do |url| + let(:report) { build(:abuse_report, url: url) } + it "can't be submitted" do + expect(report.save).to be_falsey + expect(report.errors[:base].first).to include("This comment has already been reported.") + end + end + shared_examples "alright" do |url| let(:report) { build(:abuse_report, url: url) } it "can be submitted" do @@ -195,6 +203,9 @@ it_behaves_like "enough already", "http://archiveofourown.org/works/789/chapters/123?ending=1" it_behaves_like "enough already", "http://archiveofourown.org/works/789/chapters/123?ending=2#major-character-death" + # a comment on the work + it_behaves_like "alright", "http://archiveofourown.org/works/789/comments/876" + # the same work: variations we don't cover it_behaves_like "alright", "http://archiveofourown.org/chapters/123" it_behaves_like "alright", "http://archiveofourown.org/comments/show_comments?work_id=789" @@ -215,6 +226,59 @@ end end + context "for a comment reported the maximum number of times" do + comment_url = "http://archiveofourown.org/comments/876" + + before do + ArchiveConfig.ABUSE_REPORTS_PER_COMMENT_MAX.times do + create(:abuse_report, url: comment_url) + end + expect(AbuseReport.count).to eq(ArchiveConfig.ABUSE_REPORTS_PER_COMMENT_MAX) + end + + # obviously + it_behaves_like "enough comment reports", comment_url + + # the same comment, different protocol + it_behaves_like "enough comment reports", "https://archiveofourown.org/comments/876" + + # the same comment, with parameters/anchors + it_behaves_like "enough comment reports", "http://archiveofourown.org/comments/876?smut=yes" + it_behaves_like "enough comment reports", "http://archiveofourown.org/comments/876#timeline" + it_behaves_like "enough comment reports", "http://archiveofourown.org/comments/876?smut=yes#timeline" + it_behaves_like "enough comment reports", "http://archiveofourown.org/comments/876/?smut=yes" + it_behaves_like "enough comment reports", "http://archiveofourown.org/comments/876/#timeline" + it_behaves_like "enough comment reports", "http://archiveofourown.org/comments/876/?smut=yes#timeline" + + # the same comment, under users + it_behaves_like "enough comment reports", "http://archiveofourown.org/users/author/comments/876" + + # the same comment, under works + it_behaves_like "enough comment reports", "http://archiveofourown.org/works/789/comments/876" + it_behaves_like "enough comment reports", "http://archiveofourown.org/works/789/chapters/123/comments/876" + + # the same comment, under chapters + it_behaves_like "enough comment reports", "http://archiveofourown.org/chapters/123/comments/876" + + # the same comment: variations we don't cover + it_behaves_like "alright", "https://archiveofourown.org/comments/add_comment_reply?chapter_id=123&id=876" + + # not the same comment + it_behaves_like "alright", "http://archiveofourown.org/comments/9009" + it_behaves_like "alright", "http://archiveofourown.org/comments/87" + + # unrelated + it_behaves_like "alright", "http://archiveofourown.org/works/876" + it_behaves_like "alright", "http://archiveofourown.org/works/876/comments" + it_behaves_like "alright", "http://archiveofourown.org/users/someone" + + context "after the over-reporting period" do + before { travel(ArchiveConfig.ABUSE_REPORTS_PER_COMMENT_PERIOD.days) } + + it_behaves_like "alright", comment_url + end + end + context "when reporting work URLs that cross the reporting period timeframe" do work_url = "http://archiveofourown.org/works/790"