Merge lp:~dylanmccall/harvest/bug-685984 into lp:harvest

[Dylan McCall]

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.

[James Westby (james-w)]

Looks good.

Thanks,

James

review: Approve

=== 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)