fix: redirect to book page with flash message after returning a book#11947
Merged
lokesh merged 7 commits intointernetarchive:masterfrom Mar 25, 2026
Merged
Conversation
bhardwajparth51
force-pushed
the
issue-9115-flash-message
branch
2 times, most recently
from
bbf4047 to
133084a
Compare
bhardwajparth51
force-pushed
the
issue-9115-flash-message
branch
from
133084a to
453e00d
Compare
Contributor
Author
|
@lokesh would appreciate a review when you get a chance. Happy to make any adjustments! |
github-actions
bot
added
the
Needs: Response
lokesh
requested changes
Mar 13, 2026
bhardwajparth51
force-pushed
the
issue-9115-flash-message
branch
2 times, most recently
from
0efedeb to
41a18f6
Compare
bhardwajparth51
force-pushed
the
issue-9115-flash-message
branch
from
41a18f6 to
9582929
Compare
Contributor
Author
|
@lokesh I've updated the PR to address your feedback. |
- Replace contextlib.suppress with explicit try/except for clearer intent - Consolidate redirect URL encoding into a single urllib.parse.quote call - Use urlunsplit for redirect sanitization (preserves fragment, simpler code) - Move stats.increment into success branch so failed returns aren't counted Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
for more information, see https://pre-commit.ci
Add noqa for PLR0912 (too many branches) alongside existing PLR0915 suppression, and use contextlib.suppress for the PatronAccessException catch-and-pass pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
lokesh
approved these changes
Mar 13, 2026
Collaborator
lokesh
left a comment
lokesh
left a comment
There was a problem hiding this comment.
A few changes I added to the PR to bullet proof things and improve readability:
- Replace contextlib.suppress with explicit try/except for clearer intent
- Consolidate redirect URL encoding into a single urllib.parse.quote call
- Use urlunsplit for redirect sanitization (preserves fragment, simpler code)
We'll need to verify on testing server before merging.
lokesh
added
Needs: Testing
and removed
Needs: Response
github-actions
bot
added
the
Needs: Response
Contributor
Author
|
Hey @lokesh @mekarpeles, just checking in — has this been verified on the testing server yet? |
Collaborator
I was able to test successfully. Note: There is an issue on the testing server with icon rendering, thus the garbled background on the flash message. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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 #9115
Technical
This PR updates the book return flow to show a flash message confirmation upon successful return, providing much-needed feedback to the patron instead of a silent reload.
Implementation Details:
Used
add_flash_messagefromvendor/infogami/infogami/utils/flash.pyin the return block insideopenlibrary/plugins/upstream/borrow.pyto show a green success notification.Note on Commits:
I initially experimented with removing the hidden redirect input and hardcoding a redirect to
edition.url(). However, I realized the originaledition_redirectapproach is actually better UX. It ensures the user is cleanly redirected back to whichever specific page they initiated the return from (e.g., if they were on the Loans page, they stay there; if they were on the Book page, they stay there). My final commit restoresedition_redirectand the hidden form input to preserve this behavior while adding the flash message.Testing
/account/loans) or the book's specific canonical page.Stakeholders
@mekarpeles @RayBB