Modify

Opened 3 years ago

Last modified 3 years ago

#9300 assigned defect

Tickets with parent don't layout chronologically

Reported by: rjollos Owned by: ChrisNelson
Priority: normal Component: TracJsGanttPlugin
Severity: normal Keywords:
Cc: falkb Trac Release: 0.11

Description

As can be seen in the following image:


Attachments (2)

TicketsWithParentDontLayoutCorrectly.png (32.7 KB) - added by rjollos 3 years ago.
ChildTasksLayingOutCorrectly.png (69.9 KB) - added by rjollos 3 years ago.

Download all attachments as: .zip

Change History (22)

Changed 3 years ago by rjollos

comment:1 Changed 3 years ago by falkb

  • Cc falkb added

comment:2 Changed 3 years ago by rjollos

I'm seeing that the tasks are sorted chronologically, then assigned a wbs (i.e. parent/child relationships) and then sorted again. The second sort won't change the relative ordering of the parents, but the children are re-ordered such that they are no longer chronological. For example, here is the list of wbs values for a particular instance:

[1]
[4,1]
[2]
[4,4]
[3]
[4]
[4,3]
[4,2]
[5]
[6]
[7]

The issue seems to be that the children aren't sorted chronologically before assigning a wbs value. One solution might be to sort the list of children chronologically so that the wbs values get assigned chronologically.

comment:3 Changed 3 years ago by rjollos

I have a working fix for this, but I'm hoping you can come up with something cleaner. If you ignore the first part where I just copied and pasted your _compare tickets function, the change is to sort the list of child ticket ids chronologically.

Index: 0.11/tracjsgantt/tracjsgantt.py
===================================================================
--- 0.11/tracjsgantt/tracjsgantt.py (revision 10838)
+++ 0.11/tracjsgantt/tracjsgantt.py (working copy)
@@ -446,6 +446,28 @@
     # WBS is a list like [ 2, 4, 1] (the first child of the fourth
     # child of the second top-level element).
     def _compute_wbs(self):
+        def _compare_tickets(t1, t2):
+            # If t2 depends on t1, t2 is first
+            if self.fields['succ'] and \
+                    str(t1['id']) in t2[self.fields['succ']]:
+                return 1
+            # If t1 depends on t2, t1 is first
+            elif self.fields['succ'] and \
+                    str(t2['id']) in t1[self.fields['succ']]:
+                return -1            
+            # If t1 ends first, it's first
+            elif t1['calc_finish'] < t2['calc_finish']:
+                return -1
+            # If t2 ends first, it's first
+            elif t1['calc_finish'] > t2['calc_finish']:
+                return 1
+            # End dates are same. If t1 starts later, it's later
+            elif t1['calc_start'] > t2['calc_start']:
+                return 1
+            # Otherwise, preserve order (assume t1 is before t2 when called)
+            else:
+                return 0        
+        
         # Set the ticket's level and wbs then recurse to children.
         def _setLevel(id, wbs, level):
             # Update this node
@@ -453,10 +475,15 @@
             self.ticketsByID[id]['wbs'] = copy.copy(wbs)
 
             # Recurse to children
-            if self.ticketsByID[id]['children']:
+            childIDs = self.ticketsByID[id]['children']
+            if childIDs:
+                childTickets = [self.ticketsByID[id] for id in childIDs]
+                childTickets.sort(_compare_tickets)
+                childIDs = [ct['id'] for ct in childTickets]
+                
                 # Add another level
                 wbs.append(1)
-                for c in self.ticketsByID[id]['children']:
+                for c in childIDs:
                     wbs = _setLevel(c, wbs, level+1)
                 # Remove the level we added
                 wbs.pop()

Changed 3 years ago by rjollos

comment:5 Changed 3 years ago by rjollos


comment:6 Changed 3 years ago by ChrisNelson

(In [10905]) Better sorting of children under parents. Refs #9300.

Based on a patch from rjollos. Thanks.

comment:7 Changed 3 years ago by ChrisNelson

  • Status changed from new to assigned

comment:8 Changed 3 years ago by rjollos

I've pulled in your patch and it seems to be working well.

comment:9 Changed 3 years ago by ChrisNelson

(In [11079]) Cleaner date adjustment using calendarOffset. Refs #9533, #9300, #9042, #9515.

Scheduler used to assume work week. Now let calendarOffset() do that.

Also, it seems rounding errors would creep in and some datetimes would
have non-zero microseconds. This tests for beginning and end of day
with a little delta.

comment:10 Changed 3 years ago by ChrisNelson

(In [11080]) More flexible calendar offset calculation. Refs #9533, #9300.

comment:11 Changed 3 years ago by ChrisNelson

(In [11081]) Copy some dates before returning them. Refs #9042, #9515, #9533, #9300.

I'm not 100% sure why this is necessary but the wrong things seemed to
get updated if I didn't do this and give demonstrably wrong results.

comment:12 Changed 3 years ago by ChrisNelson

(In [11082]) Add calendar handling to scheduler. Refs #9533, #9300.

comment:13 Changed 3 years ago by ChrisNelson

(In [11083]) Rework calendar offset adjustment to not go outside working hours. Refs #9648, #9533, #9300.

comment:14 Changed 3 years ago by ChrisNelson

(In [11142]) Correct end-of-day adjustments for ASAP and < 100% effort. Refs #9533, #9300.

When working forward to build an ASAP schedule, partial takes caused the
tasks to shift forward more than their scheduled length.

If one resource can work 8 hours per day and another can only work 4,
the second resource takes twice as long to complete the same task. But
without this clean up, ALAP scheduling took an 8-hour task at 4 hours
per day from the end of day 2 to the middle of day 1. With this change,
if all available hours are used up, the rest of the day is consumed so
that 8-hour task goes from the end of day 2 to the start of day 1.

comment:15 Changed 3 years ago by ChrisNelson

How's this working for you now?

comment:16 Changed 3 years ago by ChrisNelson

(In [11419]) Add resource leveling to scheduler. Refs #9833, #9042, #9290, #9300, #9691, #9784.

This is rather naive. It makes sure a resource isn't doing two things
at once but doesn't yet handle priorities when conflicts occur.

comment:17 Changed 3 years ago by ChrisNelson

(In [11422]) Add task sorter interface. Refs #9833, #9042, #9290, #9300, #9691, #9784.

comment:18 Changed 3 years ago by ChrisNelson

(In [11423]) Add simple task sorter. Refs #9833, #9042, #9290, #9300, #9691, #9784.

This sorts by Trac's built-in priority field (after converting the
priority strings to integers).

comment:19 Changed 3 years ago by ChrisNelson

(In [11424]) Implement prioritized scheduling. Refs #9833, #9042, #9290, #9300, #9691, #9784.

This is orthogonal to resource leveling. That is even if resource
leveling is disabled, this schedules the tasks for a resource using the
configured sorting rule.

comment:20 Changed 3 years ago by ChrisNelson

(In [11426]) A tree-aware task sorter. Refs #9833, #9042, #9290, #9300, #9691, #9784.

A task's priority depends on its parent's priority.

Includes a base sorter class to hold common sorter methods.

SimpleSorter and ProjectSorter derive from the base class.

(Copy effective priority list, just in case.)

Also remove unneeded debugging output. Refs #9648.

Add Comment

Modify Ticket

Action
as assigned .
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.