Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 36 additions & 0 deletions broker/migrations/050_add_patron_request_state_model.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
DROP VIEW IF EXISTS patron_request_search_view;

ALTER TABLE patron_request
DROP COLUMN state_model;

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,
req_peer.name AS requester_name,
sup_peer.name AS supplier_name
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
LEFT JOIN symbol req_sym ON req_sym.symbol_value = pr.requester_symbol
LEFT JOIN peer req_peer ON req_peer.id = req_sym.peer_id
LEFT JOIN symbol sup_sym ON sup_sym.symbol_value = pr.supplier_symbol
LEFT JOIN peer sup_peer ON sup_peer.id = sup_sym.peer_id;
36 changes: 36 additions & 0 deletions broker/migrations/050_add_patron_request_state_model.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
ALTER TABLE patron_request
ADD COLUMN state_model VARCHAR NOT NULL DEFAULT 'returnables';

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,
req_peer.name AS requester_name,
sup_peer.name AS supplier_name
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
LEFT JOIN symbol req_sym ON req_sym.symbol_value = pr.requester_symbol
LEFT JOIN peer req_peer ON req_peer.id = req_sym.peer_id
LEFT JOIN symbol sup_sym ON sup_sym.symbol_value = pr.supplier_symbol
LEFT JOIN peer sup_peer ON sup_peer.id = sup_sym.peer_id;
27 changes: 26 additions & 1 deletion broker/oapi/open-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -521,6 +521,9 @@ components:
state:
type: string
description: Patron request state
stateModel:
type: string
description: State model configuration key governing this request
side:
type: string
description: Patron request side - borrowing or lending
Expand Down Expand Up @@ -602,6 +605,7 @@ components:
- id
- createdAt
- state
- stateModel
- side
- illRequest
- needsAttention
Expand Down Expand Up @@ -750,6 +754,8 @@ components:
version:
type: string
description: Version of the state model in SemVer
selector:
$ref: '#/components/schemas/StateModelSelector'
states:
type: array
description: A list of all allowed states
Expand All @@ -761,6 +767,26 @@ components:
- type
- version

StateModelSelector:
title: StateModelSelector
type: object
description: Criteria used to select this state model for an ISO 18626 request
additionalProperties: false
properties:
serviceType:
type: array
description: ISO 18626 service types handled by this state model
minItems: 1
uniqueItems: true
items:
type: string
enum:
- Copy
- Loan
- CopyOrLoan
required:
- serviceType

StateModelCapabilities:
title: StateModelCapabilities
type: object
Expand Down Expand Up @@ -3007,4 +3033,3 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Error'

19 changes: 17 additions & 2 deletions broker/patron_request/api/api-handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,11 +351,17 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h
api.AddInternalError(ctx, w, err)
return
}
actionMapping, err := a.actionMappingService.GetActionMapping(illRequest)
stateModelName, err := a.actionMappingService.GetStateModelNameForRequest(illRequest)
if err != nil {
api.AddInternalError(ctx, w, err)
return
}
stateModel, err := a.actionMappingService.GetStateModel(stateModelName)
if err != nil {
api.AddInternalError(ctx, w, err)
return
}
actionMapping := prservice.NewActionMapping(stateModel)
borrowerInitialState, ok := actionMapping.GetInitialState(prservice.SideBorrowing)
if !ok {
api.AddInternalError(ctx, w, fmt.Errorf("no initial state defined for borrower side"))
Expand Down Expand Up @@ -385,7 +391,7 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h
}
}

dbreq := buildDbPatronRequest(&newPr, params.XOkapiTenant, creationTime, requesterReqId, illRequest, borrowerInitialState)
dbreq := buildDbPatronRequest(&newPr, params.XOkapiTenant, creationTime, requesterReqId, illRequest, borrowerInitialState, stateModelName)
pr, err := a.prRepo.CreatePatronRequest(ctx, pr_db.CreatePatronRequestParams(dbreq))
if err != nil {
var pgErr *pgconn.PgError
Expand Down Expand Up @@ -604,9 +610,15 @@ func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *
api.AddInternalError(ctx, w, err)
return
}
stateModelName, err := a.actionMappingService.GetStateModelNameForRequest(illRequest)
if err != nil {
api.AddInternalError(ctx, w, err)
return
}

existingPr.RequesterReqID = getDbText(&requesterReqId)
existingPr.IllRequest = illRequest
existingPr.StateModel = stateModelName
existingPr.Patron = getDbText(newPr.Patron)
if newPr.InternalNote != nil {
var note pgtype.Text
Expand Down Expand Up @@ -1351,6 +1363,7 @@ func toApiPatronRequest(r *http.Request, request pr_db.PatronRequestSearchView)
Id: request.ID,
CreatedAt: request.CreatedAt.Time,
State: string(request.State),
StateModel: request.StateModel,
Side: string(request.Side),
Patron: toString(request.Patron),
RequesterSymbol: toString(request.RequesterSymbol),
Expand Down Expand Up @@ -1501,6 +1514,7 @@ func buildDbPatronRequest(
requesterReqId string,
illRequest iso18626.Request,
initialState pr_db.PatronRequestState,
stateModel string,
) pr_db.PatronRequest {
return pr_db.PatronRequest{
ID: requesterReqId,
Expand All @@ -1518,6 +1532,7 @@ func buildDbPatronRequest(
Items: []pr_db.PrItem{},
TerminalState: false,
NeedsAttention: true,
StateModel: stateModel,
// LastAction, LastActionOutcome and LastActionResult are not set on creation
// they will be updated when the first action is executed.
}
Expand Down
7 changes: 6 additions & 1 deletion broker/patron_request/api/api-handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,10 @@ func TestToApiPatronRequestSurfacesInternalNote(t *testing.T) {
pr := pr_db.PatronRequest{
ID: "pr-1",
InternalNote: pgtype.Text{String: "staff note", Valid: true},
StateModel: "returnables",
}
apiPr := toApiPatronRequest(req, patronRequestSearchViewFromPatronRequest(pr, false))
assert.Equal(t, "returnables", apiPr.StateModel)
if assert.NotNil(t, apiPr.InternalNote) {
assert.Equal(t, "staff note", *apiPr.InternalNote)
}
Expand Down Expand Up @@ -157,6 +159,7 @@ func patronRequestSearchViewFromPatronRequest(pr pr_db.PatronRequest, hasCost bo
UpdatedAt: pr.UpdatedAt,
IllResponse: pr.IllResponse,
InternalNote: pr.InternalNote,
StateModel: pr.StateModel,
HasCost: hasCost,
}
}
Expand Down Expand Up @@ -760,14 +763,15 @@ func TestParseAndValidateIllRequestAndBuildDbPatronRequest(t *testing.T) {
illRequest, requesterReqID, err := handler.parseAndValidateIllRequest(ctx, reqWithID, creationTime)
assert.NoError(t, err)
assert.Equal(t, id, requesterReqID)
pr := buildDbPatronRequest(reqWithID, nil, pgtype.Timestamp{Valid: true, Time: creationTime}, requesterReqID, illRequest, prservice.BorrowerStateNew)
pr := buildDbPatronRequest(reqWithID, nil, pgtype.Timestamp{Valid: true, Time: creationTime}, requesterReqID, illRequest, prservice.BorrowerStateNew, "returnables")
assert.Equal(t, id, pr.ID)
assert.True(t, pr.CreatedAt.Valid)
assert.True(t, pr.RequesterReqID.Valid)
assert.Equal(t, id, pr.RequesterReqID.String)
assert.False(t, pr.SupplierSymbol.Valid)
assert.Equal(t, patron, pr.Patron.String)
assert.Equal(t, patron, pr.IllRequest.PatronInfo.PatronId)
assert.Equal(t, "returnables", pr.StateModel)

reqWithoutID := &proapi.CreatePatronRequest{RequesterSymbol: &symbol}
_, _, err = handler.parseAndValidateIllRequest(ctx, reqWithoutID, creationTime)
Expand Down Expand Up @@ -1633,6 +1637,7 @@ func TestPutPatronRequestsIdOK(t *testing.T) {
assert.Equal(t, patron, repo.lastUpdateParams.Patron.String)
assert.True(t, repo.lastUpdateParams.InternalNote.Valid)
assert.Equal(t, note, repo.lastUpdateParams.InternalNote.String)
assert.Equal(t, "returnables", repo.lastUpdateParams.StateModel)
}
var response proapi.PatronRequest
err := json.Unmarshal(rr.Body.Bytes(), &response)
Expand Down
1 change: 1 addition & 0 deletions broker/patron_request/db/prcql.go
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ func (q *Queries) ListPatronRequestsCql(ctx context.Context, db DBTX, arg ListPa
&i.PatronRequestSearchView.NextReqID,
&i.PatronRequestSearchView.PrevReqID,
&i.PatronRequestSearchView.RetryBibInfo,
&i.PatronRequestSearchView.StateModel,
&i.PatronRequestSearchView.HasNotification,
&i.PatronRequestSearchView.HasCost,
&i.PatronRequestSearchView.HasUnreadNotification,
Expand Down
1 change: 1 addition & 0 deletions broker/patron_request/db/prrepo.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ func patronRequestFromSearchView(v PatronRequestSearchView) PatronRequest {
UpdatedAt: v.UpdatedAt,
IllResponse: v.IllResponse,
InternalNote: v.InternalNote,
StateModel: v.StateModel,
}
}

Expand Down
1 change: 1 addition & 0 deletions broker/patron_request/service/action.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ func (a *PatronRequestActionService) acceptRetryBorrowingRequest(ctx common.Exte
retryPr.Language = pr.Language
retryPr.Items = []pr_db.PrItem{}
retryPr.RetryBibInfo = nil
retryPr.StateModel = pr.StateModel
if pr.RetryBibInfo != nil {
// only take selected fields from retry bib info to allow for corrections without affecting other fields
if pr.RetryBibInfo.SupplierUniqueRecordId != "" {
Expand Down
43 changes: 41 additions & 2 deletions broker/patron_request/service/action_mapping.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package prservice

import (
"fmt"
"slices"
"sort"
"strings"

"github.com/indexdata/crosslink/broker/events"
Expand Down Expand Up @@ -29,8 +31,45 @@ func (r *ActionMappingService) GetActionMapping(request iso18626.Request) (*Acti
}

func (r *ActionMappingService) GetStateModelForRequest(request iso18626.Request) (*proapi.StateModel, error) {
//TODO: check the ISO18626Request to decide what kind of state model/mapping to return
return r.GetStateModel("returnables")
modelName, err := r.GetStateModelNameForRequest(request)
if err != nil {
return nil, err
}
return r.GetStateModel(modelName)
}

func (r *ActionMappingService) GetStateModelNameForRequest(request iso18626.Request) (string, error) {
if request.ServiceInfo == nil {
var selectableModels []string
for name, stateModel := range stateModelsConfig.StateModels {
if stateModel.Selector != nil {
selectableModels = append(selectableModels, name)
}
}
sort.Strings(selectableModels)
if len(selectableModels) == 1 {
return selectableModels[0], nil
}
return "", fmt.Errorf("cannot select state model without service info: found %d selectable models", len(selectableModels))
}

serviceType := proapi.StateModelSelectorServiceType(request.ServiceInfo.ServiceType)
var matches []string
for name, stateModel := range stateModelsConfig.StateModels {
if stateModel.Selector != nil && slices.Contains(stateModel.Selector.ServiceType, serviceType) {
matches = append(matches, name)
}
}
sort.Strings(matches)

switch len(matches) {
case 0:
return "", fmt.Errorf("no state model matches service type %q", serviceType)
case 1:
return matches[0], nil
default:
return "", fmt.Errorf("multiple state models match service type %q: %s", serviceType, strings.Join(matches, ", "))
}
}

func (r *ActionMappingService) GetStateModel(modelName string) (*proapi.StateModel, error) {
Expand Down
51 changes: 50 additions & 1 deletion broker/patron_request/service/action_mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,61 @@ var actionMappingService = ActionMappingService{}

func mustActionMapping(t *testing.T) *ActionMapping {
t.Helper()
mapping, err := actionMappingService.GetActionMapping(iso18626.Request{})
mapping, err := actionMappingService.GetActionMapping(iso18626.Request{
ServiceInfo: &iso18626.ServiceInfo{ServiceType: iso18626.TypeServiceTypeLoan},
})
assert.NoError(t, err)
assert.NotNil(t, mapping)
return mapping
}

func TestGetStateModelForRequestUsesSelector(t *testing.T) {
service := ActionMappingService{}

for _, serviceType := range []iso18626.TypeServiceType{
iso18626.TypeServiceTypeCopy,
iso18626.TypeServiceTypeLoan,
iso18626.TypeServiceTypeCopyOrLoan,
} {
t.Run(string(serviceType), func(t *testing.T) {
model, err := service.GetStateModelForRequest(iso18626.Request{
ServiceInfo: &iso18626.ServiceInfo{ServiceType: serviceType},
})
assert.NoError(t, err)
if assert.NotNil(t, model) {
assert.Equal(t, "CrossLink Returnables State Model", model.Name)
}
})
}
}

func TestGetStateModelNameForRequestUsesSelector(t *testing.T) {
name, err := (&ActionMappingService{}).GetStateModelNameForRequest(iso18626.Request{
ServiceInfo: &iso18626.ServiceInfo{ServiceType: iso18626.TypeServiceTypeLoan},
})

assert.NoError(t, err)
assert.Equal(t, "returnables", name)
}

func TestGetStateModelForRequestWithoutServiceInfoUsesOnlyConfiguredModel(t *testing.T) {
model, err := (&ActionMappingService{}).GetStateModelForRequest(iso18626.Request{})

assert.NoError(t, err)
if assert.NotNil(t, model) {
assert.Equal(t, "CrossLink Returnables State Model", model.Name)
}
}

func TestGetStateModelForRequestWithoutMatch(t *testing.T) {
model, err := (&ActionMappingService{}).GetStateModelForRequest(iso18626.Request{
ServiceInfo: &iso18626.ServiceInfo{ServiceType: iso18626.TypeServiceType("Unsupported")},
})

assert.Nil(t, model)
assert.EqualError(t, err, `no state model matches service type "Unsupported"`)
}

func TestIsActionAvailable(t *testing.T) {
mapping := mustActionMapping(t)
// Borrower
Expand Down
Loading
Loading