refactor(time): describe as @click.group of commands

Author: thekaveman

compiler_admin/commands/time/__init__.py

@@ -1,16 +1,16 @@
-from argparse import Namespace
+import click
 
-from compiler_admin.commands.time.convert import convert  # noqa: F401
-from compiler_admin.commands.time.download import download  # noqa: F401
+from compiler_admin.commands.time.convert import convert
+from compiler_admin.commands.time.download import download
 
 
-def time(args: Namespace, *extra):
-    # try to call the subcommand function directly from global (module) symbols
-    # if the subcommand function was imported above, it should exist in globals()
-    global_env = globals()
+@click.group
+def time():
+    """
+    Work with Compiler time entries.
+    """
+    pass
 
-    if args.subcommand in global_env:
-        cmd_func = global_env[args.subcommand]
-        cmd_func(args, *extra)
-    else:
-        raise NotImplementedError(f"Unknown time subcommand: {args.subcommand}")
+
+time.add_command(convert)
+time.add_command(download)

compiler_admin/commands/time/convert.py

@@ -1,6 +1,9 @@
-from argparse import Namespace
+import os
+import sys
+from typing import TextIO
+
+import click
 
-from compiler_admin import RESULT_SUCCESS
 from compiler_admin.services.harvest import CONVERTERS as HARVEST_CONVERTERS
 from compiler_admin.services.toggl import CONVERTERS as TOGGL_CONVERTERS
 
@@ -21,9 +24,44 @@ def _get_source_converter(from_fmt: str, to_fmt: str):
         )
 
 
-def convert(args: Namespace, *extras):
-    converter = _get_source_converter(args.from_fmt, args.to_fmt)
-
-    converter(source_path=args.input, output_path=args.output, client_name=args.client)
+@click.command()
+@click.option(
+    "--input",
+    default=os.environ.get("TOGGL_DATA", sys.stdin),
+    help="The path to the source data for conversion. Defaults to $TOGGL_DATA or stdin.",
+)
+@click.option(
+    "--output",
+    default=os.environ.get("HARVEST_DATA", sys.stdout),
+    help="The path to the file where converted data should be written. Defaults to $HARVEST_DATA or stdout.",
+)
+@click.option(
+    "--from",
+    "from_fmt",
+    default="toggl",
+    help="The format of the source data.",
+    show_default=True,
+    type=click.Choice(sorted(CONVERTERS.keys()), case_sensitive=False),
+)
+@click.option(
+    "--to",
+    "to_fmt",
+    default="harvest",
+    help="The format of the converted data.",
+    show_default=True,
+    type=click.Choice(sorted([to_fmt for sub in CONVERTERS.values() for to_fmt in sub.keys()]), case_sensitive=False),
+)
+@click.option("--client", help="The name of the client to use in converted data.")
+def convert(
+    input: str | TextIO = os.environ.get("TOGGL_DATA", sys.stdin),
+    output: str | TextIO = os.environ.get("HARVEST_DATA", sys.stdout),
+    from_fmt="toggl",
+    to_fmt="harvest",
+    client="",
+):
+    """
+    Convert a time report from one format into another.
+    """
+    converter = _get_source_converter(from_fmt, to_fmt)
 
-    return RESULT_SUCCESS
+    converter(source_path=input, output_path=output, client_name=client)

compiler_admin/commands/time/download.py

@@ -1,23 +1,120 @@
-from argparse import Namespace
+from datetime import datetime, timedelta
+import os
+import sys
+from typing import List
+
+import click
+from pytz import timezone
 
-from compiler_admin import RESULT_SUCCESS
 from compiler_admin.services.toggl import TOGGL_COLUMNS, download_time_entries
 
 
-def download(args: Namespace, *extras):
-    params = dict(
-        start_date=args.start, end_date=args.end, output_path=args.output, output_cols=TOGGL_COLUMNS, billable=args.billable
-    )
+TZINFO = timezone(os.environ.get("TZ_NAME", "America/Los_Angeles"))
 
-    if args.client_ids:
-        params.update(dict(client_ids=args.client_ids))
-    if args.project_ids:
-        params.update(dict(project_ids=args.project_ids))
-    if args.task_ids:
-        params.update(dict(task_ids=args.task_ids))
-    if args.user_ids:
-        params.update(dict(user_ids=args.user_ids))
 
-    download_time_entries(**params)
+def local_now():
+    return datetime.now(tz=TZINFO)
+
+
+def prior_month_end():
+    now = local_now()
+    first = now.replace(day=1)
+    return first - timedelta(days=1)
+
+
+def prior_month_start():
+    end = prior_month_end()
+    return end.replace(day=1)
 
-    return RESULT_SUCCESS
+
+@click.command()
+@click.option(
+    "--start",
+    metavar="YYYY-MM-DD",
+    default=prior_month_start(),
+    type=lambda s: TZINFO.localize(datetime.strptime(s, "%Y-%m-%d")),
+    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(),
+    type=lambda s: TZINFO.localize(datetime.strptime(s, "%Y-%m-%d")),
+    help="The end date of the reporting period. Defaults to the end of the prior month.",
+)
+@click.option(
+    "--output",
+    default=os.environ.get("TOGGL_DATA", sys.stdout),
+    help="The path to the file where downloaded data should be written. Defaults to a path calculated from the date range.",
+)
+@click.option(
+    "--all",
+    "billable",
+    is_flag=True,
+    default=True,
+    help="Download all time entries. The default is to download only billable time entries.",
+)
+@click.option(
+    "-c",
+    "--client",
+    "client_ids",
+    multiple=True,
+    help="An ID for a Toggl Client to filter for in reports. Can be supplied more than once.",
+    metavar="CLIENT_ID",
+    type=int,
+)
+@click.option(
+    "-p",
+    "--project",
+    "project_ids",
+    multiple=True,
+    help="An ID for a Toggl Project to filter for in reports. Can be supplied more than once.",
+    metavar="PROJECT_ID",
+    type=int,
+)
+@click.option(
+    "-t",
+    "--task",
+    "task_ids",
+    multiple=True,
+    metavar="TASK_ID",
+    help="An ID for a Toggl Project Task to filter for in reports. Can be supplied more than once.",
+    type=int,
+)
+@click.option(
+    "-u",
+    "--user",
+    "user_ids",
+    multiple=True,
+    help="An ID for a Toggl User to filter for in reports. Can be supplied more than once.",
+    metavar="USER_ID",
+    type=int,
+)
+def download(
+    start: datetime,
+    end: datetime,
+    output: str = "",
+    billable: bool = True,
+    client_ids: List[int] = [],
+    project_ids: List[int] = [],
+    task_ids: List[int] = [],
+    user_ids: List[int] = [],
+):
+    """
+    Download a Toggl time report in CSV format.
+    """
+    if not output:
+        output = f"Toggl_time_entries_{start.strftime('%Y-%m-%d')}_{end.strftime('%Y-%m-%d')}.csv"
+
+    params = dict(start_date=start, end_date=end, output_path=output, output_cols=TOGGL_COLUMNS, billable=billable)
+
+    if client_ids:
+        params.update(dict(client_ids=client_ids))
+    if project_ids:
+        params.update(dict(project_ids=project_ids))
+    if task_ids:
+        params.update(dict(task_ids=task_ids))
+    if user_ids:
+        params.update(dict(user_ids=user_ids))
+
+    download_time_entries(**params)

tests/commands/time/test__init__.py

@@ -1,4 +1,3 @@
-from argparse import Namespace
 import pytest
 
 from compiler_admin import RESULT_SUCCESS
@@ -40,14 +39,15 @@ def test_get_source_converter_mismatch():
         _get_source_converter("toggl", "nope")
 
 
-def test_convert(mock_get_source_converter):
-    args = Namespace(input="input", output="output", client="client", from_fmt="from", to_fmt="to")
-    res = convert(args)
+def test_convert(cli_runner, mock_get_source_converter):
+    result = cli_runner.invoke(
+        convert, ["--input", "input", "--output", "output", "--client", "client", "--from", "harvest", "--to", "toggl"]
+    )
 
-    assert res == RESULT_SUCCESS
-    mock_get_source_converter.assert_called_once_with(args.from_fmt, args.to_fmt)
+    assert result.exit_code == RESULT_SUCCESS
+    mock_get_source_converter.assert_called_once_with("harvest", "toggl")
     mock_get_source_converter.return_value.assert_called_once_with(
-        source_path=args.input, output_path=args.output, client_name=args.client
+        source_path="input", output_path="output", client_name="client"
     )
 
 

tests/commands/time/test_convert.py

@@ -1,41 +1,93 @@
-from argparse import Namespace
 from datetime import datetime
 import pytest
 
 from compiler_admin import RESULT_SUCCESS
-from compiler_admin.commands.time.download import __name__ as MODULE, download, TOGGL_COLUMNS
+from compiler_admin.commands.time.download import (
+    __name__ as MODULE,
+    download,
+    TOGGL_COLUMNS,
+    TZINFO,
+    prior_month_end,
+    prior_month_start,
+)
+
+
+@pytest.fixture
+def mock_local_now(mocker):
+    dt = datetime(2024, 9, 25, tzinfo=TZINFO)
+    mocker.patch(f"{MODULE}.local_now", return_value=dt)
+    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):
+    start = prior_month_start()
+
+    assert start == mock_start
+
+
+def test_prior_month_end(mock_end):
+    end = prior_month_end()
+
+    assert end == mock_end
+
+
 @pytest.mark.parametrize("billable", [True, False])
-def test_download(mock_download_time_entries, billable):
-    date = datetime.now()
-    args = Namespace(
-        start=date,
-        end=date,
-        output="output",
-        billable=billable,
-        client_ids=["c1", "c2"],
-        project_ids=["p1", "p2"],
-        task_ids=["t1", "t2"],
-        user_ids=["u1", "u2"],
-    )
+def test_download(cli_runner, mock_download_time_entries, billable):
+    date = datetime.now(tz=TZINFO).replace(hour=0, minute=0, second=0, microsecond=0)
+    args = [
+        "--start",
+        date.strftime("%Y-%m-%d"),
+        "--end",
+        date.strftime("%Y-%m-%d"),
+        "--output",
+        "output",
+        "-c",
+        1,
+        "-c",
+        2,
+        "-p",
+        3,
+        "-p",
+        4,
+        "-t",
+        5,
+        "-t",
+        6,
+        "-u",
+        7,
+        "-u",
+        8,
+    ]
 
-    res = download(args)
+    if not billable:
+        args.append("--all")
 
-    assert res == RESULT_SUCCESS
+    result = cli_runner.invoke(download, args)
+
+    assert result.exit_code == RESULT_SUCCESS
     mock_download_time_entries.assert_called_once_with(
-        start_date=args.start,
-        end_date=args.end,
-        output_path=args.output,
+        start_date=date,
+        end_date=date,
+        output_path="output",
         output_cols=TOGGL_COLUMNS,
-        billable=args.billable,
-        client_ids=args.client_ids,
-        project_ids=args.project_ids,
-        task_ids=args.task_ids,
-        user_ids=args.user_ids,
+        billable=billable,
+        client_ids=(1, 2),
+        project_ids=(3, 4),
+        task_ids=(5, 6),
+        user_ids=(7, 8),
     )