diff --git a/broker/cmd/archive/main_test.go b/broker/cmd/archive/main_test.go index 3b392d25a..76bd893bd 100644 --- a/broker/cmd/archive/main_test.go +++ b/broker/cmd/archive/main_test.go @@ -25,7 +25,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/cmd/broker/main_test.go b/broker/cmd/broker/main_test.go index df2335e93..e75e4c097 100644 --- a/broker/cmd/broker/main_test.go +++ b/broker/cmd/broker/main_test.go @@ -31,7 +31,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/handler/iso18626-handler.go b/broker/handler/iso18626-handler.go index cd291c133..a42902d01 100644 --- a/broker/handler/iso18626-handler.go +++ b/broker/handler/iso18626-handler.go @@ -215,6 +215,17 @@ func handleRetryRequest(ctx common.ExtendedContext, request *iso18626.Request, r illTrans.Timestamp = timestamp _, err = repo.SaveIllTransaction(ctx, ill_db.SaveIllTransactionParams(illTrans)) + if err != nil { + return err + } + + // Keep the selected supplier's LocalID in sync with the updated SupplierUniqueRecordId. + // createRequestMessage overwrites BibliographicInfo.SupplierUniqueRecordId with LocalID, so + // without this update the stale LocalID from the original holdings lookup would be sent. + if newLocalId := request.BibliographicInfo.SupplierUniqueRecordId; newLocalId != "" { + selSup.LocalID = pgtype.Text{String: newLocalId, Valid: true} + _, err = repo.SaveLocatedSupplier(ctx, ill_db.SaveLocatedSupplierParams(selSup)) + } return err }) return id, err diff --git a/broker/migrations/046_add_patron_links.down.sql b/broker/migrations/046_add_patron_links.down.sql new file mode 100644 index 000000000..a71a109d3 --- /dev/null +++ b/broker/migrations/046_add_patron_links.down.sql @@ -0,0 +1,30 @@ +DROP VIEW IF EXISTS patron_request_search_view; +ALTER TABLE patron_request DROP COLUMN next_req_id; +ALTER TABLE patron_request DROP COLUMN prev_req_id; +ALTER TABLE patron_request DROP COLUMN retry_bib_info; + +CREATE VIEW patron_request_search_view AS +SELECT + pr.*, + EXISTS ( + SELECT 1 + FROM notification n + WHERE n.pr_id = pr.id + ) AS has_notification, + EXISTS ( + SELECT 1 + FROM notification n + WHERE n.pr_id = pr.id and cost is not null + ) AS has_cost, + (unread.unread_notifications_count > 0) AS has_unread_notification, + (pr.internal_note IS NOT NULL AND btrim(pr.internal_note) <> '') AS has_internal_note, + pr.ill_request -> 'serviceInfo' ->> 'serviceType' AS service_type, + pr.ill_request -> 'serviceInfo' -> 'serviceLevel' ->> '#text' AS service_level, + immutable_to_timestamp(pr.ill_request -> 'serviceInfo' ->> 'needBeforeDate') AS needed_at, + unread.unread_notifications_count AS unread_notifications_count +FROM patron_request pr +LEFT JOIN LATERAL ( + SELECT COUNT(*) AS unread_notifications_count + FROM notification n + WHERE n.pr_id = pr.id and n.acknowledged_at is null +) unread ON true; diff --git a/broker/migrations/046_add_patron_links.up.sql b/broker/migrations/046_add_patron_links.up.sql new file mode 100644 index 000000000..303b22c9c --- /dev/null +++ b/broker/migrations/046_add_patron_links.up.sql @@ -0,0 +1,31 @@ +ALTER TABLE patron_request ADD COLUMN next_req_id VARCHAR; +ALTER TABLE patron_request ADD COLUMN prev_req_id VARCHAR; +ALTER TABLE patron_request ADD COLUMN retry_bib_info JSONB; + +DROP VIEW IF EXISTS patron_request_search_view; + +CREATE VIEW patron_request_search_view AS +SELECT + pr.*, + EXISTS ( + SELECT 1 + FROM notification n + WHERE n.pr_id = pr.id + ) AS has_notification, + EXISTS ( + SELECT 1 + FROM notification n + WHERE n.pr_id = pr.id and cost is not null + ) AS has_cost, + (unread.unread_notifications_count > 0) AS has_unread_notification, + (pr.internal_note IS NOT NULL AND btrim(pr.internal_note) <> '') AS has_internal_note, + pr.ill_request -> 'serviceInfo' ->> 'serviceType' AS service_type, + pr.ill_request -> 'serviceInfo' -> 'serviceLevel' ->> '#text' AS service_level, + immutable_to_timestamp(pr.ill_request -> 'serviceInfo' ->> 'needBeforeDate') AS needed_at, + unread.unread_notifications_count AS unread_notifications_count +FROM patron_request pr +LEFT JOIN LATERAL ( + SELECT COUNT(*) AS unread_notifications_count + FROM notification n + WHERE n.pr_id = pr.id and n.acknowledged_at is null +) unread ON true; diff --git a/broker/oapi/open-api.yaml b/broker/oapi/open-api.yaml index bb2b4e8da..057421230 100644 --- a/broker/oapi/open-api.yaml +++ b/broker/oapi/open-api.yaml @@ -582,6 +582,15 @@ components: internalNote: type: string description: Staff-only internal note, local to this request and never shared with peers + nextReqId: + type: string + description: ID of the next patron request in the sequence + prevReqId: + type: string + description: ID of the previous patron request in the sequence + retryBibInfo: + type: object + description: Bibliographic information for the item to retry the request for when in retrying state required: - id - createdAt diff --git a/broker/patron_request/api/api-handler.go b/broker/patron_request/api/api-handler.go index 293d46965..f988d5292 100644 --- a/broker/patron_request/api/api-handler.go +++ b/broker/patron_request/api/api-handler.go @@ -964,6 +964,9 @@ func toApiPatronRequest(r *http.Request, request pr_db.PatronRequestSearchView) EventsLink: eventsLink, TerminalState: request.TerminalState, InternalNote: toString(request.InternalNote), + NextReqId: toString(request.NextReqID), + PrevReqId: toString(request.PrevReqID), + RetryBibInfo: bibInfoToMap(request.RetryBibInfo), } if request.UpdatedAt.Valid { pr.UpdatedAt = &request.UpdatedAt.Time @@ -982,6 +985,21 @@ func toString(text pgtype.Text) *string { return value } +func bibInfoToMap(info *iso18626.BibliographicInfo) *map[string]interface{} { + if info == nil { + return nil + } + b, err := json.Marshal(info) + if err != nil { + return nil + } + var m map[string]interface{} + if err := json.Unmarshal(b, &m); err != nil { + return nil + } + return &m +} + func (a *PatronRequestApiHandler) parseAndValidateIllRequest( ctx common.ExtendedContext, request *proapi.CreatePatronRequest, diff --git a/broker/patron_request/db/prcql.go b/broker/patron_request/db/prcql.go index af683042c..61d865e34 100644 --- a/broker/patron_request/db/prcql.go +++ b/broker/patron_request/db/prcql.go @@ -293,6 +293,9 @@ func (q *Queries) ListPatronRequestsCql(ctx context.Context, db DBTX, arg ListPa &i.PatronRequestSearchView.UpdatedAt, &i.PatronRequestSearchView.IllResponse, &i.PatronRequestSearchView.InternalNote, + &i.PatronRequestSearchView.NextReqID, + &i.PatronRequestSearchView.PrevReqID, + &i.PatronRequestSearchView.RetryBibInfo, &i.PatronRequestSearchView.HasNotification, &i.PatronRequestSearchView.HasCost, &i.PatronRequestSearchView.HasUnreadNotification, diff --git a/broker/patron_request/service/action.go b/broker/patron_request/service/action.go index cee48bfde..8763d1864 100644 --- a/broker/patron_request/service/action.go +++ b/broker/patron_request/service/action.go @@ -31,9 +31,10 @@ type PatronRequestActionService struct { } type actionExecutionResult struct { - status events.EventStatus - result *events.EventResult - pr pr_db.PatronRequest + status events.EventStatus + result *events.EventResult + pr pr_db.PatronRequest + retryPr pr_db.PatronRequest } type autoActionFailure struct { @@ -51,6 +52,8 @@ type actionParams struct { Cost *float64 `json:"cost,omitempty"` Currency string `json:"currency,omitempty"` ReasonUnfilled string `json:"reasonUnfilled,omitempty"` + ReasonRetry string `json:"reasonRetry,omitempty"` + ItemID string `json:"itemId,omitempty"` } func CreatePatronRequestActionService(prRepo pr_db.PrRepo, eventBus events.EventBus, iso18626Handler handler.Iso18626HandlerInterface, lmsCreator lms.LmsCreator) *PatronRequestActionService { @@ -186,12 +189,30 @@ func (a *PatronRequestActionService) finalizeActionExecution(ctx common.Extended stateChanged = true } - var err error - updatedPr, err = a.prRepo.UpdatePatronRequest(ctx, pr_db.UpdatePatronRequestParams(updatedPr)) + var retryPr pr_db.PatronRequest + err := a.prRepo.WithTxFunc(ctx, func(repo pr_db.PrRepo) error { + var err error + if execResult.retryPr.ID != "" { + retryPr, err = repo.CreatePatronRequest(ctx, pr_db.CreatePatronRequestParams(execResult.retryPr)) + if err != nil { + return fmt.Errorf("create retry patron request: %w", err) + } + } + updatedPr, err = repo.UpdatePatronRequest(ctx, pr_db.UpdatePatronRequestParams(updatedPr)) + if err != nil { + return fmt.Errorf("update patron request: %w", err) + } + return nil + }) if err != nil { - return logActionErrorAndReturnResult(ctx, "failed to update patron request", err) + return logActionErrorAndReturnResult(ctx, "failed to persist patron request", err) + } + if retryPr.ID != "" { + err := a.RunAutoActionsOnStateEntry(ctx, retryPr, &event.ID, event.EventData.User) + if err != nil { + return logActionErrorAndReturnResult(ctx, "failed to run auto actions on state entry", err) + } } - if stateChanged { err := a.RunAutoActionsOnStateEntry(ctx, updatedPr, &event.ID, event.EventData.User) if err != nil { @@ -314,6 +335,10 @@ func (a *PatronRequestActionService) handleBorrowingAction(ctx common.ExtendedCo return a.acceptConditionBorrowingRequest(ctx, pr) case BorrowerActionRejectCondition: return a.rejectConditionBorrowingRequest(ctx, pr) + case BorrowerActionRejectRetry: + return a.rejectRetryBorrowingRequest(pr) + case BorrowerActionAcceptRetry: + return a.acceptRetryBorrowingRequest(ctx, pr) default: status, result := logActionErrorAndReturnResult(ctx, "borrower action "+string(action)+" is not implemented yet", errors.New("invalid action")) return actionExecutionResult{status: status, result: result, pr: pr} @@ -368,6 +393,8 @@ func (a *PatronRequestActionService) handleLenderAction(ctx common.ExtendedConte return a.markReceivedLenderRequest(ctx, pr, lms) case LenderActionAcceptCancel: return a.acceptCancelLenderRequest(ctx, pr) + case LenderActionAskRetry: + return a.askRetryLenderRequest(ctx, pr, params) default: status, result := logActionErrorAndReturnResult(ctx, "lender action "+string(action)+" is not implemented yet", errors.New("invalid action")) return actionExecutionResult{status: status, result: result, pr: pr} @@ -567,6 +594,53 @@ func (a *PatronRequestActionService) rejectConditionBorrowingRequest(ctx common. return actionExecutionResult{status: events.EventStatusSuccess, result: &result, pr: pr} } +func (a *PatronRequestActionService) rejectRetryBorrowingRequest(pr pr_db.PatronRequest) actionExecutionResult { + result := events.EventResult{} + return actionExecutionResult{status: events.EventStatusSuccess, result: &result, pr: pr} +} + +func (a *PatronRequestActionService) acceptRetryBorrowingRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) actionExecutionResult { + result := events.EventResult{} + + retryPr := pr_db.PatronRequest{} + retryPr.Side = pr.Side + retryPr.RequesterSymbol = pr.RequesterSymbol + retryPr.SupplierSymbol = pr.SupplierSymbol + retryPr.Patron = pr.Patron + retryPr.Tenant = pr.Tenant + var err error + retryPr.IllRequest, err = deepCopyISO18626Request(pr.IllRequest) + if err != nil { + status, result := logActionErrorAndReturnResult(ctx, "failed to clone IllRequest for retry", err) + return actionExecutionResult{status: status, result: result, pr: pr} + } + retryPr.State = BorrowerStateNew + retryPr.TerminalState = false + retryPr.ID = uuid.NewString() + retryPr.RequesterReqID = getDbTextPtr(&retryPr.ID) + retryPr.CreatedAt = pgtype.Timestamp{Valid: true, Time: time.Now()} + retryPr.IllRequest.Header.RequestingAgencyRequestId = retryPr.ID + retryPr.IllRequest.Header.Timestamp = utils.XSDDateTime{Time: retryPr.CreatedAt.Time} + retryPr.PrevReqID = getDbTextPtr(&pr.ID) + retryPr.Language = pr.Language + retryPr.Items = []pr_db.PrItem{} + retryPr.RetryBibInfo = nil + if pr.RetryBibInfo != nil { + // only take selected fields from retry bib info to allow for corrections without affecting other fields + if pr.RetryBibInfo.SupplierUniqueRecordId != "" { + retryPr.IllRequest.BibliographicInfo.SupplierUniqueRecordId = pr.RetryBibInfo.SupplierUniqueRecordId + } + if pr.RetryBibInfo.Title != "" { + retryPr.IllRequest.BibliographicInfo.Title = pr.RetryBibInfo.Title + } + if pr.RetryBibInfo.Author != "" { + retryPr.IllRequest.BibliographicInfo.Author = pr.RetryBibInfo.Author + } + } + pr.NextReqID = getDbTextPtr(&retryPr.ID) + return actionExecutionResult{status: events.EventStatusSuccess, result: &result, pr: pr, retryPr: retryPr} +} + func (a *PatronRequestActionService) validateLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lms lms.LmsAdapter) actionExecutionResult { institutionalPatron := lms.InstitutionalPatron(pr.RequesterSymbol.String) _, err := lms.LookupUser(institutionalPatron) @@ -811,6 +885,32 @@ func (a *PatronRequestActionService) acceptCancelLenderRequest(ctx common.Extend return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr) } +func (a *PatronRequestActionService) askRetryLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, params actionParams) actionExecutionResult { + var deliveryInfo *iso18626.DeliveryInfo + if params.ItemID != "" { + deliveryInfo = &iso18626.DeliveryInfo{ + ItemId: params.ItemID, + } + } + reasonRetry := string(iso18626.ReasonRetryNotFoundAsCited) + if params.ReasonRetry != "" { + reasonRetry = params.ReasonRetry + } + result := events.EventResult{} + status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result, + iso18626.MessageInfo{ + ReasonRetry: &iso18626.TypeSchemeValuePair{Text: reasonRetry}, + ReasonForMessage: iso18626.TypeReasonForMessageStatusChange, + Note: params.Note, + }, + iso18626.StatusInfo{Status: iso18626.TypeStatusRetryPossible}, + deliveryInfo) + if result.OutgoingMessage.SupplyingAgencyMessage != nil { + setSupplierMessage(*result.OutgoingMessage.SupplyingAgencyMessage, &pr) + } + return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr) +} + func (a *PatronRequestActionService) checkSupplyingResponse(status events.EventStatus, eventResult *events.EventResult, result *events.EventResult, httpStatus *int, pr pr_db.PatronRequest) actionExecutionResult { if httpStatus == nil { return actionExecutionResult{status: status, result: eventResult, pr: pr} diff --git a/broker/patron_request/service/action_mapping_test.go b/broker/patron_request/service/action_mapping_test.go index 824a1001b..7945e8f44 100644 --- a/broker/patron_request/service/action_mapping_test.go +++ b/broker/patron_request/service/action_mapping_test.go @@ -22,11 +22,12 @@ func TestNewReturnableActionMapping(t *testing.T) { BorrowerStateReceived: {{actionName: BorrowerActionCheckOut}}, BorrowerStateCheckedOut: {{actionName: BorrowerActionCheckIn}}, BorrowerStateCheckedIn: {{actionName: BorrowerActionShipReturn}}, + BorrowerStateRetryPending: {{actionName: BorrowerActionAcceptRetry}, {actionName: BorrowerActionRejectRetry}}, } lenderStateActionMapping := map[pr_db.PatronRequestState][]PatronRequestAction{ LenderStateNew: {{actionName: LenderActionValidate, auto: true}}, - LenderStateValidated: {{actionName: LenderActionWillSupply, auto: true}, {actionName: LenderActionCannotSupply}, {actionName: LenderActionAddCondition}}, + LenderStateValidated: {{actionName: LenderActionWillSupply, auto: true}, {actionName: LenderActionCannotSupply}, {actionName: LenderActionAddCondition}, {actionName: LenderActionAskRetry}}, LenderStateWillSupply: {{actionName: LenderActionAddCondition}, {actionName: LenderActionShip}, {actionName: LenderActionCannotSupply}}, LenderStateConditionPending: {{actionName: LenderActionAddCondition}, {actionName: LenderActionCannotSupply}}, LenderStateConditionAccepted: {{actionName: LenderActionAddCondition}, {actionName: LenderActionShip}, {actionName: LenderActionCannotSupply}}, @@ -198,7 +199,7 @@ func mapCompare(t *testing.T, map1 map[pr_db.PatronRequestState][]PatronRequestA for stateName := range map1 { listOne := map1[stateName] listTwo := map2[stateName] - assert.Equal(t, len(listOne), len(listTwo)) + assert.Equal(t, len(listOne), len(listTwo), "State %s has different number of actions in the two maps", stateName) for i := range listOne { assert.Equal(t, listOne[i].actionName, listTwo[i].actionName) assert.Equal(t, listOne[i].auto, listTwo[i].auto) diff --git a/broker/patron_request/service/action_test.go b/broker/patron_request/service/action_test.go index 264e3e304..f3978acee 100644 --- a/broker/patron_request/service/action_test.go +++ b/broker/patron_request/service/action_test.go @@ -1139,6 +1139,66 @@ func TestHandleInvokeLenderActionAddConditionOK(t *testing.T) { } } +func TestHandleInvokeLenderActionAskRetryMinimal(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lms.CreateLmsAdapterMockOK(), nil) + mockIso18626Handler := new(MockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + illRequest := iso18626.Request{} + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil) + action := LenderActionAskRetry + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{ + CommonEventData: events.CommonEventData{Action: &action}, + CustomData: map[string]any{}, + }}) + assert.Equal(t, events.EventStatusSuccess, status) + assert.NotNil(t, resultData) + assert.Equal(t, LenderStateCompletedWithRetry, mockPrRepo.savedPr.State) + + if assert.NotNil(t, mockIso18626Handler.lastSupplyingAgencyMessage) { + assert.Equal(t, iso18626.TypeStatusRetryPossible, mockIso18626Handler.lastSupplyingAgencyMessage.StatusInfo.Status) + assert.Equal(t, string(iso18626.ReasonRetryNotFoundAsCited), mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.ReasonRetry.Text) + assert.Equal(t, "", mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.Note) + assert.Nil(t, mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.OfferedCosts) + assert.Nil(t, mockIso18626Handler.lastSupplyingAgencyMessage.DeliveryInfo) + } +} + +func TestHandleInvokeLenderActionAskRetryFull(t *testing.T) { + mockPrRepo := new(MockPrRepo) + lmsCreator := new(MockLmsCreator) + lmsCreator.On("GetAdapter", "ISIL:SUP1").Return(lms.CreateLmsAdapterMockOK(), nil) + mockIso18626Handler := new(MockIso18626Handler) + prAction := CreatePatronRequestActionService(mockPrRepo, *new(events.EventBus), mockIso18626Handler, lmsCreator) + illRequest := iso18626.Request{} + mockPrRepo.On("GetPatronRequestById", patronRequestId).Return(pr_db.PatronRequest{IllRequest: illRequest, State: LenderStateValidated, Side: SideLending, SupplierSymbol: getDbText("ISIL:SUP1"), RequesterSymbol: getDbText("ISIL:REQ1")}, nil) + action := LenderActionAskRetry + + status, resultData := prAction.handleInvokeAction(appCtx, events.Event{PatronRequestID: patronRequestId, EventData: events.EventData{ + CommonEventData: events.CommonEventData{Action: &action}, + CustomData: map[string]any{ + "note": "isbn", + "itemId": "0201896834", + "reasonRetry": "Transfer", + }, + }}) + assert.Equal(t, events.EventStatusSuccess, status) + assert.NotNil(t, resultData) + assert.Equal(t, LenderStateCompletedWithRetry, mockPrRepo.savedPr.State) + + if assert.NotNil(t, mockIso18626Handler.lastSupplyingAgencyMessage) { + assert.Equal(t, iso18626.TypeStatusRetryPossible, mockIso18626Handler.lastSupplyingAgencyMessage.StatusInfo.Status) + assert.Equal(t, "Transfer", mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.ReasonRetry.Text) + assert.Equal(t, "isbn", mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.Note) + assert.Nil(t, mockIso18626Handler.lastSupplyingAgencyMessage.MessageInfo.OfferedCosts) + if assert.NotNil(t, mockIso18626Handler.lastSupplyingAgencyMessage.DeliveryInfo) { + assert.Equal(t, "0201896834", mockIso18626Handler.lastSupplyingAgencyMessage.DeliveryInfo.ItemId) + } + } +} + func TestHandleInvokeLenderActionAddConditionMissingConditionAndCost(t *testing.T) { mockPrRepo := new(MockPrRepo) lmsCreator := new(MockLmsCreator) diff --git a/broker/patron_request/service/message-handler.go b/broker/patron_request/service/message-handler.go index 964a268cf..60d1e85b3 100644 --- a/broker/patron_request/service/message-handler.go +++ b/broker/patron_request/service/message-handler.go @@ -229,6 +229,7 @@ func (m *PatronRequestMessageHandler) handleSupplyingAgencyMessageWithParent(ctx } eventName := MessageEvent("") + var retryBibInfo *iso18626.BibliographicInfo switch sam.StatusInfo.Status { case iso18626.TypeStatusExpectToSupply: eventName = SupplierExpectToSupply @@ -267,6 +268,15 @@ func (m *PatronRequestMessageHandler) handleSupplyingAgencyMessageWithParent(ctx } eventName = SupplierCancelAccepted } + case iso18626.TypeStatusRetryPossible: + eventName = SupplierRetryConditional + setSupplierMessage(sam, &pr) + // later, we can use MessageInfo.Note to pass bibliographic hints for the retry request + if sam.DeliveryInfo != nil && sam.DeliveryInfo.ItemId != "" { + retryBibInfo = &iso18626.BibliographicInfo{ + SupplierUniqueRecordId: sam.DeliveryInfo.ItemId, + } + } } if eventName == "" { @@ -283,6 +293,9 @@ func (m *PatronRequestMessageHandler) handleSupplyingAgencyMessageWithParent(ctx if !eventDefined { return statusChangeNotAllowed() } + if retryBibInfo != nil { + updatedPr.RetryBibInfo = retryBibInfo + } return m.updatePatronRequestAndCreateSamResponse(ctx, updatedPr, sam, stateChanged, parentEventID) } diff --git a/broker/patron_request/service/message_sender.go b/broker/patron_request/service/message_sender.go index 6611c8ae7..0d5ed58bd 100644 --- a/broker/patron_request/service/message_sender.go +++ b/broker/patron_request/service/message_sender.go @@ -142,6 +142,15 @@ func (ms *PatronRequestMessageSender) sendBorrowingRequest(ctx common.ExtendedCo illRequest.PatronInfo = &iso18626.PatronInfo{} } illRequest.PatronInfo.PatronId = pr.Patron.String + if illRequest.ServiceInfo == nil { + illRequest.ServiceInfo = &iso18626.ServiceInfo{} + } + requestType := iso18626.TypeRequestTypeNew + if pr.PrevReqID.Valid { + illRequest.ServiceInfo.RequestingAgencyPreviousRequestId = pr.PrevReqID.String + requestType = iso18626.TypeRequestTypeRetry + } + illRequest.ServiceInfo.RequestType = &requestType var illMessage = iso18626.NewISO18626Message() illMessage.Request = &illRequest diff --git a/broker/patron_request/service/statemodel_capabilities.go b/broker/patron_request/service/statemodel_capabilities.go index fdc1d3932..c067f1aab 100644 --- a/broker/patron_request/service/statemodel_capabilities.go +++ b/broker/patron_request/service/statemodel_capabilities.go @@ -36,6 +36,9 @@ const ( BorrowerStateCompleted pr_db.PatronRequestState = "COMPLETED" BorrowerStateCancelled pr_db.PatronRequestState = "CANCELLED" BorrowerStateUnfilled pr_db.PatronRequestState = "UNFILLED" + BorrowerStateRetryPending pr_db.PatronRequestState = "RETRY_PENDING" + BorrowerStateRetryAccepted pr_db.PatronRequestState = "RETRY_ACCEPTED" + BorrowerStateRetryRejected pr_db.PatronRequestState = "RETRY_REJECTED" BorrowerStateManuallyClosed pr_db.PatronRequestState = "MANUALLY_CLOSED" LenderStateNew pr_db.PatronRequestState = "NEW" LenderStateValidated pr_db.PatronRequestState = "VALIDATED" @@ -49,6 +52,7 @@ const ( LenderStateCompleted pr_db.PatronRequestState = "COMPLETED" LenderStateCancelled pr_db.PatronRequestState = "CANCELLED" LenderStateUnfilled pr_db.PatronRequestState = "UNFILLED" + LenderStateCompletedWithRetry pr_db.PatronRequestState = "COMPLETED_WITH_RETRY" LenderStateManuallyClosed pr_db.PatronRequestState = "MANUALLY_CLOSED" ) @@ -62,33 +66,36 @@ const ( BorrowerActionCheckOut pr_db.PatronRequestAction = "check-out" BorrowerActionCheckIn pr_db.PatronRequestAction = "check-in" BorrowerActionShipReturn pr_db.PatronRequestAction = "ship-return" - - LenderActionValidate pr_db.PatronRequestAction = "validate" - LenderActionWillSupply pr_db.PatronRequestAction = "will-supply" - LenderActionRejectCancel pr_db.PatronRequestAction = "reject-cancel" - LenderActionCannotSupply pr_db.PatronRequestAction = "cannot-supply" - LenderActionAddCondition pr_db.PatronRequestAction = "add-condition" - LenderActionShip pr_db.PatronRequestAction = "ship" - LenderActionMarkReceived pr_db.PatronRequestAction = "mark-received" - LenderActionAcceptCancel pr_db.PatronRequestAction = "accept-cancel" + BorrowerActionAcceptRetry pr_db.PatronRequestAction = "accept-retry" + BorrowerActionRejectRetry pr_db.PatronRequestAction = "reject-retry" + LenderActionValidate pr_db.PatronRequestAction = "validate" + LenderActionWillSupply pr_db.PatronRequestAction = "will-supply" + LenderActionRejectCancel pr_db.PatronRequestAction = "reject-cancel" + LenderActionCannotSupply pr_db.PatronRequestAction = "cannot-supply" + LenderActionAddCondition pr_db.PatronRequestAction = "add-condition" + LenderActionShip pr_db.PatronRequestAction = "ship" + LenderActionMarkReceived pr_db.PatronRequestAction = "mark-received" + LenderActionAcceptCancel pr_db.PatronRequestAction = "accept-cancel" + LenderActionAskRetry pr_db.PatronRequestAction = "ask-retry" TerminateAction pr_db.PatronRequestAction = "terminate" ) const ( - SupplierExpectToSupply MessageEvent = "expect-to-supply" - SupplierWillSupply MessageEvent = "will-supply" - SupplierWillSupplyCond MessageEvent = "will-supply-conditional" - SupplierLoaned MessageEvent = "loaned" - SupplierCompleted MessageEvent = "completed" - SupplierUnfilled MessageEvent = "unfilled" - SupplierCancelAccepted MessageEvent = "cancel-accepted" - SupplierCancelRejected MessageEvent = "cancel-rejected" - RequesterCancelRequest MessageEvent = "cancel-request" - RequesterReceived MessageEvent = "received" - RequesterShippedReturn MessageEvent = "shipped-return" - RequesterCondAccepted MessageEvent = "conditions-accepted" - RequesterCondRejected MessageEvent = "condition-rejected" + SupplierExpectToSupply MessageEvent = "expect-to-supply" + SupplierWillSupply MessageEvent = "will-supply" + SupplierWillSupplyCond MessageEvent = "will-supply-conditional" + SupplierLoaned MessageEvent = "loaned" + SupplierCompleted MessageEvent = "completed" + SupplierUnfilled MessageEvent = "unfilled" + SupplierCancelAccepted MessageEvent = "cancel-accepted" + SupplierCancelRejected MessageEvent = "cancel-rejected" + SupplierRetryConditional MessageEvent = "retry-conditional" + RequesterCancelRequest MessageEvent = "cancel-request" + RequesterReceived MessageEvent = "received" + RequesterShippedReturn MessageEvent = "shipped-return" + RequesterCondAccepted MessageEvent = "conditions-accepted" + RequesterCondRejected MessageEvent = "condition-rejected" ) func requesterBuiltInStates() []string { @@ -108,6 +115,9 @@ func requesterBuiltInStates() []string { string(BorrowerStateCompleted), string(BorrowerStateCancelled), string(BorrowerStateUnfilled), + string(BorrowerStateRetryAccepted), + string(BorrowerStateRetryRejected), + string(BorrowerStateRetryPending), string(BorrowerStateManuallyClosed), }) } @@ -126,6 +136,7 @@ func supplierBuiltInStates() []string { string(LenderStateCompleted), string(LenderStateCancelled), string(LenderStateUnfilled), + string(LenderStateCompletedWithRetry), string(LenderStateManuallyClosed), }) } @@ -168,6 +179,14 @@ func requesterBuiltInActions() []proapi.ActionCapability { Name: string(BorrowerActionShipReturn), Parameters: []string{}, }, + { + Name: string(BorrowerActionAcceptRetry), + Parameters: []string{}, + }, + { + Name: string(BorrowerActionRejectRetry), + Parameters: []string{}, + }, } } @@ -217,6 +236,14 @@ func supplierBuiltInActions() []proapi.ActionCapability { Name: string(LenderActionAcceptCancel), Parameters: []string{}, }, + { + Name: string(LenderActionAskRetry), + Parameters: []string{ + "note", + "reasonRetry", + "itemId", + }, + }, } } @@ -240,6 +267,7 @@ func supplierBuiltInMessageEvents() []string { string(SupplierUnfilled), string(SupplierCancelAccepted), string(SupplierCancelRejected), + string(SupplierRetryConditional), }) } diff --git a/broker/patron_request/service/statemodels/returnables.json b/broker/patron_request/service/statemodels/returnables.json index 9ba4c224e..6b144af6f 100644 --- a/broker/patron_request/service/statemodels/returnables.json +++ b/broker/patron_request/service/statemodels/returnables.json @@ -67,6 +67,11 @@ "name": "unfilled", "desc": "Supplier cannot supply (ISO18626 Unfilled)", "transition": "UNFILLED" + }, + { + "name": "retry-conditional", + "desc": "Received retry from supplier", + "transition": "RETRY_PENDING" } ] }, @@ -96,6 +101,11 @@ "desc": "Supplier will supply with conditions (ISO18626 WillSupply with conditions)", "transition": "CONDITION_PENDING" }, + { + "name": "retry-conditional", + "desc": "Received retry from supplier", + "transition": "RETRY_PENDING" + }, { "name": "loaned", "desc": "Supplier shipped the item (ISO18626 Loaned)", @@ -144,6 +154,30 @@ } ] }, + { + "name": "RETRY_PENDING", + "display": "Retry Pending", + "desc": "Received ISO18626 RetryPossible from supplier (may include updated metadata/conditions)", + "side": "REQUESTER", + "primaryAction": "accept-retry", + "needsAttention": true, + "actions": [ + { + "name": "accept-retry", + "desc": "Requester accepts supplier retry request", + "transitions": { + "success": "RETRY_ACCEPTED" + } + }, + { + "name": "reject-retry", + "desc": "Requester rejects supplier retry request", + "transitions": { + "success": "RETRY_REJECTED" + } + } + ] + }, { "name": "WILL_SUPPLY", "display": "Will Supply", @@ -293,6 +327,20 @@ "side": "REQUESTER", "terminal": true }, + { + "name": "RETRY_ACCEPTED", + "display": "Retry Accepted", + "desc": "Requester accepted supplier retry request with new metadata", + "side": "REQUESTER", + "terminal": true + }, + { + "name": "RETRY_REJECTED", + "display": "Retry Rejected", + "desc": "Requester rejected supplier retry request", + "side": "REQUESTER", + "terminal": true + }, { "name": "MANUALLY_CLOSED", "display": "Manually closed", @@ -346,6 +394,13 @@ "transitions": { "success": "CONDITION_PENDING" } + }, + { + "name": "ask-retry", + "desc": "Ask requester to retry with new ISO18626 metadata", + "transitions": { + "success": "COMPLETED_WITH_RETRY" + } } ], "events": [ @@ -556,6 +611,13 @@ "side": "SUPPLIER", "terminal": true }, + { + "name": "COMPLETED_WITH_RETRY", + "display": "Completed with Retry", + "desc": "After sending retry to requester", + "side": "SUPPLIER", + "terminal": true + }, { "name": "MANUALLY_CLOSED", "display": "Manually closed", diff --git a/broker/sqlc/pr_query.sql b/broker/sqlc/pr_query.sql index c04832230..57d5be436 100644 --- a/broker/sqlc/pr_query.sql +++ b/broker/sqlc/pr_query.sql @@ -55,14 +55,17 @@ SET ill_request = $3, language = $16, terminal_state = $17, updated_at = now(), - ill_response = $19, - internal_note = $20 + ill_response = $19, + internal_note = $20, + next_req_id = $21, + prev_req_id = $22, + retry_bib_info = $23 WHERE id = $1 AND created_at = $2 AND (updated_at is null OR updated_at = $18) RETURNING sqlc.embed(patron_request); -- name: CreatePatronRequest :one -INSERT INTO patron_request (id, created_at, ill_request, state, side, patron, requester_symbol, supplier_symbol, tenant, requester_req_id, needs_attention, last_action, last_action_outcome, last_action_result, items, language, terminal_state, updated_at, ill_response, internal_note) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20) +INSERT INTO patron_request (id, created_at, ill_request, state, side, patron, requester_symbol, supplier_symbol, tenant, requester_req_id, needs_attention, last_action, last_action_outcome, last_action_result, items, language, terminal_state, updated_at, ill_response, internal_note, next_req_id, prev_req_id, retry_bib_info) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, $23) RETURNING sqlc.embed(patron_request); -- name: UpdatePatronRequestInternalNote :exec diff --git a/broker/sqlc/pr_schema.sql b/broker/sqlc/pr_schema.sql index 3ecec8a0e..864bb8ba0 100644 --- a/broker/sqlc/pr_schema.sql +++ b/broker/sqlc/pr_schema.sql @@ -20,7 +20,10 @@ CREATE TABLE patron_request terminal_state BOOLEAN NOT NULL DEFAULT false, updated_at TIMESTAMP, ill_response jsonb NOT NULL DEFAULT '{}'::jsonb, - internal_note TEXT + internal_note TEXT, + next_req_id VARCHAR, + prev_req_id VARCHAR, + retry_bib_info JSONB ); CREATE OR REPLACE FUNCTION get_next_hrid(prefix VARCHAR) RETURNS VARCHAR AS $$ diff --git a/broker/sqlc/sqlc.yaml b/broker/sqlc/sqlc.yaml index 591fc5e43..c2ccdb26a 100644 --- a/broker/sqlc/sqlc.yaml +++ b/broker/sqlc/sqlc.yaml @@ -104,6 +104,16 @@ sql: - column: "patron_request_search_view.items" go_type: type: "[]PrItem" + - column: "patron_request.retry_bib_info" + go_type: + import: "github.com/indexdata/crosslink/iso18626" + type: "BibliographicInfo" + pointer: true + - column: "patron_request_search_view.retry_bib_info" + go_type: + import: "github.com/indexdata/crosslink/iso18626" + type: "BibliographicInfo" + pointer: true - column: "notification.side" go_type: type: "PatronRequestSide" diff --git a/broker/test/api/api-handler_test.go b/broker/test/api/api-handler_test.go index 45a7bb24b..128532012 100644 --- a/broker/test/api/api-handler_test.go +++ b/broker/test/api/api-handler_test.go @@ -58,7 +58,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/client/client_test.go b/broker/test/client/client_test.go index 7f929204c..b68ccc44a 100644 --- a/broker/test/client/client_test.go +++ b/broker/test/client/client_test.go @@ -48,7 +48,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/events/eventbus_test.go b/broker/test/events/eventbus_test.go index e4bf17c7d..290b065f5 100644 --- a/broker/test/events/eventbus_test.go +++ b/broker/test/events/eventbus_test.go @@ -41,7 +41,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/handler/iso18626-handler_test.go b/broker/test/handler/iso18626-handler_test.go index 74e5f7810..f719b6e14 100644 --- a/broker/test/handler/iso18626-handler_test.go +++ b/broker/test/handler/iso18626-handler_test.go @@ -55,7 +55,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/holdings/holdings_test.go b/broker/test/holdings/holdings_test.go index 3bd3da71b..a9d750f99 100644 --- a/broker/test/holdings/holdings_test.go +++ b/broker/test/holdings/holdings_test.go @@ -47,7 +47,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/patron_request/api/api-handler_test.go b/broker/test/patron_request/api/api-handler_test.go index 21b9aa710..0773c15a8 100644 --- a/broker/test/patron_request/api/api-handler_test.go +++ b/broker/test/patron_request/api/api-handler_test.go @@ -52,7 +52,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") @@ -638,6 +638,325 @@ func TestActionsToCompleteState(t *testing.T) { assert.Equal(t, int64(len(events.Items)), events.About.Count) } +func TestRejectRetry(t *testing.T) { + requesterSymbol := "localISIL:REQ" + uuid.NewString() + supplierSymbol := "ISIL:SUP" + uuid.NewString() + + lmsConfig := &directory.LmsConfig{ + FromAgency: "from-agency", + Address: ncipMockUrl, + } + reqPeer := apptest.CreatePeerWithModeAndVendor(t, illRepo, requesterSymbol, adapter.MOCK_PEER_URL, app.BROKER_MODE, directory.CrossLink, + directory.Entry{ + LmsConfig: lmsConfig, + }) + assert.NotNil(t, reqPeer) + supPeer := apptest.CreatePeer(t, illRepo, supplierSymbol, adapter.MOCK_PEER_URL) + assert.NotNil(t, supPeer) + + // POST + patron := "p1" + request := iso18626.Request{ + BibliographicInfo: iso18626.BibliographicInfo{ + SupplierUniqueRecordId: "RETRY:NOTFOUNDASCITED", + }, + ServiceInfo: &iso18626.ServiceInfo{ + ServiceLevel: &iso18626.TypeSchemeValuePair{ + Text: "Copy", + }, + ServiceType: iso18626.TypeServiceTypeCopy, + }, + } + id := "REQ-" + strings.ToUpper(uuid.NewString()) + newPr := proapi.CreatePatronRequest{ + Id: &id, + RequesterSymbol: &requesterSymbol, + Patron: &patron, + IllRequest: request, + } + newPrBytes, err := json.Marshal(newPr) + assert.NoError(t, err, "failed to marshal patron request") + + hres, respBytes := httpRequest2(t, "POST", basePath, newPrBytes, 201) + // Check Location header + location := hres.Header.Get("Location") + assert.NotEmpty(t, location, "Location header should be set") + assert.Equal(t, getLocalhostWithPort()+"/patron_requests/"+id, location) + + var foundPr proapi.PatronRequest + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + + assert.Equal(t, *newPr.Id, foundPr.Id) + assert.True(t, foundPr.State != "") + assert.Equal(t, string(prservice.SideBorrowing), foundPr.Side) + assert.Equal(t, *newPr.RequesterSymbol, *foundPr.RequesterSymbol) + assert.Nil(t, foundPr.SupplierSymbol) + assert.Equal(t, *newPr.Patron, *foundPr.Patron) + assertPatronRequestIllRequest(t, foundPr.IllRequest, func(r iso18626.Request) { + assert.Equal(t, "RETRY:NOTFOUNDASCITED", r.BibliographicInfo.SupplierUniqueRecordId) + assert.Equal(t, *newPr.Id, r.Header.RequestingAgencyRequestId) + assert.False(t, r.Header.Timestamp.IsZero()) + }) + assert.Equal(t, "validate", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + assert.NotNil(t, foundPr.NotificationsLink) + + // GET list + queryParams := "?side=borrowing&symbol=" + *foundPr.RequesterSymbol + respBytes = httpRequest(t, "GET", basePath+queryParams, []byte{}, 200) + var foundPrs proapi.PatronRequests + err = json.Unmarshal(respBytes, &foundPrs) + assert.NoError(t, err, "failed to unmarshal patron request") + + thisPrPath := basePath + "/" + *newPr.Id + + // POST execute action + action := proapi.ExecuteAction{ + Action: "send-request", + } + actionBytes, err := json.Marshal(action) + assert.NoError(t, err, "failed to marshal patron request action") + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 200) + var pResult proapi.ActionResult + err = json.Unmarshal(respBytes, &pResult) + assert.NoError(t, err, "failed to unmarshal patron request action result") + assert.Equal(t, "SUCCESS", pResult.Result) + assert.Equal(t, "success", pResult.Outcome) + assert.Equal(t, "VALIDATED", pResult.FromState) + assert.Equal(t, "SENT", *pResult.ToState) + assert.Nil(t, pResult.Message) + + respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, *newPr.Id, foundPr.Id) + assert.Equal(t, "send-request", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + + // Wait until we can see possible action reject-retry + assert.True(t, test.WaitForPredicateToBeTrue(func() bool { + respBytes = httpRequest(t, "GET", thisPrPath+"/actions"+queryParams, []byte{}, 200) + return strings.Contains(string(respBytes), "\"name\":\"reject-retry\"") + }), "reject-retry action did not appear in time") + respBytes = httpRequest(t, "GET", thisPrPath+"/actions"+queryParams, []byte{}, 200) + assert.Contains(t, string(respBytes), "\"name\":\"reject-retry\"") + + // POST blocking action + action = proapi.ExecuteAction{ + Action: "reject-retry", + } + actionBytes, err = json.Marshal(action) + assert.NoError(t, err, "failed to marshal patron request action") + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 200) + err = json.Unmarshal(respBytes, &pResult) + assert.NoError(t, err, "failed to unmarshal patron request action result") + assert.Equal(t, "SUCCESS", pResult.Result) + + respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, *newPr.Id, foundPr.Id) + assert.Equal(t, "reject-retry", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + + // reject again - should fail as the request state it terminated + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 400) + assert.Contains(t, string(respBytes), "Action reject-retry is not allowed for patron request") +} + +func TestAcceptRetry(t *testing.T) { + requesterSymbol := "localISIL:REQ" + uuid.NewString() + supplierSymbol := "ISIL:SUP" + uuid.NewString() + + lmsConfig := &directory.LmsConfig{ + FromAgency: "from-agency", + Address: ncipMockUrl, + } + reqPeer := apptest.CreatePeerWithModeAndVendor(t, illRepo, requesterSymbol, adapter.MOCK_PEER_URL, app.BROKER_MODE, directory.CrossLink, + directory.Entry{ + LmsConfig: lmsConfig, + }) + assert.NotNil(t, reqPeer) + supPeer := apptest.CreatePeer(t, illRepo, supplierSymbol, adapter.MOCK_PEER_URL) + assert.NotNil(t, supPeer) + + // POST + patron := "p1" + request := iso18626.Request{ + BibliographicInfo: iso18626.BibliographicInfo{ + SupplierUniqueRecordId: "RETRY:NOTFOUNDASCITED", + BibliographicItemId: []iso18626.BibliographicItemId{ + { + BibliographicItemIdentifierCode: iso18626.TypeSchemeValuePair{ + Text: "ISBN", + }, + BibliographicItemIdentifier: "1234567890", + }, + }, + }, + ServiceInfo: &iso18626.ServiceInfo{ + ServiceLevel: &iso18626.TypeSchemeValuePair{ + Text: "Copy", + }, + ServiceType: iso18626.TypeServiceTypeCopy, + }, + } + id := "REQ-" + strings.ToUpper(uuid.NewString()) + newPr := proapi.CreatePatronRequest{ + Id: &id, + RequesterSymbol: &requesterSymbol, + Patron: &patron, + IllRequest: request, + } + newPrBytes, err := json.Marshal(newPr) + assert.NoError(t, err, "failed to marshal patron request") + + hres, respBytes := httpRequest2(t, "POST", basePath, newPrBytes, 201) + // Check Location header + location := hres.Header.Get("Location") + assert.NotEmpty(t, location, "Location header should be set") + assert.Equal(t, getLocalhostWithPort()+"/patron_requests/"+id, location) + + var foundPr proapi.PatronRequest + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + + assert.Equal(t, id, foundPr.Id) + assert.True(t, foundPr.State != "") + assert.Equal(t, string(prservice.SideBorrowing), foundPr.Side) + assert.Equal(t, *newPr.RequesterSymbol, *foundPr.RequesterSymbol) + assert.Nil(t, foundPr.SupplierSymbol) + assert.Equal(t, *newPr.Patron, *foundPr.Patron) + assertPatronRequestIllRequest(t, foundPr.IllRequest, func(r iso18626.Request) { + assert.Equal(t, "RETRY:NOTFOUNDASCITED", r.BibliographicInfo.SupplierUniqueRecordId) + assert.Equal(t, id, r.Header.RequestingAgencyRequestId) + assert.False(t, r.Header.Timestamp.IsZero()) + }) + assert.Equal(t, "validate", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + assert.NotNil(t, foundPr.NotificationsLink) + + // GET list + queryParams := "?side=borrowing&symbol=" + *foundPr.RequesterSymbol + respBytes = httpRequest(t, "GET", basePath+queryParams, []byte{}, 200) + var foundPrs proapi.PatronRequests + err = json.Unmarshal(respBytes, &foundPrs) + assert.NoError(t, err, "failed to unmarshal patron request") + + thisPrPath := basePath + "/" + *newPr.Id + + // POST execute action + action := proapi.ExecuteAction{ + Action: "send-request", + } + actionBytes, err := json.Marshal(action) + assert.NoError(t, err, "failed to marshal patron request action") + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 200) + var pResult proapi.ActionResult + err = json.Unmarshal(respBytes, &pResult) + assert.NoError(t, err, "failed to unmarshal patron request action result") + assert.Equal(t, "SUCCESS", pResult.Result) + assert.Equal(t, "success", pResult.Outcome) + assert.Equal(t, "VALIDATED", pResult.FromState) + assert.Equal(t, "SENT", *pResult.ToState) + assert.Nil(t, pResult.Message) + + respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) + foundPr = proapi.PatronRequest{} + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, *newPr.Id, foundPr.Id) + assert.Equal(t, "send-request", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + + // Wait until we can see possible action accept-retry + assert.True(t, test.WaitForPredicateToBeTrue(func() bool { + respBytes = httpRequest(t, "GET", thisPrPath+"/actions"+queryParams, []byte{}, 200) + return strings.Contains(string(respBytes), "\"name\":\"accept-retry\"") + }), "accept-retry action did not appear in time") + respBytes = httpRequest(t, "GET", thisPrPath+"/actions"+queryParams, []byte{}, 200) + assert.Contains(t, string(respBytes), "\"name\":\"accept-retry\"") + + // POST blocking action + action = proapi.ExecuteAction{ + Action: "accept-retry", + } + actionBytes, err = json.Marshal(action) + assert.NoError(t, err, "failed to marshal patron request action") + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 200) + err = json.Unmarshal(respBytes, &pResult) + assert.NoError(t, err, "failed to unmarshal patron request action result") + assert.Equal(t, "SUCCESS", pResult.Result) + + // check the original request is updated with retry info and new request is created + respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) + foundPr = proapi.PatronRequest{} + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, *newPr.Id, foundPr.Id) + assert.Equal(t, "accept-retry", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + assert.NotNil(t, foundPr.NextReqId, "got pr "+string(respBytes)) + assert.Nil(t, foundPr.PrevReqId, "got pr "+string(respBytes)) + assert.Equal(t, "123456789", (*foundPr.RetryBibInfo)["supplierUniqueRecordId"]) + + // accept again - should fail as the request state it terminated + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 400) + assert.Contains(t, string(respBytes), "Action accept-retry is not allowed for patron request") + + // check cloned request + newId := *foundPr.NextReqId + assert.NotEqual(t, newId, id) + + thisPrPath = basePath + "/" + newId + + respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) + foundPr = proapi.PatronRequest{} + err = json.Unmarshal(respBytes, &foundPr) + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, newId, foundPr.Id) + assert.Equal(t, "validate", *foundPr.LastAction) + assert.Equal(t, "success", *foundPr.LastActionOutcome) + assert.Equal(t, "SUCCESS", *foundPr.LastActionResult) + assert.Equal(t, id, *foundPr.PrevReqId) + assert.Nil(t, foundPr.NextReqId) + assert.Nil(t, foundPr.RetryBibInfo) + assert.Equal(t, "123456789", foundPr.IllRequest.BibliographicInfo.SupplierUniqueRecordId) + + // retry 2nd time. The "123456789" item id should be kept as retryItemId + // illmock will use scenario unfilled + action = proapi.ExecuteAction{ + Action: "send-request", + } + actionBytes, err = json.Marshal(action) + assert.NoError(t, err, "failed to marshal patron request action") + respBytes = httpRequest(t, "POST", thisPrPath+"/action"+queryParams, actionBytes, 200) + err = json.Unmarshal(respBytes, &pResult) + assert.NoError(t, err, "failed to unmarshal patron request action result") + assert.Equal(t, "SUCCESS", pResult.Result) + assert.Equal(t, "success", pResult.Outcome) + assert.Equal(t, "VALIDATED", pResult.FromState) + assert.Equal(t, "SENT", *pResult.ToState) + assert.Nil(t, pResult.Message) + + assert.True(t, test.WaitForPredicateToBeTrue(func() bool { + respBytes = httpRequest(t, "GET", thisPrPath+queryParams, []byte{}, 200) + foundPr = proapi.PatronRequest{} + err = json.Unmarshal(respBytes, &foundPr) + return err == nil && foundPr.State == "UNFILLED" && foundPr.TerminalState + }), "patron request did not reach UNFILLED state in time") + assert.NoError(t, err, "failed to unmarshal patron request") + assert.Equal(t, "UNFILLED", foundPr.State) + assert.True(t, foundPr.TerminalState) +} + func TestPostPatronRequestRejectsInvalidIllRequest(t *testing.T) { requesterSymbol := "localISIL:REQ" + uuid.NewString() diff --git a/broker/test/patron_request/db/prrepo_test.go b/broker/test/patron_request/db/prrepo_test.go index 4b24bc836..1a68caa4c 100644 --- a/broker/test/patron_request/db/prrepo_test.go +++ b/broker/test/patron_request/db/prrepo_test.go @@ -39,7 +39,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/pullslip/api/api_handler_test.go b/broker/test/pullslip/api/api_handler_test.go index a76f27622..ec719ec3d 100644 --- a/broker/test/pullslip/api/api_handler_test.go +++ b/broker/test/pullslip/api/api_handler_test.go @@ -43,7 +43,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/service/e2e_test.go b/broker/test/service/e2e_test.go index b747f1fed..b9a47f536 100644 --- a/broker/test/service/e2e_test.go +++ b/broker/test/service/e2e_test.go @@ -42,7 +42,7 @@ func TestMain(m *testing.M) { postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) test.Expect(err, "failed to start db container") diff --git a/broker/test/utils/utils.go b/broker/test/utils/utils.go index d37ceeca1..7e1a1e3f5 100644 --- a/broker/test/utils/utils.go +++ b/broker/test/utils/utils.go @@ -90,7 +90,7 @@ func StartPGContainer() (context.Context, *postgres.PostgresContainer, string, e postgres.WithPassword("crosslink"), testcontainers.WithWaitStrategy( wait.ForLog("database system is ready to accept connections"). - WithOccurrence(2).WithStartupTimeout(5*time.Second)), + WithOccurrence(2).WithStartupTimeout(30*time.Second)), ) if err != nil { return ctx, pgContainer, "", fmt.Errorf("failed to start db container: %w", err) diff --git a/illmock/README.md b/illmock/README.md index 462ebb0f7..0f4ef949d 100644 --- a/illmock/README.md +++ b/illmock/README.md @@ -82,6 +82,7 @@ The scenario is used by the supplier to perform a particular workflow. The follo |`RETRY:COND_` ... | Response with `RetryPossible` and ReasonRetry `LoanCondition` | |`RETRY:COST_` ... | Response with `RetryPossible` and ReasonRetry+ReasonUnfilled `CostExceedsMaxCost` | |`RETRY:ONLOAN_` ... | Response with `RetryPossible` and ReasonRetry `OnLoan` | +|`RETRY:NOTFOUNDASCITED` | Response with `RetryPossible` and ReasonRetry `NotFoundAsCited` | ### Delivery method diff --git a/illmock/app/app_test.go b/illmock/app/app_test.go index 33cae6bfe..561e5e242 100644 --- a/illmock/app/app_test.go +++ b/illmock/app/app_test.go @@ -1370,6 +1370,25 @@ func TestService(t *testing.T) { assert.Equal(t, iso18626.TypeActionShippedReturn, *responseMsg.RequestingAgencyMessageConfirmation.Action) }) + t.Run("Patron request not found as cited", func(t *testing.T) { + msg := createPatronRequest() + ret := runScenario(t, isoUrl, apiUrl, msg, "RETRY:NOTFOUNDASCITED", 8) + + m := ret[1].Message + rid := m.Request.Header.RequestingAgencyRequestId + + m = ret[len(ret)-2].Message + assert.NotNil(t, m.SupplyingAgencyMessage) + assert.Equal(t, iso18626.TypeStatusRetryPossible, m.SupplyingAgencyMessage.StatusInfo.Status) + assert.Equal(t, string(iso18626.ReasonRetryNotFoundAsCited), m.SupplyingAgencyMessage.MessageInfo.ReasonRetry.Text) + assert.Equal(t, "123456789", m.SupplyingAgencyMessage.DeliveryInfo.ItemId) + + m = ret[len(ret)-1].Message + assert.NotNil(t, m.SupplyingAgencyMessageConfirmation) + assert.Equal(t, rid, m.SupplyingAgencyMessageConfirmation.ConfirmationHeader.RequestingAgencyRequestId) + assert.Equal(t, iso18626.TypeReasonForMessageRequestResponse, *m.SupplyingAgencyMessageConfirmation.ReasonForMessage) + }) + t.Run("tenant ID set", func(t *testing.T) { assert.Equal(t, "T1", app.client.Headers.Get("X-Okapi-Tenant")) }) diff --git a/illmock/app/supplier.go b/illmock/app/supplier.go index d0c4efbae..e8ad61014 100644 --- a/illmock/app/supplier.go +++ b/illmock/app/supplier.go @@ -138,6 +138,10 @@ func (app *MockApp) handleSupplierRequest(illRequest *iso18626.Request, w http.R status = append(status, iso18626.TypeStatusRetryPossible) x := iso18626.ReasonRetryLoanCondition reasonRetry = &x + case "RETRY:NOTFOUNDASCITED": + status = append(status, iso18626.TypeStatusRetryPossible) + x := iso18626.ReasonRetryNotFoundAsCited + reasonRetry = &x case "COMPLETED": if illRequest.ServiceInfo != nil && illRequest.ServiceInfo.ServiceType == iso18626.TypeServiceTypeCopy { status = append(status, iso18626.TypeStatusCopyCompleted) @@ -289,6 +293,10 @@ func (app *MockApp) sendSupplyingAgencyLater(header *iso18626.Header, statusList case iso18626.ReasonRetryLoanCondition: msg.SupplyingAgencyMessage.DeliveryInfo = &iso18626.DeliveryInfo{} msg.SupplyingAgencyMessage.DeliveryInfo.LoanCondition = &iso18626.TypeSchemeValuePair{Text: "NoReproduction"} + case iso18626.ReasonRetryNotFoundAsCited: + msg.SupplyingAgencyMessage.DeliveryInfo = &iso18626.DeliveryInfo{ + ItemId: "123456789", + } } } if state.presentResponse { diff --git a/iso18626/opencodes.go b/iso18626/opencodes.go index d32653abf..dacae20cd 100644 --- a/iso18626/opencodes.go +++ b/iso18626/opencodes.go @@ -9,6 +9,7 @@ const ( ReasonRetryCostExceedsMaxCost ReasonRetry = "CostExceedsMaxCost" ReasonRetryOnLoan ReasonRetry = "OnLoan" ReasonRetryLoanCondition ReasonRetry = "LoanCondition" + ReasonRetryNotFoundAsCited ReasonRetry = "NotFoundAsCited" ) type ReasonUnfilled string diff --git a/misc/returnables.yaml b/misc/returnables.yaml index c28a75479..ca619d86b 100644 --- a/misc/returnables.yaml +++ b/misc/returnables.yaml @@ -48,6 +48,9 @@ states: - name: unfilled desc: Supplier cannot supply (ISO18626 Unfilled) transition: UNFILLED + - name: retry-conditional + desc: Received retry from supplier + transition: RETRY_PENDING - name: SUPPLIER_LOCATED display: Supplier Located @@ -66,6 +69,9 @@ states: - name: will-supply-conditional desc: Supplier will supply with conditions (ISO18626 WillSupply with conditions) transition: CONDITION_PENDING + - name: retry-conditional + desc: Received retry from supplier + transition: RETRY_PENDING - name: loaned desc: Supplier shipped the item (ISO18626 Loaned) transition: SHIPPED @@ -96,6 +102,22 @@ states: desc: Supplier cannot supply (ISO18626 Unfilled) transition: UNFILLED + - name: RETRY_PENDING + display: Retry Pending + desc: Received ISO18626 RetryPossible from supplier (may include updated metadata/conditions) + side: REQUESTER + primaryAction: accept-retry + needsAttention: true + actions: + - name: accept-retry + desc: Requester accepts supplier retry request + transitions: + success: RETRY_ACCEPTED + - name: reject-retry + desc: Requester rejects supplier retry request + transitions: + success: RETRY_REJECTED + - name: WILL_SUPPLY display: Will Supply desc: Received ISO18626 WillSupply (without condition) @@ -200,6 +222,18 @@ states: side: REQUESTER terminal: true + - name: RETRY_ACCEPTED + display: Retry Accepted + desc: Requester accepted supplier retry request with new metadata + side: REQUESTER + terminal: true + + - name: RETRY_REJECTED + display: Retry Rejected + desc: Requester rejected supplier retry request + side: REQUESTER + terminal: true + - name: MANUALLY_CLOSED display: Manually closed desc: Closed manually by staff @@ -239,6 +273,10 @@ states: desc: Indicate will supply with conditions and send ISO18626 WillSupply transitions: success: CONDITION_PENDING + - name: ask-retry + desc: Ask requester to retry with new ISO18626 metadata + transitions: + success: COMPLETED_WITH_RETRY events: - name: cancel-request desc: Requester sent ISO18626 Cancel @@ -377,6 +415,12 @@ states: side: SUPPLIER terminal: true + - name: COMPLETED_WITH_RETRY + display: Completed with Retry + desc: After sending retry to requester + side: SUPPLIER + terminal: true + - name: MANUALLY_CLOSED display: Manually closed desc: Closed manually by staff