Fix CashCtrl API error message and long descriptions (#217)

Author: MarcoSteinacher

README.md

@@ -2,7 +2,8 @@
 
 [![Download](https://img.shields.io/badge/Download-v1.3.0-%23007ec6)](https://github.com/tegonal/cohiva/releases/tag/v1.3.0)
 [![AGPL 3](https://img.shields.io/badge/%E2%9A%96-AGPL%203-%230b45a6)](https://www.gnu.org/licenses/agpl-3.0.en.html "License")
-[![Quality Assurance](https://github.com/tegonal/cohiva/actions/workflows/quality-assurance.yml/badge.svg?event=push&branch=main)](https://github.com/tegonal/cohiva/actions/workflows/quality-assurance.yml?query=branch%3Amain)
+[![Quality Assurance](https://github.com/tegonal/cohiva/actions/workflows/quality-assurance.yml/badge.svg?event=push&branch=develop)](https://github.com/tegonal/cohiva/actions/workflows/quality-assurance.yml?query=branch%3Adevelop)
+[![Coverage](https://raw.githubusercontent.com/tegonal/cohiva/python-coverage-comment-action-data/badge.svg)](https://htmlpreview.github.io/?https://github.com/tegonal/cohiva/blob/python-coverage-comment-action-data/htmlcov/index.html)
 [![Newcomers Welcome](https://img.shields.io/badge/%F0%9F%91%8B-Newcomers%20Welcome-blueviolet)](https://github.com/tegonal/cohiva/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22 "Ask in discussions for help")
 
 <!-- for main end -->

django/cohiva/settings_for_tests.py

@@ -43,6 +43,7 @@
 DEMO = False
 RESERVATION_BLOCKER_RULES = []
 FINANCIAL_ACCOUNTS[AccountKey.DEFAULT_DEBTOR]["iban"] = "CH7730000001250094239"
+FINANCIAL_ACCOUNTS[AccountKey.DEFAULT_DEBTOR]["account_code"] = "1020.1"
 
 FINANCIAL_ACCOUNTING_CASHCTRL_LIVE_TESTS = False
 
@@ -67,6 +68,15 @@
             "TENANT": "cohiva-test",
         },
     },
+    "cashctrl_test_live": {
+        "BACKEND": "finance.accounting.CashctrlBook",
+        "DB_ID": 1,
+        "OPTIONS": {
+            "API_HOST": "cashctrl.com",
+            "API_TOKEN": f"{cbc.CASHCTRL_API_TOKEN}",
+            "TENANT": f"{cbc.CASHCTRL_TENANT}",
+        },
+    },
     "dummy_test": {
         "BACKEND": "finance.accounting.DummyBook",
         "OPTIONS": {

django/finance/accounting/cashctrl.py

@@ -96,31 +96,12 @@ def _create_collective_transaction(self, transaction):
             amount_str = f"{amount:.2f}"
             split_items_o.append({"accountId": cct_account, credit_type: amount_str})
 
-        payload = dict(
-            dateAdded=self.cct_book_ref.get_date(getattr(transaction, "date", None)).strftime(
-                "%Y-%m-%d"
-            ),
-            title=getattr(transaction, "description", ""),
-            items=json.dumps(split_items_o),
-        )
-
-        # Call create endpoint
-        response = self._construct_request_post("journal/create.json", payload=payload)
-        response.raise_for_status()
-        data = response.json()
-        self._raise_for_error(
-            response, f"create collective transaction: len: {len(transaction.splits)}"
+        payload = self._get_common_transaction_payload(transaction)
+        payload["items"] = json.dumps(split_items_o)
+        return self._create_transaction_api_call(
+            payload, f"create collective transaction: len: {len(transaction.splits)}"
         )
 
-        txn_id = None
-        if isinstance(data, dict):
-            if data.get("success"):
-                txn_id = data.get("insertId")
-        # Record inserted transaction for the current BookTransaction
-        transaction_id = self.cct_book_ref.build_transaction_id(txn_id)
-        self.insert(transaction_id)
-        return transaction_id
-
     def _create_simple_transaction(self, transaction):
         # Expecting two splits: debit and credit
         if not getattr(transaction, "splits", None) or len(transaction.splits) != 2:
@@ -136,29 +117,22 @@ def _create_simple_transaction(self, transaction):
             else str(transaction.splits[1].amount)
         )
 
-        notes = "Added through API"
-        description = getattr(transaction, "description", "")
-        # in case description is longer than 250 chars, truncate and append fully to notes
-        if len(description) > 250:
-            notes += "\n" + description
-            description = description[:250]
-
-        date_added = self.cct_book_ref.get_date(getattr(transaction, "date", None)).strftime(
-            "%Y-%m-%d"
-        )
-        attributes = (
-            f"amount={amount_str}&creditId={cct_account_credit}&debitId={cct_account_debit}"
-            f"&title={urllib.parse.quote_plus(description)}"
-            f"&dateAdded={date_added}&notes={urllib.parse.quote_plus(notes)}"
+        payload = self._get_common_transaction_payload(transaction)
+        payload["amount"] = amount_str
+        payload["creditId"] = cct_account_credit
+        payload["debitId"] = cct_account_debit
+        return self._create_transaction_api_call(
+            payload, f"create:{cct_account_debit}:{cct_account_credit}:{amount_str}"
         )
 
+    def _create_transaction_api_call(self, payload, request_info):
         # Call create endpoint
-        response = self._construct_request_post("journal/create.json?" + attributes, None)
+        response = self._construct_request_post("journal/create.json", payload=payload)
         response.raise_for_status()
         data = response.json()
         self._raise_for_error(
             response,
-            f"create:{cct_account_debit}:{cct_account_credit}:{amount_str}",
+            request_info,
         )
 
         txn_id = None
@@ -170,6 +144,26 @@ def _create_simple_transaction(self, transaction):
         self.insert(transaction_id)
         return transaction_id
 
+    def _get_common_transaction_payload(self, transaction):
+        date_added = self.cct_book_ref.get_date(getattr(transaction, "date", None)).strftime(
+            "%Y-%m-%d"
+        )
+        description = getattr(transaction, "description", "")
+        # in case description is longer than 250 chars, truncate and append fully to notes
+        if len(description) > 250:
+            notes = description
+            description = description[:250]
+        else:
+            notes = None
+
+        payload = dict(
+            dateAdded=date_added,
+            title=description,
+        )
+        if notes:
+            payload["notes"] = notes
+        return payload
+
     def save(self):
         # inserted transactions can be ignored as they were already saved to CashCtrl via API
         self._inserted_transaction_ids.clear()
@@ -266,7 +260,13 @@ def _raise_for_error(self, response, request_info=None):
         data = response.json()
         if isinstance(data, dict):
             if not data.get("success", True):
-                error_message = data.get("message", "Unknown error")
+                errors = data.get("errors")
+                if errors:
+                    error_message = "; ".join(
+                        f"{error.get('field')}: {error.get('message')}" for error in errors
+                    )
+                else:
+                    error_message = data.get("message") or "Unknown error"
                 raise RuntimeError(
                     f"CashCtrl API error: {error_message} - for request {request_info}"
                 )

django/finance/tests/test_accounting.py

@@ -125,7 +125,7 @@ def test_register_backends_from_settings(self):
         self.assertEqual(AccountingManager.default_backend_label, "test1")
 
         AccountingManager.register_backends_from_settings()
-        self.assertEqual(len(AccountingManager.backends), 4)
+        self.assertEqual(len(AccountingManager.backends), 5)
         self.assertEqual(AccountingManager.default_backend_label, "dummy_test")
         self.assertEqual(AccountingManager.backends["dummy_test2"]["db_id"], 1)
 

django/finance/tests/test_accounting_cct.py

@@ -45,7 +45,7 @@ def fetch_account_responses(url, **kw):
                         "type": "MANUAL",
                         "dateAdded": "2025-11-04 18:07:00.0",
                         "title": "Test CashCtrl add_transaction",
-                        "notes": "Added through API",
+                        "notes": "",
                         "amount": 100,
                         "currencyRate": 1,
                         "accountIds": '["1237","1477"]',
@@ -189,6 +189,12 @@ def fetch_account_responses(url, **kw):
                     status_code=200,
                 )
             return Exception("Unknown account number in URL")
+        return Mock(
+            json=lambda: {
+                "success": False,
+            },
+            status_code=200,
+        )
 
     @staticmethod
     def fetch_transaction_responses(url, **kw):
@@ -247,10 +253,15 @@ def test_add_transaction(self, mock_post, mock_get):
                 ]
             )
             called_url = mock_post.call_args[0][0]
-            assert (
-                f"{self.cohiva_test_endpoint}journal/create.json?amount=100.00&creditId=1237&debitId=1477&title=Test+CashCtrl+add_transaction&dateAdded=2026-01-01"
-                in called_url
+            self.assertIn(f"{self.cohiva_test_endpoint}journal/create.json", called_url)
+            form_data_constructed = mock_post.call_args[1]
+            self.assertEqual("100.00", form_data_constructed["data"]["amount"])
+            self.assertEqual(1237, form_data_constructed["data"]["creditId"])
+            self.assertEqual(1477, form_data_constructed["data"]["debitId"])
+            self.assertEqual(
+                "Test CashCtrl add_transaction", form_data_constructed["data"]["title"]
             )
+            self.assertEqual("2026-01-01", form_data_constructed["data"]["dateAdded"])
 
     @patch("finance.accounting.cashctrl.requests.get")
     @patch("finance.accounting.cashctrl.requests.post")
@@ -273,7 +284,7 @@ def test_add_transaction_without_date(self, mock_post, mock_get):
                 self.account1,
                 self.account2,
                 None,
-                "Test CashCtrl add_transaction",
+                "Test CashCtrl add_transaction without date",
                 autosave=False,
             )
             self.assertTrue(transaction_id.startswith("cct_"))
@@ -283,11 +294,52 @@ def test_add_transaction_without_date(self, mock_post, mock_get):
             # verify that the API was called
             today = datetime.date.today().strftime("%Y-%m-%d")
             called_url = mock_post.call_args[0][0]
-            assert (
-                f"{self.cohiva_test_endpoint}journal/create.json?amount=100.00&creditId=1237"
-                f"&debitId=1477&title=Test+CashCtrl+add_transaction&dateAdded={today}"
-                in called_url
+            self.assertIn(f"{self.cohiva_test_endpoint}journal/create.json", called_url)
+            form_data_constructed = mock_post.call_args[1]
+            self.assertEqual(
+                "Test CashCtrl add_transaction without date",
+                form_data_constructed["data"]["title"],
             )
+            self.assertEqual(today, form_data_constructed["data"]["dateAdded"])
+
+    @patch("finance.accounting.cashctrl.requests.get")
+    @patch("finance.accounting.cashctrl.requests.post")
+    def test_add_transaction_with_long_description(self, mock_post, mock_get):
+        messages = []
+        with AccountingManager(messages) as book:
+            # configure fake responses
+            mock_get.return_value.raise_for_status.side_effect = None
+            mock_get.side_effect = self.fetch_account_responses
+
+            mock_post.return_value.json.return_value = {
+                "success": True,
+                "message": "Buchung gespeichert",
+                "insertId": 700,
+            }
+            mock_post.return_value.raise_for_status.side_effect = None
+
+            long_description = (
+                "Test CashCtrl add_transaction with long description (254 chars) "
+                + 19 * "123456789_"
+            )
+            transaction_id = book.add_transaction(
+                100.00,
+                self.account1,
+                self.account2,
+                None,
+                long_description,
+                autosave=False,
+            )
+            self.assertTrue(transaction_id.startswith("cct_"))
+            self.assertEqual("cct_0_700", transaction_id)
+            book.save()
+
+            # verify that the API was called
+            called_url = mock_post.call_args[0][0]
+            self.assertIn(f"{self.cohiva_test_endpoint}journal/create.json", called_url)
+            form_data_constructed = mock_post.call_args[1]
+            self.assertEqual(long_description[:250], form_data_constructed["data"]["title"])
+            self.assertEqual(long_description, form_data_constructed["data"]["notes"])
 
     @patch("finance.accounting.cashctrl.requests.get")
     @patch("finance.accounting.cashctrl.requests.post")
@@ -430,3 +482,44 @@ def test_delete_transaction(self, mock_post, mock_get):
             mock_post.assert_called_once_with(
                 f"{self.cohiva_test_endpoint}journal/delete.json?ids=801", data=None, auth=ANY
             )
+
+    @patch("finance.accounting.cashctrl.requests.get")
+    @patch("finance.accounting.cashctrl.requests.post")
+    def test_process_api_error(self, mock_post, mock_get):
+        messages = []
+        with AccountingManager(messages) as book:
+            # configure fake responses
+            mock_get.return_value.raise_for_status.side_effect = None
+            mock_get.side_effect = self.fetch_account_responses
+
+            mock_post.return_value.json.return_value = {
+                "success": False,
+                "message": None,
+                "errors": [
+                    {
+                        "field": "creditId",
+                        "message": "This account does not exist.",
+                    },
+                    {
+                        "field": "field2",
+                        "message": "Another error for field2",
+                    },
+                ],
+            }
+            mock_post.return_value.raise_for_status.side_effect = None
+
+            with self.assertRaisesRegex(
+                RuntimeError,
+                (
+                    "CashCtrl API error: creditId: This account does not exist.; "
+                    "field2: Another error for field2 - for request"
+                ),
+            ):
+                book.add_transaction(
+                    100.00,
+                    self.account1,
+                    self.account2,
+                    None,
+                    "Test error processing",
+                    autosave=False,
+                )

django/finance/tests/test_accounting_cct_live.py

@@ -21,7 +21,7 @@ class CashctrlBookTestCase(TestCase):
     @classmethod
     def setUpClass(cls):
         super().setUpClass()
-        AccountingManager.default_backend_label = "cashctrl_test"
+        AccountingManager.default_backend_label = "cashctrl_test_live"
         cls.account1 = Account("TestAccount CashCtrl1", "43000")
         cls.account2 = Account("TestAccount CashCtrl2", "10220")
         cls.account3 = Account("TestAccount CashCtrl3", "47400")
@@ -51,6 +51,20 @@ def test_add_transaction(self):
             self.assertTrue(transaction_id.startswith("cct_"))
             book.save()
 
+    def test_add_transaction_with_long_description(self):
+        messages = []
+        with AccountingManager(messages) as book:
+            transaction_id = book.add_transaction(
+                101.00,
+                self.account1,
+                self.account2,
+                "2026-01-01",
+                "Test CashCtrl add_transaction with long description (254 chars) "
+                + 19 * "123456789_",
+                autosave=True,
+            )
+            self.assertTrue(transaction_id.startswith("cct_"))
+
     def test_add_transaction_no_commit(self):
         messages = []
         with AccountingManager(messages) as book: