From 560f2c1979ca4d49f18abd6de6aac49875bfc771 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 28 Apr 2020 16:50:37 +0400 Subject: [PATCH] Add OpenAPI spec for ReportController --- .../api_spec/operations/report_operation.ex | 78 +++++++++++++++++++ lib/pleroma/web/common_api/common_api.ex | 10 +-- lib/pleroma/web/common_api/utils.ex | 3 +- .../controllers/report_controller.ex | 6 +- .../admin_api/admin_api_controller_test.exs | 40 +++++----- test/web/admin_api/views/report_view_test.exs | 18 ++--- test/web/common_api/common_api_test.exs | 30 +++---- .../controllers/report_controller_test.exs | 24 ++++-- 8 files changed, 148 insertions(+), 61 deletions(-) create mode 100644 lib/pleroma/web/api_spec/operations/report_operation.ex diff --git a/lib/pleroma/web/api_spec/operations/report_operation.ex b/lib/pleroma/web/api_spec/operations/report_operation.ex new file mode 100644 index 000000000..da4d50703 --- /dev/null +++ b/lib/pleroma/web/api_spec/operations/report_operation.ex @@ -0,0 +1,78 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.ReportOperation do + alias OpenApiSpex.Operation + alias OpenApiSpex.Schema + alias Pleroma.Web.ApiSpec.Helpers + alias Pleroma.Web.ApiSpec.Schemas.ApiError + + def open_api_operation(action) do + operation = String.to_existing_atom("#{action}_operation") + apply(__MODULE__, operation, []) + end + + def create_operation do + %Operation{ + tags: ["reports"], + summary: "File a report", + description: "Report problematic users to your moderators", + operationId: "ReportController.create", + security: [%{"oAuth" => ["follow", "write:reports"]}], + requestBody: Helpers.request_body("Parameters", create_request(), required: true), + responses: %{ + 200 => Operation.response("Report", "application/json", create_response()), + 400 => Operation.response("Report", "application/json", ApiError) + } + } + end + + defp create_request do + %Schema{ + title: "ReportCreateRequest", + description: "POST body for creating a report", + type: :object, + properties: %{ + account_id: %Schema{type: :string, description: "ID of the account to report"}, + status_ids: %Schema{ + type: :array, + items: %Schema{type: :string}, + description: "Array of Statuses to attach to the report, for context" + }, + comment: %Schema{ + type: :string, + description: "Reason for the report" + }, + forward: %Schema{ + type: :boolean, + default: false, + description: + "If the account is remote, should the report be forwarded to the remote admin?" + } + }, + required: [:account_id], + example: %{ + "account_id" => "123", + "status_ids" => ["1337"], + "comment" => "bad status!", + "forward" => "false" + } + } + end + + defp create_response do + %Schema{ + title: "ReportResponse", + type: :object, + properties: %{ + id: %Schema{type: :string, description: "Report ID"}, + action_taken: %Schema{type: :boolean, description: "Is action taken?"} + }, + example: %{ + "id" => "123", + "action_taken" => false + } + } + end +end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index d1efe0c36..53ce7d425 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -380,9 +380,9 @@ def thread_muted?(user, activity) do ThreadMute.exists?(user.id, activity.data["context"]) end - def report(user, %{"account_id" => account_id} = data) do - with {:ok, account} <- get_reported_account(account_id), - {:ok, {content_html, _, _}} <- make_report_content_html(data["comment"]), + def report(user, data) do + with {:ok, account} <- get_reported_account(data.account_id), + {:ok, {content_html, _, _}} <- make_report_content_html(data[:comment]), {:ok, statuses} <- get_report_statuses(account, data) do ActivityPub.flag(%{ context: Utils.generate_context_id(), @@ -390,13 +390,11 @@ def report(user, %{"account_id" => account_id} = data) do account: account, statuses: statuses, content: content_html, - forward: data["forward"] || false + forward: Map.get(data, :forward, false) }) end end - def report(_user, _params), do: {:error, dgettext("errors", "Valid `account_id` required")} - defp get_reported_account(account_id) do case User.get_cached_by_id(account_id) do %User{} = account -> {:ok, account} diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 945e63e22..6540fa5d1 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -504,7 +504,8 @@ def make_report_content_html(comment) do end end - def get_report_statuses(%User{ap_id: actor}, %{"status_ids" => status_ids}) do + def get_report_statuses(%User{ap_id: actor}, %{status_ids: status_ids}) + when is_list(status_ids) do {:ok, Activity.all_by_actor_and_id(actor, status_ids)} end diff --git a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex index f5782be13..85bd52106 100644 --- a/lib/pleroma/web/mastodon_api/controllers/report_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/report_controller.ex @@ -9,12 +9,14 @@ defmodule Pleroma.Web.MastodonAPI.ReportController do action_fallback(Pleroma.Web.MastodonAPI.FallbackController) + plug(OpenApiSpex.Plug.CastAndValidate, render_error: Pleroma.Web.ApiSpec.RenderError) plug(OAuthScopesPlug, %{scopes: ["write:reports"]} when action == :create) - plug(Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug) + defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.ReportOperation + @doc "POST /api/v1/reports" - def create(%{assigns: %{user: user}} = conn, params) do + def create(%{assigns: %{user: user}, body_params: params} = conn, _) do with {:ok, activity} <- Pleroma.Web.CommonAPI.report(user, params) do render(conn, "show.json", activity: activity) end diff --git a/test/web/admin_api/admin_api_controller_test.exs b/test/web/admin_api/admin_api_controller_test.exs index f80dbf8dd..1862a9589 100644 --- a/test/web/admin_api/admin_api_controller_test.exs +++ b/test/web/admin_api/admin_api_controller_test.exs @@ -1347,9 +1347,9 @@ test "returns report by its id", %{conn: conn} do {:ok, %{id: report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) response = @@ -1374,16 +1374,16 @@ test "returns 404 when report id is invalid", %{conn: conn} do {:ok, %{id: report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) {:ok, %{id: second_report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel very offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel very offended", + status_ids: [activity.id] }) %{ @@ -1523,9 +1523,9 @@ test "returns reports", %{conn: conn} do {:ok, %{id: report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) response = @@ -1547,15 +1547,15 @@ test "returns reports with specified state", %{conn: conn} do {:ok, %{id: first_report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) {:ok, %{id: second_report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I don't like this user" + account_id: target_user.id, + comment: "I don't like this user" }) CommonAPI.update_report_state(second_report_id, "closed") @@ -3431,9 +3431,9 @@ test "it resend emails for two users", %{conn: conn, admin: admin} do {:ok, %{id: report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) post(conn, "/api/pleroma/admin/reports/#{report_id}/notes", %{ diff --git a/test/web/admin_api/views/report_view_test.exs b/test/web/admin_api/views/report_view_test.exs index 5db6629f2..8cfa1dcfa 100644 --- a/test/web/admin_api/views/report_view_test.exs +++ b/test/web/admin_api/views/report_view_test.exs @@ -15,7 +15,7 @@ test "renders a report" do user = insert(:user) other_user = insert(:user) - {:ok, activity} = CommonAPI.report(user, %{"account_id" => other_user.id}) + {:ok, activity} = CommonAPI.report(user, %{account_id: other_user.id}) expected = %{ content: nil, @@ -48,7 +48,7 @@ test "includes reported statuses" do {:ok, activity} = CommonAPI.post(other_user, %{"status" => "toot"}) {:ok, report_activity} = - CommonAPI.report(user, %{"account_id" => other_user.id, "status_ids" => [activity.id]}) + CommonAPI.report(user, %{account_id: other_user.id, status_ids: [activity.id]}) other_user = Pleroma.User.get_by_id(other_user.id) @@ -81,7 +81,7 @@ test "renders report's state" do user = insert(:user) other_user = insert(:user) - {:ok, activity} = CommonAPI.report(user, %{"account_id" => other_user.id}) + {:ok, activity} = CommonAPI.report(user, %{account_id: other_user.id}) {:ok, activity} = CommonAPI.update_report_state(activity.id, "closed") assert %{state: "closed"} = @@ -94,8 +94,8 @@ test "renders report description" do {:ok, activity} = CommonAPI.report(user, %{ - "account_id" => other_user.id, - "comment" => "posts are too good for this instance" + account_id: other_user.id, + comment: "posts are too good for this instance" }) assert %{content: "posts are too good for this instance"} = @@ -108,8 +108,8 @@ test "sanitizes report description" do {:ok, activity} = CommonAPI.report(user, %{ - "account_id" => other_user.id, - "comment" => "" + account_id: other_user.id, + comment: "" }) data = Map.put(activity.data, "content", "") @@ -125,8 +125,8 @@ test "doesn't error out when the user doesn't exists" do {:ok, activity} = CommonAPI.report(user, %{ - "account_id" => other_user.id, - "comment" => "" + account_id: other_user.id, + comment: "" }) Pleroma.User.delete(other_user) diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 1758662b0..c6ccc02c4 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -485,9 +485,9 @@ test "creates a report" do comment = "foobar" report_data = %{ - "account_id" => target_user.id, - "comment" => comment, - "status_ids" => [activity.id] + account_id: target_user.id, + comment: comment, + status_ids: [activity.id] } note_obj = %{ @@ -517,9 +517,9 @@ test "updates report state" do {:ok, %Activity{id: report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) {:ok, report} = CommonAPI.update_report_state(report_id, "resolved") @@ -538,9 +538,9 @@ test "does not update report state when state is unsupported" do {:ok, %Activity{id: report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + account_id: target_user.id, + comment: "I feel offended", + status_ids: [activity.id] }) assert CommonAPI.update_report_state(report_id, "test") == {:error, "Unsupported state"} @@ -552,16 +552,16 @@ test "updates state of multiple reports" do {:ok, %Activity{id: first_report_id}} = CommonAPI.report(reporter, %{ - "account_id" => target_user.id, - "comment" => "I feel offended", - "status_ids" => [activity.id] + 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] + account_id: target_user.id, + comment: "I feel very offended!", + status_ids: [activity.id] }) {:ok, report_ids} = diff --git a/test/web/mastodon_api/controllers/report_controller_test.exs b/test/web/mastodon_api/controllers/report_controller_test.exs index 34ec8119e..21b037237 100644 --- a/test/web/mastodon_api/controllers/report_controller_test.exs +++ b/test/web/mastodon_api/controllers/report_controller_test.exs @@ -22,8 +22,9 @@ defmodule Pleroma.Web.MastodonAPI.ReportControllerTest do test "submit a basic report", %{conn: conn, target_user: target_user} do assert %{"action_taken" => false, "id" => _} = conn + |> put_req_header("content-type", "application/json") |> post("/api/v1/reports", %{"account_id" => target_user.id}) - |> json_response(200) + |> json_response_and_validate_schema(200) end test "submit a report with statuses and comment", %{ @@ -33,23 +34,25 @@ test "submit a report with statuses and comment", %{ } do assert %{"action_taken" => false, "id" => _} = conn + |> put_req_header("content-type", "application/json") |> post("/api/v1/reports", %{ "account_id" => target_user.id, "status_ids" => [activity.id], "comment" => "bad status!", "forward" => "false" }) - |> json_response(200) + |> json_response_and_validate_schema(200) end test "account_id is required", %{ conn: conn, activity: activity } do - assert %{"error" => "Valid `account_id` required"} = + assert %{"error" => "Missing field: account_id."} = conn + |> put_req_header("content-type", "application/json") |> post("/api/v1/reports", %{"status_ids" => [activity.id]}) - |> json_response(400) + |> json_response_and_validate_schema(400) end test "comment must be up to the size specified in the config", %{ @@ -63,17 +66,21 @@ test "comment must be up to the size specified in the config", %{ assert ^error = conn + |> put_req_header("content-type", "application/json") |> post("/api/v1/reports", %{"account_id" => target_user.id, "comment" => comment}) - |> json_response(400) + |> json_response_and_validate_schema(400) end test "returns error when account is not exist", %{ conn: conn, activity: activity } do - conn = post(conn, "/api/v1/reports", %{"status_ids" => [activity.id], "account_id" => "foo"}) + conn = + conn + |> put_req_header("content-type", "application/json") + |> post("/api/v1/reports", %{"status_ids" => [activity.id], "account_id" => "foo"}) - assert json_response(conn, 400) == %{"error" => "Account not found"} + assert json_response_and_validate_schema(conn, 400) == %{"error" => "Account not found"} end test "doesn't fail if an admin has no email", %{conn: conn, target_user: target_user} do @@ -81,7 +88,8 @@ test "doesn't fail if an admin has no email", %{conn: conn, target_user: target_ assert %{"action_taken" => false, "id" => _} = conn + |> put_req_header("content-type", "application/json") |> post("/api/v1/reports", %{"account_id" => target_user.id}) - |> json_response(200) + |> json_response_and_validate_schema(200) end end