refactor(convert): explicit from/to format args

allows for more than just harvest<>toggl conversions

Author: thekaveman

compiler_admin/commands/time/convert.py

@@ -1,25 +1,28 @@
 from argparse import Namespace
 
-import pandas as pd
-
 from compiler_admin import RESULT_SUCCESS
-from compiler_admin.services.harvest import HARVEST_COLUMNS, convert_to_toggl
-from compiler_admin.services.toggl import TOGGL_COLUMNS, convert_to_harvest
+from compiler_admin.services.harvest import CONVERTERS as HARVEST_CONVERTERS
+from compiler_admin.services.toggl import CONVERTERS as TOGGL_CONVERTERS
+
+
+CONVERTERS = {"harvest": HARVEST_CONVERTERS, "toggl": TOGGL_CONVERTERS}
 
 
-def _get_source_converter(source):
-    columns = pd.read_csv(source, nrows=0).columns.tolist()
+def _get_source_converter(from_fmt: str, to_fmt: str):
+    from_fmt = from_fmt.lower().strip() if from_fmt else ""
+    to_fmt = to_fmt.lower().strip() if to_fmt else ""
+    converter = CONVERTERS.get(from_fmt, {}).get(to_fmt)
 
-    if set(HARVEST_COLUMNS) <= set(columns):
-        return convert_to_toggl
-    elif set(TOGGL_COLUMNS) <= set(columns):
-        return convert_to_harvest
+    if converter:
+        return converter
     else:
-        raise NotImplementedError("A converter for the given source data does not exist.")
+        raise NotImplementedError(
+            f"A converter for the given source and target formats does not exist: {from_fmt} to {to_fmt}"
+        )
 
 
 def convert(args: Namespace, *extras):
-    converter = _get_source_converter(args.input)
+    converter = _get_source_converter(args.from_fmt, args.to_fmt)
 
     converter(source_path=args.input, output_path=args.output, client_name=args.client)
 

compiler_admin/main.py

@@ -9,6 +9,7 @@
 from compiler_admin.commands.info import info
 from compiler_admin.commands.init import init
 from compiler_admin.commands.time import time
+from compiler_admin.commands.time.convert import CONVERTERS
 from compiler_admin.commands.user import user
 from compiler_admin.commands.user.convert import ACCOUNT_TYPE_OU
 
@@ -78,6 +79,20 @@ def setup_time_command(cmd_parsers: _SubParsersAction):
         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.",
     )
+    time_convert.add_argument(
+        "--from",
+        default="toggl",
+        choices=sorted(CONVERTERS.keys()),
+        dest="from_fmt",
+        help="The format of the source data. Defaults to 'toggl'.",
+    )
+    time_convert.add_argument(
+        "--to",
+        default="harvest",
+        choices=sorted([to_fmt for sub in CONVERTERS.values() for to_fmt in sub.keys()]),
+        dest="to_fmt",
+        help="The format of the converted data. Defaults to 'harvest'.",
+    )
     time_convert.add_argument("--client", default=None, help="The name of the client to use in converted data.")
 
     time_download = add_sub_cmd(time_subcmds, "download", help="Download a Toggl report in CSV format.")

compiler_admin/services/harvest.py

@@ -90,3 +90,6 @@ def convert_to_toggl(
     output_data.sort_values(["Start date", "Start time", "Email"], inplace=True)
 
     files.write_csv(output_path, output_data, output_cols)
+
+
+CONVERTERS = {"toggl": convert_to_toggl}

compiler_admin/services/toggl.py

@@ -217,3 +217,6 @@ def download_time_entries(
 
     df = pd.read_csv(io.StringIO(csv))
     files.write_csv(output_path, df, columns=output_cols)
+
+
+CONVERTERS = {"harvest": convert_to_harvest, "justworks": convert_to_justworks}

tests/commands/time/test_convert.py

@@ -1,47 +1,56 @@
 from argparse import Namespace
-from io import StringIO
 import pytest
 
 from compiler_admin import RESULT_SUCCESS
 from compiler_admin.commands.time.convert import (
     __name__ as MODULE,
+    CONVERTERS,
     _get_source_converter,
-    convert_to_harvest,
-    convert_to_toggl,
     convert,
 )
+from compiler_admin.services.harvest import CONVERTERS as HARVEST_CONVERTERS
+from compiler_admin.services.toggl import CONVERTERS as TOGGL_CONVERTERS
 
 
 @pytest.fixture
 def mock_get_source_converter(mocker):
     return mocker.patch(f"{MODULE}._get_source_converter")
 
 
-def test_get_source_converter_match_toggl(toggl_file):
-    result = _get_source_converter(toggl_file)
-
-    assert result == convert_to_harvest
+@pytest.fixture
+def mock_converters(mocker):
+    return mocker.patch(f"{MODULE}.CONVERTERS", new={})
 
 
-def test_get_source_converter_match_harvest(harvest_file):
-    result = _get_source_converter(harvest_file)
+def test_get_source_converter_match(mock_converters):
+    mock_converters["toggl"] = {"test_fmt": "converter"}
+    result = _get_source_converter("toggl", "test_fmt")
 
-    assert result == convert_to_toggl
+    assert result == "converter"
 
 
 def test_get_source_converter_mismatch():
-    data = StringIO("one,two,three\n1,2,3")
-
-    with pytest.raises(NotImplementedError, match="A converter for the given source data does not exist."):
-        _get_source_converter(data)
+    with pytest.raises(
+        NotImplementedError, match="A converter for the given source and target formats does not exist: nope to toggl"
+    ):
+        _get_source_converter("nope", "toggl")
+    with pytest.raises(
+        NotImplementedError, match="A converter for the given source and target formats does not exist: toggl to nope"
+    ):
+        _get_source_converter("toggl", "nope")
 
 
 def test_convert(mock_get_source_converter):
-    args = Namespace(input="input", output="output", client="client")
+    args = Namespace(input="input", output="output", client="client", from_fmt="from", to_fmt="to")
     res = convert(args)
 
     assert res == RESULT_SUCCESS
-    mock_get_source_converter.assert_called_once_with(args.input)
+    mock_get_source_converter.assert_called_once_with(args.from_fmt, args.to_fmt)
     mock_get_source_converter.return_value.assert_called_once_with(
         source_path=args.input, output_path=args.output, client_name=args.client
     )
+
+
+def test_converters():
+    assert CONVERTERS.get("harvest") == HARVEST_CONVERTERS
+    assert CONVERTERS.get("toggl") == TOGGL_CONVERTERS

tests/services/test_harvest.py

@@ -12,6 +12,7 @@
     files,
     HARVEST_COLUMNS,
     TOGGL_COLUMNS,
+    CONVERTERS,
     _calc_start_time,
     _duration_str,
     _toggl_client_name,
@@ -109,3 +110,7 @@ def test_convert_to_toggl_sample(harvest_file, toggl_file):
     assert set(output_df.columns.to_list()) <= set(sample_output_df.columns.to_list())
     assert output_df["Client"].eq("Test Client 123").all()
     assert output_df["Project"].eq("Test Client 123").all()
+
+
+def test_converters():
+    assert CONVERTERS.get("toggl") == convert_to_toggl

tests/services/test_toggl.py

@@ -9,6 +9,7 @@
 
 import compiler_admin.services.toggl
 from compiler_admin.services.toggl import (
+    CONVERTERS,
     __name__ as MODULE,
     _get_first_name,
     _get_last_name,
@@ -251,3 +252,8 @@ def test_download_time_entries(toggl_file):
         # as corresponding column values from the mock DataFrame
         for col in response_df.columns:
             assert response_df[col].equals(mock_df[col])
+
+
+def test_converters():
+    assert CONVERTERS.get("harvest") == convert_to_harvest
+    assert CONVERTERS.get("justworks") == convert_to_justworks

tests/test_main.py

@@ -104,7 +104,7 @@ def test_main_init_gyb(mock_commands_init):
 def test_main_init_no_username(mock_commands_init):
     with pytest.raises(SystemExit):
         main(argv=["init"])
-        assert mock_commands_init.call_count == 0
+    assert mock_commands_init.call_count == 0
 
 
 def test_main_time_convert_default(mock_commands_time):
@@ -114,7 +114,14 @@ def test_main_time_convert_default(mock_commands_time):
     call_args = mock_commands_time.call_args.args
     assert (
         Namespace(
-            func=mock_commands_time, command="time", subcommand="convert", client=None, input=sys.stdin, output=sys.stdout
+            func=mock_commands_time,
+            command="time",
+            subcommand="convert",
+            client=None,
+            input=sys.stdin,
+            output=sys.stdout,
+            from_fmt="toggl",
+            to_fmt="harvest",
         )
         in call_args
     )
@@ -129,7 +136,16 @@ def test_main_time_convert_env(monkeypatch, mock_commands_time):
     mock_commands_time.assert_called_once()
     call_args = mock_commands_time.call_args.args
     assert (
-        Namespace(func=mock_commands_time, command="time", subcommand="convert", client=None, input="toggl", output="harvest")
+        Namespace(
+            func=mock_commands_time,
+            command="time",
+            subcommand="convert",
+            client=None,
+            input="toggl",
+            output="harvest",
+            from_fmt="toggl",
+            to_fmt="harvest",
+        )
         in call_args
     )
 
@@ -231,6 +247,8 @@ def test_main_time_convert_client(mock_commands_time):
             client="client123",
             input=sys.stdin,
             output=sys.stdout,
+            from_fmt="toggl",
+            to_fmt="harvest",
         )
         in call_args
     )
@@ -249,6 +267,8 @@ def test_main_time_convert_input(mock_commands_time):
             client=None,
             input="file.csv",
             output=sys.stdout,
+            from_fmt="toggl",
+            to_fmt="harvest",
         )
         in call_args
     )
@@ -267,10 +287,62 @@ def test_main_time_convert_output(mock_commands_time):
             client=None,
             input=sys.stdin,
             output="file.csv",
+            from_fmt="toggl",
+            to_fmt="harvest",
+        )
+        in call_args
+    )
+
+
+def test_main_time_convert_from(mock_commands_time):
+    main(argv=["time", "convert", "--from", "harvest"])
+
+    mock_commands_time.assert_called_once()
+    call_args = mock_commands_time.call_args.args
+    assert (
+        Namespace(
+            func=mock_commands_time,
+            command="time",
+            subcommand="convert",
+            client=None,
+            input=sys.stdin,
+            output=sys.stdout,
+            from_fmt="harvest",
+            to_fmt="harvest",
         )
         in call_args
     )
 
+    with pytest.raises(SystemExit):
+        main(argv=["time", "convert", "--from", "nope"])
+    # it should not have been called an additional time from the first
+    mock_commands_time.assert_called_once()
+
+
+def test_main_time_convert_to(mock_commands_time):
+    main(argv=["time", "convert", "--to", "toggl"])
+
+    mock_commands_time.assert_called_once()
+    call_args = mock_commands_time.call_args.args
+    assert (
+        Namespace(
+            func=mock_commands_time,
+            command="time",
+            subcommand="convert",
+            client=None,
+            input=sys.stdin,
+            output=sys.stdout,
+            from_fmt="toggl",
+            to_fmt="toggl",
+        )
+        in call_args
+    )
+
+    with pytest.raises(SystemExit):
+        main(argv=["time", "convert", "--to", "nope"])
+    # it should not have been called an additional time from the first
+    mock_commands_time.assert_called_once()
+
 
 def test_main_user_alumni(mock_commands_user):
     main(argv=["user", "alumni", "username"])
@@ -355,7 +427,7 @@ def test_main_user_create_extras(mock_commands_user):
 def test_main_user_create_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "create"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_convert(mock_commands_user):
@@ -380,13 +452,13 @@ def test_main_user_convert(mock_commands_user):
 def test_main_user_convert_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "convert"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_convert_bad_account_type(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "convert", "username", "account_type"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_delete(mock_commands_user):
@@ -412,7 +484,7 @@ def test_main_user_delete_force(mock_commands_user):
 def test_main_user_delete_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "delete"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_offboard(mock_commands_user):
@@ -458,7 +530,7 @@ def test_main_user_offboard_with_alias(mock_commands_user):
 def test_main_user_offboard_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "offboard"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_reset(mock_commands_user):
@@ -504,7 +576,7 @@ def test_main_user_reset_force(mock_commands_user):
 def test_main_user_reset_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "reset"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_restore(mock_commands_user):
@@ -518,7 +590,7 @@ def test_main_user_restore(mock_commands_user):
 def test_main_user_restore_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "restore"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 def test_main_user_signout(mock_commands_user):
@@ -544,7 +616,7 @@ def test_main_user_signout_force(mock_commands_user):
 def test_main_user_signout_no_username(mock_commands_user):
     with pytest.raises(SystemExit):
         main(argv=["user", "signout"])
-        assert mock_commands_user.call_count == 0
+    assert mock_commands_user.call_count == 0
 
 
 @pytest.mark.e2e