From a297ad0174bb5ce71093c5c3bedd7a6f738d7e00 Mon Sep 17 00:00:00 2001
From: Kegan Maher <kegan@compiler.la>
Date: Wed, 13 Nov 2024 23:48:14 +0000
Subject: [PATCH] refactor(toggl): remove extra helper functions

---
 compiler_admin/services/toggl.py | 38 ++++++++------------------------
 tests/services/test_toggl.py     | 18 +--------------
 2 files changed, 10 insertions(+), 46 deletions(-)

diff --git a/compiler_admin/services/toggl.py b/compiler_admin/services/toggl.py
index 115dbff..3ee5dea 100644
--- a/compiler_admin/services/toggl.py
+++ b/compiler_admin/services/toggl.py
@@ -128,11 +128,6 @@ def detailed_time_entries(self, start_date: datetime, end_date: datetime, **kwar
         return response
 
 
-def _harvest_client_name():
-    """Gets the value of the HARVEST_CLIENT_NAME env var."""
-    return os.environ.get("HARVEST_CLIENT_NAME")
-
-
 def _get_info(obj: dict, key: str, env_key: str):
     """Read key from obj, populating obj once from a file path at env_key."""
     if obj == {}:
@@ -143,19 +138,6 @@ def _get_info(obj: dict, key: str, env_key: str):
     return obj.get(key)
 
 
-def _toggl_api_token():
-    """Gets the value of the TOGGL_API_TOKEN env var."""
-    return os.environ.get("TOGGL_API_TOKEN")
-
-
-def _toggl_client_id():
-    """Gets the value of the TOGGL_CLIENT_ID env var."""
-    client_id = os.environ.get("TOGGL_CLIENT_ID")
-    if client_id:
-        return int(client_id)
-    return None
-
-
 def _toggl_project_info(project: str):
     """Return the cached project for the given project key."""
     return _get_info(PROJECT_INFO, project, "TOGGL_PROJECT_INFO")
@@ -166,11 +148,6 @@ def _toggl_user_info(email: str):
     return _get_info(USER_INFO, email, "TOGGL_USER_INFO")
 
 
-def _toggl_workspace():
-    """Gets the value of the TOGGL_WORKSPACE_ID env var."""
-    return os.environ.get("TOGGL_WORKSPACE_ID")
-
-
 def _get_first_name(email: str) -> str:
     """Get cached first name or derive from email."""
     user = _toggl_user_info(email)
@@ -226,7 +203,7 @@ def convert_to_harvest(
         None. Either prints the resulting CSV data or writes to output_path.
     """
     if client_name is None:
-        client_name = _harvest_client_name()
+        client_name = os.environ.get("HARVEST_CLIENT_NAME")
 
     # read CSV file, parsing dates and times
     source = files.read_csv(source_path, usecols=INPUT_COLUMNS, parse_dates=["Start date"], cache_dates=True)
@@ -277,11 +254,14 @@ def download_time_entries(
     Returns:
         None. Either prints the resulting CSV data or writes to output_path.
     """
-    if ("client_ids" not in kwargs or not kwargs["client_ids"]) and isinstance(_toggl_client_id(), int):
-        kwargs["client_ids"] = [_toggl_client_id()]
-
-    token = _toggl_api_token()
-    workspace = _toggl_workspace()
+    env_client_id = os.environ.get("TOGGL_CLIENT_ID")
+    if env_client_id:
+        env_client_id = int(env_client_id)
+    if ("client_ids" not in kwargs or not kwargs["client_ids"]) and isinstance(env_client_id, int):
+        kwargs["client_ids"] = [env_client_id]
+
+    token = os.environ.get("TOGGL_API_TOKEN")
+    workspace = os.environ.get("TOGGL_WORKSPACE_ID")
     toggl = Toggl(token, workspace)
 
     response = toggl.detailed_time_entries(start_date, end_date, **kwargs)
diff --git a/tests/services/test_toggl.py b/tests/services/test_toggl.py
index ca32cb5..c811f73 100644
--- a/tests/services/test_toggl.py
+++ b/tests/services/test_toggl.py
@@ -17,7 +17,6 @@
     PROJECT_INFO,
     USER_INFO,
     Toggl,
-    _harvest_client_name,
     _get_info,
     _toggl_project_info,
     _toggl_user_info,
@@ -46,11 +45,6 @@ def spy_files(mocker):
     return mocker.patch.object(compiler_admin.services.toggl, "files", wraps=files)
 
 
-@pytest.fixture
-def mock_harvest_client_name(mocker):
-    return mocker.patch(f"{MODULE}._harvest_client_name")
-
-
 @pytest.fixture
 def mock_get_info(mocker):
     return mocker.patch(f"{MODULE}._get_info")
@@ -169,14 +163,6 @@ def test_toggl_detailed_time_entries_dynamic_timeout(mock_requests, toggl):
     assert mock_requests.post.call_args.kwargs["timeout"] == 30
 
 
-def test_harvest_client_name(monkeypatch):
-    assert _harvest_client_name() == "Test_Client"
-
-    monkeypatch.setenv("HARVEST_CLIENT_NAME", "New Test Client")
-
-    assert _harvest_client_name() == "New Test Client"
-
-
 def test_get_info(monkeypatch):
     with NamedTemporaryFile("w") as temp:
         monkeypatch.setenv("INFO_FILE", temp.name)
@@ -286,13 +272,11 @@ def test_str_timedelta():
     assert result.total_seconds() == (1 * 60 * 60) + (30 * 60) + 15
 
 
-def test_convert_to_harvest_mocked(toggl_file, spy_files, mock_harvest_client_name, mock_google_user_info):
+def test_convert_to_harvest_mocked(toggl_file, spy_files, mock_google_user_info):
     mock_google_user_info.return_value = {}
 
     convert_to_harvest(toggl_file, client_name=None)
 
-    mock_harvest_client_name.assert_called_once()
-
     spy_files.read_csv.assert_called_once()
     call_args = spy_files.read_csv.call_args
     assert (toggl_file,) in call_args