Repository navigation
Release 1.10.0 (versionCode 24) - #54
Merged
Merged
Conversation
The loan-request sheet proposed the chosen start plus the library's loan_duration_days whatever the calendar showed. When a scheduled loan or a reservation on every copy began inside that window the server refused the whole request with a bare "not available", although a shorter loan was possible, and the reader was never told how long. LoanWindow.kt reads the availability calendar the sheet already holds: the first fully-booked day inside the duration bounds the loan, the return date becomes the day before it, and the sheet explains it under the calendar: maximum loan length, return date, and the day from which the book is already booked. The request then carries that date as end_date of POST /reservations (Mobile API 1.8.2), so the server accepts the shortened loan the way the website calendar does. Tapping a red day, which cannot be picked, now says why. A not_available refusal from the server carries its own explanation and meta figures (max_end_date, max_duration_days, first_conflict_date); the message is what the reader sees and the figures are modelled for later use. Strings in it, en, de, fr. LoanWindowTest covers the capped, uncapped, conflict-beyond-duration, next-day-blocked, partial-day, unavailable- dates and unknown-duration cases; the sheet tests take the two-argument confirm callback. The API contract documents end_date.
…et and the envelope
A commitment on the very next day leaves no loan of at least one day:
LoanWindow.possible is false, the sheet says so ("no loan is possible
from this start date: the book is already booked from D") and disables
the request instead of proposing a same-day return. The availability
payload's max_loan_duration_days (Mobile API 1.8.2) is modelled.
LoanRequestWindowTest renders the sheet: a scheduled loan inside the
duration shortens the return date and the note says so; confirming sends
the shortened date as end_date; with no conflict nothing is explained
and no end_date is sent; tapping a fully-booked day explains why it
cannot be picked and leaves the pick unchanged; a commitment on the next
day disables the request. ReservationRefusalEnvelopeTest pins what the
server says to the app: the not_available sentence and meta figures, a
refusal without figures, the max_duration_exceeded code, end_date sent
only when shortened, both durations in the availability payload.
…r note CalendarModel painted a day labelled free with no free copy red and unpickable, but loanWindowFrom only looked at unavailable_dates and the full state, so such a day inside the duration did not cap the request and the server refused it. AvailabilityCalendar.isFullyBooked() is now the one predicate both use, with a test for the stale label. A cap one day out read "1 days": book_loan_limit_note_one is the singular variant (the string generator has no plurals). The zero-day case already reads as "no loan is possible from this start date".
Bumps the version and records in STATUS what 1.10.0 brings: loan requests bounded by a scheduled loan and by the configured maximum (#53, Pinakes #490). LoanRequestSheetUiTest follows the sheet's new onConfirm(start, end) and checks that no end date is sent when nothing caps the window.
|
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.
Important
On a Pinakes 0.8.6 server the sheet shows a shortened loan that the server cannot grant. The release notes say older servers keep the 1.9.0 behaviour, but that is not true for 0.8.6 (see inline).
Reviewed changes
I reviewed the 1.10.0 release branch. It merges the still-open #53 (the loan window cap) and bumps the version.
- Loan window: the new
loanWindowFromends the request the day before the first fully-booked day, or atmax_loan_duration_days. The sheet explains the cap, explains a tapped red day, disables the request when no window is left, and sendsend_date. - Single red-day predicate:
AvailabilityCalendar.isFullyBookednow drives bothCalendarModel.kindand the window, so the days the calendar paints red are the days that cap the loan. - API plumbing:
ReservationRequest.end_dateis passed throughLibraryRepository.reserveandBookDetailViewModel.reserve.MetaandAvailabilityCalendargain the optional Mobile API 1.8.2 fields. - Release bookkeeping:
versionCodegoes to 24 andversionNameto 1.10.0,STATUS.mdgains a 1.10.0 section, and four i18n keys are added in all four languages. The count of 444 JVM@Tests matches the tree.
I checked the server side against the Pinakes tags. LoanWindow::cap in v0.8.7 picks the first conflict strictly after the start, which is the same rule the app uses.
claude-opus-5-5 ｜ 𝕏
Pinakes 0.8.6 (Mobile API 1.8.1) already sends loan_duration_days but ignores end_date, so the sheet showed a shortened return date the server would refuse or end on another day. loanWindowFrom() caps only when the calendar carries max_loan_duration_days, which Pinakes 0.8.7 always sends; without it the window is the default one, uncapped and without end_date, as in 1.9.0. LoanWindowTest covers both cases (they fail without the check), and the fixtures of the capped cases now carry the maximum as a 0.8.7 server sends it.
There was a problem hiding this comment.
✅ No new issues found. The new commit fixes the 0.8.6 compatibility issue from the previous review.
Reviewed changes
I reviewed the one commit pushed since the previous review (aa5feb7).
- The cap now depends on the server version: when the calendar has no
max_loan_duration_days, as with Pinakes 0.8.6 / Mobile API 1.8.1,loanWindowFromnow returns the default window: start +loan_duration_days, uncapped,possible, and noend_date. That matches the 1.9.0 behaviour the release notes promise. The hint shows the same date the oldloanReturnDategave, the request button stays enabled, and no cap or no-window note appears. On 1.8.2 servers nothing changes. - Tests: the fixtures in
LoanWindowTestthat expect a cap now setmaxLoanDurationDays = 90, like a 0.8.7 server. Two new cases check that an old server never caps the window and never says no loan is possible. The fixture inLoanRequestWindowTestalready set the field, and the tests without it (LoanRequestReturnDateTest,LoanRequestSheetUiTest) still expect the uncapped result. STATUS.md: the 1.10.0 notes now describe the gate, and the unit test count went from 444 to 446. I checked that 446 matches the@Testcount in the tree.
claude-opus-5-5 ｜ 𝕏
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
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.
What this release contains
max_loan_duration_days, and sends it asend_date; a start whose next day is booked says no loan is possible. Pairs with Pinakes 0.8.7 (#490, Mobile API 1.8.2); older servers keep the 1.9.0 behaviour.Release commit 495d0f7:
versionCode23 → 24,versionName1.9.0 → 1.10.0, a 1.10.0 section inSTATUS.md, andLoanRequestSheetUiTestupdated to the sheet'sonConfirm(start, end)(it no longer compiled after the merge), now also checking that no end date is sent when nothing caps the window.Checks
assembleDebug.LoanRequestSheetUiTest,CalendarLayoutTest,DesiderataStaffFlowTest,DigitalLinkRefreshFlowTest), none failed.max_loan_duration_daysin the availability calendar,end_dateonPOST /reservations,meta.max_end_date/max_duration_days/first_conflict_dateon a refusal. All are optional in the app.Merging this PR also merges #53.