| Message ID | 20180927182601.4676-1-stephen@that.guru |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/2] filters: Return dictionaries | expand |
Stephen Finucane <stephen@that.guru> writes: > This makes it a little easier to figure out what filters are active and > will be used in a future patch. > > Signed-off-by: Stephen Finucane <stephen@that.guru> > --- > patchwork/filters.py | 7 +++---- > patchwork/templates/patchwork/filters.html | 2 +- > patchwork/views/__init__.py | 2 +- > 3 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/patchwork/filters.py b/patchwork/filters.py > index 6cfe41c9..75eaedd6 100644 > --- a/patchwork/filters.py > +++ b/patchwork/filters.py > @@ -456,11 +456,10 @@ class Filters: > return queryset.filter(**kwargs) > > def params(self): > - return [(f.param, f.key()) for f in self._filters > - if f.key() is not None] > + return {f.param: f.key() for f in self._filters if f.key() is not None} > > def querystring(self, remove=None): > - params = dict(self.params()) > + params = self.params() > > for (k, v) in self.values.items(): > if k not in params: > @@ -481,7 +480,7 @@ class Filters: > return self.querystring(filter) > > def applied_filters(self): > - return [x for x in self._filters if x.applied] > + return {x.param: x for x in self._filters if x.applied} > > def available_filters(self): > return self._filters > diff --git a/patchwork/templates/patchwork/filters.html b/patchwork/templates/patchwork/filters.html > index 5331ac85..e760310b 100644 > --- a/patchwork/templates/patchwork/filters.html > +++ b/patchwork/templates/patchwork/filters.html > @@ -130,7 +130,7 @@ $(document).ready(function() { > <div id="filtersummary"> > <a href="javascript:filter_click()">Show patches with</a>: > {% if filters.applied_filters %} > - {% for filter in filters.applied_filters %} > + {% for filter in filters.applied_filters.values %} Are these going to have the same ordering? IIRC dictionaries don't guarantee any particular iteration ordering, whereas lists obviously do. IOW, will this ever lead to user-visible change? Regards, Daniel > {{ filter.name }} = <strong>{{ filter.condition }}</strong> > {% if not filter.forced %} > <a class="filter-action" > diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py > index 0c64c93e..cce00d67 100644 > --- a/patchwork/views/__init__.py > +++ b/patchwork/views/__init__.py > @@ -190,7 +190,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None, > > value = data.get(param, None) > if value: > - params.append((param, value)) > + params['param'] = value > > data = {} > if request.method == 'GET': > -- > 2.17.1 > > _______________________________________________ > Patchwork mailing list > Patchwork@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/patchwork
On Sat, 2018-09-29 at 02:11 +1000, Daniel Axtens wrote: > Stephen Finucane <stephen@that.guru> writes: > > > This makes it a little easier to figure out what filters are active and > > will be used in a future patch. > > > > Signed-off-by: Stephen Finucane <stephen@that.guru> > > --- > > patchwork/filters.py | 7 +++---- > > patchwork/templates/patchwork/filters.html | 2 +- > > patchwork/views/__init__.py | 2 +- > > 3 files changed, 5 insertions(+), 6 deletions(-) > > > > diff --git a/patchwork/filters.py b/patchwork/filters.py > > index 6cfe41c9..75eaedd6 100644 > > --- a/patchwork/filters.py > > +++ b/patchwork/filters.py > > @@ -456,11 +456,10 @@ class Filters: > > return queryset.filter(**kwargs) > > > > def params(self): > > - return [(f.param, f.key()) for f in self._filters > > - if f.key() is not None] > > + return {f.param: f.key() for f in self._filters if f.key() is not None} > > > > def querystring(self, remove=None): > > - params = dict(self.params()) > > + params = self.params() > > > > for (k, v) in self.values.items(): > > if k not in params: > > @@ -481,7 +480,7 @@ class Filters: > > return self.querystring(filter) > > > > def applied_filters(self): > > - return [x for x in self._filters if x.applied] > > + return {x.param: x for x in self._filters if x.applied} > > > > def available_filters(self): > > return self._filters > > diff --git a/patchwork/templates/patchwork/filters.html b/patchwork/templates/patchwork/filters.html > > index 5331ac85..e760310b 100644 > > --- a/patchwork/templates/patchwork/filters.html > > +++ b/patchwork/templates/patchwork/filters.html > > @@ -130,7 +130,7 @@ $(document).ready(function() { > > <div id="filtersummary"> > > <a href="javascript:filter_click()">Show patches with</a>: > > {% if filters.applied_filters %} > > - {% for filter in filters.applied_filters %} > > + {% for filter in filters.applied_filters.values %} > > Are these going to have the same ordering? IIRC dictionaries don't > guarantee any particular iteration ordering, whereas lists obviously do. > > IOW, will this ever lead to user-visible change? Yeah, there'll be a slight change if using Python < 3.5 (dictionaries are insertion ordered in 3.6+). I don't think this is an issue as we were already using dictionaries for the querystring generation above. However, I can use 'collections.OrderedDict' in both places instead, if you'd rather that? Stephen > Regards, > Daniel > > > {{ filter.name }} = <strong>{{ filter.condition }}</strong> > > {% if not filter.forced %} > > <a class="filter-action" > > diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py > > index 0c64c93e..cce00d67 100644 > > --- a/patchwork/views/__init__.py > > +++ b/patchwork/views/__init__.py > > @@ -190,7 +190,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None, > > > > value = data.get(param, None) > > if value: > > - params.append((param, value)) > > + params['param'] = value > > > > data = {} > > if request.method == 'GET': > > -- > > 2.17.1 > > > > _______________________________________________ > > Patchwork mailing list > > Patchwork@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork
diff --git a/patchwork/filters.py b/patchwork/filters.py index 6cfe41c9..75eaedd6 100644 --- a/patchwork/filters.py +++ b/patchwork/filters.py @@ -456,11 +456,10 @@ class Filters: return queryset.filter(**kwargs) def params(self): - return [(f.param, f.key()) for f in self._filters - if f.key() is not None] + return {f.param: f.key() for f in self._filters if f.key() is not None} def querystring(self, remove=None): - params = dict(self.params()) + params = self.params() for (k, v) in self.values.items(): if k not in params: @@ -481,7 +480,7 @@ class Filters: return self.querystring(filter) def applied_filters(self): - return [x for x in self._filters if x.applied] + return {x.param: x for x in self._filters if x.applied} def available_filters(self): return self._filters diff --git a/patchwork/templates/patchwork/filters.html b/patchwork/templates/patchwork/filters.html index 5331ac85..e760310b 100644 --- a/patchwork/templates/patchwork/filters.html +++ b/patchwork/templates/patchwork/filters.html @@ -130,7 +130,7 @@ $(document).ready(function() { <div id="filtersummary"> <a href="javascript:filter_click()">Show patches with</a>: {% if filters.applied_filters %} - {% for filter in filters.applied_filters %} + {% for filter in filters.applied_filters.values %} {{ filter.name }} = <strong>{{ filter.condition }}</strong> {% if not filter.forced %} <a class="filter-action" diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py index 0c64c93e..cce00d67 100644 --- a/patchwork/views/__init__.py +++ b/patchwork/views/__init__.py @@ -190,7 +190,7 @@ def generic_list(request, project, view, view_args=None, filter_settings=None, value = data.get(param, None) if value: - params.append((param, value)) + params['param'] = value data = {} if request.method == 'GET':
This makes it a little easier to figure out what filters are active and will be used in a future patch. Signed-off-by: Stephen Finucane <stephen@that.guru> --- patchwork/filters.py | 7 +++---- patchwork/templates/patchwork/filters.html | 2 +- patchwork/views/__init__.py | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-)