From e54cff2274fad75b0bd7d6bd9e38055ef1fbaf6d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Wed, 3 May 2023 14:38:06 +0100
Subject: [PATCH 1/9] Set images to public-read

Defining an alternate s3 configuration set to `public: true` means we can use it selectively. It sets the objects to `acl: "public-read"` by default (read-only) and means any image tags for those assets can use direct public links in the src attribute (without hitting the ActiveStorage::Representation endpoint). The default non-public service will still be used by default for any other files on instances using s3.
---
 app/models/application_record.rb | 4 ++++
 app/models/enterprise.rb         | 6 +++---
 app/models/enterprise_group.rb   | 4 ++--
 app/models/spree/image.rb        | 2 +-
 config/storage.yml               | 8 ++++++++
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index d36b82d6f7..6bfeb85c3a 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -10,4 +10,8 @@ class ApplicationRecord < ActiveRecord::Base
   include ArelHelpers::JoinAssociation
 
   self.abstract_class = true
+
+  def self.image_service
+    ENV["S3_BUCKET"].present? ? :amazon_public : :local
+  end
 end
diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index e7b81947ae..bd813f287f 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -87,10 +87,10 @@ class Enterprise < ApplicationRecord
                                             }
   accepts_nested_attributes_for :custom_tab
 
-  has_one_attached :logo
-  has_one_attached :promo_image
+  has_one_attached :logo, service: image_service
+  has_one_attached :promo_image, service: image_service
   has_one_attached :terms_and_conditions
-  has_one_attached :white_label_logo
+  has_one_attached :white_label_logo, service: image_service
 
   validates :logo,
             processable_image: true,
diff --git a/app/models/enterprise_group.rb b/app/models/enterprise_group.rb
index 9616c24722..4221776554 100644
--- a/app/models/enterprise_group.rb
+++ b/app/models/enterprise_group.rb
@@ -25,8 +25,8 @@ class EnterpriseGroup < ApplicationRecord
 
   delegate :phone, :address1, :address2, :city, :zipcode, :state, :country, to: :address
 
-  has_one_attached :logo
-  has_one_attached :promo_image
+  has_one_attached :logo, service: image_service
+  has_one_attached :promo_image, service: image_service
 
   validates :logo,
             processable_image: true,
diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index e328eea790..8bdaeb65bb 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -9,7 +9,7 @@ module Spree
       large: { resize_to_limit: [600, 600] },
     }.freeze
 
-    has_one_attached :attachment
+    has_one_attached :attachment, service: image_service
 
     validates :attachment,
               attached: true,
diff --git a/config/storage.yml b/config/storage.yml
index 255c8298d9..271782041f 100644
--- a/config/storage.yml
+++ b/config/storage.yml
@@ -19,3 +19,11 @@ amazon:
   secret_access_key: <%= ENV["S3_SECRET"] %>
   bucket: <%= ENV["S3_BUCKET"] %>
   region: <%= ENV.fetch("S3_REGION", "us-east-1") %>
+
+amazon_public:
+  service: S3
+  public: true
+  access_key_id: <%= ENV["S3_ACCESS_KEY"] %>
+  secret_access_key: <%= ENV["S3_SECRET"] %>
+  bucket: <%= ENV["S3_BUCKET"] %>
+  region: <%= ENV.fetch("S3_REGION", "us-east-1") %>

From a89891572408ac55492471e8ff33b867b24b334b Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 4 May 2023 00:04:29 +0100
Subject: [PATCH 2/9] Use named image variants

---
 app/models/enterprise.rb                      | 40 +++++++++----------
 app/models/spree/image.rb                     | 16 ++++----
 .../api/admin/enterprise_serializer.rb        |  4 +-
 config/application.rb                         |  1 +
 4 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index bd813f287f..b9a9f15388 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -5,20 +5,9 @@ class Enterprise < ApplicationRecord
   ENTERPRISE_SEARCH_RADIUS = 100
   # The next Rails version will have named variants but we need to store them
   # ourselves for now.
-  LOGO_SIZES = {
-    thumb: { gravity: "Center", resize: "100x100^", crop: '100x100+0+0' },
-    small: { gravity: "Center", resize: "180x180^", crop: '180x180+0+0' },
-    medium: { gravity: "Center", resize: "300x300^", crop: '300x300+0+0' },
-  }.freeze
-  PROMO_IMAGE_SIZES = {
-    thumb: { resize_to_limit: [100, 100] },
-    medium: { resize_to_fill: [720, 156] },
-    large: { resize_to_fill: [1200, 260] },
-  }.freeze
-  WHITE_LABEL_LOGO_SIZES = {
-    default: { gravity: "Center", resize_to_fill: [217, 44] },
-    mobile: { gravity: "Center", resize_to_fill: [75, 26] },
-  }.freeze
+  LOGO_SIZES = [:thumb, :small, :medium].freeze
+  PROMO_IMAGE_SIZES = [:thumb, :medium, :large].freeze
+  WHITE_LABEL_LOGO_SIZES = [:default, :mobile].freeze
   VALID_INSTAGRAM_REGEX = %r{\A[a-zA-Z0-9._]{1,30}([^/-]*)\z}
 
   searchable_attributes :sells, :is_primary_producer, :name
@@ -87,10 +76,21 @@ class Enterprise < ApplicationRecord
                                             }
   accepts_nested_attributes_for :custom_tab
 
-  has_one_attached :logo, service: image_service
-  has_one_attached :promo_image, service: image_service
   has_one_attached :terms_and_conditions
-  has_one_attached :white_label_logo, service: image_service
+  has_one_attached :logo, service: image_service do |attachment|
+    attachment.variant :thumb, resize_to_fill: [100, 100], crop: [0, 0, 100, 100]
+    attachment.variant :small, resize_to_fill: [180, 180], crop: [0, 0, 180, 180]
+    attachment.variant :medium, resize_to_fill: [300, 300], crop: [0, 0, 300, 300]
+  end
+  has_one_attached :promo_image, service: image_service do |attachment|
+    attachment.variant :thumb, resize_to_limit: [100, 100]
+    attachment.variant :medium, resize_to_fill: [720, 156]
+    attachment.variant :large, resize_to_fill: [1200, 260]
+  end
+  has_one_attached :white_label_logo, service: image_service do |attachment|
+    attachment.variant :default, resize_to_fill: [217, 44]
+    attachment.variant :mobile,  resize_to_fill: [75, 26]
+  end
 
   validates :logo,
             processable_image: true,
@@ -298,7 +298,7 @@ class Enterprise < ApplicationRecord
     return unless logo.variable?
 
     Rails.application.routes.url_helpers.url_for(
-      logo.variant(LOGO_SIZES[name])
+      logo.variant(name)
     )
   end
 
@@ -306,7 +306,7 @@ class Enterprise < ApplicationRecord
     return unless promo_image.variable?
 
     Rails.application.routes.url_helpers.url_for(
-      promo_image.variant(PROMO_IMAGE_SIZES[name])
+      promo_image.variant(name)
     )
   end
 
@@ -314,7 +314,7 @@ class Enterprise < ApplicationRecord
     return unless white_label_logo.variable?
 
     Rails.application.routes.url_helpers.url_for(
-      white_label_logo.variant(WHITE_LABEL_LOGO_SIZES[name])
+      white_label_logo.variant(name)
     )
   end
 
diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index 8bdaeb65bb..58263e0138 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -2,14 +2,12 @@
 
 module Spree
   class Image < Asset
-    SIZES = {
-      mini: { resize_to_fill: [48, 48] },
-      small: { resize_to_fill: [227, 227] },
-      product: { resize_to_limit: [240, 240] },
-      large: { resize_to_limit: [600, 600] },
-    }.freeze
-
-    has_one_attached :attachment, service: image_service
+    has_one_attached :attachment, service: image_service do |attachment|
+      attachment.variant :mini, resize_to_fill: [48, 48]
+      attachment.variant :small, resize_to_fill: [227, 227]
+      attachment.variant :product, resize_to_limit: [240, 240]
+      attachment.variant :large, resize_to_limit: [600, 600]
+    end
 
     validates :attachment,
               attached: true,
@@ -19,7 +17,7 @@ module Spree
 
     def variant(name)
       if attachment.variable?
-        attachment.variant(SIZES[name])
+        attachment.variant(name)
       else
         attachment
       end
diff --git a/app/serializers/api/admin/enterprise_serializer.rb b/app/serializers/api/admin/enterprise_serializer.rb
index f28e0a3ef7..30c3d2045b 100644
--- a/app/serializers/api/admin/enterprise_serializer.rb
+++ b/app/serializers/api/admin/enterprise_serializer.rb
@@ -96,9 +96,9 @@ module Api
       def attachment_urls(attachment, styles)
         return unless attachment.variable?
 
-        styles.transform_values do |transformation|
+        styles.index_with do |style|
           Rails.application.routes.url_helpers.
-            url_for(attachment.variant(transformation))
+            url_for(attachment.variant(style))
         end
       end
     end
diff --git a/config/application.rb b/config/application.rb
index 7ea51da3e5..c3cb2db1c5 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -244,6 +244,7 @@ module Openfoodnetwork
     config.active_storage.service = ENV["S3_BUCKET"].present? ? :amazon : :local
     config.active_storage.content_types_to_serve_as_binary -= ["image/svg+xml"]
     config.active_storage.variable_content_types += ["image/svg+xml"]
+    config.active_storage.url_options = config.action_controller.default_url_options
 
     config.exceptions_app = self.routes
   end

From 551d0d3a938c05a4b9e488c0aa8f3d812dead8f3 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 4 May 2023 01:01:12 +0100
Subject: [PATCH 3/9] Simplify calling #url_for in models

---
 app/models/application_record.rb |  4 ++++
 app/models/enterprise.rb         | 12 +++---------
 app/models/spree/image.rb        |  2 +-
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index 6bfeb85c3a..4e726e63b6 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -14,4 +14,8 @@ class ApplicationRecord < ActiveRecord::Base
   def self.image_service
     ENV["S3_BUCKET"].present? ? :amazon_public : :local
   end
+
+  def url_for(*args)
+    Rails.application.routes.url_helpers.url_for(*args)
+  end
 end
diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index b9a9f15388..10441b45a6 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -297,25 +297,19 @@ class Enterprise < ApplicationRecord
   def logo_url(name)
     return unless logo.variable?
 
-    Rails.application.routes.url_helpers.url_for(
-      logo.variant(name)
-    )
+    url_for(logo.variant(name))
   end
 
   def promo_image_url(name)
     return unless promo_image.variable?
 
-    Rails.application.routes.url_helpers.url_for(
-      promo_image.variant(name)
-    )
+    url_for(promo_image.variant(name))
   end
 
   def white_label_logo_url(name = :default)
     return unless white_label_logo.variable?
 
-    Rails.application.routes.url_helpers.url_for(
-      white_label_logo.variant(name)
-    )
+    url_for(white_label_logo.variant(name))
   end
 
   def website
diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index 58263e0138..55bb66f0af 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -26,7 +26,7 @@ module Spree
     def url(size)
       return unless attachment.attached?
 
-      Rails.application.routes.url_helpers.url_for(variant(size))
+      url_for(variant(size))
     end
 
     # if there are errors from the plugin, then add a more meaningful message

From 6f12d012edcadfc4e0641f43c207a167804b9c04 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 4 May 2023 11:35:05 +0100
Subject: [PATCH 4/9] Use #url helper

---
 app/views/spree/admin/images/edit.html.haml  | 4 +---
 app/views/spree/admin/images/index.html.haml | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/app/views/spree/admin/images/edit.html.haml b/app/views/spree/admin/images/edit.html.haml
index 711dd32fbb..40ff067b0a 100644
--- a/app/views/spree/admin/images/edit.html.haml
+++ b/app/views/spree/admin/images/edit.html.haml
@@ -11,9 +11,7 @@
     .field.alpha.three.columns.align-center
       = f.label t('spree.thumbnail')
       %br/
-      - # A Rails bug makes it necessary to call `main_app.url_for` here.
-      - # https://github.com/rails/rails/issues/31325
-      = link_to image_tag(main_app.url_for(@image.variant(:small))), main_app.url_for(@image.variant(:product))
+      = link_to image_tag(@image.url(:small)), @image.url(:product)
     .nine.columns.omega
       = render partial: 'form', locals: { f: f }
     .clear
diff --git a/app/views/spree/admin/images/index.html.haml b/app/views/spree/admin/images/index.html.haml
index 978de27092..4757f869de 100644
--- a/app/views/spree/admin/images/index.html.haml
+++ b/app/views/spree/admin/images/index.html.haml
@@ -32,9 +32,7 @@
           %td.no-border
             %span.handle
           %td
-            - # A Rails bug makes it necessary to call `main_app.url_for` here.
-            - # https://github.com/rails/rails/issues/31325
-            = link_to image_tag(main_app.url_for(image.variant(:mini))), main_app.url_for(image.variant(:product))
+            = link_to image_tag(image.url(:mini)), image.url(:product)
           %td= options_text_for(image)
           %td= image.alt
           %td.actions

From b510668ecfe8fb9a0145dbdbb93c210ede31d81d Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 4 May 2023 01:30:50 +0100
Subject: [PATCH 5/9] Render direct URLs for s3 images

---
 app/models/spree/image.rb | 1 +
 1 file changed, 1 insertion(+)

diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index 55bb66f0af..dcdbf8ddb2 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -25,6 +25,7 @@ module Spree
 
     def url(size)
       return unless attachment.attached?
+      return variant(size).processed.url if attachment.service.name == :amazon_public
 
       url_for(variant(size))
     end

From 4c7e947738d802672097bee97369ed9e64e4db95 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Thu, 4 May 2023 01:31:20 +0100
Subject: [PATCH 6/9] Migrate data

So it turns out all images uploaded before the ActiveStorage migration are internally set to "public-read", and all images uploaded after the migration are internally set to "private". This migration switches all images in S3 buckets back to "public-read".
---
 db/migrate/20230504001141_public_images.rb | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 db/migrate/20230504001141_public_images.rb

diff --git a/db/migrate/20230504001141_public_images.rb b/db/migrate/20230504001141_public_images.rb
new file mode 100644
index 0000000000..ea0695f67b
--- /dev/null
+++ b/db/migrate/20230504001141_public_images.rb
@@ -0,0 +1,40 @@
+require 'aws-sdk-s3'
+
+class PublicImages < ActiveRecord::Migration[7.0]
+  def up
+    return unless s3_bucket
+
+    check_connection!
+    set_objects_to_readable
+
+    ActiveStorage::Blob.
+      where(service_name: "amazon").
+      where("content_type LIKE 'image/%'").
+      update_all(service_name: "amazon_public")
+  end
+
+  def down
+    ActiveStorage::Blob.
+      where(service_name: "amazon_public").
+      where("content_type LIKE 'image/%'").
+      update_all(service_name: "amazon")
+  end
+
+  private
+
+  # Returns an Aws::S3::Bucket object
+  def s3_bucket
+    @s3_bucket ||= ActiveStorage::Blob.where(service_name: "amazon").first&.service&.bucket
+  end
+
+  # Checks bucket status. Throws an error if connection fails
+  def check_connection!
+    s3_bucket.exists?
+  end
+
+  # Sets bucket objects' ACL to "public-read". Performs batched processing internally
+  # with a custom enumerator, see AWS::Resources::Collection#each for details.
+  def set_objects_to_readable
+    s3_bucket.objects.each{|object| object.acl.put(acl: "public-read") }
+  end
+end

From 5ad37ce6a58b22da884fde21ef1cd92a40a05684 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 9 Jun 2023 16:07:42 +0100
Subject: [PATCH 7/9] Return default image url on ActiveStorage errors in image
 loading

---
 .../templates/product_modal.html.haml         |  2 +-
 app/models/spree/image.rb                     | 14 ++++++++-
 .../api/admin/product_serializer.rb           |  4 +--
 app/serializers/api/variant_serializer.rb     |  2 +-
 app/views/spree/admin/products/new.html.haml  |  2 +-
 .../spree/shared/_variant_thumbnail.html.haml |  2 +-
 spec/models/spree/image_spec.rb               | 30 +++++++++++++++++--
 7 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/app/assets/javascripts/templates/product_modal.html.haml b/app/assets/javascripts/templates/product_modal.html.haml
index 95c74c5f72..9102183872 100644
--- a/app/assets/javascripts/templates/product_modal.html.haml
+++ b/app/assets/javascripts/templates/product_modal.html.haml
@@ -15,6 +15,6 @@
 
   .columns.small-12.medium-6.large-6.product-img
     %img{"ng-src" => "{{::product.largeImage}}", "ng-if" => "::product.largeImage"}
-    %img.placeholder{ src: "/noimage/large.png", "ng-if" => "::!product.largeImage"}
+    %img.placeholder{ src: Spree::Image.default_image_url(:large), "ng-if" => "::!product.largeImage"}
 
 %ng-include{src: "'partials/close.html'"}
diff --git a/app/models/spree/image.rb b/app/models/spree/image.rb
index dcdbf8ddb2..f6732cfae7 100644
--- a/app/models/spree/image.rb
+++ b/app/models/spree/image.rb
@@ -15,6 +15,12 @@ module Spree
               content_type: %r{\Aimage/(png|jpeg|gif|jpg|svg\+xml|webp)\Z}
     validate :no_attachment_errors
 
+    def self.default_image_url(size)
+      return "/noimage/product.png" unless size&.to_sym.in?([:mini, :small, :product, :large])
+
+      "/noimage/#{size}.png"
+    end
+
     def variant(name)
       if attachment.variable?
         attachment.variant(name)
@@ -24,12 +30,18 @@ module Spree
     end
 
     def url(size)
-      return unless attachment.attached?
+      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))
+    rescue ActiveStorage::Error => e
+      Rails.logger.error(e.message)
+
+      self.class.default_image_url(size)
     end
 
+    private
+
     # if there are errors from the plugin, then add a more meaningful message
     def no_attachment_errors
       return if errors[:attachment].empty?
diff --git a/app/serializers/api/admin/product_serializer.rb b/app/serializers/api/admin/product_serializer.rb
index b313d4297e..6dd930f7da 100644
--- a/app/serializers/api/admin/product_serializer.rb
+++ b/app/serializers/api/admin/product_serializer.rb
@@ -27,11 +27,11 @@ module Api
       end
 
       def image_url
-        object.images.first&.url(:product) || "/noimage/product.png"
+        object.images.first&.url(:product) || Spree::Image.default_image_url(:product)
       end
 
       def thumb_url
-        object.images.first&.url(:mini) || "/noimage/mini.png"
+        object.images.first&.url(:mini) || Spree::Image.default_image_url(:mini)
       end
 
       def on_hand
diff --git a/app/serializers/api/variant_serializer.rb b/app/serializers/api/variant_serializer.rb
index 020a48fb93..79b492b528 100644
--- a/app/serializers/api/variant_serializer.rb
+++ b/app/serializers/api/variant_serializer.rb
@@ -40,7 +40,7 @@ class Api::VariantSerializer < ActiveModel::Serializer
   end
 
   def thumb_url
-    object.product.images.first&.url(:mini) || "/noimage/mini.png"
+    object.product.images.first&.url(:mini) || Spree::Image.default_image_url(:mini)
   end
 
   def unit_price_price
diff --git a/app/views/spree/admin/products/new.html.haml b/app/views/spree/admin/products/new.html.haml
index 4d770bb4af..28a2bbbd8a 100644
--- a/app/views/spree/admin/products/new.html.haml
+++ b/app/views/spree/admin/products/new.html.haml
@@ -99,7 +99,7 @@
     %fieldset.no-border-bottom{ id: "image" }
       %legend{align: "center"}= t(".image")
       .row
-        = image_tag "/noimage/product.png", class: "four columns alpha"
+        = image_tag Spree::Image.default_image_url(:product), class: "four columns alpha"
       .row
         = f.fields_for 'images_attributes[]', f.object.images.build do |image_fields|
           = image_fields.file_field :attachment
diff --git a/app/views/spree/shared/_variant_thumbnail.html.haml b/app/views/spree/shared/_variant_thumbnail.html.haml
index 12c2589984..d97d6175ca 100644
--- a/app/views/spree/shared/_variant_thumbnail.html.haml
+++ b/app/views/spree/shared/_variant_thumbnail.html.haml
@@ -1 +1 @@
-= image_tag(variant.product.images.first&.url(:mini) || "/noimage/mini.png")
+= image_tag(variant.product.images.first&.url(:mini) || Spree::Image.default_image_url(:mini))
diff --git a/spec/models/spree/image_spec.rb b/spec/models/spree/image_spec.rb
index 40481cf1ea..3b913799ca 100644
--- a/spec/models/spree/image_spec.rb
+++ b/spec/models/spree/image_spec.rb
@@ -31,10 +31,36 @@ module Spree
         )
       end
 
-      it "returns nil when the attachment is missing" do
+      it "returns default image when the attachment is missing" do
         subject.attachment = nil
 
-        expect(subject.url(:small)).to eq nil
+        expect(subject.url(:small)).to eq "/noimage/small.png"
+      end
+
+      context "when no image attachment is found" do
+        it "returns a default product image" do
+          expect(subject).to receive_message_chain(:attachment, :attached?) { false }
+
+          expect(subject.url(:mini)).to eq "/noimage/mini.png"
+        end
+      end
+
+      context "when accessing the image raises an ActiveStorage error" do
+        it "rescues the error and returns a default product image" do
+          expect(subject).to receive(:attachment) { raise ActiveStorage::FileNotFoundError }
+
+          expect(subject.url(:small)).to eq "/noimage/small.png"
+        end
+      end
+    end
+
+    describe "#default_image_url" do
+      it "returns default product image for a given size" do
+        expect(subject.class.default_image_url(:mini)).to eq "/noimage/mini.png"
+      end
+
+      it "returns default product image when no size is given" do
+        expect(subject.class.default_image_url(nil)).to eq "/noimage/product.png"
       end
     end
 

From 395cf7fb8d11af3498bc7353077bb92d1c361b12 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Fri, 9 Jun 2023 17:48:55 +0100
Subject: [PATCH 8/9] Use direct image URLs for enterprise logos

---
 app/models/enterprise.rb | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb
index 10441b45a6..a2eefa6ac5 100644
--- a/app/models/enterprise.rb
+++ b/app/models/enterprise.rb
@@ -295,21 +295,15 @@ class Enterprise < ApplicationRecord
   end
 
   def logo_url(name)
-    return unless logo.variable?
-
-    url_for(logo.variant(name))
+    image_url_for(logo, name)
   end
 
   def promo_image_url(name)
-    return unless promo_image.variable?
-
-    url_for(promo_image.variant(name))
+    image_url_for(promo_image, name)
   end
 
   def white_label_logo_url(name = :default)
-    return unless white_label_logo.variable?
-
-    url_for(white_label_logo.variant(name))
+    image_url_for(white_label_logo, name)
   end
 
   def website
@@ -461,6 +455,17 @@ class Enterprise < ApplicationRecord
     errors.add(:white_label_logo_link, I18n.t(:invalid_url))
   end
 
+  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))
+  rescue ActiveStorage::Error => e
+    Rails.logger.error(e.message)
+
+    nil
+  end
+
   def current_exchange_variants
     ExchangeVariant.joins(exchange: :order_cycle)
       .merge(Exchange.outgoing)

From 64508d3c3d1664b449acc6f2c920696eea817266 Mon Sep 17 00:00:00 2001
From: Matt-Yorkley <9029026+Matt-Yorkley@users.noreply.github.com>
Date: Sat, 10 Jun 2023 11:34:07 +0100
Subject: [PATCH 9/9] Add unit test for direct image URLs

---
 spec/models/spree/image_spec.rb | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/spec/models/spree/image_spec.rb b/spec/models/spree/image_spec.rb
index 3b913799ca..d6ac3b5b0c 100644
--- a/spec/models/spree/image_spec.rb
+++ b/spec/models/spree/image_spec.rb
@@ -52,6 +52,16 @@ module Spree
           expect(subject.url(:small)).to eq "/noimage/small.png"
         end
       end
+
+      context "when using public images" do
+        it "returns the direct URL for the processed image" do
+          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.url(:small)).to eq "https://ofn-s3/123.png"
+        end
+      end
     end
 
     describe "#default_image_url" do