Show room/location aliases in search results#3851
Show room/location aliases in search results#3851johannaengland merged 2 commits intoUninett:masterfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3851 +/- ##
=======================================
Coverage 63.73% 63.73%
=======================================
Files 624 624
Lines 46139 46145 +6
Branches 43 43
=======================================
+ Hits 29408 29412 +4
- Misses 16721 16723 +2
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
9afa68e to
2f90d0f
Compare
There was a problem hiding this comment.
First off, the search result template changes are clean and a dedicated "Aliases" column is a good approach.
However, changing the __str__ method directly like this has a larger "blast radius" than expected. In addition to the search results, aliases are now shown in tools like ipdevinfo, device history, seed db location tree and netmap.
Another example, if rooms and locations are rendered in a dropdown, showing aliases could make the dropdown options quite long and unwieldy, especially if there are multiple aliases with fairly long names, i.e.
MyRoom: Some description that could have been longer (alias1, this room is important and has a long alias, another long alias from external system, alias4)
- Suggestion 1: Maybe use a variant of
aliases_stringin the templates instead, and leave__str__intact? - Suggestion 2: Maybe change the format when including aliases to this? It is additive and uses square brackets to separate the extra "meta info"
<roomid>: <description> (<alias1>, <alias2>)to<roomid> (<room description>) [<alias1>, <alias2>]
95ece71 to
1698b0e
Compare
1698b0e to
54ce149
Compare
|
I have factored out the string representation change and will create a new PR for that so that we can discuss everything there and so that #3855 is not blocked by that discussion. |
54ce149 to
b468e5c
Compare
|
Scope and purpose
Follow up to #3832. Now after searching for a room by its alias one also is shown the aliases of a room/location.
I have factored out the controversial string representation change and will make a new PR for that where we can discuss that separately.
How to manually test:
/search/room/. See that that the aliases are shown./search/location/. See that the aliases are shown.Screenshots:
Before:
After:
Contributor Checklist
Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.