add tag alias validations

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 =

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

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

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"