Adds ability to search/filter one's reading log

[scottbarnes]

Closes #5080. This may need significantly more work, however.

Feature. Adds basic capacity to filter/search one's reading log.

Technical

What this does, in theory

This draft PR likely needs some work, but before getting the problems, first comes an explanation of how this theoretically works:

• From the Reading Log one can enter a 3+ character search term and search (1) up to 1000 of their logged books to see (2) up to 100 matching results within that 1000.
• Currently, searching is not limited to title or author, but rather the defaults from do_search. This would be easy enough to change I think by switching to run_solr_query (unless it's possible to specify fields in do_search.

Issues

• Because I could not figure out a good way to filter the results, there is the possibility for a fairly hefty pair of queries to occur. When a user enters a query of 3+ characters, this:
  1. requests up to 1000 of the user's logged books from the current shelf (e.g. "Want to Read") via get_users_logged_books in core/bookshelves.py;
  2. searches Solr for those work IDs and any matches for the search term (currently not limited to author/title); and
  3. returns only logged books that match the work IDs in the Solr search results -- meaning that only logged works that match the search term are returned and passed on to process_logged_books in upstream/mybooks.py, etc.
• This requires new page loads for each search.
• I have not tested this with more than 29 books in one shelf.
• To avoid a circular imports issue in bookshelves.py, do_search is conditionally imported. This likely will need a better solution, though I didn't put any time into it on the theory that maybe this implementation has no future.
• There is no pagination. The number of results on a page is limited to the number of filtered results, which is up to 100. Owing to the need to do two queries and 'merge' the results, I couldn't figure out a way to use query offsets effectively (which isn't to say there isn't an easy and obvious way). Additionally, I was thinking that if this is limited to a certain number of characters and/or title/author, not many searches would go near 100 results. I am unsure of the performance implications of this, or the reality of my supposition.

Testing

• Visit the Reading Log and try filtering via the search box for "Currently Reading", "Want to Read", and "Already Read".
• Verify that the results appear correct and that sorting by date added works.
• Try searching for <= 2 characters and see if that does anything, both through the search box and via a GET request in the URL bar. With the direct GET, the search should just be ignored.

Screenshot

Already Read, without any filtering:

Searching for 'twain':

Same search, but with the sort order showing oldest-added first:

Searching a shelf of 29 books for the phrase "web":

A not very good screenshot of trying to enter < 3 characters into the search:

Doing a GET request for a search of <= 2 characters just returns everything on the shelf, with the usual pagination and no filtering. (Though there is no indication that the search didn't work, aside from not mentioning results):

Thanks to

@cdrini for walking me through a lot of it, and @mekarpeles for the detailed explanation of the steps.

Stakeholders

@mekarpeles, @cdrini

[cdrini]

The approach looks solid to me :P We'll need to provide all the data from the db to the endpoints anyways. The only potential downside is that there are now two solr requests, but we can speed that up by limiting the info we get back here. And honestly if that means having to update half the codebase so that it expects solr docs, then your approach is definitely the better solution! This looks good to me! Just need to do some testing :)

[scottbarnes]

Thank you for all the suggested changes, @cdrini. I have updated the PR to include them. Additionally I've addressed, I believe, the bug where filtering the reading log was not working when a user was not logged in. (I had simply failed to pass q to that code path. Go figure!)

I will address pagination in a separate PR.

[cdrini]

Code looking good! One little change. Pushing to testing afresh for QA!

[scottbarnes]

@cdrini

I've updated the draft PR and if possible I would like to try it on testing.

Some places that deserve special attention:

• I'm a bit worried about my change to line 6 of SearchResultsWork.html. I think I tested all references, but I want to draw attention to this specifically as it could change how documents are interpreted.
• The output of readinglog_stats is suspect. I know for sure I am not getting the right data out of Solr for "Works by People", and I think my entire parsing of doc.get("seed") in the SearchResponse.docs is probably wrongheaded.
• I think the strategy to get the ratings through to the template is obtuse.
• The JSON representation of the reading log should work now, but this also deserves extra attention.

Other things of note:

• There is much less querying of the databases now. Rather than 3-4 queries for the same data in different forms, it's usually just one query (in addition to the query to the reading log db) to Solr in get_users_logged_books in bookshelves.py, with that query response data passed through to the templates.
• Pagination should work for search results (and not be broken for non-searches).
• The limit for search queries is just over 2000 books; there is no limit on the number of matches (I think...).

Things that remain:

• There is not a yet a notification for people who have more books than the limit. I simply forgot about it until looking at my notes just now, and I figured I should commit this now in case people have a chance to look at it before I get to the too-many-books notification. I will update this PR when I add that.

Sadly, I don't think the code is any more clear now (yet?).

[cdrini]

Let's also add q to the json endpoint for consistency!

Otherwise is looking good! Investigating a weird possible perm issue :P Possible unrelated to this PR.

Thank you for taking the time to try to clean this up a little 😊

[cdrini]

Looking good! Let me push up that solr param change, and put up on testing!

[cdrini]

1 failing test left! :)

Test:

• ✅ Pagination w no search

• ✅ No q= more than 2k

• ✅ Sorting no search

• ✅ Set sort, then search

• ✅ Set page, then search, page removed

• ✅ JSON endpoint w/o q

• ❌ JSON endpoint w/ q

  • edition IDs mis-aligned

• ✅ JSON endpoint w/ q and sort

• ✅ JSON endpoint w/ q and sort and page

• ✅ Moving things btwn shelves

• ✅ Add rating when querying

• ✅ Move btwn shelves when querying

• ✅ Can't see other users reading log

• ✅ Can't see other users reading log with q

• ✅ Can't see other users reading log with .json

• ✅ Searching user reading log with 20k books works!

• ✅ Reading log stats

[scottbarnes]

@cdrini

This adds querying to the JSON endpoints. I am hopeful it's fixed the edition IDs being mis-aligned, but for some reason in my local development environment I never have the edition IDs.

That said, injecting the edition IDs into the Solr results now uses a dictionary with a key of the work ID, and a value of a datclass with properties for the logged_date and the edition_id, so it no longer relies on ordering. I am hopeful this works.

I included two commits in this latest change: one addressing the JSON endpoint and the logged_date/edition ID stuff, and another refactoring get_works() and some related functions to hopefully make it more clear.

Another thing to draw attention to, in the latter commit, is that I made it so if someone goes to /WANT-TO-READ/, it will work. From the inclusion of key = key.lower() in get_works() I think this was intended, but it didn't appear to work.

Lastly, I did not do a great job reducing redundancy in get_users_logged_books(), though I think it is perhaps slightly more clear.

Edit:
I removed ReadingLog.KEYS and two of the functions it called (and a third that never got called) because I don't think any of them are used and they create the illusion that loans and waitlists are controlled from ReadingLog.

We need to test loans/waitlists. I think I can't fully test that locally.

Why I suspect we can remove the waitlists and loans things that I've removed in ReadingLog:

• account/books.html renders account/loans.html if key == 'loans';
• account/books.html is itself rendered by MyBooksTemplate, and not ReadingLog;
• ReadingLog.get_loans(), ReadingLog.get_waitlist_summary(), and ReadingLog.get_waitlisted_editions() are not called when viewing the reading log, or any link in the left-hand menu that displays when viewing the reading log, including "Loans";
• ReadingLog.get_waitlist_summary is not called by anything;
• ReadingLog.get_loans() and ReadingLog.get_waitlisted_editions() are only called by ReadingLog.KEYS when the key is loans or waitlists, respectively;
• Only ReadingLog.get_works() calls ReadingLog.KEYS;
• ReadingLog.get_works() is only called in the following situations:
  • public_my_books_json with calls get_works() with a key of want-to-read;
  • readinglog_stats calls get_works() with a default key of loans, for some reason, but the key parameter is always supplied a value of either "want-to-read", "already-read", or "currently-reading";
  • MyBooksTemplate.render() calls get_works(), but only if the key is one of "currently-reading", "want-to-read", or "already-read";
• There is no mention of loans or waitlists in accounts/reading_log.html

I just hope I am right about all of that.

[cdrini]

In a future PR it would be great to make this one bulk request! Now that it's in one place :)

[cdrini]

Ok!! Final stretch!

[cdrini]

Ran the tests one more time; works like a charm!

[cdrini]

Solr-side of things deployed! This should be able to just go out normally on Monday :)