Skip to content

Receive a wanted title with a clear action, then open its new copy - #49

Open
fabiodalez-dev wants to merge 1 commit into
mainfrom
fix/wanted-receive-button
Open

fabiodalez-dev wants to merge 1 commit into
mainfrom
fix/wanted-receive-button

Conversation

@fabiodalez-dev

Copy link
Copy Markdown
Owner

Two changes to the donation offers in the admin area.

The wanted-title button is an action, not a status. In "Titoli cercati" each row had a trailing text button labelled "Ricevuto" (Received). It read as the title's state, so a book still wanted with 0 copies looked already received. One tap also added a copy straight away. The button now says "Ricevi" / "Receive" / "Empfangen" / "Recevoir", and it asks for confirmation before adding the copy.

After a receipt the new copy opens on the record. Receiving a wanted title or an offer creates a copy that still needs its inventory code and place. The app now opens the book record, scrolls to the copies and opens that copy's editor. The book editor route takes an optional copy argument for this.

Tests: DesiderataStaffFlowTest covers the confirm dialog (cancel sends nothing, confirm sends one request) and the jump to the new copy's editor for a wanted title, an offer for a wanted title and an offer received into a picked record. Full suites on the emulator: 93 instrumented and 389 unit tests pass.

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.
@coderabbitai

coderabbitai Bot commented Oct 14, 2026

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 1854b8a0-89e8-4907-acb3-271324b1a738
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ℹ️ No critical issues — two rough edges in the new jump-to-copy flow, inline.

Reviewed changes

Reviewed the single commit that turns the wanted-title button into a confirmed action and routes every receipt to the new copy's editor.

  • Confirm before receiving a wanted title — the row button now reads as an action (staff_wanted_mark_received relabelled in all four locales), and askReceiveWanted / receiveWanted go through a ConfirmDialog.
  • openCopy hand-off — a successful wanted or offer receipt sets DesiderataUiState.openCopy, which the screen consumes and passes to the new onOpenCopy callback.
  • Optional copy route argument — STAFF_BOOK_EDIT gains ?copy={copyId} (default 0); existing staffBookEdit(id) callers still resolve to the same destination.
  • Auto-open in StaffCopiesSection — once the copy shows up in state.copies, the section scrolls into view and calls startEdit. StaffBookEditViewModel.openCopyId is cleared on first use, so recompositions and rotations don't reopen it.
  • Device tests — cancel sends nothing, confirm sends one request, and all three receipt paths land on copy-save.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using claude-opus-5-5 | 𝕏

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 },

/** 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

// 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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant