Skip to content

Commit

Permalink
Filter out legacy assets when checking if all assets uploaded
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
ChrisBAshton authored and lauraghiorghisor-tw committed Dec 9, 2024
1 parent 0b06804 commit 34f4a80
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
2 changes: 1 addition & 1 deletion app/models/attachment_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 15 additions & 0 deletions test/unit/app/models/attachment_data_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,21 @@ 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)
# Unable to set `variant: :thumbnail` here (which is what we really want to test)
# as it's not a valid enum and there seems to be no way of stubbing it.
# But setting to `nil` does the same thing for the purposes of this test.
legacy_thumbnail_asset = build(:asset, asset_manager_id: "asset_manager_id_thumbnail", variant: nil, 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)

Expand Down

0 comments on commit 34f4a80

Please sign in to comment.