fix(download): simplify date arg parsing

no need to pass or deal with hours, minutes, seconds, etc.

Author: thekaveman

compiler_admin/api/toggl.py

@@ -55,7 +55,7 @@ def detailed_time_entries(self, start_date: datetime, end_date: datetime, **kwar
         Args:
             start_date (datetime): The beginning of the reporting period.
 
-            end_date (str): The end of the reporting period.
+            end_date (datetime): The end of the reporting period.
 
         Extra `kwargs` are passed through as a POST json body.
 

compiler_admin/commands/time/download.py

@@ -16,7 +16,7 @@ def local_now():
 
 def prior_month_end():
     now = local_now()
-    first = now.replace(day=1, hour=0, minute=0, second=0, microsecond=0)
+    first = now.replace(day=1)
     return first - timedelta(days=1)
 
 
@@ -29,15 +29,15 @@ def prior_month_start():
 @click.option(
     "--start",
     metavar="YYYY-MM-DD",
-    default=prior_month_start(),
-    callback=lambda ctx, param, val: datetime.strptime(val, "%Y-%m-%d %H:%M:%S%z"),
+    default=prior_month_start().strftime("%Y-%m-%d"),
+    callback=lambda ctx, param, val: datetime.strptime(val, "%Y-%m-%d").replace(tzinfo=TZINFO),
     help="The start date of the reporting period. Defaults to the beginning of the prior month.",
 )
 @click.option(
     "--end",
     metavar="YYYY-MM-DD",
-    default=prior_month_end(),
-    callback=lambda ctx, param, val: datetime.strptime(val, "%Y-%m-%d %H:%M:%S%z"),
+    default=prior_month_end().strftime("%Y-%m-%d"),
+    callback=lambda ctx, param, val: datetime.strptime(val, "%Y-%m-%d").replace(tzinfo=TZINFO),
     help="The end date of the reporting period. Defaults to the end of the prior month.",
 )
 @click.option(
@@ -122,3 +122,6 @@ def download(
         click.echo(f"  {k}: {v}")
 
     download_time_entries(**params)
+
+    click.echo()
+    click.echo(f"Download complete: ./{output}")

tests/commands/time/test_download.py

@@ -4,9 +4,9 @@
 from compiler_admin import RESULT_SUCCESS
 from compiler_admin.commands.time.download import (
     __name__ as MODULE,
-    download,
     TOGGL_COLUMNS,
     TZINFO,
+    download,
     prior_month_end,
     prior_month_start,
 )
@@ -19,40 +19,35 @@ def mock_local_now(mocker):
     return dt
 
 
-@pytest.fixture
-def mock_start(mock_local_now):
-    return datetime(2024, 8, 1, tzinfo=TZINFO)
-
-
-@pytest.fixture
-def mock_end(mock_local_now):
-    return datetime(2024, 8, 31, tzinfo=TZINFO)
-
-
 @pytest.fixture
 def mock_download_time_entries(mocker):
     return mocker.patch(f"{MODULE}.download_time_entries")
 
 
-def test_prior_month_start(mock_start):
+def test_prior_month_start(mock_local_now):
     start = prior_month_start()
 
-    assert start == mock_start
+    assert start.year == 2024
+    assert start.month == 8
+    assert start.day == 1
+    assert start.tzinfo == TZINFO
 
 
-def test_prior_month_end(mock_end):
+def test_prior_month_end(mock_local_now):
     end = prior_month_end()
 
-    assert end == mock_end
+    assert end.year == 2024
+    assert end.month == 8
+    assert end.day == 31
+    assert end.tzinfo == TZINFO
 
 
-def test_download(cli_runner, mock_download_time_entries):
-    date = datetime.now(tz=TZINFO).replace(hour=0, minute=0, second=0, microsecond=0)
+def test_download(cli_runner, mock_local_now, mock_download_time_entries):
     args = [
         "--start",
-        date.strftime("%Y-%m-%d %H:%M:%S%z"),
+        mock_local_now.strftime("%Y-%m-%d"),
         "--end",
-        date.strftime("%Y-%m-%d %H:%M:%S%z"),
+        mock_local_now.strftime("%Y-%m-%d"),
         "--output",
         "output",
         "-c",
@@ -77,8 +72,8 @@ def test_download(cli_runner, mock_download_time_entries):
 
     assert result.exit_code == RESULT_SUCCESS
     mock_download_time_entries.assert_called_once_with(
-        start_date=date,
-        end_date=date,
+        start_date=mock_local_now,
+        end_date=mock_local_now,
         output_path="output",
         billable=True,
         output_cols=TOGGL_COLUMNS,
@@ -89,43 +84,48 @@ def test_download(cli_runner, mock_download_time_entries):
     )
 
 
-def test_download_client_envvar(cli_runner, monkeypatch, mock_download_time_entries):
-    monkeypatch.setenv("TOGGL_CLIENT_ID", 1234)
-
-    date = datetime.now(tz=TZINFO).replace(hour=0, minute=0, second=0, microsecond=0)
-    args = [
-        "--start",
-        date.strftime("%Y-%m-%d %H:%M:%S%z"),
-        "--end",
-        date.strftime("%Y-%m-%d %H:%M:%S%z"),
-        "--output",
-        "output",
-    ]
+def test_download_default(cli_runner, mock_download_time_entries):
+    expected_start, expected_end = prior_month_start(), prior_month_end()
 
-    result = cli_runner.invoke(download, args)
+    result = cli_runner.invoke(download, [])
 
     assert result.exit_code == RESULT_SUCCESS
-    mock_download_time_entries.assert_called_once_with(
-        start_date=date, end_date=date, output_path="output", output_cols=TOGGL_COLUMNS, billable=True, client_ids=(1234,)
+    mock_download_time_entries.assert_called_once()
+    call = mock_download_time_entries.mock_calls[0]
+
+    actual_start = call.kwargs["start_date"]
+    assert actual_start.year == expected_start.year
+    assert actual_start.month == expected_start.month
+    assert actual_start.day == expected_start.day
+
+    actual_end = call.kwargs["end_date"]
+    assert actual_end.year == expected_end.year
+    assert actual_end.month == expected_end.month
+    assert actual_end.day == expected_end.day
+
+    assert (
+        call.kwargs["output_path"]
+        == f"Toggl_time_entries_{expected_start.strftime('%Y-%m-%d')}_{expected_end.strftime('%Y-%m-%d')}.csv"
     )
+    assert call.kwargs["output_cols"] == TOGGL_COLUMNS
+    assert call.kwargs["billable"] is True
 
 
-def test_download_all(cli_runner, monkeypatch, mock_download_time_entries):
-    monkeypatch.delenv("TOGGL_CLIENT_ID", raising=False)
-    date = datetime.now(tz=TZINFO).replace(hour=0, minute=0, second=0, microsecond=0)
-    args = [
-        "--start",
-        date.strftime("%Y-%m-%d %H:%M:%S%z"),
-        "--end",
-        date.strftime("%Y-%m-%d %H:%M:%S%z"),
-        "--output",
-        "output",
-        "--all",
-    ]
+def test_download_client_envvar(cli_runner, monkeypatch, mock_download_time_entries):
+    monkeypatch.setenv("TOGGL_CLIENT_ID", 1234)
 
-    result = cli_runner.invoke(download, args)
+    result = cli_runner.invoke(download, [])
 
     assert result.exit_code == RESULT_SUCCESS
-    mock_download_time_entries.assert_called_once_with(
-        start_date=date, end_date=date, output_path="output", output_cols=TOGGL_COLUMNS
-    )
+    mock_download_time_entries.assert_called_once()
+    call = mock_download_time_entries.mock_calls[0]
+    assert call.kwargs["client_ids"] == (1234,)
+
+
+def test_download_all(cli_runner, mock_download_time_entries):
+    result = cli_runner.invoke(download, ["--all"])
+
+    assert result.exit_code == RESULT_SUCCESS
+    mock_download_time_entries.assert_called_once()
+    call = mock_download_time_entries.mock_calls[0]
+    assert "billable" not in call.kwargs