lms requestItem returns itemBarcode, callNumber by adamdickmeiss · Pull Request #436 · indexdata/crosslink

adamdickmeiss · 2026-03-06T11:28:45Z

Logging the values. Not storing them yet.

Copilot

Pull request overview

Updates the LMS RequestItem flow to return additional metadata (item barcode + call number) from NCIP, and surfaces those values in the patron-request lender “will supply” action (currently logged, not persisted).

Changes:

Change LmsAdapter.RequestItem to return (itemBarcode, callNumber, error) and update adapter implementations/mocks accordingly.
Parse barcode/call number from the NCIP RequestItemResponse and extend unit tests to assert the returned values.
Log the RequestItem response values in willSupplyLenderRequest and rename CheckOutItem’s parameter from itemId to itemBarcode in the interface and implementations.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
broker/patron_request/service/action.go	Captures `RequestItem` return values; logs them; adjusts ship flow to use renamed `itemBarcode` parameter.
broker/patron_request/service/action_test.go	Updates failing LMS adapter mock to match new `RequestItem` return signature.
broker/lms/lms_adapter.go	Updates `LmsAdapter` interface signatures (`RequestItem` returns values; `CheckOutItem` param renamed).
broker/lms/lms_adapter_ncip.go	Implements parsing/returning barcode + call number from NCIP `RequestItemResponse`; renames `CheckOutItem` param.
broker/lms/lms_adapter_manual.go	Updates manual adapter stubs to satisfy new interface signatures.
broker/lms/lms_adapter_test.go	Extends tests/mocks to validate the new `RequestItem` return values.

Copilot · 2026-03-06T11:35:49Z

broker/patron_request/service/action.go

+	itemBarcode := illRequest.BibliographicInfo.SupplierUniqueRecordId // TODO: read from item table
 	requestId := illRequest.Header.RequestingAgencyRequestId
 	userId := lmsAdapter.InstitutionalPatron(pr.RequesterSymbol.String)
-	// TODO set these values properly
 	externalReferenceValue := ""
-	err := lmsAdapter.CheckOutItem(requestId, itemId, userId, externalReferenceValue)
+	err := lmsAdapter.CheckOutItem(requestId, itemBarcode, userId, externalReferenceValue)


itemBarcode is assigned from SupplierUniqueRecordId, but that field is also used as a bibliographic/record identifier elsewhere (e.g., RequestItem input). Renaming it to itemBarcode here is misleading until the actual barcode is retrieved from storage; consider keeping itemId (or similar) until the item table integration is done, to avoid accidental misuse.

Copilot · 2026-03-06T11:35:49Z

broker/lms/lms_adapter_ncip.go

+	response, err := l.ncipClient.RequestItem(arg)
+	if err != nil {
+		return "", "", err
+	}
+	barCode := ""
+	callNumber := ""
+	if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
+		barCode = response.ItemOptionalFields.ItemDescription.CopyNumber


response is dereferenced without checking for nil. Since the NcipClient interface allows returning (nil, nil), this can panic if an implementation (or future mock) returns a nil response with no error. Consider explicitly handling response == nil as an error (or returning empty values).

Copilot · 2026-03-06T11:35:49Z

broker/lms/lms_adapter_ncip.go

+	if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
+		barCode = response.ItemOptionalFields.ItemDescription.CopyNumber


The code treats ItemDescription.CopyNumber as the item barcode. In NCIP responses the item identifier is typically available via RequestItemResponse.ItemId.ItemIdentifierValue; CopyNumber is usually a copy/enumeration value and may not be a barcode. Consider preferring response.ItemId when present and using CopyNumber only as a fallback (if at all).

Suggested change

if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {

barCode = response.ItemOptionalFields.ItemDescription.CopyNumber

// Prefer the primary item identifier from ItemId when available.

if response.ItemId != nil && response.ItemId.ItemIdentifierValue != "" {

barCode = response.ItemId.ItemIdentifierValue

}

if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {

// Use CopyNumber only as a fallback if no ItemId barcode is present.

if barCode == "" {

barCode = response.ItemOptionalFields.ItemDescription.CopyNumber

}

Copilot · 2026-03-06T11:35:50Z

broker/lms/lms_adapter_test.go

+	barCode, callNumber, err := ad.RequestItem("req1", "item1", "testuser", "pickloc", itemLocation)
 	assert.NoError(t, err)
+	assert.Equal(t, "123.456", barCode)


Go naming: barCode should be barcode (initialisms/words are typically lowercased in mixedCase). Renaming will improve readability and consistency across the codebase, especially now that this value is part of the method contract.

Suggested change

barCode, callNumber, err := ad.RequestItem("req1", "item1", "testuser", "pickloc", itemLocation)

assert.NoError(t, err)

assert.Equal(t, "123.456", barCode)

barcode, callNumber, err := ad.RequestItem("req1", "item1", "testuser", "pickloc", itemLocation)

assert.NoError(t, err)

assert.Equal(t, "123.456", barcode)

Copilot · 2026-03-06T11:35:50Z

broker/lms/lms_adapter_ncip.go

+	barCode := ""
+	callNumber := ""
+	if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
+		barCode = response.ItemOptionalFields.ItemDescription.CopyNumber
+		callNumber = response.ItemOptionalFields.ItemDescription.CallNumber
+	}
+	return barCode, callNumber, err


Go naming: barCode should be barcode (and similarly callNumber is fine). This local variable name is now part of the core RequestItem implementation, so keeping it idiomatic will help avoid spreading inconsistent casing to callers/tests.

Suggested change

barCode := ""

callNumber := ""

if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {

barCode = response.ItemOptionalFields.ItemDescription.CopyNumber

callNumber = response.ItemOptionalFields.ItemDescription.CallNumber

}

return barCode, callNumber, err

barcode := ""

callNumber := ""

if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {

barcode = response.ItemOptionalFields.ItemDescription.CopyNumber

callNumber = response.ItemOptionalFields.ItemDescription.CallNumber

}

return barcode, callNumber, err

Copilot · 2026-03-06T11:35:50Z

broker/patron_request/service/action.go

 		return actionExecutionResult{status: status, result: result, outcome: ActionOutcomeFailure, pr: pr}
 	}
+	// TODO: store barCode in item table and use it in CheckOutItem
+	ctx.Logger().Info("RequestItem response", "itemBarcode", itemBarcode, "callNumber", callNumber)


This adds an INFO log on the normal (successful) RequestItem path. In this service there are no other INFO logs for routine operations, and this may create noisy logs and potentially expose item identifiers in production. Consider removing this log, downgrading it to DEBUG, or attaching these values to the transaction/event result instead of stdout logs.

Suggested change

ctx.Logger().Info("RequestItem response", "itemBarcode", itemBarcode, "callNumber", callNumber)

jakub-id · 2026-03-06T12:03:17Z

broker/lms/lms_adapter_ncip.go

+	barCode := ""
+	callNumber := ""
+	if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
+		barCode = response.ItemOptionalFields.ItemDescription.CopyNumber


@adamdickmeiss this is wrong, FOLIO at least returns the barcode as:

<ns1:ItemId> <ns1:ItemIdentifierType ns1:Scheme="Scheme">Item Barcode</ns1:ItemIdentifierType> <ns1:ItemIdentifierValue>tl-b02</ns1:ItemIdentifierValue> </ns1:ItemId>

lms requestItem returns itemBarcode, callNumber

57d3997

Logging the values. Not storing them yet.

adamdickmeiss requested review from JanisSaldabols and jakub-id as code owners March 6, 2026 11:28

Copilot AI review requested due to automatic review settings March 6, 2026 11:28

Copilot started reviewing on behalf of adamdickmeiss March 6, 2026 11:29 View session

Copilot AI reviewed Mar 6, 2026

View reviewed changes

jakub-id requested changes Mar 6, 2026

View reviewed changes

adamdickmeiss added 2 commits March 6, 2026 15:23

get barcode from itemId

e2b1ffc

item save/load for requestitem, checkoutitem in supplier

1f1283c

adamdickmeiss closed this Mar 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lms requestItem returns itemBarcode, callNumber#436

lms requestItem returns itemBarcode, callNumber#436
adamdickmeiss wants to merge 3 commits intomainfrom
supplier-request-item-barcode

adamdickmeiss commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

Copilot AI Mar 6, 2026

Uh oh!

jakub-id Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

		if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
		barCode = response.ItemOptionalFields.ItemDescription.CopyNumber

-	if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
-		barCode = response.ItemOptionalFields.ItemDescription.CopyNumber
+	// Prefer the primary item identifier from ItemId when available.
+	if response.ItemId != nil && response.ItemId.ItemIdentifierValue != "" {
+		barCode = response.ItemId.ItemIdentifierValue
+	}
+	if response.ItemOptionalFields != nil && response.ItemOptionalFields.ItemDescription != nil {
+		// Use CopyNumber only as a fallback if no ItemId barcode is present.
+		if barCode == "" {
+			barCode = response.ItemOptionalFields.ItemDescription.CopyNumber
+		}

Conversation

adamdickmeiss commented Mar 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

jakub-id Mar 6, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants