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
Added customers import to shopify connector #3252
Added customers import to shopify connector #3252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wayann this is great! Thanks for working on this.
One suggestion here: I'd prefer to have a single "import" button that imports all resources that are selected. This is initially why I added a checkbox for Products. We can then have a checkbox for Products, Customers, Orders, etc
This will probably require a new "aggregation" method that we can call from the client with the selected resources that can then schedule the imports for each resource.
Another thought:
As we add additional resources and these imports grow larger, I think it reinforces the need to move these imports to a queue where they can be processed outside of the main thread, or even off of the app server.
I started working towards that a bit in the Products importer by setting up a Job for each image that needs to get imported, and I think that could be used as a reference.
You don't necessarily have to set this up as a Job as part of this PR, but I want to plant the seed because that's the direction I'd like our connectors services to head.
@kieha is working on converting this dashboard to React as well, and that should probably be considered before we merge this PR.
@kieha will you chime in with any work here that might overlap with your current work on the Shopify dashboard?
@spencern @wayann there's no major overlap here with what I'm working on. As you said, instead of having 2 import buttons, the checkboxes should be used to decide what to import, then the single import button handles all the selected options. As for the React conversion, the only thing that I'd need to add UI-wise is the |
@spencern @kieha thanks for considering my PR Single import button [commit] (46e607c) I hope this won't break React conversion from @kieha it crashed several times when importing 22k customers, just need to understand how @spencern did it with the images jobs, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made a few adjustments to the code here, but overall, this looks like a good PR.
It adds customer importing functionality, without causing any errors or removing any existing functionality.
One drawback is the added code is in Blaze - but this was an existing Blaze template, and therefore on our end for not having this whole component updated to React.
@kieckhafer 🥇 👍 ;) |
@zenweasel as said earlier in the chat I've tested successfully this import with 22k customers and 620 products.