getIntervalsById: Only add truly synthetic intervals to synthetic vector

This fixes an error where the latest interval, a non-synthetic interval,
is added to the synthetic array (and marked synthetic) if flatten() is
otherwise not able to convert it to a group of synthetic intervals.

When modify was attempting to edit this interval, it would fail since
synthetic intervals should not be written to the database unless the
database is being flattened and timewarrior believed the interval to be
modified was synthetic.

Closes #422

Signed-off-by: Shaun Ruffell <sruffell@sruffell.net>

src/data.cpp

@@ -25,6 +25,7 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 #include <algorithm>
+#include <cassert>
 #include <deque>
 
 #include <cmake.h>
@@ -175,15 +176,28 @@ std::vector <Interval> getIntervalsByIds (
     auto exclusions = getAllExclusions (rules, {latest.start, Datetime ()});
     if (! exclusions.empty ())
     {
-      // We're converting the latest interval into synthetic intervals so we need to skip it
-      ++it;
+      std::vector <Interval> flattened = flatten (latest, exclusions);
 
-      for (auto& interval : flatten (latest, exclusions))
+      // If flatten() converted the latest interval into a group of synthetic
+      // intervals, the number of returned intervals will be greater than 1,
+      // otherwise, it just returned the non-synthetic latest interval.
+      if (flattened.size () > 1)
       {
-        ++current_id;
-        interval.synthetic = true;
-        interval.id = current_id;
-        synthetic.push_front (interval);
+        // Skip over the latest interval since we're converting it into a group
+        // of synthetic intervals
+        ++it;
+
+        for (auto& interval : flattened)
+        {
+          ++current_id;
+          interval.synthetic = true;
+          interval.id = current_id;
+          synthetic.push_front (std::move(interval));
+        }
+      }
+      else
+      {
+        assert (latest == flattened[0]);
       }
     }
   }

test/modify.t

@@ -249,6 +249,35 @@ class TestModify(TestCase):
                                 expectedTags=[],
                                 description="moved interval")
 
+    def test_modify_move_start_inside_exclusion(self):
+        """`timew modify` should handle moving start times within an exclusion."""
+
+        now = datetime.now().replace(second=0, microsecond=0, minute=0)
+        three_hours_before = now - timedelta(hours=3)
+        four_hours_before = now - timedelta(hours=4)
+
+        now_utc = now.utcnow().replace(second=0, microsecond=0, minute=0)
+        day_before = now_utc - timedelta(days=1)
+        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.configure_exclusions((four_hours_before.time(), three_hours_before.time()))
+
+        # Start an interval within the exclusion
+        self.t("start {:%Y-%m-%dT%H:%M:%S} foo".format(four_hours_before + timedelta(minutes=20)))
+
+        # Now modify the start time, but keep the start within the exclusion
+        self.t("modify start @1 {:%Y-%m-%dT%H:%M:%S} :debug".format(four_hours_before + timedelta(minutes=10)))
+
+        j = self.t.export() 
+
+        self.assertEqual(len(j), 1)
+        self.assertOpenInterval(j[0],
+                                expectedStart=four_hours_before_utc + timedelta(minutes=10),
+                                expectedTags=['foo'])
+
+
 if __name__ == "__main__":
     from simpletap import TAPTestRunner