Skip to content
Open
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
3 changes: 3 additions & 0 deletions broker/migrations/019_add_attention_field.down.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
DROP VIEW patron_request_search_view ;

ALTER TABLE patron_request DROP COLUMN needs_attention;
24 changes: 24 additions & 0 deletions broker/migrations/019_add_attention_field.up.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
ALTER TABLE patron_request ADD COLUMN needs_attention BOOLEAN NOT NULL DEFAULT false;

CREATE OR REPLACE 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,
EXISTS (
SELECT 1
FROM notification n
WHERE n.pr_id = pr.id and acknowledged_at is null
) AS has_unreaded_notification,
pr.ill_request -> 'serviceInfo' ->> 'serviceType' AS service_type,
pr.ill_request -> 'serviceInfo' -> 'serviceLevel' ->> '#text' AS service_level,
(pr.ill_request -> 'serviceInfo' ->> 'needBeforeDate')::timestamptz AS needed_at
FROM patron_request pr;
9 changes: 9 additions & 0 deletions broker/oapi/open-api.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -465,12 +465,16 @@ components:
requesterRequestId:
type: string
description: Requester patron request ID
needsAttention:
Copy link
Contributor

Choose a reason for hiding this comment

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

@JanisSaldabols would it make sense to ser needsAttention true when action fails?

type: boolean
description: Indicates if the request needs attention
required:
- id
- timestamp
- state
- side
- illRequest
- needsAttention
PatronRequests:
type: object
required:
Expand Down Expand Up @@ -1115,6 +1119,11 @@ paths:
/patron_requests:
get:
summary: Retrieve patron requests
description: |
Use this endpoint to retrieve patron requests.
Query parameter cql can be used to filter the results.
With cql you can use these fields state, side, requester_symbol, supplier_symbol, needs_attention,
has_notification, has_cost, has_unreaded_notification, service_type, service_level, created_at, needed_at.
tags:
- patron-requests-api
parameters:
Expand Down
2 changes: 2 additions & 0 deletions broker/patron_request/api/api-handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,7 @@ func toApiPatronRequest(request pr_db.PatronRequest, illRequest iso18626.Request
SupplierSymbol: toString(request.SupplierSymbol),
IllRequest: utils.Must(common.StructToMap(illRequest)),
RequesterRequestId: toString(request.RequesterReqID),
NeedsAttention: request.NeedsAttention,
}
}

Expand Down Expand Up @@ -593,6 +594,7 @@ func (a *PatronRequestApiHandler) toDbPatronRequest(ctx common.ExtendedContext,
SupplierSymbol: getDbText(request.SupplierSymbol),
IllRequest: illRequest,
Tenant: getDbText(tenant),
RequesterReqID: getDbText(&id),
}, nil
}

Expand Down
47 changes: 36 additions & 11 deletions broker/patron_request/db/prcql.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,30 @@ func handlePatronRequestsQuery(cqlString string, noBaseArgs int) (pgcql.Query, e
f = pgcql.NewFieldString().WithExact()
def.AddField("supplier_symbol", f)

f = pgcql.NewFieldString().WithExact()
def.AddField("needs_attention", f)

f = pgcql.NewFieldString().WithExact()
def.AddField("has_notification", f)

f = pgcql.NewFieldString().WithExact()
def.AddField("has_cost", f)

f = pgcql.NewFieldString().WithExact()
def.AddField("has_unreaded_notification", f)

f = pgcql.NewFieldString().WithExact()
def.AddField("service_type", f)

f = pgcql.NewFieldString().WithExact()
def.AddField("service_level", f)

nf := pgcql.NewFieldDate().WithColumn("timestamp")
def.AddField("created_at", nf)

nf = pgcql.NewFieldDate()
def.AddField("needed_at", nf)

var parser cql.Parser
query, err := parser.Parse(cqlString)
if err != nil {
Expand All @@ -49,7 +73,7 @@ func (q *Queries) ListPatronRequestsCql(ctx context.Context, db DBTX, arg ListPa
if cqlString == nil {
return q.ListPatronRequests(ctx, db, arg)
}
noBaseArgs := 2 // weh have two base arguments: limit and offset
noBaseArgs := 2 // we have two base arguments: limit and offset
res, err := handlePatronRequestsQuery(*cqlString, noBaseArgs)
if err != nil {
return nil, err
Expand All @@ -76,16 +100,17 @@ func (q *Queries) ListPatronRequestsCql(ctx context.Context, db DBTX, arg ListPa
for rows.Next() {
var i ListPatronRequestsRow
if err := rows.Scan(
&i.PatronRequest.ID,
&i.PatronRequest.Timestamp,
&i.PatronRequest.IllRequest,
&i.PatronRequest.State,
&i.PatronRequest.Side,
&i.PatronRequest.Patron,
&i.PatronRequest.RequesterSymbol,
&i.PatronRequest.SupplierSymbol,
&i.PatronRequest.Tenant,
&i.PatronRequest.RequesterReqID,
&i.ID,
&i.Timestamp,
&i.IllRequest,
&i.State,
&i.Side,
&i.Patron,
&i.RequesterSymbol,
&i.SupplierSymbol,
&i.Tenant,
&i.RequesterReqID,
&i.NeedsAttention,
&i.FullCount,
); err != nil {
return nil, err
Expand Down
19 changes: 16 additions & 3 deletions broker/patron_request/db/prrepo.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type PrRepo interface {
UpdatePatronRequest(ctx common.ExtendedContext, params UpdatePatronRequestParams) (PatronRequest, error)
CreatePatronRequest(ctx common.ExtendedContext, params CreatePatronRequestParams) (PatronRequest, error)
DeletePatronRequest(ctx common.ExtendedContext, id string) error
GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.ExtendedContext, supplierSymbol string, requesterReId string) (PatronRequest, error)
GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.ExtendedContext, supplierSymbol string, requesterReId string, side PatronRequestSide) (PatronRequest, error)
GetNextHrid(ctx common.ExtendedContext, prefix string) (string, error)
SaveItem(ctx common.ExtendedContext, params SaveItemParams) (Item, error)
GetItemById(ctx common.ExtendedContext, id string) (Item, error)
Expand Down Expand Up @@ -56,7 +56,19 @@ func (r *PgPrRepo) ListPatronRequests(ctx common.ExtendedContext, params ListPat
fullCount = rows[0].FullCount
for _, r := range rows {
fullCount = r.FullCount
list = append(list, r.PatronRequest)
list = append(list, PatronRequest{
ID: r.ID,
Timestamp: r.Timestamp,
IllRequest: r.IllRequest,
State: PatronRequestState(r.State),
Side: PatronRequestSide(r.Side),
Patron: r.Patron,
RequesterSymbol: r.RequesterSymbol,
SupplierSymbol: r.SupplierSymbol,
Tenant: r.Tenant,
RequesterReqID: r.RequesterReqID,
NeedsAttention: r.NeedsAttention,
})
}
} else {
params.Limit = 1
Expand All @@ -83,7 +95,7 @@ func (r *PgPrRepo) DeletePatronRequest(ctx common.ExtendedContext, id string) er
return r.queries.DeletePatronRequest(ctx, r.GetConnOrTx(), id)
}

func (r *PgPrRepo) GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.ExtendedContext, supplierSymbol string, requesterReId string) (PatronRequest, error) {
func (r *PgPrRepo) GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.ExtendedContext, supplierSymbol string, requesterReId string, side PatronRequestSide) (PatronRequest, error) {
row, err := r.queries.GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx, r.GetConnOrTx(), GetPatronRequestBySupplierSymbolAndRequesterReqIdParams{
SupplierSymbol: pgtype.Text{
String: supplierSymbol,
Expand All @@ -93,6 +105,7 @@ func (r *PgPrRepo) GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.
String: requesterReId,
Valid: true,
},
Side: side,
})
return row.PatronRequest, err
}
Expand Down
4 changes: 2 additions & 2 deletions broker/patron_request/service/action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -731,8 +731,8 @@ func (r *MockPrRepo) CreatePatronRequest(ctx common.ExtendedContext, params pr_d
return pr_db.PatronRequest(params), nil
}

func (r *MockPrRepo) GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.ExtendedContext, symbol string, requesterReqId string) (pr_db.PatronRequest, error) {
args := r.Called(symbol, requesterReqId)
func (r *MockPrRepo) GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx common.ExtendedContext, symbol string, requesterReqId string, side pr_db.PatronRequestSide) (pr_db.PatronRequest, error) {
args := r.Called(symbol, requesterReqId, side)
return args.Get(0).(pr_db.PatronRequest), args.Error(1)
}

Expand Down
4 changes: 2 additions & 2 deletions broker/patron_request/service/message-handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ func (m *PatronRequestMessageHandler) getPatronRequest(ctx common.ExtendedContex
return m.prRepo.GetPatronRequestById(ctx, msg.RequestingAgencyMessage.Header.SupplyingAgencyRequestId)
Copy link
Contributor

Choose a reason for hiding this comment

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

@JanisSaldabols this function should also ensure the side

} else {
symbol := msg.RequestingAgencyMessage.Header.SupplyingAgencyId.AgencyIdType.Text + ":" + msg.RequestingAgencyMessage.Header.SupplyingAgencyId.AgencyIdValue
return m.prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx, symbol, msg.RequestingAgencyMessage.Header.RequestingAgencyRequestId)
return m.prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx, symbol, msg.RequestingAgencyMessage.Header.RequestingAgencyRequestId, SideLending)
}
} else if msg.Request != nil {
return m.prRepo.GetPatronRequestById(ctx, msg.Request.Header.RequestingAgencyRequestId)
Expand Down Expand Up @@ -292,7 +292,7 @@ func (m *PatronRequestMessageHandler) handleRequestMessage(ctx common.ExtendedCo
}
supplierSymbol := request.Header.SupplyingAgencyId.AgencyIdType.Text + ":" + request.Header.SupplyingAgencyId.AgencyIdValue
requesterSymbol := request.Header.RequestingAgencyId.AgencyIdType.Text + ":" + request.Header.RequestingAgencyId.AgencyIdValue
_, err := m.prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx, supplierSymbol, raRequestId)
_, err := m.prRepo.GetPatronRequestBySupplierSymbolAndRequesterReqId(ctx, supplierSymbol, raRequestId, SideLending)
if err != nil {
if !errors.Is(err, pgx.ErrNoRows) {
return createRequestResponse(request, iso18626.TypeMessageStatusERROR, &iso18626.ErrorData{
Expand Down
14 changes: 7 additions & 7 deletions broker/patron_request/service/message-handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func TestGetPatronRequest(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockPrRepo.On("GetPatronRequestById", "req-id-1").Return(pr_db.PatronRequest{ID: "req-id-1"}, nil)
mockPrRepo.On("GetPatronRequestById", "sam-id-1").Return(pr_db.PatronRequest{ID: "sam-id-1"}, nil)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1").Return(pr_db.PatronRequest{ID: "sam-id-1"}, nil)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1", SideLending).Return(pr_db.PatronRequest{ID: "sam-id-1"}, nil)

handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), *new(events.EventBus))
msg := iso18626.ISO18626Message{
Expand Down Expand Up @@ -581,7 +581,7 @@ func TestHandleRequestMessage(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockEventBus := new(MockEventBus)
mockAutoActionRunner := &MockAutoActionRunner{}
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1").Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1", SideLending).Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), mockEventBus)
handler.SetAutoActionRunner(mockAutoActionRunner)

Expand Down Expand Up @@ -614,7 +614,7 @@ func TestHandleRequestMessageAutoActionError(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockEventBus := new(MockEventBus)
mockAutoActionRunner := &MockAutoActionRunner{err: errors.New("auto action failed")}
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1").Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1", SideLending).Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), mockEventBus)
handler.SetAutoActionRunner(mockAutoActionRunner)

Expand All @@ -640,7 +640,7 @@ func TestHandleRequestMessageAutoActionError(t *testing.T) {
func TestHandleRequestMessageMissingRequestId(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockEventBus := new(MockEventBus)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1").Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1", SideLending).Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), mockEventBus)

status, resp, err := handler.handleRequestMessage(appCtx, iso18626.Request{
Expand Down Expand Up @@ -669,7 +669,7 @@ func TestHandleRequestMessageMissingRequestId(t *testing.T) {
func TestHandleRequestMessageExistingRequest(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockEventBus := new(MockEventBus)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1").Return(pr_db.PatronRequest{}, nil)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1", SideLending).Return(pr_db.PatronRequest{}, nil)
handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), mockEventBus)

status, resp, err := handler.handleRequestMessage(appCtx, iso18626.Request{
Expand Down Expand Up @@ -698,7 +698,7 @@ func TestHandleRequestMessageExistingRequest(t *testing.T) {
func TestHandleRequestMessageSearchDbError(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockEventBus := new(MockEventBus)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1").Return(pr_db.PatronRequest{}, errors.New("db error"))
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "req-id-1", SideLending).Return(pr_db.PatronRequest{}, errors.New("db error"))
handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), mockEventBus)

status, resp, err := handler.handleRequestMessage(appCtx, iso18626.Request{
Expand Down Expand Up @@ -727,7 +727,7 @@ func TestHandleRequestMessageSearchDbError(t *testing.T) {
func TestHandleRequestMessageSaveError(t *testing.T) {
mockPrRepo := new(MockPrRepo)
mockEventBus := new(MockEventBus)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "error").Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
mockPrRepo.On("GetPatronRequestBySupplierSymbolAndRequesterReqId", "ISIL:SUP1", "error", SideLending).Return(pr_db.PatronRequest{}, pgx.ErrNoRows)
handler := CreatePatronRequestMessageHandler(mockPrRepo, *new(events.EventRepo), *new(ill_db.IllRepo), mockEventBus)

status, resp, err := handler.handleRequestMessage(appCtx, iso18626.Request{
Expand Down
15 changes: 8 additions & 7 deletions broker/sqlc/pr_query.sql
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ WHERE id = $1
LIMIT 1;

-- name: ListPatronRequests :many
SELECT sqlc.embed(patron_request), COUNT(*) OVER () as full_count
FROM patron_request
SELECT id, timestamp, ill_request, state, side, patron, requester_symbol, supplier_symbol, tenant, requester_req_id, needs_attention, COUNT(*) OVER () as full_count
FROM patron_request_search_view
ORDER BY timestamp
LIMIT $1 OFFSET $2;

Expand All @@ -20,13 +20,14 @@ SET timestamp = $2,
requester_symbol = $7,
supplier_symbol = $8,
tenant = $9,
requester_req_id = $10
requester_req_id = $10,
needs_attention = $11
WHERE id = $1
RETURNING sqlc.embed(patron_request);

-- name: CreatePatronRequest :one
INSERT INTO patron_request (id, timestamp, ill_request, state, side, patron, requester_symbol, supplier_symbol, tenant, requester_req_id)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10)
INSERT INTO patron_request (id, timestamp, ill_request, state, side, patron, requester_symbol, supplier_symbol, tenant, requester_req_id, needs_attention)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11)
RETURNING sqlc.embed(patron_request);

-- name: DeletePatronRequest :exec
Expand All @@ -35,10 +36,10 @@ FROM patron_request
WHERE id = $1;

-- name: GetPatronRequestBySupplierSymbolAndRequesterReqId :one
-- params: supplier_symbol string, requester_req_id string
-- params: supplier_symbol string, requester_req_id string, side string
SELECT sqlc.embed(patron_request)
FROM patron_request
WHERE supplier_symbol = $1 AND requester_req_id = $2
WHERE supplier_symbol = $1 AND requester_req_id = $2 AND side = $3
LIMIT 1;

-- name: GetNextHrid :one
Expand Down
26 changes: 25 additions & 1 deletion broker/sqlc/pr_schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ CREATE TABLE patron_request
requester_symbol VARCHAR,
supplier_symbol VARCHAR,
tenant VARCHAR,
requester_req_id VARCHAR
requester_req_id VARCHAR,
needs_attention BOOLEAN NOT NULL DEFAULT false
);

CREATE OR REPLACE FUNCTION get_next_hrid(prefix VARCHAR) RETURNS VARCHAR AS $$
Expand Down Expand Up @@ -45,3 +46,26 @@ CREATE TABLE notification
created_at TIMESTAMP NOT NULL DEFAULT now(),
acknowledged_at TIMESTAMP
);

CREATE OR REPLACE 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,
EXISTS (
SELECT 1
FROM notification n
WHERE n.pr_id = pr.id and acknowledged_at is null
) AS has_unreaded_notification,
pr.ill_request -> 'serviceInfo' ->> 'serviceType' AS service_type,
pr.ill_request -> 'serviceInfo' -> 'serviceLevel' ->> '#text' AS service_level,
(pr.ill_request -> 'serviceInfo' ->> 'needBeforeDate')::timestamptz AS needed_at
FROM patron_request pr;
5 changes: 3 additions & 2 deletions broker/test/patron_request/api/api-handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,7 @@ func TestCrud(t *testing.T) {
assert.Equal(t, *newPr.RequesterSymbol, *foundPr.RequesterSymbol)
assert.Equal(t, *newPr.SupplierSymbol, *foundPr.SupplierSymbol)
assert.Equal(t, *newPr.Patron, *foundPr.Patron)
assert.Equal(t, false, foundPr.NeedsAttention)

respBytes = httpRequest(t, "POST", basePath, newPrBytes, 400)
assert.Contains(t, string(respBytes), "a patron request with this ID already exists")
Expand Down Expand Up @@ -266,10 +267,10 @@ func TestActionsToCompleteState(t *testing.T) {

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

Expand Down