Skip to content

Commit 10f82b5

Browse files
adamdickmeissadamdickmeiss
committed
CROSSLINK-302 Clean up supplier NCIP requests
Cancel RequestItem before ask-retry, cannot-supply, and accept-cancel when a supplier request exists. Compensate when saving the item after RequestItem fails. Leave CheckOutItem/CheckInItem and manual termination flows unchanged.
1 parent fcc2a1a commit 10f82b5

2 files changed

Lines changed: 93 additions & 17 deletions

File tree

‎broker/patron_request/service/action.go‎

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -414,17 +414,17 @@ func (a *PatronRequestActionService) handleLenderAction(ctx common.ExtendedConte
414414
case LenderActionRejectCancel:
415415
return a.rejectCancelLenderRequest(ctx, pr)
416416
case LenderActionCannotSupply:
417-
return a.cannotSupplyLenderRequest(ctx, pr, params)
417+
return a.cannotSupplyLenderRequest(ctx, pr, lms, illRequest, params)
418418
case LenderActionAddCondition:
419419
return a.addConditionsLenderRequest(ctx, pr, params)
420420
case LenderActionShip:
421421
return a.shipLenderRequest(ctx, pr, lms, illRequest, params)
422422
case LenderActionMarkReceived:
423423
return a.markReceivedLenderRequest(ctx, pr, lms)
424424
case LenderActionAcceptCancel:
425-
return a.acceptCancelLenderRequest(ctx, pr)
425+
return a.acceptCancelLenderRequest(ctx, pr, lms, illRequest)
426426
case LenderActionAskRetry:
427-
return a.askRetryLenderRequest(ctx, pr, params)
427+
return a.askRetryLenderRequest(ctx, pr, lms, illRequest, params)
428428
case LenderActionSendNotification:
429429
return a.sendNotificationLenderRequest(ctx, pr, params)
430430
default:
@@ -738,6 +738,9 @@ func (a *PatronRequestActionService) willSupplyLenderRequest(ctx common.Extended
738738
Barcode: itemBarcode,
739739
})
740740
if err != nil {
741+
if cancelErr := lmsAdapter.CancelRequestItem(requestId, userId); cancelErr != nil {
742+
err = errors.Join(err, fmt.Errorf("LMS CancelRequestItem compensation failed: %w", cancelErr))
743+
}
741744
status, result := logActionErrorAndReturnResult(ctx, "failed to save item", err)
742745
return actionExecutionResult{status: status, result: result, pr: pr}
743746
}
@@ -755,7 +758,24 @@ func (a *PatronRequestActionService) willSupplyLenderRequest(ctx common.Extended
755758
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
756759
}
757760

758-
func (a *PatronRequestActionService) cannotSupplyLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, params actionParams) actionExecutionResult {
761+
func (a *PatronRequestActionService) cancelLenderRequestItem(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request) error {
762+
items, err := a.prRepo.GetItemsByPrId(ctx, pr.ID)
763+
if err != nil {
764+
return fmt.Errorf("failed to get items: %w", err)
765+
}
766+
if len(items) == 0 {
767+
return nil
768+
}
769+
requestId := illRequest.Header.RequestingAgencyRequestId
770+
userId := lmsAdapter.InstitutionalPatron(pr.RequesterSymbol.String)
771+
return lmsAdapter.CancelRequestItem(requestId, userId)
772+
}
773+
774+
func (a *PatronRequestActionService) cannotSupplyLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request, params actionParams) actionExecutionResult {
775+
if err := a.cancelLenderRequestItem(ctx, pr, lmsAdapter, illRequest); err != nil {
776+
status, result := logActionErrorAndReturnResult(ctx, "LMS CancelRequestItem failed", err)
777+
return actionExecutionResult{status: status, result: result, pr: pr}
778+
}
759779
result := events.EventResult{}
760780
var reasonUnfilled *iso18626.TypeSchemeValuePair
761781
if params.ReasonUnfilled != "" {
@@ -935,7 +955,11 @@ func (a *PatronRequestActionService) rejectCancelLenderRequest(ctx common.Extend
935955
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
936956
}
937957

938-
func (a *PatronRequestActionService) acceptCancelLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) actionExecutionResult {
958+
func (a *PatronRequestActionService) acceptCancelLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request) actionExecutionResult {
959+
if err := a.cancelLenderRequestItem(ctx, pr, lmsAdapter, illRequest); err != nil {
960+
status, result := logActionErrorAndReturnResult(ctx, "LMS CancelRequestItem failed", err)
961+
return actionExecutionResult{status: status, result: result, pr: pr}
962+
}
939963
yes := iso18626.TypeYesNoY
940964
result := events.EventResult{}
941965
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,
@@ -948,7 +972,7 @@ func (a *PatronRequestActionService) acceptCancelLenderRequest(ctx common.Extend
948972
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
949973
}
950974

951-
func (a *PatronRequestActionService) askRetryLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, params actionParams) actionExecutionResult {
975+
func (a *PatronRequestActionService) askRetryLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request, params actionParams) actionExecutionResult {
952976
var deliveryInfo *iso18626.DeliveryInfo
953977
switch params.ReasonRetry {
954978
case "":
@@ -966,6 +990,10 @@ func (a *PatronRequestActionService) askRetryLenderRequest(ctx common.ExtendedCo
966990
status, result := logActionErrorAndReturnResult(ctx, fmt.Sprintf("unsupported reasonRetry %q for ask-retry action (supported: %q)", params.ReasonRetry, iso18626.ReasonRetryNotFoundAsCited), nil)
967991
return actionExecutionResult{status: status, result: result, pr: pr}
968992
}
993+
if err := a.cancelLenderRequestItem(ctx, pr, lmsAdapter, illRequest); err != nil {
994+
status, result := logActionErrorAndReturnResult(ctx, "LMS CancelRequestItem failed", err)
995+
return actionExecutionResult{status: status, result: result, pr: pr}
996+
}
969997
reasonRetry := iso18626.TypeSchemeValuePair{Text: params.ReasonRetry}
970998
result := events.EventResult{}
971999
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,

‎broker/patron_request/service/action_test.go‎

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,10 +1092,13 @@ func TestHandleInvokeLenderActionWillSupplyNcipFailed(t *testing.T) {
10921092
func TestHandleInvokeLenderActionWillSupplySaveItemFailed(t *testing.T) {
10931093
mockPrRepo := new(MockPrRepo)
10941094
lmsCreator := new(MockLmsCreator)
1095-
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lms.CreateLmsAdapterMockOK(), nil)
1095+
lmsAdapter := new(mockLmsAdapter)
1096+
lmsAdapter.On("RequestItem", "req-1", "", "", "", "").Return("item-1", "", "", nil)
1097+
lmsAdapter.On("CancelRequestItem", "req-1", "").Return(nil)
1098+
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lmsAdapter, nil)
10961099
mockIso18626Handler := new(MockIso18626Handler)
10971100
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
1098-
illRequest := iso18626.Request{}
1101+
illRequest := iso18626.Request{Header: iso18626.Header{RequestingAgencyRequestId: "req-1"}}
10991102
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
11001103
mockPrRepo.saveItemFail = true
11011104
mockPrRepo.On("GetPatronRequestByIdForUpdate", patronRequestId).Return(pr_db.PatronRequest{RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:x"}, State: BorrowerStateNew, Side: SideBorrowing, Tenant: pgtype.Text{Valid: true, String: "testlib"}, IllRequest: illRequest}, nil)
@@ -1104,16 +1107,20 @@ func TestHandleInvokeLenderActionWillSupplySaveItemFailed(t *testing.T) {
11041107

11051108
assert.Equal(t, events.EventStatusError, status)
11061109
assert.Equal(t, "failed to save item", resultData.EventError.Message)
1110+
lmsAdapter.AssertExpectations(t)
11071111
}
11081112

11091113
func TestHandleInvokeLenderActionCannotSupply(t *testing.T) {
11101114
mockPrRepo := new(MockPrRepo)
11111115
lmsCreator := new(MockLmsCreator)
1112-
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lms.CreateLmsAdapterMockOK(), nil)
1116+
lmsAdapter := new(mockLmsAdapter)
1117+
lmsAdapter.On("CancelRequestItem", "req-1", "").Return(nil)
1118+
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lmsAdapter, nil)
11131119
mockIso18626Handler := new(MockIso18626Handler)
11141120
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
1115-
illRequest := iso18626.Request{}
1116-
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1121+
illRequest := iso18626.Request{Header: iso18626.Header{RequestingAgencyRequestId: "req-1"}}
1122+
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1123+
mockPrRepo.On("GetItemsByPrId", patronRequestId).Return([]pr_db.Item{{Barcode: "item-1"}}, nil)
11171124
action := LenderActionCannotSupply
11181125
status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{
11191126
CommonEventData: events.CommonEventData{Action: &action},
@@ -1122,6 +1129,7 @@ func TestHandleInvokeLenderActionCannotSupply(t *testing.T) {
11221129
assert.Equal(t, events.EventStatusSuccess, status)
11231130
assert.NotNil(t, resultData)
11241131
assert.Equal(t, LenderStateUnfilled, mockPrRepo.savedPr.State)
1132+
lmsAdapter.AssertExpectations(t)
11251133

11261134
if assert.NotNil(t, mockIso18626Handler.lastSupplyingAgencyMessage) {
11271135
assert.Equal(t, iso18626.TypeStatusUnfilled, mockIso18626Handler.lastSupplyingAgencyMessage.StatusInfo.Status)
@@ -1139,7 +1147,8 @@ func TestHandleInvokeLenderActionCannotSupplyWithReason(t *testing.T) {
11391147
mockIso18626Handler := new(MockIso18626Handler)
11401148
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
11411149
illRequest := iso18626.Request{}
1142-
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1150+
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1151+
mockPrRepo.On("GetItemsByPrId", patronRequestId).Return([]pr_db.Item{}, nil)
11431152
action := LenderActionCannotSupply
11441153
status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{
11451154
CommonEventData: events.CommonEventData{Action: &action},
@@ -1164,6 +1173,31 @@ func TestHandleInvokeLenderActionCannotSupplyWithReason(t *testing.T) {
11641173
}
11651174
}
11661175

1176+
func TestHandleInvokeLenderActionCannotSupplyCancelRequestItemFailed(t *testing.T) {
1177+
mockPrRepo := new(MockPrRepo)
1178+
lmsCreator := new(MockLmsCreator)
1179+
lmsAdapter := new(mockLmsAdapter)
1180+
lmsAdapter.On("CancelRequestItem", "req-1", "").Return(errors.New("cancel failed"))
1181+
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lmsAdapter, nil)
1182+
mockIso18626Handler := new(MockIso18626Handler)
1183+
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
1184+
illRequest := iso18626.Request{Header: iso18626.Header{RequestingAgencyRequestId: "req-1"}}
1185+
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1186+
mockPrRepo.On("GetItemsByPrId", patronRequestId).Return([]pr_db.Item{{Barcode: "item-1"}}, nil)
1187+
action := LenderActionCannotSupply
1188+
1189+
status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{
1190+
CommonEventData: events.CommonEventData{Action: &action},
1191+
}})
1192+
1193+
assert.Equal(t, events.EventStatusError, status)
1194+
assert.Equal(t, "LMS CancelRequestItem failed", resultData.EventError.Message)
1195+
assert.Nil(t, mockIso18626Handler.lastSupplyingAgencyMessage)
1196+
assert.Equal(t, LenderStateValidated, mockPrRepo.savedPr.State)
1197+
assert.True(t, mockPrRepo.savedPr.NeedsAttention)
1198+
lmsAdapter.AssertExpectations(t)
1199+
}
1200+
11671201
func TestHandleInvokeLenderActionAddConditionOK(t *testing.T) {
11681202
mockPrRepo := new(MockPrRepo)
11691203
lmsCreator := new(MockLmsCreator)
@@ -1272,11 +1306,14 @@ func TestHandleInvokeLenderActionAskRetryMissingReasonRetry(t *testing.T) {
12721306
func TestHandleInvokeLenderActionAskRetryFull(t *testing.T) {
12731307
mockPrRepo := new(MockPrRepo)
12741308
lmsCreator := new(MockLmsCreator)
1275-
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lms.CreateLmsAdapterMockOK(), nil)
1309+
lmsAdapter := new(mockLmsAdapter)
1310+
lmsAdapter.On("CancelRequestItem", "req-1", "").Return(nil)
1311+
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lmsAdapter, nil)
12761312
mockIso18626Handler := new(MockIso18626Handler)
12771313
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
1278-
illRequest := iso18626.Request{}
1279-
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1314+
illRequest := iso18626.Request{Header: iso18626.Header{RequestingAgencyRequestId: "req-1"}}
1315+
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil)
1316+
mockPrRepo.On("GetItemsByPrId", patronRequestId).Return([]pr_db.Item{{Barcode: "item-1"}}, nil)
12801317
action := LenderActionAskRetry
12811318

12821319
status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{
@@ -1290,6 +1327,7 @@ func TestHandleInvokeLenderActionAskRetryFull(t *testing.T) {
12901327
assert.Equal(t, events.EventStatusSuccess, status)
12911328
assert.NotNil(t, resultData)
12921329
assert.Equal(t, LenderStateCompletedWithRetry, mockPrRepo.savedPr.State)
1330+
lmsAdapter.AssertExpectations(t)
12931331

12941332
if assert.NotNil(t, mockIso18626Handler.lastSupplyingAgencyMessage) {
12951333
assert.Equal(t, iso18626.TypeStatusRetryPossible, mockIso18626Handler.lastSupplyingAgencyMessage.StatusInfo.Status)
@@ -1704,18 +1742,22 @@ func TestHandleInvokeLenderActionMarkReceivedLmsFailed(t *testing.T) {
17041742
func TestHandleInvokeLenderActionAcceptCancel(t *testing.T) {
17051743
mockPrRepo := new(MockPrRepo)
17061744
lmsCreator := new(MockLmsCreator)
1707-
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lms.CreateLmsAdapterMockOK(), nil)
1745+
lmsAdapter := new(mockLmsAdapter)
1746+
lmsAdapter.On("CancelRequestItem", "req-1", "").Return(nil)
1747+
lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lmsAdapter, nil)
17081748
mockIso18626Handler := new(MockIso18626Handler)
17091749
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
1710-
illRequest := iso18626.Request{}
1750+
illRequest := iso18626.Request{Header: iso18626.Header{RequestingAgencyRequestId: "req-1"}}
17111751
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateCancelRequested, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1"), RequesterReqID: getDbText("req-1")}, nil)
1752+
mockPrRepo.On("GetItemsByPrId", patronRequestId).Return([]pr_db.Item{{Barcode: "item-1"}}, nil)
17121753
action := LenderActionAcceptCancel
17131754

17141755
status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{CommonEventData: events.CommonEventData{Action: &action}}})
17151756

17161757
assert.Equal(t, events.EventStatusSuccess, status)
17171758
assert.NotNil(t, resultData)
17181759
assert.Equal(t, LenderStateCancelled, mockPrRepo.savedPr.State)
1760+
lmsAdapter.AssertExpectations(t)
17191761
assert.NotNil(t, mockIso18626Handler.lastSupplyingAgencyMessage)
17201762
assert.Equal(t, iso18626.TypeReasonForMessageCancelResponse, mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.ReasonForMessage)
17211763
assert.Equal(t, iso18626.TypeStatusCancelled, mockIso18626Handler.lastSupplyingAgencyMessage.StatusInfo.Status)
@@ -1733,6 +1775,7 @@ func TestHandleInvokeLenderActionAcceptCancelMissingRequesterSymbol(t *testing.T
17331775
prAction := CreatePatronRequestActionService(mockPrRepo, new(IllRepoMock), *new(events.EventBus), mockIso18626Handler, lmsCreator, new(EmailSenderMock))
17341776
illRequest := iso18626.Request{}
17351777
mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{ID: patronRequestId, IllRequest: illRequest, State: LenderStateCancelRequested, Side: SideLending, RequesterSymbol: pgtype.Text{Valid: false, String: ""}, SupplierSymbol: getDbText("ISIL:SUP1")}, nil)
1778+
mockPrRepo.On("GetItemsByPrId", patronRequestId).Return([]pr_db.Item{}, nil)
17361779
mockPrRepo.On("GetPatronRequestByIdForUpdate", patronRequestId).Return(pr_db.PatronRequest{RequesterSymbol: pgtype.Text{Valid: true, String: "ISIL:x"}, State: BorrowerStateNew, Side: SideBorrowing, Tenant: pgtype.Text{Valid: true, String: "testlib"}, IllRequest: illRequest}, nil)
17371780
action := LenderActionAcceptCancel
17381781

@@ -2930,6 +2973,11 @@ type mockLmsAdapter struct {
29302973
lms.LmsAdapterManual
29312974
}
29322975

2976+
func (m *mockLmsAdapter) CancelRequestItem(requestId string, userId string) error {
2977+
args := m.Called(requestId, userId)
2978+
return args.Error(0)
2979+
}
2980+
29332981
func (m *mockLmsAdapter) CheckOutItem(
29342982
requestId string,
29352983
itemBarcode string,

0 commit comments

Comments
 (0)