Fix report category not being saved in REST API (#17682)

app/controllers/api/v1/reports_controller.rb

@@ -10,9 +10,7 @@ class Api::V1::ReportsController < Api::BaseController
     @report = ReportService.new.call(
       current_account,
       reported_account,
-      status_ids: reported_status_ids,
+      report_params
-      comment: report_params[:comment],
-      forward: report_params[:forward]
     )
 
     render json: @report, serializer: REST::ReportSerializer
@@ -20,14 +18,6 @@ class Api::V1::ReportsController < Api::BaseController
 
   private
 
-  def reported_status_ids
-    reported_account.statuses.with_discarded.find(status_ids).pluck(:id)
-  end
-
-  def status_ids
-    Array(report_params[:status_ids])
-  end
-
   def reported_account
     Account.find(report_params[:account_id])
   end

app/services/report_service.rb

@@ -6,10 +6,10 @@ class ReportService < BaseService
   def call(source_account, target_account, options = {})
     @source_account = source_account
     @target_account = target_account
-    @status_ids     = options.delete(:status_ids) || []
+    @status_ids     = options.delete(:status_ids).presence || []
-    @comment        = options.delete(:comment) || ''
+    @comment        = options.delete(:comment).presence || ''
-    @category       = options.delete(:category) || 'other'
+    @category       = options.delete(:category).presence || 'other'
-    @rule_ids       = options.delete(:rule_ids)
+    @rule_ids       = options.delete(:rule_ids).presence
     @options        = options
 
     raise ActiveRecord::RecordNotFound if @target_account.suspended?
@@ -26,7 +26,7 @@ class ReportService < BaseService
   def create_report!
     @report = @source_account.reports.create!(
       target_account: @target_account,
-      status_ids: @status_ids,
+      status_ids: reported_status_ids,
       comment: @comment,
       uri: @options[:uri],
       forwarded: forward?,
@@ -56,6 +56,10 @@ class ReportService < BaseService
     !@target_account.local? && ActiveModel::Type::Boolean.new.cast(@options[:forward])
   end
 
+  def reported_status_ids
+    @target_account.statuses.with_discarded.find(Array(@status_ids)).pluck(:id)
+  end
+
   def payload
     Oj.dump(serialize_payload(@report, ActivityPub::FlagSerializer, account: some_local_account))
   end

spec/controllers/api/v1/reports_controller_spec.rb

@@ -13,22 +13,64 @@ RSpec.describe Api::V1::ReportsController, type: :controller do
   end
 
   describe 'POST #create' do
-    let(:scopes)  { 'write:reports' }
+    let!(:admin) { Fabricate(:user, admin: true) }
-    let!(:status) { Fabricate(:status) }
+
-    let!(:admin)  { Fabricate(:user, admin: true) }
+    let(:scopes) { 'write:reports' }
+    let(:status) { Fabricate(:status) }
+    let(:target_account) { status.account }
+    let(:category) { nil }
+    let(:forward) { nil }
+    let(:rule_ids){ nil }
 
     before do
       allow(AdminMailer).to receive(:new_report).and_return(double('email', deliver_later: nil))
-      post :create, params: { status_ids: [status.id], account_id: status.account.id, comment: 'reasons' }
+      post :create, params: { status_ids: [status.id], account_id: target_account.id, comment: 'reasons', category: category, rule_ids: rule_ids, forward: forward }
+    end
+
+    it 'returns http success' do
+      expect(response).to have_http_status(200)
     end
 
     it 'creates a report' do
-      expect(status.reload.account.targeted_reports).not_to be_empty
+      expect(target_account.targeted_reports).to_not be_empty
-      expect(response).to have_http_status(200)
+    end
+
+    it 'saves comment' do
+      expect(target_account.targeted_reports.first.comment).to eq 'reasons'
     end
 
     it 'sends e-mails to admins' do
       expect(AdminMailer).to have_received(:new_report).with(admin.account, Report)
     end
+
+    context 'when a status does not belong to the reported account' do
+      let(:target_account) { Fabricate(:account) }
+
+      it 'returns http not found' do
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    context 'when a category is chosen' do
+      let(:category) { 'spam' }
+
+      it 'saves category' do
+        expect(target_account.targeted_reports.first.spam?).to be true
+      end
+    end
+
+    context 'when violated rules are chosen' do
+      let(:rule) { Fabricate(:rule) }
+      let(:category) { 'violation' }
+      let(:rule_ids) { [rule.id] }
+
+      it 'saves category' do
+        expect(target_account.targeted_reports.first.violation?).to be true
+      end
+
+      it 'saves rule_ids' do
+        expect(target_account.targeted_reports.first.rule_ids).to match_array([rule.id])
+      end
+    end
   end
 end

spec/fabricators/rule_fabricator.rb

@@ -1,5 +1,5 @@
 Fabricator(:rule) do
-  priority   ""
+  priority   0
-  deleted_at "2021-02-21 05:51:09"
+  deleted_at nil
-  text       "MyText"
+  text       { Faker::Lorem.paragraph }
 end