fix: call _fetch_item from _process_image for easier override #4280
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #4280 +/- ##
==========================================
+ Coverage 88.28% 88.36% +0.08%
==========================================
Files 293 293
Lines 16046 16093 +47
==========================================
+ Hits 14166 14221 +55
+ Misses 1880 1872 -8
|
|
hi @Chadys thanks for your contributions! We actually never use this Importer in production. see: https://github.com/django-oscar/django-oscar/blob/master/src/oscar/apps/catalogue/utils.py#L25 so I don't think will merge this. Recently I have been working on a new project for easily mapping catalogue data from random input (in any format like json, xml, excel etc.) to product models, and from product models to any other format. you should check out django-oscar-odin: https://github.com/django-oscar/django-oscar-odin Could be really beneficial to your marketplace integrations as it makes creating products much faster (bulk updates and creates) |
|
I never used the I'm not working for that marketplace anymore (that's the reason why I finally have some time to myself haha), but some utilities to map catalogue data would definitely have been useful! |
In
catalogue.Importer, we have the_fetch_itemfunction as a practical override entry point.But instead of using it,
_process_imageis reimplementing the same logic directly in the function.I fixed it so that
_fetch_itemis used and I removed the now unnecessary lookup argument in_process_image.With those changes, in
handleL.41, thelookup_valueis retrieved but is only used to construct some error logs. I think we should change the text of the logs so as not to need to retrieve this value just for them, but I wanted to discuss about it first.(PS: I have been working using Oscar for more than 2 years now, building a marketplace from scratch. I finally have some times to give back to that awesome project by providing some fixes for issues I found along the way so I'll be pretty active in the following days!)