Should we touch product if variant is master too? #1583

Closed
joneslee85 opened this Issue May 23, 2012 · 4 comments

Comments

Projects
None yet
2 participants
Member

joneslee85 commented May 23, 2012

In Variant before_save :touch_product:

https://github.com/spree/spree/blob/master/core/app/models/spree/variant.rb#L149

It seems to me that we should touch product if variant is master. Any thought?

Member

radar commented May 23, 2012

The master variant is only updated when a product is updated also, and so the product will be "touched" then. When updating a variant on its own, it doesn't touch the product and so this callback needs to be there.

Is there a problem that you're seeing that would make you think that the product needs to be touched?

Member

joneslee85 commented May 23, 2012

@radar: I see, what if the master variant is updated (I know we should not touch master variant directly but I do in few occasions) so should it touch product?

IMHO, I think we should not care if it is master or not.

Member

radar commented May 23, 2012

I don't see anything that could go wrong if we removed that restriction. Do the core tests still pass if you remove it?

Member

joneslee85 commented May 24, 2012

@radar: I think so too. My opinion is not to enforce unnecessary rule. Have not yet run any test, but I think it should pass.

tvdeyen pushed a commit to magiclabs/spree that referenced this issue Jun 1, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment