fix: verify that parent resource exists before returning child

Author: morremeyer

internal/controllers/account.go

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

internal/controllers/allocation.go

@@ -76,7 +76,16 @@ func CreateAllocation(c *gin.Context) {
 // GetAllocations retrieves all allocations.
 func GetAllocations(c *gin.Context) {
 	var allocations []models.Allocation
-	models.DB.Where("envelope_id = ?", c.Param("envelopeId")).Find(&allocations)
+
+	// Check if the envelope exists
+	envelope, err := getEnvelope(c)
+	if err != nil {
+		return
+	}
+
+	models.DB.Where(&models.Allocation{
+		EnvelopeID: envelope.ID,
+	}).Find(&allocations)
 
 	c.JSON(http.StatusOK, gin.H{"data": allocations})
 }

internal/controllers/allocation_test.go

@@ -54,6 +54,33 @@ func TestNoAllocationNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestNonexistingEnvelopeAllocations404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing envelope, no matter if the category or budget exists,
+// the allocations endpoint raises a 404 instead of returning an empty list.
+func TestNonexistingEnvelopeAllocations404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/1/categories/1/envelopes/999/allocations", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
+// TestNonexistingCategoryAllocations404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing category, no matter if the envelope or budget exists,
+// the allocations endpoint raises a 404 instead of returning an empty list.
+func TestNonexistingCategoryAllocations404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/1/categories/999/envelopes/1/allocations", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
+// TestNonexistingBudgetAllocations404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing budget, no matter if the envelope or category exists,
+// the allocations endpoint raises a 404 instead of returning an empty list.
+func TestNonexistingBudgetAllocations404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/999/categories/1/envelopes/1/allocations", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
 func TestCreateAllocation(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/categories/1/envelopes/1/allocations", `{ "month": 10, "year": 2022, "amount": 15.42 }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)

internal/controllers/budget.go

@@ -58,10 +58,8 @@ func GetBudgets(c *gin.Context) {
 
 // GetBudget retrieves a budget by its ID.
 func GetBudget(c *gin.Context) {
-	var budget models.Budget
-	err := models.DB.First(&budget, c.Param("budgetId")).Error
+	budget, err := getBudget(c)
 	if err != nil {
-		FetchErrorHandler(c, err)
 		return
 	}
 

internal/controllers/category.go

@@ -53,24 +53,22 @@ func GetCategories(c *gin.Context) {
 	var categories []models.Category
 
 	// Check if the budget exists at all
-	budgetID, err := checkBudgetExists(c, c.Param("budgetId"))
+	budget, err := getBudget(c)
 	if err != nil {
 		return
 	}
 
 	models.DB.Where(&models.Category{
-		BudgetID: budgetID,
+		BudgetID: budget.ID,
 	}).Find(&categories)
 
 	c.JSON(http.StatusOK, gin.H{"data": categories})
 }
 
 // GetCategory retrieves a category by its ID.
 func GetCategory(c *gin.Context) {
-	var category models.Category
-	err := models.DB.First(&category, c.Param("categoryId")).Error
+	category, err := getCategory(c)
 	if err != nil {
-		FetchErrorHandler(c, err)
 		return
 	}
 

internal/controllers/category_test.go

@@ -130,7 +130,14 @@ func TestUpdateNonExistingCategory(t *testing.T) {
 }
 
 func TestDeleteCategory(t *testing.T) {
-	recorder := test.Request(t, "DELETE", "/v1/budgets/1/categories/1", "")
+	recorder := test.Request(t, "POST", "/v1/budgets/1/categories", `{ "name": "Delete me now!" }`)
+	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)
+
+	var category CategoryDetailResponse
+	test.DecodeResponse(t, &recorder, &category)
+
+	path := fmt.Sprintf("/v1/budgets/1/categories/%v", category.Data.ID)
+	recorder = test.Request(t, "DELETE", path, "")
 	test.AssertHTTPStatus(t, http.StatusNoContent, &recorder)
 }
 

internal/controllers/envelope.go

@@ -51,17 +51,24 @@ func CreateEnvelope(c *gin.Context) {
 // GetEnvelopes retrieves all envelopes.
 func GetEnvelopes(c *gin.Context) {
 	var envelopes []models.Envelope
-	models.DB.Where("category_id = ?", c.Param("categoryId")).Find(&envelopes)
+
+	// Check if the category exists at all
+	category, err := getCategory(c)
+	if err != nil {
+		return
+	}
+
+	models.DB.Where(&models.Envelope{
+		CategoryID: category.ID,
+	}).Find(&envelopes)
 
 	c.JSON(http.StatusOK, gin.H{"data": envelopes})
 }
 
 // GetEnvelope retrieves a envelope by its ID.
 func GetEnvelope(c *gin.Context) {
-	var envelope models.Envelope
-	err := models.DB.First(&envelope, c.Param("envelopeId")).Error
+	envelope, err := getEnvelope(c)
 	if err != nil {
-		FetchErrorHandler(c, err)
 		return
 	}
 

internal/controllers/envelope_test.go

@@ -49,6 +49,24 @@ func TestNoEnvelopeNotFound(t *testing.T) {
 	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
 }
 
+// TestNonexistingCategoryEnvelopes404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing category, the envelopes endpoint raises a 404
+// instead of returning an empty list.
+func TestNonexistingCategoryEnvelopes404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/1/categories/999/envelopes", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
+// TestNonexistingBudgetEnvelopes404 is a regression test for https://github.com/envelope-zero/backend/issues/89.
+//
+// It verifies that for a non-existing budget, no matter if the category with the ID exists,
+// the envelopes endpoint raises a 404 instead of returning an empty list.
+func TestNonexistingBudgetEnvelopes404(t *testing.T) {
+	recorder := test.Request(t, "GET", "/v1/budgets/999/categories/1/envelopes", "")
+	test.AssertHTTPStatus(t, http.StatusNotFound, &recorder)
+}
+
 func TestCreateEnvelope(t *testing.T) {
 	recorder := test.Request(t, "POST", "/v1/budgets/1/categories/1/envelopes", `{ "name": "New Envelope", "note": "More tests something something" }`)
 	test.AssertHTTPStatus(t, http.StatusCreated, &recorder)

internal/controllers/helper.go

@@ -43,10 +43,11 @@ 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) {
+// getBudget verifies that the budget from the URL parameters exists and returns it.
+func getBudget(c *gin.Context) (models.Budget, error) {
 	var budget models.Budget
 
-	budgetID, _ := strconv.ParseUint(budgetIDString, 10, 64)
+	budgetID, _ := strconv.ParseUint(c.Param("budgetId"), 10, 64)
 
 	// Check that the budget exists. If not, return a 404
 	err := models.DB.Where(&models.Budget{
@@ -56,10 +57,62 @@ func checkBudgetExists(c *gin.Context, budgetIDString string) (uint64, error) {
 	}).First(&budget).Error
 	if err != nil {
 		FetchErrorHandler(c, err)
-		return 0, err
+		return models.Budget{}, err
 	}
 
-	return budgetID, nil
+	return budget, nil
+}
+
+// getCategory verifies that the category from the URL parameters exists and returns it
+//
+// It also verifies that the budget that is referred to exists.
+func getCategory(c *gin.Context) (models.Category, error) {
+	var category models.Category
+
+	categoryID, _ := strconv.ParseUint(c.Param("categoryId"), 10, 64)
+
+	_, err := getBudget(c)
+	if err != nil {
+		return models.Category{}, err
+	}
+
+	err = models.DB.Where(&models.Category{
+		Model: models.Model{
+			ID: categoryID,
+		},
+	}).First(&category).Error
+	if err != nil {
+		FetchErrorHandler(c, err)
+		return models.Category{}, err
+	}
+
+	return category, nil
+}
+
+// getEnvelope verifies that the envelope from the URL parameters exists and returns it
+//
+// It also verifies that the budget and the category that are referred to exist.
+func getEnvelope(c *gin.Context) (models.Envelope, error) {
+	var envelope models.Envelope
+
+	envelopeID, _ := strconv.ParseUint(c.Param("envelopeId"), 10, 64)
+
+	_, err := getCategory(c)
+	if err != nil {
+		return models.Envelope{}, err
+	}
+
+	err = models.DB.Where(&models.Envelope{
+		Model: models.Model{
+			ID: envelopeID,
+		},
+	}).First(&envelope).Error
+	if err != nil {
+		FetchErrorHandler(c, err)
+		return models.Envelope{}, err
+	}
+
+	return envelope, nil
 }
 
 // FetchErrorHandler handles errors for fetching data from the database.

internal/controllers/transaction.go

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