Repository navigation
Show the digital files a reader cannot open yet, with the reason - #50
fabiodalez-dev wants to merge 1 commit into
Conversation
Pinakes 0.8.6 decides who may open each digital file (fabiodalez-dev/Pinakes#480) and tells the app per file: a closed one keeps its entry with url null and access login_required or loan_required. The detail page now keeps that card, with a lock and the reason ("For readers who have this book on loan", "Sign in to open it") in place of the button, in the four languages. Closed files all arrive with the same empty address, so the list no longer folds them into one: only the open files are told apart by their address. Servers before 0.8.6 send neither field and every file stays open, as before. The links to the files the reader may open are signed and last a day. A tap on one that has expired fetches the record again for a fresh link and asks to tap again, instead of opening a file the server would refuse.
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues. There are two rough edges in the expired-link path, both inline.
Reviewed changes
I reviewed the initial commit, which adds client support for Pinakes 0.8.6 per-file digital access (fabiodalez-dev/Pinakes#480).
- Access-aware
DigitalAttachment: new nullablevisibility/accessfields, alockedflag, andpk_expparsing for signed-link expiry. Older servers send neither field, so their files stay open. digitalFilesdedup: locked files with an emptyurlare kept. Only open files are deduplicated by address.- Locked card UI:
LockedDigitalFileshows a lock and the reason (login or loan) in place of the Read/Play button. - Expired-link refresh: tapping a signed link that is about to expire reloads the record and asks the reader to tap again.
- i18n: four new keys in all of
en/it/fr/de, with no em dashes. - Tests: three Robolectric tests cover closed cards, pre-0.8.6 servers and an expired link.
claude-opus-5-5 ｜ 𝕏
| onReserve = vm::openLoanSheet, | ||
| onToggleWishlist = vm::toggleWishlist, | ||
| onFindWorks = onFindWorks, | ||
| onRefreshLinks = vm::load, |
There was a problem hiding this comment.
vm::load first sets content = UiState.Loading, which removes DetailContent from the screen. When the record comes back, its rememberScrollState() starts at the top again, so the reader is sent back to the cover and has to scroll down to the digital section to "tap again". The "Link refreshed" snackbar also appears before the fetch has finished. If that fetch fails with anything other than a network error (an expired session, a 5xx), the whole page turns into an error state.
Technical details
# Link refresh tears down the detail page
## Affected sites
- app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt:205 — `onRefreshLinks = vm::load`
- app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt:88 — `load()` sets `UiState.Loading` before fetching
- app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt:531-533 — snackbar shown synchronously, before the refresh completes
## Required outcome
- Refreshing links keeps the current page (and its scroll position) on screen, and only swaps in the new `BookDetail` on success.
- A failed refresh leaves the page as it was and reports the failure, without replacing it with `ErrorState`.
- The "Link refreshed" message is only shown after the refresh has succeeded.
## Suggested approach (optional)
- Add a `refreshLinks()` to `BookDetailViewModel` that calls `catalog.book(bookId)` without touching `content` first, updates `content` on `ApiResult.Success`, and exposes a one-shot result (e.g. like `requestedConfirmation`) that the screen turns into the snackbar.
## Open questions for the human (optional)
- Once the refresh keeps the page in place, should the file open by itself after a successful refresh instead of asking for a second tap?|
|
||
| /** The signed link has expired, or will within a minute: the record must be fetched again first. */ | ||
| fun linkExpired(nowSeconds: Long = System.currentTimeMillis() / 1000): Boolean = | ||
| expiresAt?.let { it <= nowSeconds + 60 } == true |
There was a problem hiding this comment.
This compares the server-issued pk_exp with the device's wall clock. If a phone's clock is more than about a day ahead, every newly signed link already looks expired. Each tap then reloads the record and asks for another tap, so the file can never be opened. Before this PR, the app passed the link to the server and let the server decide.
Technical details
# Device clock skew turns the expiry check into an endless refresh
## Affected sites
- app/src/main/java/com/pinakes/app/data/model/Models.kt:384-385 — `linkExpired` uses `System.currentTimeMillis()`
- app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt:531 — every tap on an "expired" link refreshes instead of opening
## Required outcome
- A reader whose device clock is wrong can still open a link the server has just issued; the client-side check must not block a freshly fetched link forever.
## Suggested approach (optional)
- Refresh at most once: if a link that was just fetched still reads as expired, open it anyway and let the server accept or refuse it. Another option is to measure link age from when the record was fetched (`SystemClock.elapsedRealtime()` in the ViewModel) instead of comparing `pk_exp` with the wall clock.This branch has not been deployed
1 participant
Pinakes 0.8.6 decides who may open each digital file (fabiodalez-dev/Pinakes#480) and says so per file in the book detail: a file the reader may not open yet keeps its entry, with
url: nullandaccessset tologin_requiredorloan_required.I checked it in the emulator against a local Pinakes with the new access control: an open edition and a signed-in-only edition open with their signed links, the loan-only edition shows the lock and the reason, and once the reader has the book on loan it opens as well. The full unit suite passes (392 tests), with three new ones for the closed cards, older servers and an expired link.