From 0d359ee81e56caee232d5053f57f67d24ac78f90 Mon Sep 17 00:00:00 2001 From: "byte[]" Date: Sun, 6 Sep 2020 14:19:21 -0400 Subject: [PATCH] add tag alias validations --- lib/philomena/tags.ex | 118 ++++++++++-------- lib/philomena/tags/tag.ex | 53 ++++++++ .../controllers/tag/alias_controller.ex | 14 ++- .../templates/tag/alias/edit.html.slime | 2 + 4 files changed, 129 insertions(+), 58 deletions(-) diff --git a/lib/philomena/tags.ex b/lib/philomena/tags.ex index c49314b8..6e2c513d 100644 --- a/lib/philomena/tags.ex +++ b/lib/philomena/tags.ex @@ -180,64 +180,78 @@ defmodule Philomena.Tags do end def alias_tag(%Tag{} = tag, attrs) do - target_tag = Repo.get_by!(Tag, name: attrs["target_tag"]) + target_tag = Repo.get_by(Tag, name: String.downcase(attrs["target_tag"])) - if tag.id == target_tag.id do - tag - else - filters_hidden = - where(Filter, [f], fragment("? @> ARRAY[?]::integer[]", f.hidden_tag_ids, ^tag.id)) + tag + |> Repo.preload(:aliased_tag) + |> Tag.alias_changeset(target_tag) + |> Repo.update() + |> case do + {:ok, tag} -> + spawn(fn -> + perform_alias(tag, target_tag) + end) - filters_spoilered = - where(Filter, [f], fragment("? @> ARRAY[?]::integer[]", f.spoilered_tag_ids, ^tag.id)) + {:ok, tag} - users_watching = - where(User, [u], fragment("? @> ARRAY[?]::integer[]", u.watched_tag_ids, ^tag.id)) - - array_replace(filters_hidden, :hidden_tag_ids, tag.id, target_tag.id) - array_replace(filters_spoilered, :spoilered_tag_ids, tag.id, target_tag.id) - array_replace(users_watching, :watched_tag_ids, tag.id, target_tag.id) - - # Manual insert all because ecto won't do it for us - Repo.query!( - "INSERT INTO image_taggings (image_id, tag_id) " <> - "SELECT i.id, #{target_tag.id} FROM images i " <> - "INNER JOIN image_taggings it on it.image_id = i.id " <> - "WHERE it.tag_id = #{tag.id} " <> - "ON CONFLICT DO NOTHING" - ) - - # Delete taggings on the source tag - Tagging - |> where(tag_id: ^tag.id) - |> Repo.delete_all() - - # Update other assocations - UserLink - |> where(tag_id: ^tag.id) - |> Repo.update_all(set: [tag_id: target_tag.id]) - - DnpEntry - |> where(tag_id: ^tag.id) - |> Repo.update_all(set: [tag_id: target_tag.id]) - - Channel - |> where(associated_artist_tag_id: ^tag.id) - |> Repo.update_all(set: [associated_artist_tag_id: target_tag.id]) - - # Update counter - Tag - |> where(id: ^tag.id) - |> Repo.update_all( - set: [images_count: 0, aliased_tag_id: target_tag.id, updated_at: DateTime.utc_now()] - ) - - # Finally, reindex - reindex_tag_images(target_tag) - reindex_tags([tag, target_tag]) + error -> + error end end + defp perform_alias(tag, target_tag) do + filters_hidden = + where(Filter, [f], fragment("? @> ARRAY[?]::integer[]", f.hidden_tag_ids, ^tag.id)) + + filters_spoilered = + where(Filter, [f], fragment("? @> ARRAY[?]::integer[]", f.spoilered_tag_ids, ^tag.id)) + + users_watching = + where(User, [u], fragment("? @> ARRAY[?]::integer[]", u.watched_tag_ids, ^tag.id)) + + array_replace(filters_hidden, :hidden_tag_ids, tag.id, target_tag.id) + array_replace(filters_spoilered, :spoilered_tag_ids, tag.id, target_tag.id) + array_replace(users_watching, :watched_tag_ids, tag.id, target_tag.id) + + # Manual insert all because ecto won't do it for us + Repo.query!( + "INSERT INTO image_taggings (image_id, tag_id) " <> + "SELECT i.id, #{target_tag.id} FROM images i " <> + "INNER JOIN image_taggings it on it.image_id = i.id " <> + "WHERE it.tag_id = #{tag.id} " <> + "ON CONFLICT DO NOTHING" + ) + + # Delete taggings on the source tag + Tagging + |> where(tag_id: ^tag.id) + |> Repo.delete_all() + + # Update other assocations + UserLink + |> where(tag_id: ^tag.id) + |> Repo.update_all(set: [tag_id: target_tag.id]) + + DnpEntry + |> where(tag_id: ^tag.id) + |> Repo.update_all(set: [tag_id: target_tag.id]) + + Channel + |> where(associated_artist_tag_id: ^tag.id) + |> Repo.update_all(set: [associated_artist_tag_id: target_tag.id]) + + # Update counter + Tag + |> where(id: ^tag.id) + |> Repo.update_all( + set: [images_count: 0, aliased_tag_id: target_tag.id, updated_at: DateTime.utc_now()] + ) + + # Finally, reindex + reindex_tag_images(target_tag) + reindex_tags([tag, target_tag]) + end + def reindex_tag_images(%Tag{} = tag) do # First recount the tag image_count = diff --git a/lib/philomena/tags/tag.ex b/lib/philomena/tags/tag.ex index 8c2fba2c..e6144339 100644 --- a/lib/philomena/tags/tag.ex +++ b/lib/philomena/tags/tag.ex @@ -1,12 +1,14 @@ defmodule Philomena.Tags.Tag do use Ecto.Schema import Ecto.Changeset + import Ecto.Query alias Philomena.Channels.Channel alias Philomena.DnpEntries.DnpEntry alias Philomena.UserLinks.UserLink alias Philomena.Tags.Tag alias Philomena.Slug + alias Philomena.Repo @namespaces [ "artist", @@ -111,6 +113,15 @@ defmodule Philomena.Tags.Tag do |> put_change(:image, nil) end + def alias_changeset(tag, target_tag) do + change(tag) + |> put_assoc(:aliased_tag, target_tag) + |> validate_required([:aliased_tag]) + |> validate_not_aliased_to_self() + |> validate_alias_not_transitive() + |> validate_incoming_aliases() + end + def unalias_changeset(tag) do change(tag, aliased_tag_id: nil) end @@ -245,4 +256,46 @@ defmodule Philomena.Tags.Tag do category -> change(changeset, category: category) end end + + defp validate_not_aliased_to_self(changeset) do + aliased_tag = get_field(changeset, :aliased_tag) + id = get_field(changeset, :id) + + case aliased_tag do + %{id: ^id} -> + add_error(changeset, :aliased_tag, "is the same tag as the source") + + _tag -> + changeset + end + end + + defp validate_alias_not_transitive(changeset) do + case get_field(changeset, :aliased_tag) do + %{aliased_tag_id: tag} when not is_nil(tag) -> + add_error( + changeset, + :aliased_tag, + "is itself aliased and would create a transitive alias" + ) + + _tag -> + changeset + end + end + + defp validate_incoming_aliases(changeset) do + id = get_field(changeset, :id) + + count = + Tag + |> where(aliased_tag_id: ^id) + |> Repo.aggregate(:count, :id) + + if count > 0 do + add_error(changeset, :tag, "has incoming aliases and cannot be aliased") + else + changeset + end + end end diff --git a/lib/philomena_web/controllers/tag/alias_controller.ex b/lib/philomena_web/controllers/tag/alias_controller.ex index b4d8da60..cc130bc2 100644 --- a/lib/philomena_web/controllers/tag/alias_controller.ex +++ b/lib/philomena_web/controllers/tag/alias_controller.ex @@ -19,13 +19,15 @@ defmodule PhilomenaWeb.Tag.AliasController do end def update(conn, %{"tag" => tag_params}) do - spawn(fn -> - Tags.alias_tag(conn.assigns.tag, tag_params) - end) + case Tags.alias_tag(conn.assigns.tag, tag_params) do + {:ok, tag} -> + conn + |> put_flash(:info, "Tag alias queued.") + |> redirect(to: Routes.tag_alias_path(conn, :edit, tag)) - conn - |> put_flash(:info, "Tag alias queued.") - |> redirect(to: Routes.tag_alias_path(conn, :edit, conn.assigns.tag)) + {:error, changeset} -> + render(conn, "edit.html", changeset: changeset) + end end def delete(conn, _params) do diff --git a/lib/philomena_web/templates/tag/alias/edit.html.slime b/lib/philomena_web/templates/tag/alias/edit.html.slime index 463c6e64..706545bc 100644 --- a/lib/philomena_web/templates/tag/alias/edit.html.slime +++ b/lib/philomena_web/templates/tag/alias/edit.html.slime @@ -10,6 +10,8 @@ h1 .field => label f, "Alias target:" = text_input f, :target_tag, value: alias_target(@tag), class: "input" + = error_tag f, :tag + = error_tag f, :aliased_tag .field => submit "Alias tag", class: "button"