Add admin search for public bodies - #9435
Conversation
125bd81 to
ddb1ca4
Compare
garethrees
left a comment
garethrees
left a comment
There was a problem hiding this comment.
Review as discussed on our catchup
| ".tag_string": "D" | ||
| }, | ||
| admin_index: { | ||
| ".searchable_admin_field_versions_as_string": "A", |
There was a problem hiding this comment.
Instead of all the versions, index the current values for:
request_email: A
last_edit_editor: D
last_edit_comment: D
For past versions we'll make PublicBody::Version itself searchable. We'll need to add a bit more UI to handle that, but we can do that in a separate pull request.
| ".short_name": "A", | ||
| "home_page": "B", | ||
| ".notes_as_string": "C", | ||
| ".tag_string": "D" |
There was a problem hiding this comment.
Should add:
- disclosure_log: C or D
- publication_scheme: C or D
Happy if we need to remove tag string as per the PR description
| ".name": "A", | ||
| ".short_name": "A", | ||
| "home_page": "B", | ||
| ".notes_as_string": "C", |
There was a problem hiding this comment.
Should limit this to only notes on the body directly.
This is meant to be a temporary/test of the new search engine for public bodies, VS the legacy one.
ddb1ca4 to
0f057af
Compare
|
Skipping changelog as it won't really provide meaningful info. Might add a line once we've added all models to search |
Yeah +1 |
|
The tests should be fixed (they pass locally). |
laurentS
changed the title
|
Ok, tests finally green. Sorry for the back and forth. Just as a side note, in the tests, under "stop containers", there is a bunch of |
The test was incorrectly passing before this PR because PublicBody had 'include Searchable' in its definition, but no call to 'searchable'. Without the include, there is no search_documents attribute available on the instance.
4b2aaae to
61deb9f
Compare
3 participants
Relevant issue(s)
Relates to #8814
What does this do?
This sets up a simple UI in /admin/bodies to allow admins to use either the legacy search or the new postgres search engine to find public bodies.
Why was this needed?
We need to test the new search engine on actual data with actual use cases. Making it available on live sites without breaking existing workflows seems like a good way to do this.
Implementation notes
I have included the few tests we had from previous commits to ensure a base level of consistency in results. We need a lot more!
To allow admin search through all the history of edits, I had to add an extra method. Naming improvements welcome :)
Screenshots
Notes to reviewer
With Ma Dada data, searching for
mairie(cityhall) leads to 35k+ results under the "exact tag match" section. As there is no pagination on that part of the page, the server has to issue something like 100k db queries to fetch translations+tags, then sends 100+MB to the browser. The client then struggles to render the result. I would suggest we remove the tag based search from this page, and rely on /admin/tags for that purpose, as it handles pagination and all (and we don't actually do partial tag matches anyway, so there is no additional use case handled here).Have you updated the changelog? If this is not necessary, put square brackets around this: [skip changelog]