On Wed, 2018-06-13 at 22:54 +0100, Stephen Finucane wrote: > The pagination functionality used in the 'patchwork.view.generic_list' > generates the filter querystring from scratch. To do this, it calls the > 'patchwork.filters.Filters.params' function, which in turn calls the > 'patchwork.filters.Filter.key' function for each filter. If any of these > 'key' functions return None, the relevant filter is not included in the > querystring. This ensures we don't end up with a load of filters like > the below: > > ?submitter=&state=&series=&q=&delegate=&archive=both > > which would be functionally equivalent to: > > ?archive=both > > There is one exception to this rule, however: ArchiveFilter. This is a > little unusual in that it is active by default, excluding patches that > are "archived" from the list. As a result, the 'key' function should > return None for this active state, not for the disabled state. This has > been the case up until commit d848f046 which falsely equated 'is False' > with 'is None'. This small typo resulted in the filter being ignored > when generating pagination links and essentially broke pagination for > some use cases. > > Fix this up, adding a test to prevent regressions. We could probably > simplify this thing greatly by not recalculating filters for pagination > at least or, better yet, by using django-filter here too. That is a > change for another day though. > > Signed-off-by: Stephen Finucane <step...@that.guru> > Reported-by: John McNamara <john.mcnam...@intel.com> > Fixes: d848f046 ("trivial: Don't shadow built-ins") > Closes: #190
I've gone ahead and merged this as a trivial fix. I also backported this and it is one of the three important fixes included in v2.0.3. Stephen _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork