Bug Fix - recurrence

- Fixed bug whereby handleRecurrence was being called after the tasks
  were loaded and filtered, and thus handleRecurrence operated on a
  filtered set, and failed.  The fix is to move the call to before the
  TDB::load call, and to add another TDB::loadPending call inside
  handleRecurrence.  This means TDB::load needs to be reentrant without
  re-reading the file, and can therefore be called twice, with the
  likelihood of there being a different filter for each call.  This in
  turn led to the problem whereby handleRecurrence would generate the
  synthetic tasks, which then sat uncommitted in TDB::mNew.  The fix
  for this is that every call to TDB::loadPending gets the contents of
  TDB::mNew appended (with correct IDs).  This bug is what you might
  call a good one.

src/TDB.cpp

@@ -85,6 +85,7 @@ TDB::~TDB ()
 ////////////////////////////////////////////////////////////////////////////////
 void TDB::clear ()
 {
+  mPending.clear ();
   mLocations.clear ();
   mLock = true;
 
@@ -196,33 +197,47 @@ int TDB::loadPending (std::vector <Task>& tasks, Filter& filter)
 
   try
   {
-    mPending.clear ();
-
-    mId = 1;
-    char line[T_LINE_MAX];
-    foreach (location, mLocations)
+    if (mPending.size () == 0)
     {
-      line_number = 1;
-      file = location->path + "/pending.data";
-
-      fseek (location->pending, 0, SEEK_SET);
-      while (fgets (line, T_LINE_MAX, location->pending))
+      mId = 1;
+      char line[T_LINE_MAX];
+      foreach (location, mLocations)
       {
-        int length = ::strlen (line);
-        if (length > 1)
+        line_number = 1;
+        file = location->path + "/pending.data";
+
+        fseek (location->pending, 0, SEEK_SET);
+        while (fgets (line, T_LINE_MAX, location->pending))
         {
-          // TODO Add hidden attribute indicating source?
-          Task task (line);
-          task.id = mId++;
+          int length = ::strlen (line);
+          if (length > 1)
+          {
+            // TODO Add hidden attribute indicating source?
+            Task task (line);
+            task.id = mId++;
 
-          mPending.push_back (task);
-          if (filter.pass (task))
-            tasks.push_back (task);
+            mPending.push_back (task);
+          }
+
+          ++line_number;
         }
-
-        ++line_number;
       }
     }
+
+    // Now filter and return.
+    foreach (task, mPending)
+      if (filter.pass (*task))
+        tasks.push_back (*task);
+
+    // Hand back any accumulated additions, if TDB::loadPending is being called
+    // repeatedly.
+    int fakeId = mId;
+    foreach (task, mNew)
+    {
+      task->id = fakeId++;
+      if (filter.pass (*task))
+        tasks.push_back (*task);
+    }
   }
 
   catch (std::string& e)
@@ -267,7 +282,7 @@ int TDB::loadCompleted (std::vector <Task>& tasks, Filter& filter)
             line[length - 1] = '\0';
 
           Task task (line);
-          task.id = mId++;
+          // Note: no id is set for completed tasks.
 
           if (filter.pass (task))
             tasks.push_back (task);
@@ -294,6 +309,7 @@ int TDB::loadCompleted (std::vector <Task>& tasks, Filter& filter)
 void TDB::add (const Task& task)
 {
   mNew.push_back (task);
+
 }
 
 ////////////////////////////////////////////////////////////////////////////////

src/command.cpp

@@ -98,8 +98,8 @@ std::string handleProjects ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   int quantity = context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -157,8 +157,8 @@ std::string handleTags ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   int quantity = context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -455,8 +455,8 @@ std::string handleDelete ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   context.filter.applySequence (tasks, context.sequence);
@@ -553,8 +553,8 @@ std::string handleStart ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   context.filter.applySequence (tasks, context.sequence);
@@ -602,8 +602,8 @@ std::string handleStop ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   context.filter.applySequence (tasks, context.sequence);
@@ -648,8 +648,8 @@ std::string handleDone ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   std::vector <Task> all = tasks;
@@ -736,8 +736,8 @@ std::string handleExport ()
   // Get all the tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -762,8 +762,8 @@ std::string handleModify ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   std::vector <Task> all = tasks;
@@ -824,8 +824,8 @@ std::string handleAppend ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   std::vector <Task> all = tasks;

src/custom.cpp

@@ -101,8 +101,8 @@ std::string handleCustomReport (const std::string& report)
   // Get all the tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.load (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 

src/edit.cpp

@@ -490,8 +490,8 @@ std::string handleEdit ()
 
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
 
   // Filter sequence.
   context.filter.applySequence (tasks, context.sequence);

src/main.h

@@ -49,7 +49,7 @@ void gatherNextTasks (std::vector <Task>&, std::vector <int>&);
 void onChangeCallback ();
 
 // recur.cpp
-void handleRecurrence (std::vector <Task>&);
+void handleRecurrence ();
 Date getNextRecurrence (Date&, std::string&);
 bool generateDueDates (Task&, std::vector <Date>&);
 void updateRecurrenceMask (std::vector <Task>&, Task&);

src/recur.cpp

@@ -53,8 +53,12 @@ extern Context context;
 ////////////////////////////////////////////////////////////////////////////////
 // Scans all tasks, and for any recurring tasks, determines whether any new
 // child tasks need to be generated to fill gaps.
-void handleRecurrence (std::vector <Task>& tasks)
+void handleRecurrence ()
 {
+  std::vector <Task> tasks;
+  Filter filter;
+  context.tdb.loadPending (tasks, filter);
+
   std::vector <Task> modified;
 
   // Look at all tasks and find any recurring ones.
@@ -97,7 +101,6 @@ void handleRecurrence (std::vector <Task>& tasks)
           changed = true;
 
           Task rec (*t);                         // Clone the parent.
-          rec.id = context.tdb.nextId ();        // Assign a unique id.
           rec.set ("uuid", uuid ());             // New UUID.
           rec.setStatus (Task::pending);         // Shiny.
           rec.set ("parent", t->get ("uuid"));   // Remember mom.

src/report.cpp

@@ -268,8 +268,8 @@ std::string handleInfo ()
   // Get all the tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -485,8 +485,8 @@ std::string handleReportSummary ()
   // Scan the pending tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.load (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -650,8 +650,8 @@ std::string handleReportNext ()
   // Get all the tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -814,8 +814,8 @@ std::string handleReportHistory ()
   // Scan the pending tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.load (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -968,8 +968,8 @@ std::string handleReportGHistory ()
   // Scan the pending tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.load (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -1159,8 +1159,8 @@ std::string handleReportTimesheet ()
   // Scan the pending tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.load (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -1490,8 +1490,8 @@ std::string handleReportCalendar ()
   // Get all the tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.loadPending (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();
 
@@ -1613,8 +1613,8 @@ std::string handleReportStats ()
   // Get all the tasks.
   std::vector <Task> tasks;
   context.tdb.lock (context.config.get ("locking", true));
+  handleRecurrence ();
   context.tdb.load (tasks, context.filter);
-  handleRecurrence (tasks);
   context.tdb.commit ();
   context.tdb.unlock ();