Do not try to generate a URL for unpersisted blobs in development/test environment

Explicitly raise an error in `image_variant_url_for` if an Active Storage variant's blob is not persisted.

This addresses `ArgumentError`/`URI::InvalidURIError` in Rails 7.1, which occurs when attempting to generate a URL for an unsaved Active Storage blob. By raising, we ensure existing error handling in calling methods (e.g., `Spree::Image#url`) can provide graceful fallbacks (default image URLs).

This should only affect test and development environments where blobs may not be immediately persisted. Tests in `SuppliedProductImporter` have been updated to reflect this behavior.

References:
  - Suggestion: https://github.com/openfoodfoundation/openfoodnetwork/pull/13232#discussion_r2071116581
  - Example of failing test due to this: https://github.com/openfoodfoundation/openfoodnetwork/actions/runs/14739687958/job/41374346184?pr=13232
  - Related: https://github.com/rails/rails/issues/50234
This commit is contained in:
Carlos Chitty
2025-04-30 12:36:39 -04:00
parent 75b2119dd1
commit b43fa55a7b
2 changed files with 16 additions and 3 deletions

View File

@@ -20,6 +20,10 @@ class ApplicationRecord < ActiveRecord::Base
if ENV["S3_BUCKET"].present? && variant.service.public?
variant.processed.url
else
unless variant.blob.persisted?
raise "ActiveStorage blob for variant is not persisted. Cannot generate URL."
end
url_for(variant)
end
end

View File

@@ -34,9 +34,15 @@ RSpec.describe SuppliedProductImporter do
before do
taxon.save!
stub_request(:get, "https://cd.net/tomato.png?v=5").to_return(
status: 200,
body: black_logo_path.read
)
allow(product).to receive(:image).and_return("https://cd.net/tomato.png?v=5")
end
it "stores a new Spree Product and Variant" do
it "stores a new Spree Product and Variant with image" do
expect { subject }.to change {
Spree::Product.count
}.by(1)
@@ -49,6 +55,10 @@ RSpec.describe SuppliedProductImporter do
expect(subject.variant_unit_scale).to eq(nil)
expect(subject.variant_unit_with_scale).to eq("items")
expect(subject.unit_value).to eq(1)
expect(subject.product.image).to be_present
expect(subject.product.image.attachment).to be_attached
expect(subject.product.image.url(:product)).to match(/^http.*tomato\.png/)
end
end
@@ -107,7 +117,7 @@ RSpec.describe SuppliedProductImporter do
)
end
it "creates a new Spree::Product" do
it "builds an unsaved Spree::Product with all mapped attributes" do
product = importer.import_product(supplied_product, supplier)
expect(product).to be_a(Spree::Product)
@@ -116,7 +126,6 @@ RSpec.describe SuppliedProductImporter do
expect(product.variant_unit).to eq("weight")
expect(product.image).to be_present
expect(product.image.attachment).to be_attached
expect(product.image.url(:product)).to match /^http.*tomato\.png/
end
describe "taxon" do