Skip to content

Commit

Permalink
Simplify all_asset_variants_uploaded? method
Browse files Browse the repository at this point in the history
Now that the logic defining `version :thumbnail, if: :pdf?` in
`AttachmentUploader` has been removed, there is no attachment
type that has anything other than an `:original` version. We
therefore know that `required_variants` is always just `:original`
and can simplify the method.

I did consider renaming the method to simply `uploaded?` or
`original_uploaded?`, but a method called
`all_asset_variants_uploaded?` is defined in 8 different places,
and called in almost a hundred places, so it's difficult to know
which variation of the method is being called in which place (and
therefore which method calls to rename). Safer to just leave it.

With the method simplified, we can remove the test (since it
would no longer work).
  • Loading branch information
ChrisBAshton committed Dec 4, 2024
1 parent 4cbe6ec commit 502b427
Show file tree
Hide file tree
Showing 2 changed files with 1 addition and 12 deletions.
3 changes: 1 addition & 2 deletions app/models/attachment_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,8 @@ def indexable?

def all_asset_variants_uploaded?
asset_variants = assets.map(&:variant).map(&:to_sym)
required_variants = pdf? ? AttachmentUploader.versions.keys.push(:original) : [Asset.variants[:original].to_sym]

(required_variants - asset_variants).empty?
(%i[original] - asset_variants).empty?
end

def update_file_attributes
Expand Down
10 changes: 0 additions & 10 deletions test/unit/app/models/attachment_data_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -437,14 +437,4 @@ class AttachmentDataTest < ActiveSupport::TestCase

assert_not attachment_data.all_asset_variants_uploaded?
end

# TODO: commented out to get the tests passing.
# We should consider rewriting the test using a temporary model that has an asset variant.
#
# test "#all_asset_variants_uploaded? returns false if some asset variants are missing" do
# attachment_data = build(:attachment_data_with_no_assets, content_type: AttachmentUploader::PDF_CONTENT_TYPE)
# attachment_data.assets << build(:asset)

# assert_not attachment_data.all_asset_variants_uploaded?
# end
end

0 comments on commit 502b427

Please sign in to comment.