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