launchpad-reviewers team mailing list archive

[Steve Kowalik <stevenk@xxxxxxxxxx>]

Steve Kowalik has proposed merging lp:~stevenk/launchpad/exclude-inactive-projects-questions into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1177254 in Launchpad itself: "QuestionTargetSearch for a ProjectGroup does not exclude inactive projects"
  https://bugs.launchpad.net/launchpad/+bug/1177254

For more details, see:
https://code.launchpad.net/~stevenk/launchpad/exclude-inactive-projects-questions/+merge/163460

IProjectGroup.searchQuestions() (which is a thin wrapper around QuestionTargetSearch), did not filter by Product.active, so it may return inactive products, which will result in a 404 when regular users try and see them.
-- 
https://code.launchpad.net/~stevenk/launchpad/exclude-inactive-projects-questions/+merge/163460
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~stevenk/launchpad/exclude-inactive-projects-questions into lp:launchpad.

=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py	2013-05-01 17:12:51 +0000
+++ lib/lp/answers/model/question.py	2013-05-13 05:28:30 +0000
@@ -914,7 +914,7 @@
                         self.sourcepackagename))
         elif self.project:
             constraints.append("""
-                Question.product = Product.id AND
+                Question.product = Product.id AND Product.active AND
                 Product.project = %s""" % sqlvalues(self.project))
 
         return constraints

=== added file 'lib/lp/answers/tests/test_question.py'
--- lib/lp/answers/tests/test_question.py	1970-01-01 00:00:00 +0000
+++ lib/lp/answers/tests/test_question.py	2013-05-13 05:28:30 +0000
@@ -0,0 +1,22 @@
+# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from zope.security.proxy import removeSecurityProxy
+
+from lp.testing import TestCaseWithFactory
+from lp.testing.layers import DatabaseFunctionalLayer
+
+
+class TestQuestionSearch(TestCaseWithFactory):
+    layer = DatabaseFunctionalLayer
+
+    def test_projectgroup_with_inactive_products_not_in_results(self):
+        group = self.factory.makeProject()
+        product = self.factory.makeProduct(project=group)
+        inactive = self.factory.makeProduct(project=group)
+        question = self.factory.makeQuestion(target=product)
+        self.factory.makeQuestion(target=inactive)
+        removeSecurityProxy(inactive).active = False
+        self.assertContentEqual([question], group.searchQuestions())