JSHint
timed out.
Use the search index for the search resource when available
Review Request #9116 — Created Aug. 2, 2017 and submitted
When indexing is enabled, we can use it to perform better full text
search for review requests and users. Since review groups are not
indexed, we still have to query the database for them even if search is
enabled. This does have the side effect that review requests published
by a user will show up in a search for that user's username, but that is
a convenient benefit.
Ran unit tests.
| Description | From | Last Updated |
|---|---|---|
|
Typo in the description: "databse" |
 chipx86
|
|
|
I know the name existed before, but can we call this reindex_search or something, just so it's a little bit … |
chipx86
|
|
|
, optional |
chipx86
|
|
|
Do tests depend on these settings after using this context manager? Other tests will already reload the siteconfig, and you … |
chipx86
|
|
|
haystack.inputs is in the wrong import group. |
chipx86
|
|
|
Rather than have this function, which just appears to be serializing content the right way from the search functions, can … |
chipx86
|
|
|
No need for the else. I'd instead say we should have comments within the if above saying that if search … |
chipx86
|
|
|
Only one blank line. |
chipx86
|
|
|
Trailing newline. |
chipx86
|
|
|
E999 SyntaxError: invalid syntax |
 reviewbot
|
|
|
F401 'django.db.models.query.QuerySet' imported but unused |
reviewbot
|
|
|
F401 'haystack.inputs.Clean' imported but unused |
reviewbot
|
|
|
F401 'haystack.models.SearchResult' imported but unused |
reviewbot
|
|
|
There's an extra newline here that doesn't make sense. Or perhaps the comment should be moved above FILTER_TYPES? |
 david
|
|
|
The "however" is awkward. How about just "If search is disabled" |
david
|
|
|
Same here. |
david
|
-
-
-
I know the name existed before, but can we call this
reindex_searchor something, just so it's a little bit more clear in call sites? -
-
Do tests depend on these settings after using this context manager? Other tests will already reload the siteconfig, and you shouldn't need to re-fetch the siteconfig, so I think you can basically just set the keys and save.
-
-
Rather than have this function, which just appears to be serializing content the right way from the search functions, can we just return the right data from the beginning in those functions?
-
No need for the
else. I'd instead say we should have comments within theifabove saying that if search is enabled, we'll be making use of the search index for results, and one below the whole new block, explaining that we're falling back to using more restrictive database lookups. It'll help when reading the code.Same comments apply below.
-
-
- Change Summary:
-
Addressed Christian's feedback.
- Description:
-
When indexing is enabled, we can use it to perform better full text
search for review requests and users. Since review groups are not ~ indexed, we still have to query the databse for them even if search is ~ indexed, we still have to query the database for them even if search is enabled. This does have the side effect that review requests published by a user will show up in a search for that user's username, but that is a convenient benefit. - Depends On:
-
- Diff:
Revision 3 (+221 -45)
