remove redundant delegation from product model

[brchristian]

It appears that since b6cd1ad we have declared delegate twice for the :display_amount and :display_price attributes in the product model -- once in the MASTER_ATTRIBUTES line and once explicitly below.

This PR removes the redundant second declaration.

[kennyadsl]

I'm not sure all attributes inside the MASTER_ATTRIBUTES array should have a setter. For example the 2 duplicated attributes do not seem to have setters method. Maybe it's better to remove stuff from the MASTER_ATTRIBUTES array if not needed.

[brchristian]

Hi @kennyadsl thanks for taking a look. To clarify, you would like me to leave :display_amount and :display_price in the getter method line below, but remove them from the MASTER_ATTRIBUTES getter+setter block above?

[kennyadsl]

Yes, I think it's better at a first look, even if I honestly didn't investigate deeply.

I think we don't have display_amount= and display_price= on Spree::Variant (or somewhere else) so it's probably better to do not delegate them, you can test this by trying to assign for example a display_price to a variant:

# rails console
> Spree::Product.first.display_price = 12
NoMethodError: undefined method `display_price=' for #<Spree::Variant:0x007fa80910e498>

They are not technically attributes, so I think it's better to have them in the delegate below, which just delegate the method without the method_name= part.

Along with those two methods, I think we can remove other elements from that list, but maybe we can do that in another PR?

Sorry for not being clear on my first comment and thanks for your contribution.

[brchristian]

Your earlier comment was quite clear, I just wanted to double check before I did anything!

That all makes total sense to me and I’ve made the changes and pushed the latest version to this branch just now.

[jhawthorn]

Thank you