Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/src/androidTest/assets/fixtures/desiderata_wanted.json
Original file line number Diff line number Diff line change
@@ -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}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{"data":{"book_id":77,"copy_id":901,"offer_id":55},"meta":{},"error":null}
Original file line number Diff line number Diff line change
@@ -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}
Original file line number Diff line number Diff line change
@@ -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}
Original file line number Diff line number Diff line change
@@ -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. */
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
7 changes: 5 additions & 2 deletions app/src/main/java/com/pinakes/app/ui/navigation/Routes.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,14 @@ class StaffBookEditViewModel @Inject constructor(
private var bookId: Int = savedStateHandle.get<Int>(Routes.ARG_BOOK_ID) ?: 0
private val isbnArg: String = savedStateHandle.get<String>(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<Int>(Routes.ARG_COPY_ID) ?: 0
private set

fun consumeOpenCopy() {
openCopyId = 0
}

private val _state = MutableStateFlow(StaffBookEditUiState(bookId = bookId))
val state: StateFlow<StaffBookEditUiState> = _state.asStateFlow()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<StaffCopy>,
Expand All @@ -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()
Expand All @@ -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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The section also renders when only areas.labels is on (line 240), but this effect doesn't check areas.copies. On a server with labels but no per-copy management, the receipt still opens CopyEditorSheet, which is UI the user otherwise can't reach, and saving it calls updateCopy on an area the server doesn't offer. Gating the auto-open on areas.copies keeps the jump consistent with the list.

Suggested change
val copy = state.copies.firstOrNull { it.id == openCopyId } ?: return@LaunchedEffect
val copy = state.copies.firstOrNull { it.id == openCopyId }?.takeIf { areas.copies } ?: 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Int, Int>? = null,
val pickingBookFor: StaffOffer? = null,
val busy: Boolean = false,
val message: StaffMessage? = null,
Expand Down Expand Up @@ -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))
}
}
Expand All @@ -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 },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WantedReceiveResult.bookId defaults to 0. If the server leaves out book_id but sends copy_id, this navigates to staffBookEdit(0, copy), and StaffBookEditViewModel.isEdit (bookId > 0) then opens the new-book form instead of the record. The title being received is already in book.id, so falling back to it removes the risk.

Suggested change
openCopy = res.data.copyId?.takeIf { c -> c > 0 }?.let { c -> res.data.bookId to c },
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(),
message = if (res.meta?.reason == "open_offers") StaffMessage(R.string.staff_wanted_open_offers) else res.toStaffMessage(R.string.staff_error_save),
Expand All @@ -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<Int, Int>? {
if (offer.status != "received") return null
val book = offer.receivedBook?.id ?: offer.book?.id ?: return null

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same 0 id problem as the wanted path. IdTitle.id defaults to 0, so a received_book object without an id never falls back to book and sends you to the new-book form. OfferSheet already guards this with (offer.receivedBook ?: offer.book)?.takeIf { it.id > 0 }, and reusing that shape keeps the two consistent.

Suggested change
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
}
}

internal fun offerStatusLabel(status: String): Int? = when (status) {
Expand All @@ -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()
Expand All @@ -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) },
Expand Down Expand Up @@ -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}"),
)
}
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 3 additions & 1 deletion i18n/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 3 additions & 1 deletion i18n/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading