Incident search filter improvements #1626
Conversation
chigby
force-pushed
the
search-filter-performance
branch
from
3a3d38b
to
c5c7f60
Compare
|
@chigby I can review the code, but like you mentioned, it mostly needs to be tested at a staging environment, so should we wait for it to be merged till the experimental staging gets available again? |
|
@SaptakS That sounds reasonable to me |
| index.RelatedFields('updates', [ | ||
| index.SearchField('title'), | ||
| index.SearchField('body'), | ||
| ]), |
There was a problem hiding this comment.
I see that we are using only the title and body indexes in the search. Adding these fields is to future proof?
There was a problem hiding this comment.
So how Wagtail's IndexEntry model works is not super well documented, but the title and body fields on the from your link are its built-in fields that contain searchable data (there is also a third one called autocomplete that allows for partial matches -- intended to be used for find-as-you-type varieties of searches). From how they are named, it might seem like there is a 1-to-1 correspondence between those fields and the fields of the model being indexed, but that's only true for title. The body field (on IndexEntry) includes all the search data that's not part of the title -- so all the fields that are listed in the search_fields attribute of IncidentPage will have their searchable text pulled in and indexed. You can actually test this out by performing a search based on, say, text in the introduction field and see that you will get results.
There was a problem hiding this comment.
Codewise looks good to me. We definitely need to test it in staging (probably experimental?) to get a better idea of whether it improves the speed. I am approving the PR but I think should be merged once further testing has been done. Though I don't think it necessarily breaks anything.
chigby
force-pushed
the
search-filter-performance
branch
from
714bfe7
to
1537016
Compare
chigby
force-pushed
the
search-filter-performance
branch
2 times, most recently
from
3a9f506
to
9752e26
Compare
This adds some factories for all the blocks that make up the incident page streamfield body. Nice to be able to now test this field easily!
This permits: 1. Faster searches, since Wagtail's data is already computed 2. More fields to be searched, since Wagtail knows how to index just the text from certain non-string DB fields.
chigby
force-pushed
the
search-filter-performance
branch
from
9752e26
to
a7ad545
Compare
|
We can test this further in staging. Merging. |
This pull request:
SearchFilterto employ theIndexEntrymodel's pre-computed textsearch vectors for querying against. This improves performance since we are not having to calculate these ourselves. I did observe faster load times locally, but staging is the place to test this. I will note that the performance gains were dependent on the search term used, with the greatest gains coming when it was a term that appeared in many incidents.I think this tends to make the search more useful.
Refs #1573