id="<%= spree_dom_id product %>" data-hook="admin_products_index_rows" class="<%= cycle('odd', 'even') %>">
| <%= product.sku rescue '' %> |
- <%= image_tag product.display_image.attachment(:mini) %> |
+ <%= spree_image_tag product.gallery.best_image, :mini %> |
<%= link_to product.try(:name), edit_admin_product_path(product) %> |
<%= product.display_price.to_html rescue '' %> |
diff --git a/backend/app/views/spree/admin/stock_items/_stock_management.html.erb b/backend/app/views/spree/admin/stock_items/_stock_management.html.erb
index 188ac91c1dd..e1621b7a078 100644
--- a/backend/app/views/spree/admin/stock_items/_stock_management.html.erb
+++ b/backend/app/views/spree/admin/stock_items/_stock_management.html.erb
@@ -30,7 +30,7 @@
|
- <%= image_tag(variant.display_image(fallback: false).attachment(:small)) %>
+ <%= spree_image_tag(variant.gallery.primary_image, :small) %>
diff --git a/backend/app/views/spree/admin/stock_transfers/_transfer_item_table.html.erb b/backend/app/views/spree/admin/stock_transfers/_transfer_item_table.html.erb
index c3944ee7e98..3947741fcab 100644
--- a/backend/app/views/spree/admin/stock_transfers/_transfer_item_table.html.erb
+++ b/backend/app/views/spree/admin/stock_transfers/_transfer_item_table.html.erb
@@ -22,7 +22,7 @@
- <%= image_tag(variant.display_image(fallback: false).attachment(:small)) %>
+ <%= spree_image_tag(variant.gallery.primary_image, :small) %>
diff --git a/backend/app/views/spree/admin/users/items.html.erb b/backend/app/views/spree/admin/users/items.html.erb
index eb2095d0ffe..98408905dec 100644
--- a/backend/app/views/spree/admin/users/items.html.erb
+++ b/backend/app/views/spree/admin/users/items.html.erb
@@ -39,7 +39,7 @@
| <%= l(order.completed_at.to_date) if order.completed_at %> |
- <%= image_tag item.variant.display_image.attachment(:mini) %>
+ <%= spree_image_tag item.variant.gallery.best_image, :mini %>
|
<%= item.product.name %>
<%= "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? %>
diff --git a/core/app/helpers/spree/images_helper.rb b/core/app/helpers/spree/images_helper.rb
new file mode 100644
index 00000000000..b3b0d79c679
--- /dev/null
+++ b/core/app/helpers/spree/images_helper.rb
@@ -0,0 +1,32 @@
+module Spree
+ module ImagesHelper
+ # Return the provided image if it is not nil,
+ # otherwise return a default {Spree::Image}.
+ #
+ # This is useful when displaying images for a resource when it exists,
+ # or falling back to the Paperclip default image when none is provided.
+ #
+ # @param image [Spree::Image, nil] The image to return, or nil if the default
+ # should be used.
+ #
+ # @return [Spree::Image] the provided image if it exists, or a default
+ # image otherwise
+ def image_or_default(image)
+ image || Spree::Image.new
+ end
+
+ # Display an image_tag for the given spree image, if one exists, for the
+ # provided style, or a image_tag for the default image if no image is provided.
+ #
+ # @param image [Spree::Image, nil] The image to display, if provided, or nil
+ # if the default image should be displayed.
+ #
+ # @param style [symbol] The paperclip {Spree::Image} style to display the image_tag
+ # for
+ #
+ # @return [String] A string of the image_tag built from the provided image
+ def spree_image_tag(image, style, options={})
+ image_tag image_or_default(image).attachment(style), options
+ end
+ end
+end
diff --git a/core/app/models/spree/app_configuration.rb b/core/app/models/spree/app_configuration.rb
index a859691f09e..043fc39adf5 100644
--- a/core/app/models/spree/app_configuration.rb
+++ b/core/app/models/spree/app_configuration.rb
@@ -270,6 +270,22 @@ def variant_search_class
@variant_search_class ||= Spree::Core::Search::Variant
end
+ attr_writer :variant_gallery_class
+ # Returns the class to model the variants's gallery
+ #
+ # @return a class implementing {Spree::Gallery::Base}
+ def variant_gallery_class
+ @variant_gallery_class ||= Spree::Gallery::VariantAssociation
+ end
+
+ attr_writer :product_gallery_class
+ # Returns the class to model the products's gallery
+ #
+ # @return a class implementing {Spree::Gallery::Base}
+ def product_gallery_class
+ @product_gallery_class ||= Spree::Gallery::ProductVariantAssociation
+ end
+
# promotion_chooser_class allows extensions to provide their own PromotionChooser
attr_writer :promotion_chooser_class
def promotion_chooser_class
diff --git a/core/app/models/spree/gallery/base.rb b/core/app/models/spree/gallery/base.rb
new file mode 100644
index 00000000000..dd2610208a7
--- /dev/null
+++ b/core/app/models/spree/gallery/base.rb
@@ -0,0 +1,51 @@
+module Spree
+ module Gallery
+
+ # A gallery represents a collection of images.
+ #
+ # @abstract base class for all galleries. Implement your own
+ # to define how images should be associated to objects in your
+ # store
+ class Base
+
+ # A list of all images associated with this gallery
+ #
+ # @return [Enumerable] All images associated to the gallery
+ def images; raise NotImplementedError end
+
+ # The "primary" {Spree::Image} associated with the gallery
+ #
+ # Typically the most appropriate single image to display representing
+ # the gallery, but will not follow the fallback rules of {#best_image}
+ #
+ # @return [Spree::Image, nil] if primary image for the gallery exists or
+ # nil otherwise
+ #
+ # @abstract
+ def primary_image; raise NotImplementedError end
+
+ # The "best" {Spree::Image} associated with this gallery.
+ #
+ # Will attempt to fall back to other sources and logic to find a picture
+ # if the {#primary_image} does not exist
+ #
+ # @return [Spree::Image, nil] if an image can be found to represent the gallery,
+ # following fallback rules, and nli otherwise
+ #
+ # @abstract
+ def best_image; raise NotImplementedError end
+
+ # Return an ActiveRecord-compatible Array of objects to preload
+ #
+ # An unfortunate method to allow the application to minimize queries while
+ # allowing different Gallery implementations to maintain their own
+ # database structures
+ #
+ # @return [Array] an ActiveRecord::QueryMethods#includes compatible array
+ def self.preload_params
+ []
+ end
+
+ end
+ end
+end
diff --git a/core/app/models/spree/gallery/product_variant_association.rb b/core/app/models/spree/gallery/product_variant_association.rb
new file mode 100644
index 00000000000..06ed700aa62
--- /dev/null
+++ b/core/app/models/spree/gallery/product_variant_association.rb
@@ -0,0 +1,46 @@
+module Spree
+ # Uses the ActiveRecord association for a Prodcut's Variant's Images
+ module Gallery
+ class ProductVariantAssociation < Gallery::Base
+
+ # Creates a gallery for the given product
+ # @param [Spree::Product] product The product to create the gallery for
+ def initialize(product)
+ @product = product
+ end
+
+ # (see Spree::Gallery#images)
+ def images
+ product.variant_images
+ end
+
+ # (see Spree::Gallery#primary_image)
+ def primary_image
+ product.images.first || product.variant_images.first
+ end
+
+ # Returns the primary image for the gallery
+ #
+ # Does not follow any fallback rules
+ #
+ # (see Spree::Gallery#best_image)
+ def best_image
+ primary_image
+ end
+
+ # Return a list of image associations to preload a Spree::Product's images
+ #
+ # @return An array compatible with ActiveRecord :includes
+ # for a Spree::Products images preload
+ #
+ # (see Spree::Galery#preload_params)
+ def self.preload_params
+ [:variant_images]
+ end
+
+ private
+ attr_reader :product
+
+ end
+ end
+end
diff --git a/core/app/models/spree/gallery/variant_association.rb b/core/app/models/spree/gallery/variant_association.rb
new file mode 100644
index 00000000000..4eb322cf0cb
--- /dev/null
+++ b/core/app/models/spree/gallery/variant_association.rb
@@ -0,0 +1,50 @@
+module Spree
+ # Uses the ActiveRecord association between Spree::Variant and
+ # Spree::Image
+ module Gallery
+ class VariantAssociation < Gallery::Base
+
+ # Creates a gallery for the given variant
+ # @param [Spree::Variant] variant variant to build the gallery for
+ def initialize(variant)
+ @variant = variant
+ end
+
+ # (see Spree::Gallery#images)
+ def images
+ variant.images
+ end
+
+ # (see Spree::Gallery#primary_image)
+ def primary_image
+ variant.images.first
+ end
+
+ # Will fall back to the first image on the variant's product if
+ # the variant has no associated images.
+ # @return [Spree::Image, nil] The variant's primary image, if it exists,
+ # or the variant's products first image, if it exists, nil otherwise
+ def best_image
+ primary_image || products_images.first
+ end
+
+ # Return a list of image associations to preload a Spree::Variant's images
+ #
+ # @return An array compatible with ActiveRecord :includes
+ # for a Spree::Variant images preload
+ #
+ # (see Spree::Galery#preload_params)
+ def self.preload_params
+ [:images]
+ end
+
+ private
+ attr_reader :variant
+
+ def products_images
+ variant.product.try!(:variant_images) || []
+ end
+
+ end
+ end
+end
diff --git a/core/app/models/spree/product.rb b/core/app/models/spree/product.rb
index fb42d876f4f..8d530ac4ecd 100644
--- a/core/app/models/spree/product.rb
+++ b/core/app/models/spree/product.rb
@@ -270,6 +270,12 @@ def master
# variants. If all else fails, will return a new image object.
# @return [Spree::Image] the image to display
def display_image
+ ActiveSupport::Deprecation.warn(<'
+ end
+
+ context 'given an image' do
+ let(:image) { Spree::Image.new attachment_file_name: 'legit_image.png' }
+
+ it 'uses the provided image' do
+ expect(subject).to eq ' '
+ end
+ end
+
+ context 'large style used' do
+ let(:style) { :large }
+
+ it 'uses default image with large style' do
+ expect(subject).to eq ' '
+ end
+ end
+
+ context 'with options provided' do
+ let(:options) { { itemprop: 'image' } }
+
+ it 'passes the options to image_tag' do
+ expect(subject).to eq ' '
+ end
+ end
+ end
+end
diff --git a/core/spec/models/spree/gallery/product_variant_association_spec.rb b/core/spec/models/spree/gallery/product_variant_association_spec.rb
new file mode 100644
index 00000000000..279d05453d6
--- /dev/null
+++ b/core/spec/models/spree/gallery/product_variant_association_spec.rb
@@ -0,0 +1,83 @@
+require 'spec_helper'
+require 'spree/testing_support/shared_examples/gallery'
+
+RSpec.describe Spree::Gallery::ProductVariantAssociation, type: :model do
+ let(:gallery) { described_class.new(product) }
+
+ let(:product) { Spree::Product.new }
+
+ include_examples 'is a gallery'
+
+ shared_context 'has multiple images' do
+ let(:first_image) { Spree::Image.new }
+ let(:second_image) { Spree::Image.new }
+
+ before do
+ product.images << first_image
+ product.images << second_image
+ end
+ end
+
+ describe '#images' do
+ subject { product.images }
+
+ it 'is empty' do
+ expect(subject).to be_empty
+ end
+
+ context 'has multiple images' do
+
+ include_context 'has multiple images'
+
+ it 'has the images of the association' do
+ expect(subject).to eq [first_image, second_image]
+ end
+
+ end
+ end
+
+ shared_examples_for 'image picking' do
+ subject { gallery.best_image }
+
+ it 'is nil' do
+ expect(subject).to be_nil
+ end
+
+ context 'has images' do
+ include_context 'has multiple images'
+
+ it 'uses the first image' do
+ expect(subject).to eq first_image
+ end
+ end
+
+ context 'with non-master variant with images' do
+ let(:product) { create :product }
+ let(:variant) { create :variant, product: product }
+ let!(:image) { create :image, viewable: variant }
+
+ it 'falls back to the products image' do
+ expect(subject).to eq image
+ end
+ end
+ end
+
+ describe '#primary_image' do
+ include_examples 'image picking'
+ end
+
+ describe '#best_image' do
+ include_examples 'image picking'
+ end
+
+ describe '.preload_params' do
+ it 'uses the right preload_params' do
+ expect(described_class.preload_params).to eq [:variant_images]
+ end
+
+ it 'is valid on products' do
+ create :product
+ expect(Spree::Product.includes(described_class.preload_params).first).to be
+ end
+ end
+end
diff --git a/core/spec/models/spree/gallery/variant_association.rb b/core/spec/models/spree/gallery/variant_association.rb
new file mode 100644
index 00000000000..f46d6b66bc4
--- /dev/null
+++ b/core/spec/models/spree/gallery/variant_association.rb
@@ -0,0 +1,100 @@
+require 'spec_helper'
+require 'spree/testing_support/shared_examples/gallery'
+
+RSpec.describe Spree::Gallery::VariantAssociation, type: :model do
+ let(:gallery) { described_class.new(variant) }
+
+ let(:variant) { Spree::Variant.new }
+
+ include_examples 'is a gallery'
+
+ shared_context 'has multiple images' do
+ let(:first_image) { Spree::Image.new }
+ let(:second_image) { Spree::Image.new }
+
+ before do
+ variant.images << first_image
+ variant.images << second_image
+ end
+ end
+
+ describe '#images' do
+ subject { gallery.images }
+
+ it 'is empty' do
+ expect(subject).to be_empty
+ end
+
+ context 'has multiple images' do
+
+ include_context 'has multiple images'
+
+ it 'has the images of the association' do
+ expect(subject).to eq [first_image, second_image]
+ end
+
+ end
+ end
+
+ describe '#primary_image' do
+ subject { gallery.primary_image }
+
+ it 'is nil' do
+ expect(subject).to be_nil
+ end
+
+ context 'has images' do
+ include_context 'has multiple images'
+
+ it 'uses the first image' do
+ expect(subject).to eq first_image
+ end
+ end
+ end
+
+ describe '#best_image' do
+ subject { gallery.best_image }
+
+ it 'is nil' do
+ expect(subject).to be_nil
+ end
+
+ context 'has images' do
+ include_context 'has multiple images'
+
+ it 'uses the first image' do
+ expect(subject).to eq first_image
+ end
+ end
+
+ context 'variant has a product' do
+ let(:product) { Spree::Product.new }
+ let(:variant) { Spree::Variant.new product: product }
+
+ it 'is nil' do
+ expect(subject).to be_nil
+ end
+
+ context 'that has images' do
+ let(:product) { create :product }
+ let(:variant) { create :variant, product: product }
+ let!(:image) { create :image, viewable: product.master }
+
+ it 'falls back to the products image' do
+ expect(subject).to eq image
+ end
+ end
+ end
+ end
+
+ describe '.preload_params' do
+ it 'uses the right preload_params' do
+ expect(described_class.preload_params).to eq [:images]
+ end
+
+ it 'is valid on products' do
+ create :variant
+ expect(Spree::Variant.includes(described_class.preload_params).first).to be
+ end
+ end
+end
diff --git a/core/spec/models/spree/product_spec.rb b/core/spec/models/spree/product_spec.rb
index 10b4c725334..9cb5d910394 100644
--- a/core/spec/models/spree/product_spec.rb
+++ b/core/spec/models/spree/product_spec.rb
@@ -501,4 +501,13 @@ def build_option_type_with_values(name, values)
product = Spree::Product.new
expect(product.master.is_master).to be true
end
+
+ describe '#gallery' do
+ let(:product) { Spree::Product.new }
+ subject { product.gallery }
+
+ it 'is a Spree::Gallery::Base' do
+ expect(subject).to be_a Spree::Gallery::Base
+ end
+ end
end
diff --git a/core/spec/models/spree/variant_spec.rb b/core/spec/models/spree/variant_spec.rb
index 5fd1bd8acf4..b693812dc5e 100644
--- a/core/spec/models/spree/variant_spec.rb
+++ b/core/spec/models/spree/variant_spec.rb
@@ -598,4 +598,13 @@
end
end
end
+
+ describe '#gallery' do
+ let(:variant) { Spree::Variant.new }
+ subject { variant.gallery }
+
+ it 'is a Spree::Gallery::Base' do
+ expect(subject).to be_a Spree::Gallery::Base
+ end
+ end
end
diff --git a/frontend/app/controllers/spree/products_controller.rb b/frontend/app/controllers/spree/products_controller.rb
index c94e00369d7..43234104685 100644
--- a/frontend/app/controllers/spree/products_controller.rb
+++ b/frontend/app/controllers/spree/products_controller.rb
@@ -15,7 +15,8 @@ def index
end
def show
- @variants = @product.variants_including_master.active(current_currency).includes([:option_values, :images])
+ @variants = @product.variants_including_master.active(current_currency).includes(:option_values,
+ Spree::Config.variant_gallery_class.preload_params)
@product_properties = @product.product_properties.includes(:property)
@taxon = Spree::Taxon.find(params[:taxon_id]) if params[:taxon_id]
end
diff --git a/frontend/app/controllers/spree/store_controller.rb b/frontend/app/controllers/spree/store_controller.rb
index 5acddf03cf3..7206cecf1d6 100644
--- a/frontend/app/controllers/spree/store_controller.rb
+++ b/frontend/app/controllers/spree/store_controller.rb
@@ -2,6 +2,8 @@ module Spree
class StoreController < Spree::BaseController
include Spree::Core::ControllerHelpers::Order
+ helper 'spree/images'
+
skip_before_action :set_current_order, only: :cart_link
def unauthorized
diff --git a/frontend/app/views/spree/checkout/_delivery.html.erb b/frontend/app/views/spree/checkout/_delivery.html.erb
index 124bbbf0d9d..14aa9ff90e2 100644
--- a/frontend/app/views/spree/checkout/_delivery.html.erb
+++ b/frontend/app/views/spree/checkout/_delivery.html.erb
@@ -26,7 +26,7 @@
<% ship_form.object.manifest.each do |item| %>
- | <%= image_tag item.variant.display_image.attachment(:mini) %> |
+ <%= spree_image_tag(item.variant.gallery.best_image, :mini) %> |
<%= item.variant.name %> |
<%= item.quantity %> |
<%= display_price(item.variant) %> |
@@ -72,7 +72,7 @@
<% @differentiator.missing.each do |variant, quantity| %>
- | <%= image_tag variant.display_image.attachment(:mini) %> |
+ <%= spree_image_tag(variant.gallery.best_image, :mini) %> |
<%= variant.name %> |
<%= quantity %> |
<%= display_price(variant) %> |
diff --git a/frontend/app/views/spree/orders/_line_item.html.erb b/frontend/app/views/spree/orders/_line_item.html.erb
index 2a1c6e23734..645feffcecb 100644
--- a/frontend/app/views/spree/orders/_line_item.html.erb
+++ b/frontend/app/views/spree/orders/_line_item.html.erb
@@ -2,11 +2,7 @@
<%= order_form.fields_for :line_items, line_item do |item_form| -%>
|
- <% if variant.images.length == 0 %>
- <%= link_to image_tag(variant.product.display_image.attachment(:small)), variant.product %>
- <% else %>
- <%= link_to image_tag(variant.images.first.attachment.url(:small)), variant.product %>
- <% end %>
+ <%= link_to spree_image_tag(variant.gallery.best_image, :small), variant.product %>
|
<%= link_to line_item.name, product_path(variant.product) %>
diff --git a/frontend/app/views/spree/products/_image.html.erb b/frontend/app/views/spree/products/_image.html.erb
index b63b07742be..aa334c6652c 100644
--- a/frontend/app/views/spree/products/_image.html.erb
+++ b/frontend/app/views/spree/products/_image.html.erb
@@ -1,5 +1,5 @@
<% if defined?(image) && image %>
<%= image_tag image.attachment.url(:product), itemprop: "image" %>
<% else %>
- <%= image_tag @product.display_image.attachment(:product), itemprop: "image" %>
+ <%= spree_image_tag @product.gallery.best_image, :product, itemprop: "image" %>
<% end %>
diff --git a/frontend/app/views/spree/shared/_order_details.html.erb b/frontend/app/views/spree/shared/_order_details.html.erb
index 523108abacd..0da37977e88 100644
--- a/frontend/app/views/spree/shared/_order_details.html.erb
+++ b/frontend/app/views/spree/shared/_order_details.html.erb
@@ -61,11 +61,7 @@
<% order.line_items.each do |item| %>
|
|
- <% if item.variant.images.length == 0 %>
- <%= link_to image_tag(item.variant.product.display_image.attachment(:small)), item.variant.product %>
- <% else %>
- <%= link_to image_tag(item.variant.images.first.attachment.url(:small)), item.variant.product %>
- <% end %>
+ <%= link_to spree_image_tag(item.variant.gallery.best_image, :small), item.variant.product %>
|
<%= item.variant.product.name %>
diff --git a/frontend/app/views/spree/shared/_products.html.erb b/frontend/app/views/spree/shared/_products.html.erb
index 8ac55dc91a0..baf3435d3b2 100644
--- a/frontend/app/views/spree/shared/_products.html.erb
+++ b/frontend/app/views/spree/shared/_products.html.erb
@@ -28,7 +28,7 @@
" data-hook="products_list_item" itemscope itemtype="http://schema.org/Product">
<% cache(@taxon.present? ? [I18n.locale, current_currency, @taxon, product] : [I18n.locale, current_currency, product]) do %>
- <%= link_to image_tag(product.display_image.attachment(:small), itemprop: "image"), url, itemprop: 'url' %>
+ <%= link_to spree_image_tag(product.gallery.best_image, :small, itemprop: "image"), url, itemprop: 'url' %>
<%= link_to truncate(product.name, length: 50), url, class: 'info', itemprop: "name", title: product.name %>
| | | |