ILLDEV-430 put patron request

broker/oapi/open-api.yaml

@@ -847,6 +847,9 @@ 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.
+        editable:
+          type: boolean
+          description: Indicates that patron request can be updated in this state
         actions:
           type: array
           description: List of all actions that may be performed on the request when in this state
@@ -1829,6 +1832,49 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/Error'
+    put:
+      summary: Update a patron request
+      tags:
+        - patron-requests-api
+      parameters:
+        - in: path
+          name: id
+          schema:
+            type: string
+          required: true
+          description: ID of the patron request to update
+        - $ref: '#/components/parameters/Tenant'
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/CreatePatronRequest'
+      responses:
+        '200':
+          description: Patron request updated successfully
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/PatronRequest'
+        '400':
+          description: Bad Request.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '404':
+          description: Patron request not found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
+        '500':
+          description: Internal Server Error
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
     delete:
       summary: Delete a patron request
       tags:

broker/patron_request/api/api-handler.go

@@ -505,6 +505,81 @@ func isSideParamValid(side *string) bool {
 	return side != nil && (*side == string(prservice.SideBorrowing) || *side == string(prservice.SideLending))
 }
 
+func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *http.Request, id string, params proapi.PutPatronRequestsIdParams) {
+	logParams := map[string]string{"method": "PutPatronRequestsId", "id": id}
+	ctx := common.CreateExtCtxWithArgs(r.Context(), &common.LoggerArgs{Other: logParams})
+
+	var newPr proapi.CreatePatronRequest
+	err := decodeRequiredBody(r, &newPr)
+	if err != nil {
+		api.AddBadRequestError(ctx, w, err)
+		return
+	}
+	tenant, err := a.tenantResolver.Resolve(ctx, r, newPr.RequesterSymbol)
+	if err != nil {
+		api.AddBadRequestError(ctx, w, err)
+		return
+	}
+	symbol, err := tenant.GetRequestSymbol()
+	if err != nil {
+		api.AddBadRequestError(ctx, w, err)
+		return
+	}
+	logParams["symbol"] = symbol
+	ctx = common.CreateExtCtxWithArgs(r.Context(), &common.LoggerArgs{Other: logParams})
+
+	if newPr.Id == nil || *newPr.Id != id {
+		api.AddBadRequestError(ctx, w, fmt.Errorf("patron request id does not match"))
+		return
+	}
+	_, err = a.illRepo.GetIllTransactionByRequesterRequestId(ctx, pgtype.Text{String: id, Valid: true})
+	if err == nil {
+		// request already sent
+		api.AddBadRequestError(ctx, w, fmt.Errorf("request already sent"))
+		return
+	}
+	if !errors.Is(err, pgx.ErrNoRows) {
+		api.AddInternalError(ctx, w, err)
+		return
+	}
+	existingPr, err := a.prRepo.GetPatronRequestById(ctx, id)
+	if err != nil {
+		handleDbError(w, ctx, err)
+		return
+	}
+	if !a.checkOwnership(w, ctx, existingPr.Side, existingPr.RequesterSymbol, existingPr.SupplierSymbol, nil, tenant) {
+		return
+	}
+	newPr.RequesterSymbol = &symbol
+	creationTime := pgtype.Timestamp{Valid: true, Time: time.Now()}
+	illRequest, requesterReqId, err := a.parseAndValidateIllRequest(ctx, &newPr, creationTime.Time)
+	if err != nil {
+		if errors.Is(err, errInvalidPatronRequest) {
+			api.AddBadRequestError(ctx, w, err)
+			return
+		}
+		api.AddInternalError(ctx, w, err)
+		return
+	}
+
+	existingPr.RequesterReqID = getDbText(&requesterReqId)
+	existingPr.IllRequest = illRequest
+	existingPr.Patron = getDbText(newPr.Patron)
+	existingPr.InternalNote = getDbText(newPr.InternalNote)
+	updatedPr, err := a.prRepo.UpdatePatronRequest(ctx, pr_db.UpdatePatronRequestParams(existingPr))
+	if err != nil {
+		api.AddInternalError(ctx, w, err)
+		return
+	}
+	prView, err := a.prRepo.GetPatronRequestSearchView(ctx, updatedPr.ID)
+	if err != nil {
+		api.AddInternalError(ctx, w, err)
+		return
+	}
+	w.Header().Set("Content-Type", "application/json")
+	_ = json.NewEncoder(w).Encode(toApiPatronRequest(r, prView))
+}
+
 func (a *PatronRequestApiHandler) GetPatronRequestsId(w http.ResponseWriter, r *http.Request, id string, params proapi.GetPatronRequestsIdParams) {
 	logParams := map[string]string{"method": "GetPatronRequestsId", "id": id}
 	if params.Side != nil {

broker/patron_request/api/api-handler_test.go

@@ -1355,3 +1355,228 @@ func TestMetadataUpdateAutoModeWithoutIdentifierMerges(t *testing.T) {
 	assert.Equal(t, "4321-4321", req.BibliographicInfo.BibliographicItemId[1].BibliographicItemIdentifier)     // added (not present)
 	assert.Equal(t, "ISSN", req.BibliographicInfo.BibliographicItemId[1].BibliographicItemIdentifierCode.Text) // added (not present)
 }
+
+// --- PutPatronRequestsId tests ---
+
+// illRepoNoTx returns pgx.ErrNoRows for GetIllTransactionByRequesterRequestId,
+// meaning no ILL transaction has been sent for the given patron request yet.
+type illRepoNoTx struct {
+	mocks.MockIllRepositorySuccess
+}
+
+func (r *illRepoNoTx) GetIllTransactionByRequesterRequestId(ctx common.ExtendedContext, requesterRequestID pgtype.Text) (ill_db.IllTransaction, error) {
+	return ill_db.IllTransaction{}, pgx.ErrNoRows
+}
+
+// illRepoReqIdError returns a generic error for GetIllTransactionByRequesterRequestId.
+type illRepoReqIdError struct {
+	mocks.MockIllRepositorySuccess
+}
+
+func (r *illRepoReqIdError) GetIllTransactionByRequesterRequestId(ctx common.ExtendedContext, requesterRequestID pgtype.Text) (ill_db.IllTransaction, error) {
+	return ill_db.IllTransaction{}, errors.New("DB error")
+}
+
+// PrRepoUpdateCapture captures the params passed to UpdatePatronRequest and returns success.
+type PrRepoUpdateCapture struct {
+	PrRepoError
+	lastUpdateParams *pr_db.UpdatePatronRequestParams
+}
+
+func (r *PrRepoUpdateCapture) UpdatePatronRequest(ctx common.ExtendedContext, params pr_db.UpdatePatronRequestParams) (pr_db.PatronRequest, error) {
+	paramsCopy := params
+	r.lastUpdateParams = &paramsCopy
+	return pr_db.PatronRequest(paramsCopy), nil
+}
+
+// PrRepoWrongOwner returns a PR whose RequesterSymbol does not match the requesting symbol.
+type PrRepoWrongOwner struct {
+	PrRepoError
+}
+
+func (r *PrRepoWrongOwner) GetPatronRequestById(ctx common.ExtendedContext, id string) (pr_db.PatronRequest, error) {
+	if id == "3" {
+		return pr_db.PatronRequest{
+			ID:              id,
+			State:           prservice.BorrowerStateNew,
+			Side:            prservice.SideBorrowing,
+			RequesterSymbol: pgtype.Text{String: "ISIL:OTHER", Valid: true},
+		}, nil
+	}
+	return r.PrRepoError.GetPatronRequestById(ctx, id)
+}
+
+func putBody(t *testing.T, id string, illReq iso18626.Request) *bytes.Buffer {
+	t.Helper()
+	toUpdate := proapi.CreatePatronRequest{
+		Id:              &id,
+		RequesterSymbol: &symbol,
+		IllRequest:      illReq,
+	}
+	jsonBytes, err := json.Marshal(toUpdate)
+	assert.NoError(t, err)
+	return bytes.NewBuffer(jsonBytes)
+}
+
+func TestPutPatronRequestsIdMissingBody(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	req, _ := http.NewRequest("PUT", "/", nil)
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+	assert.Contains(t, rr.Body.String(), "body is required")
+}
+
+func TestPutPatronRequestsIdInvalidJson(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	req, _ := http.NewRequest("PUT", "/", bytes.NewBufferString("{bad json"))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+}
+
+func TestPutPatronRequestsIdMissingId(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	body := proapi.CreatePatronRequest{RequesterSymbol: &symbol, IllRequest: validIllRequest()}
+	jsonBytes, _ := json.Marshal(body)
+	req, _ := http.NewRequest("PUT", "/", bytes.NewBuffer(jsonBytes))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+	assert.Contains(t, rr.Body.String(), "patron request id does not match")
+}
+
+func TestPutPatronRequestsIdIdMismatch(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	otherId := "other-id"
+	body := proapi.CreatePatronRequest{Id: &otherId, RequesterSymbol: &symbol, IllRequest: validIllRequest()}
+	jsonBytes, _ := json.Marshal(body)
+	req, _ := http.NewRequest("PUT", "/", bytes.NewBuffer(jsonBytes))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+	assert.Contains(t, rr.Body.String(), "patron request id does not match")
+}
+
+func TestPutPatronRequestsIdRequestAlreadySent(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(mocks.MockIllRepositorySuccess))
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "3", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+	assert.Contains(t, rr.Body.String(), "request already sent")
+}
+
+func TestPutPatronRequestsIdIllRepoError(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoReqIdError))
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "3", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusInternalServerError, rr.Code)
+}
+
+func TestPutPatronRequestsIdNotFound(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "2", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "2", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusNotFound, rr.Code)
+}
+
+func TestPutPatronRequestsIdPrRepoError(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "1", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "1", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusInternalServerError, rr.Code)
+	assert.Contains(t, rr.Body.String(), "DB error")
+}
+
+func TestPutPatronRequestsIdWrongOwner(t *testing.T) {
+	// WithIllRepo on the resolver is required so that IsOwnerOf can look up branch symbols
+	// without erroring; the mock returns no branch symbols for "ISIL:OTHER", producing 404.
+	tenantResolver := tenant.NewResolver().WithIllRepo(new(mocks.MockIllRepositorySuccess))
+	handler := NewPrApiHandler(new(PrRepoWrongOwner), mockEventBus, mockEventRepo, tenantResolver, nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "3", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusNotFound, rr.Code)
+}
+
+func TestPutPatronRequestsIdUpdateError(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "3", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusInternalServerError, rr.Code)
+	assert.Contains(t, rr.Body.String(), "DB error")
+}
+
+func TestPutPatronRequestsIdOK(t *testing.T) {
+	repo := new(PrRepoUpdateCapture)
+	handler := NewPrApiHandler(repo, mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	id := "3"
+	patron := "user-1"
+	note := "staff note"
+	toUpdate := proapi.CreatePatronRequest{
+		Id:              &id,
+		RequesterSymbol: &symbol,
+		IllRequest:      validIllRequest(),
+		Patron:          &patron,
+		InternalNote:    &note,
+	}
+	jsonBytes, _ := json.Marshal(toUpdate)
+	req, _ := http.NewRequest("PUT", "/", bytes.NewBuffer(jsonBytes))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusOK, rr.Code)
+	if assert.NotNil(t, repo.lastUpdateParams) {
+		assert.Equal(t, validIllRequest().BibliographicInfo.Title, repo.lastUpdateParams.IllRequest.BibliographicInfo.Title)
+		assert.True(t, repo.lastUpdateParams.Patron.Valid)
+		assert.Equal(t, patron, repo.lastUpdateParams.Patron.String)
+		assert.True(t, repo.lastUpdateParams.InternalNote.Valid)
+		assert.Equal(t, note, repo.lastUpdateParams.InternalNote.String)
+	}
+	var response proapi.PatronRequest
+	err := json.Unmarshal(rr.Body.Bytes(), &response)
+	assert.NoError(t, err)
+	assert.Equal(t, id, response.Id)
+}
+
+func TestPutPatronRequestsIdEmptyIllRequest(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	id := "3"
+	body := proapi.CreatePatronRequest{Id: &id, RequesterSymbol: &symbol}
+	// IllRequest is zero-valued, which parseAndValidateIllRequest rejects as errInvalidPatronRequest
+	jsonBytes, _ := json.Marshal(body)
+	req, _ := http.NewRequest("PUT", "/", bytes.NewBuffer(jsonBytes))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusBadRequest, rr.Code)
+	assert.Contains(t, rr.Body.String(), "illRequest must not be empty")
+}
+
+func TestPutPatronRequestsIdInvalidBrokerSymbol(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	handler.SetIllRepo(new(illRepoNoTx))
+	previousBrokerSymbol := brokerSymbol
+	brokerSymbol = "BROKER"
+	defer func() {
+		brokerSymbol = previousBrokerSymbol
+	}()
+	req, _ := http.NewRequest("PUT", "/", putBody(t, "3", validIllRequest()))
+	rr := httptest.NewRecorder()
+	handler.PutPatronRequestsId(rr, req, "3", proapi.PutPatronRequestsIdParams{})
+	assert.Equal(t, http.StatusInternalServerError, rr.Code)
+	assert.Contains(t, rr.Body.String(), "invalid BROKER_SYMBOL")
+}

broker/patron_request/service/statemodels/state-models.json

@@ -49,6 +49,7 @@
           "side": "REQUESTER",
           "primaryAction": "send-request",
           "needsAttention": true,
+          "editable": true,
           "actions": [
             {
               "name": "send-request",

misc/state-model.json

@@ -72,6 +72,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."
                 },
+                "editable": {
+                    "type": "boolean",
+                    "description": "Indicates that patron request can be updated"
+                },
                 "actions": {
                     "type": "array",
                     "description": "List of all actions that may be performed on the request when in this state",

misc/state-models.yaml

@@ -38,6 +38,7 @@ stateModels:
         side: REQUESTER
         primaryAction: send-request
         needsAttention: true
+        editable: true
         actions:
           - name: send-request
             desc: Send ISO18626 request to the supplier or broker