Tests, guards before ill trans check

Author: adamdickmeiss

broker/patron_request/api/api-handler.go

@@ -550,6 +550,14 @@ func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *
 		api.AddBadRequestError(ctx, w, fmt.Errorf("only borrower-side patron requests can be updated"))
 		return
 	}
+	if !existingPr.RequesterSymbol.Valid || existingPr.RequesterSymbol.String == "" {
+		api.AddInternalError(ctx, w, fmt.Errorf("existing patron request missing requesterSymbol"))
+		return
+	}
+	if newPr.RequesterSymbol != nil && *newPr.RequesterSymbol != "" && *newPr.RequesterSymbol != existingPr.RequesterSymbol.String {
+		api.AddBadRequestError(ctx, w, fmt.Errorf("requesterSymbol does not match existing patron request"))
+		return
+	}
 	if a.illRepo == nil {
 		api.AddInternalError(ctx, w, errors.New("illRepo is not configured"))
 		return
@@ -564,14 +572,6 @@ func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *
 		api.AddInternalError(ctx, w, err)
 		return
 	}
-	if !existingPr.RequesterSymbol.Valid || existingPr.RequesterSymbol.String == "" {
-		api.AddInternalError(ctx, w, fmt.Errorf("existing patron request missing requesterSymbol"))
-		return
-	}
-	if newPr.RequesterSymbol != nil && *newPr.RequesterSymbol != "" && *newPr.RequesterSymbol != existingPr.RequesterSymbol.String {
-		api.AddBadRequestError(ctx, w, fmt.Errorf("requesterSymbol does not match existing patron request"))
-		return
-	}
 	symbol = existingPr.RequesterSymbol.String
 	newPr.RequesterSymbol = &symbol
 	creationTime := time.Now()

broker/patron_request/api/api-handler_test.go

@@ -1413,6 +1413,25 @@ type PrRepoLendingSide struct {
 	PrRepoError
 }
 
+// PrRepoBranchOwner returns a borrowing-side PR for id "3" whose RequesterSymbol
+// is "ISIL:S1" — the branch symbol that MockIllRepositorySuccess returns for any
+// peer, allowing a test to reach the requesterSymbol mismatch check.
+type PrRepoBranchOwner struct {
+	PrRepoError
+}
+
+func (r *PrRepoBranchOwner) 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:S1", Valid: true},
+		}, nil
+	}
+	return r.PrRepoError.GetPatronRequestById(ctx, id)
+}
+
 func (r *PrRepoWrongOwner) GetPatronRequestById(ctx common.ExtendedContext, id string) (pr_db.PatronRequest, error) {
 	if id == "3" {
 		return pr_db.PatronRequest{
@@ -1466,6 +1485,17 @@ func TestPutPatronRequestsIdInvalidJson(t *testing.T) {
 	assert.Equal(t, http.StatusBadRequest, rr.Code)
 }
 
+func TestPutPatronRequestsIdEmptySymbol(t *testing.T) {
+	handler := NewPrApiHandler(new(PrRepoError), mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
+	body := proapi.CreatePatronRequest{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(), "symbol must be specified")
+}
+
 func TestPutPatronRequestsIdMissingId(t *testing.T) {
 	repo := new(PrRepoUpdateCapture)
 	handler := NewPrApiHandler(repo, mockEventBus, mockEventRepo, tenant.NewResolver(), nil, 10)
@@ -1568,6 +1598,23 @@ func TestPutPatronRequestsIdNotBorrowingSide(t *testing.T) {
 	assert.Contains(t, rr.Body.String(), "only borrower-side patron requests can be updated")
 }
 
+func TestPutPatronRequestsIdRequesterSymbolMismatch(t *testing.T) {
+	// "ISIL:DIFFERENT" is the body/request symbol; MockIllRepositorySuccess returns
+	// "ISIL:S1" as a branch of that symbol, so ownership passes. The existing PR
+	// stores "ISIL:S1" directly, which differs from the body symbol → mismatch error.
+	tenantResolver := tenant.NewResolver().WithIllRepo(new(mocks.MockIllRepositorySuccess))
+	handler := NewPrApiHandler(new(PrRepoBranchOwner), mockEventBus, mockEventRepo, tenantResolver, nil, 10)
+	differentSymbol := "ISIL:DIFFERENT"
+	id := "3"
+	body := proapi.CreatePatronRequest{Id: &id, RequesterSymbol: &differentSymbol, 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(), "requesterSymbol does not match existing patron request")
+}
+
 func TestPutPatronRequestsIdPreservesCreatedAt(t *testing.T) {
 	originalCreatedAt := time.Date(2024, 1, 15, 10, 0, 0, 0, time.UTC)
 	repo := &PrRepoUpdateCapturePreset{