Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add concept of Image gallery #625

Closed
wants to merge 16 commits into from
Closed
10 changes: 8 additions & 2 deletions api/app/controllers/spree/api/variants_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def destroy
# we render on the view so we better update it any time a node is included
# or removed from the views.
def index
@variants = scope.includes({ option_values: :option_type }, :product, :default_price, :images, { stock_items: :stock_location })
@variants = scope.includes(include_list)
.ransack(params[:q]).result.page(params[:page]).per(params[:per_page])
respond_with(@variants)
end
Expand All @@ -32,7 +32,7 @@ def new
end

def show
@variant = scope.includes({ option_values: :option_type }, :option_values, :product, :default_price, :images, { stock_items: :stock_location })
@variant = scope.includes(include_list)
.find(params[:id])
respond_with(@variant)
end
Expand Down Expand Up @@ -70,6 +70,12 @@ def scope
def variant_params
params.require(:variant).permit(permitted_variant_attributes)
end

def include_list
[{ option_values: :option_type }, :product, :default_price,
Spree::Config.variant_gallery_class.preload_params,
{ stock_items: :stock_location }]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The readability of this could be improved by putting one thing per line:

[
  :default_price,
  :product,
  { option_values: :option_type },
  { stock_items: :stock_location },
  Spree::Config.variant_gallery_class.preload_params
]

end
end
end
end
1 change: 1 addition & 0 deletions backend/app/controllers/spree/admin/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ module Admin
class BaseController < Spree::BaseController
helper 'spree/admin/navigation'
helper 'spree/admin/tables'
helper 'spree/images'
layout '/spree/layouts/admin'

before_action :authorize_admin
Expand Down
9 changes: 7 additions & 2 deletions backend/app/controllers/spree/admin/products_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,15 @@ def update_before
end

def product_includes
[{ variants: [:images], master: [:images, :default_price] }]
[{ variants: variant_image_includes, master: (variant_image_includes + [:default_price]) }]
end

def clone_object_url(resource)
clone_admin_product_url resource
end

def variant_stock_includes
[:images, stock_items: :stock_location, option_values: :option_type]
[variant_image_includes, stock_items: :stock_location, option_values: :option_type]
end

def variant_scope
Expand All @@ -146,6 +146,11 @@ def variant_scope
def updating_variant_property_rules?
params[:product][:variant_property_rules_attributes].present?
end

private
def variant_image_includes
Spree::Config.variant_gallery_class.preload_params
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,11 @@ def load_stock_management_data
@stock_item_stock_locations = params[:stock_location_id].present? ? @stock_locations.where(id: params[:stock_location_id]) : @stock_locations
@variant_display_attributes = self.class.variant_display_attributes
@variants = Spree::Config.variant_search_class.new(params[:variant_search_term], scope: variant_scope).results
@variants = @variants.includes(:images, stock_items: :stock_location, product: :variant_images)
@variants = @variants.includes(
Spree::Config.variant_gallery_class.preload_params,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer some simpler indentation rather than sticking everything far to the right:

@variants = @variants.includes(
  Spree::Config.variant_gallery_class.preload_params,
  stock_items: :stock_location,
  product: Spree::Config.product_gallery_class.preload_params
)

stock_items: :stock_location,
product: Spree::Config.product_gallery_class.preload_params
)
@variants = @variants.includes(option_values: :option_type)
@variants = @variants.order(id: :desc).page(params[:page]).per(params[:per_page] || Spree::Config[:orders_per_page])
end
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/cancellations/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
<% @inventory_units.each do |inventory_unit| %>
<tr class="inventory-unit">
<td class="inventory-unit-image">
<%= image_tag inventory_unit.variant.display_image.attachment(:mini) %>
<%= spree_image_tag inventory_unit.variant.gallery.best_image, :mini %>
</td>
<td class="inventory-unit-name">
<%= inventory_unit.variant.product.name %><br><%= "(" + variant_options(inventory_unit.variant) + ")" unless inventory_unit.variant.option_values.empty? %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% carton.manifest_for_order(order).each do |item| %>
<tr class="stock-item" data-item-quantity="<%= item.quantity %>">
<td class="item-image">
<%= image_tag item.variant.display_image.attachment(:mini) %>
<%= spree_image_tag item.variant.gallery.best_image, :mini %>
</td>
<td class="item-name">
<%= item.variant.product.name %><br><%= "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? %>
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/orders/_line_items.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<tbody>
<% order.line_items.each do |item| %>
<%= content_tag :tr, class: 'line-item', id: "line-item-#{item.id}", data: { line_item_id: item.id } do %>
<td class="line-item-image"><%= image_tag item.variant.display_image.attachment(:mini) %></td>
<td class="line-item-image"><%= spree_image_tag item.variant.gallery.best_image, :mini %></td>
<td class="line-item-name">
<%= item.variant.product.name %><br><%= "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? %>
<% if item.variant.sku.present? %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% shipment_manifest.each do |item| %>
<tr class="stock-item" data-item-quantity="<%= item.quantity %>">
<td class="item-image">
<%= image_tag item.variant.display_image.attachment(:mini) %>
<%= spree_image_tag item.variant.gallery.best_image, :mini %>
</td>
<td class="item-name">
<%= item.variant.product.name %><br><%= "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? %>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<% shipment.manifest.each do |item| %>
<tr class="stock-item" data-item-quantity="<%= item.quantity %>">
<td class="item-image">
<%= image_tag item.variant.display_image.attachment(:mini) %>
<%= spree_image_tag item.variant.gallery.best_image, :mini %>
</td>
<td class="item-name">
<%= item.variant.product.name %><br><%= "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? %>
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/products/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
<% @collection.each do |product| %>
<tr <%== "style='color: red;'" if product.deleted? %> id="<%= spree_dom_id product %>" data-hook="admin_products_index_rows" class="<%= cycle('odd', 'even') %>">
<td class="align-center"><%= product.sku rescue '' %></td>
<td class="align-center"><%= image_tag product.display_image.attachment(:mini) %></td>
<td class="align-center"><%= spree_image_tag product.gallery.best_image, :mini %></td>
<td><%= link_to product.try(:name), edit_admin_product_path(product) %></td>
<td class="align-center"><%= product.display_price.to_html rescue '' %></td>
<td class="actions" data-hook="admin_products_index_row_actions">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
<td class="align-center no-padding" rowspan="<%= row_count %>">
<div class='variant-container'>
<div class='variant-image'>
<%= image_tag(variant.display_image(fallback: false).attachment(:small)) %>
<%= spree_image_tag(variant.gallery.primary_image, :small) %>
</div>
<div class='variant-details'>
<table class='stock-variant-field-table'>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
<td class="align-center no-padding">
<div class='variant-container' data-variant-id="<%= variant.id %>">
<div class='variant-image'>
<%= image_tag(variant.display_image(fallback: false).attachment(:small)) %>
<%= spree_image_tag(variant.gallery.primary_image, :small) %>
</div>
<div class='variant-details'>
<table class='stock-variant-field-table'>
Expand Down
2 changes: 1 addition & 1 deletion backend/app/views/spree/admin/users/items.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
<tr class="stock-item" data-item-quantity="<%= item.quantity %>">
<td class="align-center order-completed-at"><%= l(order.completed_at.to_date) if order.completed_at %></td>
<td class="item-image">
<%= image_tag item.variant.display_image.attachment(:mini) %>
<%= spree_image_tag item.variant.gallery.best_image, :mini %>
</td>
<td class="item-name">
<%= item.product.name %><br><%= "(" + variant_options(item.variant) + ")" unless item.variant.option_values.empty? %>
Expand Down
32 changes: 32 additions & 0 deletions core/app/helpers/spree/images_helper.rb
Original file line number Diff line number Diff line change
@@ -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
16 changes: 16 additions & 0 deletions core/app/models/spree/app_configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
51 changes: 51 additions & 0 deletions core/app/models/spree/gallery/base.rb
Original file line number Diff line number Diff line change
@@ -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<Spree::Image>] 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
46 changes: 46 additions & 0 deletions core/app/models/spree/gallery/product_variant_association.rb
Original file line number Diff line number Diff line change
@@ -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
50 changes: 50 additions & 0 deletions core/app/models/spree/gallery/variant_association.rb
Original file line number Diff line number Diff line change
@@ -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
Loading