fix: return 404 for endpoints of non-existing budgets

Author: morremeyer

internal/controllers/account.go

@@ -71,7 +71,15 @@ func CreateAccount(c *gin.Context) {
 func GetAccounts(c *gin.Context) {
 	var accounts []models.Account
 
-	models.DB.Where("budget_id = ?", c.Param("budgetId")).Find(&accounts)
+	// Check if the budget exists at all
+	budgetID, err := checkBudgetExists(c, c.Param("budgetId"))
+	if err != nil {
+		return
+	}
+
+	models.DB.Where(&models.Account{
+		BudgetID: budgetID,
+	}).Find(&accounts)
 
 	for i, account := range accounts {
 		response, err := account.WithCalculations()

internal/controllers/account_test.go

@@ -86,6 +86,15 @@ func TestNoAccountNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestNonexistingBudgetAccounts404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing budget, the accounts endpoint raises a 404
+// instead of returning an empty list.
+func TestNonexistingBudgetAccounts404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/999/accounts", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
 func TestCreateAccount(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/accounts", `{ "name": "New Account", "note": "More tests something something" }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)
@@ -144,15 +153,7 @@ func TestGetAccountTransactions(t *testing.T) {
 
 func TestGetAccountTransactionsNonExistingAccount(t *testing.T) {
 	recorder := test.Request(t, "GET", "/v1/budgets/1/accounts/57372/transactions", "")
-
-	var response TransactionListResponse
-	err := json.NewDecoder(recorder.Body).Decode(&response)
-	if err != nil {
-		assert.Fail(t, "Unable to parse response from server %q into APIListResponse, '%v'", recorder.Body, err)
-	}
-
 	assert.Equal(t, 404, recorder.Code)
-	assert.Len(t, response.Data, 0)
 }
 
 func TestUpdateAccount(t *testing.T) {

internal/controllers/category.go

@@ -20,7 +20,7 @@ func RegisterCategoryRoutes(r *gin.RouterGroup) {
 		r.POST("", CreateCategory)
 	}
 
-	// Transaction with ID
+	// Category with ID
 	{
 		r.OPTIONS("/:categoryId", func(c *gin.Context) {
 			c.Header("allow", "GET, PATCH, DELETE")
@@ -51,7 +51,16 @@ func CreateCategory(c *gin.Context) {
 // GetCategories retrieves all categories.
 func GetCategories(c *gin.Context) {
 	var categories []models.Category
-	models.DB.Where("budget_id = ?", c.Param("budgetId")).Find(&categories)
+
+	// Check if the budget exists at all
+	budgetID, err := checkBudgetExists(c, c.Param("budgetId"))
+	if err != nil {
+		return
+	}
+
+	models.DB.Where(&models.Category{
+		BudgetID: budgetID,
+	}).Find(&categories)
 
 	c.JSON(http.StatusOK, gin.H{"data": categories})
 }

internal/controllers/category_test.go

@@ -53,6 +53,15 @@ func TestNoCategoryNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestNonexistingBudgetCategories404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing budget, the accounts endpoint raises a 404
+// instead of returning an empty list.
+func TestNonexistingBudgetCategories404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/999/categories", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
 func TestCreateCategory(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/categories", `{ "name": "New Category", "note": "More tests something something" }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)

internal/controllers/helper.go

@@ -5,7 +5,9 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"strconv"
 
+	"github.com/envelope-zero/backend/internal/models"
 	"github.com/gin-contrib/requestid"
 	"github.com/rs/zerolog/log"
 
@@ -41,10 +43,29 @@ func requestURL(c *gin.Context) string {
 	return scheme + "://" + c.Request.Host + forwardedPrefix + c.Request.URL.Path
 }
 
+func checkBudgetExists(c *gin.Context, budgetIDString string) (uint64, error) {
+	var budget models.Budget
+
+	budgetID, _ := strconv.ParseUint(budgetIDString, 10, 64)
+
+	// Check that the budget exists. If not, return a 404
+	err := models.DB.Where(&models.Budget{
+		Model: models.Model{
+			ID: budgetID,
+		},
+	}).First(&budget).Error
+	if err != nil {
+		FetchErrorHandler(c, err)
+		return 0, err
+	}
+
+	return budgetID, nil
+}
+
 // FetchErrorHandler handles errors for fetching data from the database.
 func FetchErrorHandler(c *gin.Context, err error) {
 	if errors.Is(err, gorm.ErrRecordNotFound) {
-		c.JSON(http.StatusNotFound, gin.H{"error": "Record not found"})
+		c.AbortWithStatus(http.StatusNotFound)
 	} else {
 		log.Error().Str("request-id", requestid.Get(c)).Msgf("%T: %v", err, err.Error())
 		c.JSON(http.StatusInternalServerError, gin.H{

internal/controllers/transaction.go

@@ -56,7 +56,16 @@ func CreateTransaction(c *gin.Context) {
 // GetTransactions retrieves all transactions.
 func GetTransactions(c *gin.Context) {
 	var transactions []models.Transaction
-	models.DB.Where("budget_id = ?", c.Param("budgetId")).Find(&transactions)
+
+	// Check if the budget exists at all
+	budgetID, err := checkBudgetExists(c, c.Param("budgetId"))
+	if err != nil {
+		return
+	}
+
+	models.DB.Where(&models.Category{
+		BudgetID: budgetID,
+	}).Find(&transactions)
 
 	c.JSON(http.StatusOK, gin.H{"data": transactions})
 }

internal/controllers/transaction_test.go

@@ -85,6 +85,15 @@ func TestNoTransactionNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestNonexistingBudgetTransactions404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing budget, the accounts endpoint raises a 404
+// instead of returning an empty list.
+func TestNonexistingBudgetTransactions404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/999/transactions", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
 func TestCreateTransaction(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/transactions", `{ "note": "More tests something something", "amount": 1253.17 }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)