From b43fa55a7b8638eae2cf626b20f9c830f49f388e Mon Sep 17 00:00:00 2001
From: Carlos Chitty <chitty.carlosj@gmail.com>
Date: Wed, 30 Apr 2025 12:36:39 -0400
Subject: [PATCH] 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
---
 app/models/application_record.rb                  |  4 ++++
 .../services/supplied_product_importer_spec.rb    | 15 ++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index e54fcb49aa..2655f99cb4 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -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
diff --git a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb
index b407a4c132..815a1c6897 100644
--- a/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb
+++ b/engines/dfc_provider/spec/services/supplied_product_importer_spec.rb
@@ -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