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 supplier barcode order numbers #6158

Merged

Conversation

30350n
Copy link
Contributor

@30350n 30350n commented Jan 5, 2024

Seems like the logic for retrieving a purchase order from an order number from a supplier barcode got broken when refactoring. This fixes the logic and also some supplier specific details.

Copy link

netlify bot commented Jan 5, 2024

Deploy Preview for inventree-web-pui-preview ready!

Name Link
🔨 Latest commit f459df0
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/659ecc5c3c1c3900088c1f4f
😎 Deploy Preview https://deploy-preview-6158--inventree-web-pui-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 86 (no change from production)
Best Practices: 92 (no change from production)
SEO: 70 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@30350n
Copy link
Contributor Author

30350n commented Jan 5, 2024

(I'll add some test cases later to confirm this keeps working in later versions.)

@SchrodingersGat
Copy link
Member

@30350n can you please provide some more specific information about how the current implementation is "broken"?

@30350n
Copy link
Contributor Author

30350n commented Jan 10, 2024

Sure!

  1. The current implementation always checks for both the PO and CPO matching, if both are present in the barcode. Having a match on one of them should be sufficient.
  2. LCSCs and TMEs barcode POs were incorrectly labeled as CPOs. TME actually has a CPO field though, which hasn't been used.
  3. Mouser uses the ecia barcode 'K' field for both, POs and CPOs interchangeably (yeah I know, don't ask me why).

@30350n
Copy link
Contributor Author

30350n commented Jan 10, 2024

Actuall the implementation I came up is kind of convoluted I guess, I'll try to refactor it a bit.

@30350n
Copy link
Contributor Author

30350n commented Jan 10, 2024

Not sure if this makes it better 😅

@SchrodingersGat
Copy link
Member

@30350n if you have the data on hand to test that this works, I'm satisfied with that. please keep an eye on whether this continues to work

@SchrodingersGat SchrodingersGat merged commit 36bb3c5 into inventree:master Jan 10, 2024
24 checks passed
@SchrodingersGat SchrodingersGat added bug Identifies a bug which needs to be addressed barcode Barcode scanning and integration labels Jan 10, 2024
@SchrodingersGat SchrodingersGat added this to the 0.14.0 milestone Jan 10, 2024
@30350n
Copy link
Contributor Author

30350n commented Jan 10, 2024

Thanks, will definitely do! I actually also wanted to confirm everythings actually working correctly with some tests but didn't find time to implement them yet. I'll try to get that down the coming days and create another PR 👍

(I did quickly test this on my production server though and it did the trick.)

30350n added a commit to 30350n/InvenTree that referenced this pull request Jan 14, 2024
* Add tme barcode CPO field

* Fix LCSC order number field

* Fix mouser order number field

* Fix get_purchase_orders logic

* Refine get_purchase_orders logic

* Slightly refactor get_purchase_orders logic
SchrodingersGat pushed a commit that referenced this pull request Jan 14, 2024
* Add tme barcode CPO field

* Fix LCSC order number field

* Fix mouser order number field

* Fix get_purchase_orders logic

* Refine get_purchase_orders logic

* Slightly refactor get_purchase_orders logic
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
barcode Barcode scanning and integration bug Identifies a bug which needs to be addressed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants