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

Fix some issues with supplier barcode plugins #5919

Merged
merged 6 commits into from
Nov 15, 2023

Conversation

30350n
Copy link
Contributor

@30350n 30350n commented Nov 15, 2023

I went through a bunch of parts with the new barcode scanning functionality again and noticed some bugs / possible improvements:

  1. the most obvious one is in get_supplier_parts(...) in mixins.py (accidentally doing a fresh query there instead of reusing the existing one)
  2. the digikey plugin should only handle barcodes which contain a SKU (the issue here is that digikey/mouser barcodes look identical, so for mouser barcodes, the digikey plugin tries to parse the barcode and calls get_supplier_parts(...), makes a bunch of queries and fails, then the mouser one gets called and does the same queries again)
  3. if any plugin finds more than one valid SupplierPart via get_supplier_parts I return an error response, which leads to that error being directly returned to the end user. i think the better behavior would be to let the other plugins run first and only return the error if all of them fail (don't return a result) aswell. alternatively we could just get rid of said error response and return None instead
  4. unused imports for logging in all supplier plugins (forgot to remove these apparently)

Copy link

netlify bot commented Nov 15, 2023

Deploy Preview for inventree-web-pui-preview canceled.

Name Link
🔨 Latest commit 48da53d
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/6554204ac20a06000817f156

@30350n
Copy link
Contributor Author

30350n commented Nov 15, 2023

(This should be solved before a potential 0.13.0 release imo).

@SchrodingersGat
Copy link
Member

@30350n thanks for pushing these. I was playing around with barcodes yesterday and noticed some issues. I started a PR here - #5914 - but I think that I will close that one and try a different approach.

I noted that some digikey barcodes use 30P as the header for SKU. Have you seen this?

@SchrodingersGat SchrodingersGat added barcode Barcode scanning and integration plugin Plugin ecosystem labels Nov 15, 2023
@30350n
Copy link
Contributor Author

30350n commented Nov 15, 2023

I noted that some digikey barcodes use 30P as the header for SKU. Have you seen this?

Don't think so, can you share one of those barcodes?

I started a PR here - #5914 - but I think that I will close that one and try a different approach.

Interesting, that seems like a larger refactor. Honestly I'm not super sure about the code structure we ended up with ... The main idea with having a plugin per supplier was to create a starting point for further supplier specific integrations, like search/part importing (i.e. directly integrating stuff like my part importing CLI into InvenTree.

Not sure how the roadmap looks like for that though exactly. If that's not going to be a thing (in core), then it might be better to refactor everything into one single big plugin again, which would be able to detect all the different supplier barcode formats and handle them accordingly.

@SchrodingersGat SchrodingersGat merged commit cf3d96a into inventree:master Nov 15, 2023
24 checks passed
@SchrodingersGat
Copy link
Member

@30350n thanks for the updates. I'll ping you when I make a new PR

@SchrodingersGat SchrodingersGat mentioned this pull request Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
barcode Barcode scanning and integration plugin Plugin ecosystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants