From b1e80994b4d5c74e454c03e25b043dabf9d95f74 Mon Sep 17 00:00:00 2001
From: Gabriel Augendre <gabriel@augendre.info>
Date: Sat, 29 Feb 2020 13:35:51 +0100
Subject: [PATCH] Improve and test bad config file handling

---
 ofx_processor/utils/ynab.py                   | 30 ++++++++++++-------
 .../samples/config_broken_missing_account.ini |  9 ++++++
 .../config_broken_missing_account_key.ini     | 11 +++++++
 .../samples/config_broken_missing_budget.ini  | 11 +++++++
 tests/samples/config_broken_missing_token.ini | 11 +++++++
 tests/test_end_to_end.py                      | 22 ++++++++++----
 6 files changed, 78 insertions(+), 16 deletions(-)
 create mode 100644 tests/samples/config_broken_missing_account.ini
 create mode 100644 tests/samples/config_broken_missing_account_key.ini
 create mode 100644 tests/samples/config_broken_missing_budget.ini
 create mode 100644 tests/samples/config_broken_missing_token.ini

diff --git a/ofx_processor/utils/ynab.py b/ofx_processor/utils/ynab.py
index 2b313c7..1e346cb 100644
--- a/ofx_processor/utils/ynab.py
+++ b/ofx_processor/utils/ynab.py
@@ -59,20 +59,21 @@ def push_transactions(transactions, account):
     try:
         config.read(config_file)
     except configparser.Error as e:
-        click.secho(f"Error while parsing config file: {str(e)}", fg="red", bold=True)
-        click.secho("Opening the file...")
-        click.pause()
-        click.edit(filename=config_file)
-        click.secho("Exiting...", fg="red", bold=True)
-        sys.exit(1)
-    section = config[account]
-    budget_id = section["budget"]
+        return handle_config_file_error(config_file, e)
+
+    try:
+        section = config[account]
+        budget_id = section["budget"]
+        token = section["token"]
+        account = section["account"]
+    except KeyError as e:
+        return handle_config_file_error(config_file, e)
+
     url = f"{BASE_URL}/budgets/{budget_id}/transactions"
     for transaction in transactions:
-        transaction["account_id"] = section["account"]
+        transaction["account_id"] = account
 
     data = {"transactions": transactions}
-    token = section["token"]
     headers = {"Authorization": f"Bearer {token}"}
 
     res = requests.post(url, json=data, headers=headers)
@@ -95,3 +96,12 @@ def push_transactions(transactions, account):
         click.secho(
             f"{len(duplicates)} transactions ignored (duplicates).", fg="yellow"
         )
+
+
+def handle_config_file_error(config_file, e):
+    click.secho(f"Error while parsing config file: {str(e)}", fg="red", bold=True)
+    click.secho("Opening the file...")
+    click.pause()
+    click.edit(filename=config_file)
+    click.secho("Exiting...", fg="red", bold=True)
+    sys.exit(1)
diff --git a/tests/samples/config_broken_missing_account.ini b/tests/samples/config_broken_missing_account.ini
new file mode 100644
index 0000000..6f6bf8b
--- /dev/null
+++ b/tests/samples/config_broken_missing_account.ini
@@ -0,0 +1,9 @@
+[DEFAULT]
+token = <YOUR API TOKEN>
+budget = <YOUR BUDGET ID>
+
+[bpvf]
+account = <YOUR BPVF ACCOUNT ID>
+
+[ce]
+account = <YOUR CE ACCOUNT ID>
diff --git a/tests/samples/config_broken_missing_account_key.ini b/tests/samples/config_broken_missing_account_key.ini
new file mode 100644
index 0000000..04fc889
--- /dev/null
+++ b/tests/samples/config_broken_missing_account_key.ini
@@ -0,0 +1,11 @@
+[DEFAULT]
+token = <YOUR API TOKEN>
+budget = <YOUR BUDGET ID>
+
+[bpvf]
+account = <YOUR BPVF ACCOUNT ID>
+
+[revolut]
+
+[ce]
+account = <YOUR CE ACCOUNT ID>
diff --git a/tests/samples/config_broken_missing_budget.ini b/tests/samples/config_broken_missing_budget.ini
new file mode 100644
index 0000000..90416f5
--- /dev/null
+++ b/tests/samples/config_broken_missing_budget.ini
@@ -0,0 +1,11 @@
+[DEFAULT]
+token = <YOUR API TOKEN>
+
+[bpvf]
+account = <YOUR BPVF ACCOUNT ID>
+
+[revolut]
+account = <YOUR REVOLUT ACCOUNT ID>
+
+[ce]
+account = <YOUR CE ACCOUNT ID>
diff --git a/tests/samples/config_broken_missing_token.ini b/tests/samples/config_broken_missing_token.ini
new file mode 100644
index 0000000..cd7fb54
--- /dev/null
+++ b/tests/samples/config_broken_missing_token.ini
@@ -0,0 +1,11 @@
+[DEFAULT]
+budget = <YOUR BUDGET ID>
+
+[bpvf]
+account = <YOUR BPVF ACCOUNT ID>
+
+[revolut]
+account = <YOUR REVOLUT ACCOUNT ID>
+
+[ce]
+account = <YOUR CE ACCOUNT ID>
diff --git a/tests/test_end_to_end.py b/tests/test_end_to_end.py
index 61ee70c..c3d77e4 100644
--- a/tests/test_end_to_end.py
+++ b/tests/test_end_to_end.py
@@ -64,12 +64,22 @@ class ConfigTestCase(unittest.TestCase):
     )
     @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "tests/samples")
     def test_broken_config_file(self, edit):
-        runner = CliRunner()
-        result = runner.invoke(self.cli, ["revolut", "tests/samples/revolut.csv"])
-
-        expected_filename = "tests/samples/config_broken_duplicate_key.ini"
-        self.assertIn("Error while parsing config file", result.output)
-        edit.assert_called_once_with(filename=expected_filename)
+        broken_files = [
+            "config_broken_duplicate_key.ini",
+            "config_broken_missing_account.ini",
+            "config_broken_missing_account_key.ini",
+            "config_broken_missing_budget.ini",
+            "config_broken_missing_token.ini",
+        ]
+        for name in broken_files:
+            with mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", name):
+                runner = CliRunner()
+                result = runner.invoke(
+                    self.cli, ["revolut", "tests/samples/revolut.csv"]
+                )
+                expected_filename = ynab.get_config_file_name()
+                self.assertIn("Error while parsing config file", result.output)
+                edit.assert_called_with(filename=expected_filename)
 
     @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_FILENAME", "file.ini")
     @mock.patch("ofx_processor.utils.ynab.DEFAULT_CONFIG_DIR", "some/config/folder")