Make QueryCarousels Edition Aware by benbdeitch · Pull Request #10472 · internetarchive/openlibrary

benbdeitch · 2025-02-18T21:51:21Z

Addresses #7252

As of this moment, carousels link to and display works, rather than specific editions. This prevents users from specifically locating given editions of works on carousels, which is not ideal. This PR alters the functionality of carousels to link to and show covers of editions rather than works.

Technical

This issue originally called for restricting the editions that show to ones matching the user's language. In the name of getting this feature out sooner, that has been left out on Drini's advice, to be achieved in a later PR.

Testing

Create a new page on your local version, and create a Carousel macro. {{QueryCarousel("*")}} should be a functional command.

Screenshot

And when clicked:

Stakeholders

@jimchamp

This makes more sense for browse-ish carousels

codecov-commenter · 2025-02-18T21:52:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (edition-aware-carousels@b3a7779). Learn more about missing BASE report.

Additional details and impacted files

@@                    Coverage Diff                     @@
##             edition-aware-carousels   #10472   +/-   ##
==========================================================
  Coverage                           ?   17.27%           
==========================================================
  Files                              ?       87           
  Lines                              ?     4845           
  Branches                           ?      860           
==========================================================
  Hits                               ?      837           
  Misses                             ?     3477           
  Partials                           ?      531

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

for more information, see https://pre-commit.ci

benbdeitch · 2025-02-20T22:18:38Z

So, this mostly works so far. I'm still ironing out a few kinks. I've stopped it from breaking the current macro, but haven't yet figured out why the addition of 'lang' causes problems. Also, it doesn't yet show editions, which is a problem.

…ibraryfork into edition-aware-carousels

This reverts commit e1e5c1e.

benbdeitch · 2025-02-22T18:05:17Z

Alright, everything should be good to go here. The commit history is slightly messy, but nothing that should cause a problem. It's been tested on my end, and works fine.

mekarpeles · 2025-02-27T20:22:42Z

It looks like our Trusted Book Providers and/or Direct Provider books may show up as Locate instead of Read (which may be an issue in LoanStatus macro or in solr).

EDIT: Books from trusted & direct book providers should already be in solr via the ebook_access = "public" parameter, e.g:
https://testing.openlibrary.org/search.json?q=OL57790023M&fields=* shows us ebook_access = "public".

In LoanStatus, if we notice ebook_access is "public" then we can show the read button and use /books/{olid}/{title}/borrow?action=read as the target.

See: https://github.com/internetarchive/openlibrary/blob/master/openlibrary/macros/LoanStatus.html#L56-L60

mekarpeles · 2025-02-28T11:21:20Z

--- a/openlibrary/macros/LoanStatus.html
+++ b/openlibrary/macros/LoanStatus.html
@@ -59,6 +59,10 @@ $elif availability.get('is_readable') or availability.get('status') == 'open':
   $if secondary_action:
     $:macros.BookSearchInside(ocaid)
 
+$elif is_edition and doc.get('ebook_access') == 'public'):
+  $ olid = doc.key.split('/')[2]
+  $:macros.TrustedBookProviderReadButton(olid, title=doc.get('title'))
+
 $elif ocaid and ctx.user and ctx.user.is_printdisabled():
   $# Exemptions for patrons with Print Disabilities
   $ pd_eligible = availability and availability.get('is_printdisabled')
diff --git a/openlibrary/macros/TrustedBookProviderReadButton.html b/openlibrary/macros/TrustedBookProviderReadButton.html
new file mode 100644
index 000000000..2078d240b
--- /dev/null
+++ b/openlibrary/macros/TrustedBookProviderReadButton.html
@@ -0,0 +1,13 @@
+$def with(olid, title="ebook", provider="Trusted Book Provider")
+
+$ stream_url = "/books/%s/-/borrow?ref=ol" % olid
+$ action = "read"
+$ label = _("Read")
+$ atitle = _("Read '%s' using %s" % (title, provider))
+
+<div class="cta-button-group">
+  <a href="/books/$olid/-/borrow?action=$action" title="$atitle"
+     $:analytics_attr('Read') target="_blank"
+     aria-haspopup="true" aria-controls="direct-provider-toast-$(olid)"
+     class="cta-btn cta-btn--available cta-btn--read cta-btn--external cta-btn--direct">$label</a>
+</div>

mekarpeles · 2025-02-28T17:45:24Z

Another possible blocker:

On loadmore, we may need to update js rendering to ensure it also works with editions (e.g. title)

The templates in our js are out of date

https://github.com/internetarchive/openlibrary/blob/master/openlibrary/plugins/openlibrary/js/carousel/Carousel.js#L194

Options

Fix carousel js 🤷
We could create some template similar to... https://github.com/internetarchive/openlibrary/blob/master/openlibrary/macros/SearchResultsWork.html
Which is CarouselBook... And then have a partial endpoint that returns a list of CarouselBook(s)
Refactor the https://github.com/internetarchive/openlibrary/blob/master/openlibrary/templates/books/custom_carousel.html to include another template that is just the CarouselBook and then have both the custom_carousel render CarouselBook and the partial render CarouselBook
Extend search.json -> search.html to return as standard html (basically returns custom_carousel)

mekarpeles · 2025-02-28T17:49:26Z

Clicking on provider read link opens toast multiple times.

mekarpeles · 2025-03-03T21:40:38Z

https://github.com/internetarchive/openlibrary/blob/f2c9b8d8ec4e984bb9c917117bf893a4bb535611/openlibrary/templates/book_providers/wikisource_read_button.html

jimchamp · 2025-03-03T22:45:44Z

Thanks @benbdeitch! I'm going to go ahead and merge this into the edition-aware-carousel branch, and fix the loadmore and toast message issues there. This is great!

## Implement Unified Read Button Changes Creates a Unified Read Button Dropper that works across the Books Page, Authors Page, and Search Page. This includes the Book Page editions table. Currently uses a conditional check to avoid rendering in carousels because (a) carousels have not yet been made #10472 Edition-Aware and also because the styling is broken. The dropper shows the right primary action based on the case (e.g. Borrow, Read, Special Access) The dropdown options (which display in cases other than carousels) show `locate` and `listen` where applicable, with icons, and will soon show `search inside` and `download` options. We switched the Locate feature to use a new /borrow?action=locate method if an OCLC or ISBN identifier exists for an edition and simplified the code logic away from WorldcatURL.html. --------- Co-authored-by: Mek <[email protected]>

cdrini added 2 commits February 18, 2025 16:12

Make QueryCarousel use edition-aware search

b684fba

Make MoreLikeThis carousels require user language

e1e5c1e

This makes more sense for browse-ish carousels

github-actions Bot assigned cdrini Feb 18, 2025

github-actions Bot added the Priority: 2 Important, as time permits. [managed] label Feb 18, 2025

benbdeitch marked this pull request as draft February 18, 2025 21:51

github-actions Bot added the Needs: Response Issues which require feedback from lead label Feb 19, 2025

Forward lang param to partials endpoint

be9f271

benbdeitch force-pushed the edition-aware-carousels branch from 702a924 to be9f271 Compare February 20, 2025 18:27

Checking solution to the carousel not loading.

b38a1a4

benbdeitch force-pushed the edition-aware-carousels branch from a191464 to b38a1a4 Compare February 20, 2025 21:53

[pre-commit.ci] auto fixes from pre-commit.com hooks

fbbd118

for more information, see https://pre-commit.ci

benbdeitch requested a review from cdrini February 20, 2025 21:59

Readded accidentally deleted argument.

da5a11c

jimchamp assigned jimchamp and unassigned cdrini Feb 21, 2025

benbdeitch added 3 commits February 21, 2025 20:29

Fixed misc errors, removed incomplete 'require_user_lang' functionality.

91f253d

Merge branch 'edition-aware-carousels' of github.com:benbdeitch/openl…

570964c

…ibraryfork into edition-aware-carousels

Revert "Make MoreLikeThis carousels require user language"

4cc7da1

This reverts commit e1e5c1e.

benbdeitch marked this pull request as ready for review February 22, 2025 18:04

mekarpeles reviewed Feb 28, 2025

View reviewed changes

Comment thread openlibrary/macros/RawQueryCarousel.html Outdated

adding providers to carousels query

83e0e8f

jimchamp changed the base branch from master to edition-aware-carousels March 3, 2025 22:43

jimchamp merged commit a5e9776 into internetarchive:edition-aware-carousels Mar 3, 2025

jimchamp mentioned this pull request Mar 3, 2025

Make QueryCarousels edition-aware #7252

Closed

cdrini mentioned this pull request Mar 10, 2025

Make QueryCarousel use edition-aware search #7295

Closed

Navigation Menu

Uh oh!

Uh oh!

Make QueryCarousels Edition Aware - #10472

Make QueryCarousels Edition Aware#10472
jimchamp merged 10 commits into
internetarchive:edition-aware-carouselsfrom
benbdeitch:edition-aware-carousels

benbdeitch commented Feb 18, 2025 •

edited by jimchamp

Loading

Uh oh!

codecov-commenter commented Feb 18, 2025 •

edited

Loading

Uh oh!

benbdeitch commented Feb 20, 2025

Uh oh!

benbdeitch commented Feb 22, 2025

Uh oh!

mekarpeles commented Feb 27, 2025 •

edited

Loading

Uh oh!

mekarpeles commented Feb 28, 2025

Uh oh!

Uh oh!

mekarpeles commented Feb 28, 2025 •

edited

Loading

Uh oh!

mekarpeles commented Feb 28, 2025 •

edited

Loading

Uh oh!

mekarpeles commented Mar 3, 2025

Uh oh!

jimchamp commented Mar 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Uh oh!

Uh oh!

Conversation

benbdeitch commented Feb 18, 2025 • edited by jimchamp Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Technical

Testing

Screenshot

Stakeholders

Uh oh!

codecov-commenter commented Feb 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

benbdeitch commented Feb 20, 2025

Uh oh!

benbdeitch commented Feb 22, 2025

Uh oh!

mekarpeles commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mekarpeles commented Feb 28, 2025

Uh oh!

Uh oh!

mekarpeles commented Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Options

Uh oh!

mekarpeles commented Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mekarpeles commented Mar 3, 2025

Uh oh!

jimchamp commented Mar 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

benbdeitch commented Feb 18, 2025 •

edited by jimchamp

Loading

codecov-commenter commented Feb 18, 2025 •

edited

Loading

mekarpeles commented Feb 27, 2025 •

edited

Loading

mekarpeles commented Feb 28, 2025 •

edited

Loading

mekarpeles commented Feb 28, 2025 •

edited

Loading