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
4 changes: 4 additions & 0 deletions broker/oapi/open-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,10 @@ components:
type: string
description: Name of the primary action for this state.
Must be one of the actions defined in the actions array and is only meaningful when actions is present.
closingAction:
type: string
description: Name of the closing action for this state.
Must be one of the actions defined in the actions array.
editable:
type: boolean
description: Indicates that a patron request can be updated in this state
Expand Down
15 changes: 15 additions & 0 deletions broker/patron_request/service/action_mapping.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ type PatronRequestAction struct {
}

type ActionMapping struct {
StateModelName string
borrowerStateActionMapping map[pr_db.PatronRequestState][]PatronRequestAction
lenderStateActionMapping map[pr_db.PatronRequestState][]PatronRequestAction
borrowerStateConfig map[pr_db.PatronRequestState]stateConfig
Expand All @@ -106,6 +107,7 @@ type stateConfig struct {
autoActions []proapi.ModelAction
terminal bool
needsAttention bool
closingAction *pr_db.PatronRequestAction
}

// Constructor function to initialize the mappings for given StateModel
Expand All @@ -114,6 +116,7 @@ func NewActionMapping(stateModel *proapi.StateModel) *ActionMapping {
if stateModel == nil || stateModel.States == nil {
return r
}
r.StateModelName = stateModel.Name

borrowerMap := make(map[pr_db.PatronRequestState][]PatronRequestAction)
lenderMap := make(map[pr_db.PatronRequestState][]PatronRequestAction)
Expand All @@ -134,6 +137,10 @@ func NewActionMapping(stateModel *proapi.StateModel) *ActionMapping {
if state.NeedsAttention != nil && *state.NeedsAttention {
currentStateConfig.needsAttention = true
}
if state.ClosingAction != nil && *state.ClosingAction != "" {
closingAction := pr_db.PatronRequestAction(*state.ClosingAction)
currentStateConfig.closingAction = &closingAction
}
Comment thread
JanisSaldabols marked this conversation as resolved.
actionEntries := make([]PatronRequestAction, 0)
if state.Actions != nil {
for _, action := range *state.Actions {
Expand Down Expand Up @@ -351,6 +358,14 @@ func (r *ActionMapping) IsTerminalState(pr pr_db.PatronRequest) bool {
return ok && stateConfig.terminal
}

func (r *ActionMapping) GetClosingAction(pr pr_db.PatronRequest) *pr_db.PatronRequestAction {
config, ok := r.getStateConfig(pr)
if !ok {
return nil
}
return config.closingAction
}

func (r *ActionMapping) getStateConfig(pr pr_db.PatronRequest) (stateConfig, bool) {
if pr.Side == SideBorrowing {
cfg, ok := r.borrowerStateConfig[pr.State]
Expand Down
26 changes: 26 additions & 0 deletions broker/patron_request/service/action_mapping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,32 @@ func TestGetEventTransitionRetryConditionalFromBorrowerWillSupply(t *testing.T)
assert.Equal(t, BorrowerStateRetryPending, transition)
}

func TestGetClosingAction(t *testing.T) {
mapping := mustActionMapping(t)

action := mapping.GetClosingAction(pr_db.PatronRequest{Side: SideLending, State: LenderStateValidated})
assert.NotNil(t, action)
assert.Equal(t, LenderActionCannotSupply, *action)

action = mapping.GetClosingAction(pr_db.PatronRequest{Side: SideLending, State: LenderStateWillSupply})
assert.NotNil(t, action)
assert.Equal(t, LenderActionCannotSupply, *action)

action = mapping.GetClosingAction(pr_db.PatronRequest{Side: SideLending, State: LenderStateConditionPending})
assert.NotNil(t, action)
assert.Equal(t, LenderActionCannotSupply, *action)

action = mapping.GetClosingAction(pr_db.PatronRequest{Side: SideLending, State: LenderStateConditionAccepted})
assert.NotNil(t, action)
assert.Equal(t, LenderActionCannotSupply, *action)

action = mapping.GetClosingAction(pr_db.PatronRequest{Side: SideLending, State: LenderStateShipped})
assert.Nil(t, action)

action = mapping.GetClosingAction(pr_db.PatronRequest{State: LenderStateWillSupply})
assert.Nil(t, action)
}

func listCompare(t *testing.T, list1 []pr_db.PatronRequestAction, list2 []pr_db.PatronRequestAction) {
assert.Equal(t, len(list1), len(list2), "list1=%v, list2=%v", list1, list2)
for i := range list1 {
Expand Down
5 changes: 5 additions & 0 deletions broker/patron_request/service/statemodel.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,11 @@ func ValidateStateModel(stateModel *proapi.StateModel) error {
return fmt.Errorf("primary action %s undefined in state %s side %s", *state.PrimaryAction, state.Name, state.Side)
}

if state.ClosingAction != nil && (state.Actions == nil || !slices.ContainsFunc(*state.Actions, func(a proapi.ModelAction) bool {
return a.Name == string(*state.ClosingAction)
})) {
return fmt.Errorf("closing action %s undefined in state %s side %s", *state.ClosingAction, state.Name, state.Side)
}
if state.Events != nil {
for _, event := range *state.Events {
if !slices.Contains(allowedEvents, event.Name) {
Expand Down
49 changes: 49 additions & 0 deletions broker/patron_request/service/statemodel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,55 @@ func TestValidateStateModelPrimaryActionNoActionsDefined(t *testing.T) {
assert.Equal(t, "primary action other undefined in state NEW side REQUESTER", err.Error())
}

func TestValidateStateModelClosingActionUndefined(t *testing.T) {
s := "ship"
valid := "will-supply"
tt := true
model := &proapi.StateModel{
Type: proapi.StateModelTypeStateModel,
Name: "test",
Version: "1.0.0",
States: []proapi.ModelState{
{
Name: "VALIDATED",
Side: proapi.SUPPLIER,
Initial: &tt,
Actions: &[]proapi.ModelAction{
{Name: valid},
},
PrimaryAction: &valid,
ClosingAction: &s,
},
},
}

err := ValidateStateModel(model)
assert.Error(t, err)
assert.Equal(t, "closing action ship undefined in state VALIDATED side SUPPLIER", err.Error())
}

func TestValidateStateModelClosingActionNoActionsDefined(t *testing.T) {
s := "ship"
tt := true
model := &proapi.StateModel{
Type: proapi.StateModelTypeStateModel,
Name: "test",
Version: "1.0.0",
States: []proapi.ModelState{
{
Name: "VALIDATED",
Side: proapi.SUPPLIER,
Initial: &tt,
ClosingAction: &s,
},
},
}

err := ValidateStateModel(model)
assert.Error(t, err)
assert.Equal(t, "closing action ship undefined in state VALIDATED side SUPPLIER", err.Error())
}

func TestValidateStateModelManualCloseTerminal(t *testing.T) {
tt := true
model := &proapi.StateModel{
Expand Down
40 changes: 27 additions & 13 deletions broker/scheduler/service/batch_action.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,21 @@ const BATCH_COMP = "batch_action"
const TIME_FORMAT = "2006-01-02 15:04:05"

type BatchActionService struct {
eventBus events.EventBus
prRepo pr_db.PrRepo
emailSenderService *EmailSenderService
eventBus events.EventBus
prRepo pr_db.PrRepo
emailSenderService *EmailSenderService
actionMappingService prservice.ActionMappingService
}

func NewBatchActionService(eventBus events.EventBus, prRepo pr_db.PrRepo, emailSenderService *EmailSenderService) *BatchActionService {
return &BatchActionService{
eventBus: eventBus,
prRepo: prRepo,
emailSenderService: emailSenderService,
eventBus: eventBus,
prRepo: prRepo,
emailSenderService: emailSenderService,
actionMappingService: prservice.ActionMappingService{SMService: &prservice.StateModelService{}},
}
}

func (s *BatchActionService) BatchAction(ctx common.ExtendedContext, event events.Event) {
_, _ = s.eventBus.ProcessTask(ctx, event, events.SignalConsumers, s.batchAction)
}
Expand Down Expand Up @@ -141,25 +144,36 @@ func (s *BatchActionService) RequestAging(ctx common.ExtendedContext, event even
var processedCount = 0
if len(prs) > 0 {
for _, pr := range prs {
var action = prservice.BorrowerActionCancelRequest
if pr.Side == prservice.SideLending {
action = prservice.LenderActionCannotSupply
processedCount++
var action *pr_db.PatronRequestAction
actionMapping, mappingErr := s.actionMappingService.GetActionMapping(pr.IllRequest)
if mappingErr != nil {
result.CustomData[pr.ID] = "could not find action mapping for patron request: " + pr.ID + ", error: " + mappingErr.Error()
continue
}
action = actionMapping.GetClosingAction(pr)
if action == nil {
result.CustomData[pr.ID] = "could not find closing action for patron request state: " + string(pr.State) + " within state model: " + actionMapping.StateModelName
continue
}
childBatchActionData := *batchActionData
data := events.EventData{CommonEventData: events.CommonEventData{
Action: &action,
Action: action,
BatchActionData: &childBatchActionData,
}, CustomData: backgroundActionParams(event.EventData.CustomData)}
_, eventErr := s.eventBus.CreateTask(pr.ID, events.EventNameInvokeBackgroundAction, data, events.EventDomainPatronRequest, &event.ID, events.SignalConsumers)
if eventErr != nil {
result.CustomData[pr.ID] = "error creating close action: " + eventErr.Error()
}
processedCount++
}
}
result.Note = "processed patron request count: " + strconv.Itoa(processedCount)

return events.EventStatusSuccess, result
status := events.EventStatusSuccess
if len(result.CustomData) > 0 {
status = events.EventStatusError
result.Note += ", failed: " + strconv.Itoa(len(result.CustomData)) + " with ids and errors in custom data"
}
return status, result
}

func backgroundActionParams(customData map[string]any) map[string]any {
Expand Down
35 changes: 26 additions & 9 deletions broker/scheduler/service/batch_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,10 +323,10 @@ func TestRequestAging_NoPatronRequestsReturnsSuccess(t *testing.T) {
assert.Empty(t, eventBus.createTaskCalls)
}

func TestRequestAging_CreatesBackgroundTasksForBorrowingAndLending(t *testing.T) {
func TestRequestAging_CreatesBackgroundTasksForLending(t *testing.T) {
repo := &mockEmailPrRepo{listResult: []pr_db.PatronRequest{
{ID: "borrowing-1", Side: prservice.SideBorrowing},
{ID: "lending-1", Side: prservice.SideLending},
{ID: "lending-1", Side: prservice.SideLending, State: prservice.LenderStateValidated},
{ID: "lending-2", Side: prservice.SideLending, State: prservice.LenderStateWillSupply},
}}
eventBus := &mockBatchActionEventBus{}
svc := NewBatchActionService(eventBus, repo, nil)
Expand All @@ -337,8 +337,8 @@ func TestRequestAging_CreatesBackgroundTasksForBorrowingAndLending(t *testing.T)
assert.NotNil(t, result)
assert.Equal(t, "processed patron request count: 2", result.Note)
if assert.Len(t, eventBus.createTaskCalls, 2) {
assertRequestAgingCreateTask(t, eventBus.createTaskCalls[0], "borrowing-1", prservice.BorrowerActionCancelRequest)
assertRequestAgingCreateTask(t, eventBus.createTaskCalls[1], "lending-1", prservice.LenderActionCannotSupply)
assertRequestAgingCreateTask(t, eventBus.createTaskCalls[0], "lending-1", prservice.LenderActionCannotSupply)
assertRequestAgingCreateTask(t, eventBus.createTaskCalls[1], "lending-2", prservice.LenderActionCannotSupply)
for _, call := range eventBus.createTaskCalls {
assert.Equal(t, "Closing stale request", call.data.CustomData["note"])
assert.Equal(t, "Expired", call.data.CustomData["reasonUnfilled"])
Expand All @@ -348,19 +348,36 @@ func TestRequestAging_CreatesBackgroundTasksForBorrowingAndLending(t *testing.T)
}
}

func TestRequestAging_FailsIfNoClosingActionForState(t *testing.T) {
repo := &mockEmailPrRepo{listResult: []pr_db.PatronRequest{
{ID: "lending-1", Side: prservice.SideLending, State: prservice.LenderStateShipped},
{ID: "lending-2", Side: prservice.SideLending, State: prservice.LenderStateWillSupply},
}}
eventBus := &mockBatchActionEventBus{}
svc := NewBatchActionService(eventBus, repo, nil)

status, result := svc.RequestAging(testCtx, requestAgingEvent("cql.allRecords=1", map[string]any{"interval": "24h", "note": "Closing stale request", "reasonUnfilled": "Expired"}))

assert.Equal(t, events.EventStatusError, status)
assert.NotNil(t, result)
assert.Equal(t, "processed patron request count: 2, failed: 1 with ids and errors in custom data", result.Note)
assert.Equal(t, "could not find closing action for patron request state: SHIPPED within state model: CrossLink Returnables State Model", result.CustomData["lending-1"])
assert.Len(t, eventBus.createTaskCalls, 1)
}

func TestRequestAging_CreateTaskErrorRecordsCustomDataAndContinues(t *testing.T) {
repo := &mockEmailPrRepo{listResult: []pr_db.PatronRequest{
{ID: "failed-1", Side: prservice.SideBorrowing},
{ID: "ok-1", Side: prservice.SideBorrowing},
{ID: "failed-1", Side: prservice.SideLending, State: prservice.LenderStateValidated},
{ID: "ok-1", Side: prservice.SideLending, State: prservice.LenderStateWillSupply},
}}
eventBus := &mockBatchActionEventBus{createTaskErrByID: map[string]error{"failed-1": errors.New("create failed")}}
svc := NewBatchActionService(eventBus, repo, nil)

status, result := svc.RequestAging(testCtx, requestAgingEvent("cql.allRecords=1", map[string]any{"interval": "24h"}))

assert.Equal(t, events.EventStatusSuccess, status)
assert.Equal(t, events.EventStatusError, status)
assert.NotNil(t, result)
assert.Equal(t, "processed patron request count: 2", result.Note)
assert.Equal(t, "processed patron request count: 2, failed: 1 with ids and errors in custom data", result.Note)
assert.Equal(t, "error creating close action: create failed", result.CustomData["failed-1"])
_, ok := result.CustomData["ok-1"]
assert.False(t, ok)
Expand Down
4 changes: 4 additions & 0 deletions misc/state-model.json
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@
"type": "string",
"description": "Indicates the primary action for the state, i.e., the most common or recommended action to be performed when in the state. The value must match the name of one of the actions defined for the state."
},
"closingAction": {
"type": "string",
"description": "Indicates the closing action for the state, i.e., the action to be performed when closing the state. The value must match the name of one of the actions defined for the state."
},
"editable": {
"type": "boolean",
"description": "Indicates that a patron request can be updated"
Expand Down
4 changes: 4 additions & 0 deletions misc/state-models.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,7 @@ stateModels:
desc: Request is valid
side: SUPPLIER
primaryAction: will-supply
closingAction: cannot-supply
actions:
- name: will-supply
desc: Indicate supplier will supply and send ISO18626 WillSupply
Expand Down Expand Up @@ -373,6 +374,7 @@ stateModels:
desc: After manual will-supply or successful auto-responder
side: SUPPLIER
primaryAction: ship
closingAction: cannot-supply
actions:
- name: add-condition
desc: Add conditions and notify requester
Expand Down Expand Up @@ -400,6 +402,7 @@ stateModels:
desc: Conditions are sent with a special ISO WillSupply
side: SUPPLIER
primaryAction: cannot-supply
closingAction: cannot-supply
actions:
- name: add-condition
desc: Add additional conditions and notify requester
Expand All @@ -425,6 +428,7 @@ stateModels:
desc: After receiving conditions accepted notification
side: SUPPLIER
primaryAction: ship
closingAction: cannot-supply
actions:
- name: add-condition
desc: Add additional conditions and notify requester
Expand Down
Loading