Fixes #18845: restores sort behavior for DeviceTable.name column #18861
Conversation
jnovinger
requested review from
a team and
jeremystretch
and removed request for
a team
netbox/dcim/tables/devices.py
Outdated
| class DeviceTable(TenancyColumnsMixin, ContactsColumnMixin, NetBoxTable): | ||
| name = tables.TemplateColumn( | ||
| verbose_name=_('Name'), | ||
| accessor=Accessor('label'), |
There was a problem hiding this comment.
i think this might also need to be changed?
| accessor=Accessor('label'), | |
| accessor=Accessor('name'), |
There was a problem hiding this comment.
Instead of switching the accessor to name, it should be removed entirely. It was added in bf836c9 to display a value other than name.
I believe removing this line and switching the template DEVICE_LINK from value to record.label might preserve displaying the label but fix sorting. Sorry I didn't catch this before.
netbox/dcim/tables/devices.py
Outdated
| template_code=DEVICE_LINK, | ||
| linkify=True | ||
| linkify=True, | ||
| order_by=('name',) |
There was a problem hiding this comment.
This isn't needed if we correct the accessor.
netbox/dcim/tables/devices.py
Outdated
| class DeviceTable(TenancyColumnsMixin, ContactsColumnMixin, NetBoxTable): | ||
| name = tables.TemplateColumn( | ||
| verbose_name=_('Name'), | ||
| accessor=Accessor('label'), |
There was a problem hiding this comment.
As @mraerino points out, the accessor needs to be reverted to name.
|
@mraerino @jeremystretch , thanks for pointing that out. I had interpreted the previous change (that caused this behavior) to be intentional with respect to the display value of the object's Reverting the Behavior based on this PR as filed: Behavior with PR suggested changes: I just want to clarify, this is the behavior we want--that is, these devices show up with |
Yep, that's okay. The "Unnamed device" label signifies that |
@jeremystretch the intention of #17357 was exactly to change this behavior and show the same display name as in the rack elevations view. I proposed a change to fix sorting but preserving #17357 in the comments above: #18861 (comment) |
|
The column accessor should be set to a concrete value in the table data. If you want the column to display something other than the accessor's value, you need to use TemplateColumn or override the columns' However, be aware that rendering attributes of any related objects which have not been prefetched in the querysey may trigger additional queries. |
Thanks to @alehaa for the suggestion. This also includes an additional `.select_related()` operation on `DeviceListView.queryset` to avoid extra queries. Thanks to @renatoalmeidaoliveira and @jeremystretch for pointing out the need for this.
| accessor=Accessor('label'), | ||
| template_code=DEVICE_LINK, | ||
| linkify=True | ||
| linkify=True, |
There was a problem hiding this comment.
Oops, forgot I added this when adding the previous order_by argument. I'm a monster!
There was a problem hiding this comment.
@cvitan! The new guy is sneaking in commas again! I told you he was going to be trouble.
4 participants
Fixes: #18845
Device.namecolumn onDeviceTable.nametable column.This was a regression introduced in bf836c9 (in v4.2.5).