librivox-catalog
librivox-catalog copied to clipboard
[WIP: on staging] Expand author autocomplete
Status of this PR:
- Code: is in need of some further tests before going live.
- "Do we want it?": Precisely the reason for the early PR. I'd like folks to try it out on staging, when you have the time to put it there.
Until now, autocomplete for filling in author names has been confined to only searching by last_name
of the "real" author entry in the database. Pseudonyms are not included in the search list (#42), and for common names like "smith", we have too many results for folks to find the right one (#197)... resulting in duplicates as authors are re-added. This PR addresses both.
I know this may be one of those 'too clever for your own good' situations, so please do let me know what you think, when you get the time. I've rewritten this series of commits, which should hopefully make each one easier to follow than the larger chunk all at once.
To make for easier reading of the code, I've adjusted this based on #170. The logic remains the same, I've only adjusted the $bindings array.
Deployed on staging.
I've removed the 'WIP' tag! Ready for review or inclusion, and willing to make changes if requested.
- Admin consensus: It does what we want! :+1:
- Code: Feedback welcome. Here's what I'm aware of, that might be questionable:
- The SQL queries are kind of "doubled up". The intent is to narrow down the possible authors by one name (first/last), before we put together the full names for the more refined search.
- The logic for identifying elements in Javascript looks weird to me... but I see it being used in several other places, so at least it's not new and weird.
- The tests rely on the current snapshot database... but I don't expect any of these values to change in the foreseeable future of database snapshots. If we get a shiny new database for each set of tests, then I'll be happy to re-write my tests for that scheme.
I'm not an expert in any of that code (as mentioned previously, the original developer has long since moved on, I'm just the sysadmin), but nothing jumps out as being insane ;) I think we can merge and deploy.