harvest-dev team mailing list archive
-
harvest-dev team
-
Mailing list archive
-
Message #00537
[Merge] lp:~dylanmccall/harvest/filter-applied into lp:harvest
Dylan McCall has proposed merging lp:~dylanmccall/harvest/filter-applied into lp:harvest.
Requested reviews:
harvest-dev (harvest-dev)
Related bugs:
#665345 Add filter to show / hide Irrelevant and Applied opportunities
https://bugs.launchpad.net/bugs/665345
#722934 Invalid opportunities appear as "hidden"
https://bugs.launchpad.net/bugs/722934
For more details, see:
https://code.launchpad.net/~dylanmccall/harvest/filter-applied/+merge/50690
Yay, I'm back!
This is implementing the feature described in bug #665345.
Filters only do anything when they are enabled, so I implemented the applied / irrelevant filters as a single ChoiceFilter, enabled by default and with two choices: applied and irrelevant. Normally the filter makes sure applied and irrelevant opportunities are excluded from the results. If one choice is selected, opportunities with that status are shown.
Implementing that required another addition: translatable labels for ChoiceFilters. (Previously we didn't have anything that could be translated there, so I missed that detail). To implement that neatly, I had to remove that funny way of overriding the default_choices_dict method in each ChoiceFilter subclass, instead calling the class initialiser and passing the dictionary through that.
While I was working on this (and contemplating a “Valid” filter), I also happened to fix a very visible issue with valid opportunities being exposed. I reported that as bug #722934 in case somebody goes looking for it.
This should be consistent with how the other filters behave and it feels sane to me, but keep in mind I haven't tested it on anyone so far (and this dastardly cold has some influence on my judgement).
--
https://code.launchpad.net/~dylanmccall/harvest/filter-applied/+merge/50690
Your team harvest-dev is requested to review the proposed merge of lp:~dylanmccall/harvest/filter-applied into lp:harvest.
=== modified file 'harvest/filters/filters.py'
--- harvest/filters/filters.py 2010-12-08 08:15:50 +0000
+++ harvest/filters/filters.py 2011-02-22 03:46:56 +0000
@@ -289,18 +289,10 @@
html_class = 'filter setfilter choicefilter'
- def __init__(self, id_str, choices_dict = None, **kwargs):
+ def __init__(self, id_str, choices_dict = None, choices_labels_dict = None, **kwargs):
SetFilter.__init__(self, id_str, **kwargs)
- if choices_dict:
- self.choices_dict = choices_dict
- else:
- self.choices_dict = self.default_choices_dict()
-
- def default_choices_dict(self):
- """
- @return the default dictionary of choices if none is given to __init__
- """
- return None
+ self.choices_dict = choices_dict
+ self.choices_labels_dict = choices_labels_dict
def id_allowed(self, item_id): #overrides SetFilter
return item_id in self.choices_dict
@@ -326,8 +318,12 @@
def render_html_value_choice(self, item_id):
toggle_params = self.serialize(self.get_value_with_selection(item_id))
item_href = self.get_system().get_url_with_parameters(toggle_params)
-
- return '<a class="item-toggle" href="%s">%s</a>' % (item_href, item_id)
+ item_label = item_id
+
+ if self.choices_labels_dict and item_id in self.choices_labels_dict:
+ item_label = self.choices_labels_dict[item_id]
+
+ return '<a class="item-toggle" href="%s">%s</a>' % (item_href, item_label)
class FilterGroup(FilterContainer, ChoiceFilter): #final
=== modified file 'harvest/opportunities/filters.py'
--- harvest/opportunities/filters.py 2010-12-08 08:15:50 +0000
+++ harvest/opportunities/filters.py 2011-02-22 03:46:56 +0000
@@ -9,11 +9,16 @@
return queryset.filter(name__startswith = self.get_value())
class PkgSetFilter(filters.ChoiceFilter):
- def default_choices_dict(self):
- choices_dict = OrderedDict()
+ def __init__(self, id_str, **kwargs):
+ choices = OrderedDict()
for s in models.PackageSet.objects.all(): #TODO: find a way to sort these
- choices_dict[s.name] = s
- return choices_dict
+ choices[s.name] = s
+
+ filters.ChoiceFilter.__init__(
+ self,
+ id_str,
+ choices_dict = choices,
+ **kwargs)
def process_queryset(self, queryset):
return queryset.filter(packagesets__in = self.get_selected_choices())
@@ -25,12 +30,17 @@
return queryset.filter(opportunitylist__featured = True)
class OppListFilter(filters.ChoiceFilter):
- def default_choices_dict(self):
- choices_dict = OrderedDict()
+ def __init__(self, id_str, **kwargs):
+ choices = OrderedDict()
for l in models.OpportunityList.objects.all(): #TODO: find a way to sort these
if l.active:
- choices_dict[l.name] = l
- return choices_dict
+ choices[l.name] = l
+
+ filters.ChoiceFilter.__init__(
+ self,
+ id_str,
+ choices_dict = choices,
+ **kwargs)
def process_queryset(self, queryset):
return queryset.filter(opportunitylist__in = self.get_selected_choices())
@@ -49,24 +59,46 @@
self.choices_dict[item_id].explanation
return '<a class="item-toggle" href="%s" %s>%s</a> %s' % (item_href, title_attribute, item_id, help_html)
+class OppStatusFilter(filters.ChoiceFilter):
+ def __init__(self, id_str, **kwargs):
+ choices = OrderedDict.fromkeys(['applied', 'irrelevant'])
+ labels = {'applied' : _("Applied"),
+ 'irrelevant' : _("Irrelevant")}
+
+ filters.ChoiceFilter.__init__(
+ self,
+ id_str,
+ choices_dict = choices,
+ choices_labels_dict = labels,
+ **kwargs)
+
+ def process_queryset(self, queryset):
+ selection = self.get_value()
+ if not 'applied' in selection:
+ queryset = queryset.exclude(applied = True)
+ if not 'irrelevant' in selection:
+ queryset = queryset.exclude(reviewed = True)
+ return queryset
#we don't really need to create a special type here, but it may be handy
class HarvestFilters(containers.FilterSystem):
def __init__(self):
- super(HarvestFilters, self).__init__(
+ containers.FilterSystem.__init__(
+ self,
[
filters.FilterGroup('pkg', [
- PkgNameFilter('name', name='Named'),
+ PkgNameFilter('name', name=_('Named')),
PkgSetFilter('set', name=_('Limit to'))
], name='Packages' ),
filters.FilterGroup('opp', [
- OppFeaturedFilter('featured', name='Featured'),
- OppListFilter('list', name=_('Limit to'))
+ OppFeaturedFilter('featured', name=_('Featured')),
+ OppListFilter('list', name=_('Limit to')),
+ OppStatusFilter('status', name=_('Status'))
], name='Opportunities' )
],
- default_parameters = { 'pkg' : 'set',
- 'pkg.set' : ['ubuntu-desktop'],
- 'opp' : 'list',
- 'opp.list' : ['bitesize'] }
+ default_parameters = { 'pkg' : ['set'],
+ 'pkg.set' : ['ubuntu-desktop'],
+ 'opp' : ['list', 'status'],
+ 'opp.list' : ['bitesize'] }
)
=== modified file 'harvest/opportunities/views.py'
--- harvest/opportunities/views.py 2011-01-14 21:32:09 +0000
+++ harvest/opportunities/views.py 2011-02-22 03:46:56 +0000
@@ -1,6 +1,5 @@
# -*- coding: utf-8 -*-
from django.contrib.auth.decorators import login_required
-from django.db.models import F
from django.http import HttpResponseRedirect, HttpResponse
from django.shortcuts import get_object_or_404
from django.shortcuts import render_to_response as render
@@ -14,11 +13,6 @@
import json
-# Returns all valid opportunities in the given QuerySet
-def _get_valid_opportunities(opportunities_list):
- return opportunities_list.filter(valid=True, opportunitylist__active=True,
- last_updated=F('opportunitylist__last_updated'))
-
def _create_packages_list(request, filters_pkg, filters_opp):
# XXX: rockstar: Eep! We shouldn't be storing the None as a string. We
# should re-think this model relationship.
@@ -28,7 +22,7 @@
sourcepackages_list = filters_pkg.process_queryset(sourcepackages_list)
#opportunities_list is filtered right away to only check opportunities belonging to selected packages
- opportunities_list = _get_valid_opportunities(models.Opportunity.objects)
+ opportunities_list = models.Opportunity.objects
opportunities_list = opportunities_list.filter(sourcepackage__in=sourcepackages_list)
opportunities_list = filters_opp.process_queryset(opportunities_list)
@@ -66,7 +60,7 @@
def single_package(request, package_name):
package = get_object_or_404(models.SourcePackage, name=package_name)
- opportunities_list = _get_valid_opportunities(package.opportunity_set)
+ opportunities_list = package.opportunity_set
package_wrapper = PackageWrapper(request, package, visible_opportunities = opportunities_list)
context = {
@@ -80,7 +74,7 @@
def json_package_summary(request, package_name):
package = get_object_or_404(models.SourcePackage, name=package_name)
- opportunities_list = _get_valid_opportunities(package.opportunity_set)
+ opportunities_list = package.opportunity_set
data = {}
for opportunity in opportunities_list:
if not data.has_key(opportunity.opportunitylist.name):
@@ -176,7 +170,7 @@
package = get_object_or_404(models.SourcePackage, id=package_id)
- opportunities_list = _get_valid_opportunities(package.opportunity_set)
+ opportunities_list = package.opportunity_set
opportunities_list = filters.find('opp').process_queryset(opportunities_list).all()
package_wrapper = PackageWrapper(request, package, visible_opportunities = opportunities_list, expanded = True)
=== modified file 'harvest/opportunities/wrappers.py'
--- harvest/opportunities/wrappers.py 2010-08-31 07:55:08 +0000
+++ harvest/opportunities/wrappers.py 2011-02-22 03:46:56 +0000
@@ -1,5 +1,13 @@
+from django.db.models import F
from harvest.common.url_tools import update_url_with_parameters
+# Returns all valid opportunities in the given QuerySet
+def get_valid_opportunities(opportunities_list):
+ if opportunities_list:
+ return opportunities_list.filter(valid=True,
+ opportunitylist__active=True,
+ last_updated=F('opportunitylist__last_updated'))
+
class PackageWrapper(object):
"""
Describes a visible source package, for specific use in a
@@ -32,15 +40,17 @@
#order_by should really happen in the template, but the template
#language's dictsort is unstable pending a fix to Django bug
#11008: <http://code.djangoproject.com/ticket/11008>
- return self.visible_opportunities.order_by('-since', 'opportunitylist', 'experience')
+ opps = get_valid_opportunities(self.visible_opportunities)
+ return opps.order_by('-since', 'opportunitylist', 'experience')
def get_hidden_opportunities(self):
"""
Returns opportunities that belong to the given package but have
been hidden from view
"""
- opps_visible = self.get_visible_opportunities()
- return self.package.opportunity_set.exclude(id__in=opps_visible)
+ visible_opps = self.get_visible_opportunities()
+ all_opportunities = self.package.opportunity_set.exclude(id__in=visible_opps)
+ return get_valid_opportunities(all_opportunities)
class PackageListWrapper(object):
"""
@@ -69,8 +79,8 @@
opps = opportunities_list.filter(sourcepackage=package)
package_wrapper = PackageWrapper(request, package,
- visible_opportunities = opps,
- expanded = expand)
+ visible_opportunities = opps,
+ expanded = expand)
self.visible_packages_list.append(package_wrapper)
else:
