From bdf83d1ffd8a9171047235e2f5269d5e8e156b64 Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 13 Feb 2022 16:55:26 -0600 Subject: [PATCH] uploads: refactor /uploads/:id page for multi-file uploads. --- app/controllers/application_controller.rb | 2 +- app/controllers/posts_controller.rb | 2 +- app/controllers/uploads_controller.rb | 8 +- app/models/post.rb | 10 +- app/models/upload.rb | 13 +-- app/models/upload_media_asset.rb | 13 +++ app/views/upload_media_assets/show.html.erb | 9 ++ .../uploads/_multiple_asset_upload.html.erb | 7 ++ .../uploads/_single_asset_upload.html.erb | 95 +++++++++++++++++ app/views/uploads/show.html.erb | 100 +----------------- test/factories/upload.rb | 6 +- test/factories/upload_media_asset.rb | 1 + 12 files changed, 150 insertions(+), 116 deletions(-) create mode 100644 app/views/upload_media_assets/show.html.erb create mode 100644 app/views/uploads/_multiple_asset_upload.html.erb create mode 100644 app/views/uploads/_single_asset_upload.html.erb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index daee00722..5366c3146 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,7 +2,7 @@ class ApplicationController < ActionController::Base include Pundit - helper_method :search_params + helper_method :search_params, :permitted_attributes self.responder = ApplicationResponder diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 728d72e89..bc49b5018 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -65,7 +65,7 @@ class PostsController < ApplicationController def create @upload_media_asset = UploadMediaAsset.find(params[:upload_media_asset_id]) - @post = authorize Post.new_from_upload(@upload_media_asset.upload, @upload_media_asset.media_asset, **permitted_attributes(Post).to_h.symbolize_keys) + @post = authorize Post.new_from_upload(@upload_media_asset, **permitted_attributes(Post).to_h.symbolize_keys) @post.save_if_unique(:md5) if @post.errors.none? diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index e4383b9bb..8fb9a3cd7 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -39,14 +39,8 @@ class UploadsController < ApplicationController def show @upload = authorize Upload.find(params[:id]) - @upload_media_asset = @upload.upload_media_assets.first - @media_asset = @upload_media_asset&.media_asset - @post = Post.new_from_upload(@upload, @media_asset, source: @upload.source, **permitted_attributes(Post).to_h.symbolize_keys) - @post.tag_string = "#{@post.tag_string} #{@upload.source_strategy&.artists.to_a.map(&:tag).map(&:name).join(" ")}".strip - @post.tag_string += " " if @post.tag_string.present? - - if request.format.html? && @upload.is_completed? && @upload.media_assets.first&.post.present? + if request.format.html? && @upload.media_asset_count == 1 && @upload.media_assets.first&.post.present? flash[:notice] = "Duplicate of post ##{@upload.media_assets.first.post.id}" redirect_to @upload.media_assets.first.post else diff --git a/app/models/post.rb b/app/models/post.rb index 4bcfb9940..6ba8f34ca 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -74,7 +74,10 @@ class Post < ApplicationRecord has_many :versions, -> { Rails.env.test? ? order("post_versions.updated_at ASC, post_versions.id ASC") : order("post_versions.updated_at ASC") }, class_name: "PostVersion", dependent: :destroy end - def self.new_from_upload(upload, media_asset, tag_string: nil, rating: nil, parent_id: nil, source: nil, artist_commentary_title: nil, artist_commentary_desc: nil, translated_commentary_title: nil, translated_commentary_desc: nil, is_pending: nil) + def self.new_from_upload(upload_media_asset, tag_string: nil, rating: nil, parent_id: nil, source: nil, artist_commentary_title: nil, artist_commentary_desc: nil, translated_commentary_title: nil, translated_commentary_desc: nil, is_pending: nil, add_artist_tag: false) + upload = upload_media_asset.upload + media_asset = upload_media_asset.media_asset + # XXX depends on CurrentUser commentary = ArtistCommentary.new( original_title: artist_commentary_title, @@ -83,6 +86,11 @@ class Post < ApplicationRecord translated_description: translated_commentary_desc, ) + if add_artist_tag + tag_string = "#{tag_string} #{upload_media_asset.source_strategy&.artists.to_a.map(&:tag).map(&:name).join(" ")}".strip + tag_string += " " if tag_string.present? + end + post = Post.new( uploader: upload.uploader, uploader_ip_addr: upload.uploader_ip_addr, diff --git a/app/models/upload.rb b/app/models/upload.rb index 912d6f74a..a431c818c 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class Upload < ApplicationRecord - extend Memoist - attr_accessor :file belongs_to :uploader, class_name: "User" @@ -45,6 +43,10 @@ class Upload < ApplicationRecord def is_errored? status == "error" end + + def is_finished? + is_completed? || is_errored? + end end concerning :ValidationMethods do @@ -65,11 +67,6 @@ class Upload < ApplicationRecord Addressable::URI.normalized_encode(url) end end - - def source_strategy - return nil if source.blank? - Sources::Strategies.find(source, referer_url) - end end def self.search(params) @@ -116,6 +113,4 @@ class Upload < ApplicationRecord def self.available_includes [:uploader, :upload_media_assets, :media_assets] end - - memoize :source_strategy end diff --git a/app/models/upload_media_asset.rb b/app/models/upload_media_asset.rb index 8cc5d646f..1b43f3999 100644 --- a/app/models/upload_media_asset.rb +++ b/app/models/upload_media_asset.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class UploadMediaAsset < ApplicationRecord + extend Memoist + belongs_to :upload belongs_to :media_asset, optional: true @@ -23,4 +25,15 @@ class UploadMediaAsset < ApplicationRecord q = search_attributes(params, :id, :created_at, :updated_at, :status, :source_url, :page_url, :error, :upload, :media_asset) q.apply_default_order(params) end + + def finished? + active? || failed? + end + + def source_strategy + return nil if source_url.blank? + Sources::Strategies.find(source_url, page_url) + end + + memoize :source_strategy end diff --git a/app/views/upload_media_assets/show.html.erb b/app/views/upload_media_assets/show.html.erb new file mode 100644 index 000000000..b2f75790a --- /dev/null +++ b/app/views/upload_media_assets/show.html.erb @@ -0,0 +1,9 @@ +
+
+

Upload

+ + <%= render "uploads/single_asset_upload", upload_media_asset: @upload_media_asset %> +
+
+ +<%= render "uploads/secondary_links" %> diff --git a/app/views/uploads/_multiple_asset_upload.html.erb b/app/views/uploads/_multiple_asset_upload.html.erb new file mode 100644 index 000000000..e11b1e934 --- /dev/null +++ b/app/views/uploads/_multiple_asset_upload.html.erb @@ -0,0 +1,7 @@ +<%= render(MediaAssetGalleryComponent.new) do |gallery| %> + <% upload.upload_media_assets.order(id: :asc).each do |upload_media_asset| %> + <% gallery.media_asset do %> + <%= render "upload_media_assets/preview", upload_media_asset: upload_media_asset, size: gallery.size %> + <% end %> + <% end %> +<% end %> diff --git a/app/views/uploads/_single_asset_upload.html.erb b/app/views/uploads/_single_asset_upload.html.erb new file mode 100644 index 000000000..8af446735 --- /dev/null +++ b/app/views/uploads/_single_asset_upload.html.erb @@ -0,0 +1,95 @@ +<% if CurrentUser.user.upload_limit.limited? %> +

You have reached your upload limit. Please wait for your pending uploads to be approved before uploading more.

+ +

Upload Limit: <%= render "users/upload_limit", user: CurrentUser.user %>

+<% elsif upload_media_asset.pending? %> +

Preparing to upload <%= external_link_to upload_media_asset.source_url %>...

+<% elsif upload_media_asset.processing? %> +

Processing <%= external_link_to upload_media_asset.source_url %>...

+<% elsif upload_media_asset.failed? %> +

Error: <%= upload_media_asset.error %>.

+<% else %> + <% upload = upload_media_asset.upload %> + <% media_asset = upload_media_asset.media_asset %> + + <%= embed_wiki("help:upload_notice", id: "upload-guide-notice") %> + + <% unless CurrentUser.can_upload_free? %> +

Upload Limit: <%= render "users/upload_limit", user: CurrentUser.user %>

+ <% end %> + + + +
+ <%= render MediaAssetComponent.new(media_asset: media_asset) %> + +

+ Size + <%= link_to media_asset.variant(:original).file_url do %> + <%= number_to_human_size(media_asset.file_size) %> .<%= media_asset.file_ext %> + <% end %> + (<%= media_asset.image_width %>x<%= media_asset.image_height %>) +

+
+ + <%= render "uploads/related_posts", source: upload_media_asset.source_strategy %> + + <% if upload_media_asset.source_strategy.present? %> + <%= render_source_data(upload_media_asset.source_strategy) %> + <% end %> + + <% post = Post.new_from_upload(upload_media_asset, add_artist_tag: true, source: upload_media_asset.source_strategy.canonical_url, **permitted_attributes(Post).to_h.symbolize_keys) %> + <%= edit_form_for(post, html: { id: "form" }) do |f| %> + <%= hidden_field_tag :media_asset_id, media_asset.id %> <%# used by iqdb javascript %> + <%= hidden_field_tag :upload_media_asset_id, upload_media_asset.id %> + + <%= f.input :source, as: :string, input_html: { value: post.source } %> + <%= f.input :rating, collection: [["Explicit", "e"], ["Questionable", "q"], ["Safe", "s"]], as: :radio_buttons, selected: post.rating %> + <%= f.input :parent_id, label: "Parent ID", as: :string, input_html: { value: post.parent_id } %> + +
+ Commentary + show » + + +
+ + + +
+
+ <%= f.label :tag_string, "Tags" %> + + + + + + + <%= external_link_icon(id: "open-edit-dialog", "data-shortcut": "shift+e") %> + +
+ + <%= f.input :tag_string, label: false, hint: "Ctrl+Enter to submit", input_html: { "data-autocomplete": "tag-edit", "data-shortcut": "e", value: post.tag_string } %> + <%= render "related_tags/buttons" %> +
+ + <%= f.submit "Post" %> + + <% if CurrentUser.can_upload_free? %> + <%= f.input :is_pending, as: :boolean, label: "Upload for approval", wrapper_html: { class: "inline-block" }, input_html: { checked: post.is_pending? } %> + <% end %> + + <%= render "related_tags/container" %> + <% end %> +<% end %> diff --git a/app/views/uploads/show.html.erb b/app/views/uploads/show.html.erb index 9897a594c..84bc33ae3 100644 --- a/app/views/uploads/show.html.erb +++ b/app/views/uploads/show.html.erb @@ -2,104 +2,14 @@

Upload

- <% if @upload.is_pending? || @upload.is_processing? %> - <% content_for(:html_header) do %> - - <% end %> - <% end %> - <% if @upload.is_errored? %>

Error: <%= @upload.error %>.

- <% elsif @upload.is_pending? && @upload.source.present? %> -

Preparing to upload <%= external_link_to @upload.source %>...

- <% elsif @upload.is_processing? && @upload.source.present? %> -

Processing <%= external_link_to @upload.source %>...

- <% elsif !@upload.is_completed? %> + <% elsif @upload.media_asset_count == 0 %>

Processing upload...

- <% elsif CurrentUser.user.upload_limit.limited? %> -

You have reached your upload limit. Please wait for your pending uploads to be approved before uploading more.

- -

Upload Limit: <%= render "users/upload_limit", user: CurrentUser.user %>

- <% else %> - <%= embed_wiki("help:upload_notice", id: "upload-guide-notice") %> - - <% unless CurrentUser.can_upload_free? %> -

Upload Limit: <%= render "users/upload_limit", user: CurrentUser.user %>

- <% end %> - - - -
- <%= render MediaAssetComponent.new(media_asset: @media_asset) %> - -

- Size - <%= link_to @media_asset.variant(:original).file_url do %> - <%= number_to_human_size(@media_asset.file_size) %> .<%= @media_asset.file_ext %> - <% end %> - (<%= @media_asset.image_width %>x<%= @media_asset.image_height %>) -

-
- - <%= render "uploads/related_posts", source: @upload.source_strategy %> - - <% if @upload.source_strategy.present? %> - <%= render_source_data(@upload.source_strategy) %> - <% end %> - - <%= edit_form_for(@post, html: { id: "form" }) do |f| %> - <%= hidden_field_tag :media_asset_id, @media_asset.id %> <%# used by iqdb javascript %> - <%= hidden_field_tag :upload_media_asset_id, @upload_media_asset.id %> - - <%= f.input :source, as: :string, input_html: { value: @upload.source_strategy&.canonical_url } %> - <%= f.input :rating, collection: [["Explicit", "e"], ["Questionable", "q"], ["Safe", "s"]], as: :radio_buttons, selected: @post.rating %> - <%= f.input :parent_id, label: "Parent ID", as: :string, input_html: { value: @post.parent_id } %> - -
- Commentary - show » - - -
- - - -
-
- <%= f.label :tag_string, "Tags" %> - - - - - - - <%= external_link_icon(id: "open-edit-dialog", "data-shortcut": "shift+e") %> - -
- - <%= f.input :tag_string, label: false, hint: "Ctrl+Enter to submit", input_html: { "data-autocomplete": "tag-edit", "data-shortcut": "e", value: @post.tag_string } %> - <%= render "related_tags/buttons" %> -
- - <%= f.submit "Post" %> - - <% if CurrentUser.can_upload_free? %> - <%= f.input :is_pending, as: :boolean, label: "Upload for approval", wrapper_html: { class: "inline-block" }, input_html: { checked: @post.is_pending? } %> - <% end %> - - <%= render "related_tags/container" %> - <% end %> + <% elsif @upload.media_asset_count > 1 %> + <%= render "multiple_asset_upload", upload: @upload %> + <% elsif @upload.media_asset_count == 1 %> + <%= render "single_asset_upload", upload_media_asset: @upload.upload_media_assets.first %> <% end %>
diff --git a/test/factories/upload.rb b/test/factories/upload.rb index c5ed3489c..31940aa64 100644 --- a/test/factories/upload.rb +++ b/test/factories/upload.rb @@ -9,16 +9,18 @@ FactoryBot.define do factory(:completed_source_upload) do status { "completed" } - upload_media_assets { [build(:upload_media_asset)] } + media_asset_count { 1 } + upload_media_assets { [build(:upload_media_asset, source_url: "https://example.com/file.jpg", status: "active")] } end factory(:completed_file_upload) do status { "completed" } source { nil } + media_asset_count { 1 } file { Rack::Test::UploadedFile.new("#{Rails.root}/test/files/test.jpg") } upload_media_assets do - [build(:upload_media_asset, media_asset: build(:media_asset, file: "test/files/test.jpg"))] + [build(:upload_media_asset, media_asset: build(:media_asset, file: "test/files/test.jpg"), source_url: "file://test.jpg", status: "active")] end end end diff --git a/test/factories/upload_media_asset.rb b/test/factories/upload_media_asset.rb index 6552fdc3c..a4b1ad922 100644 --- a/test/factories/upload_media_asset.rb +++ b/test/factories/upload_media_asset.rb @@ -2,5 +2,6 @@ FactoryBot.define do factory(:upload_media_asset) do upload media_asset + source_url { FFaker::Internet.http_url } end end