Automatics period for report

[J-Jay]

Hi,

I opened a ticket regarding this user list thread https://groups.google.com/forum/#!topic/taskjuggler-users/4RSLropuNGY because I think there is a bug.

In current implementation, filterTaskList is performed after adjustReportPeriod, so TaskList contains all tasks, and calculated period is wrong. I made the following changes to perform filterTaskList before adjustReportPeriod. That works like a charme but I am not happy with this implementation :

• filterTaskList is performed twice -> poor performances
• assignedTaskList contains duplicated tasks -> poor performances on adjustReportPeriod loop.

Please have a look on my patch

diff --git a/lib/taskjuggler/reports/ResourceListRE.rb b/lib/taskjuggler/reports/ResourceListRE.rb
index 05d13a8..b6c9e32 100644
--- a/lib/taskjuggler/reports/ResourceListRE.rb
+++ b/lib/taskjuggler/reports/ResourceListRE.rb
@@ -52,7 +52,14 @@ class TaskJuggler
       taskList.query = @report.project.reportContexts.last.query
       taskList.sort!

-      adjustReportPeriod(taskList, @report.get('scenarios'),
+      assignedTaskList = []
+      resourceList.each do |resource|
+        assignedTaskList.concat(filterTaskList(taskList, resource,
+                                          @report.get('hideTask'), @report.get('rollupTask'),
+                                          @report.get('openNodes')))
+      end
+
+      adjustReportPeriod(assignedTaskList, @report.get('scenarios'),
                          @report.get('columns'))

       # Generate the table header.

[J-Jay]

I think there is the same issue in TaskListRE.rb

[scrapper]

I don't think there is a way around the 2 passes if you want this functionality. But it can be made more efficient. I'll massage it a bit more. TaskListRE does not have the problem as adjustReportPeriod is operating on the filtered list already.

[J-Jay]

For information, I modified my patch to keep only 1 instance of each task in assignedTaskList. I am newbie in ruby so you certainly will have a better implementation.

diff --git a/lib/taskjuggler/reports/ResourceListRE.rb b/lib/taskjuggler/reports/ResourceListRE.rb
index 05d13a8..fd57ea8 100644
--- a/lib/taskjuggler/reports/ResourceListRE.rb
+++ b/lib/taskjuggler/reports/ResourceListRE.rb
@@ -52,7 +52,18 @@ class TaskJuggler
       taskList.query = @report.project.reportContexts.last.query
       taskList.sort!

-      adjustReportPeriod(taskList, @report.get('scenarios'),
+      assignedTaskList = []
+      resourceList.each do |resource|
+        
+        assignedTaskList = assignedTaskList  + filterTaskList(taskList, resource,
+                                          @report.get('hideTask'), @report.get('rollupTask'),
+                                          @report.get('openNodes'))
+                                          
+      end
+      
+      assignedTaskList = assignedTaskList & taskList
+
+      adjustReportPeriod(assignedTaskList, @report.get('scenarios'),
                          @report.get('columns'))

       # Generate the table header.

[scrapper]

Instead of the Array.& an Array.uniq! right inside the loop is probably faster. Other than that, the code looks good. I would remove the blank lines inside the loop and match the indentation to the coding style used in the rest of the code. Can you send a pull request then?

[J-Jay]

Is it my role to close this issue ?