From c9bcda4e0a4876e007725ab8c5b96dd05eda042f Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 20 Jul 2024 22:20:13 -0400 Subject: [PATCH 1/3] Document create_system_report and flip argument order --- lib/philomena/comments.ex | 2 +- lib/philomena/conversations.ex | 2 +- lib/philomena/images.ex | 2 +- lib/philomena/posts.ex | 2 +- lib/philomena/reports.ex | 14 ++++++++++++-- 5 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/philomena/comments.ex b/lib/philomena/comments.ex index 90569d6e..8b98b11e 100644 --- a/lib/philomena/comments.ex +++ b/lib/philomena/comments.ex @@ -210,8 +210,8 @@ defmodule Philomena.Comments do def report_non_approved(comment) do Reports.create_system_report( - comment.id, "Comment", + comment.id, "Approval", "Comment contains externally-embedded images and has been flagged for review." ) diff --git a/lib/philomena/conversations.ex b/lib/philomena/conversations.ex index e9ed7500..46a3ce8f 100644 --- a/lib/philomena/conversations.ex +++ b/lib/philomena/conversations.ex @@ -236,8 +236,8 @@ defmodule Philomena.Conversations do def report_non_approved(id) do Reports.create_system_report( - id, "Conversation", + id, "Approval", "PM contains externally-embedded images and has been flagged for review." ) diff --git a/lib/philomena/images.ex b/lib/philomena/images.ex index 5fea2e35..90bdd5b4 100644 --- a/lib/philomena/images.ex +++ b/lib/philomena/images.ex @@ -193,8 +193,8 @@ defmodule Philomena.Images do defp maybe_suggest_user_verification(%User{id: id, uploads_count: 5, verified: false}) do Reports.create_system_report( - id, "User", + id, "Verification", "User has uploaded enough approved images to be considered for verification." ) diff --git a/lib/philomena/posts.ex b/lib/philomena/posts.ex index fc339c97..bde21f16 100644 --- a/lib/philomena/posts.ex +++ b/lib/philomena/posts.ex @@ -114,8 +114,8 @@ defmodule Philomena.Posts do def report_non_approved(post) do Reports.create_system_report( - post.id, "Post", + post.id, "Approval", "Post contains externally-embedded images and has been flagged for review." ) diff --git a/lib/philomena/reports.ex b/lib/philomena/reports.ex index 4f10391d..f39838d5 100644 --- a/lib/philomena/reports.ex +++ b/lib/philomena/reports.ex @@ -95,7 +95,17 @@ defmodule Philomena.Reports do update: [set: [open: false, state: "closed", admin_id: ^closing_user.id]] end - def create_system_report(reportable_id, reportable_type, category, reason) do + @doc """ + Automatically create a report with the given category and reason on the given + `reportable_id` and `reportable_type`. + + ## Examples + + iex> create_system_report("Comment", 1, "Other", "Custom report reason") + {:ok, %Report{}} + + """ + def create_system_report(reportable_type, reportable_id, category, reason) do attrs = %{ reason: reason, category: category @@ -109,7 +119,7 @@ defmodule Philomena.Reports do "Mozilla/5.0 (X11; Philomena; Linux x86_64; rv:86.0) Gecko/20100101 Firefox/86.0" } - %Report{reportable_id: reportable_id, reportable_type: reportable_type} + %Report{reportable_type: reportable_type, reportable_id: reportable_id} |> Report.creation_changeset(attrs, attributes) |> Repo.insert() |> reindex_after_update() From c8410e7957f347f8c8bef528a905b3fe332b449b Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 20 Jul 2024 22:30:58 -0400 Subject: [PATCH 2/3] More documentation --- lib/philomena/reports.ex | 78 ++++++++++++++++--- .../plugs/admin_counters_plug.ex | 2 +- 2 files changed, 69 insertions(+), 11 deletions(-) diff --git a/lib/philomena/reports.ex b/lib/philomena/reports.ex index f39838d5..4ce7d868 100644 --- a/lib/philomena/reports.ex +++ b/lib/philomena/reports.ex @@ -12,6 +12,31 @@ defmodule Philomena.Reports do alias Philomena.IndexWorker alias Philomena.Polymorphic + @doc """ + Returns the current number of open reports. + + If the user is allowed to view reports, returns the current count. + If the user is not allowed to view reports, returns `nil`. + + ## Examples + + iex> count_reports(%User{}) + nil + + iex> count_reports(%User{role: "admin"}) + 4 + + """ + def count_open_reports(user) do + if Canada.Can.can?(user, :index, Report) do + Report + |> where(open: true) + |> Repo.aggregate(:count) + else + nil + end + end + @doc """ Returns the list of reports. @@ -173,6 +198,15 @@ defmodule Philomena.Reports do Report.changeset(report, %{}) end + @doc """ + Marks the report as claimed by the given user. + + ## Example + + iex> claim_report(%Report{}, %User{}) + {:ok, %Report{}} + + """ def claim_report(%Report{} = report, user) do report |> Report.claim_changeset(user) @@ -180,6 +214,15 @@ defmodule Philomena.Reports do |> reindex_after_update() end + @doc """ + Marks the report as unclaimed. + + ## Example + + iex> unclaim_report(%Report{}) + {:ok, %Report{}} + + """ def unclaim_report(%Report{} = report) do report |> Report.unclaim_changeset() @@ -187,6 +230,15 @@ defmodule Philomena.Reports do |> reindex_after_update() end + @doc """ + Marks the report as closed by the given user. + + ## Example + + iex> close_report(%Report{}, %User{}) + {:ok, %Report{}} + + """ def close_report(%Report{} = report, user) do report |> Report.close_changeset(user) @@ -194,6 +246,15 @@ defmodule Philomena.Reports do |> reindex_after_update() end + @doc """ + Reindex all reports where the user or admin has `old_name`. + + ## Example + + iex> user_name_reindex("Administrator", "Administrator2") + {:ok, %Req.Response{}} + + """ def user_name_reindex(old_name, new_name) do data = ReportIndex.user_name_update_by_query(old_name, new_name) @@ -210,18 +271,25 @@ defmodule Philomena.Reports do result end + @doc """ + Callback for post-transaction update. + + See `close_report_query/2` for more information and example. + """ def reindex_reports(report_ids) do Exq.enqueue(Exq, "indexing", IndexWorker, ["Reports", "id", report_ids]) report_ids end + @doc false def reindex_report(%Report{} = report) do Exq.enqueue(Exq, "indexing", IndexWorker, ["Reports", "id", [report.id]]) report end + @doc false def perform_reindex(column, condition) do Report |> where([r], field(r, ^column) in ^condition) @@ -230,14 +298,4 @@ defmodule Philomena.Reports do |> Polymorphic.load_polymorphic(reportable: [reportable_id: :reportable_type]) |> Enum.map(&Search.index_document(&1, Report)) end - - def count_reports(user) do - if Canada.Can.can?(user, :index, Report) do - Report - |> where(open: true) - |> Repo.aggregate(:count, :id) - else - nil - end - end end diff --git a/lib/philomena_web/plugs/admin_counters_plug.ex b/lib/philomena_web/plugs/admin_counters_plug.ex index 9e01a559..bf271c0e 100644 --- a/lib/philomena_web/plugs/admin_counters_plug.ex +++ b/lib/philomena_web/plugs/admin_counters_plug.ex @@ -34,7 +34,7 @@ defmodule PhilomenaWeb.AdminCountersPlug do defp maybe_assign_admin_metrics(conn, user, true) do pending_approvals = Images.count_pending_approvals(user) duplicate_reports = DuplicateReports.count_duplicate_reports(user) - reports = Reports.count_reports(user) + reports = Reports.count_open_reports(user) artist_links = ArtistLinks.count_artist_links(user) dnps = DnpEntries.count_dnp_entries(user) From c898b83e6dec5ee75a8ff96e980ad90c4a3edf61 Mon Sep 17 00:00:00 2001 From: Liam Date: Sat, 20 Jul 2024 22:40:57 -0400 Subject: [PATCH 3/3] Reverse order of type and id arguments --- lib/philomena/comments.ex | 7 +++---- lib/philomena/conversations.ex | 5 ++--- lib/philomena/images.ex | 5 ++--- lib/philomena/posts.ex | 7 +++---- lib/philomena/reports.ex | 12 ++++++------ .../controllers/conversation/report_controller.ex | 2 +- .../controllers/gallery/report_controller.ex | 2 +- .../controllers/image/comment/report_controller.ex | 2 +- .../controllers/image/report_controller.ex | 2 +- .../profile/commission/report_controller.ex | 2 +- .../controllers/profile/report_controller.ex | 2 +- lib/philomena_web/controllers/report_controller.ex | 4 ++-- .../controllers/topic/post/report_controller.ex | 2 +- 13 files changed, 25 insertions(+), 29 deletions(-) diff --git a/lib/philomena/comments.ex b/lib/philomena/comments.ex index 8b98b11e..d3a819d9 100644 --- a/lib/philomena/comments.ex +++ b/lib/philomena/comments.ex @@ -144,7 +144,7 @@ defmodule Philomena.Comments do end def hide_comment(%Comment{} = comment, attrs, user) do - report_query = Reports.close_report_query("Comment", comment.id, user) + report_query = Reports.close_report_query({"Comment", comment.id}, user) comment = Comment.hide_changeset(comment, attrs, user) Multi.new() @@ -185,7 +185,7 @@ defmodule Philomena.Comments do end def approve_comment(%Comment{} = comment, user) do - report_query = Reports.close_report_query("Comment", comment.id, user) + report_query = Reports.close_report_query({"Comment", comment.id}, user) comment = Comment.approve_changeset(comment) Multi.new() @@ -210,8 +210,7 @@ defmodule Philomena.Comments do def report_non_approved(comment) do Reports.create_system_report( - "Comment", - comment.id, + {"Comment", comment.id}, "Approval", "Comment contains externally-embedded images and has been flagged for review." ) diff --git a/lib/philomena/conversations.ex b/lib/philomena/conversations.ex index 46a3ce8f..82a2e495 100644 --- a/lib/philomena/conversations.ex +++ b/lib/philomena/conversations.ex @@ -208,7 +208,7 @@ defmodule Philomena.Conversations do end def approve_conversation_message(message, user) do - reports_query = Reports.close_report_query("Conversation", message.conversation_id, user) + reports_query = Reports.close_report_query({"Conversation", message.conversation_id}, user) message_query = message @@ -236,8 +236,7 @@ defmodule Philomena.Conversations do def report_non_approved(id) do Reports.create_system_report( - "Conversation", - id, + {"Conversation", id}, "Approval", "PM contains externally-embedded images and has been flagged for review." ) diff --git a/lib/philomena/images.ex b/lib/philomena/images.ex index 90bdd5b4..605c6f0a 100644 --- a/lib/philomena/images.ex +++ b/lib/philomena/images.ex @@ -193,8 +193,7 @@ defmodule Philomena.Images do defp maybe_suggest_user_verification(%User{id: id, uploads_count: 5, verified: false}) do Reports.create_system_report( - "User", - id, + {"User", id}, "Verification", "User has uploaded enough approved images to be considered for verification." ) @@ -577,7 +576,7 @@ defmodule Philomena.Images do end defp hide_image_multi(changeset, image, user, multi) do - report_query = Reports.close_report_query("Image", image.id, user) + report_query = Reports.close_report_query({"Image", image.id}, user) galleries = Gallery diff --git a/lib/philomena/posts.ex b/lib/philomena/posts.ex index bde21f16..723ea6b6 100644 --- a/lib/philomena/posts.ex +++ b/lib/philomena/posts.ex @@ -114,8 +114,7 @@ defmodule Philomena.Posts do def report_non_approved(post) do Reports.create_system_report( - "Post", - post.id, + {"Post", post.id}, "Approval", "Post contains externally-embedded images and has been flagged for review." ) @@ -203,7 +202,7 @@ defmodule Philomena.Posts do end def hide_post(%Post{} = post, attrs, user) do - report_query = Reports.close_report_query("Post", post.id, user) + report_query = Reports.close_report_query({"Post", post.id}, user) topics = Topic @@ -250,7 +249,7 @@ defmodule Philomena.Posts do end def approve_post(%Post{} = post, user) do - report_query = Reports.close_report_query("Post", post.id, user) + report_query = Reports.close_report_query({"Post", post.id}, user) post = Post.approve_changeset(post) Multi.new() diff --git a/lib/philomena/reports.ex b/lib/philomena/reports.ex index 4ce7d868..5cf508dd 100644 --- a/lib/philomena/reports.ex +++ b/lib/philomena/reports.ex @@ -78,8 +78,8 @@ defmodule Philomena.Reports do {:error, %Ecto.Changeset{}} """ - def create_report(reportable_id, reportable_type, attribution, attrs \\ %{}) do - %Report{reportable_id: reportable_id, reportable_type: reportable_type} + def create_report({reportable_type, reportable_id} = _type_and_id, attribution, attrs \\ %{}) do + %Report{reportable_type: reportable_type, reportable_id: reportable_id} |> Report.creation_changeset(attrs, attribution) |> Repo.insert() |> reindex_after_update() @@ -92,7 +92,7 @@ defmodule Philomena.Reports do Because this is only a query due to the limitations of `m:Ecto.Multi`, this must be coupled with an associated call to `reindex_reports/1` to operate correctly, e.g.: - report_query = Reports.close_system_report_query("Image", image.id, user) + report_query = Reports.close_system_report_query({"Image", image.id}, user) Multi.new() |> Multi.update_all(:reports, report_query, []) @@ -113,7 +113,7 @@ defmodule Philomena.Reports do #Ecto.Query<...> """ - def close_report_query(reportable_type, reportable_id, closing_user) do + def close_report_query({reportable_type, reportable_id} = _type_and_id, closing_user) do from r in Report, where: r.reportable_type == ^reportable_type and r.reportable_id == ^reportable_id, select: r.id, @@ -126,11 +126,11 @@ defmodule Philomena.Reports do ## Examples - iex> create_system_report("Comment", 1, "Other", "Custom report reason") + iex> create_system_report({"Comment", 1}, "Other", "Custom report reason") {:ok, %Report{}} """ - def create_system_report(reportable_type, reportable_id, category, reason) do + def create_system_report({reportable_type, reportable_id} = _type_and_id, category, reason) do attrs = %{ reason: reason, category: category diff --git a/lib/philomena_web/controllers/conversation/report_controller.ex b/lib/philomena_web/controllers/conversation/report_controller.ex index dab81482..0f5736ca 100644 --- a/lib/philomena_web/controllers/conversation/report_controller.ex +++ b/lib/philomena_web/controllers/conversation/report_controller.ex @@ -42,6 +42,6 @@ defmodule PhilomenaWeb.Conversation.ReportController do conversation = conn.assigns.conversation action = ~p"/conversations/#{conversation}/reports" - ReportController.create(conn, action, conversation, "Conversation", params) + ReportController.create(conn, action, "Conversation", conversation, params) end end diff --git a/lib/philomena_web/controllers/gallery/report_controller.ex b/lib/philomena_web/controllers/gallery/report_controller.ex index 3d4b5fd5..c5d8b0a2 100644 --- a/lib/philomena_web/controllers/gallery/report_controller.ex +++ b/lib/philomena_web/controllers/gallery/report_controller.ex @@ -41,6 +41,6 @@ defmodule PhilomenaWeb.Gallery.ReportController do gallery = conn.assigns.gallery action = ~p"/galleries/#{gallery}/reports" - ReportController.create(conn, action, gallery, "Gallery", params) + ReportController.create(conn, action, "Gallery", gallery, params) end end diff --git a/lib/philomena_web/controllers/image/comment/report_controller.ex b/lib/philomena_web/controllers/image/comment/report_controller.ex index d957abbd..cb2f0b98 100644 --- a/lib/philomena_web/controllers/image/comment/report_controller.ex +++ b/lib/philomena_web/controllers/image/comment/report_controller.ex @@ -44,6 +44,6 @@ defmodule PhilomenaWeb.Image.Comment.ReportController do comment = conn.assigns.comment action = ~p"/images/#{comment.image}/comments/#{comment}/reports" - ReportController.create(conn, action, comment, "Comment", params) + ReportController.create(conn, action, "Comment", comment, params) end end diff --git a/lib/philomena_web/controllers/image/report_controller.ex b/lib/philomena_web/controllers/image/report_controller.ex index 6956832e..c00cee3d 100644 --- a/lib/philomena_web/controllers/image/report_controller.ex +++ b/lib/philomena_web/controllers/image/report_controller.ex @@ -41,6 +41,6 @@ defmodule PhilomenaWeb.Image.ReportController do image = conn.assigns.image action = ~p"/images/#{image}/reports" - ReportController.create(conn, action, image, "Image", params) + ReportController.create(conn, action, "Image", image, params) end end diff --git a/lib/philomena_web/controllers/profile/commission/report_controller.ex b/lib/philomena_web/controllers/profile/commission/report_controller.ex index 0ad943ef..9fe29308 100644 --- a/lib/philomena_web/controllers/profile/commission/report_controller.ex +++ b/lib/philomena_web/controllers/profile/commission/report_controller.ex @@ -53,7 +53,7 @@ defmodule PhilomenaWeb.Profile.Commission.ReportController do commission = conn.assigns.user.commission action = ~p"/profiles/#{user}/commission/reports" - ReportController.create(conn, action, commission, "Commission", params) + ReportController.create(conn, action, "Commission", commission, params) end defp ensure_commission(conn, _opts) do diff --git a/lib/philomena_web/controllers/profile/report_controller.ex b/lib/philomena_web/controllers/profile/report_controller.ex index 80a68895..b57c3a64 100644 --- a/lib/philomena_web/controllers/profile/report_controller.ex +++ b/lib/philomena_web/controllers/profile/report_controller.ex @@ -41,6 +41,6 @@ defmodule PhilomenaWeb.Profile.ReportController do user = conn.assigns.user action = ~p"/profiles/#{user}/reports" - ReportController.create(conn, action, user, "User", params) + ReportController.create(conn, action, "User", user, params) end end diff --git a/lib/philomena_web/controllers/report_controller.ex b/lib/philomena_web/controllers/report_controller.ex index 7860a32e..803f224e 100644 --- a/lib/philomena_web/controllers/report_controller.ex +++ b/lib/philomena_web/controllers/report_controller.ex @@ -33,7 +33,7 @@ defmodule PhilomenaWeb.ReportController do # plug PhilomenaWeb.CheckCaptchaPlug when action in [:create] # plug :load_and_authorize_resource, model: Image, id_name: "image_id", persisted: true - def create(conn, action, reportable, reportable_type, %{"report" => report_params}) do + def create(conn, action, reportable_type, reportable, %{"report" => report_params}) do attribution = conn.assigns.attributes case too_many_reports?(conn) do @@ -46,7 +46,7 @@ defmodule PhilomenaWeb.ReportController do |> redirect(to: "/") _falsy -> - case Reports.create_report(reportable.id, reportable_type, attribution, report_params) do + case Reports.create_report({reportable_type, reportable.id}, attribution, report_params) do {:ok, _report} -> conn |> put_flash( diff --git a/lib/philomena_web/controllers/topic/post/report_controller.ex b/lib/philomena_web/controllers/topic/post/report_controller.ex index f09df511..b93ab225 100644 --- a/lib/philomena_web/controllers/topic/post/report_controller.ex +++ b/lib/philomena_web/controllers/topic/post/report_controller.ex @@ -42,6 +42,6 @@ defmodule PhilomenaWeb.Topic.Post.ReportController do post = conn.assigns.post action = ~p"/forums/#{topic.forum}/topics/#{topic}/posts/#{post}/reports" - ReportController.create(conn, action, post, "Post", params) + ReportController.create(conn, action, "Post", post, params) end end