Sorting by age broken in browser view

[chub@…]

The key to sort the browser view is "age" not "date".

Patch is inlined. =)

trac/versioncontrol/templates/browser.html

@@ -80 +80 @@
               ${sortable_th(dir.order, dir.desc, 'name', 'Name')}
               ${sortable_th(dir.order, dir.desc, 'size', 'Size')}
               <th class="rev">Rev</th>
-              ${sortable_th(dir.order, dir.desc, 'date', 'Age')}
+              ${sortable_th(dir.order, dir.desc, 'age', 'Age')}
               <th class="change">Last Change</th>
             </tr>
           </thead>

[Remy Blank]

Sorry, no more tickets for 0.11.2.

I quickly tested on trunk, and the sorting by age seems to work ok. But if I apply your patch, the sorting is messed up.

Could you describe more precisely what problem you observe and how to reproduce it? Also, what Trac version are you using (set the Version field)?

[anonymous]

Version 0.11.1 installed, I believe, using a CentOS package.

The problem happens when we try viewing a branch that we merge to sorted by age, though there are a dozen other branches that we can load using the same trac (and of course in the svn repository) that don't exhibit problems when we sort by age.

Here's the trace (and the patch probably didn't alleviate the problem, I apologize):

AttributeError: 'dict' object has no attribute 'date' File "/usr/lib/python2.4/site-packages/trac/web/main.py", line 423, in _dispatch_request

dispatcher.dispatch(req)

File "/usr/lib/python2.4/site-packages/trac/web/main.py", line 197, in dispatch

resp = chosen_handler.process_request(req)

File "/usr/lib/python2.4/site-packages/trac/versioncontrol/web_ui/browser.py", line 359, in process_request

node.get_properties()),

File "/usr/lib/python2.4/site-packages/trac/versioncontrol/web_ui/browser.py", line 447, in _render_dir

entries = sorted(entries, key=browse_order, reverse=desc)

File "/usr/lib/python2.4/site-packages/trac/versioncontrol/web_ui/browser.py", line 446, in browse_order

return a.isdir and dir_order or 0, file_order(a)

File "/usr/lib/python2.4/site-packages/trac/versioncontrol/web_ui/browser.py", line 434, in file_order

return changes[a.rev].date

Trac: 0.11.1 Python: 2.4.3 (#1, May 24 2008, 13:57:05) [GCC 4.1.2 20070626 (Red Hat 4.1.2-14)] setuptools: 0.6c5 SQLite: 3.3.6 pysqlite: 1.1.7 Genshi: 0.5.1 mod_python: 3.2.8 Subversion: 1.5.3 (r33570) jQuery: 1.2.6

Thanks!

[Remy Blank]

Ok, that helps a lot more. I suspect the line:

return changes[a.rev].date

should actually be:

return changes[a.rev]["date"]

I'll put this on my to-do.

[Christian Boos]

It looks like a duplicate of #6143.

I think it's the changeset = repos.get_changeset(rev)​ line which fails for some reason, and then the fallback value ({}) leads to the problem shown above (and to the one seen in #6143). We should:

• log the error
• provide a safer fallback

[Remy Blank]

Something like the following?

trac/versioncontrol/web_ui/browser.py

@@ -546 +546 @@
                 
         entries = [entry(n) for n in node.get_entries()
                    if n.can_view(req.perm)]
-        changes = get_changes(repos, [i.rev for i in entries])
+        changes = get_changes(repos, [i.rev for i in entries], self.log)
 
         if rev:
             newest = repos.get_changeset(rev).date

trac/versioncontrol/web_ui/log.py

@@ -230 +230 @@
             info[-1]['change'] = None
         
         revisions = [i['rev'] for i in info]
-        changes = get_changes(repos, revisions)
+        changes = get_changes(repos, revisions, self.log)
         extra_changes = {}
         
         if format == 'changelog':

trac/versioncontrol/web_ui/util.py

@@ -19 +19 @@
 from genshi.builder import tag
 
 from trac.resource import ResourceNotFound 
-from trac.util.datefmt import pretty_timedelta
-from trac.util.text import shorten_line
+from trac.util.datefmt import datetime, utc
 from trac.util.translation import tag_, _
-from trac.versioncontrol.api import NoSuchNode, NoSuchChangeset
+from trac.versioncontrol.api import Changeset, NoSuchNode, NoSuchChangeset
 
 __all__ = ['get_changes', 'get_path_links', 'get_existing_node']
 
-def get_changes(repos, revs):
+def get_changes(repos, revs, log=None):
     changes = {}
     for rev in revs:
         if rev in changes:
@@ -34 +33 @@
         try:
             changeset = repos.get_changeset(rev)
         except NoSuchChangeset:
-            changeset = {}
+            changeset = Changeset(repos, rev, '', '',
+                                  datetime(1970, 1, 1, tzinfo=utc))
+            if log is not None:
+                log.warning("Unable to get changeset [%s]", rev)
         changes[rev] = changeset
     return changes
 

[Christian Boos]

We could do the logging only at the web_ui level, that would be enough I think.

[Remy Blank]

Replying to cboos:

We could do the logging only at the web_ui level, that would be enough I think.

You mean, outside of get_changes() (it's only ever used in the web_ui.* modules). But that would require iterating over the result list and find the "dummy" changesets. Or did I misunderstand?

[Christian Boos]

No, I misunderstood, thought get_changes was at the trac.versioncontrol.api level, I only had a few moments to look at the diffs :-) It's fine then.

[Remy Blank]

Patch applied in [10116].

[chub]

Unfortunately, the particular Trac instance that was exhibiting this behavior has been retired, so I'm unable to confirm the fix. However, thanks for tracking and fixing this!