From c7eef95fee934fe66e84a9e602c9eb5128f95c33 Mon Sep 17 00:00:00 2001 From: ChrisBAshton Date: Fri, 6 Dec 2024 14:36:27 +0000 Subject: [PATCH] Filter out legacy assets when checking if all assets uploaded If the asset doesn't respond to `variant`, it's a legacy asset (PDF thumbnail) and we want to filter it out. I did consider applying this filter more generally by overloading the `assets` call as below, but that breaks a bunch of tests - so any filtering needs to be applied on a per-case basis. ``` def assets super.select { |asset| asset.variant.present? } end ``` --- app/models/attachment_data.rb | 2 +- test/unit/app/models/attachment_data_test.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/app/models/attachment_data.rb b/app/models/attachment_data.rb index 18939956708..350c366de69 100644 --- a/app/models/attachment_data.rb +++ b/app/models/attachment_data.rb @@ -60,7 +60,7 @@ def indexable? end def all_asset_variants_uploaded? - asset_variants = assets.map(&:variant).map(&:to_sym) + asset_variants = assets.map(&:variant).compact.map(&:to_sym) (%i[original] - asset_variants).empty? end diff --git a/test/unit/app/models/attachment_data_test.rb b/test/unit/app/models/attachment_data_test.rb index 5f81e8b2a53..60f9b5f2ef5 100644 --- a/test/unit/app/models/attachment_data_test.rb +++ b/test/unit/app/models/attachment_data_test.rb @@ -432,6 +432,18 @@ class AttachmentDataTest < ActiveSupport::TestCase assert attachment_data.all_asset_variants_uploaded? end + test "#all_asset_variants_uploaded? skips over any legacy asset variants present" do + attachment_data = build(:attachment_data) + + original_pdf_asset = build(:asset, asset_manager_id: "asset_manager_id_original", variant: Asset.variants[:original], filename: attachment_data.filename) + legacy_thumbnail_asset = build(:asset, asset_manager_id: "asset_manager_id_thumbnail", variant: Asset.variants[:thumbnail], filename: "thumbnail_#{attachment_data.filename}.png") + + attachment_data.assets << original_pdf_asset + attachment_data.assets << legacy_thumbnail_asset + + assert attachment_data.all_asset_variants_uploaded? + end + test "#all_asset_variants_uploaded? returns false if there are no assets" do attachment_data = build(:attachment_data_with_no_assets)