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

Improperly referenced product id when parsing CSV for bulk product update via import strategy #2350

Closed
a-finocchiaro opened this issue Oct 4, 2022 · 6 comments

Comments

@a-finocchiaro
Copy link
Contributor

Bug report

Describe the bug

In the ProductImportStrategy class, the getImportInstructions improperly checks the product.id for each row. Please see the code snippets section for the exact line.

With it setup this way, products will always be considered new because the product.id column is not read here. So products will never be updated.

Expected behavior

I would expect this to be row['product.id'] instead.

Code snippets

@ayushthe1
Copy link
Contributor

Hey ,I would like to work on this issue .

@a-finocchiaro
Copy link
Contributor Author

Hey ,I would like to work on this issue .

Go for it! I have actually already fixed it in another project where I'm actually using medusa, I just haven't done the same on my forked version of it but if you'd like to fix it, you're more than welcome.

@ayushthe1
Copy link
Contributor

Thanks @a-finocchiaro .
I made a pr for this issue . Could you please review it.

@olivermrbl
Copy link
Contributor

Nice catch @a-finocchiaro and appreciate the contribution @ayushthe1 💪

@ayushthe1
Copy link
Contributor

Nice catch @a-finocchiaro and appreciate the contribution @ayushthe1 💪

Thanks . Would have liked to fully fix the issue but currently I don't know much.

kodiakhq bot pushed a commit that referenced this issue Oct 7, 2022
…tegy (#2351)

* What - fixing PR #2350 

* Why - In the ProductImportStrategy class, the getImportInstructions improperly checks the product.id for each row. With it setup this way, products will always be considered new because the product.id column is not read here. So products will never be updated. [Link to the code line](https:/medusajs/medusa/blob/bd941309161bcee4fbfc7918d310c426ba8d8f15/packages/medusa/src/strategies/batch-jobs/product/import.ts#L166)

* How - Replacing `row["product.product.id"]` to  `row["product.id"]`

Co-authored-by: Oliver Windall Juhl <[email protected]>
@olivermrbl
Copy link
Contributor

Solved #2351

Again, thanks for the contribution. To show our appreciation, we'd love to send you some swag/merch. If that's of interest reach out to me on our Discord and we'll make it happen! 💪

adrien2p pushed a commit that referenced this issue Oct 11, 2022
…tegy (#2351)

* What - fixing PR #2350 

* Why - In the ProductImportStrategy class, the getImportInstructions improperly checks the product.id for each row. With it setup this way, products will always be considered new because the product.id column is not read here. So products will never be updated. [Link to the code line](https:/medusajs/medusa/blob/bd941309161bcee4fbfc7918d310c426ba8d8f15/packages/medusa/src/strategies/batch-jobs/product/import.ts#L166)

* How - Replacing `row["product.product.id"]` to  `row["product.id"]`

Co-authored-by: Oliver Windall Juhl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants