From patchwork Wed Jun 13 21:54:04 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 929107 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 415gWm2rwtz9s1B for ; Thu, 14 Jun 2018 07:54:28 +1000 (AEST) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="Cw0t2bw9"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 415gWm1DdbzDrvG for ; Thu, 14 Jun 2018 07:54:28 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="Cw0t2bw9"; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (mailfrom) smtp.mailfrom=that.guru (client-ip=23.83.214.30; helo=caracal.maple.relay.mailchannels.net; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="Cw0t2bw9"; dkim-atps=neutral Received: from caracal.maple.relay.mailchannels.net (caracal.maple.relay.mailchannels.net [23.83.214.30]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 415gWh3sFJzDrcd for ; Thu, 14 Jun 2018 07:54:22 +1000 (AEST) X-Sender-Id: 5xi41l16bi|x-authuser|stephen@that.guru Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id B545F5C2DC6 for ; Wed, 13 Jun 2018 21:54:18 +0000 (UTC) Received: from one.mxroute.com (unknown [100.96.16.125]) (Authenticated sender: 5xi41l16bi) by relay.mailchannels.net (Postfix) with ESMTPA id 469CF5C1D4B for ; Wed, 13 Jun 2018 21:54:18 +0000 (UTC) X-Sender-Id: 5xi41l16bi|x-authuser|stephen@that.guru Received: from one.mxroute.com (one-outgoing.mxroute.com [172.18.36.240]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384) by 0.0.0.0:2500 (trex/5.15.2); Wed, 13 Jun 2018 21:54:18 +0000 X-MC-Relay: Neutral X-MailChannels-SenderId: 5xi41l16bi|x-authuser|stephen@that.guru X-MailChannels-Auth-Id: 5xi41l16bi X-Lettuce-Tangy: 7278b56252eb81cb_1528926858557_1679842382 X-MC-Loop-Signature: 1528926858557:2814157693 X-MC-Ingress-Time: 1528926858556 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:MIME-Version :Content-Type:Content-Transfer-Encoding:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=/m0b5BayEraWzmTJtoZdwWubdzHgaW2uyv471nT7/r8=; b=Cw0t2bw9iDbobBA+5tvghf4i/J K7dNLyBhbeKNnIYNsjnZvLLjj+zI6bElpBLLyrr3+S6dipvEw/de3Hnv27aTIvcaukH1szqOyf3Tx h8uqYtKDsn4eBecp/rENp839bTai5WikdAFShnUR2sXEK/PbFljsk95oSgpj9OGo4HVG8ZCHJbzTY nMI2rba3xAUroHGOhf15h3EVKplXJuqAPSKOgYovwjjixE0X1fa8iIoghXQEU3WHLSzuusdlxtahL SINbl3JIezwjt4/rksKRQNdKDVCR557QvoC67itZUAU4cybW6O1s7R0II7i4SzOEyogvHDGOCxDdw vwz4kHgg==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH] Don't discard values from 'archive' filter Date: Wed, 13 Jun 2018 22:54:04 +0100 Message-Id: <20180613215405.12113-1-stephen@that.guru> X-Mailer: git-send-email 2.17.1 X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" 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 Reported-by: John McNamara Fixes: d848f046 ("trivial: Don't shadow built-ins") Closes: #190 --- patchwork/filters.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/patchwork/filters.py b/patchwork/filters.py index bc8ca41a..8d0f82f2 100644 --- a/patchwork/filters.py +++ b/patchwork/filters.py @@ -340,7 +340,9 @@ class ArchiveFilter(Filter): return self.description_map[self.archive_state] def key(self): - if not self.archive_state: + # NOTE(stephenfin): this is a shortcut to ensure we don't both + # including the 'archive' querystring filter for the default case + if self.archive_state is False: return None return self.param_map[self.archive_state]