diff --git a/app/components/file_upload_component/file_upload_component.js b/app/components/file_upload_component/file_upload_component.js index 60ecd6f26..265656634 100644 --- a/app/components/file_upload_component/file_upload_component.js +++ b/app/components/file_upload_component/file_upload_component.js @@ -115,12 +115,12 @@ export default class FileUploadComponent { } else { window.location.assign(url); } - } else { + } else if (upload.status === "error") { this.$dropzone.removeClass("success"); this.$component.find("progress").addClass("hidden"); this.$component.find("input").removeAttr("disabled"); - Utility.error(upload.status); + Utility.error(`Upload failed: ${upload.error}.`); } } diff --git a/app/models/upload.rb b/app/models/upload.rb index da76e3cee..ec8c6449f 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -20,6 +20,7 @@ class Upload < ApplicationRecord scope :pending, -> { where(status: "pending") } scope :preprocessed, -> { where(status: "preprocessed") } scope :completed, -> { where(status: "completed") } + scope :failed, -> { where(status: "error") } def self.visible(user) if user.is_admin? @@ -43,7 +44,7 @@ class Upload < ApplicationRecord end def is_errored? - status.match?(/error:/) + status == "error" end end @@ -102,7 +103,7 @@ class Upload < ApplicationRecord media_asset = MediaAsset.upload!(media_file) update!(media_assets: [media_asset], status: "completed") rescue Exception => e - update!(status: "error: #{e.message}") + update!(status: "error", error: e.message) end def self.available_includes diff --git a/app/views/uploads/_table.html.erb b/app/views/uploads/_table.html.erb index 856755125..033ac407e 100644 --- a/app/views/uploads/_table.html.erb +++ b/app/views/uploads/_table.html.erb @@ -59,7 +59,7 @@
Error - <%= upload.status.delete_prefix("error: ") %> + <%= upload.error %>
<% end %> @@ -72,11 +72,7 @@ <% end %> <% t.column :status do |upload| %> - <% if upload.is_errored? %> - error - <% elsif !upload.is_completed? %> - <%= upload.status %> - <% end %> + <%= upload.status %> <% end %> <% end %> diff --git a/app/views/uploads/show.html.erb b/app/views/uploads/show.html.erb index ab7a3bbfd..0afdb959d 100644 --- a/app/views/uploads/show.html.erb +++ b/app/views/uploads/show.html.erb @@ -9,7 +9,7 @@ <% end %> <% if @upload.is_errored? %> -

<%= @upload.status %>

+

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? %> diff --git a/db/migrate/20220207195123_add_error_to_uploads.rb b/db/migrate/20220207195123_add_error_to_uploads.rb new file mode 100644 index 000000000..0a15da3a2 --- /dev/null +++ b/db/migrate/20220207195123_add_error_to_uploads.rb @@ -0,0 +1,6 @@ +class AddErrorToUploads < ActiveRecord::Migration[7.0] + def change + add_column :uploads, :error, :text + add_index :uploads, :error, where: "error IS NOT NULL" + end +end diff --git a/db/structure.sql b/db/structure.sql index 33dbd4943..a3b36dcce 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1950,7 +1950,8 @@ CREATE TABLE public.uploads ( status text DEFAULT 'pending'::text NOT NULL, created_at timestamp without time zone NOT NULL, updated_at timestamp without time zone NOT NULL, - referer_url text + referer_url text, + error text ); @@ -4534,6 +4535,13 @@ CREATE INDEX index_upload_media_assets_on_media_asset_id ON public.upload_media_ CREATE INDEX index_upload_media_assets_on_upload_id ON public.upload_media_assets USING btree (upload_id); +-- +-- Name: index_uploads_on_error; Type: INDEX; Schema: public; Owner: - +-- + +CREATE INDEX index_uploads_on_error ON public.uploads USING btree (error) WHERE (error IS NOT NULL); + + -- -- Name: index_uploads_on_referer_url; Type: INDEX; Schema: public; Owner: - -- @@ -5741,6 +5749,7 @@ INSERT INTO "schema_migrations" (version) VALUES ('20220120233850'), ('20220124195900'), ('20220203040648'), -('20220204075610'); +('20220204075610'), +('20220207195123'); diff --git a/script/fixes/100_migrate_upload_errors.rb b/script/fixes/100_migrate_upload_errors.rb new file mode 100755 index 000000000..0bf4f185d --- /dev/null +++ b/script/fixes/100_migrate_upload_errors.rb @@ -0,0 +1,11 @@ +#!/usr/bin/env ruby + +require_relative "base" + +with_confirmation do + Upload.where("status ~ '^error:'").find_each do |upload| + message = upload.status.delete_prefix("error: ").strip + upload.update_columns(status: "error", error: message) + puts({ id: upload.id, status: "error", error: message }.to_json) + end +end diff --git a/test/factories/upload.rb b/test/factories/upload.rb index e105133a8..c5ed3489c 100644 --- a/test/factories/upload.rb +++ b/test/factories/upload.rb @@ -5,6 +5,7 @@ FactoryBot.define do status { "pending" } source { "https://cdn.donmai.us/original/d3/4e/d34e4cf0a437a5d65f8e82b7bcd02606.jpg" } + error { nil } factory(:completed_source_upload) do status { "completed" } diff --git a/test/functional/uploads_controller_test.rb b/test/functional/uploads_controller_test.rb index b2bba1319..0063f8959 100644 --- a/test/functional/uploads_controller_test.rb +++ b/test/functional/uploads_controller_test.rb @@ -111,7 +111,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest end should "render a failed upload" do - upload = create(:upload, uploader: @user, status: "error: Not an image or video") + upload = create(:upload, uploader: @user, status: "error", error: "Not an image or video") get_auth upload_path(upload), @user assert_response :success @@ -174,7 +174,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest post_auth uploads_path(format: :json), @user, params: { upload: { file: file }} assert_response 201 - assert_match("Not an image or video", Upload.last.status) + assert_match("Not an image or video", Upload.last.error) end should "fail if the file size is too large" do @@ -186,7 +186,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest perform_enqueued_jobs assert_response 201 - assert_match("File size must be less than or equal to", Upload.last.status) + assert_match("File size must be less than or equal to", Upload.last.error) Danbooru.config.unstub(:max_file_size) end @@ -194,18 +194,18 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest context "for a corrupted image" do should "fail for a corrupted jpeg" do create_upload!("test/files/test-corrupt.jpg", user: @user) - assert_match("corrupt", Upload.last.status) + assert_match("corrupt", Upload.last.error) end should "fail for a corrupted gif" do create_upload!("test/files/test-corrupt.gif", user: @user) - assert_match("corrupt", Upload.last.status) + assert_match("corrupt", Upload.last.error) end # https://schaik.com/pngsuite/pngsuite_xxx_png.html should "fail for a corrupted png" do create_upload!("test/files/test-corrupt.png", user: @user) - assert_match("corrupt", Upload.last.status) + assert_match("corrupt", Upload.last.error) end end @@ -216,7 +216,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest perform_enqueued_jobs assert_response 201 - assert_match("Duration must be less than", Upload.last.status) + assert_match("Duration must be less than", Upload.last.error) end end @@ -241,7 +241,7 @@ class UploadsControllerTest < ActionDispatch::IntegrationTest upload = Upload.last assert_redirected_to upload - assert_match("Upload failed, try again", upload.reload.status) + assert_match("Upload failed, try again", upload.reload.error) assert_equal("failed", asset.reload.status) end end