Restore behaviour for adding a new interval with same tag set

- Closes #351

Signed-off-by: Thomas Lauf <thomas.lauf@tngtech.com>

src/commands/CmdContinue.cpp

@@ -114,10 +114,11 @@ int CmdContinue (
   to_copy.end = end_time;
 
   journal.startTransaction ();
-  validate (cli, rules, database, to_copy);
-  database.addInterval (to_copy, verbose);
-  journal.endTransaction ();
-
+  if (validate (cli, rules, database, to_copy))
+  {
+    database.addInterval (to_copy, verbose);
+    journal.endTransaction ();
+  }
   if (verbose)
   {
     std::cout << intervalSummarize (database, rules, to_copy);

src/commands/CmdStart.cpp

@@ -51,10 +51,11 @@ int CmdStart (
   }
 
   journal.startTransaction ();
-  validate (cli, rules, database, interval);
-  database.addInterval (interval, verbose);
-  journal.endTransaction ();
-
+  if (validate (cli, rules, database, interval))
+  {
+    database.addInterval (interval, verbose);
+    journal.endTransaction ();
+  }
   if (verbose)
   {
     std::cout << intervalSummarize (database, rules, interval);

src/timew.h

@@ -59,7 +59,7 @@ Range                   getFullDay        (const Datetime&);
 
 // validate.cpp
 void autoFill (const Rules&, Database&, Interval&);
-void validate (const CLI& cli, const Rules& rules, Database&, Interval&);
+bool validate (const CLI& cli, const Rules& rules, Database&, Interval&);
 
 // init.cpp
 bool lightweightVersionCheck (int, const char**);

src/validate.cpp

@@ -91,7 +91,7 @@ void autoFill (
 //   can involve rejection, adjustment of modified interval, or adjustment of
 //   recorded data.
 //
-static void autoAdjust (
+static bool autoAdjust (
   bool adjust,
   const Rules& rules,
   Database& database,
@@ -108,7 +108,7 @@ static void autoAdjust (
     {
       // If the new interval tags match those of the currently open interval,
       // then do nothing - the tags are already being tracked.
-      return;
+      return false;
     }
 
     database.deleteInterval (latest);
@@ -128,7 +128,7 @@ static void autoAdjust (
 
   if (overlaps.empty ())
   {
-    return;
+    return true;
   }
 
   debug ("Input         " + interval.dump ());
@@ -195,10 +195,11 @@ static void autoAdjust (
       }
     }
   }
+  return true;
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-void validate (
+bool validate (
   const CLI& cli,
   const Rules& rules,
   Database& database,
@@ -210,7 +211,7 @@ void validate (
     autoFill (rules, database, interval);
   }
 
-  autoAdjust (findHint (cli, ":adjust"), rules, database, interval);
+  return autoAdjust (findHint (cli, ":adjust"), rules, database, interval);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

test/continue.t

@@ -394,6 +394,70 @@ class TestContinue(TestCase):
                                 expectedTags=["FOO"],
                                 description="continued interval")
 
+    def test_continue_with_id_is_idempotent(self):
+        """Verify that continuing with id is idempotent"""
+        now_utc = datetime.now().utcnow()
+
+        two_hours_before_utc = now_utc - timedelta(hours=2)
+        three_hours_before_utc = now_utc - timedelta(hours=3)
+        four_hours_before_utc = now_utc - timedelta(hours=4)
+        five_hours_before_utc = now_utc - timedelta(hours=5)
+
+        self.t("track FOO {:%Y-%m-%dT%H}:00:00Z - {:%Y-%m-%dT%H}:00:00Z".format(five_hours_before_utc, four_hours_before_utc))
+        self.t("track BAR {:%Y-%m-%dT%H}:00:00Z - {:%Y-%m-%dT%H}:00:00Z".format(four_hours_before_utc, three_hours_before_utc))
+        self.t("start FOO {:%Y-%m-%dT%H}:00:00Z".format(three_hours_before_utc))
+        self.t("continue @3 {:%Y-%m-%dT%H}:00:00Z".format(now_utc))
+
+        j = self.t.export()
+
+        self.assertEqual(len(j), 3)
+        self.assertClosedInterval(j[0],
+                                  expectedStart="{:%Y%m%dT%H}0000Z".format(five_hours_before_utc),
+                                  expectedEnd="{:%Y%m%dT%H}0000Z".format(four_hours_before_utc),
+                                  expectedTags=["FOO"],
+                                  description="third interval")
+        self.assertClosedInterval(j[1],
+                                  expectedStart="{:%Y%m%dT%H}0000Z".format(four_hours_before_utc),
+                                  expectedEnd="{:%Y%m%dT%H}0000Z".format(three_hours_before_utc),
+                                  expectedTags=["BAR"],
+                                  description="second interval")
+        self.assertOpenInterval(j[2],
+                                expectedStart="{:%Y%m%dT%H}0000Z".format(three_hours_before_utc),
+                                expectedTags=["FOO"],
+                                description="first interval")
+
+    def test_continue_with_tag_is_idempotent(self):
+        """Verify that continuing with id is idempotent"""
+        now_utc = datetime.now().utcnow()
+
+        two_hours_before_utc = now_utc - timedelta(hours=2)
+        three_hours_before_utc = now_utc - timedelta(hours=3)
+        four_hours_before_utc = now_utc - timedelta(hours=4)
+        five_hours_before_utc = now_utc - timedelta(hours=5)
+
+        self.t("track FOO {:%Y-%m-%dT%H}:00:00Z - {:%Y-%m-%dT%H}:00:00Z".format(five_hours_before_utc, four_hours_before_utc))
+        self.t("track BAR {:%Y-%m-%dT%H}:00:00Z - {:%Y-%m-%dT%H}:00:00Z".format(four_hours_before_utc, three_hours_before_utc))
+        self.t("start FOO {:%Y-%m-%dT%H}:00:00Z".format(three_hours_before_utc))
+        self.t("continue FOO {:%Y-%m-%dT%H}:00:00Z".format(now_utc))
+
+        j = self.t.export()
+
+        self.assertEqual(len(j), 3)
+        self.assertClosedInterval(j[0],
+                                  expectedStart="{:%Y%m%dT%H}0000Z".format(five_hours_before_utc),
+                                  expectedEnd="{:%Y%m%dT%H}0000Z".format(four_hours_before_utc),
+                                  expectedTags=["FOO"],
+                                  description="third interval")
+        self.assertClosedInterval(j[1],
+                                  expectedStart="{:%Y%m%dT%H}0000Z".format(four_hours_before_utc),
+                                  expectedEnd="{:%Y%m%dT%H}0000Z".format(three_hours_before_utc),
+                                  expectedTags=["BAR"],
+                                  description="second interval")
+        self.assertOpenInterval(j[2],
+                                expectedStart="{:%Y%m%dT%H}0000Z".format(three_hours_before_utc),
+                                expectedTags=["FOO"],
+                                description="first interval")
+
     def test_continue_with_id_and_range(self):
         """Verify that continue with a range adds a copy with same tags"""
         now_utc = datetime.now().utcnow()

test/start.t

@@ -131,11 +131,17 @@ class TestStart(TestCase):
 
     def test_start_with_same_tags_as_current_tracking(self):
         """Test 'start' with same tags as current tracking should not start new tracking"""
-        self.t("start 1h ago bar foo")
+        utc_now = datetime.now().utcnow()
+        one_hour_ago_utc = utc_now - timedelta(hours=1)
 
-        code, out, err = self.t("start foo bar")
+        self.t("start {:%Y-%m-%dT%H:%M:%S}Z bar foo".format(one_hour_ago_utc))
 
-        self.assertNotIn("Recorded bar foo", out)
+        self.t("start foo bar")
+
+        j = self.t.export()
+
+        self.assertEqual(len(j), 1)
+        self.assertOpenInterval(j[0], expectedStart=one_hour_ago_utc, expectedTags=["foo", "bar"])
 
     def test_single_interval_enclosing_exclusion(self):
         """Add one interval that encloses an exclusion, and is therefore flattened"""