Fix web book toast message not matching the web book read #9663
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
scottbarnes
force-pushed
the
9639/fix/ensure-web-book-toast-message-matches-the-web-book-read
branch
2 times, most recently
from
a4b93b0 to
8d930d0
Compare
scottbarnes
added
On testing.openlibrary.org
cdrini
requested changes
Jul 31, 2024
scottbarnes
force-pushed
the
9639/fix/ensure-web-book-toast-message-matches-the-web-book-read
branch
4 times, most recently
from
604ba3c to
b3fb885
Compare
This commit stops non-DirectProvider web books from being matched as `DirectProvider`s. The `Storage` objects that ultimately come from `SearchResponse` have `Acquisition` objects in their `providers` fields, so every web book search result will have a `providers` attribute. This commit limits the matching for `DirectProvider` to cases where the `Acquisition` object has a value other than `None`, which is the default initialization value for `Acquisition.provider_name`. More exploration will be needed to see whether a similar mis-match problem occurs when there is more than one `Acquisition` per `Work`.
The `direct_read_button.html` needs a unique `data-toast-trigger` per domain, otherwise every `DirectProvider` book, when multiple are displayed from different providers, will have trigger the same class id.
Co-authored-by: Drini Cami <cdrini@gmail.com>
scottbarnes
force-pushed
the
9639/fix/ensure-web-book-toast-message-matches-the-web-book-read
branch
3 times, most recently
from
bafeb9a to
04708e6
Compare
cdrini
added
Needs: Submitter Input
scottbarnes
force-pushed
the
9639/fix/ensure-web-book-toast-message-matches-the-web-book-read
branch
from
04708e6 to
d6c8001
Compare
github-actions
bot
removed
the
Needs: Submitter Input
cdrini
approved these changes
Aug 9, 2024
cdrini
changed the title
scottbarnes
deleted the
9639/fix/ensure-web-book-toast-message-matches-the-web-book-read
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #9639
This PR stops non-DirectProvider web books from being matched as
DirectProviders.It also stops every
DirectProviderbook on a page from triggering the same toast message.Technical
With respect to the first issue:
The
Storageobjects that ultimately come fromSearchResponsehaveAcquisitionobjects in theirprovidersfields, so every web book search result will have aprovidersattribute.This PR limits the matching for
DirectProviderto cases where theAcquisitionobject has a value other thanNone, which is the default initialization value forAcquisition.provider_name.More exploration will be needed to see whether a similar mis-match problem occurs when there is more than one
AcquisitionperWork.With respect to the second issue:
The
direct_read_button.htmlneeds a uniquedata-toast-triggerper domain, otherwise everyDirectProviderbook, when multiple are displayed from different providers, will have trigger the same class id.Testing
Visit https://testing.openlibrary.org/search?q=format%3A%22Web+book%22&mode=everything&page=2
Screenshot
Stakeholders