Fix import regression for ActiveStorage::VariantRecord attachments

app/models/account/data_transfer/active_storage/attachment_record_set.rb

@@ -8,4 +8,24 @@ def records
       ::ActiveStorage::Attachment.where(account: account)
         .where.not(record_type: INTERNAL_RECORD_TYPES)
     end
+
+    def check_record(file_path)
+      data = load(file_path)
+      return if data["record_type"].in?(INTERNAL_RECORD_TYPES)
+
+      super
+    end
+
+    def import_batch(files)
+      batch_data = files.filter_map do |file|
+        data = load(file)
+        next if data["record_type"].in?(INTERNAL_RECORD_TYPES)
+
+        data.slice(*attributes).merge("account_id" => account.id).tap do |record_data|
+          record_data["updated_at"] = Time.current if record_data.key?("updated_at")
+        end
+      end
+
+      model.insert_all!(batch_data) if batch_data.any?
+    end
 end

app/models/account/data_transfer/active_storage/blob_record_set.rb

@@ -17,15 +17,33 @@ def excluded_blob_ids
     end
 
     def import_batch(files)
-      batch_data = files.map do |file|
+      batch_data = files.filter_map do |file|
         data = load(file)
+        next if internal_blob_ids.include?(data["id"])
+
         data.slice(*attributes).merge(
           "account_id" => account.id,
           "key" => ::ActiveStorage::Blob.generate_unique_secure_token(length: ::ActiveStorage::Blob::MINIMUM_TOKEN_LENGTH),
           "service_name" => ::ActiveStorage::Blob.service.name
         )
       end
 
-      model.insert_all!(batch_data)
+      model.insert_all!(batch_data) if batch_data.any?
+    end
+
+    def internal_blob_ids
+      @internal_blob_ids ||= build_internal_blob_ids
+    end
+
+    def build_internal_blob_ids
+      zip.glob("data/active_storage_attachments/*.json").each_with_object(Set.new) do |file, ids|
+        data = load(file)
+        ids << data["blob_id"] if data["record_type"].in?(INTERNAL_RECORD_TYPES)
+      end
+    end
+
+    def with_zip(zip)
+      @internal_blob_ids = nil
+      super
     end
 end

app/models/account/data_transfer/active_storage/file_record_set.rb

@@ -29,6 +29,7 @@ def import_batch(files)
         old_key = file.delete_prefix("storage/")
         blob_id = old_key_to_blob_id[old_key]
         raise IntegrityError, "Storage file #{old_key} has no matching blob metadata in export" unless blob_id
+        next if internal_blob_ids.include?(blob_id)
 
         blob = ::ActiveStorage::Blob.find_by(id: blob_id, account: account)
         raise IntegrityError, "Blob #{blob_id} not found for storage key #{old_key}" unless blob
@@ -54,16 +55,29 @@ def build_old_key_to_blob_id
       end
     end
 
-    def with_zip(zip)
-      @old_key_to_blob_id = nil
-      super
-    end
-
     def check_record(file_path)
       old_key = file_path.delete_prefix("storage/")
+      blob_id = old_key_to_blob_id[old_key]
 
-      unless old_key_to_blob_id.key?(old_key)
+      unless blob_id
         raise IntegrityError, "Storage file #{old_key} has no matching blob metadata in export"
       end
     end
+
+    def internal_blob_ids
+      @internal_blob_ids ||= build_internal_blob_ids
+    end
+
+    def build_internal_blob_ids
+      zip.glob("data/active_storage_attachments/*.json").each_with_object(Set.new) do |file, ids|
+        data = load(file)
+        ids << data["blob_id"] if data["record_type"].in?(INTERNAL_RECORD_TYPES)
+      end
+    end
+
+    def with_zip(zip)
+      @internal_blob_ids = nil
+      @old_key_to_blob_id = nil
+      super
+    end
 end

app/models/account/data_transfer/record_set.rb

@@ -3,7 +3,7 @@ class IntegrityError < StandardError; end
   class ConflictError < IntegrityError; end
 
   IMPORT_BATCH_SIZE = 100
-  INTERNAL_RECORD_TYPES = %w[Export Account::Import].freeze
+  INTERNAL_RECORD_TYPES = %w[Export Account::Import ActiveStorage::VariantRecord].freeze
 
   attr_accessor :importable_model_names
   attr_reader :account, :model, :attributes

test/models/account/data_transfer/active_storage/attachment_record_set_test.rb

@@ -0,0 +1,66 @@
+require "test_helper"
+
+class Account::DataTransfer::ActiveStorage::AttachmentRecordSetTest < ActiveSupport::TestCase
+  test "check skips attachments for internal record types" do
+    attachment_data = build_attachment_data(record_type: "ActiveStorage::VariantRecord")
+
+    record_set = Account::DataTransfer::ActiveStorage::AttachmentRecordSet.new(importing_account)
+    record_set.importable_model_names = %w[ActiveStorage::Attachment ActiveStorage::Blob Card]
+
+    assert_nothing_raised do
+      record_set.check(from: build_reader(data: attachment_data))
+    end
+  end
+
+  test "import skips attachments for internal record types" do
+    variant_attachment = build_attachment_data(record_type: "ActiveStorage::VariantRecord")
+    card_attachment = build_attachment_data(record_type: "Card")
+
+    record_set = Account::DataTransfer::ActiveStorage::AttachmentRecordSet.new(importing_account)
+
+    record_set.import(from: build_reader(data: [ variant_attachment, card_attachment ]))
+
+    assert_not ActiveStorage::Attachment.exists?(id: variant_attachment["id"])
+    assert ActiveStorage::Attachment.exists?(id: card_attachment["id"])
+  end
+
+  private
+    def importing_account
+      @importing_account ||= Account.create!(name: "Importing Account", external_account_id: 88888888)
+    end
+
+    def build_attachment_data(record_type:)
+      {
+        "id" => ActiveRecord::Type::Uuid.generate,
+        "account_id" => ActiveRecord::Type::Uuid.generate,
+        "blob_id" => ActiveRecord::Type::Uuid.generate,
+        "created_at" => Time.current.iso8601,
+        "name" => "file",
+        "record_id" => ActiveRecord::Type::Uuid.generate,
+        "record_type" => record_type
+      }
+    end
+
+    def build_reader(data:)
+      data = Array.wrap(data)
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+      data.each do |attachment|
+        writer.add_file("data/active_storage_attachments/#{attachment['id']}.json", attachment.to_json)
+      end
+      writer.close
+
+      tempfile.rewind
+      @tempfiles ||= []
+      @tempfiles << tempfile
+
+      ZipFile::Reader.new(tempfile)
+    end
+
+    def teardown
+      @tempfiles&.each { |f| f.close; f.unlink }
+      @importing_account&.destroy
+    end
+end

test/models/account/data_transfer/active_storage/blob_record_set_test.rb

@@ -13,6 +13,29 @@ class Account::DataTransfer::ActiveStorage::BlobRecordSetTest < ActiveSupport::T
     assert_equal 28, blob.key.length
   end
 
+  test "import skips blobs referenced only by internal record type attachments" do
+    internal_blob_id = ActiveRecord::Type::Uuid.generate
+    normal_blob_id = ActiveRecord::Type::Uuid.generate
+
+    zip = build_zip_with_blobs_and_attachments(
+      blobs: [
+        { id: internal_blob_id, key: "internal-key", filename: "variant.jpg" },
+        { id: normal_blob_id, key: "normal-key", filename: "photo.jpg" }
+      ],
+      attachments: [
+        { id: ActiveRecord::Type::Uuid.generate, blob_id: internal_blob_id, record_type: "ActiveStorage::VariantRecord" },
+        { id: ActiveRecord::Type::Uuid.generate, blob_id: normal_blob_id, record_type: "Card" }
+      ]
+    )
+
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+
+    assert_not ActiveStorage::Blob.exists?(id: internal_blob_id),
+      "Should not import blob for internal record type"
+    assert ActiveStorage::Blob.exists?(id: normal_blob_id),
+      "Should still import blob for non-internal record type"
+  end
+
   test "import preserves blob metadata" do
     blob_id = ActiveRecord::Type::Uuid.generate
 
@@ -55,4 +78,42 @@ def build_zip_with_blob(id:, key:, filename: "test.txt", content_type: "text/pla
       tempfile.rewind
       ZipFile::Reader.new(tempfile)
     end
+
+    def build_zip_with_blobs_and_attachments(blobs:, attachments:)
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+
+      blobs.each do |blob|
+        writer.add_file("data/active_storage_blobs/#{blob[:id]}.json", {
+          id: blob[:id],
+          account_id: ActiveRecord::Type::Uuid.generate,
+          byte_size: 32,
+          checksum: "",
+          content_type: "image/jpeg",
+          created_at: Time.current.iso8601,
+          filename: blob[:filename],
+          key: blob[:key],
+          metadata: {}
+        }.to_json)
+      end
+
+      attachments.each do |attachment|
+        writer.add_file("data/active_storage_attachments/#{attachment[:id]}.json", {
+          id: attachment[:id],
+          account_id: ActiveRecord::Type::Uuid.generate,
+          blob_id: attachment[:blob_id],
+          record_type: attachment[:record_type],
+          record_id: ActiveRecord::Type::Uuid.generate,
+          name: "file",
+          created_at: Time.current.iso8601
+        }.to_json)
+      end
+
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
 end

test/models/account/data_transfer/active_storage/file_record_set_test.rb

@@ -77,6 +77,50 @@ class Account::DataTransfer::ActiveStorage::FileRecordSetTest < ActiveSupport::T
     end
   end
 
+  test "check skips storage files for internal record type blobs" do
+    internal_blob_id = ActiveRecord::Type::Uuid.generate
+    normal_blob_id = ActiveRecord::Type::Uuid.generate
+
+    zip = build_zip_with_internal_and_normal_blobs(
+      internal_blob_id: internal_blob_id,
+      internal_key: "internal-variant-key",
+      normal_blob_id: normal_blob_id,
+      normal_key: "normal-photo-key"
+    )
+
+    # check should not raise for the internal blob's storage file
+    assert_nothing_raised do
+      Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).check(from: zip)
+    end
+  end
+
+  test "import skips uploading storage files for internal record type blobs" do
+    internal_blob_id = ActiveRecord::Type::Uuid.generate
+    normal_blob_id = ActiveRecord::Type::Uuid.generate
+    normal_content = "normal file content"
+
+    zip = build_zip_with_internal_and_normal_blobs(
+      internal_blob_id: internal_blob_id,
+      internal_key: "internal-variant-key",
+      normal_blob_id: normal_blob_id,
+      normal_key: "normal-photo-key",
+      normal_file_content: normal_content
+    )
+
+    # Import only the normal blob (internal one should be skipped by BlobRecordSet too)
+    Account::DataTransfer::ActiveStorage::BlobRecordSet.new(Current.account).import(from: zip)
+
+    assert_not ActiveStorage::Blob.exists?(id: internal_blob_id),
+      "Internal blob should not have been imported"
+    assert ActiveStorage::Blob.exists?(id: normal_blob_id),
+      "Normal blob should have been imported"
+
+    Account::DataTransfer::ActiveStorage::FileRecordSet.new(Current.account).import(from: zip)
+
+    blob = ActiveStorage::Blob.find(normal_blob_id)
+    assert_equal normal_content, blob.download
+  end
+
   test "import raises IntegrityError for duplicate blob keys in export" do
     blob_id_1 = ActiveRecord::Type::Uuid.generate
     blob_id_2 = ActiveRecord::Type::Uuid.generate
@@ -141,6 +185,70 @@ def build_zip_with_orphaned_storage_file(blob_id:, old_key:, orphan_key:)
       ZipFile::Reader.new(tempfile)
     end
 
+    def build_zip_with_internal_and_normal_blobs(internal_blob_id:, internal_key:, normal_blob_id:, normal_key:, normal_file_content: "normal data")
+      tempfile = Tempfile.new([ "test_export", ".zip" ])
+      tempfile.binmode
+
+      writer = ZipFile::Writer.new(tempfile)
+
+      # Blob metadata JSONs
+      writer.add_file("data/active_storage_blobs/#{internal_blob_id}.json", {
+        id: internal_blob_id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: 10,
+        checksum: "",
+        content_type: "image/jpeg",
+        created_at: Time.current.iso8601,
+        filename: "variant.jpg",
+        key: internal_key,
+        metadata: {}
+      }.to_json)
+
+      writer.add_file("data/active_storage_blobs/#{normal_blob_id}.json", {
+        id: normal_blob_id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        byte_size: normal_file_content.bytesize,
+        checksum: Digest::MD5.base64digest(normal_file_content),
+        content_type: "image/jpeg",
+        created_at: Time.current.iso8601,
+        filename: "photo.jpg",
+        key: normal_key,
+        metadata: {}
+      }.to_json)
+
+      # Attachment metadata JSONs
+      internal_attachment_id = ActiveRecord::Type::Uuid.generate
+      writer.add_file("data/active_storage_attachments/#{internal_attachment_id}.json", {
+        id: internal_attachment_id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        blob_id: internal_blob_id,
+        record_type: "ActiveStorage::VariantRecord",
+        record_id: ActiveRecord::Type::Uuid.generate,
+        name: "file",
+        created_at: Time.current.iso8601
+      }.to_json)
+
+      normal_attachment_id = ActiveRecord::Type::Uuid.generate
+      writer.add_file("data/active_storage_attachments/#{normal_attachment_id}.json", {
+        id: normal_attachment_id,
+        account_id: ActiveRecord::Type::Uuid.generate,
+        blob_id: normal_blob_id,
+        record_type: "Card",
+        record_id: ActiveRecord::Type::Uuid.generate,
+        name: "file",
+        created_at: Time.current.iso8601
+      }.to_json)
+
+      # Storage files
+      writer.add_file("storage/#{internal_key}", "internal data", compress: false)
+      writer.add_file("storage/#{normal_key}", normal_file_content, compress: false)
+
+      writer.close
+
+      tempfile.rewind
+      ZipFile::Reader.new(tempfile)
+    end
+
     def build_zip_with_duplicate_keys(blob_id_1:, blob_id_2:, key:)
       tempfile = Tempfile.new([ "test_export", ".zip" ])
       tempfile.binmode