Update to /api/barcode/po-recieve/ error messages#11369
Update to /api/barcode/po-recieve/ error messages#11369Idea-Junkie wants to merge 22 commits intoinventree:masterfrom
Conversation
Added DebugResponse Added Suppliers Tested Added Suppliers Errors
Changed "NotFound" to 'No_Match' different style for Debug response
Idea-Junkie
requested review from
SchrodingersGat and
matmair
as code owners
✅ Deploy Preview for inventree-web-pui-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Idea-Junkie
changed the title
|
@matmair I will be making the documentation edits on here as well which is why its a PR draft. I believe this was a good first issue for me to get my feet wet, and I hope to be more of assistance on some other issues. any comments on how I went about this? Should I work on it and then make a PR draft request and include the conversation in there instead of how I did it with the discussion? or is the discussion a better method? |
matmair
left a comment
matmair
left a comment
There was a problem hiding this comment.
Looks good in general; a few smaller things to address
I will allow CI now to see if there are style issues etc.
matmair
added
enhancement
making generalized rules is hard; if it is a concrete change or widening to/of system behavior, a feature request with "I want to dev this" selected is the way to go. That allows us more concrete roadmap management and improves visibility to the change and that someone is working on it. Small things can just be done and PR'ed (with sufficient PR description). If the thing will take you longer than 1 week to dev I would certainly open an issue or discussion to ensure you do not waste time on duplicate / unwanted things. |
Added more variables updated formatting Added more in depth explanation block for Debug response
…-Junkie/InvenTree into More-Barcode-Error-messages
fixed some issues with comments made in original PR added more comments and small adjustments got rid of "line item" in debug response
|
Updated the requests you made, let me know if there is anything else I need to add or change, as the workflow changed a little bit with the changes your requested. Also updated documentation (was only like two notes really) Also while Im at it, is there a way to make the images on the documentation smaller? Its always overblown for me on my desktop because its a vertical picture instead of horizontal, also tried to preview it in VScode but it just showed how the text would work. |
matmair
left a comment
matmair
left a comment
There was a problem hiding this comment.
LGTM; adding some test coverage would be good too
| response[current_plugin.slug + "_debug"] = { | ||
| 'purchase_order':supplier_purchase_order, | ||
| 'no_match':no_match, | ||
| 'supplier':plugin_supplier, | ||
| 'supplier_part': supplier_part, | ||
| } |
There was a problem hiding this comment.
As mentioned before: Please put all of these debug messages into one dict, not under multiple generated keys that can not be easily checked.
in the end the repsonse should be something likes this:
{
...
'supplier_matches': {
'slug1': {
'purchase_order': 123,
'no_match': true,
...
},
'slug2': {
'purchace_order': 456,
'no_match': false,
}
...
}
There was a problem hiding this comment.
I am confused by this comment? it will only output the plugin debugresponse dict that found it as it breaks the for loop looking for more plugins to match the barcode info to.
So it wont have multiple plugin debugresponses like it seems how youre describing it in the response as we decided that would be an issue, so I went away with that and stuck to only output the plugin it found that matches.
or are you just talking about defining the dict to set variable instead of having the suppliers name in the dict?
like plugin_debug or something? this is pretty much what I am doing down below just without the current_plugin.slug which I can see would make accessing it a little bit more predictable in testing.
...
'plugin_debug': {
'purchase_order': 123,
'no_match': true,
...
},
...
I guess your asking for this?
...
'plugin_debug': {
'current_plugin.slug': {
'purchase_order': 123,
'no_match': true,
...
},
},
...
Unless I'm misunderstanding what you are explaining. let me know
New format
There was a problem hiding this comment.
This is not about one single log item, this is about barcode log messages being made harder to parse
ok I going to try explain this with a scenario, maybe that is clearer:
- admin enables barcode logging
- users scan broken / not working barcodes
- logs for different plugins get created
- admin starts to analyze the logs by downloading them
- admin wants to create stats about problematic pruchaseorders: We arrive at the problem
Aggregating the log lines with your format requires manual aggregating the fields "digikeyplugin_debug", "mysecondbarcode_debug", "hopefullyIrememerberthisplugin_debug" into one field to get usable stats across failing plugins
Aggregating the log lines with my suggestion: aggregate "supplier_matches"; always the same field; no data shape changing depending on the plugins / pluginslugs (which can and have changed)
adding matmirs comments Co-authored-by: Matthias Mair <code@mjmair.com>
|
@Idea-Junkie looking good in general -are you going to add support for these messages to the app, too? |
Mobile Error Pop-UpThe original line was too long but with a carriage return added to it, it is readable on the app as well 
is this what youre talking about?
|
|
@Idea-Junkie yes, perfect! |
3 participants
Includes
Updates to barcode error messages, letting users know if a barcode was found with a plugin, and what is missing from that information.
Example: PO found, Supplier part Not found
See more in #11361
No Supplier Associated to Plugin
Also added "no_supplier_plugin _error" to notify the user if plugins do not have suppliers set in settings
This is set to only pop up if there is an error, if a match is found it skips this step