Merge pull request #14 from caseneuve/9-log-command-allow-more-than-6…

…0-minutes-in-time-option-if-only-minutes-provided

Closes #9

README.md

@@ -109,7 +109,8 @@ Usage: dzira log [OPTIONS] ISSUE
   string.
 
   TIME spent should be in format '[[Nh][ ]][Nm]'; or it can be calculated when
-  START time is be provided;
+  START time is be provided; it's assumed that time spent for a single task
+  cannot be greater than 1 day (8 hours).
 
   END time is optional, both should match 'H:M' format.
 
@@ -123,9 +124,9 @@ Usage: dzira log [OPTIONS] ISSUE
 
     "YYYY-mm-dd", "YYYY-mm-ddTHH:MM", "YYYY-mm-dd HH:MM".
 
-  Time is assumed from the START option, if present and date is not
-  specifying it.  The script will try to figure out local timezone and adjust
-  the log started time accordingly.
+  Time is calculated from the START option, if present, and date is
+  not specifying it.  The script will try to figure out local
+  timezone and adjust the log started time accordingly.
 
 Options:
   -t, --time TEXT        Time to spend in JIRA accepted format, e.g. '2h 10m'

src/dzira/dzira.py

@@ -59,7 +59,7 @@ def show_cursor():
 class D(dict):
     def __call__(self, *keys) -> Iterable:
         if keys:
-            return [self.get(k) for k in keys]
+            return [self.get(*k) if isinstance(k, tuple) else self.get(k) for k in keys]
         else:
             return self.values()
 
@@ -262,14 +262,13 @@ def get_issues(jira: JIRA, sprint: Sprint) -> Result:
 def add_worklog(
         jira: JIRA,
         issue: str,
-        time: str | None = None,
         comment: str | None = None,
         seconds: int | None = None,
         date: datetime | None = None,
         **_
 ) -> Result:
     work_log = jira.add_worklog(
-        issue=issue, timeSpent=time, timeSpentSeconds=seconds, comment=comment, started=date
+        issue=issue, timeSpentSeconds=seconds, comment=comment, started=date
     )
     return Result(
         stdout=(
@@ -493,26 +492,31 @@ def ls(ctx, state, sprint_id, format):
 ### Validators
 
 def matches_time_re(time: str) -> D:
-    m = re.match(
-        r"^(?P<h>([1-9]|1\d|2[0-3])h)?(\s*(?=\d))?(?P<m>([1-5]\d|[1-9])m)?$",
-        time
-    )
+    """
+    Allows strings '[h][ [m]]' with or without format indicators 'h/m',
+    not greater than 8h 59m, or only minutes not greater than 499m.
+    """
+    only_mins = re.compile(r"^(?P<m>(\d{2}|[1-4]\d{2}))m$")
+    hours_and_mins = re.compile(r"^(?P<h>([1-8]))h(\s*(?=\d))?((?P<m>([1-5]\d|[1-9]))m?)?$")
+    m = only_mins.match(time) or hours_and_mins.match(time)
     return D(m.groupdict() if m is not None else {})
 
 
 def is_valid_hour(hour) -> bool:
     return re.match(r"^(([01]?\d|2[0-3])[:.h,])+([0-5]?\d)$", hour) is not None
 
 
-def validate_time(_, __, time):
+def validate_time(_, __, time) -> int:
     if time is None:
-        return
+        return 0
     if (match := matches_time_re(time)):
-        h, m = match("h", "m")
-        time = f"{h + ' ' if h is not None else ''}{m or ''}".strip()
-        return time
+        return sum(int(t) * s for t, s in zip(match("h", "m"), [3600, 60]) if t)
     raise click.BadParameter(
-        "time has to be in format '[Nh] [Nm]', e.g. '2h', '30m', '4h 15m'"
+        (
+            "time cannot be greater than 8h (1 day), "
+            "and has to be in format '[Nh][ N[m]]' or 'Nm', "
+            "e.g. '2h', '91m', '4h 37m', '1h59'."
+        )
     )
 
 
@@ -590,7 +594,7 @@ def calculate_seconds(payload: D) -> D:
     start, end = payload("start", "end")
 
     if start is None:
-        return payload
+        return payload.update("seconds", payload.get("time"))
 
     fmt = "%H:%M"
     unify = lambda t: datetime.strptime(re.sub(r"[,.h]", ":", t), fmt)
@@ -678,11 +682,13 @@ def perform_log_action(jira: JIRA, payload: D) -> None:
 )
 @click.help_option("-h", "--help")
 def log(ctx, **_):
-    """Log time spent on ISSUE number or ISSUE with description containing
+    """
+    Log time spent on ISSUE number or ISSUE with description containing
     matching string.
 
-    TIME spent should be in format '[[Nh][ ]][Nm]'; or it can be calculated
-    when START time is be provided;
+    TIME spent should be in format '[[Nh][ ]][Nm]'; or it can be
+    calculated when START time is be provided; it's assumed that time
+    spent for a single task cannot be greater than 1 day (8 hours).
 
     END time is optional, both should match 'H:M' format.
 
@@ -697,7 +703,8 @@ def log(ctx, **_):
     \b
       "YYYY-mm-dd", "YYYY-mm-ddTHH:MM", "YYYY-mm-dd HH:MM".
 
-    \b Time is assumed from the START option, if present and date is
+    \b
+    Time is calculated from the START option, if present, and date is
     not specifying it.  The script will try to figure out local
     timezone and adjust the log started time accordingly.
     """

tests/test_dzira.py

@@ -193,6 +193,17 @@ def test_inherits_from_dict(self):
     def test_call_returns_unpacked_values_of_selected_keys_or_none(self, input, expected):
         assert self.d(*input) == expected
 
+    @pytest.mark.parametrize(
+        "input,expected",
+        [
+            ((("x", 99), "c"), [99, 3]),
+            (("a", ("z", 42)), [1, 42]),
+            ((("b", 22), ("c", 88)), [2, 3])
+        ]
+    )
+    def test_call_accepts_tuples_with_fallback_values(self, input, expected):
+        assert self.d(*input) == expected
+
     def test_update_returns_self_with_key_of_given_value(self):
         assert self.d.update("a", 99) == D({**self.d, "a": 99})
         assert self.d.update("x", 77) == D({**self.d, "x": 77})
@@ -457,21 +468,19 @@ def test_calls_jira_add_worklog_with_provided_values(self):
         mock_worklog = Mock(raw={"timeSpent": "2h"}, issueId="123", id=321)
         mock_jira = Mock(add_worklog=Mock(return_value=mock_worklog))
 
-        result1 = add_worklog(mock_jira, "333", time="2h", date=sentinel.date)
+        result1 = add_worklog(mock_jira, "333", seconds=7200, date=sentinel.date)
         result2 = add_worklog(mock_jira, "333", seconds=60 * 60 * 2, comment="blah!")
 
         assert mock_jira.add_worklog.call_args_list == (
             [
                 call(
                     issue="333",
-                    timeSpent="2h",
-                    timeSpentSeconds=None,
+                    timeSpentSeconds=7200,
                     comment=None,
                     started=sentinel.date,
                 ),
                 call(
                     issue="333",
-                    timeSpent=None,
                     timeSpentSeconds=7200,
                     comment="blah!",
                     started=None,
@@ -536,10 +545,15 @@ def test_calls_private_function_and_wraps_the_result(self, mocker):
 
 
 class TestCalculateSeconds:
-    def test_returns_the_unchanged_payload_if_no_start_time(self):
+    def test_returns_the_unchanged_payload_with_extra_key_seconds_if_no_start_time(self):
         input = D({"start": None, "end": None, "foo": "bar"})
 
-        assert calculate_seconds(input) == input
+        assert calculate_seconds(input) == input.update("seconds", None)
+
+    def test_copies_value_of_time_to_seconds_if_time_provided(self):
+        input = D({"time": 3600, "end": None, "foo": "bar"})
+
+        assert calculate_seconds(input) == input.update("seconds", 3600)
 
     @pytest.mark.parametrize(
         "start,end,expected",
@@ -882,18 +896,24 @@ class TestCorrectTimeFormats:
     @pytest.mark.parametrize(
         "input, expected",
         [
-            ("1h 1m", D(h="1h", m="1m")),
-            ("1h1m", D(h="1h", m="1m")),
-            ("1h 59m", D(h="1h", m="59m")),
-            ("1h59m", D(h="1h", m="59m")),
-            ("1h 0m", D()),
-            ("1h 60m", D()),
-            ("23h 1m", D(h="23h", m="1m")),
-            ("23h1m", D(h="23h", m="1m")),
+            # valid
+            ("1h 1m", D(h="1", m="1")),
+            ("1h1m", D(h="1", m="1")),
+            ("1h 59m", D(h="1", m="59")),
+            ("1h59m", D(h="1", m="59")),
+            ("3h1m", D(h="3", m="1")),
+            ("2h", D(h="2", m=None)),
+            ("42m", D(m="42")),
+            ("8h 59", D(h="8", m="59")),
+            # invalid
+            ("9h 1m", D()),
+            ("8 20", D()),
             ("24h 1m", D()),
             ("0h 1m", D()),
-            ("2h", D(h="2h", m=None)),
-            ("42m", D(h=None, m="42m")),
+            ("1h 0m", D()),
+            ("1h 60m", D()),
+            ("500m", D()),  # more than 8 h (exactly 8h 19m), invalid
+            ("9m", D()),  # less than 10 min, invalid
         ],
     )
     def test_evaluates_time_format(self, input, expected):
@@ -922,15 +942,15 @@ class TestValidateTime:
     def test_passes_when_time_is_none(self, mock_matches_time_re):
         result = validate_time(Mock(), Mock(), None)
 
-        assert result is None
+        assert result == 0
         mock_matches_time_re.assert_not_called()
 
     def test_passes_when_validator_passes(self, mock_matches_time_re):
-        mock_matches_time_re.return_value = D(h="2h")
+        mock_matches_time_re.return_value = D(h="2")
 
         result = validate_time(Mock(), Mock(), "2h")
 
-        assert result == "2h"
+        assert result == 7200
         mock_matches_time_re.assert_called_with("2h")
 
     def test_raises_otherwise(self, mock_matches_time_re):
@@ -940,7 +960,8 @@ def test_raises_otherwise(self, mock_matches_time_re):
             validate_time(Mock(), Mock(), "invalid")
 
         mock_matches_time_re.assert_called_with("invalid")
-        assert "time has" in str(exc_info)
+        assert "time cannot be greater than" in str(exc_info)
+        assert "has to be in format '[Nh][ N[m]]' or 'Nm'" in str(exc_info)
 
 
 @patch("src.dzira.dzira.is_valid_hour")