Move Review Board's search to a SearchForm + generic SearchView
Review Request #9105 — Created July 31, 2017 and submitted
This patch updates Review Board's
RBSearchView
, which was previously
using Haystack's deprecatedSearchView
with the generic version added
in 2.4 that builds on top of existing Django forms infrastructure. This
means that we now require a minimum version of 2.4.0 for Haystack.The actual search code has been split out into a form that does the
actual searching. This will allow searches to be performed without
setting up a bunch of view rendering infrastructure that we won't need
each time (e.g., from the API).
Ran unit tests.
Description | From | Last Updated |
---|---|---|
Can you add unit tests to check for access to the search page under the standard conditions, and for when … |
chipx86 | |
Swap these. |
chipx86 | |
This won't link right. Just say the parent form. |
chipx86 | |
"If no filter is provided" |
chipx86 | |
self.user gets accessed a lot, so let's remove the attribute lookup and pull it in as a local variable. |
chipx86 | |
No blank line. |
chipx86 | |
reviewboard.accounts is in the wrong place. |
chipx86 | |
This sentence kinda ends strangely. |
chipx86 | |
Can you add , unused to these? |
chipx86 | |
There's something wrong here. Make sure results with multiple pages are tested thoroughly. |
chipx86 | |
W292 no newline at end of file |
reviewbot | |
Can we call this something other than filter, since that's a reserved word? |
david | |
Should this validate that filter is actually a list of unicode? |
david | |
We don't need the fallback here, since it's done in clean_filter. Right? |
david |
-
-
Can you add unit tests to check for access to the search page under the standard conditions, and for when search is disabled?
-
-
-
-
reviewboard/search/forms.py (Diff revision 1) self.user
gets accessed a lot, so let's remove the attribute lookup and pull it in as a local variable. -
-
-
-
-
reviewboard/search/views.py (Diff revision 1) There's something wrong here.
Make sure results with multiple pages are tested thoroughly.
Checks run (2 succeeded)
Change Summary:
added tests
Commit: |
|
||
---|---|---|---|
Diff: |
Revision 3 (+418 -228) |
Checks run (1 failed, 1 succeeded)
flake8
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+418 -228) |
Checks run (2 succeeded)
-
-
reviewboard/search/forms.py (Diff revision 4) Can we call this something other than
filter
, since that's a reserved word? -
reviewboard/search/forms.py (Diff revision 4) Should this validate that
filter
is actually a list of unicode? -
reviewboard/search/forms.py (Diff revision 4) We don't need the fallback here, since it's done in
clean_filter
. Right?
Change Summary:
Addressed David's issues
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 5 (+418 -228) |