[Refactor] Custom states #8438
[Refactor] Custom states #8438
Conversation
- More intuitive form actions
SchrodingersGat
added
enhancement
✅ Deploy Preview for inventree-web-pui-preview ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
@matmair I have been looking at the implementation of custom states and there are a few areas which I think can be cleaned up a bit. One question I have is: what is the value of the I would suggest that we remove the |
|
@SchrodingersGat there is only an implicit link between status groups and models. When you start shipping custom models in plugins and using custom states there you need some way to prefilter possible selections. |
|
I don't understand the use case here - in what circumstance would you want to make a custom state which overrides a |
|
The use case is enabling order when you have 40+ state groups and need to filter to manage them. If you see no value you can remove this and I will patch my custom fork for support, it seems anyway like I will never be able to use upstream. There is no reliable link between a state named "SomeModelStatus" and the model "SomeModel" - especially with possible model name collisions between (3rd party) apps. |
|
(Just a question, no opinion here, I just want to understand your design choice) Do you have any example scenario where custom states and models are in a relationship other than n:1? (E.g. the same custom state is assigned to multiple models. If not, doesn't that say that each custom state has only one model, which implicitly means there is a reliable link between a state and the model. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #8438 +/- ##
==========================================
- Coverage 84.59% 84.57% -0.02%
==========================================
Files 1182 1182
Lines 54004 53905 -99
Branches 2030 2026 -4
==========================================
- Hits 45683 45590 -93
+ Misses 7818 7814 -4
+ Partials 503 501 -2 ☔ View full report in Codecov by Sentry. |
|
Implicit is the key here; there is no reliable way to map a custom state to a model, thus automation that needs to make this link will not be able to function anymore if it is removed. I do not rely on implicit mapping in my business-critical LoB |
- Add StatusCode.custom_values method
|
Looking into this further, I think that if you want to link a "status group" to a particular database model, that should be done at the class level, rather than linking for each custom status field. Otherwise, you could end up with a (supposedly valid) situation as below:
There is no validation that custom status values must point to the same model, or that the linked model must be in any way related to the parent "status reference". To me, this does not make much sense. I would suggest that we a It removes the requirement that the user knows what model a custom status needs to point to (currently this must be picked as it is a required field). Unless you can make a case for allowing arbitrary assignment of a DB model to a custom state, I'd like to remove this field and implement as discussed above. |
|
I really would have liked to discuss these things before the original PR was merged. I built a ton of plugins and integrations around the merged (and thus I thought agreed upon) state and at this point, I am just frustrated when I open up GitHub with the instability of interfaces. |
|
@matmair I probably should have reviewed this more closely the first time around. As part of trying to fix #7990 I have identified a number of bugs / shortcomings of the current implementation. I am trying my best to rectify these without too many changes to the existing interface. One of the outstanding issues is the "model allocation". I would like to understand more about how you are using it, and whether a strict in-code link between the particular StatusCode sub-class and a DB model would be sufficient |
This PR is an update to the "custom states" functionality, to improve usability and extend user interface integration
Tasks