Check ownership before checking if already sent

Author: adamdickmeiss

broker/patron_request/api/api-handler.go

@@ -534,16 +534,6 @@ func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *
 		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)
@@ -556,6 +546,16 @@ func (a *PatronRequestApiHandler) PutPatronRequestsId(w http.ResponseWriter, r *
 		api.AddBadRequestError(ctx, w, fmt.Errorf("only borrower-side patron requests can be updated"))
 		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
+	}
 	newPr.RequesterSymbol = &symbol
 	creationTime := time.Now()
 	if existingPr.CreatedAt.Valid {

broker/patron_request/api/api-handler_test.go

@@ -1483,7 +1483,6 @@ func TestPutPatronRequestsIdMissingId(t *testing.T) {
 
 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)
@@ -1515,7 +1514,6 @@ func TestPutPatronRequestsIdIllRepoError(t *testing.T) {
 
 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{})
@@ -1524,7 +1522,6 @@ func TestPutPatronRequestsIdNotFound(t *testing.T) {
 
 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{})
@@ -1537,7 +1534,6 @@ func TestPutPatronRequestsIdWrongOwner(t *testing.T) {
 	// 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{})
@@ -1556,7 +1552,6 @@ func TestPutPatronRequestsIdUpdateError(t *testing.T) {
 
 func TestPutPatronRequestsIdNotBorrowingSide(t *testing.T) {
 	handler := NewPrApiHandler(new(PrRepoLendingSide), 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{})