From 9466280afa8d559b48420018496644157d4bffe2 Mon Sep 17 00:00:00 2001 From: Bilka Date: Sat, 1 Mar 2025 18:00:43 +0100 Subject: [PATCH 1/2] AO3-6830 Add creator IDs to abuse reports --- app/models/abuse_report.rb | 35 +++- .../feedback_reporters/abuse_reporter.rb | 5 +- spec/models/abuse_report_spec.rb | 150 +++++++++++++++++- .../feedback_reporters/abuse_reporter_spec.rb | 14 +- 4 files changed, 192 insertions(+), 12 deletions(-) diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index a8aebe151ed..c4b730dd25d 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -106,7 +106,8 @@ def email_and_send end def send_report - return unless %w(staging production).include?(Rails.env) + return unless zoho_enabled? + reporter = AbuseReporter.new( title: summary, description: comment, @@ -114,7 +115,8 @@ def send_report email: email, username: username, ip_address: ip_address, - url: url + url: url, + creator_ids: creator_ids ) response = reporter.send_report! ticket_id = response["id"] @@ -123,10 +125,27 @@ def send_report attach_work_download(ticket_id) end + def creator_ids + work_id = reported_work_id + return unless work_id + + work = Work.find_by(id: work_id) + return "deletedwork" unless work + + ids = work.pseuds.pluck(:user_id).push(*work.original_creators.pluck(:user_id)).uniq.sort! + ids.prepend("orphanedwork") if ids.delete(User.orphan_account.id) + ids.join(", ") + end + + # ID of the reported work, unless the report is about comment(s) on the work + def reported_work_id + comments = url[%r{/comments/}, 0] + url[%r{/works/(\d+)}, 1] if comments.nil? + end + def attach_work_download(ticket_id) - is_not_comments = url[%r{/comments/}, 0].nil? - work_id = url[%r{/works/(\d+)}, 1] - return unless work_id && is_not_comments + work_id = reported_work_id + return unless work_id work = Work.find_by(id: work_id) ReportAttachmentJob.perform_later(ticket_id, work) if work @@ -174,4 +193,10 @@ def email_is_not_over_reporting out violations to report, but only report violations you encounter during your normal browsing.")) end + + private + + def zoho_enabled? + %w[staging production].include?(Rails.env) + end end diff --git a/app/models/feedback_reporters/abuse_reporter.rb b/app/models/feedback_reporters/abuse_reporter.rb index 5a03b7e1bbe..936b278239c 100644 --- a/app/models/feedback_reporters/abuse_reporter.rb +++ b/app/models/feedback_reporters/abuse_reporter.rb @@ -1,4 +1,6 @@ class AbuseReporter < FeedbackReporter + attr_accessor :creator_ids + def report_attributes super.deep_merge( "departmentId" => department_id, @@ -13,7 +15,8 @@ def report_attributes def custom_zoho_fields { "cf_ip" => ip_address.presence || "Unknown IP", - "cf_url" => url.presence || "Unknown URL" + "cf_url" => url.presence || "Unknown URL", + "cf_user_id" => creator_ids.presence || "" } end diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 941844a3c6d..a9ae3dacdf8 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -1,4 +1,4 @@ -require 'spec_helper' +require "spec_helper" describe AbuseReport do context "when report is not spam" do @@ -9,7 +9,7 @@ end context "comment missing" do - let(:report_without_comment) {build(:abuse_report, comment: nil)} + let(:report_without_comment) { build(:abuse_report, comment: nil) } it "is invalid" do expect(report_without_comment.save).to be_falsey expect(report_without_comment.errors[:comment]).not_to be_empty @@ -328,8 +328,8 @@ context "when report is spam" do let(:legit_user) { create(:user) } - let(:spam_report) { build(:abuse_report, username: 'viagra-test-123') } - let!(:safe_report) { build(:abuse_report, username: 'viagra-test-123', email: legit_user.email) } + let(:spam_report) { build(:abuse_report, username: "viagra-test-123") } + let!(:safe_report) { build(:abuse_report, username: "viagra-test-123", email: legit_user.email) } before do allow(Akismetor).to receive(:spam?).and_return(true) @@ -380,4 +380,146 @@ .to have_enqueued_job end end + + describe "#creator_ids" do + it "returns no creator ids for non-work URLs" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/users/someone/") + + expect(subject.creator_ids).to be_nil + end + + it "returns no creator ids for comment sub-URLs" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/123/comments/") + + expect(subject.creator_ids).to be_nil + end + + context "for work URLs" do + it "returns deletedwork for a work that doesn't exist" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/000/") + + expect(subject.creator_ids).to eq("deletedwork") + end + + context "for a single creator" do + let(:work) { create(:work) } + + it "returns a single creator id" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq(work.users.first.id.to_s) + end + end + + context "for an anonymous work" do + let(:anonymous_collection) { create(:anonymous_collection) } + let(:work) { create(:work, collections: [anonymous_collection]) } + + it "returns a single creator id" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq(work.users.first.id.to_s) + end + end + + context "for an unrevealed work" do + let(:unrevealed_collection) { create(:unrevealed_collection) } + let(:work) { create(:work, collections: [unrevealed_collection]) } + + it "returns a single creator id" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq(work.users.first.id.to_s) + end + end + + context "for multiple pseuds of one creator" do + let(:user) { create(:user) } + let(:pseud) { create(:pseud, user: user) } + let(:work) { create(:work, authors: [pseud, user.default_pseud]) } + + it "returns a single creator id" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq(user.id.to_s) + end + end + + context "for multiple creators" do + let(:user1) { create(:user, id: 10) } + let(:user2) { create(:user, id: 11) } + let(:work) { create(:work, authors: [user2.default_pseud, user1.default_pseud]) } + + it "returns a sorted list of creator ids" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq("#{user1.id}, #{user2.id}") + end + end + + context "for an invited co-creator that hasn't accepted yet" do + let(:user) { create(:user) } + let(:invited) { create(:user) } + let(:work) { create(:work, authors: [user.default_pseud, invited.default_pseud]) } + let(:creatorship) { work.creatorships.last } + + before do + creatorship.approved = false + creatorship.save!(validate: false) + end + + it "returns only the creator" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq(user.id.to_s) + end + end + end + + context "for an orphaned work" do + let!(:orphan_account) { create(:user, login: "orphan_account") } + let(:orphaneer) { create(:user, id: 20) } + let(:work) { create(:work, authors: [orphaneer.default_pseud]) } + + context "recently orphaned" do + before do + Creatorship.orphan([orphaneer.default_pseud], [work], false) + end + + it "returns orphanedwork and the original creator" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq("orphanedwork, #{orphaneer.id}") + end + end + + context "orphaned a long time ago" do + before do + Creatorship.orphan([orphaneer.default_pseud], [work], false) + work.original_creators.destroy_all + end + + it "returns orphanedwork" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq("orphanedwork") + end + end + + context "partially orphaned" do + let(:cocreator) { create(:user, id: 21) } + let(:work) { create(:work, authors: [cocreator.default_pseud, orphaneer.default_pseud]) } + + before do + Creatorship.orphan([orphaneer.default_pseud], [work], false) + end + + it "returns a sorted list of orphanedwork, the co-creator and the original creator" do + allow(subject).to receive(:url).and_return("http://archiveofourown.org/works/#{work.id}/") + + expect(subject.creator_ids).to eq("orphanedwork, #{orphaneer.id}, #{cocreator.id}") + end + end + end + end end diff --git a/spec/models/feedback_reporters/abuse_reporter_spec.rb b/spec/models/feedback_reporters/abuse_reporter_spec.rb index d5459c30ac8..415c14b6747 100644 --- a/spec/models/feedback_reporters/abuse_reporter_spec.rb +++ b/spec/models/feedback_reporters/abuse_reporter_spec.rb @@ -13,7 +13,8 @@ email: "walrus@example.org", username: "Walrus", ip_address: "127.0.0.1", - url: "http://localhost" + url: "http://localhost", + creator_ids: "3, 4" } end @@ -28,7 +29,8 @@ "cf_language" => "English", "cf_name" => "Walrus", "cf_ip" => "127.0.0.1", - "cf_url" => "http://localhost" + "cf_url" => "http://localhost", + "cf_user_id" => "3, 4" } } end @@ -87,5 +89,13 @@ expect(subject.report_attributes.fetch("description")).to eq("Hi!http://example.com/Camera-icon.svgBye!") end end + + context "if the report does not have creator_ids" do + it "returns a hash containing a blank string for the user id" do + allow(subject).to receive(:creator_ids).and_return(nil) + + expect(subject.report_attributes.fetch("cf").fetch("cf_user_id")).to eq("") + end + end end end From 484863a24658b6be045045d7628b877712300ee4 Mon Sep 17 00:00:00 2001 From: Bilka Date: Mon, 3 Mar 2025 17:23:17 +0100 Subject: [PATCH 2/2] AO3-6830 Review --- app/models/abuse_report.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index c4b730dd25d..0146f291b58 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -132,7 +132,7 @@ def creator_ids work = Work.find_by(id: work_id) return "deletedwork" unless work - ids = work.pseuds.pluck(:user_id).push(*work.original_creators.pluck(:user_id)).uniq.sort! + ids = work.pseuds.pluck(:user_id).push(*work.original_creators.pluck(:user_id)).uniq.sort ids.prepend("orphanedwork") if ids.delete(User.orphan_account.id) ids.join(", ") end