From 687db1b98db66ad39e06323a5d71a27a2361ea0f Mon Sep 17 00:00:00 2001 From: "byte[]" Date: Sun, 8 Dec 2019 23:41:35 -0500 Subject: [PATCH] duplicate reports --- lib/philomena/duplicate_reports.ex | 65 ++++++++++++++++--- .../duplicate_reports/duplicate_report.ex | 24 +++++++ lib/philomena/images.ex | 3 +- lib/philomena/images/image.ex | 1 + lib/philomena/interactions.ex | 13 +++- .../duplicate_report/accept_controller.ex | 17 +++++ .../accept_reverse_controller.ex | 17 +++++ .../duplicate_report/claim_controller.ex | 25 +++++++ .../duplicate_report/reject_controller.ex | 19 ++++++ .../duplicate_report_controller.ex | 2 +- .../controllers/image/reporting_controller.ex | 2 +- lib/philomena_web/router.ex | 7 ++ .../duplicate_report/_image_cell.html.slime | 8 +-- .../duplicate_report/_list.html.slime | 24 +++---- .../image/_image_container.html.slime | 2 +- .../templates/image/_image_target.html.slime | 2 +- .../templates/image/deleted.html.slime | 54 ++++++++------- lib/philomena_web/views/app_view.ex | 1 + lib/philomena_web/views/image_view.ex | 20 ++++-- 19 files changed, 247 insertions(+), 59 deletions(-) create mode 100644 lib/philomena_web/controllers/duplicate_report/accept_controller.ex create mode 100644 lib/philomena_web/controllers/duplicate_report/accept_reverse_controller.ex create mode 100644 lib/philomena_web/controllers/duplicate_report/claim_controller.ex create mode 100644 lib/philomena_web/controllers/duplicate_report/reject_controller.ex diff --git a/lib/philomena/duplicate_reports.ex b/lib/philomena/duplicate_reports.ex index 046ba3e2..5f63ff43 100644 --- a/lib/philomena/duplicate_reports.ex +++ b/lib/philomena/duplicate_reports.ex @@ -9,6 +9,7 @@ defmodule Philomena.DuplicateReports do alias Philomena.DuplicateReports.DuplicateReport alias Philomena.ImageIntensities.ImageIntensity alias Philomena.Images.Image + alias Philomena.Images def generate_reports(source) do source = Repo.preload(source, :intensity) @@ -68,21 +69,65 @@ defmodule Philomena.DuplicateReports do |> Repo.insert() end - @doc """ - Updates a duplicate_report. + # TODO: can we get this in a single transaction? + def accept_duplicate_report(%DuplicateReport{} = duplicate_report, user) do + result = + duplicate_report + |> DuplicateReport.accept_changeset(user) + |> Repo.update() - ## Examples + case result do + {:ok, duplicate_report} -> + duplicate_report = Repo.preload(duplicate_report, [:image, :duplicate_of_image]) - iex> update_duplicate_report(duplicate_report, %{field: new_value}) - {:ok, %DuplicateReport{}} + Images.merge_image(duplicate_report.image, duplicate_report.duplicate_of_image) - iex> update_duplicate_report(duplicate_report, %{field: bad_value}) - {:error, %Ecto.Changeset{}} + _error -> + result + end + end - """ - def update_duplicate_report(%DuplicateReport{} = duplicate_report, attrs) do + def accept_reverse_duplicate_report(%DuplicateReport{} = duplicate_report, user) do + {:ok, duplicate_report} = reject_duplicate_report(duplicate_report, user) + + # Need a constraint for upsert, so have to do it the hard way + new_report = Repo.get_by(DuplicateReport, duplicate_of_image_id: duplicate_report.image_id) + + new_report = + if new_report do + new_report + else + {:ok, duplicate_report} = + %DuplicateReport{ + image_id: duplicate_report.duplicate_of_image_id, + duplicate_of_image_id: duplicate_report.image_id, + reason: Enum.join([duplicate_report.reason, "(Reverse accepted)"], "\n"), + user_id: user.id + } + |> DuplicateReport.changeset(%{}) + |> Repo.insert() + + duplicate_report + end + + accept_duplicate_report(new_report, user) + end + + def claim_duplicate_report(%DuplicateReport{} = duplicate_report, user) do duplicate_report - |> DuplicateReport.changeset(attrs) + |> DuplicateReport.claim_changeset(user) + |> Repo.update() + end + + def unclaim_duplicate_report(%DuplicateReport{} = duplicate_report) do + duplicate_report + |> DuplicateReport.unclaim_changeset() + |> Repo.update() + end + + def reject_duplicate_report(%DuplicateReport{} = duplicate_report, user) do + duplicate_report + |> DuplicateReport.reject_changeset(user) |> Repo.update() end diff --git a/lib/philomena/duplicate_reports/duplicate_report.ex b/lib/philomena/duplicate_reports/duplicate_report.ex index f94d15a4..231e41c7 100644 --- a/lib/philomena/duplicate_reports/duplicate_report.ex +++ b/lib/philomena/duplicate_reports/duplicate_report.ex @@ -31,4 +31,28 @@ defmodule Philomena.DuplicateReports.DuplicateReport do |> put_assoc(:user, attribution[:user]) |> validate_length(:reason, max: 250, count: :bytes) end + + def accept_changeset(duplicate_report, user) do + change(duplicate_report) + |> put_change(:modifier_id, user.id) + |> put_change(:state, "accepted") + end + + def claim_changeset(duplicate_report, user) do + change(duplicate_report) + |> put_change(:modifier_id, user.id) + |> put_change(:state, "claimed") + end + + def unclaim_changeset(duplicate_report) do + change(duplicate_report) + |> put_change(:modifier_id, nil) + |> put_change(:state, "open") + end + + def reject_changeset(duplicate_report, user) do + change(duplicate_report) + |> put_change(:modifier_id, user.id) + |> put_change(:state, "rejected") + end end diff --git a/lib/philomena/images.ex b/lib/philomena/images.ex index a41b2e20..92336318 100644 --- a/lib/philomena/images.ex +++ b/lib/philomena/images.ex @@ -245,7 +245,7 @@ defmodule Philomena.Images do |> Repo.isolated_transaction(:serializable) end - def unhide_image(%Image{} = image) do + def unhide_image(%Image{hidden_from_users: true} = image) do key = image.hidden_image_key Multi.new @@ -267,6 +267,7 @@ defmodule Philomena.Images do end) |> Repo.isolated_transaction(:serializable) end + def unhide_image(image), do: {:ok, image} @doc """ Deletes a Image. diff --git a/lib/philomena/images/image.ex b/lib/philomena/images/image.ex index b3500eaa..fa707ce8 100644 --- a/lib/philomena/images/image.ex +++ b/lib/philomena/images/image.ex @@ -201,6 +201,7 @@ defmodule Philomena.Images.Image do |> put_change(:hidden_image_key, nil) |> put_change(:hidden_from_users, false) |> put_change(:deletion_reason, nil) + |> put_change(:duplicate_id, nil) end def cache_changeset(image) do diff --git a/lib/philomena/interactions.ex b/lib/philomena/interactions.ex index fce02254..1590e4e9 100644 --- a/lib/philomena/interactions.ex +++ b/lib/philomena/interactions.ex @@ -56,7 +56,7 @@ defmodule Philomena.Interactions do end def migrate_interactions(source, target) do - now = DateTime.utc_now() + now = NaiveDateTime.utc_now() |> NaiveDateTime.truncate(:second) source = Repo.preload(source, [:hiders, :favers, :upvoters, :downvoters]) new_hides = Enum.map(source.hiders, &%{image_id: target.id, user_id: &1.id, created_at: now}) @@ -88,7 +88,16 @@ defmodule Philomena.Interactions do |> Multi.run(:image, fn repo, %{hides: hides, faves: faves, upvotes: upvotes, downvotes: downvotes} -> image_query = where(Image, id: ^target.id) - repo.update_all(image_query, inc: [hides: hides, faves: faves, upvotes: upvotes, downvotes: downvotes, score: upvotes - downvotes]) + repo.update_all( + image_query, + inc: [ + hides_count: hides, + faves_count: faves, + upvotes_count: upvotes, + downvotes_count: downvotes, + score: upvotes - downvotes + ] + ) {:ok, nil} end) diff --git a/lib/philomena_web/controllers/duplicate_report/accept_controller.ex b/lib/philomena_web/controllers/duplicate_report/accept_controller.ex new file mode 100644 index 00000000..ff4c30c7 --- /dev/null +++ b/lib/philomena_web/controllers/duplicate_report/accept_controller.ex @@ -0,0 +1,17 @@ +defmodule PhilomenaWeb.DuplicateReport.AcceptController do + use PhilomenaWeb, :controller + + alias Philomena.DuplicateReports.DuplicateReport + alias Philomena.DuplicateReports + + plug PhilomenaWeb.CanaryMapPlug, create: :edit, delete: :edit + plug :load_and_authorize_resource, model: DuplicateReport, id_name: "duplicate_report_id", persisted: true + + def create(conn, _params) do + {:ok, _report} = DuplicateReports.accept_duplicate_report(conn.assigns.duplicate_report, conn.assigns.current_user) + + conn + |> put_flash(:info, "Successfully accepted report.") + |> redirect(external: conn.assigns.referrer) + end +end \ No newline at end of file diff --git a/lib/philomena_web/controllers/duplicate_report/accept_reverse_controller.ex b/lib/philomena_web/controllers/duplicate_report/accept_reverse_controller.ex new file mode 100644 index 00000000..a94eaa7d --- /dev/null +++ b/lib/philomena_web/controllers/duplicate_report/accept_reverse_controller.ex @@ -0,0 +1,17 @@ +defmodule PhilomenaWeb.DuplicateReport.AcceptReverseController do + use PhilomenaWeb, :controller + + alias Philomena.DuplicateReports.DuplicateReport + alias Philomena.DuplicateReports + + plug PhilomenaWeb.CanaryMapPlug, create: :edit, delete: :edit + plug :load_and_authorize_resource, model: DuplicateReport, id_name: "duplicate_report_id", persisted: true + + def create(conn, _params) do + {:ok, _report} = DuplicateReports.accept_reverse_duplicate_report(conn.assigns.duplicate_report, conn.assigns.current_user) + + conn + |> put_flash(:info, "Successfully accepted report in reverse.") + |> redirect(external: conn.assigns.referrer) + end +end \ No newline at end of file diff --git a/lib/philomena_web/controllers/duplicate_report/claim_controller.ex b/lib/philomena_web/controllers/duplicate_report/claim_controller.ex new file mode 100644 index 00000000..5fe66286 --- /dev/null +++ b/lib/philomena_web/controllers/duplicate_report/claim_controller.ex @@ -0,0 +1,25 @@ +defmodule PhilomenaWeb.DuplicateReport.ClaimController do + use PhilomenaWeb, :controller + + alias Philomena.DuplicateReports.DuplicateReport + alias Philomena.DuplicateReports + + plug PhilomenaWeb.CanaryMapPlug, create: :edit, delete: :edit + plug :load_and_authorize_resource, model: DuplicateReport, id_name: "duplicate_report_id", persisted: true + + def create(conn, _params) do + {:ok, _report} = DuplicateReports.claim_duplicate_report(conn.assigns.duplicate_report, conn.assigns.current_user) + + conn + |> put_flash(:info, "Successfully claimed report.") + |> redirect(external: conn.assigns.referrer) + end + + def delete(conn, _params) do + {:ok, _report} = DuplicateReports.unclaim_duplicate_report(conn.assigns.duplicate_report) + + conn + |> put_flash(:info, "Successfully released report.") + |> redirect(external: conn.assigns.referrer) + end +end \ No newline at end of file diff --git a/lib/philomena_web/controllers/duplicate_report/reject_controller.ex b/lib/philomena_web/controllers/duplicate_report/reject_controller.ex new file mode 100644 index 00000000..1baf2ab3 --- /dev/null +++ b/lib/philomena_web/controllers/duplicate_report/reject_controller.ex @@ -0,0 +1,19 @@ +defmodule PhilomenaWeb.DuplicateReport.RejectController do + use PhilomenaWeb, :controller + + alias Philomena.DuplicateReports.DuplicateReport + alias Philomena.DuplicateReports + alias Philomena.Images + + plug PhilomenaWeb.CanaryMapPlug, create: :edit, delete: :edit + plug :load_and_authorize_resource, model: DuplicateReport, id_name: "duplicate_report_id", persisted: true, preload: [:image] + + def create(conn, _params) do + {:ok, _report} = DuplicateReports.reject_duplicate_report(conn.assigns.duplicate_report, conn.assigns.current_user) + {:ok, _image} = Images.unhide_image(conn.assigns.duplicate_report.image) + + conn + |> put_flash(:info, "Successfully rejected report.") + |> redirect(external: conn.assigns.referrer) + end +end \ No newline at end of file diff --git a/lib/philomena_web/controllers/duplicate_report_controller.ex b/lib/philomena_web/controllers/duplicate_report_controller.ex index c46bff78..06773784 100644 --- a/lib/philomena_web/controllers/duplicate_report_controller.ex +++ b/lib/philomena_web/controllers/duplicate_report_controller.ex @@ -22,7 +22,7 @@ defmodule PhilomenaWeb.DuplicateReportController do duplicate_reports = DuplicateReport |> where([d], d.state in ^states) - |> preload([:user, image: [:user, :tags], duplicate_of_image: [:user, :tags]]) + |> preload([:user, :modifier, image: [:user, :tags], duplicate_of_image: [:user, :tags]]) |> order_by(desc: :created_at) |> Repo.paginate(conn.assigns.pagination) diff --git a/lib/philomena_web/controllers/image/reporting_controller.ex b/lib/philomena_web/controllers/image/reporting_controller.ex index b9d75f8d..f968c361 100644 --- a/lib/philomena_web/controllers/image/reporting_controller.ex +++ b/lib/philomena_web/controllers/image/reporting_controller.ex @@ -14,7 +14,7 @@ defmodule PhilomenaWeb.Image.ReportingController do dupe_reports = DuplicateReport - |> preload([:user, image: [:user, :tags], duplicate_of_image: [:user, :tags]]) + |> preload([:user, :modifier, image: [:user, :tags], duplicate_of_image: [:user, :tags]]) |> where([d], d.image_id == ^image.id or d.duplicate_of_image_id == ^image.id) |> Repo.all() diff --git a/lib/philomena_web/router.ex b/lib/philomena_web/router.ex index f75b74cd..be458411 100644 --- a/lib/philomena_web/router.ex +++ b/lib/philomena_web/router.ex @@ -173,6 +173,13 @@ defmodule PhilomenaWeb.Router do resources "/close", Report.CloseController, only: [:create], singleton: true end end + + resources "/duplicate_reports", DuplicateReportController, only: [] do + resources "/accept", DuplicateReport.AcceptController, only: [:create], singleton: true + resources "/accept_reverse", DuplicateReport.AcceptReverseController, only: [:create], singleton: true + resources "/reject", DuplicateReport.RejectController, only: [:create], singleton: true + resources "/claim", DuplicateReport.ClaimController, only: [:create, :delete], singleton: true + end end scope "/", PhilomenaWeb do diff --git a/lib/philomena_web/templates/duplicate_report/_image_cell.html.slime b/lib/philomena_web/templates/duplicate_report/_image_cell.html.slime index cdd0beaa..f7364217 100644 --- a/lib/philomena_web/templates/duplicate_report/_image_cell.html.slime +++ b/lib/philomena_web/templates/duplicate_report/_image_cell.html.slime @@ -16,14 +16,14 @@ p = render PhilomenaWeb.UserAttributionView, "_anon_user.html", object: @image, conn: @conn - /- if report.valid? && can_manage_dr - - if source - a href=duplicate_report_accept_reverse_path(report) data-method="post" + = if can?(@conn, :edit, @report) do + = if @source do + a href=Routes.duplicate_report_accept_reverse_path(@conn, :create, @report) data-method="post" button.button ' Keep Source i.fa.fa-arrow-left - else - a href=duplicate_report_accept_path(report) data-method="post" + a href=Routes.duplicate_report_accept_path(@conn, :create, @report) data-method="post" button.button i.fa.fa-arrow-right> | Keep Target diff --git a/lib/philomena_web/templates/duplicate_report/_list.html.slime b/lib/philomena_web/templates/duplicate_report/_list.html.slime index 1843789a..46a0914d 100644 --- a/lib/philomena_web/templates/duplicate_report/_list.html.slime +++ b/lib/philomena_web/templates/duplicate_report/_list.html.slime @@ -125,23 +125,25 @@ .flex.flex--column.grid--dupe-report-list__cell.border-vertical id="report_options_#{report.id}" .dr__status-options class=background_class - = String.capitalize(report.state) + => String.capitalize(report.state) - /- if can_manage_dr && report.modifier.present? - ' by + = if can?(@conn, :edit, report) and not is_nil(report.modifier) do + ' by = report.modifier.name - /- if can_manage_dr + + = if can?(@conn, :edit, report) do div - - if report.state == 'open' - a href=duplicate_report_claim_path(report, target: "report_options_#{report.id}") data-method="post" + = if report.state == "open" do + a href=(Routes.duplicate_report_claim_path(@conn, :create, report) <> "#report_options_#{report.id}") data-method="post" button.button.button--separate-right i.fa.fa-clipboard> - | Claim - - if report.state != 'rejected' - a href=duplicate_report_reject_path(report) data-method="post" + ' Claim + + = if report.state != "rejected" do + a href=Routes.duplicate_report_reject_path(@conn, :create, report) data-method="post" button.button i.fa.fa-times> - | Reject + ' Reject .dr__status-options div @@ -152,4 +154,4 @@ ' by =< link report.user.name, to: Routes.profile_path(@conn, :show, report.user) - = report.reason + = escape_nl2br(report.reason) diff --git a/lib/philomena_web/templates/image/_image_container.html.slime b/lib/philomena_web/templates/image/_image_container.html.slime index 6345c206..b7b78d2f 100644 --- a/lib/philomena_web/templates/image/_image_container.html.slime +++ b/lib/philomena_web/templates/image/_image_container.html.slime @@ -1,6 +1,6 @@ - link = assigns[:link] || Routes.image_path(@conn, :show, @image) -= image_container @image, @size, fn -> += image_container @conn, @image, @size, fn -> = cond do - @image.duplicate_id -> .media-box__overlay diff --git a/lib/philomena_web/templates/image/_image_target.html.slime b/lib/philomena_web/templates/image/_image_target.html.slime index ae48962e..607c2bbf 100644 --- a/lib/philomena_web/templates/image/_image_target.html.slime +++ b/lib/philomena_web/templates/image/_image_target.html.slime @@ -1,4 +1,4 @@ -= content_tag :div, [data: image_container_data(@image, :full), class: "image-show-container"] do += content_tag :div, [data: image_container_data(@conn, @image, :full), class: "image-show-container"] do .block.block--fixed.block--warning.block--no-margin.image-filtered.hidden strong = link("This image is blocked by your current filter - click here to display it anyway", to: "#", data: [click_unfilter: @image.id]) diff --git a/lib/philomena_web/templates/image/deleted.html.slime b/lib/philomena_web/templates/image/deleted.html.slime index a95f4674..d8d86f94 100644 --- a/lib/philomena_web/templates/image/deleted.html.slime +++ b/lib/philomena_web/templates/image/deleted.html.slime @@ -1,26 +1,36 @@ -.walloftext - .block.block--fixed.block--warning - h1 This image has been deleted - p - ' Reason: - strong - = @image.deletion_reason || "Unknown (likely deleted in error). Please contact a moderator." - - = if can?(@conn, :hide, @image) do += if @image.duplicate_id do + .walloftext + .block.block--fixed.block--warning + h1 This image has been merged into another image p - strong> Spoilers! - ' Done by: - strong = deleter(@image) - p - ' If you originally uploaded the file previously located here, please don't re-upload it - - => link "contact us", to: "/pages/contact" - ' if you feel this was in error and we'll talk! We're only human, and mistakes happen. - p - ' Here's the - => link "tagging guidelines", to: "/pages/tags" - ' and - = link "rules of the site", to: "/pages/rules" - ' . Other useful links can be found at the bottom of the page. + ' This image was merged into image + => link "##{@image.duplicate_id}", to: Routes.image_path(@conn, :show, @image.duplicate_id) + ' because it was determined to be a duplicate of that image. + +- else + .walloftext + .block.block--fixed.block--warning + h1 This image has been deleted + p + ' Reason: + strong + = @image.deletion_reason || "Unknown (likely deleted in error). Please contact a moderator." + + = if can?(@conn, :hide, @image) do + p + strong> Spoilers! + ' Done by: + strong = deleter(@image) + p + ' If you originally uploaded the file previously located here, please don't re-upload it - + => link "contact us", to: "/pages/contact" + ' if you feel this was in error and we'll talk! We're only human, and mistakes happen. + p + ' Here's the + => link "tagging guidelines", to: "/pages/tags" + ' and + = link "rules of the site", to: "/pages/rules" + ' . Other useful links can be found at the bottom of the page. = if can?(@conn, :hide, @image) do = render PhilomenaWeb.ImageView, "show.html", assigns \ No newline at end of file diff --git a/lib/philomena_web/views/app_view.ex b/lib/philomena_web/views/app_view.ex index 39df7621..b3b5ff3c 100644 --- a/lib/philomena_web/views/app_view.ex +++ b/lib/philomena_web/views/app_view.ex @@ -104,6 +104,7 @@ defmodule PhilomenaWeb.AppView do end) end + def escape_nl2br(nil), do: nil def escape_nl2br(text) do text |> String.split("\n") diff --git a/lib/philomena_web/views/image_view.ex b/lib/philomena_web/views/image_view.ex index 01eac6db..9277e0cf 100644 --- a/lib/philomena_web/views/image_view.ex +++ b/lib/philomena_web/views/image_view.ex @@ -9,7 +9,7 @@ defmodule PhilomenaWeb.ImageView do # this is a bit ridculous def render_intent(_conn, %{thumbnails_generated: false}, _size), do: :not_rendered def render_intent(conn, image, size) do - uris = thumb_urls(image, false) + uris = thumb_urls(image, can?(conn, :show, image)) vid? = image.image_mime_type == "video/webm" gif? = image.image_mime_type == "image/gif" tags = Tag.display_order(image.tags) |> Enum.map_join(", ", & &1.name) @@ -49,9 +49,19 @@ defmodule PhilomenaWeb.ImageView do tall: thumb_url(image, show_hidden, :tall), full: pretty_url(image, true, false) } + |> append_full_url(image, show_hidden) |> append_gif_urls(image, show_hidden) end + defp append_full_url(urls, %{hidden_from_users: false} = image, _show_hidden), + do: Map.put(urls, :full, pretty_url(image, true, false)) + + defp append_full_url(urls, %{hidden_from_users: true} = image, true), + do: Map.put(urls, :full, thumb_url(image, true, :full)) + + defp append_full_url(urls, _image, _show_hidden), + do: urls + defp append_gif_urls(urls, %{image_mime_type: "image/gif"} = image, show_hidden) do full_url = thumb_url(image, show_hidden, :full) @@ -104,7 +114,7 @@ defmodule PhilomenaWeb.ImageView do Application.get_env(:philomena, :image_url_root) end - def image_container_data(image, size) do + def image_container_data(conn, image, size) do [ image_id: image.id, image_tags: Jason.encode!(Enum.map(image.tags, & &1.id)), @@ -116,7 +126,7 @@ defmodule PhilomenaWeb.ImageView do comment_count: image.comments_count, created_at: NaiveDateTime.to_iso8601(image.created_at), source_url: image.source_url, - uris: Jason.encode!(thumb_urls(image, false)), + uris: Jason.encode!(thumb_urls(image, can?(conn, :show, image))), width: image.image_width, height: image.image_height, aspect_ratio: image.image_aspect_ratio, @@ -124,8 +134,8 @@ defmodule PhilomenaWeb.ImageView do ] end - def image_container(image, size, block) do - content_tag(:div, block.(), class: "image-container #{size}", data: image_container_data(image, size)) + def image_container(conn, image, size, block) do + content_tag(:div, block.(), class: "image-container #{size}", data: image_container_data(conn, image, size)) end def display_order(tags) do