uploads: add column for error messages.

Change it so uploads store errors in an `error` column instead of in the
`status` field.

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}.`);
     }
   }
 

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

app/views/uploads/_table.html.erb

@@ -59,7 +59,7 @@
       <div>
         <strong>Error</strong>
         <span>
-          <%= upload.status.delete_prefix("error: ") %>
+          <%= upload.error %>
         </span>
       </div>
     <% end %>
@@ -72,11 +72,7 @@
   <% end %>
 
   <% t.column :status do |upload| %>
-    <% if upload.is_errored? %>
+    <%= upload.status %>
-      error
-    <% elsif !upload.is_completed? %>
-      <%= upload.status %>
-    <% end %>
   <% end %>
 <% end %>
 

app/views/uploads/show.html.erb

@@ -9,7 +9,7 @@
     <% end %>
 
     <% if @upload.is_errored? %>
-      <p><%= @upload.status %></p>
+      <p>Error: <%= @upload.error %>.</p>
     <% elsif @upload.is_pending? && @upload.source.present? %>
       <p>Preparing to upload <%= external_link_to @upload.source %>...</p>
     <% elsif @upload.is_processing? && @upload.source.present? %>

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

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');
 
 

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

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" }

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