Closes #20808: Show occupying device in rack position selector

[Guibi1]

Fixes: #20808

Added the name and device type of the racked device inside the device rack position select.

[pheus]

Thanks for working on this. I agree with the goal, but I don't think this implementation works in its current form.

For me, the main issue is not the specific string formatting, but that this changes the public rack-elevation API payload itself. display is being repurposed from the rack-unit label into selector-specific UI text, and description is also being introduced for that same purpose. As a result, every caller of this endpoint gets different semantics, not just the device position dropdown.

Because of that, I think this needs a different approach.

The rack position field is the consumer we want to improve, so I don't think the general API response should change in order to support that.

I also think the test should reflect that. Right now it locks in the API contract change by asserting the modified display and new description values. I'd expect the existing rack-elevation response to remain stable, so from my side this needs a rework.

Thanks again for the PR.

[Guibi1]

I see that makes sense. I will add the device information to another api field then. Im pretty sure that will require me to add JS though?

[pheus]

Thanks for looking into this.

I don't think adding custom JS should be necessary, since we already have API-backed widgets for this kind of use case, although I haven't looked into the exact implementation path myself.

Could you please take another look through the existing codebase and see whether there is a solution that keeps the current API contract intact and avoids introducing additional code we would need to maintain?

[Guibi1]

The API backed widget is what is currently being used, and it simply uses the display and description fields of the objects it receives. We can either

• Change what the api returns (my current impl)
• Change how the widget shows the data on the client (in the JS)
• Create a new api endpoint
• Create a new widget (probably a better idea than writing JS)

If we dont want to modify the API, i feel like making a new path is the better option.
I might be missing an option that would make it easier... (like an attr on the widget to select another field so we can preserve display)

[Guibi1]

I might be missing an option that would make it easier... (like an attr on the widget to select another field so we can preserve display)

Welp i just found ts-label-field, which allows us to use another field on the data. I can add a device-display field on the api that contains the name of the device, and undo my changes to the display field.
Would that work?
This PR would then just add two fields on the rack unit serializer. (description and device-display)

[pheus]

You might also want to take a look at the widget context option, depending on how you plan to pass the additional data through.

I haven't had the bandwidth to work through all implementation details on my side, so I can't fully validate every possible approach, but this direction sounds reasonable to me. Thanks for digging into it.

[pheus]

Thanks for reworking this and for taking another pass at it.

I had a chance to look at it more closely, and I think we are getting much closer.

Looking at other parts of the NetBox UI, additional information in dropdowns is typically shown underneath each option via the description field. In the current implementation, the dropdown also includes the device type, which was not part of the original feature request.

Because of that, I think we can keep this implementation simpler.

From my side, I’d prefer to keep display unchanged and add a single description field on the rack unit serializer for the occupying device information, rather than introducing separate device_display / device_type fields plus additional widget configuration.

That would keep the existing display semantics intact, avoid the extra widget-specific code, and still address the UI problem we want to solve here.

Could you please rework it in that direction? Thanks again for the update.

[github-actions]

This PR has been automatically marked as stale because it has not had recent activity. It will be closed automatically if no further action is taken.

[pheus]

Thanks for the rework.

This is still not ready for review while CI is failing. Please fix the lint/test failures and make sure CI is green before requesting another review.

Also, please update the new test name/docstring so it matches the current implementation: the device information is now exposed via description, not display.

[Guibi1]

Sorry about that, but i need approval to run the CI...
This is out of scope, but running ruff locally changes hundreds of files. I am probably missing something?

[pheus]

Thanks for sticking with this and reworking it.

This looks good to me now. The implementation keeps display unchanged, adds the occupying device information via description, and includes API coverage for occupied and unoccupied rack units.

One small nit: the new test docstring still mentions the display label, but the implementation now uses description.

Thanks again

[pheus]

Thanks for the rework and for getting CI green.

This looks good to me now. The implementation keeps display unchanged, exposes the occupying device information via description, and includes coverage for both occupied and unoccupied rack units.

Approved from my side. Thanks again.