Merge lp:~abentley/launchpad/product-specifications-storm into lp:launchpad

[Aaron Bentley]

Reimplement Product.specifications.

= Summary =
Make it easy to implement privacy for Product.specifications.

== Proposed fix ==
Reimplement Product.specifications in terms of get_specification_filters.

== Pre-implementation notes ==
None.

== LOC Rationale ==
Part of Private Products.

== Implementation details ==
None

== Tests ==
bin/test test_product.py -t TestSpecifications

== Demo and Q/A ==
None

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/testing/factory.py
  lib/lp/blueprints/model/specification.py
  lib/lp/blueprints/model/sprint.py
  lib/lp/registry/model/product.py
  lib/lp/registry/model/person.py
  lib/lp/registry/tests/test_person.py
  lib/lp/registry/tests/test_product.py

[Deryck Hodge (deryck)]

review: Approve

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2012-10-16 18:11:17 +0000
+++ lib/lp/blueprints/model/specification.py	2012-10-17 14:46:55 +0000
@@ -977,7 +977,7 @@
         elif sort == SpecificationSort.DATE:
             return (Desc(Specification.datecreated), Specification.id)
 
-    def _preload_specifications_people(self, query):
+    def _preload_specifications_people(self, clauses):
         """Perform eager loading of people and their validity for query.
 
         :param query: a string query generated in the 'specifications'
@@ -986,6 +986,8 @@
         """
         # Circular import.
         from lp.registry.model.person import Person
+        if isinstance(clauses, basestring):
+            clauses = [SQL(clauses)]
 
         def cache_people(rows):
             # Find the people we need:
@@ -1018,7 +1020,7 @@
 
         results = Store.of(self).find(
             Specification,
-            SQL(query),
+            *clauses
             )
         return DecoratedResultSet(results, pre_iter_hook=cache_people)
 

=== modified file 'lib/lp/registry/model/product.py'
--- lib/lp/registry/model/product.py	2012-10-16 14:28:22 +0000
+++ lib/lp/registry/model/product.py	2012-10-17 14:46:55 +0000
@@ -92,11 +92,10 @@
     )
 from lp.app.model.launchpad import InformationTypeMixin
 from lp.blueprints.enums import (
-    SpecificationDefinitionStatus,
     SpecificationFilter,
-    SpecificationImplementationStatus,
     )
 from lp.blueprints.model.specification import (
+    get_specification_filters,
     HasSpecificationsMixin,
     Specification,
     SPECIFICATION_POLICY_ALLOWED_TYPES,
@@ -193,7 +192,6 @@
     )
 from lp.services.database.lpstorm import IStore
 from lp.services.database.sqlbase import (
-    quote,
     SQLBase,
     sqlvalues,
     )
@@ -1357,47 +1355,12 @@
         #  - completeness.
         #  - informational.
         #
-        base = 'Specification.product = %s' % self.id
-        query = base
-        # look for informational specs
-        if SpecificationFilter.INFORMATIONAL in filter:
-            query += (' AND Specification.implementation_status = %s' %
-              quote(SpecificationImplementationStatus.INFORMATIONAL))
-
-        # filter based on completion. see the implementation of
-        # Specification.is_complete() for more details
-        completeness = Specification.completeness_clause
-
-        if SpecificationFilter.COMPLETE in filter:
-            query += ' AND ( %s ) ' % completeness
-        elif SpecificationFilter.INCOMPLETE in filter:
-            query += ' AND NOT ( %s ) ' % completeness
-
-        # Filter for validity. If we want valid specs only then we should
-        # exclude all OBSOLETE or SUPERSEDED specs
-        if SpecificationFilter.VALID in filter:
-            query += (' AND Specification.definition_status NOT IN '
-                '( %s, %s ) ' % sqlvalues(
-                    SpecificationDefinitionStatus.OBSOLETE,
-                    SpecificationDefinitionStatus.SUPERSEDED))
-
-        # ALL is the trump card
-        if SpecificationFilter.ALL in filter:
-            query = base
-
-        # Filter for specification text
-        for constraint in filter:
-            if isinstance(constraint, basestring):
-                # a string in the filter is a text search filter
-                query += ' AND Specification.fti @@ ftq(%s) ' % quote(
-                    constraint)
-
+        clauses = [Specification.product == self]
+        clauses.extend(get_specification_filters(filter))
         if prejoin_people:
-            results = self._preload_specifications_people(query)
+            results = self._preload_specifications_people(clauses)
         else:
-            results = Store.of(self).find(
-                Specification,
-                SQL(query))
+            results = Store.of(self).find(Specification, *clauses)
         results.order_by(order)
         if quantity is not None:
             results = results[:quantity]