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

Delete import products #17

Open
wants to merge 41 commits into
base: master
Choose a base branch
from

Conversation

sbounmy
Copy link

@sbounmy sbounmy commented Mar 25, 2012

Hi Joshua,

Along with #16

I need admin to be able to 'rollback' imports if anything goes wrong so I basically implemented it for my own usage, if you don't feel the usage of this feature, I totally understand :).

This pull request :

  • set the import process in a SQL transaction so we can rollback when an exception occurs : prevent half created importation.
  • allows admin to track what product an import have been created
  • admin can delete import which destroy associated products

This PR is quite long but has model and integration tests aswell, let me know if you have any questions.

@sbounmy sbounmy mentioned this pull request Mar 25, 2012
@joshmcarthur
Copy link
Owner

I'll have to think about this one.

Things I like:

  • Tests
  • Wrapping the import in a transaction

Things I'm not sure about:

  • Adding a whole new way of managing this import - I think it's cool, and it's something I've considered as well, but it's still a pretty big change. I'll have to have a think on this point.

@sbounmy
Copy link
Author

sbounmy commented Mar 25, 2012

Thanks for your feedback @joshmcarthur !

Sure, take your time and let me know if you have any questions !

sbounmy added a commit to sbounmy/spree-import-products that referenced this pull request Mar 27, 2012
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.

6 participants