harvest-dev team mailing list archive

[Dylan McCall <dylanmccall@xxxxxxxxx>]

Dylan McCall has proposed merging lp:~dylanmccall/harvest/bug-685984 into lp:harvest.

Requested reviews:
  harvest-dev (harvest-dev)
Related bugs:
  #685984 Inactive opportunity lists are not always filtered out
  https://bugs.launchpad.net/bugs/685984


Some little changes to opportunities.views to ignore invalid opportunities. An invalid opportunity is identified as having its valid property set to False, belonging to an inactive opportunity list, or having a last_updated property different from its opportunity list.

While I was at it, I removed some calls to the .distinct() method for our querysets, which are leftovers from earlier fiddling.
-- 
https://code.launchpad.net/~dylanmccall/harvest/bug-685984/+merge/43024
Your team harvest-dev is requested to review the proposed merge of lp:~dylanmccall/harvest/bug-685984 into lp:harvest.

=== modified file 'harvest/opportunities/views.py'
--- harvest/opportunities/views.py	2010-10-21 08:47:37 +0000
+++ harvest/opportunities/views.py	2010-12-07 23:01:55 +0000
@@ -13,20 +13,24 @@
 from filters import HarvestFilters
 from wrappers import PackageWrapper, PackageListWrapper
 
+# 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.
     #sourcepackages_list = models.SourcePackage.objects.exclude(name='None')
     
-    sourcepackages_list = models.SourcePackage.objects.distinct()
+    sourcepackages_list = models.SourcePackage.objects
     sourcepackages_list = filters_pkg.process_queryset(sourcepackages_list)
     
     #opportunities_list is filtered right away to only check opportunities belonging to selected packages
-    opportunities_list = models.Opportunity.objects.distinct().filter(sourcepackage__in=sourcepackages_list,
-                                                                      last_updated=F('opportunitylist__last_updated'))
+    opportunities_list = _get_valid_opportunities(models.Opportunity.objects)
+    opportunities_list = opportunities_list.filter(sourcepackage__in=sourcepackages_list)
     opportunities_list = filters_opp.process_queryset(opportunities_list)
     
-    #TODO: need to filter out opportunities with valid=False again
     #TODO: would it be more efficient to group opportunities by their sourcepackages first, then run filters_opp.process_queryset() for each of those groups?
     
     pkg_list_wrapper = PackageListWrapper(request, sourcepackages_list, opportunities_list)
@@ -61,7 +65,8 @@
 def single_package(request, package_name):
     package = get_object_or_404(models.SourcePackage, name=package_name)
     
-    package_wrapper = PackageWrapper(request, package, visible_opportunities = package.opportunity_set)
+    opportunities_list = _get_valid_opportunities(package.opportunity_set)
+    package_wrapper = PackageWrapper(request, package, visible_opportunities = opportunities_list)
     
     context = {
         'package': package_wrapper
@@ -158,7 +163,8 @@
     
     package = get_object_or_404(models.SourcePackage, id=package_id)
     
-    opportunities_list = filters.find('opp').process_queryset(package.opportunity_set).all()
+    opportunities_list = _get_valid_opportunities(package.opportunity_set)
+    opportunities_list = filters.find('opp').process_queryset(opportunities_list).all()
     
     package_wrapper = PackageWrapper(request, package, visible_opportunities = opportunities_list, expanded = True)