Warning: Can't synchronize with repository "(default)" (Unsupported version control system "svn": No module named svn). Look in the Trac log for more information.

Ticket #1605 (closed defect: fixed)

Opened 9 years ago

Last modified 9 years ago

[PATCH] Paginate.get_href method shouldn't change instance inner state

Reported by: droc Owned by: roger.demetrescu
Priority: normal Milestone:
Component: TurboGears Version: 1.0.4b2
Severity: normal Keywords:
Cc:

Description

There is a bug in Paginate's get_href method:

file paginate.py:

class Paginate:
...
    def get_href(self, page, order=None, reverse_order=None):
        # Note that reverse_order is not used.  It should be cleaned up here
        # and in the template.  I'm not removing it now because I don't want
        # to break the API.
        order = order or None
        self.input_values[self.var_name + '_tgp_no'] = page
        if order:
            self.input_values[self.var_name + '_tgp_order'] = order

        return turbogears.url('', self.input_values)

Now, this code is used in the default template first to output the pagination links (previous, next, and a range of pages), and then is used to output the table headers, including the sortable ones. When getting the href for a sortable column, the inner state of the objects is changed (self.input_values[self.var_name + '_tgp_order'] is redefined because order is not None).

I think this is unnecessary/incorrect and it fires a bug if you don't use the default template and do something as simple as moving the pagination links below the table (the order is now that set when the header for the last column of the table was generated, as it was the last change made to self.input_values).

Attached are a Demo project and my patch solving this issue. It exposes two methods in its root controller: demo and demo_template. Both render a paginated data grid with enough data to require two pages so the pagination links are shown. The second method, demo_template, changes the template for the data grid, just by moving the pagination links from above to below the table. If you compare the link generated for the two methods you'll see they differ, with the pagination links in the demo_template call defining a new ordering for the table.

Expected behavior

The pagination links should be the same whether they are generated before or after the table (or: get_href should return always the same result for the same set of parameters)

Actual behavior

The pagination links change if they are generated below the table (or: get_href returns a different value if a previous call has been made with a the second parameter -order- not None)

Attachments

paginate.diff Download (787 bytes) - added by droc 9 years ago.
A patch that fixes the issue
Demo.tar.gz Download (90.2 KB) - added by droc 9 years ago.
A Demo project showing the use case, the problem and to verify the fix

Change History

Changed 9 years ago by droc

A patch that fixes the issue

Changed 9 years ago by droc

A Demo project showing the use case, the problem and to verify the fix

comment:1 Changed 9 years ago by roger.demetrescu

  • Owner changed from anonymous to roger.demetrescu

comment:2 Changed 9 years ago by roger.demetrescu

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in [3718] and [3719]. Thanks !!

Note: See TracTickets for help on using tickets.