From 8dcc2f9f5ecbbc81bc026c85582695de4fbc1a0f Mon Sep 17 00:00:00 2001 From: Maxim Filippov Date: Fri, 4 Oct 2019 19:00:58 +0300 Subject: [PATCH] Admin API: Allow changing the state of multiple reports at once --- CHANGELOG.md | 1 + lib/pleroma/web/activity_pub/utils.ex | 12 +++ .../web/admin_api/admin_api_controller.ex | 29 +++--- lib/pleroma/web/common_api/common_api.ex | 7 ++ lib/pleroma/web/router.ex | 2 +- .../admin_api/admin_api_controller_test.exs | 89 +++++++++++++++---- test/web/common_api/common_api_test.exs | 29 ++++++ 7 files changed, 142 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a71a9dae6..d7afed783 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - **Breaking:** Elixir >=1.8 is now required (was >= 1.7) - **Breaking:** Admin API: Return link alongside with token on password reset +- **Breaking:** Admin API: Changing report state now uses `PATCH` (it was `PUT` before) and allows updating multiple reports at once (API changed) - Replaced [pleroma_job_queue](https://git.pleroma.social/pleroma/pleroma_job_queue) and `Pleroma.Web.Federator.RetryQueue` with [Oban](https://github.com/sorentwo/oban) (see [`docs/config.md`](docs/config.md) on migrating customized worker / retry settings) - Introduced [quantum](https://github.com/quantum-elixir/quantum-core) job scheduler - Admin API: Return `total` when querying for reports diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index 0828591ee..824957314 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -672,6 +672,18 @@ def update_report_state(%Activity{} = activity, state) when state in @supported_ |> Repo.update() end + def update_report_state(activity_ids, state) when state in @supported_report_states do + activities_num = length(activity_ids) + + from(a in Activity, where: a.id in ^activity_ids) + |> update(set: [data: fragment("jsonb_set(data, '{state}', ?)", ^state)]) + |> Repo.update_all([]) + |> case do + {^activities_num, _} -> :ok + _ -> {:error, activity_ids} + end + end + def update_report_state(_, _), do: {:error, "Unsupported state"} def update_activity_visibility(activity, visibility) when visibility in @valid_visibilities do diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex index 21da8a7ff..0e8c9dac8 100644 --- a/lib/pleroma/web/admin_api/admin_api_controller.ex +++ b/lib/pleroma/web/admin_api/admin_api_controller.ex @@ -480,17 +480,26 @@ def report_show(conn, %{"id" => id}) do end end - def report_update_state(%{assigns: %{user: admin}} = conn, %{"id" => id, "state" => state}) do - with {:ok, report} <- CommonAPI.update_report_state(id, state) do - ModerationLog.insert_log(%{ - action: "report_update", - actor: admin, - subject: report - }) + def reports_update(%{assigns: %{user: admin}} = conn, %{"reports" => reports}) do + result = + reports + |> Enum.map(fn report -> + with {:ok, activity} <- CommonAPI.update_report_state(report["id"], report["state"]) do + ModerationLog.insert_log(%{ + action: "report_update", + actor: admin, + subject: activity + }) - conn - |> put_view(ReportView) - |> render("show.json", Report.extract_report_info(report)) + activity + else + {:error, message} -> %{id: report["id"], error: message} + end + end) + + case Enum.any?(result, &Map.has_key?(&1, :error)) do + true -> json_response(conn, :bad_request, result) + false -> json_response(conn, :no_content, "") end end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index ce73b3270..2b80598ea 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -346,6 +346,13 @@ defp get_reported_account(account_id) do end end + def update_report_state(activity_ids, state) when is_list(activity_ids) do + case Utils.update_report_state(activity_ids, state) do + :ok -> {:ok, activity_ids} + _ -> {:error, dgettext("errors", "Could not update state")} + end + end + def update_report_state(activity_id, state) do with %Activity{} = activity <- Activity.get_by_id(activity_id) do Utils.update_report_state(activity, state) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index f91af8137..563b01dc5 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -194,7 +194,7 @@ defmodule Pleroma.Web.Router do get("/reports", AdminAPIController, :list_reports) get("/reports/:id", AdminAPIController, :report_show) - put("/reports/:id", AdminAPIController, :report_update_state) + patch("/reports", AdminAPIController, :reports_update) post("/reports/:id/respond", AdminAPIController, :report_respond) put("/statuses/:id", AdminAPIController, :status_update) diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index b5c355e66..cec3570eb 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -1224,7 +1224,7 @@ test "returns 404 when report id is invalid", %{conn: conn} do end end - describe "PUT /api/pleroma/admin/reports/:id" do + describe "PATCH /api/pleroma/admin/reports" do setup %{conn: conn} do admin = insert(:user, info: %{is_admin: true}) [reporter, target_user] = insert_pair(:user) @@ -1237,16 +1237,32 @@ test "returns 404 when report id is invalid", %{conn: conn} do "status_ids" => [activity.id] }) - %{conn: assign(conn, :user, admin), id: report_id, admin: admin} + {:ok, %{id: second_report_id}} = + CommonAPI.report(reporter, %{ + "account_id" => target_user.id, + "comment" => "I feel very offended", + "status_ids" => [activity.id] + }) + + %{ + conn: assign(conn, :user, admin), + id: report_id, + admin: admin, + second_report_id: second_report_id + } end test "mark report as resolved", %{conn: conn, id: id, admin: admin} do - response = - conn - |> put("/api/pleroma/admin/reports/#{id}", %{"state" => "resolved"}) - |> json_response(:ok) + conn + |> patch("/api/pleroma/admin/reports", %{ + "reports" => [ + %{"state" => "resolved", "id" => id} + ] + }) + |> json_response(:no_content) - assert response["state"] == "resolved" + activity = Activity.get_by_id(id) + assert activity.data["state"] == "resolved" log_entry = Repo.one(ModerationLog) @@ -1255,12 +1271,16 @@ test "mark report as resolved", %{conn: conn, id: id, admin: admin} do end test "closes report", %{conn: conn, id: id, admin: admin} do - response = - conn - |> put("/api/pleroma/admin/reports/#{id}", %{"state" => "closed"}) - |> json_response(:ok) + conn + |> patch("/api/pleroma/admin/reports", %{ + "reports" => [ + %{"state" => "closed", "id" => id} + ] + }) + |> json_response(:no_content) - assert response["state"] == "closed" + activity = Activity.get_by_id(id) + assert activity.data["state"] == "closed" log_entry = Repo.one(ModerationLog) @@ -1271,17 +1291,54 @@ test "closes report", %{conn: conn, id: id, admin: admin} do test "returns 400 when state is unknown", %{conn: conn, id: id} do conn = conn - |> put("/api/pleroma/admin/reports/#{id}", %{"state" => "test"}) + |> patch("/api/pleroma/admin/reports", %{ + "reports" => [ + %{"state" => "test", "id" => id} + ] + }) - assert json_response(conn, :bad_request) == "Unsupported state" + assert hd(json_response(conn, :bad_request))["error"] == "Unsupported state" end test "returns 404 when report is not exist", %{conn: conn} do conn = conn - |> put("/api/pleroma/admin/reports/test", %{"state" => "closed"}) + |> patch("/api/pleroma/admin/reports", %{ + "reports" => [ + %{"state" => "closed", "id" => "test"} + ] + }) - assert json_response(conn, :not_found) == "Not found" + assert hd(json_response(conn, :bad_request))["error"] == "not_found" + end + + test "updates state of multiple reports", %{ + conn: conn, + id: id, + admin: admin, + second_report_id: second_report_id + } do + conn + |> patch("/api/pleroma/admin/reports", %{ + "reports" => [ + %{"state" => "resolved", "id" => id}, + %{"state" => "closed", "id" => second_report_id} + ] + }) + |> json_response(:no_content) + + activity = Activity.get_by_id(id) + second_activity = Activity.get_by_id(second_report_id) + assert activity.data["state"] == "resolved" + assert second_activity.data["state"] == "closed" + + [first_log_entry, second_log_entry] = Repo.all(ModerationLog) + + assert ModerationLog.get_log_entry_message(first_log_entry) == + "@#{admin.nickname} updated report ##{id} with 'resolved' state" + + assert ModerationLog.get_log_entry_message(second_log_entry) == + "@#{admin.nickname} updated report ##{second_report_id} with 'closed' state" end end diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 2d3c41e82..c57fdb6af 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -423,6 +423,35 @@ test "does not update report state when state is unsupported" do assert CommonAPI.update_report_state(report_id, "test") == {:error, "Unsupported state"} end + + test "updates state of multiple reports" do + [reporter, target_user] = insert_pair(:user) + activity = insert(:note_activity, user: target_user) + + {:ok, %Activity{id: first_report_id}} = + CommonAPI.report(reporter, %{ + "account_id" => target_user.id, + "comment" => "I feel offended", + "status_ids" => [activity.id] + }) + + {:ok, %Activity{id: second_report_id}} = + CommonAPI.report(reporter, %{ + "account_id" => target_user.id, + "comment" => "I feel very offended!", + "status_ids" => [activity.id] + }) + + {:ok, report_ids} = + CommonAPI.update_report_state([first_report_id, second_report_id], "resolved") + + first_report = Activity.get_by_id(first_report_id) + second_report = Activity.get_by_id(second_report_id) + + assert report_ids -- [first_report_id, second_report_id] == [] + assert first_report.data["state"] == "resolved" + assert second_report.data["state"] == "resolved" + end end describe "reblog muting" do