Merge pull request #11109 from mkllnk/image-urls

Gracefully deal with missing S3 config

app/models/application_record.rb

@@ -15,6 +15,16 @@ class ApplicationRecord < ActiveRecord::Base
     ENV["S3_BUCKET"].present? ? :amazon_public : :local
   end
 
+  # We might have a development environment without S3 but with a database
+  # dump pointing to S3 images. Accessing the service fails then.
+  def image_variant_url_for(variant)
+    if ENV["S3_BUCKET"].present? && variant.service.public?
+      variant.processed.url
+    else
+      url_for(variant)
+    end
+  end
+
   def url_for(*args)
     Rails.application.routes.url_helpers.url_for(*args)
   end

app/models/enterprise.rb

@@ -459,9 +459,8 @@ class Enterprise < ApplicationRecord
 
   def image_url_for(image, name)
     return unless image.variable?
-    return image.variant(name).processed.url if image.attachment.service.name == :amazon_public
 
-    url_for(image.variant(name))
+    image_variant_url_for(image.variant(name))
   rescue ActiveStorage::Error => e
     Bugsnag.notify "Enterprise ##{id} #{image.try(:name)} error: #{e.message}"
     Rails.logger.error(e.message)

app/models/spree/image.rb

@@ -31,9 +31,8 @@ module Spree
 
     def url(size)
       return self.class.default_image_url(size) unless attachment.attached?
-      return variant(size).processed.url if attachment.service.name == :amazon_public
 
-      url_for(variant(size))
+      image_variant_url_for(variant(size))
     rescue ActiveStorage::Error => e
       Bugsnag.notify "Product ##{viewable_id} Image ##{id} error: #{e.message}"
       Rails.logger.error(e.message)

spec/models/spree/image_spec.rb

@@ -55,9 +55,14 @@ module Spree
 
       context "when using public images" do
         it "returns the direct URL for the processed image" do
+          allow(ENV).to receive(:[])
+          expect(ENV).to receive(:[]).with("S3_BUCKET").and_return("present")
+
+          variant = double(:variant)
           allow(subject).to receive_message_chain(:attachment, :attached?) { true }
-          expect(subject).to receive_message_chain(:attachment, :service, :name) { :amazon_public }
-          expect(subject).to receive_message_chain(:variant, :processed, :url) { "https://ofn-s3/123.png" }
+          expect(subject).to receive(:variant) { variant }
+          expect(variant).to receive_message_chain(:service, :public?) { true }
+          expect(variant).to receive_message_chain(:processed, :url) { "https://ofn-s3/123.png" }
 
           expect(subject.url(:small)).to eq "https://ofn-s3/123.png"
         end