Skip to content

CROSSLINK-236 action params #496

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
adamdickmeiss wants to merge 6 commits into
base: main
Choose a base branch
from
+275 −26
Open

CROSSLINK-236 action params #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
78a5ce6
CROSSLINK-236 ActionParams relayed (but not tested yet)
adamdickmeiss Mar 27, 2026
d13b408
Testing
adamdickmeiss Mar 27, 2026
8e0ad4b
Test currency too
adamdickmeiss Mar 27, 2026
3a9cacc
Testing, keep shim.RESHARE_ADD_LOAN_CONDITION
adamdickmeiss Mar 27, 2026
286a457
Fix shim.RESHARE_ADD_LOAN_CONDITION usage
adamdickmeiss Mar 27, 2026
12914d6
MapToStruct
adamdickmeiss Mar 27, 2026
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
8 changes: 8 additions & 0 deletions broker/common/common.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ func StructToMap(obj any) (map[string]any, error) {
return result, nil
}

func MapToStruct(obj map[string]any, v any) error {
b, err := json.Marshal(obj)
if err != nil {
return err
}
return json.Unmarshal(b, v)
}

func UnpackItemsNote(note string) ([][]string, int, int) {
startIdx := strings.Index(note, MULTIPLE_ITEMS)
endIdx := strings.Index(note, MULTIPLE_ITEMS_END)
Expand Down
40 changes: 40 additions & 0 deletions broker/common/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,46 @@ func TestStructToMap(t *testing.T) {
}
}

func TestMapToStruct(t *testing.T) {
tests := []struct {
name string
input map[string]interface{}
want User
wantErr bool
}{
{
name: "Basic map conversion",
input: map[string]interface{}{
"id": float64(1),
"name": "Alice",
"Active": true,
},
want: User{ID: 1, Name: &alice, Active: true},
wantErr: false,
},
{
name: "42 as string instead of int",
input: map[string]interface{}{"id": "42"},
want: User{},
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var got User
err := MapToStruct(tt.input, &got)
if (err != nil) != tt.wantErr {
t.Errorf("MapToStruct() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("MapToStruct() got = %v, want %v", got, tt.want)
}
})
}
}

func TestUnpackItemsNote(t *testing.T) {
// Just ID
note := MULTIPLE_ITEMS + "\n1\n" + MULTIPLE_ITEMS_END
Expand Down
106 changes: 85 additions & 21 deletions broker/patron_request/service/action.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"net/http"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -46,6 +47,14 @@ func (e *autoActionFailure) Error() string {
return e.msg
}

type ActionParams struct {
Note string `json:"note,omitempty"`
LoanCondition string `json:"loanCondition,omitempty"`
Cost *float64 `json:"cost,omitempty"`
Currency string `json:"currency,omitempty"`
ReasonUnfilled string `json:"reasonUnfilled,omitempty"`
}
Comment on lines +50 to +56
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActionParams.Cost is modeled as *float64, and later converted via FormatFloat -> ExtractDecimal. Using floating point for monetary values can introduce rounding artifacts (e.g., 12.34 may format to a longer representation), resulting in incorrect MonetaryValue being sent. Consider representing cost as a decimal-safe type in params (e.g., string/json.Number and parse, or accept base/exp directly) to preserve exact values.

Copilot uses AI. Check for mistakes.

func CreatePatronRequestActionService(prRepo pr_db.PrRepo, eventBus events.EventBus, iso18626Handler handler.Iso18626HandlerInterface, lmsCreator lms.LmsCreator) *PatronRequestActionService {
return &PatronRequestActionService{
prRepo: prRepo,
Expand Down Expand Up @@ -267,7 +276,7 @@ func (a *PatronRequestActionService) handleBorrowingAction(ctx common.ExtendedCo
}
}

func (a *PatronRequestActionService) handleLenderAction(ctx common.ExtendedContext, action pr_db.PatronRequestAction, pr pr_db.PatronRequest, illRequest iso18626.Request, actionParams map[string]interface{}, eventID *string) actionExecutionResult {
func (a *PatronRequestActionService) handleLenderAction(ctx common.ExtendedContext, action pr_db.PatronRequestAction, pr pr_db.PatronRequest, illRequest iso18626.Request, actionCustomData map[string]any, eventID *string) actionExecutionResult {
if !pr.SupplierSymbol.Valid {
status, result := a.logErrorAndReturnResult(ctx, "missing supplier symbol", nil)
return actionExecutionResult{status: status, result: result, pr: pr}
Expand All @@ -291,19 +300,26 @@ func (a *PatronRequestActionService) handleLenderAction(ctx common.ExtendedConte
ctx.Logger().Error("failed to create LMS log event", "error", createErr)
}
})

var actionParams ActionParams
err = common.MapToStruct(actionCustomData, &actionParams)
if err != nil {
status, result := a.logErrorAndReturnResult(ctx, "failed to unmarshal action parameters", err)
return actionExecutionResult{status: status, result: result, pr: pr}
}
switch action {
case LenderActionValidate:
return a.validateLenderRequest(ctx, pr, lms)
case LenderActionWillSupply:
return a.willSupplyLenderRequest(ctx, pr, lms, illRequest)
return a.willSupplyLenderRequest(ctx, pr, lms, illRequest, actionParams)
case LenderActionRejectCancel:
return a.rejectCancelLenderRequest(ctx, pr)
case LenderActionCannotSupply:
return a.cannotSupplyLenderRequest(ctx, pr)
return a.cannotSupplyLenderRequest(ctx, pr, actionParams)
case LenderActionAddCondition:
return a.addConditionsLenderRequest(ctx, pr, actionParams)
case LenderActionShip:
return a.shipLenderRequest(ctx, pr, lms, illRequest)
return a.shipLenderRequest(ctx, pr, lms, illRequest, actionParams)
case LenderActionMarkReceived:
return a.markReceivedLenderRequest(ctx, pr, lms, illRequest)
case LenderActionAcceptCancel:
Expand Down Expand Up @@ -597,7 +613,7 @@ func (a *PatronRequestActionService) validateLenderRequest(ctx common.ExtendedCo
return actionExecutionResult{status: events.EventStatusSuccess, pr: pr}
}

func (a *PatronRequestActionService) willSupplyLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request) actionExecutionResult {
func (a *PatronRequestActionService) willSupplyLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request, actionParams ActionParams) actionExecutionResult {
itemId := illRequest.BibliographicInfo.SupplierUniqueRecordId
requestId := illRequest.Header.RequestingAgencyRequestId
userId := lmsAdapter.InstitutionalPatron(pr.RequesterSymbol.String)
Expand Down Expand Up @@ -625,28 +641,65 @@ func (a *PatronRequestActionService) willSupplyLenderRequest(ctx common.Extended
return actionExecutionResult{status: status, result: result, pr: pr}
}
result := events.EventResult{}
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result, iso18626.MessageInfo{ReasonForMessage: iso18626.TypeReasonForMessageStatusChange}, iso18626.StatusInfo{Status: iso18626.TypeStatusWillSupply})
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,
iso18626.MessageInfo{
ReasonForMessage: iso18626.TypeReasonForMessageStatusChange,
Note: actionParams.Note,
},
iso18626.StatusInfo{Status: iso18626.TypeStatusWillSupply},
nil)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

func (a *PatronRequestActionService) cannotSupplyLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest) actionExecutionResult {
func (a *PatronRequestActionService) cannotSupplyLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, actionParams ActionParams) actionExecutionResult {
result := events.EventResult{}
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result, iso18626.MessageInfo{ReasonForMessage: iso18626.TypeReasonForMessageStatusChange}, iso18626.StatusInfo{Status: iso18626.TypeStatusUnfilled})
var reasonUnfilled *iso18626.TypeSchemeValuePair
if actionParams.ReasonUnfilled != "" {
reasonUnfilled = &iso18626.TypeSchemeValuePair{Text: actionParams.ReasonUnfilled}
}
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,
iso18626.MessageInfo{
ReasonForMessage: iso18626.TypeReasonForMessageStatusChange,
Note: actionParams.Note,
ReasonUnfilled: reasonUnfilled,
},
iso18626.StatusInfo{Status: iso18626.TypeStatusUnfilled},
nil)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

func (a *PatronRequestActionService) addConditionsLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, actionParams map[string]interface{}) actionExecutionResult {
func (a *PatronRequestActionService) addConditionsLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, actionParams ActionParams) actionExecutionResult {
var offeredCosts *iso18626.TypeCosts
if actionParams.Cost != nil {
if actionParams.Currency == "" {
status, result := a.logErrorAndReturnResult(ctx, "currency is required when cost is provided", nil)
return actionExecutionResult{status: status, result: result, pr: pr}
}
_, costBase, costExp := utils.ExtractDecimal(strconv.FormatFloat(*actionParams.Cost, 'f', -1, 64), -1)
offeredCosts = &iso18626.TypeCosts{
CurrencyCode: iso18626.TypeSchemeValuePair{Text: actionParams.Currency},
MonetaryValue: utils.XSDDecimal{Base: costBase, Exp: costExp},
}
Comment on lines +671 to +682
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ActionParams.Cost is modeled as a float64 and then converted to an XSD decimal via FormatFloat/ExtractDecimal. Using binary floating point for money can produce surprising decimal expansions (and therefore incorrect Base/Exp) for some inputs. Consider accepting cost as a string/decimal representation (or as explicit base/exp) and converting deterministically, to avoid rounding artifacts.

Copilot uses AI. Check for mistakes.
}
var deliveryInfo *iso18626.DeliveryInfo
if actionParams.LoanCondition != "" {
deliveryInfo = &iso18626.DeliveryInfo{
LoanCondition: &iso18626.TypeSchemeValuePair{Text: actionParams.LoanCondition},
}
}
result := events.EventResult{}
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,
iso18626.MessageInfo{
ReasonForMessage: iso18626.TypeReasonForMessageNotification,
Note: shim.RESHARE_ADD_LOAN_CONDITION, // TODO add action params
Note: actionParams.Note + shim.RESHARE_ADD_LOAN_CONDITION,
OfferedCosts: offeredCosts,
},
Comment on lines 692 to 696
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actionParams.Note is concatenated directly with the #ReShareAddLoanCondition# marker. If a note is provided, add a separator (space/newline) before the marker to avoid the marker being glued to the last word of the note and to keep the combined field easier to read/debug.

Copilot uses AI. Check for mistakes.
iso18626.StatusInfo{Status: iso18626.TypeStatusWillSupply})
iso18626.StatusInfo{Status: iso18626.TypeStatusWillSupply},
deliveryInfo)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

func (a *PatronRequestActionService) shipLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request) actionExecutionResult {
func (a *PatronRequestActionService) shipLenderRequest(ctx common.ExtendedContext, pr pr_db.PatronRequest, lmsAdapter lms.LmsAdapter, illRequest iso18626.Request, actionParams ActionParams) actionExecutionResult {
requestId := illRequest.Header.RequestingAgencyRequestId
userId := lmsAdapter.InstitutionalPatron(pr.RequesterSymbol.String)
externalReferenceValue := ""
Expand Down Expand Up @@ -680,11 +733,15 @@ func (a *PatronRequestActionService) shipLenderRequest(ctx common.ExtendedContex
}
}
}
note := encodeItemsNote(items)
note := actionParams.Note + encodeItemsNote(items)
result := events.EventResult{}
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,
iso18626.MessageInfo{ReasonForMessage: iso18626.TypeReasonForMessageStatusChange, Note: note},
iso18626.StatusInfo{Status: iso18626.TypeStatusLoaned})
iso18626.MessageInfo{
ReasonForMessage: iso18626.TypeReasonForMessageStatusChange,
Note: note,
},
Comment on lines +736 to +742
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When composing the shipped-message note, actionParams.Note is concatenated directly with the #MultipleItems#... payload. If a human note is provided, consider inserting a delimiter (e.g., newline) before the encoded items block to keep the note readable and consistent with other code paths that prepend notes before item markers.

Copilot uses AI. Check for mistakes.
iso18626.StatusInfo{Status: iso18626.TypeStatusLoaned},
nil)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

Expand Down Expand Up @@ -718,7 +775,11 @@ func (a *PatronRequestActionService) markReceivedLenderRequest(ctx common.Extend
}
}
result := events.EventResult{}
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result, iso18626.MessageInfo{ReasonForMessage: iso18626.TypeReasonForMessageStatusChange}, iso18626.StatusInfo{Status: iso18626.TypeStatusLoanCompleted})
status, eventResult, httpStatus := a.sendSupplyingAgencyMessage(ctx, pr, &result,
iso18626.MessageInfo{
ReasonForMessage: iso18626.TypeReasonForMessageStatusChange},
iso18626.StatusInfo{Status: iso18626.TypeStatusLoanCompleted},
nil)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

Expand All @@ -730,7 +791,8 @@ func (a *PatronRequestActionService) rejectCancelLenderRequest(ctx common.Extend
ReasonForMessage: iso18626.TypeReasonForMessageCancelResponse,
AnswerYesNo: &no,
},
iso18626.StatusInfo{Status: iso18626.TypeStatusWillSupply})
iso18626.StatusInfo{Status: iso18626.TypeStatusWillSupply},
nil)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

Expand All @@ -742,11 +804,12 @@ func (a *PatronRequestActionService) acceptCancelLenderRequest(ctx common.Extend
ReasonForMessage: iso18626.TypeReasonForMessageCancelResponse,
AnswerYesNo: &yes,
},
iso18626.StatusInfo{Status: iso18626.TypeStatusCancelled})
iso18626.StatusInfo{Status: iso18626.TypeStatusCancelled},
nil)
return a.checkSupplyingResponse(status, eventResult, &result, httpStatus, pr)
}

func (a *PatronRequestActionService) sendSupplyingAgencyMessage(ctx common.ExtendedContext, pr pr_db.PatronRequest, result *events.EventResult, messageInfo iso18626.MessageInfo, statusInfo iso18626.StatusInfo) (events.EventStatus, *events.EventResult, *int) {
func (a *PatronRequestActionService) sendSupplyingAgencyMessage(ctx common.ExtendedContext, pr pr_db.PatronRequest, result *events.EventResult, messageInfo iso18626.MessageInfo, statusInfo iso18626.StatusInfo, deliveryInfo *iso18626.DeliveryInfo) (events.EventStatus, *events.EventResult, *int) {
if !pr.RequesterSymbol.Valid {
status, eventResult := a.logErrorAndReturnResult(ctx, "missing requester symbol", nil)
return status, eventResult, nil
Expand All @@ -773,8 +836,9 @@ func (a *PatronRequestActionService) sendSupplyingAgencyMessage(ctx common.Exten
RequestingAgencyRequestId: pr.RequesterReqID.String,
SupplyingAgencyRequestId: pr.ID,
},
MessageInfo: messageInfo,
StatusInfo: statusInfo,
MessageInfo: messageInfo,
StatusInfo: statusInfo,
DeliveryInfo: deliveryInfo,
},
}
if illMessage.SupplyingAgencyMessage.StatusInfo.LastChange.IsZero() {
Expand Down
Loading
Loading