Just one function to get items for patron request

[adamdickmeiss]

Both sides.

[Copilot]

Pull request overview

This PR consolidates patron-request item retrieval into a single helper used by both borrowing- and lending-side actions, and updates tests to match the new error strings.

Changes:

• Replace separate item-fetching helpers with a shared getItems method in the patron request action service.
• Update action handlers to call the shared getItems method.
• Adjust unit test expectations for updated error/cause messages.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
broker/patron_request/service/action.go	Unifies item lookup logic via getItems and updates borrowing/lending action paths to use it.
broker/patron_request/service/action_test.go	Updates assertions for revised error cause strings from the unified item lookup.

Comments suppressed due to low confidence (3)

broker/patron_request/service/action.go:686

• getItems logs "multiple items found... using the first one", but the function returns the full items slice unchanged and callers like shipLenderRequest also use all items (via encodeItemsNote(items)). This warning is misleading and can create unnecessary noise in logs; either actually truncate to items[:1] (if only one is supported) or adjust/remove the warning and let the specific callers decide what to do with multiple items.

	if len(items) > 1 {
		ctx.Logger().Warn("multiple items found for patron request, using the first one", "itemCount", len(items))
	}
	return items, nil

broker/patron_request/service/action.go:685

• getItems (and several call sites) rely on items[0], but GetItemsByPrId does not specify an ORDER BY (see broker/sqlc/pr_query.sql), so the "first" item is nondeterministic when multiple rows exist. To make behavior stable, sort the returned slice (e.g., by created_at/id) before using index 0, or update the query to order deterministically.

func (a *PatronRequestActionService) getItems(ctx common.ExtendedContext, pr pr_db.PatronRequest) ([]pr_db.Item, error) {
	items, err := a.prRepo.GetItemsByPrId(ctx, pr.ID)
	if err != nil {
		return nil, fmt.Errorf("failed to get items: %w", err)
	}
	if len(items) == 0 {
		return nil, fmt.Errorf("no items found for patron request")
	}
	if len(items) > 1 {
		ctx.Logger().Warn("multiple items found for patron request, using the first one", "itemCount", len(items))
	}

broker/patron_request/service/action.go:685

• The warning for multiple items doesn't include the patron request identifier, which makes it hard to correlate in logs. Consider adding prId (and potentially the barcodes/count) as structured fields on the warning log entry.

		ctx.Logger().Warn("multiple items found for patron request, using the first one", "itemCount", len(items))
	}