Edgewall Software
Modify

Opened 14 years ago

Closed 12 years ago

Last modified 12 years ago

#7800 closed defect (fixed)

Sorting by age broken in browser view

Reported by: chub@… Owned by: Remy Blank
Priority: high Milestone: 0.12.1
Component: version control/browser Version: 0.11-stable
Severity: normal Keywords: age date order
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description (last modified by Remy Blank)

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

Patch is inlined. =)

  • trac/versioncontrol/templates/browser.html

    old new  
    8080              ${sortable_th(dir.order, dir.desc, 'name', 'Name')}
    8181              ${sortable_th(dir.order, dir.desc, 'size', 'Size')}
    8282              <th class="rev">Rev</th>
    83               ${sortable_th(dir.order, dir.desc, 'date', 'Age')}
     83              ${sortable_th(dir.order, dir.desc, 'age', 'Age')}
    8484              <th class="change">Last Change</th>
    8585            </tr>
    8686          </thead>

Attachments (0)

Change History (10)

comment:1 by Remy Blank, 14 years ago

Description: modified (diff)
Keywords: needinfo added; browser versioncontrol removed
Milestone: 0.11.2

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)?

in reply to:  1 comment:2 by anonymous, 14 years ago

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!

Version 0, edited 14 years ago by anonymous (next)

comment:3 by Remy Blank, 14 years ago

Keywords: needinfo removed
Milestone: 0.11.3
Owner: set to 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.

comment:4 by Christian Boos, 14 years ago

Priority: normalhigh
Version: none0.11-stable

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

comment:5 by Remy Blank, 12 years ago

Something like the following?

  • trac/versioncontrol/web_ui/browser.py

    diff --git a/trac/versioncontrol/web_ui/browser.py b/trac/versioncontrol/web_ui/browser.py
    a b  
    546546               
    547547        entries = [entry(n) for n in node.get_entries()
    548548                   if n.can_view(req.perm)]
    549         changes = get_changes(repos, [i.rev for i in entries])
     549        changes = get_changes(repos, [i.rev for i in entries], self.log)
    550550
    551551        if rev:
    552552            newest = repos.get_changeset(rev).date
  • trac/versioncontrol/web_ui/log.py

    diff --git a/trac/versioncontrol/web_ui/log.py b/trac/versioncontrol/web_ui/log.py
    a b  
    230230            info[-1]['change'] = None
    231231       
    232232        revisions = [i['rev'] for i in info]
    233         changes = get_changes(repos, revisions)
     233        changes = get_changes(repos, revisions, self.log)
    234234        extra_changes = {}
    235235       
    236236        if format == 'changelog':
  • trac/versioncontrol/web_ui/util.py

    diff --git a/trac/versioncontrol/web_ui/util.py b/trac/versioncontrol/web_ui/util.py
    a b  
    1919from genshi.builder import tag
    2020
    2121from trac.resource import ResourceNotFound
    22 from trac.util.datefmt import pretty_timedelta
    23 from trac.util.text import shorten_line
     22from trac.util.datefmt import datetime, utc
    2423from trac.util.translation import tag_, _
    25 from trac.versioncontrol.api import NoSuchNode, NoSuchChangeset
     24from trac.versioncontrol.api import Changeset, NoSuchNode, NoSuchChangeset
    2625
    2726__all__ = ['get_changes', 'get_path_links', 'get_existing_node']
    2827
    29 def get_changes(repos, revs):
     28def get_changes(repos, revs, log=None):
    3029    changes = {}
    3130    for rev in revs:
    3231        if rev in changes:
     
    3433        try:
    3534            changeset = repos.get_changeset(rev)
    3635        except NoSuchChangeset:
    37             changeset = {}
     36            changeset = Changeset(repos, rev, '', '',
     37                                  datetime(1970, 1, 1, tzinfo=utc))
     38            if log is not None:
     39                log.warning("Unable to get changeset [%s]", rev)
    3840        changes[rev] = changeset
    3941    return changes
    4042

comment:6 by Christian Boos, 12 years ago

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

in reply to:  6 comment:7 by Remy Blank, 12 years ago

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?

comment:8 by Christian Boos, 12 years ago

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.

comment:9 by Remy Blank, 12 years ago

Milestone: next-minor-0.12.x0.12.1
Resolution: fixed
Status: newclosed

Patch applied in [10116].

comment:10 by chub, 12 years ago

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!

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.