Skip to content

Commit

Permalink
Use asset_manager_id instead of attachment_data_id
Browse files Browse the repository at this point in the history
When sending attachment information to publishing api,
use asset_manager_id instead of attachment_data_id,
as this is the id that the asset manager expects.
  • Loading branch information
unoduetre committed Feb 24, 2025
1 parent 29b9498 commit cfe3742
Show file tree
Hide file tree
Showing 8 changed files with 40 additions and 6 deletions.
2 changes: 1 addition & 1 deletion app/models/attachment_data.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class AttachmentData < ApplicationRecord
as: :assetable,
inverse_of: :assetable

delegate :url, :path, to: :file, allow_nil: true
delegate :url, :path, :asset_manager_id, to: :file, allow_nil: true

before_save :update_file_attributes

Expand Down
6 changes: 3 additions & 3 deletions app/models/file_attachment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def publishing_api_details_for_format
filename:,
number_of_pages:,
preview_url:,
attachment_data_id:,
asset_manager_id:,
}
end

Expand All @@ -67,10 +67,10 @@ def alternative_format_contact_email
nil
end

def attachment_data_id
def asset_manager_id
return unless csv? && attachable.is_a?(Edition) && attachment_data.all_asset_variants_uploaded?

attachment_data.id
attachment_data.asset_manager_id
end

def preview_url
Expand Down
4 changes: 4 additions & 0 deletions app/uploaders/attachment_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ def extension_allowlist
EXTENSION_ALLOW_LIST
end

def asset_manager_id
file.try(:asset_manager_id)
end

class ZipFile
class NonUTF8ContentsError < RuntimeError; end

Expand Down
4 changes: 4 additions & 0 deletions lib/whitehall/asset_manager_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ def url
end
end

def asset_manager_id
get_asset&.asset_manager_id
end

def path
# We keep this because carrierwave needs this in after commit hook
# to look for previous files to delete
Expand Down
6 changes: 6 additions & 0 deletions test/unit/app/models/attachment_data_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -452,4 +452,10 @@ class AttachmentDataTest < ActiveSupport::TestCase

assert_not attachment_data.all_asset_variants_uploaded?
end

test "#asset_manager_id returns asset manager id" do
attachment_data = create(:attachment_data_for_csv, attachable: create(:draft_edition, id: 1))

assert_equal "asset_manager_id", attachment_data.asset_manager_id
end
end
5 changes: 3 additions & 2 deletions test/unit/app/models/file_attachment_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,9 @@ def assert_delegated(attachment, method)
assert_equal Plek.asset_root + "/media/#{attachment.attachment_data.id}/sample.csv/preview", attachment.publishing_api_details_for_format[:preview_url]
end

test "return media attachment_data_id if all_asset_variants_uploaded?" do
test "#asset_manager_id returns asset manager id if all_asset_variants_uploaded?" do
attachment = create(:csv_attachment, attachable: create(:edition))
assert_equal attachment.attachment_data.id, attachment.publishing_api_details_for_format[:attachment_data_id]
assert_not_nil attachment.attachment_data.asset_manager_id
assert_equal attachment.attachment_data.asset_manager_id, attachment.publishing_api_details_for_format[:asset_manager_id]
end
end
7 changes: 7 additions & 0 deletions test/unit/app/uploaders/attachment_uploader_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,13 @@ class AttachmentUploaderTest < ActiveSupport::TestCase
end
end

test "#asset_manager_id should return asset manager id" do
uploader = AttachmentUploader.new(@attachment_data, "mounted-as")
uploader.store!(file_fixture("sample_attachment.zip"))

assert_equal "asset_manager_id", uploader.asset_manager_id
end

def required_arcgis_file_list
%w[london.shp london.shx london.dbf]
end
Expand Down
12 changes: 12 additions & 0 deletions test/unit/lib/whitehall/asset_manager_storage_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -68,4 +68,16 @@ class Whitehall::AssetManagerStorage::FileTest < ActiveSupport::TestCase

assert_equal model.file.path, model.file.url
end

test "returns nil when asset is not available" do
model = create(:attachment_data_with_no_assets, attachable: create(:draft_edition, id: 1))

assert_nil model.file.file.asset_manager_id
end

test "returns asset_manager_id when asset is available" do
model = create(:attachment_data_for_csv, attachable: create(:draft_edition, id: 1))

assert_equal "asset_manager_id", model.file.file.asset_manager_id
end
end

0 comments on commit cfe3742

Please sign in to comment.