From 66f411fba0ecb350a2cd80293aabdecf402abaf9 Mon Sep 17 00:00:00 2001 From: Maksim Pechnikov Date: Thu, 19 Nov 2020 22:13:45 +0300 Subject: [PATCH] added subject actor to moderation log --- lib/pleroma/activity.ex | 13 ++ lib/pleroma/moderation_log.ex | 122 +++++++++++------- .../controllers/report_controller.ex | 17 ++- test/pleroma/activity_test.exs | 7 + test/pleroma/moderation_log_test.exs | 33 +++-- .../controllers/report_controller_test.exs | 18 +-- .../views/moderation_log_view_test.exs | 98 ++++++++++++++ 7 files changed, 240 insertions(+), 68 deletions(-) create mode 100644 test/pleroma/web/admin_api/views/moderation_log_view_test.exs diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 553834da0..d2066f7a0 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -194,6 +194,19 @@ def get_by_id(id) do end end + def get_by_id_with_user_actor(id) do + case FlakeId.flake_id?(id) do + true -> + Activity + |> where([a], a.id == ^id) + |> with_preloaded_user_actor() + |> Repo.one() + + _ -> + nil + end + end + def get_by_id_with_object(id) do Activity |> where(id: ^id) diff --git a/lib/pleroma/moderation_log.ex b/lib/pleroma/moderation_log.ex index 142dd8e0a..0a701127f 100644 --- a/lib/pleroma/moderation_log.ex +++ b/lib/pleroma/moderation_log.ex @@ -112,16 +112,19 @@ def insert_log(%{ @spec insert_log(%{actor: User, subject: User, action: String.t()}) :: {:ok, ModerationLog} | {:error, any} - def insert_log(%{ - actor: %User{} = actor, - action: "report_update", - subject: %Activity{data: %{"type" => "Flag"}} = subject - }) do + def insert_log( + %{ + actor: %User{} = actor, + action: "report_update", + subject: %Activity{data: %{"type" => "Flag"}} = subject + } = attrs + ) do %ModerationLog{ data: %{ "actor" => user_to_map(actor), "action" => "report_update", "subject" => report_to_map(subject), + "subject_actor" => user_to_map(attrs[:subject_actor]), "message" => "" } } @@ -130,17 +133,20 @@ def insert_log(%{ @spec insert_log(%{actor: User, subject: Activity, action: String.t(), text: String.t()}) :: {:ok, ModerationLog} | {:error, any} - def insert_log(%{ - actor: %User{} = actor, - action: "report_note", - subject: %Activity{} = subject, - text: text - }) do + def insert_log( + %{ + actor: %User{} = actor, + action: "report_note", + subject: %Activity{} = subject, + text: text + } = attrs + ) do %ModerationLog{ data: %{ "actor" => user_to_map(actor), "action" => "report_note", "subject" => report_to_map(subject), + "subject_actor" => user_to_map(attrs[:subject_actor]), "text" => text } } @@ -149,17 +155,20 @@ def insert_log(%{ @spec insert_log(%{actor: User, subject: Activity, action: String.t(), text: String.t()}) :: {:ok, ModerationLog} | {:error, any} - def insert_log(%{ - actor: %User{} = actor, - action: "report_note_delete", - subject: %Activity{} = subject, - text: text - }) do + def insert_log( + %{ + actor: %User{} = actor, + action: "report_note_delete", + subject: %Activity{} = subject, + text: text + } = attrs + ) do %ModerationLog{ data: %{ "actor" => user_to_map(actor), "action" => "report_note_delete", "subject" => report_to_map(subject), + "subject_actor" => user_to_map(attrs[:subject_actor]), "text" => text } } @@ -345,17 +354,18 @@ defp insert_log_entry_with_message(entry) do end defp user_to_map(users) when is_list(users) do - users |> Enum.map(&user_to_map/1) + Enum.map(users, &user_to_map/1) end defp user_to_map(%User{} = user) do user - |> Map.from_struct() |> Map.take([:id, :nickname]) |> Map.new(fn {k, v} -> {Atom.to_string(k), v} end) |> Map.put("type", "user") end + defp user_to_map(_), do: nil + defp report_to_map(%Activity{} = report) do %{ "type" => "report", @@ -512,38 +522,48 @@ def get_log_entry_message(%ModerationLog{ end @spec get_log_entry_message(ModerationLog) :: String.t() - def get_log_entry_message(%ModerationLog{ - data: %{ - "actor" => %{"nickname" => actor_nickname}, - "action" => "report_update", - "subject" => %{"id" => subject_id, "state" => state, "type" => "report"} - } - }) do - "@#{actor_nickname} updated report ##{subject_id} with '#{state}' state" + def get_log_entry_message( + %ModerationLog{ + data: %{ + "actor" => %{"nickname" => actor_nickname}, + "action" => "report_update", + "subject" => %{"id" => subject_id, "state" => state, "type" => "report"} + } + } = log + ) do + "@#{actor_nickname} updated report ##{subject_id}" <> + subject_actor_nickname(log, " (on user ", ")") <> + " with '#{state}' state" end @spec get_log_entry_message(ModerationLog) :: String.t() - def get_log_entry_message(%ModerationLog{ - data: %{ - "actor" => %{"nickname" => actor_nickname}, - "action" => "report_note", - "subject" => %{"id" => subject_id, "type" => "report"}, - "text" => text - } - }) do - "@#{actor_nickname} added note '#{text}' to report ##{subject_id}" + def get_log_entry_message( + %ModerationLog{ + data: %{ + "actor" => %{"nickname" => actor_nickname}, + "action" => "report_note", + "subject" => %{"id" => subject_id, "type" => "report"}, + "text" => text + } + } = log + ) do + "@#{actor_nickname} added note '#{text}' to report ##{subject_id}" <> + subject_actor_nickname(log, " on user ") end @spec get_log_entry_message(ModerationLog) :: String.t() - def get_log_entry_message(%ModerationLog{ - data: %{ - "actor" => %{"nickname" => actor_nickname}, - "action" => "report_note_delete", - "subject" => %{"id" => subject_id, "type" => "report"}, - "text" => text - } - }) do - "@#{actor_nickname} deleted note '#{text}' from report ##{subject_id}" + def get_log_entry_message( + %ModerationLog{ + data: %{ + "actor" => %{"nickname" => actor_nickname}, + "action" => "report_note_delete", + "subject" => %{"id" => subject_id, "type" => "report"}, + "text" => text + } + } = log + ) do + "@#{actor_nickname} deleted note '#{text}' from report ##{subject_id}" <> + subject_actor_nickname(log, " on user ") end @spec get_log_entry_message(ModerationLog) :: String.t() @@ -676,4 +696,16 @@ defp users_to_nicknames_string(users) do |> Enum.map(&"@#{&1["nickname"]}") |> Enum.join(", ") end + + defp subject_actor_nickname(%ModerationLog{data: data}, prefix_msg, postfix_msg \\ "") do + case data do + %{"subject_actor" => %{"nickname" => subject_actor}} -> + [prefix_msg, "@#{subject_actor}", postfix_msg] + |> Enum.reject(&(&1 == "")) + |> Enum.join() + + _ -> + "" + end + end end diff --git a/lib/pleroma/web/admin_api/controllers/report_controller.ex b/lib/pleroma/web/admin_api/controllers/report_controller.ex index 6a0e56f5f..cc77cbfdf 100644 --- a/lib/pleroma/web/admin_api/controllers/report_controller.ex +++ b/lib/pleroma/web/admin_api/controllers/report_controller.ex @@ -50,10 +50,13 @@ def update(%{assigns: %{user: admin}, body_params: %{reports: reports}} = conn, Enum.map(reports, fn report -> case CommonAPI.update_report_state(report.id, report.state) do {:ok, activity} -> + report = Activity.get_by_id_with_user_actor(activity.id) + ModerationLog.insert_log(%{ action: "report_update", actor: admin, - subject: activity + subject: activity, + subject_actor: report.user_actor }) activity @@ -73,11 +76,13 @@ def update(%{assigns: %{user: admin}, body_params: %{reports: reports}} = conn, def notes_create(%{assigns: %{user: user}, body_params: %{content: content}} = conn, %{ id: report_id }) do - with {:ok, _} <- ReportNote.create(user.id, report_id, content) do + with {:ok, _} <- ReportNote.create(user.id, report_id, content), + report <- Activity.get_by_id_with_user_actor(report_id) do ModerationLog.insert_log(%{ action: "report_note", actor: user, - subject: Activity.get_by_id(report_id), + subject: report, + subject_actor: report.user_actor, text: content }) @@ -91,11 +96,13 @@ def notes_delete(%{assigns: %{user: user}} = conn, %{ id: note_id, report_id: report_id }) do - with {:ok, note} <- ReportNote.destroy(note_id) do + with {:ok, note} <- ReportNote.destroy(note_id), + report <- Activity.get_by_id_with_user_actor(report_id) do ModerationLog.insert_log(%{ action: "report_note_delete", actor: user, - subject: Activity.get_by_id(report_id), + subject: report, + subject_actor: report.user_actor, text: note.content }) diff --git a/test/pleroma/activity_test.exs b/test/pleroma/activity_test.exs index ee6a99cc3..dfb811d77 100644 --- a/test/pleroma/activity_test.exs +++ b/test/pleroma/activity_test.exs @@ -197,6 +197,13 @@ test "all_by_ids_with_object/1" do assert [%{id: ^id1, object: %Object{}}, %{id: ^id2, object: %Object{}}] = activities end + test "get_by_id_with_user_actor/1" do + user = insert(:user) + activity = insert(:note_activity, note: insert(:note, user: user)) + + assert Activity.get_by_id_with_user_actor(activity.id).user_actor == user + end + test "get_by_id_with_object/1" do %{id: id} = insert(:note_activity) diff --git a/test/pleroma/moderation_log_test.exs b/test/pleroma/moderation_log_test.exs index 59f4d67f8..fe705def1 100644 --- a/test/pleroma/moderation_log_test.exs +++ b/test/pleroma/moderation_log_test.exs @@ -186,7 +186,8 @@ test "logging report update", %{moderator: moderator} do id: "9m9I1F4p8ftrTP6QTI", data: %{ "type" => "Flag", - "state" => "resolved" + "state" => "resolved", + "actor" => "http://localhost:4000/users/max" } } @@ -204,25 +205,37 @@ test "logging report update", %{moderator: moderator} do end test "logging report response", %{moderator: moderator} do + user = insert(:user) + report = %Activity{ id: "9m9I1F4p8ftrTP6QTI", data: %{ - "type" => "Note" + "type" => "Note", + "actor" => user.ap_id } } - {:ok, _} = - ModerationLog.insert_log(%{ - actor: moderator, - action: "report_note", - subject: report, - text: "look at this" - }) + attrs = %{ + actor: moderator, + action: "report_note", + subject: report, + text: "look at this" + } - log = Repo.one(ModerationLog) + {:ok, log1} = ModerationLog.insert_log(attrs) + log = Repo.get(ModerationLog, log1.id) assert log.data["message"] == "@#{moderator.nickname} added note 'look at this' to report ##{report.id}" + + {:ok, log2} = ModerationLog.insert_log(Map.merge(attrs, %{subject_actor: user})) + + log = Repo.get(ModerationLog, log2.id) + + assert log.data["message"] == + "@#{moderator.nickname} added note 'look at this' to report ##{report.id} on user @#{ + user.nickname + }" end test "logging status sensitivity update", %{moderator: moderator} do diff --git a/test/pleroma/web/admin_api/controllers/report_controller_test.exs b/test/pleroma/web/admin_api/controllers/report_controller_test.exs index 958e1d3ab..cbfc2e7b0 100644 --- a/test/pleroma/web/admin_api/controllers/report_controller_test.exs +++ b/test/pleroma/web/admin_api/controllers/report_controller_test.exs @@ -122,13 +122,13 @@ test "mark report as resolved", %{conn: conn, id: id, admin: admin} do }) |> json_response_and_validate_schema(:no_content) - activity = Activity.get_by_id(id) + activity = Activity.get_by_id_with_user_actor(id) assert activity.data["state"] == "resolved" log_entry = Repo.one(ModerationLog) assert ModerationLog.get_log_entry_message(log_entry) == - "@#{admin.nickname} updated report ##{id} with 'resolved' state" + "@#{admin.nickname} updated report ##{id} (on user @#{activity.user_actor.nickname}) with 'resolved' state" end test "closes report", %{conn: conn, id: id, admin: admin} do @@ -141,13 +141,13 @@ test "closes report", %{conn: conn, id: id, admin: admin} do }) |> json_response_and_validate_schema(:no_content) - activity = Activity.get_by_id(id) + activity = Activity.get_by_id_with_user_actor(id) assert activity.data["state"] == "closed" log_entry = Repo.one(ModerationLog) assert ModerationLog.get_log_entry_message(log_entry) == - "@#{admin.nickname} updated report ##{id} with 'closed' state" + "@#{admin.nickname} updated report ##{id} (on user @#{activity.user_actor.nickname}) with 'closed' state" end test "returns 400 when state is unknown", %{conn: conn, id: id} do @@ -193,18 +193,20 @@ test "updates state of multiple reports", %{ }) |> json_response_and_validate_schema(:no_content) - activity = Activity.get_by_id(id) - second_activity = Activity.get_by_id(second_report_id) + activity = Activity.get_by_id_with_user_actor(id) + second_activity = Activity.get_by_id_with_user_actor(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" + "@#{admin.nickname} updated report ##{id} (on user @#{activity.user_actor.nickname}) with 'resolved' state" assert ModerationLog.get_log_entry_message(second_log_entry) == - "@#{admin.nickname} updated report ##{second_report_id} with 'closed' state" + "@#{admin.nickname} updated report ##{second_report_id} (on user @#{ + second_activity.user_actor.nickname + }) with 'closed' state" end end diff --git a/test/pleroma/web/admin_api/views/moderation_log_view_test.exs b/test/pleroma/web/admin_api/views/moderation_log_view_test.exs new file mode 100644 index 000000000..e6c5aaa7f --- /dev/null +++ b/test/pleroma/web/admin_api/views/moderation_log_view_test.exs @@ -0,0 +1,98 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only +defmodule Pleroma.Web.AdminAPI.ModerationLogViewTest do + use Pleroma.DataCase + + alias Pleroma.Web.AdminAPI.ModerationLogView + + describe "renders `report_note_delete` log messages" do + setup do + log1 = %Pleroma.ModerationLog{ + data: %{ + "action" => "report_note_delete", + "actor" => %{"id" => "A1I7G8", "nickname" => "admin", "type" => "user"}, + "message" => "@admin deleted note 'mistake' from report #A1I7be on user @b-612", + "subject" => %{"id" => "A1I7be", "state" => "open", "type" => "report"}, + "subject_actor" => %{"id" => "A1I7G8", "nickname" => "b-612", "type" => "user"}, + "text" => "mistake" + }, + inserted_at: ~N[2020-11-17 14:13:20] + } + + log2 = %Pleroma.ModerationLog{ + data: %{ + "action" => "report_note_delete", + "actor" => %{"id" => "A1I7G8", "nickname" => "admin", "type" => "user"}, + "message" => "@admin deleted note 'fake user' from report #A1I7be on user @j-612", + "subject" => %{"id" => "A1I7be", "state" => "open", "type" => "report"}, + "subject_actor" => %{"id" => "A1I7G8", "nickname" => "j-612", "type" => "user"}, + "text" => "fake user" + }, + inserted_at: ~N[2020-11-17 14:13:20] + } + + {:ok, %{log1: log1, log2: log2}} + end + + test "renders `report_note_delete` log messages", %{log1: log1, log2: log2} do + assert ModerationLogView.render( + "index.json", + %{log: %{items: [log1, log2], count: 2}} + ) == %{ + items: [ + %{ + data: %{ + "action" => "report_note_delete", + "actor" => %{"id" => "A1I7G8", "nickname" => "admin", "type" => "user"}, + "message" => + "@admin deleted note 'mistake' from report #A1I7be on user @b-612", + "subject" => %{"id" => "A1I7be", "state" => "open", "type" => "report"}, + "subject_actor" => %{ + "id" => "A1I7G8", + "nickname" => "b-612", + "type" => "user" + }, + "text" => "mistake" + }, + message: "@admin deleted note 'mistake' from report #A1I7be on user @b-612", + time: 1_605_622_400 + }, + %{ + data: %{ + "action" => "report_note_delete", + "actor" => %{"id" => "A1I7G8", "nickname" => "admin", "type" => "user"}, + "message" => + "@admin deleted note 'fake user' from report #A1I7be on user @j-612", + "subject" => %{"id" => "A1I7be", "state" => "open", "type" => "report"}, + "subject_actor" => %{ + "id" => "A1I7G8", + "nickname" => "j-612", + "type" => "user" + }, + "text" => "fake user" + }, + message: "@admin deleted note 'fake user' from report #A1I7be on user @j-612", + time: 1_605_622_400 + } + ], + total: 2 + } + end + + test "renders `report_note_delete` log message", %{log1: log} do + assert ModerationLogView.render("show.json", %{log_entry: log}) == %{ + data: %{ + "action" => "report_note_delete", + "actor" => %{"id" => "A1I7G8", "nickname" => "admin", "type" => "user"}, + "message" => "@admin deleted note 'mistake' from report #A1I7be on user @b-612", + "subject" => %{"id" => "A1I7be", "state" => "open", "type" => "report"}, + "subject_actor" => %{"id" => "A1I7G8", "nickname" => "b-612", "type" => "user"}, + "text" => "mistake" + }, + message: "@admin deleted note 'mistake' from report #A1I7be on user @b-612", + time: 1_605_622_400 + } + end + end +end