From 2170961f47ccbde2477d1958dcda7046d91f2aac Mon Sep 17 00:00:00 2001 From: evazion Date: Thu, 27 Dec 2018 15:03:11 -0600 Subject: [PATCH] artists: improve prefilling of new artist form (#4028) * When creating an artist by clicking the '?' next to the artist tag in the tag list, prefill the new artist form by finding the artist's last upload and fetching its source data. Previously we filled the urls with the source of the artist's last upload, which was wrong because it was usually a direct image URL (#3078). * Fix the other names field not escaping spaces within names to underscores. * Fix the other names field being potentially prefilled with duplicate names. --- app/controllers/artists_controller.rb | 5 +++-- app/logical/sources/strategies/base.rb | 10 ++++++++++ app/models/artist.rb | 25 +++++++++++++++++-------- app/views/sources/_info.html.erb | 2 +- test/unit/artist_test.rb | 21 +++++++++++++++++++++ 5 files changed, 52 insertions(+), 11 deletions(-) diff --git a/app/controllers/artists_controller.rb b/app/controllers/artists_controller.rb index b06226798..10f240af9 100644 --- a/app/controllers/artists_controller.rb +++ b/app/controllers/artists_controller.rb @@ -6,7 +6,7 @@ class ArtistsController < ApplicationController before_action :load_artist, :only => [:ban, :unban, :show, :edit, :update, :destroy, :undelete] def new - @artist = Artist.new_with_defaults(artist_params) + @artist = Artist.new_with_defaults(artist_params(:new)) respond_with(@artist) end @@ -104,9 +104,10 @@ private sp.permit! end - def artist_params + def artist_params(context = nil) permitted_params = %i[name other_names other_names_string group_name url_string notes] permitted_params << :is_active if CurrentUser.is_builder? + permitted_params << :source if context == :new params.fetch(:artist, {}).permit(permitted_params) end diff --git a/app/logical/sources/strategies/base.rb b/app/logical/sources/strategies/base.rb index 822924c14..329f87c71 100644 --- a/app/logical/sources/strategies/base.rb +++ b/app/logical/sources/strategies/base.rb @@ -166,6 +166,16 @@ module Sources Artist.find_artists(normalize_for_artist_finder) end + # A new artist entry with suggested defaults for when the artist doesn't + # exist. Used in Artist.new_with_defaults to prefill the new artist form. + def new_artist + Artist.new( + name: unique_id, + other_names: [artist_name], + url_string: [profile_url, normalize_for_artist_finder].sort.uniq.join("\n") + ) + end + def file_url image_url end diff --git a/app/models/artist.rb b/app/models/artist.rb index c5379f531..ab3da1040 100644 --- a/app/models/artist.rb +++ b/app/models/artist.rb @@ -310,17 +310,26 @@ class Artist < ApplicationRecord end module FactoryMethods + # Make a new artist, fetching the defaults either from the given source, or + # from the source of the artist's last upload. def new_with_defaults(params) - Artist.new(params).tap do |artist| - if artist.name.present? - post = CurrentUser.without_safe_mode do - Post.tag_match("source:http #{artist.name}").where("true /* Artist.new_with_defaults */").first - end - unless post.nil? || post.source.blank? - artist.url_string = post.source - end + source = params.delete(:source) + + if source.blank? && params[:name].present? + CurrentUser.without_safe_mode do + post = Post.tag_match("source:http* #{params[:name]}").first + source = post.try(:source) end end + + if source.present? + artist = Sources::Strategies.find(source).new_artist + artist.attributes = params + else + artist = Artist.new(params) + end + + artist.tap(&:validate) # run before_validation callbacks to normalize the names end end diff --git a/app/views/sources/_info.html.erb b/app/views/sources/_info.html.erb index da23706df..17a424077 100644 --- a/app/views/sources/_info.html.erb +++ b/app/views/sources/_info.html.erb @@ -15,7 +15,7 @@ <%= link_to @source.artist_name, @source.profile_url, id: "source-info-artist-profile" %> <% if @source.artists.empty? %> - (<%= link_to "Create new artist", new_artist_path(artist: { name: @source.unique_id, other_names: @source.artist_name, url_string: [@source.profile_url, @source.normalize_for_artist_finder].uniq.sort.join("\n") }), id: "source-info-create-new-artist" %>) + (<%= link_to "Create new artist", new_artist_path(artist: { source: @source.canonical_url }), id: "source-info-create-new-artist" %>) <% else %> (