Implement direct upload of import files with progress bar

app/controllers/imports_controller.rb

@@ -31,28 +31,39 @@ class ImportsController < ApplicationController
   end
 
   def create
-    files = import_params[:files].reject(&:blank?)
+    raw_files = params.dig(:import, :files).reject(&:blank?)
 
-    files.each do |file|
-      import = current_user.imports.build(
-        name: file.original_filename,
-        source: params[:import][:source]
-      )
-
-      import.file.attach(io: file, filename: file.original_filename, content_type: file.content_type)
-
-      import.save!
+    if raw_files.empty?
+      redirect_to new_import_path, alert: 'No files were selected for upload', status: :unprocessable_entity
+      return
     end
 
-    redirect_to imports_url, notice: "#{files.size} files are queued to be imported in background", status: :see_other
-  rescue StandardError => e
-    Import.where(user: current_user, name: files.map(&:original_filename)).destroy_all
+    imports = raw_files.map do |item|
+      next if item.is_a?(ActionDispatch::Http::UploadedFile)
 
+      Rails.logger.debug "Processing signed ID: #{item[0..20]}..."
+
+      create_import_from_signed_id(item)
+    end
+
+    if imports.any?
+      redirect_to imports_url,
+                  notice: "#{imports.size} files are queued to be imported in background",
+                  status: :see_other
+    else
+      redirect_to new_import_path,
+                  alert: 'No valid file references were found. Please upload files using the file selector.',
+                  status: :unprocessable_entity
+    end
+  rescue StandardError => e
+    # Clean up recent imports if there was an error
+    Import.where(user: current_user).where('created_at > ?', 5.minutes.ago).destroy_all
+
+    Rails.logger.error "Import error: #{e.message}"
+    Rails.logger.error e.backtrace.join("\n")
     ExceptionReporter.call(e)
 
-    flash.now[:error] = e.message
-
-    redirect_to new_import_path, notice: e.message, status: :unprocessable_entity
+    redirect_to new_import_path, alert: e.message, status: :unprocessable_entity
   end
 
   def destroy
@@ -70,4 +81,24 @@ class ImportsController < ApplicationController
   def import_params
     params.require(:import).permit(:source, files: [])
   end
+
+  def create_import_from_signed_id(signed_id)
+    Rails.logger.debug "Creating import from signed ID: #{signed_id[0..20]}..."
+
+    # Find the blob using the signed ID
+    blob = ActiveStorage::Blob.find_signed(signed_id)
+
+    # Create the import
+    import = current_user.imports.build(
+      name: blob.filename.to_s,
+      source: params[:import][:source]
+    )
+
+    # Attach the blob to the import
+    import.file.attach(blob)
+
+    import.save!
+
+    import
+  end
 end

app/javascript/controllers/direct_upload_controller.js

@@ -2,41 +2,126 @@ import { Controller } from "@hotwired/stimulus"
 import { DirectUpload } from "@rails/activestorage"
 
 export default class extends Controller {
-  static targets = ["input", "progress", "submit"]
+  static targets = ["input", "progress", "progressBar", "submit", "form"]
   static values = {
     url: String
   }
 
   connect() {
     this.inputTarget.addEventListener("change", this.upload.bind(this))
+
+    // Add form submission handler to disable the file input
+    if (this.hasFormTarget) {
+      this.formTarget.addEventListener("submit", this.onSubmit.bind(this))
+    }
+  }
+
+  onSubmit(event) {
+    if (this.isUploading) {
+      // If still uploading, prevent submission
+      event.preventDefault()
+      console.log("Form submission prevented during upload")
+      return
+    }
+
+    // Disable the file input to prevent it from being submitted with the form
+    // This ensures only our hidden inputs with signed IDs are submitted
+    this.inputTarget.disabled = true
+
+    // Check if we have any signed IDs
+    const signedIds = this.element.querySelectorAll('input[name="import[files][]"][type="hidden"]')
+    if (signedIds.length === 0) {
+      event.preventDefault()
+      console.log("No files uploaded yet")
+      alert("Please select and upload files first")
+    } else {
+      console.log(`Submitting form with ${signedIds.length} uploaded files`)
+    }
   }
 
   upload() {
     const files = this.inputTarget.files
     if (files.length === 0) return
 
+    console.log(`Uploading ${files.length} files`)
+    this.isUploading = true
+
     // Disable submit button during upload
     this.submitTarget.disabled = true
 
-    // Create progress bar if it doesn't exist
-    if (!this.hasProgressTarget) {
-      const progressBar = document.createElement("div")
-      progressBar.setAttribute("data-direct-upload-target", "progress")
-      progressBar.className = "w-full bg-gray-200 rounded-full h-2.5 mt-2"
-      this.inputTarget.parentNode.appendChild(progressBar)
+    // Always remove any existing progress bar to ensure we create a fresh one
+    if (this.hasProgressTarget) {
+      this.progressTarget.remove()
     }
 
+    // Create a wrapper div for better positioning and visibility
+    const progressWrapper = document.createElement("div")
+    progressWrapper.className = "mt-4 mb-6 border p-4 rounded-lg bg-gray-50"
+
+    // Add a label
+    const progressLabel = document.createElement("div")
+    progressLabel.className = "font-medium mb-2 text-gray-700"
+    progressLabel.textContent = "Upload Progress"
+    progressWrapper.appendChild(progressLabel)
+
+    // Create a new progress container
+    const progressContainer = document.createElement("div")
+    progressContainer.setAttribute("data-direct-upload-target", "progress")
+    progressContainer.className = "w-full bg-gray-200 rounded-full h-4"
+
+    // Create the progress bar fill element
+    const progressBarFill = document.createElement("div")
+    progressBarFill.setAttribute("data-direct-upload-target", "progressBar")
+    progressBarFill.className = "bg-blue-600 h-4 rounded-full transition-all duration-300"
+    progressBarFill.style.width = "0%"
+
+    // Add the fill element to the container
+    progressContainer.appendChild(progressBarFill)
+    progressWrapper.appendChild(progressContainer)
+    progressBarFill.dataset.percentageDisplay = "true"
+
+    // Add the progress wrapper AFTER the file input field but BEFORE the submit button
+    this.submitTarget.parentNode.insertBefore(progressWrapper, this.submitTarget)
+
+    console.log("Progress bar created and inserted before submit button")
+
+    let uploadCount = 0
+    const totalFiles = files.length
+
+    // Clear any existing hidden fields for files
+    this.element.querySelectorAll('input[name="import[files][]"][type="hidden"]').forEach(el => {
+      if (el !== this.inputTarget) {
+        el.remove()
+      }
+    });
+
     Array.from(files).forEach(file => {
+      console.log(`Starting upload for ${file.name}`)
       const upload = new DirectUpload(file, this.urlValue, this)
       upload.create((error, blob) => {
+        uploadCount++
+
         if (error) {
           console.error("Error uploading file:", error)
         } else {
+          console.log(`Successfully uploaded ${file.name} with ID: ${blob.signed_id}`)
+
+          // Create a hidden field with the correct name
           const hiddenField = document.createElement("input")
           hiddenField.setAttribute("type", "hidden")
-          hiddenField.setAttribute("name", this.inputTarget.name)
+          hiddenField.setAttribute("name", "import[files][]")
           hiddenField.setAttribute("value", blob.signed_id)
           this.element.appendChild(hiddenField)
+
+          console.log("Added hidden field with signed ID:", blob.signed_id)
+        }
+
+        // Enable submit button when all uploads are complete
+        if (uploadCount === totalFiles) {
+          this.submitTarget.disabled = false
+          this.isUploading = false
+          console.log("All uploads completed")
+          console.log(`Ready to submit with ${this.element.querySelectorAll('input[name="import[files][]"][type="hidden"]').length} files`)
         }
       })
     })
@@ -44,13 +129,21 @@ export default class extends Controller {
 
   directUploadWillStoreFileWithXHR(request) {
     request.upload.addEventListener("progress", event => {
+      if (!this.hasProgressBarTarget) {
+        console.warn("Progress bar target not found")
+        return
+      }
+
       const progress = (event.loaded / event.total) * 100
-      this.progressTarget.style.width = `${progress}%`
+      const progressPercentage = `${progress.toFixed(1)}%`
+      console.log(`Upload progress: ${progressPercentage}`)
+      this.progressBarTarget.style.width = progressPercentage
+
+      // Update text percentage if exists
+      const percentageDisplay = this.element.querySelector('[data-percentage-display="true"]')
+      if (percentageDisplay) {
+        percentageDisplay.textContent = progressPercentage
+      }
     })
   }
-
-  directUploadDidProgress(event) {
-    // This method is called by ActiveStorage during the upload
-    // We're handling progress in directUploadWillStoreFileWithXHR instead
-  }
 }

app/views/imports/_form.html.erb

@@ -1,4 +1,8 @@
-<%= form_with model: import, class: "contents", data: { controller: "direct-upload", direct_upload_url_value: rails_direct_uploads_url } do |form| %>
+<%= form_with model: import, class: "contents", data: {
+  controller: "direct-upload",
+  direct_upload_url_value: rails_direct_uploads_url,
+  direct_upload_target: "form"
+} do |form| %>
   <div class="form-control w-full">
     <label class="label">
       <span class="label-text">Select source</span>
@@ -67,8 +71,12 @@
     </div>
     <%= form.file_field :files,
         multiple: true,
+        direct_upload: true,
         class: "file-input file-input-bordered w-full max-w-xs",
         data: { direct_upload_target: "input" } %>
+    <div class="text-sm text-gray-500 mt-2">
+      Files will be uploaded directly to storage. Please be patient during upload.
+    </div>
   </label>
 
   <div class="inline">

spec/requests/imports_spec.rb

@@ -42,16 +42,22 @@ RSpec.describe 'Imports', type: :request do
 
       context 'when importing owntracks data' do
         let(:file) { fixture_file_upload('owntracks/2024-03.rec', 'text/plain') }
+        let(:blob) { create_blob_for_file(file) }
+        let(:signed_id) { generate_signed_id_for_blob(blob) }
 
         it 'queues import job' do
+          allow(ActiveStorage::Blob).to receive(:find_signed).with(signed_id).and_return(blob)
+
           expect do
-            post imports_path, params: { import: { source: 'owntracks', files: [file] } }
+            post imports_path, params: { import: { source: 'owntracks', files: [signed_id] } }
           end.to have_enqueued_job(Import::ProcessJob).on_queue('imports').at_least(1).times
         end
 
         it 'creates a new import' do
+          allow(ActiveStorage::Blob).to receive(:find_signed).with(signed_id).and_return(blob)
+
           expect do
-            post imports_path, params: { import: { source: 'owntracks', files: [file] } }
+            post imports_path, params: { import: { source: 'owntracks', files: [signed_id] } }
           end.to change(user.imports, :count).by(1)
 
           expect(response).to redirect_to(imports_path)
@@ -60,16 +66,22 @@ RSpec.describe 'Imports', type: :request do
 
       context 'when importing gpx data' do
         let(:file) { fixture_file_upload('gpx/gpx_track_single_segment.gpx', 'application/gpx+xml') }
+        let(:blob) { create_blob_for_file(file) }
+        let(:signed_id) { generate_signed_id_for_blob(blob) }
 
         it 'queues import job' do
+          allow(ActiveStorage::Blob).to receive(:find_signed).with(signed_id).and_return(blob)
+
           expect do
-            post imports_path, params: { import: { source: 'gpx', files: [file] } }
+            post imports_path, params: { import: { source: 'gpx', files: [signed_id] } }
           end.to have_enqueued_job(Import::ProcessJob).on_queue('imports').at_least(1).times
         end
 
         it 'creates a new import' do
+          allow(ActiveStorage::Blob).to receive(:find_signed).with(signed_id).and_return(blob)
+
           expect do
-            post imports_path, params: { import: { source: 'gpx', files: [file] } }
+            post imports_path, params: { import: { source: 'gpx', files: [signed_id] } }
           end.to change(user.imports, :count).by(1)
 
           expect(response).to redirect_to(imports_path)
@@ -138,4 +150,17 @@ RSpec.describe 'Imports', type: :request do
       end
     end
   end
+
+  # Helper methods for creating ActiveStorage blobs and signed IDs in tests
+  def create_blob_for_file(file)
+    ActiveStorage::Blob.create_and_upload!(
+      io: file.open,
+      filename: file.original_filename,
+      content_type: file.content_type
+    )
+  end
+
+  def generate_signed_id_for_blob(blob)
+    blob.signed_id
+  end
 end