fix: disallow transfers between two on-budget accounts with an envelope set

Transfers between two on-budget accounts must not have an envelope set.
Such a transaction would be incoming and outgoing for the envelope at
the same time, which is not possible.

Author: morremeyer

pkg/controllers/transaction.go

@@ -160,23 +160,27 @@ func (co Controller) CreateTransaction(c *gin.Context) {
 	}
 
 	// Check the source account
-	_, ok = co.getAccountResource(c, transaction.SourceAccountID)
+	sourceAccount, ok := co.getAccountResource(c, transaction.SourceAccountID)
 	if !ok {
 		return
 	}
 
 	// Check the destination account
-	_, ok = co.getAccountResource(c, transaction.DestinationAccountID)
+	destinationAccount, ok := co.getAccountResource(c, transaction.DestinationAccountID)
 	if !ok {
 		return
 	}
 
-	// Check the envelope ID only if it is set.
+	// Check the envelope only if it is set.
 	if transaction.EnvelopeID != nil {
 		_, ok := co.getEnvelopeResource(c, *transaction.EnvelopeID)
 		if !ok {
 			return
 		}
+
+		if sourceAccount.OnBudget && destinationAccount.OnBudget {
+			httperrors.New(c, http.StatusBadRequest, "Transfers between two on-budget accounts must not have an envelope set. Such a transaction would be incoming and outgoing for this envelope at the same time, which is not possible")
+		}
 	}
 
 	if !decimal.Decimal.IsPositive(transaction.Amount) {
@@ -346,7 +350,7 @@ func (co Controller) UpdateTransaction(c *gin.Context) {
 	if data.SourceAccountID != uuid.Nil {
 		sourceAccountID = data.SourceAccountID
 	}
-	_, ok = co.getAccountResource(c, sourceAccountID)
+	sourceAccount, ok := co.getAccountResource(c, sourceAccountID)
 	if !ok {
 		return
 	}
@@ -356,7 +360,7 @@ func (co Controller) UpdateTransaction(c *gin.Context) {
 	if data.DestinationAccountID != uuid.Nil {
 		destinationAccountID = data.DestinationAccountID
 	}
-	_, ok = co.getAccountResource(c, destinationAccountID)
+	destinationAccount, ok := co.getAccountResource(c, destinationAccountID)
 	if !ok {
 		return
 	}
@@ -367,6 +371,10 @@ func (co Controller) UpdateTransaction(c *gin.Context) {
 		if !ok {
 			return
 		}
+
+		if sourceAccount.OnBudget && destinationAccount.OnBudget {
+			httperrors.New(c, http.StatusBadRequest, "Transfers between two on-budget accounts must not have an envelope set. Such a transaction would be incoming and outgoing for this envelope at the same time, which is not possible")
+		}
 	}
 
 	if !queryWithRetry(c, co.DB.Model(&transaction).Select("", updateFields...).Updates(data)) {

pkg/controllers/transaction_test.go

@@ -345,6 +345,40 @@ func (suite *TestSuiteStandard) TestCreateNoEnvelopeTransactionOutgoing() {
 	suite.assertHTTPStatus(&recorder, http.StatusCreated)
 }
 
+func (suite *TestSuiteStandard) TestCreateTransferOnBudgetWithEnvelope() {
+	eID := suite.createTestEnvelope(models.EnvelopeCreate{}).Data.ID
+	c := models.TransactionCreate{
+		BudgetID:             suite.createTestBudget(models.BudgetCreate{Name: "Testing budget for transfer"}).Data.ID,
+		SourceAccountID:      suite.createTestAccount(models.AccountCreate{Name: "Internal On-Budget Source Account", External: false, OnBudget: true}).Data.ID,
+		DestinationAccountID: suite.createTestAccount(models.AccountCreate{Name: "Internal On-Budget destination account", External: false, OnBudget: true}).Data.ID,
+		Amount:               decimal.NewFromFloat(1337),
+		EnvelopeID:           &eID,
+	}
+
+	recorder := test.Request(suite.controller, suite.T(), http.MethodPost, "http://example.com/v1/transactions", c)
+	suite.assertHTTPStatus(&recorder, http.StatusBadRequest)
+}
+
+func (suite *TestSuiteStandard) TestUpdateTransferOnBudgetWithEnvelope() {
+	eID := suite.createTestEnvelope(models.EnvelopeCreate{}).Data.ID
+	c := models.TransactionCreate{
+		BudgetID:             suite.createTestBudget(models.BudgetCreate{Name: "Testing budget for transfer"}).Data.ID,
+		SourceAccountID:      suite.createTestAccount(models.AccountCreate{Name: "Internal On-Budget Source Account", External: false, OnBudget: true}).Data.ID,
+		DestinationAccountID: suite.createTestAccount(models.AccountCreate{Name: "Internal On-Budget destination account", External: false, OnBudget: true}).Data.ID,
+		Amount:               decimal.NewFromFloat(1337),
+	}
+
+	recorder := test.Request(suite.controller, suite.T(), http.MethodPost, "http://example.com/v1/transactions", c)
+	suite.assertHTTPStatus(&recorder, http.StatusCreated)
+
+	var transaction controllers.TransactionResponse
+	suite.decodeResponse(&recorder, &transaction)
+
+	c.EnvelopeID = &eID
+	recorder = test.Request(suite.controller, suite.T(), http.MethodPatch, transaction.Data.Links.Self, c)
+	suite.assertHTTPStatus(&recorder, http.StatusBadRequest)
+}
+
 func (suite *TestSuiteStandard) TestCreateNonExistingEnvelopeTransactionTransfer() {
 	id := uuid.New()