Skip to content

CROSSLINK-208 Make side as optional parameter #389

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

Merged
JanisSaldabols merged 2 commits into from
Feb 5, 2026
Merged

CROSSLINK-208 Make side as optional parameter #389

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b9cfc06
CROSSLINK-208 Make side as optional parameter
JanisSaldabols Feb 4, 2026
42e26f0
CROSSLINK-208 Make side as optional parameter
JanisSaldabols Feb 5, 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
1 change: 0 additions & 1 deletion broker/oapi/open-api.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ components:
name: side
in: query
description: Patron request side, valid values "borrowing" and "lending"
required: true
schema:
type: string
Symbol:
Expand Down
53 changes: 34 additions & 19 deletions broker/patron_request/api/api-handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ func (a *PatronRequestApiHandler) GetStateModelModelsModel(w http.ResponseWriter

func (a *PatronRequestApiHandler) GetPatronRequests(w http.ResponseWriter, r *http.Request, params proapi.GetPatronRequestsParams) {
symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol)
logParams := map[string]string{"method": "GetPatronRequests", "side": params.Side, "symbol": symbol}
logParams := map[string]string{"method": "GetPatronRequests", "symbol": symbol}
if params.Side != nil {
logParams["side"] = *params.Side
}
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{
Other: logParams,
})
Expand Down Expand Up @@ -84,8 +87,8 @@ func (a *PatronRequestApiHandler) GetPatronRequests(w http.ResponseWriter, r *ht
}
var side pr_db.PatronRequestSide
if isSideParamValid(params.Side) {
side = pr_db.PatronRequestSide(params.Side)
_, err = qb.And().Search("side").Term(params.Side).Build()
side = pr_db.PatronRequestSide(*params.Side)
_, err = qb.And().Search("side").Term(*params.Side).Build()
if err != nil {
addBadRequestError(ctx, w, err)
return
Expand Down Expand Up @@ -205,9 +208,12 @@ func (a *PatronRequestApiHandler) PostPatronRequests(w http.ResponseWriter, r *h

func (a *PatronRequestApiHandler) DeletePatronRequestsId(w http.ResponseWriter, r *http.Request, id string, params proapi.DeletePatronRequestsIdParams) {
symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol)
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{
Other: map[string]string{"method": "DeletePatronRequestsId", "id": id, "side": params.Side, "symbol": symbol},
})
logParams := map[string]string{"method": "DeletePatronRequestsId", "id": id, "symbol": symbol}
if params.Side != nil {
logParams["side"] = *params.Side
}
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams})

if err != nil {
addBadRequestError(ctx, w, err)
return
Expand All @@ -231,22 +237,22 @@ func (a *PatronRequestApiHandler) DeletePatronRequestsId(w http.ResponseWriter,
w.WriteHeader(http.StatusNoContent)
}

func (a *PatronRequestApiHandler) getPatronRequestById(ctx common.ExtendedContext, id string, side string, symbol string) (*pr_db.PatronRequest, error) {
func (a *PatronRequestApiHandler) getPatronRequestById(ctx common.ExtendedContext, id string, side *string, symbol string) (*pr_db.PatronRequest, error) {
pr, err := a.prRepo.GetPatronRequestById(ctx, id)
if err != nil {
if errors.Is(err, pgx.ErrNoRows) {
return nil, nil
}
return nil, err
}
if isOwner(pr, symbol) && (!isSideParamValid(side) || string(pr.Side) == side) {
if isOwner(pr, symbol) && (!isSideParamValid(side) || string(pr.Side) == *side) {
return &pr, nil
}
return nil, nil
}

func isSideParamValid(side string) bool {
return side == string(prservice.SideBorrowing) || side == string(prservice.SideLending)
func isSideParamValid(side *string) bool {
return side != nil && (*side == string(prservice.SideBorrowing) || *side == string(prservice.SideLending))
}

func isOwner(pr pr_db.PatronRequest, symbol string) bool {
Expand All @@ -256,9 +262,12 @@ func isOwner(pr pr_db.PatronRequest, symbol string) bool {

func (a *PatronRequestApiHandler) GetPatronRequestsId(w http.ResponseWriter, r *http.Request, id string, params proapi.GetPatronRequestsIdParams) {
symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol)
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{
Other: map[string]string{"method": "GetPatronRequestsId", "id": id, "side": params.Side, "symbol": symbol},
})
logParams := map[string]string{"method": "GetPatronRequestsId", "id": id, "symbol": symbol}
if params.Side != nil {
logParams["side"] = *params.Side
}
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams})

if err != nil {
addBadRequestError(ctx, w, err)
return
Expand All @@ -283,9 +292,12 @@ func (a *PatronRequestApiHandler) GetPatronRequestsId(w http.ResponseWriter, r *

func (a *PatronRequestApiHandler) GetPatronRequestsIdActions(w http.ResponseWriter, r *http.Request, id string, params proapi.GetPatronRequestsIdActionsParams) {
symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol)
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{
Other: map[string]string{"method": "GetPatronRequestsIdActions", "id": id, "side": params.Side, "symbol": symbol},
})
logParams := map[string]string{"method": "GetPatronRequestsIdActions", "id": id, "symbol": symbol}
if params.Side != nil {
logParams["side"] = *params.Side
}
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams})

if err != nil {
addBadRequestError(ctx, w, err)
return
Expand All @@ -305,9 +317,12 @@ func (a *PatronRequestApiHandler) GetPatronRequestsIdActions(w http.ResponseWrit

func (a *PatronRequestApiHandler) PostPatronRequestsIdAction(w http.ResponseWriter, r *http.Request, id string, params proapi.PostPatronRequestsIdActionParams) {
symbol, err := api.GetSymbolForRequest(r, a.tenant, params.XOkapiTenant, params.Symbol)
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{
Other: map[string]string{"method": "PostPatronRequestsIdAction", "id": id, "side": params.Side, "symbol": symbol},
})
logParams := map[string]string{"method": "PostPatronRequestsIdAction", "id": id, "symbol": symbol}
if params.Side != nil {
logParams["side"] = *params.Side
}
ctx := common.CreateExtCtxWithArgs(context.Background(), &common.LoggerArgs{Other: logParams})

if err != nil {
addBadRequestError(ctx, w, err)
return
Expand Down
29 changes: 16 additions & 13 deletions broker/patron_request/api/api-handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ import (

var mockEventBus = new(MockEventBus)
var symbol = "ISIL:REQ"
var lendingString = string(prservice.SideLending)
var proapiBorrowingSide = proapi.Side(prservice.SideBorrowing)
var proapiLendingSide = proapi.Side(prservice.SideLending)

func TestGetId(t *testing.T) {
assert.True(t, getId("") != "")
Expand All @@ -46,7 +49,7 @@ func TestGetPatronRequests(t *testing.T) {
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
params := proapi.GetPatronRequestsParams{
Side: string(prservice.SideLending),
Side: &lendingString,
Symbol: &symbol,
}
handler.GetPatronRequests(rr, req, params)
Expand All @@ -59,7 +62,7 @@ func TestGetPatronRequestsNoSymbol(t *testing.T) {
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
params := proapi.GetPatronRequestsParams{
Side: string(prservice.SideLending),
Side: &lendingString,
}
handler.GetPatronRequests(rr, req, params)
assert.Equal(t, http.StatusBadRequest, rr.Code)
Expand All @@ -74,7 +77,7 @@ func TestGetPatronRequestsWithLimits(t *testing.T) {
limit := proapi.Limit(10)
cql := "state = NEW"
params := proapi.GetPatronRequestsParams{
Side: string(prservice.SideLending),
Side: &lendingString,
Symbol: &symbol,
Offset: &offset,
Limit: &limit,
Expand Down Expand Up @@ -153,7 +156,7 @@ func TestDeletePatronRequestsId(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("POST", "/", nil)
rr := httptest.NewRecorder()
handler.DeletePatronRequestsId(rr, req, "3", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)})
handler.DeletePatronRequestsId(rr, req, "3", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusInternalServerError, rr.Code)
assert.Contains(t, rr.Body.String(), "DB error")
}
Expand All @@ -162,7 +165,7 @@ func TestDeletePatronRequestsIdDeleted(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("POST", "/", nil)
rr := httptest.NewRecorder()
handler.DeletePatronRequestsId(rr, req, "4", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)})
handler.DeletePatronRequestsId(rr, req, "4", proapi.DeletePatronRequestsIdParams{Symbol: &symbol, Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusNoContent, rr.Code)
}

Expand Down Expand Up @@ -196,7 +199,7 @@ func TestGetPatronRequestsIdActions(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)})
handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusOK, rr.Code)
assert.Contains(t, rr.Body.String(), "validate")
}
Expand All @@ -205,7 +208,7 @@ func TestGetPatronRequestsIdActionsNoSymbol(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Side: proapi.Side(prservice.SideBorrowing)})
handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusBadRequest, rr.Code)
assert.Contains(t, rr.Body.String(), "symbol must be specified")
}
Expand All @@ -214,7 +217,7 @@ func TestGetPatronRequestsIdActionsDbError(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.GetPatronRequestsIdActions(rr, req, "1", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)})
handler.GetPatronRequestsIdActions(rr, req, "1", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusInternalServerError, rr.Code)
assert.Contains(t, rr.Body.String(), "DB error")
}
Expand All @@ -223,7 +226,7 @@ func TestGetPatronRequestsIdActionsNotFoundBecauseOfSide(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: proapi.Side(prservice.SideLending)})
handler.GetPatronRequestsIdActions(rr, req, "3", proapi.GetPatronRequestsIdActionsParams{Symbol: &symbol, Side: &proapiLendingSide})
assert.Equal(t, http.StatusNotFound, rr.Code)
assert.Contains(t, rr.Body.String(), "not found")
}
Expand All @@ -232,7 +235,7 @@ func TestPostPatronRequestsIdActionNoSymbol(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Side: proapi.Side(prservice.SideBorrowing)})
handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusBadRequest, rr.Code)
assert.Contains(t, rr.Body.String(), "symbol must be specified")
}
Expand All @@ -241,7 +244,7 @@ func TestPostPatronRequestsIdActionDbError(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.PostPatronRequestsIdAction(rr, req, "1", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)})
handler.PostPatronRequestsIdAction(rr, req, "1", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusInternalServerError, rr.Code)
assert.Contains(t, rr.Body.String(), "DB error")
}
Expand All @@ -250,7 +253,7 @@ func TestPostPatronRequestsIdActionNotFoundBecauseOfSide(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", nil)
rr := httptest.NewRecorder()
handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: proapi.Side(prservice.SideLending)})
handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: &proapiLendingSide})
assert.Equal(t, http.StatusNotFound, rr.Code)
assert.Contains(t, rr.Body.String(), "not found")
}
Expand All @@ -259,7 +262,7 @@ func TestPostPatronRequestsIdActionErrorParsing(t *testing.T) {
handler := NewPrApiHandler(new(PrRepoError), mockEventBus, common.NewTenant(""), 10)
req, _ := http.NewRequest("GET", "/", strings.NewReader("{"))
rr := httptest.NewRecorder()
handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: proapi.Side(prservice.SideBorrowing)})
handler.PostPatronRequestsIdAction(rr, req, "3", proapi.PostPatronRequestsIdActionParams{Symbol: &symbol, Side: &proapiBorrowingSide})
assert.Equal(t, http.StatusBadRequest, rr.Code)
assert.Contains(t, rr.Body.String(), "unexpected EOF")
}
Expand Down
28 changes: 20 additions & 8 deletions broker/test/patron_request/api/api-handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/http"
"os"
"strconv"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -123,13 +124,27 @@ func TestCrud(t *testing.T) {
assert.Equal(t, int64(1), foundPrs.About.Count)
assert.Equal(t, *newPr.Id, foundPrs.Items[0].Id)

// GET by id
// GET list with offset in
respBytes = httpRequest(t, "GET", basePath+queryParams+"&offset=100000", []byte{}, 200)
err = json.Unmarshal(respBytes, &foundPrs)
assert.NoError(t, err, "failed to unmarshal patron request")

assert.Equal(t, int64(1), foundPrs.About.Count)
assert.Len(t, foundPrs.Items, 0)

// GET by id with symbol and side
thisPrPath := basePath + "/" + *newPr.Id
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)

// GET by id with symbol
respBytes = httpRequest(t, "GET", thisPrPath+"?symbol="+*foundPr.RequesterSymbol, []byte{}, 200)
err = json.Unmarshal(respBytes, &foundPr)
assert.NoError(t, err, "failed to unmarshal patron request")
assert.Equal(t, *newPr.Id, foundPr.Id)

// GET actions by PR id
test.WaitForPredicateToBeTrue(func() bool {
respBytes = httpRequest(t, "GET", thisPrPath+"/actions"+queryParams, []byte{}, 200)
Expand Down Expand Up @@ -187,9 +202,7 @@ func TestActionsToCompleteState(t *testing.T) {
SupplierUniqueRecordId: "return-" + supplierSymbol + "::WILLSUPPLY_LOANED",
},
}
id := uuid.NewString()
newPr := proapi.CreatePatronRequest{
Id: &id,
SupplierSymbol: &supplierSymbol,
RequesterSymbol: &requesterSymbol,
Patron: &patron,
Expand All @@ -204,8 +217,8 @@ func TestActionsToCompleteState(t *testing.T) {
err = json.Unmarshal(respBytes, &foundPr)
assert.NoError(t, err, "failed to unmarshal patron request")

assert.Equal(t, *newPr.Id, foundPr.Id)
requesterPrPath := basePath + "/" + *newPr.Id
assert.Equal(t, strings.ToUpper(strings.Split(requesterSymbol, ":")[1]+"-1"), foundPr.Id)
requesterPrPath := basePath + "/" + foundPr.Id
queryParams := "?side=borrowing&symbol=" + *foundPr.RequesterSymbol

// Wait till action available
Expand All @@ -224,10 +237,10 @@ func TestActionsToCompleteState(t *testing.T) {

// Find supplier patron request
test.WaitForPredicateToBeTrue(func() bool {
supPr, _ := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, *newPr.Id)
supPr, _ := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, foundPr.Id)
return supPr.ID != ""
})
supPr, err := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, *newPr.Id)
supPr, err := prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(appCtx, supplierSymbol, foundPr.Id)
assert.NoError(t, err)
assert.NotNil(t, supPr.ID)

Expand Down Expand Up @@ -342,7 +355,6 @@ func TestActionsToCompleteState(t *testing.T) {
respBytes = httpRequest(t, "GET", requesterPrPath+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, string(prservice.BorrowerStateCompleted), foundPr.State)

// Check supplier patron request done
Expand Down
Loading