From acf024b0e1a35a75a3bfda75fbf8761d3e9e73e7 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Sat, 3 May 2025 21:35:02 +0200 Subject: [PATCH] Implement direct upload of import files with progress bar --- app/controllers/imports_controller.rb | 63 ++++++--- .../controllers/direct_upload_controller.js | 121 ++++++++++++++++-- app/views/imports/_form.html.erb | 10 +- spec/requests/imports_spec.rb | 33 ++++- 4 files changed, 192 insertions(+), 35 deletions(-) diff --git a/app/controllers/imports_controller.rb b/app/controllers/imports_controller.rb index 1a663c60..66d2d4fb 100644 --- a/app/controllers/imports_controller.rb +++ b/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 diff --git a/app/javascript/controllers/direct_upload_controller.js b/app/javascript/controllers/direct_upload_controller.js index 4885f3bd..ec6a31c9 100644 --- a/app/javascript/controllers/direct_upload_controller.js +++ b/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 - } } diff --git a/app/views/imports/_form.html.erb b/app/views/imports/_form.html.erb index 68e6e2de..35d2ec34 100644 --- a/app/views/imports/_form.html.erb +++ b/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| %>
<%= 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" } %> +
+ Files will be uploaded directly to storage. Please be patient during upload. +
diff --git a/spec/requests/imports_spec.rb b/spec/requests/imports_spec.rb index 8b155cb3..7b20f81a 100644 --- a/spec/requests/imports_spec.rb +++ b/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