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

Apply import defaults to variants of new products in Spree 2 #3425

Merged
merged 1 commit into from
Feb 20, 2019
Merged

Apply import defaults to variants of new products in Spree 2 #3425

merged 1 commit into from
Feb 20, 2019

Conversation

mkllnk
Copy link
Member

@mkllnk mkllnk commented Feb 1, 2019

What? Why?

Closes #2783

Spree 2 doesn't have Product#on_hand= any more. We actually can't save
stock levels without saving the product first which creates a stock
item.

This hacky solution overwrites the attribute in the entry so that it
gets copied to the variant later on. I think a better solution needs
some refactoring as proposed in:
#2783 (comment)

What should we test?

Product import with some default settings to overwrite stock levels and other properties.

Release notes

Changed: Updated the product import code for future Spree updates.

Changelog Category: Changed

How is this related to the Spree upgrade?

This fixes overriding stock levels in Spree 2.

@mkllnk mkllnk self-assigned this Feb 1, 2019
Copy link
Contributor

@luisramos0 luisramos0 left a comment

Choose a reason for hiding this comment

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

That is a smart solution!!!

Spree 2 doesn't have `Product#on_hand=` any more. We actually can't save
stock levels without saving the product first which creates a stock
item.

This hacky solution overwrites the attribute in the entry so that it
gets copied to the variant later on. I think a better solution needs
some refactoring as proposed in:
#2783 (comment)
@sigmundpetersen
Copy link
Contributor

Should this be staged on fr staging then?

@luisramos0
Copy link
Contributor

no, this is master, should follow normal testing process.
it will then be merged to 2-0 and help fix a problem there.

@sigmundpetersen
Copy link
Contributor

Yes of course 🙈 Was confused by the Spree Upgrade prefix. And I was thinking we had started testing every Spree PR 😺

@RachL RachL added the pr-staged-uk staging.openfoodnetwork.org.uk label Feb 11, 2019
@RachL RachL self-assigned this Feb 12, 2019
@RachL
Copy link
Contributor

RachL commented Feb 12, 2019

@mkllnk I've tried the very simple case of trying to create a product with the import and my product category was not recognized although I copy-paste the category from the category list...

Any idea where it can come from?

My testing notes:

https://docs.google.com/document/d/14OpOmsyk-jcdkmIrnL61PACZweg5OCArwSaH5rzLTx8/edit#

@RachL RachL removed the pr-staged-uk staging.openfoodnetwork.org.uk label Feb 12, 2019
@mkllnk
Copy link
Member Author

mkllnk commented Feb 14, 2019

@RachL I looked at your testing notes and I'm very surprised. I tested it on my computer and it all worked. Can you test it on another staging server?

If it works somewhere else, we need to work out why product import is not working on UK staging. @Matt-Yorkley Any idea?

@RachL
Copy link
Contributor

RachL commented Feb 18, 2019

@mkllnk oops sorry missed your comment. Yes will do! Moving this to test ready again. @sigmundpetersen I'm not sure I understand why you put it back in dev. Shout if you think it deserves something else than a new test.

@sigmundpetersen
Copy link
Contributor

Actually it was you who originally put it back to dev @RachL 😆 I just put it back and forth

@RachL
Copy link
Contributor

RachL commented Feb 18, 2019

@sigmundpetersen yes indeed, I always clear the testing column, but it seems to me that after Maikel's comment you put it back on test ready, but changed your mind. Anyway if it's not the case, we are all good :)

@RachL RachL added the pr-staged-au staging.openfoodnetwork.org.au label Feb 18, 2019
@RachL
Copy link
Contributor

RachL commented Feb 20, 2019

@mkllnk it worked on AU staging :) I've found some little things, but that where not introduce by this PR, so moving this to ready to go!

https://docs.google.com/document/d/14OpOmsyk-jcdkmIrnL61PACZweg5OCArwSaH5rzLTx8/edit#

@RachL RachL removed the pr-staged-au staging.openfoodnetwork.org.au label Feb 20, 2019
@luisramos0
Copy link
Contributor

I dont usually merge to master, I am merging this one to get it into the "merge master into 2-0" PR that I want to create now.

@luisramos0 luisramos0 merged commit 34acaa4 into openfoodfoundation:master Feb 20, 2019
@mkllnk mkllnk deleted the 2783-product-importer-spec branch June 25, 2019 04:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Spree Upgrade][PI] Make Product Import create stock items (1 broken spec)
5 participants