From d76b0d60ed0073a0dc09bcc2359b4a7e08fef9c8 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 13 Oct 2026 11:57:52 +0200 Subject: [PATCH 01/14] Restyle the loan request sheet after the web's loan calendar The sheet is a plain dialog instead of DatePickerDialog, which capped its height and cut the month in half: 20dp corners, a Fraunces title, confirm and cancel stacked at full width in the app's button styles. The month follows the web: a free day is a plain number, limited and unavailable days keep their tint with a faint rule of their own colour, today has an accent ring and the chosen day takes the theme's button colour. The legend uses dots. When availability cannot be loaded, the fallback Material picker takes the sheet's side padding too: on a 360dp phone its seventh column was otherwise cut in half. LoanRequestSheetUiTest covers the buttons, the pre-selected earliest day, the disabled states and the seventh column at 360dp (verified to fail without the fix). DESIGN.md documents the sheet. --- DESIGN.md | 9 + STATUS.md | 4 +- .../com/pinakes/app/LoanRequestSheetUiTest.kt | 116 ++++++++++++ .../app/ui/screens/detail/BookDetailScreen.kt | 176 ++++++++++++------ .../app/ui/screens/detail/LoanCalendar.kt | 69 ++++--- 5 files changed, 283 insertions(+), 91 deletions(-) create mode 100644 app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt diff --git a/DESIGN.md b/DESIGN.md index 6fd6602..9e05d96 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -224,6 +224,15 @@ The reference layout (top to bottom on a phone): - then dark "Copia negli appunti", outline "Scarica RIS" and outline "Cita" (all styles). 15. **Potrebbero interessarti:** white band with a top `line` border, Fraunces 40 heading, a horizontal row of book cards. +### Loan request sheet + +The sheet the "Richiedi prestito" button opens, as the web's loan request: + +- A plain dialog, not Material's `DatePickerDialog` (that one caps its height and cut the month in half): 20dp corners, 8dp from the screen edges, at most 480dp wide, a Fraunces title, scrolling only on a screen too short for it. +- The month as the web's loan calendar: a free day is a plain number in `onSurface`; only the days that need a word carry a tint, amber for limited and `errorContainer` for unavailable, each ringed by a faint rule of its own text colour. Today has an accent ring; the chosen day is filled with the theme's `button` colour and `buttonText`, as the button that opened the sheet. The legend uses dots, like the days it explains. +- Confirm (filled) and cancel (outline) stacked at full width, never Material's text-button row. Confirm waits for a day and shows a spinner while the request is sent. +- When availability cannot be loaded, Material's plain picker also takes the sheet's side padding, so its seven 48dp columns fit a 360dp phone. + ### Other screens Wishlist, loans and reservations, profile, events, periodicals (Emeroteca), archive, static pages: diff --git a/STATUS.md b/STATUS.md index 84df63d..32878d1 100644 --- a/STATUS.md +++ b/STATUS.md @@ -6,8 +6,8 @@ The app now uses the 2026 web design: Geist / Fraunces, warm neutrals, a server- - Debug APK: `pinakes-debug.apk`, copied from `app/build/outputs/apk/debug/app-debug.apk` (generated locally and ignored by Git). - Package: `com.pinakes.app`, version `1.6.2` (19), minSdk 26, target/compileSdk 35. -- Local verification: **200 debug and 200 release unit tests, 22 instrumented tests on Android 15, zero lint errors (101 warnings)**, and successful debug + R8 release builds. The 1.6.2 debug APK installs over 1.6.1 with the same signing certificate. -- Verification commands: `assembleDebug`, `testDebugUnitTest`, `lintDebug`, `assembleRelease`, `assembleDebugAndroidTest`; the 22 device tests run through `am instrument` on a separate Android 15 AVD, preserving the authenticated demo device. +- Local verification: **200 debug and 200 release unit tests, 26 instrumented tests on Android 15, zero lint errors (101 warnings)**, and successful debug + R8 release builds. The 1.6.2 debug APK installs over 1.6.1 with the same signing certificate. +- Verification commands: `assembleDebug`, `testDebugUnitTest`, `lintDebug`, `assembleRelease`, `assembleDebugAndroidTest`; the 26 device tests run through `am instrument` on a separate Android 15 AVD, preserving the authenticated demo device. - Unit tests cover the existing contracts and theme contrast/mixing. Compose device tests cover full tall artwork, missing metadata, grid actions, view selection, digital-file cards, narrow search placeholders, circulation actions theme pairings, reachable empty-home actions and query preservation during loading. - Release builds exercise R8 and resource shrinking. Without release credentials the output is unsigned; no store release is published by this change. - macOS standalone emulator startup: `tools/run-emulator.sh` keeps a power assertion scoped to the emulator PID. A control with the identical debug APK reproduced a 21-second startup timeout at host priority 4; three starts with the assertion completed in 2.3–3.1 seconds. After a VM restart through the launcher, five more cold starts completed in 1.66–1.80 seconds with no ANR events. The original startup ANR also coincided with system/launcher stalls. An older AVD still stalled on a later cold boot, and a snapshot retained a system ANR dialog: the assertion does not repair an unhealthy AVD. No Sentry events are suppressed, and no production-device ANR fix is claimed. diff --git a/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt b/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt new file mode 100644 index 0000000..1a22506 --- /dev/null +++ b/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt @@ -0,0 +1,116 @@ +package com.pinakes.app + +import androidx.compose.runtime.CompositionLocalProvider +import androidx.compose.ui.platform.LocalDensity +import androidx.compose.ui.semantics.Role +import androidx.compose.ui.semantics.SemanticsProperties +import androidx.compose.ui.semantics.getOrNull +import androidx.compose.ui.test.* +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.unit.Density +import androidx.compose.ui.unit.width +import androidx.test.platform.app.InstrumentationRegistry +import com.pinakes.app.data.model.AvailabilityCalendar +import com.pinakes.app.data.model.AvailabilityDay +import com.pinakes.app.ui.screens.detail.LoanRequestSheet +import com.pinakes.app.ui.theme.PinakesTheme +import org.junit.Assert.* +import org.junit.Rule +import org.junit.Test +import java.time.LocalDate + +class LoanRequestSheetUiTest { + @get:Rule val compose = createComposeRule() + + private val today = LocalDate.now() + private val context get() = InstrumentationRegistry.getInstrumentation().targetContext + + /** Six weeks from today: mostly free, a partial day and a full one in the first week. */ + private fun calendar(earliest: String? = today.plusDays(1).toString()) = AvailabilityCalendar( + totalCopies = 2, + earliestAvailable = earliest, + unavailableDates = listOf(today.plusDays(3).toString()), + days = (0L until 42L).map { offset -> + val date = today.plusDays(offset) + when (offset) { + 2L -> AvailabilityDay(date.toString(), available = 1, loaned = 1, state = "partial") + 3L -> AvailabilityDay(date.toString(), available = 0, loaned = 2, state = "full") + else -> AvailabilityDay(date.toString(), available = 2, state = "free") + } + }, + ) + + private fun sheet( + calendar: AvailabilityCalendar? = calendar(), + fallback: Boolean = false, + submitting: Boolean = false, + onDismiss: () -> Unit = {}, + onConfirm: (String) -> Unit = {}, + widthDp: Float? = null, + ) = compose.setContent { + // widthDp: lay the sheet out as on a phone that many dp wide, whatever the emulator. + val base = LocalDensity.current + val density = widthDp?.let { Density(context.resources.displayMetrics.widthPixels / it, base.fontScale) } ?: base + CompositionLocalProvider(LocalDensity provides density) { PinakesTheme { + LoanRequestSheet( + loading = false, + calendar = calendar, + fallback = fallback, + submitting = submitting, + onDismiss = onDismiss, + onConfirm = onConfirm, + ) + } } + } + + @Test fun bothButtonsSpanTheSheetAndConfirmSendsTheEarliestDay() { + var confirmed: String? = null + var dismissed = false + sheet(onDismiss = { dismissed = true }, onConfirm = { confirmed = it }) + + val sheet = compose.onNodeWithTag("loan-sheet").getBoundsInRoot() + val confirm = compose.onNodeWithTag("loan-confirm").assertIsDisplayed().assertIsEnabled().getBoundsInRoot() + val cancel = compose.onNodeWithTag("loan-cancel").assertIsDisplayed().getBoundsInRoot() + assertEquals("Confirm and cancel share one width", confirm.width.value, cancel.width.value, .5f) + assertTrue("Buttons stay inside the sheet", confirm.left >= sheet.left && confirm.right <= sheet.right) + assertTrue("Cancel sits under confirm", cancel.top >= confirm.bottom) + + compose.onNodeWithTag("loan-confirm").performClick() + compose.runOnIdle { assertEquals(today.plusDays(1).toString(), confirmed) } + compose.onNodeWithTag("loan-cancel").performClick() + compose.runOnIdle { assertTrue(dismissed) } + } + + @Test fun confirmWaitsForADay() { + sheet(calendar = calendar(earliest = null)) + compose.onNodeWithTag("loan-confirm").assertIsNotEnabled() + } + + @Test fun confirmIsDisabledWhileTheRequestIsSent() { + sheet(submitting = true) + compose.onNodeWithTag("loan-confirm").assertIsNotEnabled() + compose.onNodeWithTag("loan-cancel").assertIsEnabled() + } + + @Test fun fallbackPickerKeepsItsSeventhColumnInsideTheSheet() { + // 360dp, the narrowest common phone: Material's seven 48dp columns need the sheet's side padding. + sheet(calendar = null, fallback = true, widthDp = 360f) + // Pixels throughout: the dp helpers of the test rule use the device density, not the forced one. + val sheetRight = compose.onNodeWithTag("loan-sheet").fetchSemanticsNode().boundsInRoot.right + val days = compose.onAllNodes( + // Material announces each day as one button whose text is the full date. + SemanticsMatcher("a day of the month") { node -> + node.config.getOrNull(SemanticsProperties.Role) == Role.Button && + node.config.getOrNull(SemanticsProperties.Text)?.joinToString { it.text } + ?.let { Regex("\\b\\d{1,2}\\b").containsMatchIn(it) && it.contains(today.year.toString()) } == true + }, + ).fetchSemanticsNodes() + assertEquals("Every day of the month is laid out", today.lengthOfMonth(), days.size) + // boundsInRoot is clipped by the sheet: a cut seventh column shows up as narrow days. + // Each day must keep its 40dp circle in view (without the bleed, 24dp of it is left). + val px = context.resources.displayMetrics.widthPixels / 360f + val narrowest = days.minOf { it.boundsInRoot.width } / px + assertTrue("A day is cut by the sheet: ${narrowest}dp of its 40dp circle in view", narrowest >= 39.5f) + assertTrue("A day overflows the sheet", days.maxOf { it.boundsInRoot.right } <= sheetRight + 1f) + } +} diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 5be2dab..21dde7d 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -39,7 +39,11 @@ import androidx.compose.material.icons.outlined.Schedule import androidx.compose.material.icons.outlined.LocalOffer import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.DatePicker -import androidx.compose.material3.DatePickerDialog +import androidx.compose.foundation.layout.widthIn +import androidx.compose.ui.window.DialogProperties +import androidx.compose.ui.layout.layout +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.window.Dialog import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon import androidx.compose.material3.IconButton @@ -50,7 +54,6 @@ import androidx.compose.material3.SnackbarHost import androidx.compose.material3.SnackbarHostState import androidx.compose.material3.Surface import androidx.compose.material3.Text -import androidx.compose.material3.TextButton import androidx.compose.material3.rememberDatePickerState import androidx.compose.runtime.Composable import androidx.compose.runtime.LaunchedEffect @@ -210,7 +213,7 @@ fun BookDetailScreen( */ @OptIn(ExperimentalMaterial3Api::class) @Composable -private fun LoanRequestSheet( +internal fun LoanRequestSheet( loading: Boolean, calendar: AvailabilityCalendar?, fallback: Boolean, @@ -224,76 +227,112 @@ private fun LoanRequestSheet( mutableStateOf(calendar?.earliestAvailable?.takeIf { it.isNotBlank() }) } - DatePickerDialog( + // Same sheet as the web's loan request: the card shape of the availability box (20dp), a + // serif title, and two buttons stacked at full width in the page's own styles — the filled + // confirm and the outlined cancel — instead of Material's text-button row. + // A plain Dialog, not DatePickerDialog: that one caps its height at 568dp, which cut the + // month in half and pushed the legend and the buttons below the fold. This one is as tall + // as its content and scrolls only on a screen too short for it. + Dialog( onDismissRequest = onDismiss, - confirmButton = { - TextButton( - onClick = { selectedIso?.let(onConfirm) }, - enabled = selectedIso != null && !submitting, - ) { Text(stringResource(R.string.book_loan_confirm)) } - }, - dismissButton = { - TextButton(onClick = onDismiss) { Text(stringResource(R.string.book_loan_cancel)) } - }, + properties = DialogProperties(usePlatformDefaultWidth = false), ) { - Column(Modifier.padding(horizontal = Spacing.lg).fillMaxWidth()) { - Text( - stringResource(R.string.book_loan_dialog_title), - modifier = Modifier.padding(top = Spacing.lg, bottom = Spacing.sm), - style = MaterialTheme.typography.titleMedium, - color = MaterialTheme.colorScheme.onSurface, - ) - when { - loading -> { - Row( - Modifier.fillMaxWidth().padding(vertical = Spacing.xxl), - horizontalArrangement = Arrangement.Center, - verticalAlignment = Alignment.CenterVertically, - ) { - CircularProgressIndicator(modifier = Modifier.size(20.dp), strokeWidth = 2.dp) - Spacer(Modifier.width(Spacing.md)) + Surface( + shape = RoundedCornerShape(20.dp), + color = MaterialTheme.colorScheme.surface, + border = BorderStroke(1.dp, MaterialTheme.colorScheme.outlineVariant), + modifier = Modifier + // 8dp from the screen edges, as the web sheet on a phone (100vw - 1rem). + .padding(horizontal = Spacing.sm, vertical = Spacing.lg) + .widthIn(max = 480.dp) + .fillMaxWidth() + .testTag("loan-sheet"), + ) { + Column( + Modifier + .verticalScroll(rememberScrollState()) + .padding(horizontal = Spacing.lg) + .fillMaxWidth(), + ) { + Text( + stringResource(R.string.book_loan_dialog_title), + modifier = Modifier.padding(top = Spacing.lg, bottom = Spacing.md), + style = MaterialTheme.typography.titleLarge, + color = MaterialTheme.colorScheme.onSurface, + ) + when { + loading -> { + Row( + Modifier.fillMaxWidth().padding(vertical = Spacing.xxl), + horizontalArrangement = Arrangement.Center, + verticalAlignment = Alignment.CenterVertically, + ) { + CircularProgressIndicator(modifier = Modifier.size(20.dp), strokeWidth = 2.dp) + Spacer(Modifier.width(Spacing.md)) + Text( + stringResource(R.string.book_loan_availability_loading), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, + ) + } + } + + calendar != null && !fallback -> { + LoanCalendar( + calendar = calendar, + selected = selectedIso, + onSelect = { selectedIso = it }, + ) + } + + else -> { + // Fallback: plain today-or-future picker. + FallbackDatePicker( + today = today, + selectedIso = selectedIso, + onSelect = { selectedIso = it }, + // Material's picker lays out seven 48dp columns: let it use the sheet's + // side padding too, so the last column is never clipped on a phone. + modifier = Modifier.bleedHorizontally(Spacing.lg), + ) Text( - stringResource(R.string.book_loan_availability_loading), - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, + stringResource(R.string.book_loan_availability_error), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.padding(top = Spacing.sm), ) } } - calendar != null && !fallback -> { - LoanCalendar( - calendar = calendar, - selected = selectedIso, - onSelect = { selectedIso = it }, - ) - } + // Period hint / prompt. + val hint = selectedIso?.let { + stringResource(R.string.book_loan_period_hint, formatDisplayDate(it)) + } ?: stringResource(R.string.book_loan_pick_prompt) + Text( + text = hint, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(vertical = Spacing.md), + ) - else -> { - // Fallback: plain today-or-future picker. - FallbackDatePicker( - today = today, - selectedIso = selectedIso, - onSelect = { selectedIso = it }, + Column( + Modifier.padding(bottom = Spacing.lg), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + PrimaryButton( + label = stringResource(R.string.book_loan_confirm), + onClick = { selectedIso?.let(onConfirm) }, + enabled = selectedIso != null, + loading = submitting, + modifier = Modifier.fillMaxWidth().testTag("loan-confirm"), ) - Text( - stringResource(R.string.book_loan_availability_error), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.error, - modifier = Modifier.padding(top = Spacing.sm), + SecondaryButton( + label = stringResource(R.string.book_loan_cancel), + onClick = onDismiss, + modifier = Modifier.fillMaxWidth().testTag("loan-cancel"), ) } } - - // Period hint / prompt. - val hint = selectedIso?.let { - stringResource(R.string.book_loan_period_hint, formatDisplayDate(it)) - } ?: stringResource(R.string.book_loan_pick_prompt) - Text( - text = hint, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(vertical = Spacing.md), - ) } } } @@ -305,6 +344,7 @@ private fun FallbackDatePicker( today: LocalDate, selectedIso: String?, onSelect: (String) -> Unit, + modifier: Modifier = Modifier, ) { val todayUtcMillis = today.atStartOfDay(ZoneOffset.UTC).toInstant().toEpochMilli() val initialMillis = selectedIso?.let { @@ -319,7 +359,19 @@ private fun FallbackDatePicker( LaunchedEffect(datePickerState.selectedDateMillis) { datePickerState.selectedDateMillis?.let { onSelect(millisToIsoDate(it)) } } - DatePicker(state = datePickerState, title = null, headline = null, showModeToggle = false) + DatePicker(state = datePickerState, modifier = modifier, title = null, headline = null, showModeToggle = false) +} + +/** Measures the content [amount] wider on each side and centres it, over the parent's padding. */ +private fun Modifier.bleedHorizontally(amount: Dp): Modifier = layout { measurable, constraints -> + val extra = amount.roundToPx() * 2 + val wide = constraints.copy( + minWidth = constraints.minWidth + extra, + maxWidth = if (constraints.hasBoundedWidth) constraints.maxWidth + extra else constraints.maxWidth, + ) + val placeable = measurable.measure(wide) + val width = (placeable.width - extra).coerceAtLeast(0) + layout(width, placeable.height) { placeable.place(-extra / 2, 0) } } @OptIn(ExperimentalLayoutApi::class) diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt index d97352e..2678ea7 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt @@ -10,13 +10,14 @@ import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.aspectRatio +import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.fillMaxWidth import androidx.compose.foundation.layout.height import androidx.compose.foundation.layout.padding import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.sizeIn import androidx.compose.foundation.layout.width import androidx.compose.foundation.shape.CircleShape -import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.material.icons.Icons import androidx.compose.material.icons.automirrored.filled.KeyboardArrowLeft import androidx.compose.material.icons.automirrored.filled.KeyboardArrowRight @@ -39,15 +40,13 @@ import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.unit.dp import com.pinakes.app.R import com.pinakes.app.data.model.AvailabilityCalendar -import com.pinakes.app.ui.theme.AvailableContainerDark -import com.pinakes.app.ui.theme.AvailableContainerLight -import com.pinakes.app.ui.theme.AvailableOnContainerDark -import com.pinakes.app.ui.theme.AvailableOnContainerLight import com.pinakes.app.ui.theme.DueSoonContainerDark import com.pinakes.app.ui.theme.DueSoonContainerLight import com.pinakes.app.ui.theme.DueSoonOnContainerDark import com.pinakes.app.ui.theme.DueSoonOnContainerLight +import com.pinakes.app.ui.theme.LocalPinakesColors import com.pinakes.app.ui.theme.Spacing +import com.pinakes.app.ui.theme.mix import java.time.DayOfWeek import java.time.LocalDate import java.time.YearMonth @@ -55,12 +54,14 @@ import java.time.format.TextStyle import java.util.Locale // --------------------------------------------------------------------------- -// Day-cell tints. These reuse the ONLY allowed semantic colours: the availability -// tints from theme/Color.kt (green = free, amber = partial) and the error role -// (red = full). No invented hexes. (DESIGN.md.) +// Day-cell tints, as on the web loan calendar: a free day is a plain dot in the +// ink colour (a coloured month reads as noise), only the days that need a word +// carry a tint — amber = partial (theme/Color.kt), red = full (the error role) — +// each ringed by a faint rule of its own text colour. No invented hexes. +// (DESIGN.md.) // --------------------------------------------------------------------------- -private data class DayTint(val container: Color, val onContainer: Color) +private data class DayTint(val container: Color, val onContainer: Color, val rule: Color = Color.Transparent) /** Per-day availability state used to colour a calendar cell. */ private enum class DayKind { Free, Partial, Full, PastOrUnknown } @@ -68,17 +69,20 @@ private enum class DayKind { Free, Partial, Full, PastOrUnknown } @Composable private fun tintFor(kind: DayKind, dark: Boolean): DayTint = when (kind) { DayKind.Free -> - if (dark) DayTint(AvailableContainerDark, AvailableOnContainerDark) - else DayTint(AvailableContainerLight, AvailableOnContainerLight) + DayTint(Color.Transparent, MaterialTheme.colorScheme.onSurface) DayKind.Partial -> - if (dark) DayTint(DueSoonContainerDark, DueSoonOnContainerDark) - else DayTint(DueSoonContainerLight, DueSoonOnContainerLight) + if (dark) ruled(DueSoonContainerDark, DueSoonOnContainerDark) + else ruled(DueSoonContainerLight, DueSoonOnContainerLight) DayKind.Full -> - DayTint(MaterialTheme.colorScheme.errorContainer, MaterialTheme.colorScheme.onErrorContainer) + ruled(MaterialTheme.colorScheme.errorContainer, MaterialTheme.colorScheme.onErrorContainer) DayKind.PastOrUnknown -> DayTint(Color.Transparent, MaterialTheme.colorScheme.onSurfaceVariant.copy(alpha = 0.38f)) } +/** A tinted day: its rule is its own text colour, faded into the tint. */ +private fun ruled(container: Color, onContainer: Color) = + DayTint(container, onContainer, mix(onContainer, container, .22f)) + /** Index per-day data by ISO date for O(1) lookup, and the set of unavailable dates. */ private class CalendarModel(calendar: AvailabilityCalendar) { private val byDate: Map = @@ -124,7 +128,7 @@ fun LoanCalendar( onSelect: (isoDate: String) -> Unit, modifier: Modifier = Modifier, ) { - val dark = com.pinakes.app.ui.theme.LocalPinakesColors.current.darkMode + val dark = LocalPinakesColors.current.darkMode val today = remember { LocalDate.now() } val model = remember(calendar) { CalendarModel(calendar) } @@ -265,17 +269,25 @@ private fun DayCell( val selectable = model.isSelectable(date, today) val tint = tintFor(kind, dark) - val container = if (selected) MaterialTheme.colorScheme.primary else tint.container - val onContainer = if (selected) MaterialTheme.colorScheme.onPrimary else tint.onContainer + // The pick is the theme's button colour, as the "Request loan" button that opened the sheet. + val tokens = LocalPinakesColors.current + val container = if (selected) tokens.button else tint.container + val onContainer = if (selected) tokens.buttonText else tint.onContainer + val rule = when { + selected -> Color.Transparent + date == today -> MaterialTheme.colorScheme.primary + else -> tint.rule + } + // 40dp where the column allows it, the column itself on the narrowest screens, + // so the seven dots of a row never run into each other. val cellModifier = Modifier - .size(40.dp) + .sizeIn(maxWidth = 40.dp, maxHeight = 40.dp) + .fillMaxSize() .clip(CircleShape) .background(container) .then( - if (date == today && !selected) - Modifier.border(1.dp, MaterialTheme.colorScheme.primary, CircleShape) - else Modifier + if (rule != Color.Transparent) Modifier.border(1.dp, rule, CircleShape) else Modifier ) .then( if (selectable) Modifier.clickable { onSelect(iso) } else Modifier @@ -297,20 +309,23 @@ private fun Legend(dark: Boolean) { Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.spacedBy(Spacing.md), ) { - LegendItem(tintFor(DayKind.Free, dark).container, stringResource(R.string.book_loan_legend_available)) - LegendItem(tintFor(DayKind.Partial, dark).container, stringResource(R.string.book_loan_legend_limited)) - LegendItem(tintFor(DayKind.Full, dark).container, stringResource(R.string.book_loan_legend_unavailable)) + // A free day is an untinted dot, so its key is one too: the surface ringed by the rule. + LegendItem(MaterialTheme.colorScheme.surface, stringResource(R.string.book_loan_legend_available), MaterialTheme.colorScheme.outline) + tintFor(DayKind.Partial, dark).let { LegendItem(it.container, stringResource(R.string.book_loan_legend_limited), it.rule) } + tintFor(DayKind.Full, dark).let { LegendItem(it.container, stringResource(R.string.book_loan_legend_unavailable), it.rule) } } } @Composable -private fun LegendItem(color: Color, label: String) { +private fun LegendItem(color: Color, label: String, rule: Color = Color.Transparent) { Row(verticalAlignment = Alignment.CenterVertically) { + // Dots, like the days they explain. Box( Modifier .size(12.dp) - .clip(RoundedCornerShape(3.dp)) - .background(color), + .clip(CircleShape) + .background(color) + .border(1.dp, rule, CircleShape), ) Spacer(Modifier.width(Spacing.xs)) Text( From 3177b289706dc836c7b8fc2b375352884fc41064 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 13 Oct 2026 13:43:33 +0200 Subject: [PATCH 02/14] Test the loan sheet's seventh column in a real 360dp layout The test forced a 360dp density around LoanRequestSheet, but a Dialog is a window of its own and takes the device density: on a 411dp emulator the days measured 42dp instead of 48dp, so the test only held on an emulator that was already 360dp wide. The sheet's content moves into LoanRequestSheetContent, which the Dialog wraps; the test lays it out in a 360dp Box at the device density. It fails without the bleed (24dp of a 40dp day in view) and passes with it on both a 360dp and a 411dp emulator. --- .../com/pinakes/app/LoanRequestSheetUiTest.kt | 38 ++-- .../app/ui/screens/detail/BookDetailScreen.kt | 197 ++++++++++-------- 2 files changed, 127 insertions(+), 108 deletions(-) diff --git a/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt b/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt index 1a22506..0c0db26 100644 --- a/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt +++ b/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt @@ -1,18 +1,19 @@ package com.pinakes.app -import androidx.compose.runtime.CompositionLocalProvider -import androidx.compose.ui.platform.LocalDensity +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.width +import androidx.compose.ui.Modifier import androidx.compose.ui.semantics.Role import androidx.compose.ui.semantics.SemanticsProperties import androidx.compose.ui.semantics.getOrNull import androidx.compose.ui.test.* import androidx.compose.ui.test.junit4.createComposeRule -import androidx.compose.ui.unit.Density +import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.width -import androidx.test.platform.app.InstrumentationRegistry import com.pinakes.app.data.model.AvailabilityCalendar import com.pinakes.app.data.model.AvailabilityDay import com.pinakes.app.ui.screens.detail.LoanRequestSheet +import com.pinakes.app.ui.screens.detail.LoanRequestSheetContent import com.pinakes.app.ui.theme.PinakesTheme import org.junit.Assert.* import org.junit.Rule @@ -23,7 +24,6 @@ class LoanRequestSheetUiTest { @get:Rule val compose = createComposeRule() private val today = LocalDate.now() - private val context get() = InstrumentationRegistry.getInstrumentation().targetContext /** Six weeks from today: mostly free, a partial day and a full one in the first week. */ private fun calendar(earliest: String? = today.plusDays(1).toString()) = AvailabilityCalendar( @@ -46,12 +46,8 @@ class LoanRequestSheetUiTest { submitting: Boolean = false, onDismiss: () -> Unit = {}, onConfirm: (String) -> Unit = {}, - widthDp: Float? = null, ) = compose.setContent { - // widthDp: lay the sheet out as on a phone that many dp wide, whatever the emulator. - val base = LocalDensity.current - val density = widthDp?.let { Density(context.resources.displayMetrics.widthPixels / it, base.fontScale) } ?: base - CompositionLocalProvider(LocalDensity provides density) { PinakesTheme { + PinakesTheme { LoanRequestSheet( loading = false, calendar = calendar, @@ -60,7 +56,7 @@ class LoanRequestSheetUiTest { onDismiss = onDismiss, onConfirm = onConfirm, ) - } } + } } @Test fun bothButtonsSpanTheSheetAndConfirmSendsTheEarliestDay() { @@ -93,10 +89,16 @@ class LoanRequestSheetUiTest { } @Test fun fallbackPickerKeepsItsSeventhColumnInsideTheSheet() { - // 360dp, the narrowest common phone: Material's seven 48dp columns need the sheet's side padding. - sheet(calendar = null, fallback = true, widthDp = 360f) - // Pixels throughout: the dp helpers of the test rule use the device density, not the forced one. - val sheetRight = compose.onNodeWithTag("loan-sheet").fetchSemanticsNode().boundsInRoot.right + // The sheet's content in a 360dp column, the narrowest common phone, on any emulator: + // the Dialog itself is a window with the device density, so it cannot be narrowed here. + compose.setContent { + PinakesTheme { + Box(Modifier.width(360.dp)) { + LoanRequestSheetContent(loading = false, calendar = null, fallback = true, submitting = false, onDismiss = {}, onConfirm = {}) + } + } + } + val sheet = compose.onNodeWithTag("loan-sheet").getBoundsInRoot() val days = compose.onAllNodes( // Material announces each day as one button whose text is the full date. SemanticsMatcher("a day of the month") { node -> @@ -108,9 +110,9 @@ class LoanRequestSheetUiTest { assertEquals("Every day of the month is laid out", today.lengthOfMonth(), days.size) // boundsInRoot is clipped by the sheet: a cut seventh column shows up as narrow days. // Each day must keep its 40dp circle in view (without the bleed, 24dp of it is left). - val px = context.resources.displayMetrics.widthPixels / 360f - val narrowest = days.minOf { it.boundsInRoot.width } / px + val density = compose.density.density + val narrowest = days.minOf { it.boundsInRoot.width } / density assertTrue("A day is cut by the sheet: ${narrowest}dp of its 40dp circle in view", narrowest >= 39.5f) - assertTrue("A day overflows the sheet", days.maxOf { it.boundsInRoot.right } <= sheetRight + 1f) + assertTrue("A day overflows the sheet", days.maxOf { it.boundsInRoot.right } / density <= sheet.right.value + .5f) } } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 21dde7d..d5190af 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -221,12 +221,6 @@ internal fun LoanRequestSheet( onDismiss: () -> Unit, onConfirm: (isoDate: String) -> Unit, ) { - val today = LocalDate.now() - // Pre-select earliest_available when offered, else null until the user picks. - var selectedIso by remember(calendar) { - mutableStateOf(calendar?.earliestAvailable?.takeIf { it.isNotBlank() }) - } - // Same sheet as the web's loan request: the card shape of the availability box (20dp), a // serif title, and two buttons stacked at full width in the page's own styles — the filled // confirm and the outlined cancel — instead of Material's text-button row. @@ -237,102 +231,125 @@ internal fun LoanRequestSheet( onDismissRequest = onDismiss, properties = DialogProperties(usePlatformDefaultWidth = false), ) { - Surface( - shape = RoundedCornerShape(20.dp), - color = MaterialTheme.colorScheme.surface, - border = BorderStroke(1.dp, MaterialTheme.colorScheme.outlineVariant), - modifier = Modifier - // 8dp from the screen edges, as the web sheet on a phone (100vw - 1rem). - .padding(horizontal = Spacing.sm, vertical = Spacing.lg) - .widthIn(max = 480.dp) - .fillMaxWidth() - .testTag("loan-sheet"), - ) { - Column( - Modifier - .verticalScroll(rememberScrollState()) - .padding(horizontal = Spacing.lg) - .fillMaxWidth(), - ) { - Text( - stringResource(R.string.book_loan_dialog_title), - modifier = Modifier.padding(top = Spacing.lg, bottom = Spacing.md), - style = MaterialTheme.typography.titleLarge, - color = MaterialTheme.colorScheme.onSurface, - ) - when { - loading -> { - Row( - Modifier.fillMaxWidth().padding(vertical = Spacing.xxl), - horizontalArrangement = Arrangement.Center, - verticalAlignment = Alignment.CenterVertically, - ) { - CircularProgressIndicator(modifier = Modifier.size(20.dp), strokeWidth = 2.dp) - Spacer(Modifier.width(Spacing.md)) - Text( - stringResource(R.string.book_loan_availability_loading), - style = MaterialTheme.typography.bodyMedium, - color = MaterialTheme.colorScheme.onSurfaceVariant, - ) - } - } + LoanRequestSheetContent(loading, calendar, fallback, submitting, onDismiss, onConfirm) + } +} - calendar != null && !fallback -> { - LoanCalendar( - calendar = calendar, - selected = selectedIso, - onSelect = { selectedIso = it }, - ) - } +/** + * The sheet itself, without its [Dialog]: a Dialog is a window of its own and takes the device + * density, so the tests lay this out in a fixed width to check a narrow phone on any emulator. + */ +@OptIn(ExperimentalMaterial3Api::class) +@Composable +internal fun LoanRequestSheetContent( + loading: Boolean, + calendar: AvailabilityCalendar?, + fallback: Boolean, + submitting: Boolean, + onDismiss: () -> Unit, + onConfirm: (isoDate: String) -> Unit, +) { + val today = LocalDate.now() + // Pre-select earliest_available when offered, else null until the user picks. + var selectedIso by remember(calendar) { + mutableStateOf(calendar?.earliestAvailable?.takeIf { it.isNotBlank() }) + } - else -> { - // Fallback: plain today-or-future picker. - FallbackDatePicker( - today = today, - selectedIso = selectedIso, - onSelect = { selectedIso = it }, - // Material's picker lays out seven 48dp columns: let it use the sheet's - // side padding too, so the last column is never clipped on a phone. - modifier = Modifier.bleedHorizontally(Spacing.lg), - ) + Surface( + shape = RoundedCornerShape(20.dp), + color = MaterialTheme.colorScheme.surface, + border = BorderStroke(1.dp, MaterialTheme.colorScheme.outlineVariant), + modifier = Modifier + // 8dp from the screen edges, as the web sheet on a phone (100vw - 1rem). + .padding(horizontal = Spacing.sm, vertical = Spacing.lg) + .widthIn(max = 480.dp) + .fillMaxWidth() + .testTag("loan-sheet"), + ) { + Column( + Modifier + .verticalScroll(rememberScrollState()) + .padding(horizontal = Spacing.lg) + .fillMaxWidth(), + ) { + Text( + stringResource(R.string.book_loan_dialog_title), + modifier = Modifier.padding(top = Spacing.lg, bottom = Spacing.md), + style = MaterialTheme.typography.titleLarge, + color = MaterialTheme.colorScheme.onSurface, + ) + when { + loading -> { + Row( + Modifier.fillMaxWidth().padding(vertical = Spacing.xxl), + horizontalArrangement = Arrangement.Center, + verticalAlignment = Alignment.CenterVertically, + ) { + CircularProgressIndicator(modifier = Modifier.size(20.dp), strokeWidth = 2.dp) + Spacer(Modifier.width(Spacing.md)) Text( - stringResource(R.string.book_loan_availability_error), - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.error, - modifier = Modifier.padding(top = Spacing.sm), + stringResource(R.string.book_loan_availability_loading), + style = MaterialTheme.typography.bodyMedium, + color = MaterialTheme.colorScheme.onSurfaceVariant, ) } } - // Period hint / prompt. - val hint = selectedIso?.let { - stringResource(R.string.book_loan_period_hint, formatDisplayDate(it)) - } ?: stringResource(R.string.book_loan_pick_prompt) - Text( - text = hint, - style = MaterialTheme.typography.bodySmall, - color = MaterialTheme.colorScheme.onSurfaceVariant, - modifier = Modifier.padding(vertical = Spacing.md), - ) + calendar != null && !fallback -> { + LoanCalendar( + calendar = calendar, + selected = selectedIso, + onSelect = { selectedIso = it }, + ) + } - Column( - Modifier.padding(bottom = Spacing.lg), - verticalArrangement = Arrangement.spacedBy(12.dp), - ) { - PrimaryButton( - label = stringResource(R.string.book_loan_confirm), - onClick = { selectedIso?.let(onConfirm) }, - enabled = selectedIso != null, - loading = submitting, - modifier = Modifier.fillMaxWidth().testTag("loan-confirm"), + else -> { + // Fallback: plain today-or-future picker. + FallbackDatePicker( + today = today, + selectedIso = selectedIso, + onSelect = { selectedIso = it }, + // Material's picker lays out seven 48dp columns: let it use the sheet's + // side padding too, so the last column is never clipped on a phone. + modifier = Modifier.bleedHorizontally(Spacing.lg), ) - SecondaryButton( - label = stringResource(R.string.book_loan_cancel), - onClick = onDismiss, - modifier = Modifier.fillMaxWidth().testTag("loan-cancel"), + Text( + stringResource(R.string.book_loan_availability_error), + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.error, + modifier = Modifier.padding(top = Spacing.sm), ) } } + + // Period hint / prompt. + val hint = selectedIso?.let { + stringResource(R.string.book_loan_period_hint, formatDisplayDate(it)) + } ?: stringResource(R.string.book_loan_pick_prompt) + Text( + text = hint, + style = MaterialTheme.typography.bodySmall, + color = MaterialTheme.colorScheme.onSurfaceVariant, + modifier = Modifier.padding(vertical = Spacing.md), + ) + + Column( + Modifier.padding(bottom = Spacing.lg), + verticalArrangement = Arrangement.spacedBy(12.dp), + ) { + PrimaryButton( + label = stringResource(R.string.book_loan_confirm), + onClick = { selectedIso?.let(onConfirm) }, + enabled = selectedIso != null, + loading = submitting, + modifier = Modifier.fillMaxWidth().testTag("loan-confirm"), + ) + SecondaryButton( + label = stringResource(R.string.book_loan_cancel), + onClick = onDismiss, + modifier = Modifier.fillMaxWidth().testTag("loan-cancel"), + ) + } } } } From a5fd001f2164759a07ffa02ce0707305b0dd57a8 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 15 Oct 2026 00:39:00 +0200 Subject: [PATCH 03/14] Receive a wanted title with a clear action, then open its new copy On a wanted title the trailing "Ricevuto" (Received) read as a status, so a title still wanted with no copies looked already received, and one tap added a copy with no question. The button now says "Ricevi" (Receive) and asks before adding the copy. Receiving a wanted title or an offer creates a copy that still needs its code and place. The app now opens the record, scrolls to the copies and opens that copy's editor straight away, through an optional copy argument on the book editor route. --- .../assets/fixtures/desiderata_wanted.json | 1 + .../desiderata_wanted_77_received.json | 1 + .../fixtures/staff_book_5_received.json | 1 + .../fixtures/staff_book_77_received.json | 1 + .../app/staffe2e/DesiderataStaffFlowTest.kt | 44 +++++++++++++++-- .../app/ui/navigation/PinakesNavHost.kt | 11 ++++- .../com/pinakes/app/ui/navigation/Routes.kt | 7 ++- .../ui/screens/staff/StaffBookEditScreen.kt | 2 + .../screens/staff/StaffBookEditViewModel.kt | 8 ++++ .../staff/content/StaffCopiesSection.kt | 18 ++++++- .../staff/plugins/StaffDesiderataScreen.kt | 47 ++++++++++++++++--- i18n/de.json | 4 +- i18n/en.json | 4 +- i18n/fr.json | 4 +- i18n/it.json | 4 +- 15 files changed, 136 insertions(+), 21 deletions(-) create mode 100644 app/src/androidTest/assets/fixtures/desiderata_wanted.json create mode 100644 app/src/androidTest/assets/fixtures/desiderata_wanted_77_received.json create mode 100644 app/src/androidTest/assets/fixtures/staff_book_5_received.json create mode 100644 app/src/androidTest/assets/fixtures/staff_book_77_received.json diff --git a/app/src/androidTest/assets/fixtures/desiderata_wanted.json b/app/src/androidTest/assets/fixtures/desiderata_wanted.json new file mode 100644 index 0000000..d706fd4 --- /dev/null +++ b/app/src/androidTest/assets/fixtures/desiderata_wanted.json @@ -0,0 +1 @@ +{"data":[{"id":77,"title":"Non diciamo no da sole","author":"Sara Ahmed","publisher":"Fandango Libri","isbn":"9781256361909","cover_url":null,"open_offers":0}],"meta":{"next_cursor":null},"error":null} diff --git a/app/src/androidTest/assets/fixtures/desiderata_wanted_77_received.json b/app/src/androidTest/assets/fixtures/desiderata_wanted_77_received.json new file mode 100644 index 0000000..7f4da41 --- /dev/null +++ b/app/src/androidTest/assets/fixtures/desiderata_wanted_77_received.json @@ -0,0 +1 @@ +{"data":{"book_id":77,"copy_id":901,"offer_id":55},"meta":{},"error":null} diff --git a/app/src/androidTest/assets/fixtures/staff_book_5_received.json b/app/src/androidTest/assets/fixtures/staff_book_5_received.json new file mode 100644 index 0000000..749dded --- /dev/null +++ b/app/src/androidTest/assets/fixtures/staff_book_5_received.json @@ -0,0 +1 @@ +{"data": {"id": 5, "title": "Orlando", "subtitle": "A Biography", "isbn13": "9780156701600", "year": 1928, "pages": 333, "language": "English", "media_type": "libro", "copies_total": 4, "copies_available": 1, "genre": {"id": 11, "name": "Romanzo"}, "authors": [{"id": 1, "name": "Virginia Woolf", "role": "principale"}, {"id": 7, "name": "Nadia Fusini", "role": "traduttore"}], "publishers": [{"id": 4, "name": "Hogarth"}], "location": {"shelf_id": 7, "shelf_code": "A", "level_id": 70, "level_number": 1, "position": 3, "label": "A.1.3"}, "copies": [{"id": 11, "inventory_number": "INV-0011", "status": "disponibile", "notes": null}, {"id": 12, "inventory_number": "INV-0012", "status": "prestato", "notes": "Spine worn"}, {"id": 77, "inventory_number": "LIB-5-C77", "status": "disponibile", "notes": null}, {"id": 78, "inventory_number": "LIB-5-C78", "status": "disponibile", "notes": null}], "is_wanted": false}, "meta": {}, "error": null} \ No newline at end of file diff --git a/app/src/androidTest/assets/fixtures/staff_book_77_received.json b/app/src/androidTest/assets/fixtures/staff_book_77_received.json new file mode 100644 index 0000000..465cc3c --- /dev/null +++ b/app/src/androidTest/assets/fixtures/staff_book_77_received.json @@ -0,0 +1 @@ +{"data": {"id": 77, "title": "Non diciamo no da sole", "subtitle": "A Biography", "isbn13": "9780156701600", "year": 1928, "pages": 333, "language": "English", "media_type": "libro", "copies_total": 3, "copies_available": 1, "genre": {"id": 11, "name": "Romanzo"}, "authors": [{"id": 1, "name": "Virginia Woolf", "role": "principale"}, {"id": 7, "name": "Nadia Fusini", "role": "traduttore"}], "publishers": [{"id": 4, "name": "Hogarth"}], "location": {"shelf_id": 7, "shelf_code": "A", "level_id": 70, "level_number": 1, "position": 3, "label": "A.1.3"}, "copies": [{"id": 11, "inventory_number": "INV-0011", "status": "disponibile", "notes": null}, {"id": 12, "inventory_number": "INV-0012", "status": "prestato", "notes": "Spine worn"}, {"id": 901, "inventory_number": "LIB-77-C901", "status": "disponibile", "notes": null}], "is_wanted": false}, "meta": {}, "error": null} \ No newline at end of file diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt index d9b92b5..d2cfffd 100644 --- a/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt @@ -1,7 +1,12 @@ package com.pinakes.app.staffe2e import com.pinakes.app.R +import androidx.compose.ui.test.hasTestTag +import androidx.compose.ui.test.onAllNodesWithText +import androidx.compose.ui.test.onLast +import androidx.compose.ui.test.performClick import org.junit.Assert.assertEquals +import org.junit.Assert.assertTrue import org.junit.Test /** Hub → Donation offers: receive an offer for a wanted title, receive one into a picked record, a closed offer. */ @@ -18,21 +23,52 @@ class DesiderataStaffFlowTest : AppFlowTest() { } @Test fun receiveAnOfferForAWantedTitle() { - open { json("POST staff/desiderata/offers/41/receive", "offer_41_received") } + open { + json("POST staff/desiderata/offers/41/receive", "offer_41_received") + json("GET staff/books/5", "staff_book_5_received") + } clickTag("offer-41") clickTag("offer-receive") assertEquals("{}", lastBody("POST", "staff/desiderata/offers/41/receive")) - waitText(text(R.string.staff_offer_received)) + // The new copy opens on the record, ready for its code and place. + waitFor(hasTestTag("copy-save")) } @Test fun anOfferWithoutATitleIsReceivedIntoAPickedRecord() { - open { json("POST staff/desiderata/offers/42/receive", "offer_42_received") } + open { + json("POST staff/desiderata/offers/42/receive", "offer_42_received") + json("GET staff/books/5", "staff_book_5_received") + } clickTag("offer-42") clickTag("offer-receive") waitText(text(R.string.staff_offer_receive_into)) clickTag("pick-5") assertEquals("""{"received_book_id":5}""", lastBody("POST", "staff/desiderata/offers/42/receive")) - waitText(text(R.string.staff_offer_received)) + waitFor(hasTestTag("copy-save")) + } + + @Test fun aWantedTitleIsReceivedOnlyAfterConfirming() { + open { + json("GET staff/desiderata/wanted", "desiderata_wanted") + json("POST staff/desiderata/wanted/77/receive", "desiderata_wanted_77_received", 201) + json("GET staff/books/77", "staff_book_77_received") + } + click(text(R.string.staff_offers_wanted)) + waitText("Non diciamo no da sole") + // The row's button is an action, not a status: a tap only asks. + click(text(R.string.staff_wanted_mark_received)) + waitText(text(R.string.staff_wanted_receive_confirm_title)) + assertTrue(api.recorded("POST", "staff/desiderata/wanted/77/receive").isEmpty()) + click(text(R.string.action_cancel)) + assertTrue(api.recorded("POST", "staff/desiderata/wanted/77/receive").isEmpty()) + click(text(R.string.staff_wanted_mark_received)) + waitText(text(R.string.staff_wanted_receive_confirm_title)) + compose.onAllNodesWithText(text(R.string.staff_wanted_mark_received)).onLast().performClick() + waitRequest("POST", "staff/desiderata/wanted/77/receive") + assertEquals(1, api.recorded("POST", "staff/desiderata/wanted/77/receive").size) + // Received: the record opens on the new copy's editor, to give it its code and place. + waitFor(hasTestTag("copy-save")) + waitText("LIB-77-C901") } @Test fun aClosedOfferSaysSo() { diff --git a/app/src/main/java/com/pinakes/app/ui/navigation/PinakesNavHost.kt b/app/src/main/java/com/pinakes/app/ui/navigation/PinakesNavHost.kt index ddf9d5f..d37fca1 100644 --- a/app/src/main/java/com/pinakes/app/ui/navigation/PinakesNavHost.kt +++ b/app/src/main/java/com/pinakes/app/ui/navigation/PinakesNavHost.kt @@ -316,7 +316,11 @@ fun PinakesNavHost(navController: NavHostController = rememberNavController()) { } // ---- Staff plugin content (Mobile API 1.7.0) ---- composable(Routes.STAFF_DESIDERATA, enterTransition = slideIn, popExitTransition = slideOut) { - StaffDesiderataScreen(onNavigateUp = { navController.popBackStack() }, onOpenBook = { navController.navigate(Routes.staffBookEdit(it)) }) + StaffDesiderataScreen( + onNavigateUp = { navController.popBackStack() }, + onOpenBook = { navController.navigate(Routes.staffBookEdit(it)) }, + onOpenCopy = { book, copy -> navController.navigate(Routes.staffBookEdit(book, copy)) }, + ) } composable(Routes.STAFF_ARCHIVES, enterTransition = slideIn, popExitTransition = slideOut) { StaffArchivesScreen( @@ -415,7 +419,10 @@ fun PinakesNavHost(navController: NavHostController = rememberNavController()) { } composable( Routes.STAFF_BOOK_EDIT, - arguments = listOf(navArgument(Routes.ARG_BOOK_ID) { type = NavType.IntType }), + arguments = listOf( + navArgument(Routes.ARG_BOOK_ID) { type = NavType.IntType }, + navArgument(Routes.ARG_COPY_ID) { type = NavType.IntType; defaultValue = 0 }, + ), enterTransition = slideIn, popExitTransition = slideOut, ) { StaffBookEditScreen( diff --git a/app/src/main/java/com/pinakes/app/ui/navigation/Routes.kt b/app/src/main/java/com/pinakes/app/ui/navigation/Routes.kt index 18d045c..34f627d 100644 --- a/app/src/main/java/com/pinakes/app/ui/navigation/Routes.kt +++ b/app/src/main/java/com/pinakes/app/ui/navigation/Routes.kt @@ -64,8 +64,11 @@ object Routes { const val ARG_ISBN = "isbn" const val STAFF_BOOK_NEW = "staff/books/new?isbn={isbn}" fun staffBookNew(isbn: String = ""): String = "staff/books/new?isbn=${Uri.encode(isbn)}" - const val STAFF_BOOK_EDIT = "staff/books/{bookId}/edit" - fun staffBookEdit(bookId: Int): String = "staff/books/$bookId/edit" + /** Optional: the copy whose editor the book page opens straight away (a copy just received). */ + const val ARG_COPY_ID = "copyId" + const val STAFF_BOOK_EDIT = "staff/books/{bookId}/edit?copy={copyId}" + fun staffBookEdit(bookId: Int, copyId: Int? = null): String = + "staff/books/$bookId/edit" + (copyId?.takeIf { it > 0 }?.let { "?copy=$it" } ?: "") const val STAFF_AUTHORS = "staff/authors" const val STAFF_PUBLISHERS = "staff/publishers" const val ARG_RECORD_ID = "recordId" diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditScreen.kt index 816ddd3..e33a4ba 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditScreen.kt @@ -306,6 +306,8 @@ private fun BookForm(state: StaffBookEditUiState, vm: StaffBookEditViewModel) { onBookRefreshed = vm::applyCopies, onMessage = vm::showMessage, onForbidden = vm::markForbidden, + openCopyId = vm.openCopyId, + onCopyOpened = vm::consumeOpenCopy, ) // Digital Library plugin (`staff_digital_library`): eBook / audiobook upload. com.pinakes.app.ui.screens.staff.content.StaffDigitalSection(onMessage = vm::showMessage, onForbidden = vm::markForbidden) diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt index 6df3fa2..f441eec 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt @@ -121,6 +121,14 @@ class StaffBookEditViewModel @Inject constructor( private var bookId: Int = savedStateHandle.get(Routes.ARG_BOOK_ID) ?: 0 private val isbnArg: String = savedStateHandle.get(Routes.ARG_ISBN).orEmpty() + /** The copy to open on arrival (a copy just received, waiting for its code and place); 0 = none. */ + var openCopyId: Int = savedStateHandle.get(Routes.ARG_COPY_ID) ?: 0 + private set + + fun consumeOpenCopy() { + openCopyId = 0 + } + private val _state = MutableStateFlow(StaffBookEditUiState(bookId = bookId)) val state: StateFlow = _state.asStateFlow() diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt index dd3a9d4..69ba810 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt @@ -34,6 +34,10 @@ import androidx.compose.runtime.saveable.rememberSaveable import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier +import androidx.compose.foundation.ExperimentalFoundationApi +import androidx.compose.foundation.relocation.BringIntoViewRequester +import androidx.compose.foundation.relocation.bringIntoViewRequester +import androidx.compose.runtime.remember import androidx.compose.ui.platform.testTag import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.input.KeyboardType @@ -219,7 +223,7 @@ internal fun copyStateLabel(copy: StaffCopy, lookups: StaffLookups): String = * (`staff_copies`); label buttons only with `staff_labels`. Messages go to [onMessage] (the * editor's snackbar); counter changes to [onBookRefreshed]. */ -@OptIn(ExperimentalLayoutApi::class) +@OptIn(ExperimentalLayoutApi::class, ExperimentalFoundationApi::class) @Composable fun StaffCopiesSection( copies: List, @@ -228,6 +232,8 @@ fun StaffCopiesSection( onBookRefreshed: (StaffBook) -> Unit, onMessage: (String) -> Unit, onForbidden: () -> Unit, + openCopyId: Int = 0, + onCopyOpened: () -> Unit = {}, ) { val app: AppViewModel = hiltViewModel() val areas by app.staffAreas.collectAsStateWithLifecycle() @@ -238,11 +244,19 @@ fun StaffCopiesSection( // The editor's "Add copies" changed the total: read the copies again. LaunchedEffect(copiesTotal) { if (copiesTotal != state.copies.size) vm.reload() } LaunchedEffect(state.refreshed) { state.refreshed?.let { onBookRefreshed(it); vm.consumeRefreshed() } } + // Arriving from a receipt: scroll to the copies and open the new copy's editor at once. + val bringIntoView = remember { BringIntoViewRequester() } + LaunchedEffect(openCopyId, state.copies) { + val copy = state.copies.firstOrNull { it.id == openCopyId } ?: return@LaunchedEffect + onCopyOpened() + bringIntoView.bringIntoView() + vm.startEdit(copy) + } LaunchedEffect(state.forbidden) { if (state.forbidden) onForbidden() } val message = state.message?.resolve() LaunchedEffect(state.message) { if (!message.isNullOrBlank()) { onMessage(message); vm.consumeMessage() } } - Column(verticalArrangement = Arrangement.spacedBy(Spacing.xs), modifier = Modifier.testTag("copies-section")) { + Column(verticalArrangement = Arrangement.spacedBy(Spacing.xs), modifier = Modifier.bringIntoViewRequester(bringIntoView).testTag("copies-section")) { if (areas.copies) { state.copies.forEach { copy -> Surface( diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt index f712222..39eeb89 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt @@ -107,6 +107,10 @@ data class DesiderataUiState( val forbidden: Boolean = false, val selected: StaffOffer? = null, val confirmDelete: StaffOffer? = null, + /** A wanted title waiting for "mark received" to be confirmed: it adds a copy at once. */ + val confirmReceive: StaffWantedBook? = null, + /** A receipt created this copy of this record: the screen opens its editor (code, place). */ + val openCopy: Pair? = null, val pickingBookFor: StaffOffer? = null, val busy: Boolean = false, val message: StaffMessage? = null, @@ -238,7 +242,7 @@ class StaffDesiderataViewModel @Inject constructor( val res = call(offer) _state.update { when (res) { - is ApiResult.Success -> it.copy(busy = false, selected = null, message = StaffMessage(success)) + is ApiResult.Success -> it.copy(busy = false, selected = null, message = StaffMessage(success), openCopy = receivedCopy(res.data)) is ApiResult.Failure -> it.copy(busy = false, forbidden = res.isForbidden(), message = res.toStaffMessage(R.string.staff_error_save)) } } @@ -264,15 +268,22 @@ class StaffDesiderataViewModel @Inject constructor( } } - /** "Received" on a wanted title: the copy arrived outside an offer (bought, given at the desk). */ - fun receiveWanted(book: StaffWantedBook) { + fun askReceiveWanted(book: StaffWantedBook) = _state.update { it.copy(confirmReceive = book) } + fun dismissReceiveWanted() = _state.update { it.copy(confirmReceive = null) } + + /** "Mark received" on a wanted title, once confirmed: the copy arrived outside an offer (bought, given at the desk). */ + fun receiveWanted() { + val book = _state.value.confirmReceive ?: return if (_state.value.busy) return - _state.update { it.copy(busy = true) } + _state.update { it.copy(busy = true, confirmReceive = null) } viewModelScope.launch { val res = plugins.receiveWanted(book.id) _state.update { when (res) { - is ApiResult.Success -> it.copy(busy = false, message = StaffMessage(R.string.staff_wanted_received, args = listOf(book.title))) + is ApiResult.Success -> it.copy( + busy = false, message = StaffMessage(R.string.staff_wanted_received, args = listOf(book.title)), + openCopy = res.data.copyId?.takeIf { c -> c > 0 }?.let { c -> res.data.bookId to c }, + ) is ApiResult.Failure -> it.copy( busy = false, forbidden = res.isForbidden(), message = if (res.meta?.reason == "open_offers") StaffMessage(R.string.staff_wanted_open_offers) else res.toStaffMessage(R.string.staff_error_save), @@ -284,6 +295,16 @@ class StaffDesiderataViewModel @Inject constructor( } fun consumeMessage() = _state.update { it.copy(message = null) } + + fun consumeOpenCopy() = _state.update { it.copy(openCopy = null) } + + /** The record and copy a received offer produced, when the server names both. */ + private fun receivedCopy(offer: StaffOffer): Pair? { + if (offer.status != "received") return null + val book = offer.receivedBook?.id ?: offer.book?.id ?: return null + val copy = offer.copyId?.takeIf { it > 0 } ?: return null + return book to copy + } } internal fun offerStatusLabel(status: String): Int? = when (status) { @@ -297,7 +318,11 @@ internal fun offerStatusLabel(status: String): Int? = when (status) { /** Donation offers (accept, reject, receive into the catalogue, delete) and the wanted titles. */ @OptIn(ExperimentalMaterial3Api::class) @Composable -fun StaffDesiderataScreen(onNavigateUp: () -> Unit, onOpenBook: (Int) -> Unit) { +fun StaffDesiderataScreen( + onNavigateUp: () -> Unit, + onOpenBook: (Int) -> Unit, + onOpenCopy: (bookId: Int, copyId: Int) -> Unit = { book, _ -> onOpenBook(book) }, +) { val vm: StaffDesiderataViewModel = hiltViewModel() val state by vm.state.collectAsStateWithLifecycle() val app: AppViewModel = hiltViewModel() @@ -306,6 +331,8 @@ fun StaffDesiderataScreen(onNavigateUp: () -> Unit, onOpenBook: (Int) -> Unit) { ForbiddenExit(state.forbidden, onNavigateUp) LeaveWhen(!areas.desiderata, onNavigateUp) StaffSnackbarEffect(state.message, snackbar, vm::consumeMessage) + // A receipt made a copy: go straight to it, to give it its code and place. + LaunchedEffect(state.openCopy) { state.openCopy?.let { (book, copy) -> vm.consumeOpenCopy(); onOpenCopy(book, copy) } } Scaffold( topBar = { PinakesTopBar(title = stringResource(R.string.staff_desiderata), onNavigateUp = onNavigateUp) }, @@ -333,7 +360,7 @@ fun StaffDesiderataScreen(onNavigateUp: () -> Unit, onOpenBook: (Int) -> Unit) { title = w.title, coverUrl = w.coverUrl, line1 = w.author, line2 = stringResource(R.string.staff_wanted_open_offers_count, w.openOffers), onClick = { onOpenBook(w.id) }, - trailing = { PinakesTextButton(stringResource(R.string.staff_wanted_mark_received), onClick = { vm.receiveWanted(w) }, enabled = !state.busy) }, + trailing = { PinakesTextButton(stringResource(R.string.staff_wanted_mark_received), onClick = { vm.askReceiveWanted(w) }, enabled = !state.busy) }, modifier = Modifier.padding(vertical = Spacing.xs).testTag("wanted-${w.id}"), ) } @@ -368,6 +395,12 @@ fun StaffDesiderataScreen(onNavigateUp: () -> Unit, onOpenBook: (Int) -> Unit) { } state.selected?.let { offer -> OfferSheet(offer, state.busy, vm, onOpenBook) } + state.confirmReceive?.let { book -> + ConfirmDialog( + title = stringResource(R.string.staff_wanted_receive_confirm_title), body = stringResource(R.string.staff_wanted_receive_confirm_body, book.title), + confirmLabel = stringResource(R.string.staff_wanted_mark_received), onConfirm = vm::receiveWanted, onDismiss = vm::dismissReceiveWanted, + ) + } state.confirmDelete?.let { offer -> ConfirmDialog( title = stringResource(R.string.staff_offer_delete), body = stringResource(R.string.staff_offer_delete_body, offer.title), diff --git a/i18n/de.json b/i18n/de.json index da7c81b..5de45c8 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -1210,7 +1210,9 @@ "staff_offer_delete_body": "Das Angebot für „%1$s“ löschen? Das lässt sich nicht rückgängig machen.", "staff_offer_deleted": "Angebot gelöscht", "staff_wanted_open_offers_count": "Offene Angebote: %1$d", - "staff_wanted_mark_received": "Erhalten", + "staff_wanted_mark_received": "Empfangen", + "staff_wanted_receive_confirm_title": "Als erhalten markieren?", + "staff_wanted_receive_confirm_body": "„%1$s“ erhält ein Exemplar im Katalog und verschwindet aus den gesuchten Titeln.", "staff_wanted_received": "„%1$s“ erhalten: Exemplar hinzugefügt", "staff_wanted_open_offers": "Für diesen Titel gibt es offene Angebote: über das Angebot erfassen.", "staff_archives_search": "Titel oder Signatur", diff --git a/i18n/en.json b/i18n/en.json index 24a1131..8ace25f 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1210,7 +1210,9 @@ "staff_offer_delete_body": "Delete the offer of “%1$s”? This cannot be undone.", "staff_offer_deleted": "Offer deleted", "staff_wanted_open_offers_count": "Open offers: %1$d", - "staff_wanted_mark_received": "Received", + "staff_wanted_mark_received": "Receive", + "staff_wanted_receive_confirm_title": "Mark as received?", + "staff_wanted_receive_confirm_body": "“%1$s” gets a copy in the catalogue and leaves the wanted titles.", "staff_wanted_received": "“%1$s” received: copy added", "staff_wanted_open_offers": "This title has open offers: receive it from the offer instead.", "staff_archives_search": "Title or reference code", diff --git a/i18n/fr.json b/i18n/fr.json index e7a4498..5cd7c25 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -1210,7 +1210,9 @@ "staff_offer_delete_body": "Supprimer la proposition « %1$s » ? C’est irréversible.", "staff_offer_deleted": "Proposition supprimée", "staff_wanted_open_offers_count": "Propositions ouvertes : %1$d", - "staff_wanted_mark_received": "Reçu", + "staff_wanted_mark_received": "Recevoir", + "staff_wanted_receive_confirm_title": "Marquer comme reçu ?", + "staff_wanted_receive_confirm_body": "« %1$s » reçoit un exemplaire dans le catalogue et quitte les titres recherchés.", "staff_wanted_received": "« %1$s » reçu : exemplaire ajouté", "staff_wanted_open_offers": "Ce titre a des propositions ouvertes : recevez-le depuis la proposition.", "staff_archives_search": "Titre ou cote", diff --git a/i18n/it.json b/i18n/it.json index 8c3191f..7090e00 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -1210,7 +1210,9 @@ "staff_offer_delete_body": "Eliminare la proposta per «%1$s»? Non si può annullare.", "staff_offer_deleted": "Proposta eliminata", "staff_wanted_open_offers_count": "Proposte aperte: %1$d", - "staff_wanted_mark_received": "Ricevuto", + "staff_wanted_mark_received": "Ricevi", + "staff_wanted_receive_confirm_title": "Segnare come ricevuto?", + "staff_wanted_receive_confirm_body": "«%1$s» riceve una copia nel catalogo ed esce dai titoli cercati.", "staff_wanted_received": "«%1$s» ricevuto: copia aggiunta", "staff_wanted_open_offers": "Questo titolo ha proposte aperte: ricevilo dalla proposta.", "staff_archives_search": "Titolo o segnatura", From 0177791c29b652bae194bf8e3690522313e65063 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Thu, 15 Oct 2026 12:32:20 +0200 Subject: [PATCH 04/14] Show the digital files a reader cannot open yet, with the reason 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. --- .../java/com/pinakes/app/data/model/Models.kt | 38 +++++++++++++++- .../app/ui/screens/detail/BookDetailScreen.kt | 33 +++++++++++++- .../pinakes/app/DigitalAvailabilityTest.kt | 43 +++++++++++++++++++ i18n/de.json | 6 ++- i18n/en.json | 6 ++- i18n/fr.json | 6 ++- i18n/it.json | 6 ++- 7 files changed, 130 insertions(+), 8 deletions(-) diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index cc21cc9..f36f65e 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -347,17 +347,51 @@ data class BookDetail( audioUrl?.takeIf { hasAudio && it.isNotBlank() }?.let { DigitalAttachment(it, kind = "audio") }, ebookUrl?.takeIf { hasEbook && it.isNotBlank() }?.let { DigitalAttachment(it, kind = "ebook") }, ) - }.filter { it.safeUrl != null }.distinctBy { it.url } + }.filter { it.safeUrl != null || it.locked }.let { files -> + // A closed file has no address (the server sends url = null): several of them are + // all "", so only the open ones are told apart by their address. + val seen = HashSet() + files.filter { it.locked || seen.add(it.url) } + } } +/** + * One digital file of a record. Servers from Pinakes 0.8.6 (issue #480) say who may open + * it and what this reader may do: a closed file keeps its entry with an empty [url] and + * the reason in [access]. Older servers send neither field, and every file is open. + */ @Serializable -data class DigitalAttachment(val url: String = "", val label: String = "", val kind: String = "ebook") { +data class DigitalAttachment( + val url: String = "", + val label: String = "", + val kind: String = "ebook", + /** public | users | staff | loan, as resolved by the server; null on older servers. */ + val visibility: String? = null, + /** granted | login_required | loan_required | staff_only; null on older servers (= granted). */ + val access: String? = null, +) { + val locked: Boolean get() = access != null && access != ACCESS_GRANTED val safeUrl: String? get() = url.takeIf { val uri = runCatching { java.net.URI(it) }.getOrNull() uri?.scheme?.lowercase() in listOf("http", "https") && !uri?.host.isNullOrBlank() } + + /** When a signed link stops working (its pk_exp, in epoch seconds); null for an unsigned link. */ + val expiresAt: Long? get() = runCatching { java.net.URI(url).rawQuery }.getOrNull() + ?.split('&')?.firstOrNull { it.startsWith("pk_exp=") }?.substringAfter('=')?.toLongOrNull() + + /** 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 + val format: String get() = runCatching { java.net.URI(url).path.substringAfterLast('.').uppercase() }.getOrDefault("") val isPdf: Boolean get() = format == "PDF" + + companion object { + const val ACCESS_GRANTED = "granted" + const val ACCESS_LOGIN_REQUIRED = "login_required" + const val ACCESS_LOAN_REQUIRED = "loan_required" + } } @Serializable diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 6661e66..05f8a30 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -40,6 +40,7 @@ import androidx.compose.material.icons.outlined.FavoriteBorder import androidx.compose.material.icons.outlined.HourglassEmpty import androidx.compose.material.icons.outlined.Schedule import androidx.compose.material.icons.outlined.LocalOffer +import androidx.compose.material.icons.outlined.Lock import androidx.compose.material3.CircularProgressIndicator import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon @@ -76,6 +77,7 @@ import com.pinakes.app.R import com.pinakes.app.data.model.AvailabilityCalendar import com.pinakes.app.data.model.BookDetail import com.pinakes.app.data.model.CirculationRequestKind +import com.pinakes.app.data.model.DigitalAttachment import com.pinakes.app.data.model.PersonalHistory import com.pinakes.app.ui.common.AppViewModel import com.pinakes.app.ui.common.OfflinePolicy @@ -200,6 +202,7 @@ fun BookDetailScreen( onReserve = vm::openLoanSheet, onToggleWishlist = vm::toggleWishlist, onFindWorks = onFindWorks, + onRefreshLinks = vm::load, onShowMessage = { msg -> scope.launch { snackbarHost.showSnackbar(msg) } }, offlineNotice = if (state.offlineNow) stringResource(R.string.offline_actions_unavailable) else null, offlineSyncedAt = state.syncedAt, @@ -338,6 +341,8 @@ internal fun DetailContent( onToggleWishlist: () -> Unit, onShowMessage: (String) -> Unit, onFindWorks: (String, Int?) -> Unit = { _, _ -> }, + /** Fetches the record again: its signed file links last a day (issue #480). */ + onRefreshLinks: () -> Unit = {}, /** Set when the page is built from the offline cache: shown above the cover. */ offlineNotice: String? = null, /** Last sync time of the offline copy: availability is shown as "last known", never current. */ @@ -516,11 +521,17 @@ internal fun DetailContent( DigitalFileCard(type = file.format.ifBlank { if (audio) "Audio" else "eBook" }, name = file.label.ifBlank { digitalFilename(file.url, book.title) }, kind = stringResource(if (audio) R.string.book_section_audiobook else if (file.kind == "supplement") R.string.book_section_related_documents else R.string.book_section_ebook)) { - if (audio && selectedAudio == file.url) { + if (file.locked) { + LockedDigitalFile(file.access) + } else if (audio && selectedAudio == file.url) { androidx.compose.runtime.key(file.url) { AudioPlayer(audioUrl = file.url) } } else PrimaryButton(label = stringResource(if (audio) R.string.collection_play else if (file.isPdf) R.string.book_read else R.string.ebook_open_external), onClick = { - if (audio) selectedAudio = file.url + // A signed link lasts a day: past that, fetch the record again for a fresh one. + if (file.linkExpired()) { + onRefreshLinks() + onShowMessage(context.getString(R.string.digital_link_refreshed)) + } else if (audio) selectedAudio = file.url else if (file.isPdf) selectedPdf = file.url else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } @@ -602,6 +613,24 @@ internal fun DetailContent( } } +/** + * A file this reader may not open yet: no button, and the reason instead (issue #480). + * The server leaves staff-only files out of the list, so only the other reasons reach here. + */ +@Composable +private fun LockedDigitalFile(access: String?) { + val colors = LocalPinakesColors.current + Row(verticalAlignment = Alignment.CenterVertically, horizontalArrangement = Arrangement.spacedBy(8.dp), + modifier = Modifier.fillMaxWidth().testTag("digital-locked")) { + Icon(Icons.Outlined.Lock, contentDescription = null, tint = colors.muted, modifier = Modifier.size(18.dp)) + Text(stringResource(when (access) { + DigitalAttachment.ACCESS_LOGIN_REQUIRED -> R.string.digital_locked_login + DigitalAttachment.ACCESS_LOAN_REQUIRED -> R.string.digital_locked_loan + else -> R.string.digital_locked_other + }), style = MaterialTheme.typography.bodyMedium, color = colors.muted) + } +} + /** * Personal-status banner driven by [personalHistory]. Priority: active loan (primary-tinted) * → pending request (neutral secondary-tinted) → reservation (tertiary-tinted) → wishlist diff --git a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt index c00666c..cb3e09c 100644 --- a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt +++ b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt @@ -95,6 +95,49 @@ class DigitalAvailabilityTest { org.junit.Assert.assertFalse(old.copy(ebookUrl = "javascript:bad").digitalOnly) } + // The app's own parser settings (NetworkModule.json): a closed file's url = null becomes "". + private val api = Json { ignoreUnknownKeys = true; explicitNulls = false; isLenient = true; coerceInputValues = true } + + @Test fun closedFilesKeepTheirCardWithTheReasonInsteadOfAButton() { + val book = api.decodeFromString("""{"id":7,"title":"Access","availability":{"copies_total":1}, + "digital_attachments":[ + {"url":"https://library.example/uploads/digital/open.pdf?pk_exp=4102444800&pk_uid=3&pk_sig=ab","label":"Open edition","kind":"ebook","visibility":"public","access":"granted"}, + {"url":null,"label":"Readers edition","kind":"ebook","visibility":"users","access":"login_required"}, + {"url":null,"label":"Loan edition","kind":"audio","visibility":"loan","access":"loan_required"}]}""") + // Two closed files share the same empty address: both stay listed. + assertEquals(listOf("Open edition", "Readers edition", "Loan edition"), book.digitalFiles.map { it.label }) + show(book) + compose.onNodeWithText("Sign in to open it").performScrollTo().assertIsDisplayed() + compose.onNodeWithText("For readers who have this book on loan").performScrollTo().assertIsDisplayed() + compose.onAllNodesWithTag("digital-locked").assertCountEquals(2) + compose.onAllNodesWithText("Read").assertCountEquals(1) + compose.onAllNodesWithText("Play").assertCountEquals(0) + } + + @Test fun filesFromServersBeforeAccessControlAreOpen() { + val book = api.decodeFromString("""{"title":"Old","digital_attachments":[{"url":"https://library.example/a.pdf","label":"A","kind":"ebook"}]}""") + val file = book.digitalFiles.single() + org.junit.Assert.assertFalse(file.locked) + org.junit.Assert.assertFalse(file.linkExpired()) + } + + @Test fun anExpiredSignedLinkFetchesTheRecordAgainInsteadOfOpening() { + var refreshes = 0 + val messages = mutableListOf() + val expired = DigitalAttachment("https://library.example/uploads/digital/x.pdf?pk_exp=1000&pk_uid=3&pk_sig=ab", label = "Old link", access = "granted") + org.junit.Assert.assertTrue(expired.linkExpired()) + org.junit.Assert.assertFalse(expired.copy(url = "https://library.example/uploads/digital/x.pdf?pk_exp=4102444800").linkExpired()) + compose.setContent { PinakesTheme { + DetailContent(BookDetail(title = "Expired", digitalAttachments = listOf(expired)), wishlisted = false, reserveBusy = false, + canBorrow = true, showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, + onShowMessage = { messages += it }, onRefreshLinks = { refreshes++ }) + } } + compose.onNodeWithText("Read").performScrollTo().performClick() + compose.waitForIdle() + assertEquals(1, refreshes) + assertEquals(listOf("Link refreshed: tap again to open it."), messages) + } + @Test fun wantedCreditsKeepTranslatorSeparateAndSupportOlderServers() { val book = Json.decodeFromString("""{"id":133,"title":"Wanted","author":"Tomás Ibáñez","contributors":[{"id":1,"name":"Tomás Ibáñez","role":"principale"},{"id":2,"name":"Alice Zaffaroni","role":"traduttore"}]}""") val aliases = listOf(AuthorRef(3, "Italian alias", "autore"), AuthorRef(4, "English alias", "author"), diff --git a/i18n/de.json b/i18n/de.json index da7c81b..b1e83bf 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Kritische Ausgabe", "staff_expression_audio": "Audio", "staff_expression_other": "Andere", - "staff_field_title_plain": "Titel" + "staff_field_title_plain": "Titel", + "digital_locked_login": "Zum Öffnen anmelden", + "digital_locked_loan": "Für Leser, die dieses Buch ausgeliehen haben", + "digital_locked_other": "Für dein Konto nicht verfügbar", + "digital_link_refreshed": "Link aktualisiert: tippe erneut, um ihn zu öffnen." } diff --git a/i18n/en.json b/i18n/en.json index 24a1131..64d8b52 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Critical edition", "staff_expression_audio": "Audio", "staff_expression_other": "Other", - "staff_field_title_plain": "Title" + "staff_field_title_plain": "Title", + "digital_locked_login": "Sign in to open it", + "digital_locked_loan": "For readers who have this book on loan", + "digital_locked_other": "Not available to your account", + "digital_link_refreshed": "Link refreshed: tap again to open it." } diff --git a/i18n/fr.json b/i18n/fr.json index e7a4498..402ee39 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Édition critique", "staff_expression_audio": "Audio", "staff_expression_other": "Autre", - "staff_field_title_plain": "Titre" + "staff_field_title_plain": "Titre", + "digital_locked_login": "Connectez-vous pour l'ouvrir", + "digital_locked_loan": "Pour les lecteurs qui ont emprunté ce livre", + "digital_locked_other": "Non disponible pour votre compte", + "digital_link_refreshed": "Lien actualisé : touchez à nouveau pour l'ouvrir." } diff --git a/i18n/it.json b/i18n/it.json index 8c3191f..395eddc 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -1442,5 +1442,9 @@ "staff_expression_critical": "Edizione critica", "staff_expression_audio": "Audio", "staff_expression_other": "Altro", - "staff_field_title_plain": "Titolo" + "staff_field_title_plain": "Titolo", + "digital_locked_login": "Accedi per aprirlo", + "digital_locked_loan": "Per chi ha il libro in prestito", + "digital_locked_other": "Non disponibile per il tuo account", + "digital_link_refreshed": "Collegamento aggiornato: tocca di nuovo per aprire." } From c8c283a5198f58a3411e19776cd7733bf2c8d617 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 17:02:48 +0000 Subject: [PATCH 05/14] Say the loan request lasts the library's loan period, not about a month The loan-request dialog said the loan would last "about 1 month" from the chosen day. The server sets the return date from the loan duration in the library's loan settings (loan_duration_days, 30 by default), on POST /reservations as on the website, so a library with a 14 or 60 day period saw a wrong promise. Pinakes #485 made the website's return picker open on that same duration; the app now names the library's standard loan period in all four languages, the way the staff new-loan form already does. The comments that described the return as +1 month say the same. --- app/src/main/java/com/pinakes/app/data/model/Models.kt | 5 +++-- .../com/pinakes/app/data/repository/LibraryRepository.kt | 3 ++- .../com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt | 4 ++-- i18n/de.json | 2 +- i18n/en.json | 2 +- i18n/fr.json | 2 +- i18n/it.json | 2 +- 7 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index cc21cc9..4a8efaa 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -488,8 +488,9 @@ data class AvailabilityDay( @Serializable data class ReservationRequest( @SerialName("book_id") val bookId: Int, - // Single desired start date "yyyy-MM-dd"; the backend computes the return as +1 month - // (same as the website). This is the authoritative field the request flow sends. + // Single desired start date "yyyy-MM-dd"; the backend sets the return from the library's + // configured loan duration (same as the website). This is the authoritative field the + // request flow sends. @SerialName("desired_date") val desiredDate: String? = null, @SerialName("start_date") val startDate: String? = null, // legacy/compat, yyyy-MM-dd @SerialName("end_date") val endDate: String? = null, // legacy/compat, yyyy-MM-dd diff --git a/app/src/main/java/com/pinakes/app/data/repository/LibraryRepository.kt b/app/src/main/java/com/pinakes/app/data/repository/LibraryRepository.kt index 42f1120..f433437 100644 --- a/app/src/main/java/com/pinakes/app/data/repository/LibraryRepository.kt +++ b/app/src/main/java/com/pinakes/app/data/repository/LibraryRepository.kt @@ -62,7 +62,8 @@ class LibraryRepository( /** * Request a loan/reservation for [bookId] starting on [desiredDate] (yyyy-MM-dd). The - * backend computes the return date as +1 month, matching the website behaviour. + * backend sets the return date from the library's configured loan duration + * (loan_duration_days), matching the website behaviour. */ suspend fun reserve( bookId: Int, diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt index 084def9..549e768 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt @@ -160,8 +160,8 @@ class BookDetailViewModel @Inject constructor( /** * Submit a loan/reservation request for [desiredDate] (yyyy-MM-dd, today-or-future). The - * backend computes the return as +1 month. On success we surface the chosen date so the UI - * can confirm exactly what was requested. + * backend sets the return from the library's configured loan duration. On success we surface + * the chosen date so the UI can confirm exactly what was requested. */ fun reserve(desiredDate: String) { if (_state.value.reserveBusy) return diff --git a/i18n/de.json b/i18n/de.json index da7c81b..3dd74ea 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -250,7 +250,7 @@ "cover_viewer_close": "Schließen", "book_loan_dialog_title": "Startdatum wählen", "book_loan_pick_date": "Datum wählen", - "book_loan_period_hint": "Ab %1$s, etwa 1 Monat", + "book_loan_period_hint": "Ab %1$s, für die übliche Leihfrist der Bibliothek", "book_loan_confirm": "Ausleihe anfragen", "book_loan_cancel": "Abbrechen", "book_loan_legend_available": "Verfügbar", diff --git a/i18n/en.json b/i18n/en.json index 24a1131..2cfbc62 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -250,7 +250,7 @@ "cover_viewer_close": "Close", "book_loan_dialog_title": "Choose a start date", "book_loan_pick_date": "Pick a date", - "book_loan_period_hint": "From %1$s, about 1 month", + "book_loan_period_hint": "From %1$s, for the library’s standard loan period", "book_loan_confirm": "Request loan", "book_loan_cancel": "Cancel", "book_loan_legend_available": "Available", diff --git a/i18n/fr.json b/i18n/fr.json index e7a4498..4b9746e 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -250,7 +250,7 @@ "cover_viewer_close": "Fermer", "book_loan_dialog_title": "Choisir une date de début", "book_loan_pick_date": "Choisir une date", - "book_loan_period_hint": "À partir du %1$s, environ 1 mois", + "book_loan_period_hint": "À partir du %1$s, pour la durée de prêt standard de la bibliothèque", "book_loan_confirm": "Demander le prêt", "book_loan_cancel": "Annuler", "book_loan_legend_available": "Disponible", diff --git a/i18n/it.json b/i18n/it.json index 8c3191f..19f5643 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -250,7 +250,7 @@ "cover_viewer_close": "Chiudi", "book_loan_dialog_title": "Scegli una data di inizio", "book_loan_pick_date": "Scegli una data", - "book_loan_period_hint": "Dal %1$s, circa 1 mese", + "book_loan_period_hint": "Dal %1$s, per la durata di prestito standard della biblioteca", "book_loan_confirm": "Richiedi prestito", "book_loan_cancel": "Annulla", "book_loan_legend_available": "Disponibile", From df26241738b33f27010710d10a3eab7fa3c3dff3 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 17:17:13 +0000 Subject: [PATCH 06/14] Show the return date in the loan-request dialog MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mobile API 1.8.1 (Pinakes #485) adds loan_duration_days to GET /catalog/books/{id}/availability: the days POST /reservations adds to the chosen start to set the return date. The dialog now reads "Start: · Return by: " for the selected day, in all four languages. A server that does not send the field keeps the wording about the library's standard loan period. LoanRequestReturnDateTest covers the date arithmetic, the parsing with and without the field, and both dialog texts. --- .../java/com/pinakes/app/data/model/Models.kt | 3 + .../app/ui/screens/detail/BookDetailScreen.kt | 16 +++- .../pinakes/app/LoanRequestReturnDateTest.kt | 77 +++++++++++++++++++ i18n/de.json | 1 + i18n/en.json | 1 + i18n/fr.json | 1 + i18n/it.json | 1 + 7 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index 4a8efaa..81c113e 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -474,6 +474,9 @@ data class AvailabilityCalendar( @SerialName("earliest_available") val earliestAvailable: String? = null, // "yyyy-MM-dd" @SerialName("unavailable_dates") val unavailableDates: List = emptyList(), val days: List = emptyList(), + // Days POST /reservations adds to the chosen start to set the return date (Mobile API + // 1.8.1). Null on older servers: the dialog then names the standard period without a date. + @SerialName("loan_duration_days") val loanDurationDays: Int? = null, ) @Serializable diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 6661e66..13f1bdd 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -303,9 +303,11 @@ internal fun LoanRequestSheet( } } - // Period hint / prompt. - val hint = selectedIso?.let { - stringResource(R.string.book_loan_period_hint, formatDisplayDate(it)) + // Period hint / prompt: the return date when the server gives the loan duration. + val hint = selectedIso?.let { start -> + calendar?.loanDurationDays?.let { loanReturnDate(start, it) }?.let { end -> + stringResource(R.string.book_loan_period_range, formatDisplayDate(start), formatDisplayDate(end)) + } ?: stringResource(R.string.book_loan_period_hint, formatDisplayDate(start)) } ?: stringResource(R.string.book_loan_pick_prompt) Text( text = hint, @@ -707,6 +709,14 @@ private fun digitalFilename(url: String, fallback: String): String = private val displayDateFormatter: DateTimeFormatter get() = DateTimeFormatter.ofLocalizedDate(FormatStyle.MEDIUM).withLocale(com.pinakes.app.ui.common.AppLocale.current()) +/** + * Return date (yyyy-MM-dd) of a loan request starting on [startIso]: the server adds the + * library's loan duration to the start. Null for an unparseable start or a duration below 1. + */ +internal fun loanReturnDate(startIso: String, loanDurationDays: Int): String? = + if (loanDurationDays < 1) null + else runCatching { LocalDate.parse(startIso).plusDays(loanDurationDays.toLong()).toString() }.getOrNull() + /** "2026-06-20" (yyyy-MM-dd) → locale-formatted date for display. Falls back to the raw input. */ private fun formatDisplayDate(iso: String): String = runCatching { LocalDate.parse(iso).format(displayDateFormatter) }.getOrDefault(iso) diff --git a/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt b/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt new file mode 100644 index 0000000..1336d03 --- /dev/null +++ b/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt @@ -0,0 +1,77 @@ +package com.pinakes.app + +import android.app.Application +import android.content.ComponentName +import androidx.activity.ComponentActivity +import androidx.compose.ui.test.assertIsDisplayed +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performScrollTo +import androidx.test.core.app.ApplicationProvider +import com.pinakes.app.data.model.AvailabilityCalendar +import com.pinakes.app.ui.common.DateFormat +import com.pinakes.app.ui.screens.detail.LoanRequestSheet +import com.pinakes.app.ui.screens.detail.loanReturnDate +import com.pinakes.app.ui.theme.PinakesTheme +import kotlinx.serialization.json.Json +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Rule +import org.junit.Test +import org.junit.rules.RuleChain +import org.junit.rules.TestWatcher +import org.junit.runner.Description +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +/** + * The loan-request dialog names the return date the server will set: the chosen start plus + * the `loan_duration_days` of GET /catalog/books/{id}/availability (Mobile API 1.8.1). + */ +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class, qualifiers = "w411dp-h891dp-xxhdpi") +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class LoanRequestReturnDateTest { + private val compose = createComposeRule() + @get:Rule val rules: RuleChain = RuleChain.outerRule(object : TestWatcher() { + override fun starting(description: Description) { + val app = ApplicationProvider.getApplicationContext() + shadowOf(app.packageManager).addActivityIfNotPresent(ComponentName(app.packageName, ComponentActivity::class.java.name)) + } + }).around(compose) + + private fun show(calendar: AvailabilityCalendar) { + compose.setContent { PinakesTheme { + LoanRequestSheet(loading = false, calendar = calendar, fallback = false, submitting = false, + onDismiss = {}, onConfirm = {}) + } } + } + + @Test fun returnDateIsTheStartPlusTheLoanDuration() { + assertEquals("2030-03-18", loanReturnDate("2030-03-04", 14)) + assertEquals("2030-04-03", loanReturnDate("2030-03-04", 30)) + assertNull(loanReturnDate("2030-03-04", 0)) + assertNull(loanReturnDate("not a date", 14)) + } + + @Test fun availabilityCarriesTheLoanDurationAndOlderServersOmitIt() { + val json = Json { ignoreUnknownKeys = true } + assertEquals(21, json.decodeFromString("""{"total_copies":1,"loan_duration_days":21}""").loanDurationDays) + assertNull(json.decodeFromString("""{"total_copies":1}""").loanDurationDays) + } + + @Test fun dialogShowsTheReturnDateForThePreselectedStart() { + show(AvailabilityCalendar(totalCopies = 1, earliestAvailable = "2030-03-04", loanDurationDays = 14)) + compose.onNodeWithText("Start: ${DateFormat.date("2030-03-04")} · Return by: ${DateFormat.date("2030-03-18")}") + .performScrollTo().assertIsDisplayed() + } + + @Test fun withoutTheDurationTheDialogNamesTheStandardPeriod() { + show(AvailabilityCalendar(totalCopies = 1, earliestAvailable = "2030-03-04")) + compose.onNodeWithText("From ${DateFormat.date("2030-03-04")}, for the library’s standard loan period") + .performScrollTo().assertIsDisplayed() + } +} diff --git a/i18n/de.json b/i18n/de.json index 3dd74ea..fe85b96 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -251,6 +251,7 @@ "book_loan_dialog_title": "Startdatum wählen", "book_loan_pick_date": "Datum wählen", "book_loan_period_hint": "Ab %1$s, für die übliche Leihfrist der Bibliothek", + "book_loan_period_range": "Beginn: %1$s · Rückgabe: %2$s", "book_loan_confirm": "Ausleihe anfragen", "book_loan_cancel": "Abbrechen", "book_loan_legend_available": "Verfügbar", diff --git a/i18n/en.json b/i18n/en.json index 2cfbc62..bc6033d 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -251,6 +251,7 @@ "book_loan_dialog_title": "Choose a start date", "book_loan_pick_date": "Pick a date", "book_loan_period_hint": "From %1$s, for the library’s standard loan period", + "book_loan_period_range": "Start: %1$s · Return by: %2$s", "book_loan_confirm": "Request loan", "book_loan_cancel": "Cancel", "book_loan_legend_available": "Available", diff --git a/i18n/fr.json b/i18n/fr.json index 4b9746e..3394b76 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -251,6 +251,7 @@ "book_loan_dialog_title": "Choisir une date de début", "book_loan_pick_date": "Choisir une date", "book_loan_period_hint": "À partir du %1$s, pour la durée de prêt standard de la bibliothèque", + "book_loan_period_range": "Début : %1$s · Retour : %2$s", "book_loan_confirm": "Demander le prêt", "book_loan_cancel": "Annuler", "book_loan_legend_available": "Disponible", diff --git a/i18n/it.json b/i18n/it.json index 19f5643..937b97f 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -251,6 +251,7 @@ "book_loan_dialog_title": "Scegli una data di inizio", "book_loan_pick_date": "Scegli una data", "book_loan_period_hint": "Dal %1$s, per la durata di prestito standard della biblioteca", + "book_loan_period_range": "Inizio: %1$s · Restituzione: %2$s", "book_loan_confirm": "Richiedi prestito", "book_loan_cancel": "Annulla", "book_loan_legend_available": "Disponibile", From bdb593c765825f45548a0a3fa11dcc342f1313df Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 20 Oct 2026 19:39:40 +0000 Subject: [PATCH 07/14] Document the loan-request return-date tests CodeRabbit's docstring check counted the test methods and the show() helper of LoanRequestReturnDateTest as undocumented (45% against the 80% threshold). Each now says what it pins down. Comments only. --- .../test/java/com/pinakes/app/LoanRequestReturnDateTest.kt | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt b/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt index 1336d03..46dadd0 100644 --- a/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt +++ b/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt @@ -43,6 +43,7 @@ class LoanRequestReturnDateTest { } }).around(compose) + /** Renders the loan-request dialog for [calendar], with no-op callbacks. */ private fun show(calendar: AvailabilityCalendar) { compose.setContent { PinakesTheme { LoanRequestSheet(loading = false, calendar = calendar, fallback = false, submitting = false, @@ -50,6 +51,7 @@ class LoanRequestReturnDateTest { } } } + /** The server adds the duration to the start; a duration below 1 or a bad start gives no date. */ @Test fun returnDateIsTheStartPlusTheLoanDuration() { assertEquals("2030-03-18", loanReturnDate("2030-03-04", 14)) assertEquals("2030-04-03", loanReturnDate("2030-03-04", 30)) @@ -57,18 +59,21 @@ class LoanRequestReturnDateTest { assertNull(loanReturnDate("not a date", 14)) } + /** Mobile API 1.8.1 sends loan_duration_days; an older server leaves it out and the field is null. */ @Test fun availabilityCarriesTheLoanDurationAndOlderServersOmitIt() { val json = Json { ignoreUnknownKeys = true } assertEquals(21, json.decodeFromString("""{"total_copies":1,"loan_duration_days":21}""").loanDurationDays) assertNull(json.decodeFromString("""{"total_copies":1}""").loanDurationDays) } + /** With the duration, the dialog names the start and the return date of the preselected day. */ @Test fun dialogShowsTheReturnDateForThePreselectedStart() { show(AvailabilityCalendar(totalCopies = 1, earliestAvailable = "2030-03-04", loanDurationDays = 14)) compose.onNodeWithText("Start: ${DateFormat.date("2030-03-04")} · Return by: ${DateFormat.date("2030-03-18")}") .performScrollTo().assertIsDisplayed() } + /** Without it, the dialog falls back to the library's standard loan period, with no date. */ @Test fun withoutTheDurationTheDialogNamesTheStandardPeriod() { show(AvailabilityCalendar(totalCopies = 1, earliestAvailable = "2030-03-04")) compose.onNodeWithText("From ${DateFormat.date("2030-03-04")}, for the library’s standard loan period") From 1568f7db6d71d14e42944ab21ae8a156787b03ab Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 00:19:03 +0200 Subject: [PATCH 08/14] Fix signed file renewal without device-clock or stale-cache assumptions --- .../staffe2e/DigitalLinkRefreshFlowTest.kt | 106 ++++++++++++ .../com/pinakes/app/staffe2e/FakePinakes.kt | 7 +- .../java/com/pinakes/app/data/model/Models.kt | 11 +- .../pinakes/app/data/network/PinakesApi.kt | 1 + .../app/data/repository/CatalogRepository.kt | 10 +- .../app/ui/screens/detail/BookDetailScreen.kt | 54 ++++-- .../ui/screens/detail/BookDetailViewModel.kt | 19 +++ .../pinakes/app/DigitalAvailabilityTest.kt | 11 +- .../com/pinakes/app/DigitalLinkRefreshTest.kt | 157 ++++++++++++++++++ i18n/de.json | 2 +- i18n/en.json | 2 +- i18n/fr.json | 2 +- i18n/it.json | 2 +- 13 files changed, 352 insertions(+), 32 deletions(-) create mode 100644 app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt create mode 100644 app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt new file mode 100644 index 0000000..4c59b84 --- /dev/null +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/DigitalLinkRefreshFlowTest.kt @@ -0,0 +1,106 @@ +package com.pinakes.app.staffe2e + +import androidx.lifecycle.SavedStateHandle +import androidx.lifecycle.ViewModelStore +import com.pinakes.app.data.model.BookDetail +import com.pinakes.app.data.model.DigitalAttachment +import com.pinakes.app.data.model.Envelope +import com.pinakes.app.data.network.ApiResult +import com.pinakes.app.data.network.ErrorCodes +import com.pinakes.app.ui.common.UiState +import com.pinakes.app.ui.navigation.Routes +import com.pinakes.app.ui.screens.detail.BookDetailViewModel +import kotlinx.coroutines.runBlocking +import kotlinx.serialization.encodeToString +import kotlinx.serialization.json.Json +import org.junit.After +import org.junit.Assert.* +import org.junit.Test + +/** Real repository/ViewModel, HTTP cache and access decisions on a disposable emulator. */ +class DigitalLinkRefreshFlowTest : AppFlowTest() { + private val old = DigitalAttachment("https://library.example/a.epub?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + private val fresh = old.copy(url = old.url.replace("1000", "4102444800").replace("old", "new")) + private val book = BookDetail(id = 1, title = "Digital", digitalAttachments = listOf(old)) + private val store = ViewModelStore() + + private fun FakePinakes.answer(data: BookDetail = book) = body("GET catalog/books/1", 200, + Json.encodeToString(Envelope(data = data)), mapOf("ETag" to "\"digital-v1\"", "Cache-Control" to "private, max-age=3600")) + + private fun viewModel(): BookDetailViewModel { + lateinit var vm: BookDetailViewModel + instrumentation.runOnMainSync { + vm = BookDetailViewModel(SavedStateHandle(mapOf(Routes.ARG_BOOK_ID to 1)), + deps.catalogRepository(), deps.libraryRepository(), deps.wishlistRepository(), deps.connectivity()) + store.put("digital", vm) + } + compose.waitUntil(10_000) { vm.state.value.content is UiState.Success } + return vm + } + + @After fun clearViewModels() { instrumentation.runOnMainSync { store.clear() } } + + @Test fun forcedRenewalBypassesTheHttpCacheAndGetsANewSignature() { + launch { answer() } + val repository = deps.catalogRepository() + assertTrue(runBlocking { repository.book(1) } is ApiResult.Success) + api.answer(book.copy(digitalAttachments = listOf(fresh))) + val response = runBlocking { repository.book(1, forceRefresh = true) } + assertEquals(fresh.url, (response as ApiResult.Success).data.digitalFiles.single().url) + val request = api.recorded("GET", "catalog/books/1").last() + assertNull(request.getHeader("If-None-Match")) + assertEquals("no-cache, no-store", request.getHeader("Cache-Control")) + assertEquals(2, api.recorded("GET", "catalog/books/1").size) + } + + @Test fun aServerFailureDuringRenewalCannotReturnTheCachedSignedLink() { + launch { answer() } + val repository = deps.catalogRepository() + assertTrue(runBlocking { repository.book(1) } is ApiResult.Success) + api.body("GET catalog/books/1", 503, """{"error":{"code":"server_error","message":"Unavailable"}}""") + val response = runBlocking { repository.book(1, forceRefresh = true) } + assertTrue(response is ApiResult.Failure) + assertEquals(503, (response as ApiResult.Failure).httpStatus) + } + + @Test fun anUnexpected304CannotPretendTheExpiredPayloadWasRenewed() { + launch { answer() } + val repository = deps.catalogRepository() + assertTrue(runBlocking { repository.book(1) } is ApiResult.Success) + api.body("GET catalog/books/1", 304, "") + assertTrue(runBlocking { repository.book(1, forceRefresh = true) } is ApiResult.Failure) + } + + @Test fun renewalFailurePreservesTheLoadedPageAndASubsequentRetrySucceeds() { + launch { answer() } + val vm = viewModel() + val before = vm.state.value.content + api.body("GET catalog/books/1", 500, """{"error":{"code":"server_error","message":"Unavailable"}}""") + assertTrue(runBlocking { vm.refreshDigitalFile(old) } is ApiResult.Failure) + assertSame(before, vm.state.value.content) + api.answer(book.copy(digitalAttachments = listOf(fresh))) + val result = runBlocking { vm.refreshDigitalFile(old) } + assertEquals(fresh.url, (result as ApiResult.Success).data.url) + assertEquals(fresh.url, (vm.state.value.content as UiState.Success).data.digitalFiles.single().url) + } + + @Test fun renewalSelectsTheSameFileEvenWhenTheServerReordersAndRenamesEntries() { + launch { answer() } + val vm = viewModel() + val other = fresh.copy(url = fresh.url.replace("a.epub", "b.epub")) + api.answer(book.copy(digitalAttachments = listOf(other, fresh.copy(label = "Renamed")))) + val result = runBlocking { vm.refreshDigitalFile(old) } + assertEquals(fresh.url, (result as ApiResult.Success).data.url) + } + + @Test fun revokedFileAccessUpdatesTheCardAndReturnsNoOpenableAddress() { + launch { answer() } + val vm = viewModel() + api.answer(book.copy(digitalAttachments = listOf(old.copy(url = "", access = "loan_required")))) + val result = runBlocking { vm.refreshDigitalFile(old) } + assertEquals(ErrorCodes.FORBIDDEN, (result as ApiResult.Failure).code) + val file = (vm.state.value.content as UiState.Success).data.digitalFiles.single() + assertTrue(file.locked) + assertNull(file.safeUrl) + } +} diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt index bee02aa..5366f8d 100644 --- a/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/FakePinakes.kt @@ -71,8 +71,11 @@ class FakePinakes { } } - fun body(route: String, status: Int, body: String) { - routes[route] = { MockResponse().setResponseCode(status).setHeader("Content-Type", "application/json").setBody(body) } + fun body(route: String, status: Int, body: String, headers: Map = emptyMap()) { + routes[route] = { + MockResponse().setResponseCode(status).setHeader("Content-Type", "application/json").setBody(body) + .apply { headers.forEach { (name, value) -> setHeader(name, value) } } + } } fun recorded(method: String, path: String): List = diff --git a/app/src/main/java/com/pinakes/app/data/model/Models.kt b/app/src/main/java/com/pinakes/app/data/model/Models.kt index f36f65e..b63b8a0 100644 --- a/app/src/main/java/com/pinakes/app/data/model/Models.kt +++ b/app/src/main/java/com/pinakes/app/data/model/Models.kt @@ -380,9 +380,14 @@ data class DigitalAttachment( val expiresAt: Long? get() = runCatching { java.net.URI(url).rawQuery }.getOrNull() ?.split('&')?.firstOrNull { it.startsWith("pk_exp=") }?.substringAfter('=')?.toLongOrNull() - /** 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 + /** Identity of the file across signature renewals; keep every non-signature query parameter. */ + val resourceKey: String get() { + val base = url.substringBefore('#').substringBefore('?') + val query = runCatching { java.net.URI(url).rawQuery }.getOrNull() + ?.split('&')?.filterNot { it.substringBefore('=') in setOf("pk_exp", "pk_uid", "pk_sig") } + ?.filter { it.isNotEmpty() }?.sorted().orEmpty() + return base + if (query.isEmpty()) "" else "?" + query.joinToString("&") + } val format: String get() = runCatching { java.net.URI(url).path.substringAfterLast('.').uppercase() }.getOrDefault("") val isPdf: Boolean get() = format == "PDF" diff --git a/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt b/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt index 9b78e0b..31ef01c 100644 --- a/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt +++ b/app/src/main/java/com/pinakes/app/data/network/PinakesApi.kt @@ -116,6 +116,7 @@ interface PinakesApi { suspend fun book( @Path("id") id: Int, @Header("If-None-Match") etag: String? = null, // -> 304 cache + @Header("Cache-Control") cacheControl: String? = null, ): Response> // Response<> to read ETag header + 304 @GET("catalog/books/{id}/availability") diff --git a/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt b/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt index e46d8ca..23b978c 100644 --- a/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt +++ b/app/src/main/java/com/pinakes/app/data/repository/CatalogRepository.kt @@ -256,13 +256,15 @@ class CatalogRepository( } } - suspend fun book(id: Int): ApiResult { + /** A signed-file renewal must reach the server and must never fall back to a stale payload. */ + suspend fun book(id: Int, forceRefresh: Boolean = false): ApiResult { val api = network.api() val cached = detailCache[id] - val (result, response) = apiResponse { api.book(id, etag = cached?.first) } + val (result, response) = apiResponse { api.book(id, etag = if (forceRefresh) null else cached?.first, + cacheControl = if (forceRefresh) "no-cache, no-store" else null) } // 304 Not Modified → reuse the cached payload. - if (response?.code() == 304 && cached != null) { + if (!forceRefresh && response?.code() == 304 && cached != null) { return ApiResult.Success(cached.second) } return when (result) { @@ -273,7 +275,7 @@ class CatalogRepository( } is ApiResult.Failure -> { // If the conditional request errored but we have a cached copy, serve it. - if (cached != null && result.code != ErrorCodes.NETWORK) { + if (!forceRefresh && cached != null && result.code != ErrorCodes.NETWORK) { ApiResult.Success(cached.second) } else { result diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 05f8a30..1c98c4a 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -79,6 +79,8 @@ import com.pinakes.app.data.model.BookDetail import com.pinakes.app.data.model.CirculationRequestKind import com.pinakes.app.data.model.DigitalAttachment import com.pinakes.app.data.model.PersonalHistory +import com.pinakes.app.data.network.ApiResult +import com.pinakes.app.data.network.ErrorCodes import com.pinakes.app.ui.common.AppViewModel import com.pinakes.app.ui.common.OfflinePolicy import com.pinakes.app.ui.components.PinakesBottomBar @@ -202,7 +204,7 @@ fun BookDetailScreen( onReserve = vm::openLoanSheet, onToggleWishlist = vm::toggleWishlist, onFindWorks = onFindWorks, - onRefreshLinks = vm::load, + onRefreshFile = vm::refreshDigitalFile, onShowMessage = { msg -> scope.launch { snackbarHost.showSnackbar(msg) } }, offlineNotice = if (state.offlineNow) stringResource(R.string.offline_actions_unavailable) else null, offlineSyncedAt = state.syncedAt, @@ -341,8 +343,10 @@ internal fun DetailContent( onToggleWishlist: () -> Unit, onShowMessage: (String) -> Unit, onFindWorks: (String, Int?) -> Unit = { _, _ -> }, - /** Fetches the record again: its signed file links last a day (issue #480). */ - onRefreshLinks: () -> Unit = {}, + /** Revalidates a signed file on the server, preserving this page while the request runs. */ + onRefreshFile: suspend (DigitalAttachment) -> ApiResult = { + ApiResult.Failure(ErrorCodes.NETWORK, "") + }, /** Set when the page is built from the offline cache: shown above the cover. */ offlineNotice: String? = null, /** Last sync time of the offline copy: availability is shown as "last known", never current. */ @@ -350,9 +354,20 @@ internal fun DetailContent( ) { val offline = offlineNotice != null val context = LocalContext.current + val scope = rememberCoroutineScope() + var refreshingFile by remember(book.id) { mutableStateOf(null) } var showCover by remember { mutableStateOf(false) } var selectedPdf by remember(book.id) { mutableStateOf(null) } - var selectedAudio by remember(book.id) { mutableStateOf(book.audioUrl?.takeIf { book.hasAudio }) } + var selectedAudio by remember(book.id) { mutableStateOf(null) } + + fun openFile(file: DigitalAttachment) { + if (file.locked || file.safeUrl == null) { + onShowMessage(context.getString(R.string.digital_locked_other)) + } else if (file.kind == "audio") selectedAudio = file.url + else if (file.isPdf) selectedPdf = file.url + else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } + .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } + } Column( Modifier @@ -527,15 +542,28 @@ internal fun DetailContent( androidx.compose.runtime.key(file.url) { AudioPlayer(audioUrl = file.url) } } else PrimaryButton(label = stringResource(if (audio) R.string.collection_play else if (file.isPdf) R.string.book_read else R.string.ebook_open_external), onClick = { - // A signed link lasts a day: past that, fetch the record again for a fresh one. - if (file.linkExpired()) { - onRefreshLinks() - onShowMessage(context.getString(R.string.digital_link_refreshed)) - } else if (audio) selectedAudio = file.url - else if (file.isPdf) selectedPdf = file.url - else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } - .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } - }, modifier = Modifier.fillMaxWidth(), dark = true) + // The server's clock and access decision are authoritative. Always renew + // a signed URL on demand, then open it in this same tap (no clock-skew loop). + if (refreshingFile == null) { + if (file.expiresAt == null) openFile(file) + else { + refreshingFile = file.resourceKey + scope.launch { + try { + when (val result = onRefreshFile(file)) { + is ApiResult.Success -> openFile(result.data) + is ApiResult.Failure -> onShowMessage(context.getString( + if (result.code == ErrorCodes.FORBIDDEN) R.string.digital_locked_other + else R.string.digital_link_refresh_error)) + } + } finally { + refreshingFile = null + } + } + } + } + }, enabled = refreshingFile == null, loading = refreshingFile == file.resourceKey, + modifier = Modifier.fillMaxWidth().testTag("digital-open"), dark = true) } Spacer(Modifier.height(16.dp)) } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt index 084def9..ffae9f7 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailViewModel.kt @@ -5,6 +5,7 @@ import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope import com.pinakes.app.data.model.AvailabilityCalendar import com.pinakes.app.data.model.BookDetail +import com.pinakes.app.data.model.DigitalAttachment import com.pinakes.app.data.model.CirculationRequestKind import com.pinakes.app.data.network.ApiResult import com.pinakes.app.data.connectivity.ConnectivityObserver @@ -107,6 +108,24 @@ class BookDetailViewModel @Inject constructor( } } + /** Refresh in place: a failed renewal must not replace the page or reuse a cached URL. */ + suspend fun refreshDigitalFile(file: DigitalAttachment): ApiResult { + return when (val result = catalog.book(bookId, forceRefresh = true)) { + is ApiResult.Failure -> result + is ApiResult.Success -> { + _state.update { + it.copy(content = UiState.Success(result.data), offline = false, + wishlisted = result.data.personalHistory?.hasWishlisted ?: it.wishlisted) + } + val renewed = result.data.digitalFiles.firstOrNull { + it.resourceKey == file.resourceKey && it.kind == file.kind && !it.locked && it.safeUrl != null + } + if (renewed != null) ApiResult.Success(renewed) + else ApiResult.Failure(ErrorCodes.FORBIDDEN, "", 403) + } + } + } + fun toggleWishlist() { if (_state.value.wishlistBusy || _state.value.offlineNow) return val nowWishlisted = !_state.value.wishlisted diff --git a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt index cb3e09c..3dfecf5 100644 --- a/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt +++ b/app/src/test/java/com/pinakes/app/DigitalAvailabilityTest.kt @@ -118,24 +118,23 @@ class DigitalAvailabilityTest { val book = api.decodeFromString("""{"title":"Old","digital_attachments":[{"url":"https://library.example/a.pdf","label":"A","kind":"ebook"}]}""") val file = book.digitalFiles.single() org.junit.Assert.assertFalse(file.locked) - org.junit.Assert.assertFalse(file.linkExpired()) + org.junit.Assert.assertNull(file.expiresAt) } - @Test fun anExpiredSignedLinkFetchesTheRecordAgainInsteadOfOpening() { + @Test fun anExpiredSignedLinkDoesNotOpenOrClaimSuccessWhenRenewalFails() { var refreshes = 0 val messages = mutableListOf() val expired = DigitalAttachment("https://library.example/uploads/digital/x.pdf?pk_exp=1000&pk_uid=3&pk_sig=ab", label = "Old link", access = "granted") - org.junit.Assert.assertTrue(expired.linkExpired()) - org.junit.Assert.assertFalse(expired.copy(url = "https://library.example/uploads/digital/x.pdf?pk_exp=4102444800").linkExpired()) + assertEquals(1000L, expired.expiresAt) compose.setContent { PinakesTheme { DetailContent(BookDetail(title = "Expired", digitalAttachments = listOf(expired)), wishlisted = false, reserveBusy = false, canBorrow = true, showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, - onShowMessage = { messages += it }, onRefreshLinks = { refreshes++ }) + onShowMessage = { messages += it }, onRefreshFile = { refreshes++; com.pinakes.app.data.network.ApiResult.Failure("network", "") }) } } compose.onNodeWithText("Read").performScrollTo().performClick() compose.waitForIdle() assertEquals(1, refreshes) - assertEquals(listOf("Link refreshed: tap again to open it."), messages) + assertEquals(listOf("Could not refresh the link. Please try again."), messages) } @Test fun wantedCreditsKeepTranslatorSeparateAndSupportOlderServers() { diff --git a/app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt b/app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt new file mode 100644 index 0000000..e658dc4 --- /dev/null +++ b/app/src/test/java/com/pinakes/app/DigitalLinkRefreshTest.kt @@ -0,0 +1,157 @@ +package com.pinakes.app + +import android.app.Application +import android.content.ComponentName +import androidx.activity.ComponentActivity +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.setValue +import androidx.compose.ui.test.* +import androidx.compose.ui.test.junit4.createComposeRule +import androidx.test.core.app.ApplicationProvider +import com.pinakes.app.data.model.BookDetail +import com.pinakes.app.data.model.DigitalAttachment +import com.pinakes.app.data.network.ApiResult +import com.pinakes.app.data.network.ErrorCodes +import com.pinakes.app.ui.screens.detail.DetailContent +import com.pinakes.app.ui.theme.PinakesTheme +import kotlinx.coroutines.CompletableDeferred +import org.junit.Assert.* +import org.junit.Rule +import org.junit.Test +import org.junit.rules.RuleChain +import org.junit.rules.TestWatcher +import org.junit.runner.Description +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.annotation.Config +import org.robolectric.annotation.GraphicsMode + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [34], application = Application::class, qualifiers = "w411dp-h891dp-xxhdpi") +@GraphicsMode(GraphicsMode.Mode.NATIVE) +class DigitalLinkRefreshTest { + private val compose = createComposeRule() + private val app get() = ApplicationProvider.getApplicationContext() + @get:Rule val rules: RuleChain = RuleChain.outerRule(object : TestWatcher() { + override fun starting(description: Description) { + shadowOf(app.packageManager).addActivityIfNotPresent(ComponentName(app.packageName, ComponentActivity::class.java.name)) + } + }).around(compose) + + private val signed = DigitalAttachment("https://library.example/a.epub?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + private val fresh = signed.copy(url = signed.url.replace("1000", "4102444800").replace("old", "new")) + private val messages = mutableListOf() + + private fun show(file: DigitalAttachment = signed, refresh: suspend (DigitalAttachment) -> ApiResult) { + compose.setContent { PinakesTheme { + DetailContent(BookDetail(id = 7, title = "A digital book", digitalAttachments = listOf(file)), + wishlisted = false, reserveBusy = false, canBorrow = false, showWishlist = false, showReviews = false, + onReserve = {}, onToggleWishlist = {}, onShowMessage = { messages += it }, onRefreshFile = refresh) + } } + } + + private fun open() = compose.onNodeWithText(app.getString(R.string.ebook_open_external)).performScrollTo().performClick() + private fun nextOpenedUrl() = shadowOf(app).nextStartedActivity?.dataString + + @Test fun expiredLinksOpenTheirRenewedAddressInTheSameTap() { + var calls = 0 + show { calls++; ApiResult.Success(fresh) } + open() + compose.waitForIdle() + assertEquals(1, calls) + assertEquals(fresh.url, nextOpenedUrl()) + assertTrue(messages.isEmpty()) + } + + @Test fun evenFutureDatedLinksUseTheServerInsteadOfThePhonesClock() { + var calls = 0 + show(fresh) { calls++; ApiResult.Success(fresh) } + open() + compose.waitForIdle() + assertEquals(1, calls) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun pendingRenewalDisablesRepeatedTapsWithoutClaimingSuccess() { + val response = CompletableDeferred>() + var calls = 0 + show { calls++; response.await() } + open() + compose.onNodeWithTag("digital-open").assertIsNotEnabled() + assertNull(nextOpenedUrl()) + assertTrue(messages.isEmpty()) + compose.runOnIdle { response.complete(ApiResult.Success(fresh)) } + compose.waitForIdle() + assertEquals(1, calls) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun failedRenewalKeepsTheFileAndAllowsRetry() { + var calls = 0 + show { + if (++calls == 1) ApiResult.Failure(ErrorCodes.NETWORK, "offline") else ApiResult.Success(fresh) + } + open() + compose.onNodeWithText(app.getString(R.string.ebook_open_external)).assertIsDisplayed().assertIsEnabled() + assertEquals(listOf(app.getString(R.string.digital_link_refresh_error)), messages) + assertNull(nextOpenedUrl()) + open() + compose.waitForIdle() + assertEquals(2, calls) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun revokedAccessNeverOpensTheOldSignedAddress() { + show { ApiResult.Failure(ErrorCodes.FORBIDDEN, "", 403) } + open() + compose.waitForIdle() + assertNull(nextOpenedUrl()) + assertEquals(listOf(app.getString(R.string.digital_locked_other)), messages) + } + + @Test fun olderUnsignedLinksOpenWithoutARefreshRequest() { + val old = signed.copy(url = "https://library.example/legacy.epub", access = null) + var calls = 0 + show(old) { calls++; ApiResult.Success(fresh) } + open() + compose.waitForIdle() + assertEquals(0, calls) + assertEquals(old.url, nextOpenedUrl()) + } + + @Test fun refreshedBookDataPreservesThePositionOfTheOpenButton() { + var book by mutableStateOf(BookDetail(id = 7, title = "Book", digitalAttachments = listOf(signed))) + compose.setContent { PinakesTheme { + DetailContent(book, wishlisted = false, reserveBusy = false, canBorrow = false, + showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, onShowMessage = {}, + onRefreshFile = { book = book.copy(digitalAttachments = listOf(fresh)); ApiResult.Success(fresh) }) + } } + val button = compose.onNodeWithText(app.getString(R.string.ebook_open_external)).performScrollTo() + val before = button.getBoundsInRoot() + button.performClick() + compose.waitForIdle() + button.assertIsDisplayed() + assertEquals(before.top.value, button.getBoundsInRoot().top.value, .5f) + assertEquals(fresh.url, nextOpenedUrl()) + } + + @Test fun signedAudioWaitsForTheReadersTapInsteadOfStartingAStalePlayer() { + val audio = signed.copy(url = signed.url.replace("epub", "mp3"), kind = "audio") + compose.setContent { PinakesTheme { + DetailContent(BookDetail(id = 7, title = "Audio", hasAudio = true, audioUrl = audio.url, + digitalAttachments = listOf(audio)), wishlisted = false, reserveBusy = false, canBorrow = false, + showWishlist = false, showReviews = false, onReserve = {}, onToggleWishlist = {}, onShowMessage = {}) + } } + compose.onNodeWithText(app.getString(R.string.collection_play)).performScrollTo().assertIsDisplayed() + } + + @Test fun signatureChangesKeepFileIdentityButOtherParametersDoNot() { + assertEquals(signed.resourceKey, fresh.resourceKey) + assertNotEquals(signed.resourceKey, signed.copy(url = signed.url + "&edition=2").resourceKey) + assertNotEquals(signed.resourceKey, signed.copy(url = signed.url.replace("a.epub", "b.epub")).resourceKey) + assertEquals(signed.copy(url = signed.url + "&b=2&a=1").resourceKey, + fresh.copy(url = fresh.url + "&a=1&b=2").resourceKey) + } +} diff --git a/i18n/de.json b/i18n/de.json index b1e83bf..f4d4c76 100644 --- a/i18n/de.json +++ b/i18n/de.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Zum Öffnen anmelden", "digital_locked_loan": "Für Leser, die dieses Buch ausgeliehen haben", "digital_locked_other": "Für dein Konto nicht verfügbar", - "digital_link_refreshed": "Link aktualisiert: tippe erneut, um ihn zu öffnen." + "digital_link_refresh_error": "Der Link konnte nicht aktualisiert werden. Bitte versuche es erneut." } diff --git a/i18n/en.json b/i18n/en.json index 64d8b52..2d6a501 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Sign in to open it", "digital_locked_loan": "For readers who have this book on loan", "digital_locked_other": "Not available to your account", - "digital_link_refreshed": "Link refreshed: tap again to open it." + "digital_link_refresh_error": "Could not refresh the link. Please try again." } diff --git a/i18n/fr.json b/i18n/fr.json index 402ee39..d248452 100644 --- a/i18n/fr.json +++ b/i18n/fr.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Connectez-vous pour l'ouvrir", "digital_locked_loan": "Pour les lecteurs qui ont emprunté ce livre", "digital_locked_other": "Non disponible pour votre compte", - "digital_link_refreshed": "Lien actualisé : touchez à nouveau pour l'ouvrir." + "digital_link_refresh_error": "Impossible d’actualiser le lien. Réessayez." } diff --git a/i18n/it.json b/i18n/it.json index 395eddc..4242c69 100644 --- a/i18n/it.json +++ b/i18n/it.json @@ -1446,5 +1446,5 @@ "digital_locked_login": "Accedi per aprirlo", "digital_locked_loan": "Per chi ha il libro in prestito", "digital_locked_other": "Non disponibile per il tuo account", - "digital_link_refreshed": "Collegamento aggiornato: tocca di nuovo per aprire." + "digital_link_refresh_error": "Impossibile aggiornare il collegamento. Riprova." } From 847cd72827e56ce75ac9579dcb042fdf559569e8 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 05:44:33 +0000 Subject: [PATCH 09/14] Open the received copy on its record even when the server leaves the record id out A receipt response without book_id, or a received offer whose book has id 0, fell back to staffBookEdit(0, copy), which opens the new-book form. Both now fall back to the title being received. With labels but no per-copy area the section still scrolls into view, but the copy editor stays closed. --- .../app/ui/screens/staff/content/StaffCopiesSection.kt | 3 ++- .../app/ui/screens/staff/plugins/StaffDesiderataScreen.kt | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt index 69ba810..9339117 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/content/StaffCopiesSection.kt @@ -250,7 +250,8 @@ fun StaffCopiesSection( val copy = state.copies.firstOrNull { it.id == openCopyId } ?: return@LaunchedEffect onCopyOpened() bringIntoView.bringIntoView() - vm.startEdit(copy) + // Labels alone show the section but not the per-copy editor: open it only with copies. + if (areas.copies) vm.startEdit(copy) } LaunchedEffect(state.forbidden) { if (state.forbidden) onForbidden() } val message = state.message?.resolve() diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt index 39eeb89..852978b 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt @@ -282,7 +282,8 @@ class StaffDesiderataViewModel @Inject constructor( when (res) { is ApiResult.Success -> it.copy( busy = false, message = StaffMessage(R.string.staff_wanted_received, args = listOf(book.title)), - openCopy = res.data.copyId?.takeIf { c -> c > 0 }?.let { c -> res.data.bookId to c }, + // A response without book_id still names the title being received. + openCopy = res.data.copyId?.takeIf { c -> c > 0 }?.let { c -> (res.data.bookId.takeIf { b -> b > 0 } ?: book.id) to c }, ) is ApiResult.Failure -> it.copy( busy = false, forbidden = res.isForbidden(), @@ -301,7 +302,7 @@ class StaffDesiderataViewModel @Inject constructor( /** The record and copy a received offer produced, when the server names both. */ private fun receivedCopy(offer: StaffOffer): Pair? { if (offer.status != "received") return null - val book = offer.receivedBook?.id ?: offer.book?.id ?: return null + val book = offer.receivedBook?.id?.takeIf { it > 0 } ?: offer.book?.id?.takeIf { it > 0 } ?: return null val copy = offer.copyId?.takeIf { it > 0 } ?: return null return book to copy } From 2784ecd3ae9d0475c5bb5443cf8f7f984429cd38 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 05:45:27 +0000 Subject: [PATCH 10/14] Keep a playing audiobook when another file of the record is renewed Renewing any file swaps in a record whose URLs are all re-signed, so the player, matched to its card by the full signed URL, left composition and stopped. The started file is now remembered and matched by resourceKey, and the player keeps the URL it started with. --- .../app/ui/screens/detail/BookDetailScreen.kt | 19 +++++++++++--- .../com/pinakes/app/PlayingAudioUrlTest.kt | 25 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) create mode 100644 app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 1c98c4a..72923f6 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -358,12 +358,14 @@ internal fun DetailContent( var refreshingFile by remember(book.id) { mutableStateOf(null) } var showCover by remember { mutableStateOf(false) } var selectedPdf by remember(book.id) { mutableStateOf(null) } - var selectedAudio by remember(book.id) { mutableStateOf(null) } + // The audio file the reader started: renewing any file re-signs every URL, so the card + // is matched by resourceKey and the player keeps the URL it started with. + var selectedAudio by remember(book.id) { mutableStateOf(null) } fun openFile(file: DigitalAttachment) { if (file.locked || file.safeUrl == null) { onShowMessage(context.getString(R.string.digital_locked_other)) - } else if (file.kind == "audio") selectedAudio = file.url + } else if (file.kind == "audio") selectedAudio = file else if (file.isPdf) selectedPdf = file.url else runCatching { context.startActivity(Intent(Intent.ACTION_VIEW, Uri.parse(file.url))) } .onFailure { onShowMessage(context.getString(R.string.ebook_error)) } @@ -538,8 +540,9 @@ internal fun DetailContent( kind = stringResource(if (audio) R.string.book_section_audiobook else if (file.kind == "supplement") R.string.book_section_related_documents else R.string.book_section_ebook)) { if (file.locked) { LockedDigitalFile(file.access) - } else if (audio && selectedAudio == file.url) { - androidx.compose.runtime.key(file.url) { AudioPlayer(audioUrl = file.url) } + } else if (audio && playingAudioUrl(selectedAudio, file) != null) { + val playingUrl = playingAudioUrl(selectedAudio, file)!! + androidx.compose.runtime.key(playingUrl) { AudioPlayer(audioUrl = playingUrl) } } else PrimaryButton(label = stringResource(if (audio) R.string.collection_play else if (file.isPdf) R.string.book_read else R.string.ebook_open_external), onClick = { // The server's clock and access decision are authoritative. Always renew @@ -758,6 +761,14 @@ private fun SectionTitle(text: String) { ) } +/** + * The URL the player started with when [file] is the audio the reader chose, else null. + * Matched by [DigitalAttachment.resourceKey], so a renewal of another file, which re-signs + * every URL of the record, does not stop or restart a playing audiobook. + */ +internal fun playingAudioUrl(selected: DigitalAttachment?, file: DigitalAttachment): String? = + selected?.takeIf { it.resourceKey == file.resourceKey }?.url + private fun digitalFilename(url: String, fallback: String): String = Uri.parse(url).lastPathSegment?.takeIf { it.isNotBlank() } ?: fallback diff --git a/app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt b/app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt new file mode 100644 index 0000000..d67084f --- /dev/null +++ b/app/src/test/java/com/pinakes/app/PlayingAudioUrlTest.kt @@ -0,0 +1,25 @@ +package com.pinakes.app + +import com.pinakes.app.data.model.DigitalAttachment +import com.pinakes.app.ui.screens.detail.playingAudioUrl +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Test + +/** A playing audiobook keeps its player when another file of the record is renewed. */ +class PlayingAudioUrlTest { + private val started = DigitalAttachment("https://library.example/uploads/digital/book.mp3?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + private val other = DigitalAttachment("https://library.example/uploads/digital/book.epub?pk_exp=1000&pk_uid=3&pk_sig=old", access = "granted") + + /** Renewing the eBook re-signs the audio URL too; the card still plays the URL it started with. */ + @Test fun aRenewedSignatureKeepsTheStartedUrl() { + val resigned = started.copy(url = started.url.replace("1000", "4102444800").replace("old", "new")) + assertEquals(started.url, playingAudioUrl(started, resigned)) + } + + /** Another file, or no started audio, shows the play button instead of a player. */ + @Test fun otherFilesAndNoSelectionHaveNoPlayer() { + assertNull(playingAudioUrl(started, other)) + assertNull(playingAudioUrl(null, started)) + } +} From b2e5c39e50cd02a39d88ac9b80e1201c05cf7b79 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 05:45:52 +0000 Subject: [PATCH 11/14] Keep the 1.6.2 verification record at the 22 device tests it ran The merge of main had changed the historical 1.6.2 line and its repeat to 26. --- STATUS.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/STATUS.md b/STATUS.md index 652c72d..c41e9d0 100644 --- a/STATUS.md +++ b/STATUS.md @@ -32,8 +32,8 @@ The app now uses the 2026 web design: Geist / Fraunces, warm neutrals, a server- - Debug APK: `pinakes-debug.apk`, copied from `app/build/outputs/apk/debug/app-debug.apk` (generated locally and ignored by Git). - Package: `com.pinakes.app`, version `1.6.2` (19), minSdk 26, target/compileSdk 35. -- Local verification: **200 debug and 200 release unit tests, 26 instrumented tests on Android 15, zero lint errors (101 warnings)**, and successful debug + R8 release builds. The 1.6.2 debug APK installs over 1.6.1 with the same signing certificate. -- Verification commands: `assembleDebug`, `testDebugUnitTest`, `lintDebug`, `assembleRelease`, `assembleDebugAndroidTest`; the 26 device tests run through `am instrument` on a separate Android 15 AVD, preserving the authenticated demo device. +- Local verification: **200 debug and 200 release unit tests, 22 instrumented tests on Android 15, zero lint errors (101 warnings)**, and successful debug + R8 release builds. The 1.6.2 debug APK installs over 1.6.1 with the same signing certificate. +- Verification commands: `assembleDebug`, `testDebugUnitTest`, `lintDebug`, `assembleRelease`, `assembleDebugAndroidTest`; the 22 device tests run through `am instrument` on a separate Android 15 AVD, preserving the authenticated demo device. - Unit tests cover the existing contracts and theme contrast/mixing. Compose device tests cover full tall artwork, missing metadata, grid actions, view selection, digital-file cards, narrow search placeholders, circulation actions theme pairings, reachable empty-home actions and query preservation during loading. - Release builds exercise R8 and resource shrinking. Without release credentials the output is unsigned; no store release is published by this change. - macOS standalone emulator startup: `tools/run-emulator.sh` keeps a power assertion scoped to the emulator PID. A control with the identical debug APK reproduced a 21-second startup timeout at host priority 4; three starts with the assertion completed in 2.3–3.1 seconds. After a VM restart through the launcher, five more cold starts completed in 1.66–1.80 seconds with no ANR events. The original startup ANR also coincided with system/launcher stalls. An older AVD still stalled on a later cold boot, and a snapshot retained a system ANR dialog: the assertion does not repair an unhealthy AVD. No Sentry events are suppressed, and no production-device ANR fix is claimed. From 95f4db87cf8cad569821b24b49ce6f7da7bfbeac Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 09:52:20 +0200 Subject: [PATCH 12/14] Release 1.9.0 (versionCode 23) Bumps the version and records in STATUS what 1.9.0 brings: closed digital files with their reason and signed-link renewal (#50), the restyled loan request sheet (#46), the return date from the server's loan duration (#51) and the wanted-title receive action (#49). --- STATUS.md | 11 +++++++++++ app/build.gradle.kts | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/STATUS.md b/STATUS.md index c41e9d0..b1d7b46 100644 --- a/STATUS.md +++ b/STATUS.md @@ -1,5 +1,16 @@ # Pinakes Android — Build Status +## 1.9.0 (versionCode 23): digital file access, loan request sheet, wanted-title receipts + +Pairs with Pinakes 0.8.6 (Mobile API 1.8.1). Works with older servers: every new field is optional and its absence keeps the 1.8.0 behaviour. + +- **Digital files a reader cannot open yet** ([#50](https://github.com/fabiodalez-dev/Pinakes-Android/pull/50), Pinakes #480): a file the server closes (`url: null`, `access` = `login_required` or `loan_required`) keeps its card on the book page with a lock and the reason instead of the Read/Play button. Signed links last a day; a tap revalidates the file on the server and opens the renewed address, whatever the phone clock says, bypassing both caches and never falling back to an expired link. A playing audiobook keeps playing when another file of the record is renewed. Servers before 0.8.6 send neither field and every file stays open as before. +- **Loan request sheet** ([#46](https://github.com/fabiodalez-dev/Pinakes-Android/pull/46)): a plain dialog styled after the web's loan calendar instead of `DatePickerDialog`, which cut the month in half; free days are plain numbers, limited and unavailable days keep their tint, the fallback calendar (no availability) lets every future day be chosen and says the server checks the request. +- **Return date of a loan request** ([#51](https://github.com/fabiodalez-dev/Pinakes-Android/pull/51), Pinakes #485): with Mobile API 1.8.1 the sheet shows "Start · Return by" from the library's `loan_duration_days`; older servers get a hint about the library's standard loan period instead of "about 1 month". +- **Wanted titles** ([#49](https://github.com/fabiodalez-dev/Pinakes-Android/pull/49)): the row action reads "Receive" (it read as a status, "Received") and asks before adding the copy; after receiving a wanted title or an offer the app opens the record on the new copy's editor so its inventory code and place can be filled in. +- No change to the local cache database (Room version 3). +- Verification recorded on the PRs: 411 debug and 411 release unit tests for the four branches combined, `compileDebugAndroidTestKotlin`; the 6 `DigitalLinkRefreshFlowTest` and the `DesiderataStaffFlowTest` flows on an Android 15 emulator. This release branch is verified by Android CI. + ## 1.8.0 (versionCode 22): circulation desk, book tools and plugin content for staff Requires Mobile API 1.7.0. Every area is gated on its own `/health` flag (`staff_circulation`, `staff_copies`, `staff_series`, `staff_merges`, `staff_labels`, `staff_imports`, `staff_covers`, `staff_desiderata`, `staff_archives`) plus the staff role and `staff_catalog`; a server that answers `false` or omits a flag keeps the 1.7.0 staff area for that part. The 403 `forbidden` contract is unchanged: every new call goes through the shared `staffCall`, which drops the stored role. diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 70b3161..9461502 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -166,8 +166,8 @@ android { testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" minSdk = 26 targetSdk = 35 - versionCode = 22 - versionName = "1.8.0" + versionCode = 23 + versionName = "1.9.0" vectorDrawables { useSupportLibrary = true From 048ce7f7f59cd0920c180d8db7f90c29976b5e89 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 10:49:23 +0200 Subject: [PATCH 13/14] Pin the loan sheet's today in its test, match the copy code as the editor shows it LoanCalendar, LoanRequestSheet and LoanRequestSheetContent take an optional today (null reads the device clock once, as before), and LoanRequestSheetUiTest passes a fixed day, so a run that crosses midnight cannot move the calendar away from the data the test built. LoanRequestReturnDateTest reads the two hint strings from the resources instead of English literals. DesiderataStaffFlowTest waits for the copy code as the editor shows it ("Copy LIB-77-C901"); its comment no longer says the editor gives the copy its code, since the server assigns it and no client changes it afterwards. STATUS records the verification of this branch: 422 unit tests and 17 device tests. --- STATUS.md | 2 +- .../java/com/pinakes/app/LoanRequestSheetUiTest.kt | 7 +++++-- .../com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt | 5 +++-- .../com/pinakes/app/ui/screens/detail/BookDetailScreen.kt | 8 +++++++- .../com/pinakes/app/ui/screens/detail/LoanCalendar.kt | 5 ++++- .../java/com/pinakes/app/LoanRequestReturnDateTest.kt | 5 +++-- 6 files changed, 23 insertions(+), 9 deletions(-) diff --git a/STATUS.md b/STATUS.md index b1d7b46..5b1fe8c 100644 --- a/STATUS.md +++ b/STATUS.md @@ -9,7 +9,7 @@ Pairs with Pinakes 0.8.6 (Mobile API 1.8.1). Works with older servers: every new - **Return date of a loan request** ([#51](https://github.com/fabiodalez-dev/Pinakes-Android/pull/51), Pinakes #485): with Mobile API 1.8.1 the sheet shows "Start · Return by" from the library's `loan_duration_days`; older servers get a hint about the library's standard loan period instead of "about 1 month". - **Wanted titles** ([#49](https://github.com/fabiodalez-dev/Pinakes-Android/pull/49)): the row action reads "Receive" (it read as a status, "Received") and asks before adding the copy; after receiving a wanted title or an offer the app opens the record on the new copy's editor so its inventory code and place can be filled in. - No change to the local cache database (Room version 3). -- Verification recorded on the PRs: 411 debug and 411 release unit tests for the four branches combined, `compileDebugAndroidTestKotlin`; the 6 `DigitalLinkRefreshFlowTest` and the `DesiderataStaffFlowTest` flows on an Android 15 emulator. This release branch is verified by Android CI. +- Verification on this release branch: 422 debug unit tests with none failed or skipped, lint and `assembleDebug`, locally and in Android CI; 17 device tests on an Android 15 emulator (`LoanRequestSheetUiTest`, `CalendarLayoutTest`, `DesiderataStaffFlowTest`, `DigitalLinkRefreshFlowTest`), none failed. ## 1.8.0 (versionCode 22): circulation desk, book tools and plugin content for staff diff --git a/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt b/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt index 0dac80d..16087cd 100644 --- a/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt +++ b/app/src/androidTest/java/com/pinakes/app/LoanRequestSheetUiTest.kt @@ -20,7 +20,9 @@ import java.time.LocalDate class LoanRequestSheetUiTest { @get:Rule val compose = createComposeRule() - private val today = LocalDate.now() + // A fixed day, passed to the sheet as its today: a run that crosses midnight cannot move + // the device clock away from the calendar built here. + private val today = LocalDate.of(2030, 3, 4) /** Six weeks from today: mostly free, a partial day and a full one in the first week. */ private fun calendar(earliest: String? = today.plusDays(1).toString()) = AvailabilityCalendar( @@ -52,6 +54,7 @@ class LoanRequestSheetUiTest { submitting = submitting, onDismiss = onDismiss, onConfirm = onConfirm, + today = today, ) } } @@ -91,7 +94,7 @@ class LoanRequestSheetUiTest { compose.setContent { PinakesTheme { Box(Modifier.width(360.dp)) { - LoanRequestSheetContent(loading = false, calendar = null, fallback = true, submitting = false, onDismiss = {}, onConfirm = {}) + LoanRequestSheetContent(loading = false, calendar = null, fallback = true, submitting = false, onDismiss = {}, onConfirm = {}, today = today) } } } diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt index d2cfffd..c713879 100644 --- a/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt @@ -66,9 +66,10 @@ class DesiderataStaffFlowTest : AppFlowTest() { compose.onAllNodesWithText(text(R.string.staff_wanted_mark_received)).onLast().performClick() waitRequest("POST", "staff/desiderata/wanted/77/receive") assertEquals(1, api.recorded("POST", "staff/desiderata/wanted/77/receive").size) - // Received: the record opens on the new copy's editor, to give it its code and place. + // Received: the record opens on the new copy's editor, for its status and notes. The + // server gives the copy its code, which no client changes afterwards (web included). waitFor(hasTestTag("copy-save")) - waitText("LIB-77-C901") + waitText(text(R.string.staff_copy_code_value, "LIB-77-C901")) } @Test fun aClosedOfferSaysSo() { diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt index 5db286f..9218c57 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/BookDetailScreen.kt @@ -239,6 +239,8 @@ internal fun LoanRequestSheet( submitting: Boolean, onDismiss: () -> Unit, onConfirm: (isoDate: String) -> Unit, + /** Passed to [LoanCalendar]; null reads the device clock. */ + today: LocalDate? = null, ) { // Same sheet as the web's loan request: the card shape of the availability box (20dp), a // serif title, and two buttons stacked at full width in the page's own styles — the filled @@ -251,7 +253,7 @@ internal fun LoanRequestSheet( properties = DialogProperties(usePlatformDefaultWidth = false), ) { Box(Modifier.testTag("loan-dialog")) { - LoanRequestSheetContent(loading, calendar, fallback, submitting, onDismiss, onConfirm) + LoanRequestSheetContent(loading, calendar, fallback, submitting, onDismiss, onConfirm, today) } } } @@ -268,6 +270,8 @@ internal fun LoanRequestSheetContent( submitting: Boolean, onDismiss: () -> Unit, onConfirm: (isoDate: String) -> Unit, + /** Passed to [LoanCalendar]; null reads the device clock. */ + today: LocalDate? = null, ) { // Pre-select earliest_available when offered, else null until the user picks. var selectedIso by remember(calendar) { @@ -319,6 +323,7 @@ internal fun LoanRequestSheetContent( calendar = calendar, selected = selectedIso, onSelect = { selectedIso = it }, + today = today, ) } @@ -329,6 +334,7 @@ internal fun LoanRequestSheetContent( selected = selectedIso, onSelect = { selectedIso = it }, anyFutureDay = true, + today = today, ) Text( stringResource(R.string.book_loan_availability_error), diff --git a/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt b/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt index e6317ec..7bc1e0a 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/detail/LoanCalendar.kt @@ -134,9 +134,12 @@ fun LoanCalendar( modifier: Modifier = Modifier, /** Availability could not be loaded: every future day is selectable, no colour legend. */ anyFutureDay: Boolean = false, + /** The day the grid treats as today; null reads the device clock once. Tests pin it. */ + today: LocalDate? = null, ) { val dark = LocalPinakesColors.current.darkMode - val today = remember { LocalDate.now() } + val deviceToday = remember { LocalDate.now() } + val today = today ?: deviceToday val model = remember(calendar) { CalendarModel(calendar) } // Visible month: start on the selected date's month, else the earliest-available month, else now. diff --git a/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt b/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt index 46dadd0..45ab92d 100644 --- a/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt +++ b/app/src/test/java/com/pinakes/app/LoanRequestReturnDateTest.kt @@ -36,6 +36,7 @@ import org.robolectric.annotation.GraphicsMode @GraphicsMode(GraphicsMode.Mode.NATIVE) class LoanRequestReturnDateTest { private val compose = createComposeRule() + private val context get() = ApplicationProvider.getApplicationContext() @get:Rule val rules: RuleChain = RuleChain.outerRule(object : TestWatcher() { override fun starting(description: Description) { val app = ApplicationProvider.getApplicationContext() @@ -69,14 +70,14 @@ class LoanRequestReturnDateTest { /** With the duration, the dialog names the start and the return date of the preselected day. */ @Test fun dialogShowsTheReturnDateForThePreselectedStart() { show(AvailabilityCalendar(totalCopies = 1, earliestAvailable = "2030-03-04", loanDurationDays = 14)) - compose.onNodeWithText("Start: ${DateFormat.date("2030-03-04")} · Return by: ${DateFormat.date("2030-03-18")}") + compose.onNodeWithText(context.getString(R.string.book_loan_period_range, DateFormat.date("2030-03-04"), DateFormat.date("2030-03-18"))) .performScrollTo().assertIsDisplayed() } /** Without it, the dialog falls back to the library's standard loan period, with no date. */ @Test fun withoutTheDurationTheDialogNamesTheStandardPeriod() { show(AvailabilityCalendar(totalCopies = 1, earliestAvailable = "2030-03-04")) - compose.onNodeWithText("From ${DateFormat.date("2030-03-04")}, for the library’s standard loan period") + compose.onNodeWithText(context.getString(R.string.book_loan_period_hint, DateFormat.date("2030-03-04"))) .performScrollTo().assertIsDisplayed() } } From b26be1160a8fbeb08df01f9625c35f75ffb71aba Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 21 Oct 2026 11:09:25 +0200 Subject: [PATCH 14/14] Say what the received copy's editor is for: status, notes and label STATUS and three comments still said the app opens the received copy so its code and place can be filled in. For an existing copy the editor shows the code the server assigned and offers its status, notes and label; there is no place field. --- STATUS.md | 2 +- .../java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt | 2 +- .../com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt | 2 +- .../app/ui/screens/staff/plugins/StaffDesiderataScreen.kt | 3 ++- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/STATUS.md b/STATUS.md index 5b1fe8c..7acba30 100644 --- a/STATUS.md +++ b/STATUS.md @@ -7,7 +7,7 @@ Pairs with Pinakes 0.8.6 (Mobile API 1.8.1). Works with older servers: every new - **Digital files a reader cannot open yet** ([#50](https://github.com/fabiodalez-dev/Pinakes-Android/pull/50), Pinakes #480): a file the server closes (`url: null`, `access` = `login_required` or `loan_required`) keeps its card on the book page with a lock and the reason instead of the Read/Play button. Signed links last a day; a tap revalidates the file on the server and opens the renewed address, whatever the phone clock says, bypassing both caches and never falling back to an expired link. A playing audiobook keeps playing when another file of the record is renewed. Servers before 0.8.6 send neither field and every file stays open as before. - **Loan request sheet** ([#46](https://github.com/fabiodalez-dev/Pinakes-Android/pull/46)): a plain dialog styled after the web's loan calendar instead of `DatePickerDialog`, which cut the month in half; free days are plain numbers, limited and unavailable days keep their tint, the fallback calendar (no availability) lets every future day be chosen and says the server checks the request. - **Return date of a loan request** ([#51](https://github.com/fabiodalez-dev/Pinakes-Android/pull/51), Pinakes #485): with Mobile API 1.8.1 the sheet shows "Start · Return by" from the library's `loan_duration_days`; older servers get a hint about the library's standard loan period instead of "about 1 month". -- **Wanted titles** ([#49](https://github.com/fabiodalez-dev/Pinakes-Android/pull/49)): the row action reads "Receive" (it read as a status, "Received") and asks before adding the copy; after receiving a wanted title or an offer the app opens the record on the new copy's editor so its inventory code and place can be filled in. +- **Wanted titles** ([#49](https://github.com/fabiodalez-dev/Pinakes-Android/pull/49)): the row action reads "Receive" (it read as a status, "Received") and asks before adding the copy; after receiving a wanted title or an offer the app opens the record on the new copy's editor, which shows the code the server assigned and lets staff set its status and notes or print its label. - No change to the local cache database (Room version 3). - Verification on this release branch: 422 debug unit tests with none failed or skipped, lint and `assembleDebug`, locally and in Android CI; 17 device tests on an Android 15 emulator (`LoanRequestSheetUiTest`, `CalendarLayoutTest`, `DesiderataStaffFlowTest`, `DigitalLinkRefreshFlowTest`), none failed. diff --git a/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt b/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt index c713879..ef5910f 100644 --- a/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt +++ b/app/src/androidTest/java/com/pinakes/app/staffe2e/DesiderataStaffFlowTest.kt @@ -30,7 +30,7 @@ class DesiderataStaffFlowTest : AppFlowTest() { clickTag("offer-41") clickTag("offer-receive") assertEquals("{}", lastBody("POST", "staff/desiderata/offers/41/receive")) - // The new copy opens on the record, ready for its code and place. + // The new copy opens on the record, on its editor (status, notes, label). waitFor(hasTestTag("copy-save")) } diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt index f441eec..cb69076 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/StaffBookEditViewModel.kt @@ -121,7 +121,7 @@ class StaffBookEditViewModel @Inject constructor( private var bookId: Int = savedStateHandle.get(Routes.ARG_BOOK_ID) ?: 0 private val isbnArg: String = savedStateHandle.get(Routes.ARG_ISBN).orEmpty() - /** The copy to open on arrival (a copy just received, waiting for its code and place); 0 = none. */ + /** The copy to open on arrival (a copy just received, for its status, notes and label); 0 = none. */ var openCopyId: Int = savedStateHandle.get(Routes.ARG_COPY_ID) ?: 0 private set diff --git a/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt b/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt index 852978b..018d9f2 100644 --- a/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt +++ b/app/src/main/java/com/pinakes/app/ui/screens/staff/plugins/StaffDesiderataScreen.kt @@ -332,7 +332,8 @@ fun StaffDesiderataScreen( ForbiddenExit(state.forbidden, onNavigateUp) LeaveWhen(!areas.desiderata, onNavigateUp) StaffSnackbarEffect(state.message, snackbar, vm::consumeMessage) - // A receipt made a copy: go straight to it, to give it its code and place. + // A receipt made a copy: go straight to it, for its status, notes and label (the server + // assigned its code). LaunchedEffect(state.openCopy) { state.openCopy?.let { (book, copy) -> vm.consumeOpenCopy(); onOpenCopy(book, copy) } } Scaffold(