-
Notifications
You must be signed in to change notification settings - Fork 2.9k
20911 Fix sorting in dropdown #21101
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
base: main
Are you sure you want to change the base?
Conversation
jeremystretch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like an issue with the underlying model ordering, and shouldn't require any changes to the frontend widget (unless I'm missing something). For example, we see the ordering of the REST API endpoint is incorrect:
GET /api/dcim/module-bays/?device_id=27&fields=id,name
{
"count": 6,
"next": null,
"previous": null,
"results": [
{
"id": 29,
"name": "fan1"
},
{
"id": 30,
"name": "fan2"
},
{
"id": 31,
"name": "fan3"
},
{
"id": 32,
"name": "psu1"
},
{
"id": 33,
"name": "psu2"
},
{
"id": 34,
"name": "fan4"
}
]
}
The API endpoint should be returning these in the expected order, and the form widget should just populate them in sequence. This will probably require altering order_insertion_by on the model's MPTTMeta class (which I suppose should be 'name' rather than 'module').
jnovinger
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any concerns about any of the other bulk action views at all?
jeremystretch
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to still be broken. Starting with the following list of module bays:
When I add a new module bay named fan5 via the UI, it appears at the end of the list, both in the UI and in the REST API. However, after I rebuild the tree manually via dcim.ModuleBay.objects.rebuild() the ordering is now correct.
Fixes: #20911
Two issues:
This fixes item 1, item 2 still appears but would need a front-end fix so keeping that for a separate ticket.
As noted in the issue discussion, the change to order_insertion_by will fix the ordering of newly inserted items, there were other ordering problems in bulk edit and bulk rename which this also fixes. I tried either looping and doing a save or doing a bulk_update and then a rebuild at the end, for small data-sets the bulk_update if much faster, for large data-sets they are almost the same perf but bulk_update still wins out.
Just changing order_insertion_by revealed an existing bug in the CSV import when using the order_insertion_by. We don't run into it in the tests for NestedGroupModel (the other model using it) because of the way the imports are ordered in the test case as they don't cause a re-arrangement in the tree. The ordering of the ModuleBay test actually does cause a re-ordering and hits what looks like a bug in MPTT code. The additional changes in bulk_views.py so the CSV processing if done on models that inherit form MPTTModel uses a delay_mptt_updates() and rebuild at the end fixes this issue.