Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

My Books (Reading Log) add Search & Filter capability #5080

Closed
mekarpeles opened this issue Apr 21, 2021 · 7 comments · Fixed by #7052
Closed

My Books (Reading Log) add Search & Filter capability #5080

mekarpeles opened this issue Apr 21, 2021 · 7 comments · Fixed by #7052
Assignees
Labels
Affects: Experience Issues relating directly to service design & patrons experience Good First Issue Easy issue. Good for newcomers. [managed] Lead: @mekarpeles Issues overseen by Mek (Staff: Program Lead) [managed] Needs: Help Issues, typically substantial ones, that need a dedicated developer to take them on. [managed] Priority: 2 Important, as time permits. [managed] Theme: Reading Log Related to workflows for creating, modifying, displaying a user's reading log. [managed]

Comments

@mekarpeles
Copy link
Member

mekarpeles commented Apr 21, 2021

Describe the problem that you'd like solved

(What it would take to) add search capabilities to the Reading Log page.

Proposal & Constraints

Please note, that currently, this proposal will not work because book titles, authors, and the other data we'd like to search for are not kept in our ReadingLog db table, only the OL identifiers. We may be able to achieve this with solr in the future. But assuming we did have the desired info in our database (and as a thought exercise):

NOTE: Read first, Working with Reading Log here: #4267 (comment)

  1. First, we'd need to update the Reading Log html template (https:/internetarchive/openlibrary/blob/master/openlibrary/templates/account/books.html) to include a search box (design task). For a first version, we'd probably use an html form which submits a GET search query , similar to what we have on the author's page: https://openlibrary.org/authors/OL7283091A
    openlibrary org_authors_OL7229114A_Robert_Alan_Hill (1). In the future, we might want to use javascript (similar to how we the real-time Search box works at the top of the website):
    openlibrary org_authors_OL7229114A_Robert_Alan_Hill (2)
  2. Next, we'd need to update the public_my_books controller method in https:/internetarchive/openlibrary/blob/master/openlibrary/plugins/upstream/account.py#L733-L760 to accept a GET parameter. Already, the function expects a page variable to be sent as a GET parameters (https:/internetarchive/openlibrary/blob/master/openlibrary/plugins/upstream/account.py#L738) so accomplishing this should be as straightforward as adding another parameters like, i = web.input(page=1, search=None).
  3. When/where we fetch the patron's books here: https:/internetarchive/openlibrary/blob/master/openlibrary/plugins/upstream/account.py#L754, we need to alter the logic to check whether a i.search query is present (e.g. if i.search). If the i.search value is present, we'll need change the line readlog.get_works call so this optional search parameter is passed along with our request for matching books.
  4. readlog is an instance of plugins.upstream.account.ReadingLog (class defined here:
    class ReadingLog(object):
    ). Its get_works method (
    def get_works(self, key, page=1, limit=RESULTS_PER_PAGE):
    ) will need to be updated to accept an optional search parameter (e.g. (key, page=1, limit=RESULTS_PER_PAGE, search=None)). This ReadingLog.get_works function essentially uses a KEYS dictionary (defined here:
    self.KEYS = {
    'waitlists': self.get_waitlisted_editions,
    'loans': self.get_loans,
    'want-to-read': self.get_want_to_read,
    'currently-reading': self.get_currently_reading,
    'already-read': self.get_already_read
    }
    ) to lookup and then invoke the proper book-fetching function.
  5. Each of the corresponding ReadingLog methods referenced by the KEYS dictionary (namely: get_waitlisted_editions, get_loans, get_want_to_read, get_currently_reading, get_already_read) must thus also be updated to take an optional search parameter. Each of these functions ultimately makes an API call to the same function within our Bookshelves API model: Bookshelves.get_users_logged_books (https:/internetarchive/openlibrary/blob/master/openlibrary/core/bookshelves.py#L118-L149)
  6. After a search box form has been added to the template, the public_my_books view/controller has been edited to expect a search parameter, this search parameter is forwarded to our readlog.get_works call, and the readlog object (i.e. the ReadingLog class) have all been updated to accept an optional search parameter, we'll then need to do the hard work of modifying the actual API Bookshelves.get_users_logged_books (the thing which calls the database) to consider the possibility of an optional search parameter when requesting data from the database: https:/internetarchive/openlibrary/blob/master/openlibrary/core/bookshelves.py#L118-L149).

Related to

#4262, #4267

Stakeholders

@cdrini

@mekarpeles mekarpeles added Priority: 3 Issues that we can consider at our leisure. [managed] Theme: Reading Log Related to workflows for creating, modifying, displaying a user's reading log. [managed] Lead: @mekarpeles Issues overseen by Mek (Staff: Program Lead) [managed] Affects: Experience Issues relating directly to service design & patrons experience labels Apr 21, 2021
@mekarpeles mekarpeles added the State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] label Apr 21, 2021
@mekarpeles
Copy link
Member Author

Implementation
One possible way to do this is to:

  1. take the search query...
  2. get a list of all books on a patron's want to read list (careful, this could be 10k+ books!)
  3. Fetch the work ids from infobase in bulk for the search query (perhaps, if < 1k titles -- this would serve most people, and if there are more than e.g. 1k titles, don't show the search form for now :( -- I know... the people w/ most titles most need search)
  4. Do a simple check if search query == or is in the book title.

For now, because this is expensive, we probably can't do real-time search (like we do on the topnav)

Perhaps the actual solution is to use (a) use solr or (b) have this information mirrored in their archive.org items (privately) or (c) to include the book title in the bookshelves db (which may affect performance)

@cdrini cdrini added Priority: 2 Important, as time permits. [managed] and removed Priority: 3 Issues that we can consider at our leisure. [managed] labels Jun 14, 2022
@cdrini cdrini added this to the Next (proposed) milestone Jun 14, 2022
@cdrini
Copy link
Collaborator

cdrini commented Jun 14, 2022

In short we'd probably want a solr query like:

{
    'fq': 'key:(/works/OL1W OR /works/OL234W)',  # You know but dynamic
    'q.op': 'AND',
    'q': q,  # User query
    'start': offset,
    'rows': limit,
    'fl': ','.join(DEFAULT_SEARCH_FIELDS),  # From worksearch/code.py
    'qt': 'standard',
    'sort': 'work_count desc',
    'wt': 'json',
    'defType': 'edismax',
    'qf': 'text title^20 author_name^20'
},

We should DRY this up more because it duplicates some of the search page logic, but this is fine for now. Pass through execute_solr_query to get the books!

@mekarpeles
Copy link
Member Author

mekarpeles commented Aug 25, 2022

Yes, this would be a big win! e.g. Getting list of all work ids from a patrons reading log shelf and then limit a solr search to these IDs!

@cdrini cdrini added Good First Issue Easy issue. Good for newcomers. [managed] Needs: Help Issues, typically substantial ones, that need a dedicated developer to take them on. [managed] and removed State: Blocked Work has stopped, waiting for something (Info, Dependent fix, etc. See comments). [managed] labels Aug 29, 2022
@scottbarnes
Copy link
Collaborator

I'd like to work on this, @mekarpeles.

@cdrini
Copy link
Collaborator

cdrini commented Sep 30, 2022

This should do the trick:

do_search(
    {'q': 'rowling' + ' key:(/works/OL1W OR /works/OL2W)'},
    sort=None,
    page=1,
    rows=20,
)

@cdrini
Copy link
Collaborator

cdrini commented Sep 30, 2022

Sample mockup just copying authors page
image

scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 4, 2022
scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 5, 2022
scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 5, 2022
scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 5, 2022
@mekarpeles mekarpeles changed the title Adding Search / Filter capability to Reading Log My Books (Reading Log) add Search & Filter capability Oct 8, 2022
@techrajdeep
Copy link

I am looking for an issue for my first contribution . Can you help me here.

scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 12, 2022
scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 12, 2022
scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 13, 2022
cdrini pushed a commit to scottbarnes/openlibrary that referenced this issue Oct 13, 2022
cdrini pushed a commit to scottbarnes/openlibrary that referenced this issue Oct 13, 2022
scottbarnes added a commit to scottbarnes/openlibrary that referenced this issue Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects: Experience Issues relating directly to service design & patrons experience Good First Issue Easy issue. Good for newcomers. [managed] Lead: @mekarpeles Issues overseen by Mek (Staff: Program Lead) [managed] Needs: Help Issues, typically substantial ones, that need a dedicated developer to take them on. [managed] Priority: 2 Important, as time permits. [managed] Theme: Reading Log Related to workflows for creating, modifying, displaying a user's reading log. [managed]
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants