Bug #837

- Fixed bug #837, which caused incorrect urgency calculations for tasks that
  have completed dependencies, and problems when editing those tasks (thanks
  to Matt Kraai).
- Added Task::addDependency (const std::string&) method for reconstructing
  dependencies on tasks with no ID.
- Modfified Task::urgency_blocked so that it considers the blocking task's
  status.  This is an expensive test, and so it is wrapped by a cheaper test
  to see if there are/were any dependencies at all.  This means that urgency
  calculations are not slowed for tasks without dependencies, and is slower
  but more accurate for tasks that do have dependencies.
- Modified edit command so that dependencies are shown as IDs or UUIDs
  depending on whether the task is pending or not.
- Modified edit command so that dependencies are parsed as IDs or UUIDs
  depending on whether the task is pending or not.
- Modified wording in the template of the edit command to reflect the above.
- Added unit tests bug.837.t.

ChangeLog

@@ -231,6 +231,9 @@
  + Fixed bug #835, which prevented hierarchical projects from being recognized.
  + Fixed bug #836, which preserves numeric arguments as-is (thanks to Matt Kraai
    for the patch).
+ + Fixed bug #837, which caused incorrect urgency calculations for tasks that
+   have completed dependencies, and problems when editing those tasks (thanks
+   to Matt Kraai).
  + Fixed bug #839, which caused problems when recurrence frequencies of '1m'
    were used.  This is an obsolete form, and should now be '1mo' (thanks to
    Gour D).

src/Task.cpp

@@ -743,6 +743,32 @@ void Task::addDependency (int id)
   recalc_urgency = true;
 }
 
+////////////////////////////////////////////////////////////////////////////////
+void Task::addDependency (const std::string& uuid)
+{
+  if (uuid == get ("uuid"))
+    throw std::string (STRING_TASK_DEPEND_ITSELF);
+
+  // Store the dependency.
+  std::string depends = get ("depends");
+  if (depends != "")
+  {
+    // Check for extant dependency.
+    if (depends.find (uuid) == std::string::npos)
+      set ("depends", depends + "," + uuid);
+    else
+      throw format (STRING_TASK_DEPEND_DUP, this->get ("uuid"), uuid);
+  }
+  else
+    set ("depends", uuid);
+
+  // Prevent circular dependencies.
+  if (dependencyIsCircular (*this))
+    throw std::string (STRING_TASK_DEPEND_CIRCULAR);
+
+  recalc_urgency = true;
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 void Task::removeDependency (const std::string& uuid)
 {
@@ -1333,10 +1359,25 @@ float Task::urgency_waiting () const
 }
 
 ////////////////////////////////////////////////////////////////////////////////
+// A task is blocked only if the task it depends upon is pending/waiting.
 float Task::urgency_blocked () const
 {
   if (has ("depends"))
+  {
+    std::vector <std::string> deps;
+    getDependencies (deps);
+
+    std::vector <std::string>::iterator d;
+    for (d = deps.begin (); d != deps.end (); ++d)
+    {
+      Task t;
+      if (context.tdb2.get (*d, t) &&
+          (t.getStatus () == Task::pending || t.getStatus () == Task::waiting))
+      {
         return 1.0;
+      }
+    }
+  }
 
   return 0.0;
 }

src/Task.h

@@ -91,6 +91,7 @@ public:
   void removeAnnotations ();
 
   void addDependency (int);
+  void addDependency (const std::string&);
   void removeDependency (int);
   void removeDependency (const std::string&);
   void getDependencies (std::vector <int>&) const;

src/commands/CmdEdit.cpp

@@ -212,15 +212,27 @@ std::string CmdEdit::formatTask (Task task)
   before << "  Annotation:        " << now.toString (context.config.get ("dateformat.annotation")) << " -- \n";
 
   // Add dependencies here.
-  std::vector <int> dependencies;
+  std::vector <std::string> dependencies;
   task.getDependencies (dependencies);
-  std::string allDeps;
-  join (allDeps, ",", dependencies);
+  std::stringstream allDeps;
+  for (unsigned int i = 0; i < dependencies.size (); ++i)
+  {
+    if (i)
+      allDeps << ",";
+
+    Task t;
+    context.tdb2.get (dependencies[i], t);
+    if (t.getStatus () == Task::pending ||
+        t.getStatus () == Task::waiting)
+      allDeps << t.id;
+    else
+      allDeps << dependencies[i];
+  }
 
   if (verbose)
     before << "# " << STRING_EDIT_DEP_SEP << "\n";
 
-  before << "  Dependencies:      " << allDeps << "\n";
+  before << "  Dependencies:      " << allDeps.str () << "\n";
 
   before << "# " << STRING_EDIT_END << "\n";
   return before.str ();
@@ -582,9 +594,18 @@ void CmdEdit::parseTask (Task& task, const std::string& after)
   std::vector <std::string>::iterator dep;
   for (dep = dependencies.begin (); dep != dependencies.end (); ++dep)
   {
-    int id = atoi (dep->c_str ());
+    int id = 0;
+
+    // Crude UUID check.
+    if (dep->length () == 36)
+      id = context.tdb2.pending.id (*dep);
+    else
+      id = atoi (dep->c_str ());
+
     if (id)
       task.addDependency (id);
+    else
+      task.addDependency (*dep);
   }
 }
 

src/en-US.h

@@ -597,7 +597,7 @@
 #define STRING_EDIT_UNPARSEABLE      "Taskwarrior couldn't handle your edits.  Would you like to try again?"
 #define STRING_EDIT_UNWRITABLE       "Your data.location directory is not writable."
 #define STRING_EDIT_TAG_SEP          "Separate the tags with spaces, like this: tag1 tag2"
-#define STRING_EDIT_DEP_SEP          "Dependencies should be a comma-separated list of task IDs, with no spaces."
+#define STRING_EDIT_DEP_SEP          "Dependencies should be a comma-separated list of task IDs/UUIDs, with no spaces."
 #define STRING_EDIT_END              "End"
 
 #define STRING_EDIT_PROJECT_MOD      "Project modified."

test/bug.837.t

@@ -0,0 +1,69 @@
+#! /usr/bin/perl
+################################################################################
+## taskwarrior - a command line task list manager.
+##
+## Copyright 2006-2012, Paul Beckingham, Federico Hernandez.
+##
+## Permission is hereby granted, free of charge, to any person obtaining a copy
+## of this software and associated documentation files (the "Software"), to deal
+## in the Software without restriction, including without limitation the rights
+## to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+## copies of the Software, and to permit persons to whom the Software is
+## furnished to do so, subject to the following conditions:
+##
+## The above copyright notice and this permission notice shall be included
+## in all copies or substantial portions of the Software.
+##
+## THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+## OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+## FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+## THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+## LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+## OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+## SOFTWARE.
+##
+## http://www.opensource.org/licenses/mit-license.php
+##
+################################################################################
+
+use strict;
+use warnings;
+use Test::More tests => 5;
+
+# Create the rc file.
+if (open my $fh, '>', 'bug.rc')
+{
+  print $fh "data.location=.\n";
+  close $fh;
+  ok (-r 'bug.rc', 'Created bug.rc');
+}
+
+# Bug 837: When a task is completed, tasks that depended upon it don't have
+#          correct urgency and depend on 0 when edited
+qx{../src/task rc:bug.rc add one};
+qx{../src/task rc:bug.rc add two dep:1};
+my $output = qx{../src/task rc:bug.rc long};
+
+$output = qx{../src/task rc:bug.rc 1 _urgency};
+like ($output, qr/ 8\n/, 'blocking urgency == 8');
+
+$output = qx{../src/task rc:bug.rc 2 _urgency};
+like ($output, qr/ -5\n/, 'blocked urgency == -5');
+
+qx{../src/task rc:bug.rc 1 done};
+qx{../src/task rc:bug.rc list};
+
+$output = qx{../src/task rc:bug.rc 1 _urgency};
+like ($output, qr/ 0\n/, 'unblocked urgency == 0');
+
+# Cleanup.
+unlink qw(pending.data completed.data undo.data backlog.data synch.key bug.rc);
+ok (! -r 'pending.data'   &&
+    ! -r 'completed.data' &&
+    ! -r 'undo.data'      &&
+    ! -r 'backlog.data'   &&
+    ! -r 'synch.key'      &&
+    ! -r 'bug.rc', 'Cleanup');
+
+exit 0;
+