From 4a73b76bc23735f7dcbc2d31c8882a9ce15569a8 Mon Sep 17 00:00:00 2001
From: James Taylor <james@jamestaylor.org>
Date: Mon, 19 Apr 2010 17:43:39 -0400
Subject: [PATCH] Fix ordering in display_structured, also use insane
 eagerloading to make it massively faster

---
 lib/galaxy/util/odict.py                  | 10 ++++++---
 lib/galaxy/web/controllers/history.py     | 25 +++++++++++++++--------
 templates/history/display_structured.mako | 13 +++++++++---
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/lib/galaxy/util/odict.py b/lib/galaxy/util/odict.py
index 2f799823c64..94c29624aa2 100644
--- a/lib/galaxy/util/odict.py
+++ b/lib/galaxy/util/odict.py
@@ -31,9 +31,9 @@ class odict(UserDict):
         self._keys = []
 
     def copy(self):
-	    new = odict()
-	    new.update( self )
-	    return new
+        new = odict()
+        new.update( self )
+        return new
 
     def items(self):
         return zip(self._keys, self.values())
@@ -82,3 +82,7 @@ class odict(UserDict):
     def __iter__( self ):
         for key in self._keys: 
             yield key
+    
+    def reverse( self ):
+        self._keys.reverse()
+
diff --git a/lib/galaxy/web/controllers/history.py b/lib/galaxy/web/controllers/history.py
index 26479903816..a9548f3c8a1 100644
--- a/lib/galaxy/web/controllers/history.py
+++ b/lib/galaxy/web/controllers/history.py
@@ -1,6 +1,7 @@
 from galaxy.web.base.controller import *
 from galaxy.web.framework.helpers import time_ago, iff, grids
 from galaxy import util
+from galaxy.util.odict import odict
 from galaxy.model.mapping import desc
 from galaxy.model.orm import *
 from galaxy.util.json import *
@@ -336,25 +337,31 @@ class HistoryController( BaseController, Sharable, UsesAnnotations, UsesHistory
         """
         # Get history
         if id is None:
-            history = trans.history
+            id = trans.history.id
         else:
             id = trans.security.decode_id( id )
-            history = trans.sa_session.query( model.History ).get( id )
-            assert history
-            assert history.user and ( history.user == trans.user ) or ( history == trans.history )
+        # Expunge history from the session to allow us to force a reload
+        # with a bunch of eager loaded joins
+        trans.sa_session.expunge( trans.history )
+        history = trans.sa_session.query( model.History ).options(
+                eagerload_all( 'active_datasets.creating_job_associations.job.workflow_invocation_step.workflow_invocation.workflow' ),
+                eagerload_all( 'active_datasets.children' )
+            ).get( id )
+        assert history
+        assert history.user and ( history.user.id == trans.user.id ) or ( history.id == trans.history.id )
         # Resolve jobs and workflow invocations for the datasets in the history
         # items is filled with items (hdas, jobs, or workflows) that go at the
         # top level
         items = []
         # First go through and group hdas by job, if there is no job they get
         # added directly to items
-        jobs = dict()
+        jobs = odict()
         for hda in history.active_datasets:
             # Follow "copied from ..." association until we get to the original
             # instance of the dataset
             original_hda = hda
-            while original_hda.copied_from_history_dataset_association:
-                original_hda = original_hda.copied_from_history_dataset_association
+            ## while original_hda.copied_from_history_dataset_association:
+            ##     original_hda = original_hda.copied_from_history_dataset_association
             # Check if the job has a creating job, most should, datasets from
             # before jobs were tracked, or from the upload tool before it
             # created a job, may not
@@ -370,7 +377,7 @@ class HistoryController( BaseController, Sharable, UsesAnnotations, UsesHistory
                 else:
                     jobs[ job ] = [ ( hda, None ) ]
         # Second, go through the jobs and connect to workflows
-        wf_invocations = dict()
+        wf_invocations = odict()
         for job, hdas in jobs.iteritems():
             # Job is attached to a workflow step, follow it to the
             # workflow_invocation and group
@@ -1025,4 +1032,4 @@ class HistoryController( BaseController, Sharable, UsesAnnotations, UsesHistory
             msg = 'Clone with name "%s" is now included in your previously stored histories.' % new_history.name
         else:
             msg = '%d cloned histories are now included in your previously stored histories.' % len( histories )
-        return trans.show_ok_message( msg )
\ No newline at end of file
+        return trans.show_ok_message( msg )
diff --git a/templates/history/display_structured.mako b/templates/history/display_structured.mako
index be02b9b9121..70910709a9d 100644
--- a/templates/history/display_structured.mako
+++ b/templates/history/display_structured.mako
@@ -16,6 +16,7 @@
         
         .workflow {
             border: solid gray 1px;
+            margin: 5px 0;
             border-left-width: 5px;
         }
         
@@ -96,9 +97,15 @@ elif entity_name == "WorkflowInvocation":
 <%def name="render_item_job( job, children  )">
 
     <div class="tool toolForm">
-        <div class="header toolFormTitle">Tool: ${trans.app.toolbox.tools_by_id[job.tool_id].name}</div>
+        <%
+            if job.tool_id in trans.app.toolbox.tools_by_id:
+                tool_name = trans.app.toolbox.tools_by_id[job.tool_id].name
+            else:
+                tool_name = "Unknown tool with id '%s'" % job.tool_id       
+        %>
+        <div class="header toolFormTitle">Tool: ${tool_name}</div>
         <div class="body toolFormBody">
-        %for e, c in children:
+        %for e, c in reversed( children ):
             ${render_item( e, c )}
         %endfor
         </div>
@@ -111,7 +118,7 @@ elif entity_name == "WorkflowInvocation":
     <div class="workflow">
         <div class="header">Workflow: ${wf.workflow.name}</div>
         <div class="body">
-        %for e, c in children:
+        %for e, c in reversed( children ):
             ${render_item( e, c )}
         %endfor
         </div>