search

solidusio-contrib / solidus_importer

Solidus importer extension to migrate data from other eCommerce systems
BSD 3-Clause "New" or "Revised" License
15 stars 30 forks source link

Surface after_import errors #69

Closed cesartalves closed 3 years ago

cesartalves commented 3 years ago

Currently, if the after_import method of the importer generates an error, the ProcessImport will update the import state accordingly. However, there's no way to see the reason why the process failed, which is relevant where the after_import does most of the persisting work, ie on OrderImporter.

This change simply joins all error messages found during the Order persistence and tweaks ProcessImport so that it updates the final SolidusImporter::Import message accordingly

cesartalves commented 3 years ago

@cesartalves I think this looks good.

Out of curiosity, is there a reason that you're not passing an array until the very end for context[:messages], and joining them in the @import.update call? It looks like that would spare you the need to also create a confusing context[:_messages] key.

Just wanted to flag it, but I don't think it's blocking.

No reason in particular, really. I thought this was a bit more cautious because I didn't know whether I would be overriding a messages key already present on the context. But it seems to be completely unnecessary. I will adjust accordingly 👍 Thanks for flagging this