Not able to change images size

[sechix]

I am trying to change sizes with this in the initializer:
Spree::Image.attachment_definitions[:attachment][:styles] = {
mini: '48x48>',
small: '100x100>',
product: '240x240>',
large: '600x600>'
}
But 'module Spree::Image::ActiveStorageAttachment' don't take new sizes. How to do it?

Solidus Version:
Last version of master

To Reproduce

[sechix]

Hi, any idea? @kennyadsl

[hefan]

It doesn't seem to work any longer with Rails 6 / Zeitwerk in an initializer.
Somehow it gets overriden again with the default styles from core/app/models/spree/image/paperclip_attachment.rb.
To get it to work with Rails 6 you have to decorate Spree::Image and do something like

module ImageDecorator

  def self.prepended(base)
    base.attachment_definitions[:attachment][:styles] = {
      mini: '48x48>', small: '100x100>', product: '240x240>', large: '600x600>'
    }
  end

  Spree::Image.prepend self
end

For more details about Decorations: https://guides.solidus.io/developers/customizations/decorators.html

With Rails 5.2 it works fine

@kennyadsl i am not sure how to make it work again in Zeitwerk. Maybe you can suggest something?

[kennyadsl]

I think it's a filename issue. Where this ImageDecorator lives exactly?

[hefan]

The Image Decorator would be a workaround. Normally

Spree::Image.attachment_definitions[:attachment][:styles] = {
  mini: '48x48>',
  small: '100x100>',
  product: '240x240>',
  large: '600x600>'
}

should work in an initializer like stated here Product images which seems not to work in rails 6

[kennyadsl]

Oh, you are right. This seems like a bug then, I'll try to take a look asap.

[EduardoGHdez]

There is another workaround

You are able to set your own image_attachment_module from Spree::Config 😄 , rather than decorating
https://github.com/solidusio/solidus/blob/master/core/lib/spree/app_configuration.rb#L477-L483

[nbelzer]

Given that this issue has been open for a while, I decided to verify if the bug still exists. Below I have described my findings:

To update the product_image_styles I was able to use the following initializer snippet:

Spree.config do |config|
  ...
  config.product_image_styles = {
    mini: '48x48>',
    small: '100x100>',
    product: '240x240>',
    large: '600x600>'
  }
  ...
end

This configuration option is used in Spree::Image::PaperclipAttachment andSpree::Image::ActiveStorageAttachment, of which by default the later is included in Spree::Image as image_attachment_module by default. Lets focus on the Spree::Image::ActiveStorageAttachment first:

solidus/core/app/models/spree/image/active_storage_attachment.rb

Lines 14 to 16 in cc56661

	has_attachment :attachment,
	styles: Spree::Config.product_image_styles,
	default_style: Spree::Config.product_image_style_default

The method has_attachment stems from Spree::ActiveStorageAdapter, the following snippet shows how calling has_attachment sets the @attachment_definition which is used in the attachment_definitions (note the plural name) method.

solidus/core/app/models/concerns/spree/active_storage_adapter.rb

Lines 32 to 46 in 3ba6bc6

	def has_attachment(name, definition)
	@attachment_name = name.to_sym
	@attachment_definition = definition
	has_one_attached attachment_name
	override_reader
	override_writer
	define_image_validation
	define_presence_reader
	end
	def attachment_definitions
	{ attachment_name => attachment_definition }
	end

Therefore by using the configuration snippet above you are able to customize the attachment styles, therefore there is no longer a need to override or patch the attachment_definitions method. Looking at the git blame for when the configuration option was introduced, it seems that the author indeed meant to deprecate the need to override the attachment_definitions.

Both the developer guides and edgeguides still recommend using the following snippets:

# From the developer guides
Spree::Image.attachment_definitions[:attachment][:styles] = {
  mini: '48x48>',
  small: '100x100>',
  product: '240x240>',
  large: '600x600>'
}

# From the EdgeGuides
module AmazingStore
  module Spree
    module Image
      module CustomizeSizes
        def self.prepended(klass)
          klass.attachment_definitions[:attachment][:styles] = {
            mini: '48x48>',
            small: '400x400>',
            product: '680x680>',
            large: '1200x1200>',
            jumbo: '1600x1600>'
          }
        end

        ::Spree::Image.prepend self
      end
    end
  end
end

As this is outdated, I recommend we use this issue to create a PR that updates the documentation accordingly, and adds some tests such that we verify this behaviour (I wasn't able to find any existing tests). Once merged, I believe we can close this issue. I will take a look at doing this tomorrow.

What about the PaperclipAttachment?

When the Spree::Config.image_attachment_module is set to Spree::Image::PaperclipAttachment our Spree::Config.product_image_styles are passed to has_attached_file as seen in the following snippet:

solidus/core/app/models/spree/image/paperclip_attachment.rb

Lines 9 to 15 in cc56661

	has_attached_file :attachment,
	styles: Spree::Config.product_image_styles,
	default_style: Spree::Config.product_image_style_default,
	default_url: 'noimage/:style.png',
	url: '/spree/products/:id/:style/:basename.:extension',
	path: ':rails_root/public/spree/products/:id/:style/:basename.:extension',
	convert_options: { all: '-strip -auto-orient -colorspace sRGB' }

This is a method defined by kt-paperclip which accepts these styles and default styles as defined in the gems documentation. So the configuration is also correctly applied for the Spree::Image::PaperclipAttachment.

[kennyadsl]

As this is outdated, I recommend we use this issue to create a PR that updates the documentation accordingly, and adds some tests such that we verify this behaviour (I wasn't able to find any existing tests). Once merged, I believe we can close this issue. I will take a look at doing this tomorrow.

I like the plan, when documenting this, please be aware that not all the Solidus versions support ActiveStorage (I think it has been introduced in 2.11), so we should reflect that in the documentation. Thanks!