Fix #4470: Check URLs for duplicates when creating artists

Show a warning when creating a duplicate artist; that is, when adding a
URL that already belongs to another artist.

This is a soft warning rather than a hard error because there are some
cases where multiple artists legitimately share the same site or account.

app/controllers/artists_controller.rb

@@ -46,13 +46,14 @@ class ArtistsController < ApplicationController
   def create
     @artist = authorize Artist.new(permitted_attributes(Artist))
     @artist.save
+    flash[:notice] = [*@artist.errors.full_messages, *@artist.warnings.full_messages].join(".\n \n") if @artist.warnings.any? || @artist.errors.any?
     respond_with(@artist)
   end
 
   def update
     @artist = authorize Artist.find(params[:id])
     @artist.update(permitted_attributes(@artist))
-    flash[:notice] = @artist.valid? ? "Artist updated" : @artist.errors.full_messages.join("; ")
+    flash[:notice] = [*@artist.errors.full_messages, *@artist.warnings.full_messages].join(".\n \n") if @artist.warnings.any? || @artist.errors.any?
     respond_with(@artist)
   end
 

app/models/artist.rb

@@ -15,6 +15,7 @@ class Artist < ApplicationRecord
 
   validate :validate_artist_name
   validates :name, tag_name: true, uniqueness: true
+  after_validation :add_url_warnings
 
   before_save :update_tag_category
   after_save :create_version
@@ -302,6 +303,12 @@ class Artist < ApplicationRecord
     end
   end
 
+  def add_url_warnings
+    urls.each do |url|
+      warnings.add(:base, url.warnings.full_messages.join("; ")) if url.warnings.any?
+    end
+  end
+
   include UrlMethods
   include VersionMethods
   extend FactoryMethods

app/models/artist_url.rb

@@ -5,6 +5,7 @@ class ArtistURL < ApplicationRecord
 
   validates :url, presence: true, uniqueness: { scope: :artist_id }
   validate :validate_url_format
+  validate :validate_url_is_not_duplicate
   belongs_to :artist, :touch => true
 
   scope :url_matches, ->(url) { url_attribute_matches(:url, url) }
@@ -140,6 +141,14 @@ class ArtistURL < ApplicationRecord
     errors.add(:url, "'#{uri}' is malformed: #{e}")
   end
 
+  def validate_url_is_not_duplicate
+    artists = ArtistFinder.find_artists(url).without(artist)
+
+    artists.each do |a|
+      warnings.add(:base, "Duplicate of [[#{a.name}]]")
+    end
+  end
+
   def self.available_includes
     [:artist]
   end

test/functional/artists_controller_test.rb

@@ -197,6 +197,15 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
           assert_equal("test", Artist.last.name)
         end
       end
+
+      should "warn about duplicate artists" do
+        create(:artist, name: "artist1", url_string: "https://www.pixiv.net/users/1234")
+        post_auth artists_path, @user, params: { artist: { name: "artist2", url_string: "https://www.pixiv.net/users/1234" }}
+
+        assert_response :redirect
+        assert_equal("artist2", Artist.last.name)
+        assert_equal("Duplicate of [[artist1]]", flash[:notice])
+      end
     end
 
     context "destroy action" do
@@ -213,6 +222,15 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest
         assert_redirected_to(artist_path(@artist.id))
         assert_equal(false, @artist.reload.is_deleted)
       end
+
+      should "warn about duplicate artists" do
+        @artist1 = create(:artist, name: "artist1", url_string: "https://www.pixiv.net/users/1234")
+        @artist2 = create(:artist, name: "artist2")
+        put_auth artist_path(@artist2), @user, params: { artist: { url_string: "https://www.pixiv.net/users/1234" }}
+
+        assert_redirected_to @artist2
+        assert_equal("Duplicate of [[artist1]]", flash[:notice])
+      end
     end
 
     context "revert action" do