From 9f55c47269e1b5733534712949ae4d1a02829810 Mon Sep 17 00:00:00 2001 From: Zsolt Tasnadi Date: Sun, 23 Aug 2026 09:02:22 +0200 Subject: [PATCH] The uploaded picture is written after the row is committed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Image#process_upload` ran in a `before_save`: it assigned the generated file name and copied the bytes to disk, in the same breath, before the row existed. If the insert failed afterwards — a validation on the owning record, a uniqueness clash, a rollback from the surrounding transaction — the file stayed behind with nothing pointing at it. The admin's own "Orphan" scope exists to find rows in that family; this was the other half of it, the files with no row at all. Split in two: `before_validation` assigns the attributes (so validations and the generated name still see them), `after_commit` copies the bytes. A rollback now takes the file with it, because the copy never happens. The same model is generated into every host, so the fix goes into the generator template as well as ours. While there: `Image#url` comes back from Teletype Orbit, where it was added and never flowed back, and the two admin previews use it instead of interpolating `/api/image/#{id}` by hand. Co-Authored-By: Claude Opus 5 (1M context) --- apps/api/app/admin/images.rb | 6 +++--- apps/api/app/models/image.rb | 17 +++++++++++------ .../warp_engine/install/templates/image.rb | 15 ++++++++++----- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/apps/api/app/admin/images.rb b/apps/api/app/admin/images.rb index 4d06c4c..2bcfe61 100644 --- a/apps/api/app/admin/images.rb +++ b/apps/api/app/admin/images.rb @@ -4,7 +4,7 @@ ActiveAdmin.register Image do menu parent: "🌀 WarpEngine", priority: 5, label: "🖼️ Images" used_ids = -> { - WarpEngine::SoftwareImage.unscope(:order).distinct.pluck(:image_id) + + WarpEngine::SoftwareImage.distinct.pluck(:image_id) + Member.where.not(image_id: nil).distinct.pluck(:image_id) } @@ -28,7 +28,7 @@ ActiveAdmin.register Image do column :content_type column(:preview) do |img| if File.exist?(img.file_path) - image_tag("/api/image/#{img.id}", style: "max-height:60px;max-width:120px;object-fit:contain;") + image_tag(img.url, style: "max-height:60px;max-width:120px;object-fit:contain;") end end column(:usage) do |img| @@ -52,7 +52,7 @@ ActiveAdmin.register Image do row :content_type row(:preview) do |img| if File.exist?(img.file_path) - image_tag("/api/image/#{img.id}", style: "max-height:300px;max-width:100%;object-fit:contain;") + image_tag(img.url, style: "max-height:300px;max-width:100%;object-fit:contain;") else "File not found on disk" end diff --git a/apps/api/app/models/image.rb b/apps/api/app/models/image.rb index 07c80b6..23dfa8b 100644 --- a/apps/api/app/models/image.rb +++ b/apps/api/app/models/image.rb @@ -1,6 +1,6 @@ class Image < ApplicationRecord def self.upload_path - ENV.fetch("IMAGE_CONTAINER_PATH", "/images") + Rails.configuration.x.images.container_path end has_many :software_images, class_name: "WarpEngine::SoftwareImage", dependent: :restrict_with_error @@ -10,7 +10,8 @@ class Image < ApplicationRecord attr_accessor :file_upload - before_save :process_upload, if: -> { file_upload.present? } + before_validation :assign_upload_attributes, if: -> { file_upload.present? } + after_commit :store_upload_file, on: [ :create, :update ], if: -> { file_upload.present? } def self.ransackable_attributes(auth_object = nil) %w[content_type created_at deleted_at filename id original_filename updated_at] @@ -20,14 +21,18 @@ class Image < ApplicationRecord File.join(self.class.upload_path, filename.to_s) end + def url = "/api/image/#{id}" + private - def process_upload - FileUtils.mkdir_p(self.class.upload_path) + def assign_upload_attributes self.original_filename = file_upload.original_filename self.content_type = file_upload.content_type.presence || "application/octet-stream" - ext = File.extname(file_upload.original_filename) - self.filename = "#{SecureRandom.uuid}#{ext}" + self.filename = "#{SecureRandom.uuid}#{File.extname(file_upload.original_filename)}" + end + + def store_upload_file + FileUtils.mkdir_p(self.class.upload_path) IO.copy_stream(file_upload.to_io, file_path) end end diff --git a/libs/ruby/warp_engine/lib/generators/warp_engine/install/templates/image.rb b/libs/ruby/warp_engine/lib/generators/warp_engine/install/templates/image.rb index b2263dd..94fde24 100644 --- a/libs/ruby/warp_engine/lib/generators/warp_engine/install/templates/image.rb +++ b/libs/ruby/warp_engine/lib/generators/warp_engine/install/templates/image.rb @@ -9,7 +9,8 @@ class Image < ApplicationRecord attr_accessor :file_upload - before_save :process_upload, if: -> { file_upload.present? } + before_validation :assign_upload_attributes, if: -> { file_upload.present? } + after_commit :store_upload_file, on: [ :create, :update ], if: -> { file_upload.present? } def self.ransackable_attributes(auth_object = nil) %w[content_type created_at deleted_at filename id original_filename updated_at] @@ -19,14 +20,18 @@ class Image < ApplicationRecord File.join(self.class.upload_path, filename.to_s) end + def url = "/api/image/#{id}" + private - def process_upload - FileUtils.mkdir_p(self.class.upload_path) + def assign_upload_attributes self.original_filename = file_upload.original_filename self.content_type = file_upload.content_type.presence || "application/octet-stream" - ext = File.extname(file_upload.original_filename) - self.filename = "#{SecureRandom.uuid}#{ext}" + self.filename = "#{SecureRandom.uuid}#{File.extname(file_upload.original_filename)}" + end + + def store_upload_file + FileUtils.mkdir_p(self.class.upload_path) IO.copy_stream(file_upload.to_io, file_path) end end