Merge lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date into lp:~gtg/gtg/old-trunk

[Bertrand Rousseau]

I changed the due date inheritance mechanism so that due date of children is not altered if it's not set.

Since I can't come up with a good UI to display the constraining date in the task editor, I think the best solution is to leave the due date selector widget blank and not display anything about a constraining date.

[lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date]

1233. By Bertrand Rousseau on 2012-08-29

Update CHANGELOG

[Lionel Dricot (ploum-deactivatedaccount)]

I've tested and I see a problem :

1. Set the parent due date to "soon"

2. Open the children and set due date to "now".

Everything is fine.

3. Now set child due date to "later": immediately, the parent due date is changed too.

Is that what we really want ?

review: Needs Information

[Izidor Matušov (izidor)]

I don't think the case Lionel talked about is solved correctly. Fuzzy dates are and should be fuzzy: you can put that something is due "now", "soon" or "someday" although the parent is due tomorrow. The user shouldn't care about what soon means - is it 7 or 14 days? Otherwise she might want to configure it => added complexity. Fuzzy dates should be transparent and not constrain any parent or child.

review: Needs Fixing (code)

[lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date]

1234. By Bertrand Rousseau on 2012-10-21

Merge trunk and update set_due_date/get_constrained_date to avoid applying constrains to fuzzy dates. Also fix a bug: constrained were not applied through undefined tasks.

[Bertrand Rousseau (bertrand-rousseau)]

Izidor> Constraints are now not applied anymore to tasks with fuzzy due dates. However, it's not clear for how the task list should interpret some constraints:

If have a task set to "Tomorrow", and a child set to "Soon", what should be displayed for the child task in the task list? "Soon" or "Tomorrow"? IMHO, "Tomorrow" is better since it's more precise. (It's currently implemented that way: I consider precise dates to have the priority on fuzzy dates.)

Another situation: if I have a task set to "Now", and a child set to "Soon", what should be shown for the child? "Now"? Currently, I don't consider constraints between fuzzy dates, but it might make sense. If so, then it would also make sense to update related task due dates even when they are fuzzy, but it's in opposition to Lionel's comment.

And finally, if I have a task with "Soon" and a child with an undefined due date, should I display "Soon" as well for the child? Currently, I don't do this, but it might make sense too since that's what's done for precise dates.

[Izidor Matušov (izidor)]

It is hard to say what is right, we should define it on our own. I am proposing the following semantics:

Fuzzy dates "now" and "soon" are fully transparent. It means that a task can have now or soon althoug the parent has a precise date like tomorrow. (They are fuzzy!!!) Children would heritate the fuzzy due date. However, when you put precise date into ancestors, it complains regular limitations. (Fuzzy dates are transparent). Look at the following (valid) structure:

a (tomorrow)
-b (soon)
--c (now)
----d (soon)
-----e (today)

I would say that someday is not valid for any task with parent because you already have deadline and it is something more precise than someday. It means you would always rewrite date.

What do you say?

[Bertrand Rousseau (bertrand-rousseau)]

I agree, it is quite hard. For the record, I don't clearly understand
the use case for fuzzy dates and if it were for me alone, I would
remove that feature (at least from core), since I don't really think it
is necessary to GTG. But that's another discussion.

I don't really like the idea of sticking too much to the semantics.
Indeed, since fuzzy dates are fuzzy, everyone might like to interpret
things his/her way. So I'd prefer to leave the complete liberty of
interpretation to the user here, even if it could appear incompatible
to have an ancestor due for "now", and a child due for "someday".

So, I suggest to make ALL fuzzy dates transparent (even "someday"), and
to NOT check for consistency among fuzzy dates (total transparency).
And finally, since fuzzy dates are used at the explicit request of a
user to make those tasks stand out among the others, I propose to keep
showing fuzzy dates instead of precise dates even when an ancestor has
a precise due date.

So all in all, it would correspond to your suggestion and example, with
the addition of "someday".

I think that would allow to give a clear-ish definition of fuzzy dates
to the user: "fuzzy dates are there to provide you with some liberty in
assigning due dates to tasks (until you can clearly decide, for
instance). They don't follow and don't impose any constraining rules
themselves, and they let constaints from parents and children go
through them untouched".

If people seems okay with that (Lionel? Any opinion on this?) That's
what I'll implement.

Bertrand

On Mon 22 Oct 2012 10:47:20 PM CEST, Izidor Matušov wrote:
> It is hard to say what is right, we should define it on our own. I am proposing the following semantics:
>
> Fuzzy dates "now" and "soon" are fully transparent. It means that a task can have now or soon althoug the parent has a precise date like tomorrow. (They are fuzzy!!!) Children would heritate the fuzzy due date. However, when you put precise date into ancestors, it complains regular limitations. (Fuzzy dates are transparent). Look at the following (valid) structure:
>
> a (tomorrow)
> -b (soon)
> --c (now)
> ----d (soon)
> -----e (today)
>
> I would say that someday is not valid for any task with parent because you already have deadline and it is something more precise than someday. It means you would always rewrite date.
>
> What do you say?

--
Bertrand Rousseau
<email address hidden>

[Izidor Matušov (izidor)]

I agree, let's go with the fully transparent fuzzy dates!

[lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date]

1235. By Bertrand Rousseau on 2012-10-28

Update due date update policy.

Updated the due date update policy as discussed here in launchpad [1]:
ALL fuzzy dates transparent (even "someday"), and we DONT check for consistency
among fuzzy dates (total transparency). And finally, since fuzzy dates are used
at the explicit request of a user to make those tasks stand out among the
others, we keep showing fuzzy dates instead of precise dates even when an
ancestor has a precise due date.

[1] https://code.launchpad.net/~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date/+merge/121929/comments/282054.

[Bertrand Rousseau (bertrand-rousseau)]

I just updated the code to reflect what we discussed here. Please test the branch and tell me if it is ok!

[lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date]

1236. By Bertrand Rousseau on 2012-10-28

Ooops: left out a small bug, task list was not displaying constraining due date for undefined but constrained child task

1237. By Bertrand Rousseau on 2012-10-28

Fix for bug #1036695: Date constraints after drag and drop not applied

[Bertrand Rousseau (bertrand-rousseau)]

I also inspected the code for bug #1036695: "Date constraints after drag and drop not applied" and found a fix. To reduce review work I integrate this fix directly in this merge review.

[lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date]

1238. By Bertrand Rousseau on 2012-11-01

Merge trunk

1239. By Bertrand Rousseau on 2012-11-01

Oops: accidently removed my fix for constraints update after DnD while merging with the trunk

[Izidor Matušov (izidor)]

When I drag a child task to make it a stand alone task, I get the following error:

Problem with dragging: 'NoneType' object has no attribute 'get_due_date_constraint'

review: Needs Fixing

[Izidor Matušov (izidor)]

Otherwise the patch seems good to me and it works good.

[lp:~bertrand-rousseau/gtg/bugfix-1038662-dont-touch-due-date]

1240. By Bertrand Rousseau on 2012-11-01

Fix bug in set_parent

[Bertrand Rousseau (bertrand-rousseau)]

Fix the bug found by Izidor ('NoneType' object has no attribute 'get_due_date_constraint').

FYI, It seems that this patch has a negative impact on performances (since constraining due dates are computed dynamically for fuzzy/undefined due date). On "normal" dataset, it is about 3% slower (which represents only 30ms on my laptop), but on big datasets (Bryce-ish datasets), it can go up to 10% (=3s on my laptop). I tried to fix this by updating the value only when required and caching it, but I can't succeed with this approach since during boot time it seems that the method "set_parent" is not used to update the task hierarchy. Consequently, I can't catch all task relations updates and the due date constraints are not correctly set up. I have no idea how to catch task relations updates any other way than that, so I don't know how to fix this. Now, since the impact is quite limited, I guess we could still merge the patch.

review: Needs Resubmitting

[Izidor Matušov (izidor)]

review: Approve

=== modified file 'CHANGELOG'
--- CHANGELOG	2012-11-01 10:09:06 +0000
+++ CHANGELOG	2012-11-01 11:54:31 +0000
@@ -54,6 +54,8 @@
     * Fix for bug #1036955: Due date is not preselected when start date is filled, by Steve Scheel
     * Fix for bug #1045036: Slovak Translation Updated by Slavko
     * Remove use of liblarch's "transparent" concept (since it's been removed from liblarch), fixes bugs #1001962, #1001962, #1069257, #1069963: intermediary tags, counter initialization, and regressions caused by initial versions of the patch
+    * Fix for bug #1038662: Undefined due dates in subtasks should always stay undefined and displayed as such in the editor
+    * Fix for bug #1036695: Date constraints after drag and drop not applied
 
 2012-02-13 Getting Things GNOME! 0.2.9
     * Big refractorization of code, now using liblarch
 
=== modified file 'GTG/core/task.py'
--- GTG/core/task.py	2012-11-01 10:09:06 +0000
+++ GTG/core/task.py	2012-11-01 11:54:31 +0000
@@ -246,68 +246,177 @@
     def set_modified(self, modified):
         self.last_modified = modified
 
-    def set_due_date(self, fulldate):
+    def recursive_sync(self):
+        """Recursively sync the task and all task children. Defined"""
+        self.sync()
+        for sub_id in self.children:
+            sub = self.req.get_task(sub_id)
+            sub.recursive_sync()
+
+    # ABOUT DUE DATES
+    #
+    # PLEASE READ THIS: although simple in appearance, handling task dates can
+    # actually be subtle. Take the time to understand this if you plan to work
+    # on the methods below.
+    #
+    # Due date is the date at which a task must be accomplished. Constraints
+    # exist between a task's due date and its ancestor/children's due dates.
+    #
+    # Date constraints
+    #
+    # Those are the following:
+    #   - children of a task cannot have a task due date that happens later than
+    #     the task's due date
+    #   - ancestors of a task cannot have a due that happens before the
+    #     task's due date (this is the reverse constraint from the first one)
+    #   - a task's start date cannot happen later than this task's due date
+    #
+    # Tasks with undefined or fuzzy due dates
+    #
+    # Task with no due date (="undefined" tasks) or tasks with fuzzy start/due
+    # dates are not subject to constraints. Furthermore, they are "transparent".
+    # Meaning that they let the constraints coming from their children/parents
+    # pass through them. So, for instance, a children of a task with an
+    # undefined or fuzzy task would be constrained by this latter task's
+    # ancestors. Equally, the an ancestor from the same undefined/fuzzy task
+    # would be constrained by the children due dates.
+    #
+    # Updating a task due date
+    #
+    # Whenever a task due date is changed, all ancestor/chldren of this task
+    # *must* be updated according to the constraining rules. As said above,
+    # constraints must go through tasks with undefined/fuzzy due dates too!
+    # 
+    # Undefined/fuzzy task dates are NEVER to be updated. They are not sensitive
+    # to constraint. If you want to now what constraint there is on this task's
+    # due date though, you can obtain it by using the get_due_date_constraint
+    # method.
+
+    def set_due_date(self, new_duedate):
         """Defines the task's due date."""
-        def get_due_date_constraint():
-            """ Returns the most urgent due date constraint, following
-                parents' due dates. Return Date.no_date() if no constraint
-                is applied. """
-            cur_date = Date.no_date()
-            for par_id in self.get_parents():
-                par = self.req.get_task(par_id)
-                if par.get_due_date() != Date.no_date() and \
-                   par.get_due_date() < cur_date:
-                    cur_date = par.get_due_date()
-            return cur_date
-        fulldate_obj = Date(fulldate) # caching the conversion
-        self.due_date = fulldate_obj
-        # if the task's start date happens later than the 
-        # new due date, we update it
-        # (except for fuzzy dates)
-        if self.get_start_date() != Date.no_date() and \
-           not fulldate_obj.is_fuzzy() and \
-           self.get_start_date() > fulldate_obj:
-            self.set_start_date(fulldate)
-        if fulldate_obj != Date.no_date():
-            # if the parent's due date happens before the task's new
-            # due date, we update it
-            for par_id in self.parents:
-                par = self.req.get_task(par_id)
-                if par.get_due_date() != Date.no_date() and \
-                   par.get_due_date() < fulldate_obj:
-                    par.set_due_date(fulldate)
-            # the current task being one of its children's parents, we must
-            # apply the constraints on their due/start dates as well
-            for sub_id in self.children:
-                sub = self.req.get_task(sub_id)
-                # child's due date is not set, we use the task's new 
-                # due date
-                if sub.get_due_date() == Date.no_date():
-                    sub.set_due_date(fulldate)
-                # child's due date happens later than the task's: we
+
+        def __get_defined_parent_list(task):
+            """Recursively fetch a list of parents that have a defined due date
+               which is not fuzzy"""
+            parent_list = []
+            for par_id in task.parents:
+                par = self.req.get_task(par_id)
+                if par.get_due_date() == Date.no_date() or \
+                  ( par.get_due_date() != Date.no_date() and \
+                    par.get_due_date().is_fuzzy() ):
+                    parent_list += __get_defined_parent_list(par)
+                else:
+                    parent_list.append(par)
+            return parent_list
+
+        def __get_defined_child_list(task):
+            """Recursively fetch a list of children that have a defined due date
+               which is not fuzzy"""
+            child_list = []
+            for child_id in task.children:
+                child = self.req.get_task(child_id)
+                if child.get_due_date() == Date.no_date() or \
+                   ( child.get_due_date() != Date.no_date() and \
+                     child.get_due_date().is_fuzzy() ):
+                    child_list += __get_defined_child_list(child)
+                else:
+                    child_list.append(child)
+            return child_list
+
+        old_due_date    = self.due_date
+        new_duedate_obj = Date(new_duedate) # caching the conversion
+        self.due_date   = new_duedate_obj
+        # If the new date is fuzzy or undefined, we don't update related tasks
+        if not new_duedate_obj.is_fuzzy() and new_duedate_obj != Date.no_date():
+            # if the task's start date happens later than the 
+            # new due date, we update it (except for fuzzy dates)
+            if self.get_start_date() != Date.no_date() and \
+               not self.get_start_date().is_fuzzy()    and \
+               self.get_start_date() > new_duedate_obj:
+                self.set_start_date(new_duedate)
+            # if some ancestors' due dates happen before the task's new
+            # due date, we update them (except for fuzzy dates)
+            for par in __get_defined_parent_list(self):
+                if par.get_due_date() < new_duedate_obj:
+                    par.set_due_date(new_duedate)
+            # we must apply the constraints to the defined & non-fuzzy children
+            # as well
+            for sub in __get_defined_child_list(self):
+                sub_duedate = sub.get_due_date()
+                # if the child's due date happens later than the task's: we
                 # update it to the task's new due date
-                # (= the new most restrictive)
-                if sub.get_due_date() != Date.no_date() and \
-                   sub.get_due_date() > fulldate_obj:
-                    sub.set_due_date(fulldate)
+                if sub_duedate > new_duedate_obj:
+                    sub.set_due_date(new_duedate)
                 # if the child's start date happens later than
                 # the task's new due date, we update it
-                # (except for fuzzy dates)
-                if sub.get_start_date() != Date.no_date() and \
-                   not fulldate_obj.is_fuzzy() and \
-                   sub.get_start_date() > fulldate_obj:
-                    sub.set_start_date(fulldate)
-        else:
-            self.due_date = get_due_date_constraint()
-        self.sync()
+                # (except for fuzzy start dates)
+                sub_startdate = sub.get_start_date()
+                if sub_startdate != Date.no_date() and \
+                   not sub_startdate.is_fuzzy() and \
+                   sub_startdate > new_duedate_obj:
+                    sub.set_start_date(new_duedate)
+        # If the date changed, we notify the change for the children since the
+        # constraints might have changed
+        if old_due_date != new_duedate_obj:
+            self.recursive_sync()
 
     def get_due_date(self):
         """ Returns the due date, which always respects all constraints """
         return self.due_date
 
+    def get_due_date_constraint(self):
+        """ Returns the most urgent due date constraint, following
+            parents' due dates. Return Date.no_date() if no constraint
+            is applied. """
+        # Check out for constraints depending on date definition/fuzziness.
+        strongest_const_date = self.due_date
+        if strongest_const_date == Date.no_date() or \
+           ( strongest_const_date != Date.no_date() and \
+             strongest_const_date.is_fuzzy() ):
+            for par_id in self.parents:
+                par = self.req.get_task(par_id)
+                par_duedate = par.get_due_date()
+                # if parent date is undefined or fuzzy, look further up
+                if par_duedate == Date.no_date() or \
+                   par_duedate.is_fuzzy():
+                    par_duedate = par.get_due_date_constraint()
+                # if par_duedate is still undefined/fuzzy, all parents' due
+                # dates are undefined or fuzzy: strongest_const_date is then
+                # the best choice so far, we don't update it.
+                if par_duedate == Date.no_date() or \
+                   par_duedate.is_fuzzy():
+                    continue
+                # par_duedate is not undefined/fuzzy. If strongest_const_date is
+                # still undefined or fuzzy, parent_duedate is the best choice.
+                if strongest_const_date == Date.no_date() or \
+                   ( strongest_const_date != Date.no_date() and \
+                     strongest_const_date.is_fuzzy() ):
+                    strongest_const_date = par_duedate
+                    continue
+                # strongest_const_date and par_date are defined and not fuzzy:
+                # we compare the dates
+                if par_duedate < strongest_const_date:
+                    strongest_const_date = par_duedate
+        return strongest_const_date
+
+    # ABOUT START DATE
+    #
+    # Start date is the date at which the user has decided to work or consider
+    # working on this task.
+    # 
+    # The only constraint applied to start dates is that start dates cannot
+    # happen later than the task due date.
+    # 
+    # The task due date (and any constrained relatives) is updated if a new
+    # task start date is chosen that does not respect this rule.
+    #
+    # Undefined/fizzy start dates don't constraint the task due date.
+
     def set_start_date(self, fulldate):
         self.start_date = Date(fulldate)
         if Date(fulldate) != Date.no_date() and \
+           not Date(fulldate).is_fuzzy() and \
+           self.due_date != Date.no_date() and \
            not self.due_date.is_fuzzy() and \
            Date(fulldate) > self.due_date:
             self.set_due_date(fulldate)
@@ -316,6 +425,12 @@
     def get_start_date(self):
         return self.start_date
 
+    # ABOUT CLOSED DATE
+    #
+    # Closed date is the date at which the task has been closed (done or
+    # dismissed). Closed date is not constrained and doesn't constrain other
+    # dates.
+
     def set_closed_date(self, fulldate):
         self.closed_date = Date(fulldate)
         self.sync()
@@ -476,6 +591,20 @@
         """
         return self.req.get_task(tid)
 
+    def set_parent(self, parent_id):
+        """Update the task's parent. Refresh due date constraints."""
+        TreeNode.set_parent(self, parent_id)
+        if parent_id is not None:
+            par = self.req.get_task(parent_id)
+            par_duedate = par.get_due_date_constraint()
+            if par_duedate != Date.no_date() and \
+               not par_duedate.is_fuzzy()    and \
+               self.due_date != Date.no_date() and \
+               not self.due_date.is_fuzzy()  and \
+               par_duedate < self.due_date:
+                self.set_due_date(par_duedate)
+        self.recursive_sync()
+
     def set_attribute(self, att_name, att_value, namespace=""):
         """Set an arbitrary attribute.
 
 
=== modified file 'GTG/gtk/browser/treeview_factory.py'
--- GTG/gtk/browser/treeview_factory.py	2012-10-31 15:43:32 +0000
+++ GTG/gtk/browser/treeview_factory.py	2012-11-01 11:54:31 +0000
@@ -122,7 +122,12 @@
         return node.get_start_date().to_readable_string()
         
     def task_duedate_column(self,node):
-        return node.get_due_date().to_readable_string()
+        # We show the most constraining due date for task with no due dates.
+        if node.get_due_date() == Date.no_date():
+            return node.get_due_date_constraint().to_readable_string()
+        else:
+        # Other tasks show their due date (which *can* be fuzzy)
+            return node.get_due_date().to_readable_string()
         
     def task_cdate_column(self,node):
         return node.get_closed_date().to_readable_string()
@@ -154,6 +159,10 @@
             elif para == 'due':
                 t1 = task1.get_due_date()
                 t2 = task2.get_due_date()
+                if t1 == Date.no_date():
+                    t1 = task1.get_due_date_constraint()
+                if t2 == Date.no_date():
+                    t2 = task2.get_due_date_constraint()
             elif para == 'closed':
                 t1 = task1.get_closed_date()
                 t2 = task2.get_closed_date()