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

Support for imports and migrations #467

Closed
tdecaluwe opened this issue Sep 26, 2015 · 4 comments
Closed

Support for imports and migrations #467

tdecaluwe opened this issue Sep 26, 2015 · 4 comments
Assignees

Comments

@tdecaluwe
Copy link
Contributor

I've recently started working on a reaction integration with a (rather obscure) ERP software used at our company. This means different kinds of entities need to be imported and possibly synced with their counterparts in the ERP database.

However, the basic functionality will be the same when doing SAP, Odoo, Magento or #378 integrations/migrations. Wrapping these in a simple import API might avoid duplicating these functionalities. I'm essentially talking about a set of idempotent operations importing products, variants, prices, stock, invoices, images... I think these should be provided by reaction-core (also it doesn't add client code).

Plugins can be written to talk to different ERP's. Automatic importing could also be provided by a plugin.

Anyway, this issue was in the first place meant as a place to discuss. Thoughts?

@aaronjudd
Copy link
Contributor

There is certainly a need for some good integration points and something we've talked about a fair amount - but haven't yet made any concrete plans. We do have the Fixtures.load functionality which is available for packages to use, but we'd need to add a flag to allow it to import where a collection has existing data - and it's just a json importer.. I think what we really need here is a couple of structured endpoints specifically for importing and maintaining synced live, or occasional updates from stream. It should be noted that most of this functionality exists already as Meteor / DDP methods that you can use remotely (either in a meteor package or through some of the DDP packages out there) (see: reaction-core/server/methods/*). You can also hook into the collection or methods with hooks from your package - there's some possibilities there as well.

@tdecaluwe
Copy link
Contributor Author

Some things to consider:

  • It might be useful to provide the meteor methods also as normal js functions (perhaps in the ReactionCore object) so we don't have to use Meteor.call when calling them from the server.
  • Importing should be able to leverage bulk insertions and updates in the future. To keep things simple, read operations should be avoided (this is a problem with the meteor methods already provided). This allows one to send the import as a bunch of queries to the mongo daemon.
  • Multiple imports after one another shouldn't affect the outcome. Import methods should be idempotent.
  • Import methods (and by extension all collection methods) should be able to work with keys that aren't just _id's. This might make it easier to map db structures used in external software.

A first use case to start on the integrations might be to support simple importing of product catalogs. This however has some dependencies on #150. My idea for a first implementation might consist of:

  • An import buffer
  • Methods to add products to the import buffer
  • A method which imports the buffer into MongoDB

(something along the lines of import.js)

@aaronjudd
Copy link
Contributor

This might be a little late, as we've had a few discussions elsewhere, but this is an update to bring this issue up to speed, and I've also thrown in some related points here and links to other pertinent issue so that we can get a cohesive coverage on this.

We've discussed replacing some of the current core fixture load with new functionality being developed in the https://github.com/tdecaluwe/reaction-import package.

This is inline with ongoing generalized efforts to make reaction core more modular. This is referenced in #451, but has always been a core principal.

Some of the current fixture data truly is fixture data, not just sample import data, i.e.: there are not methods to generate the required values in some cases (like currencies,etc) so it might be tough to get a properly configured shop without this sample-data to start with. Factories also aren't generating a full data set, right now the fixtures .json has the most accurate and complete data.

Any changes in #508 should remain compatible with the current structure if at all possible, or at least let's make sure that we document the breaking change. Particularly if we're thinking that it will require an update like https://github.com/reactioncommerce/reaction-shipping/pull/4, and possibly a PR to reaction-sample-data.

The philosophy I had been following, is that packages should be more independent from each other, or core, and need to be able to load their own fixture data (which is why core, and shipping follow this paradigm). After all, if you don't want to use reaction-shipping, you'll still be getting its shipping data if it's included in a centralized sample-data.

It also doesn't make any sense to have a sample-data package for each package, so I think this is another argument for packages importing their own fixture loadData.

At least for non-core packages, the Allow bringing your own fixture data issue #508 at first glance seems to be the functionality that already was provided with

Fixtures.loadData(collection, json);

Simply leaving Fixtures.loadData functionally intact but updating to use new import methods seems like it would be reasonable, as the current implementation also address the issue of Meteor not allowing cross package use of Assets.get.

However, then you'd continue to have the same issue if you wanted to override the data in reaction-shipping. I think the sticky point, maybe already solved(?), is... can ReactionImport can get assets from other packages, if so, I think we'd be good with a complete replacement. Perhaps we'll still need to load assets from the packages, but then can add to an import buffer from the individual packages.

Image importing would resolve #60, but seems like ReactionImport.product could use some expansion to create the import relationship to the images?

Consider #317 as well, for some additional thoughts about package acceptance in general.

Documentation will need to be updated in fixtures.md Hopefully we can do a thorough update to those docs, particularly with some example import routines.

@aaronjudd aaronjudd added this to the Import Migration milestone Nov 6, 2015
@tdecaluwe
Copy link
Contributor Author

About some of the remarks you made:

  • Yes, I think packages should be able to import their own fixture data. This is indeed conceptually different from the example products and tags found in the data in core. I'll update reactioncommerce/reaction-shipping#4 accordingly.

  • To accomodate for this conceptual difference I was thinking about providing a fixture() method which modifies the importer to accept fixture data instead (only importing when the data isn't present).

    ReactionImport.product(key, doc)
    ReactionImport.fixture().product(key, doc)

    This does still imply a change in how the loadData method is called however, since every import expects a key. I personally think this is an improvement. This allows us to make sure some fixture data is present even if different packages try to insert fixture data into the same collection.

  • About documentation, I'll update everything when te groundworks are finsihed. JSDocs are mostly in place, and I'll merge the reaction-import into the core docs.

  • About overriding fixture data in reaction-shipping: This can be accomplished by updating the data already present in reaction-shipping. Alternatively a package could decide to make it's data overrideable by pushing data to the buffer without flushing which would allow for another package to empty the buffer and push it's own data. I'm not sure either of those are satisfactory or which is better.

cmbirk pushed a commit to cmbirk/reaction that referenced this issue Aug 18, 2019
…-update-aug-2-2019

Daily update aug 2 2019
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

No branches or pull requests

2 participants