[PATCH] Don't discard values from 'archive' filter

Stephen Finucane stephen at that.guru
Fri Jun 15 00:33:32 AEST 2018


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 <stephen at that.guru>
> Reported-by: John McNamara <john.mcnamara at 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


More information about the Patchwork mailing list