Change order custom status via api#11982
Conversation
✅ Deploy Preview for inventree-web-pui-preview canceled.
|
johnluetke
force-pushed
the
feature/change-order-custom-status-via-api
branch
from
8c299e9 to
37783d2
Compare
|
@johnluetke nice clean implementation :) |
matmair
left a comment
matmair
left a comment
There was a problem hiding this comment.
Looks good;
there are a few formatting things, those could be fixed with pre-commit or prek; I will submit them shortly
| # can be set directly, but must be valid for the current order status | ||
| status_custom_key = serializers.IntegerField( | ||
| label=_('Custom Status Key'), | ||
| help_text=_('Update order status to a custom value for this logical value'), |
There was a problem hiding this comment.
With this set we loose dynamic choice enumeration in the schema description; @SchrodingersGat is that acceptable or should we patch the schema generation mechanism
There was a problem hiding this comment.
I've been trying to familiarize myself with how this works, but don't quite understand how it broke. It seems like the schema now thinks that the status and custom_status_key fields are more tightly coupled now?
If you can point me in the right direction, I can see if my solution can be implemented in a way that doesn't change this.
There was a problem hiding this comment.
Deleting the description or patching our custom schema generator to still add the choices if a description is set on a serializer.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #11982 +/- ##
=======================================
Coverage 91.42% 91.43%
=======================================
Files 974 974
Lines 51915 51961 +46
=======================================
+ Hits 47465 47512 +47
+ Misses 4450 4449 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
johnluetke
force-pushed
the
feature/change-order-custom-status-via-api
branch
from
423a9bf to
b1a0243
Compare
Refactor `custom_status_key` to be writable via the API and validate that the proposed value is valid for the current order status
johnluetke
force-pushed
the
feature/change-order-custom-status-via-api
branch
from
b1a0243 to
c53c768
Compare
|
@johnluetke you will need to add a new entry into |
| get_logical_value as get_custom_state_logical_value, | ||
| ) | ||
|
|
||
| custom_status = get_custom_state_logical_value( |
There was a problem hiding this comment.
This is going to result in a N + 1 query problem - each retrieved item will have multiple database hits.
In fact, this problem already exists in the codebase. I'm currently working on a patch for this which I will amend to your branch
|
@johnluetke I have addressed the mentioned query issue, which has unfortunately introduced some conflicts here, but they should be minor. Please review the changes I made in #12055 - you may have to adjust your approach here slightly |
3 participants
Fixes #11927
As discussed in the issue, an order's custom status key can be updated via the API.
Validation has been added to ensure that the custom_status_key value is aligned to the current logical state of the order.