Fix room map widget search not doing anything #3272
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports
|
stveit
force-pushed
the
fix/maps-widget
branch
from
a1ad814 to
b47b2db
Compare
Test results 9 files 9 suites 8m 20s ⏱️ Results for commit 8d622f1. ♻️ This comment has been updated with latest results. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3272 +/- ##
=======================================
Coverage 60.58% 60.58%
=======================================
Files 606 606
Lines 43733 43733
Branches 48 48
=======================================
Hits 26494 26494
Misses 17227 17227
Partials 12 12 ☔ View full report in Codecov by Sentry. |
python/nav/web/navlets/room_map.py
Outdated
| @@ -32,7 +32,7 @@ def get(self, request, *args, **kwargs): | |||
| context['can_edit_rooms'] = request.account.has_perm( | |||
| 'web_access', reverse('seeddb-room-edit') | |||
| ) | |||
| context['searchform'] = SearchForm() | |||
| context['searchform'] = SearchForm(form_action='info-search') | |||
There was a problem hiding this comment.
| context['searchform'] = SearchForm(form_action='info-search') | |
| context['searchform'] = RoomSearchForm() |
This makes it use the form_action room_search, not the general search and also changes the placeholder to Room
(The import also needs to be changed accordingly)
There was a problem hiding this comment.
Potential downside is that using RoomSearchForm() never opens the page for the room itself even if there is only one match, it always sends you to the list of search results.
I'll change it to RoomSearchForm, but this thing still has room for improvement in the future
There was a problem hiding this comment.
I did some light digging and found that this was likely broken by 6af1135 back in 2016 :P
It seems the default action of the search form was the info-search URL before this commit: The commit seems to remove the default action, but doesn't update the room widget.
So, I'd say you have returned it to the original intention - but @johannaengland's suggestion makes sense: Why do a generic search from a room focused widget?
|
Will squash before merge |
|
We could add a new issue for room search (and potential all the other specified searches): If there is only one result, then just as in the general search directly open the page to that result |
Seemed like the form sent a request to home (/) directly, when it probably should have been sent to /search/ instead. This at least makes it so if you search for a room and it finds a match it opens the page for the room, and if there is no match then it still shows the search page but with the message that nothing was found. If this is how the map widget is supposed to function is uncertain to me, I dont actually know how it originally worked before it broke (if it ever worked)
stveit
force-pushed
the
fix/maps-widget
branch
from
a6f46d7 to
8d622f1
Compare
|
Fixes #3207
Seemed like the form sent a request to home (/) directly, when it probably should have been sent to /search/ instead. This at least makes it so if you search for a room and it finds a match it opens the page for the room,
and if there is no match then it still shows the search page but with the message that nothing was found.
If this is how the map widget is supposed to function is uncertain to me, I don't actually know how it originally worked before it broke (if it ever worked)