launchpad-reviewers team mailing list archive

["j.c.sackett" <jcsackett@xxxxxxxxxxxxx>]

j.c.sackett has proposed merging lp:~jcsackett/launchpad/no-questions-on-disabled-2 into lp:launchpad with lp:~jcsackett/launchpad/no-questions-on-disabled as a prerequisite.

Commit message:
Removes products that do not have answers disabled from questiontarget picker results.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/no-questions-on-disabled-2/+merge/138362

Summary
=======
As targets without answers enabled (including private projects) are not valid
question targets, they should not appear in the question target picker when
retargeting a question.

To do this, the vocabulary needs to be modified.

Preimp
======
None. Continuation of work.

Implementation
==============
A new vocabulary, UsesAnswersProductVocabulary, is created that passes an
additional filter to its parent class, ProductVocabulary on search. This
filter removes products that do not have official_answers = True.

This vocabulary is then used in the questiontarget widget.

Ideally, the filter should use answers_usage over official_answers, but using
that for the filter resulted in empty resultsets for reasons I couldn't
adequately explain. As the companion filter,
UsesAnswersDistributionVocabulary, uses official_answers this is at least
consistent.

Tests
=====
bin/test -vvct test_products_not_using_answers_not_found

QA
==
Search for a product without answers setup in the question retargeting picker.
It should not show up in the picker results.

LoC
===
Part of private projects.

Lint
====

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/answers/configure.zcml
  lib/lp/answers/browser/tests/test_question.py
  lib/lp/answers/templates/question-add-search.pt
  lib/lp/answers/tests/test_vocabulary.py
  lib/lp/answers/vocabulary.py
  lib/lp/testing/factory.py
  lib/lp/app/widgets/launchpadtarget.py
  lib/lp/answers/browser/question.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/no-questions-on-disabled-2/+merge/138362
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/no-questions-on-disabled-2 into lp:launchpad.

=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py	2012-12-06 02:30:30 +0000
+++ lib/lp/answers/browser/question.py	2012-12-06 02:30:30 +0000
@@ -76,7 +76,10 @@
     IAnswersFrontPageSearchForm,
     IQuestionTarget,
     )
-from lp.answers.vocabulary import UsesAnswersDistributionVocabulary
+from lp.answers.vocabulary import (
+    UsesAnswersDistributionVocabulary,
+    UsesAnswersProductVocabulary,
+    )
 from lp.app.browser.launchpadform import (
     action,
     custom_widget,
@@ -739,6 +742,9 @@
 class QuestionTargetWidget(LaunchpadTargetWidget):
     """A targeting widget that is aware of pillars that use Answers."""
 
+    def getProductVocabulary(self):
+        return 'UsesAnswersProduct'
+
     def getDistributionVocabulary(self):
         distro = self.context.context.distribution
         vocabulary = UsesAnswersDistributionVocabulary(distro)

=== modified file 'lib/lp/answers/configure.zcml'
--- lib/lp/answers/configure.zcml	2012-10-30 16:59:58 +0000
+++ lib/lp/answers/configure.zcml	2012-12-06 02:30:30 +0000
@@ -187,6 +187,17 @@
     <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
   </securedutility>
 
+  <securedutility
+    name="UsesAnswersProduct"
+    component=".vocabulary.UsesAnswersProductVocabulary"
+    provides="zope.schema.interfaces.IVocabularyFactory"
+    >
+    <allow interface="zope.schema.interfaces.IVocabularyFactory"/>
+  </securedutility>
+
+  <class class=".vocabulary.UsesAnswersProductVocabulary">
+    <allow interface="lp.services.webapp.vocabulary.IHugeVocabulary"/>
+  </class>
   <!-- QuestionSet -->
   <class class=".model.question.QuestionSet">
     <allow interface=".interfaces.questioncollection.IQuestionSet" />

=== modified file 'lib/lp/answers/tests/test_vocabulary.py'
--- lib/lp/answers/tests/test_vocabulary.py	2012-01-01 02:58:52 +0000
+++ lib/lp/answers/tests/test_vocabulary.py	2012-12-06 02:30:30 +0000
@@ -5,7 +5,11 @@
 
 __metaclass__ = type
 
-from lp.answers.vocabulary import UsesAnswersDistributionVocabulary
+from lp.answers.vocabulary import (
+    UsesAnswersDistributionVocabulary,
+    UsesAnswersProductVocabulary,
+    )
+from lp.app.enums import ServiceUsage
 from lp.testing import (
     person_logged_in,
     TestCaseWithFactory,
@@ -15,6 +19,7 @@
 
 class UsesAnswersDistributionVocabularyTestCase(TestCaseWithFactory):
     """Test that the vocabulary behaves as expected."""
+
     layer = DatabaseFunctionalLayer
 
     def test_init_with_distribution(self):
@@ -23,8 +28,7 @@
         distribution = self.factory.makeDistribution()
         vocabulary = UsesAnswersDistributionVocabulary(distribution)
         self.assertEqual(distribution, vocabulary.context)
-        self.assertEqual(distribution, vocabulary.distribution)
-
+        self.assertEqual(distribution, vocabulary.distribution) 
     def test_init_without_distribution(self):
         # When the context is not adaptable to IDistribution, the
         # distribution property is None
@@ -67,3 +71,21 @@
         self.assertFalse(
             thing in vocabulary,
             "Vocabulary contains a non-distribution.")
+
+
+class UsesAnswersProductVocabularyTestCase(TestCaseWithFactory):
+    """Test that the product vocabulary behaves as expected."""
+    
+    layer = DatabaseFunctionalLayer
+
+    def test_products_not_using_answers_not_found(self):
+        using_product = self.factory.makeProduct(
+            name='foobar', answers_usage=ServiceUsage.LAUNCHPAD)
+        not_using_product = self.factory.makeProduct(
+            name='foobarbaz', answers_usage=ServiceUsage.NOT_APPLICABLE)
+        vocabulary = UsesAnswersProductVocabulary()
+        products = vocabulary.search(query='foobar')
+        self.assertTrue(using_product in products,
+            'Valid product not found in vocabulary.')
+        self.assertTrue(not_using_product not in products,
+            'Vocabulary found a product not using answers.')

=== modified file 'lib/lp/answers/vocabulary.py'
--- lib/lp/answers/vocabulary.py	2012-01-01 02:58:52 +0000
+++ lib/lp/answers/vocabulary.py	2012-12-06 02:30:30 +0000
@@ -7,16 +7,23 @@
 __all__ = [
     'FAQVocabulary',
     'UsesAnswersDistributionVocabulary',
+    'UsesAnswersProductVocabulary',
     ]
 
 from sqlobject import OR
+from storm.expr import And
 from zope.interface import implements
 from zope.schema.vocabulary import SimpleTerm
 
+from lp.app.enums import ServiceUsage
 from lp.answers.interfaces.faq import IFAQ
 from lp.answers.interfaces.faqtarget import IFAQTarget
 from lp.registry.interfaces.distribution import IDistribution
-from lp.registry.vocabularies import DistributionVocabulary
+from lp.registry.model.product import Product
+from lp.registry.vocabularies import (
+    DistributionVocabulary,
+    ProductVocabulary,
+    )
 from lp.services.webapp.vocabulary import (
     CountableIterator,
     FilteredVocabularyBase,
@@ -80,6 +87,17 @@
         return CountableIterator(results.count(), results, self.toTerm)
 
 
+class UsesAnswersProductVocabulary(ProductVocabulary):
+    """Products that use Launchpad to track questions."""
+
+    def search(self, query, vocab_filter=None):
+        if vocab_filter is None:
+            vocab_filter = []
+        vocab_filter.append(
+            And(Product.official_answers == True))
+        return super(UsesAnswersProductVocabulary, self).search(
+            query, vocab_filter)
+
 class UsesAnswersDistributionVocabulary(DistributionVocabulary):
     """Distributions that use Launchpad to track questions.
 

=== modified file 'lib/lp/app/widgets/launchpadtarget.py'
--- lib/lp/app/widgets/launchpadtarget.py	2012-10-25 05:09:07 +0000
+++ lib/lp/app/widgets/launchpadtarget.py	2012-12-06 02:30:30 +0000
@@ -52,6 +52,9 @@
 
     def getDistributionVocabulary(self):
         return 'Distribution'
+    
+    def getProductVocabulary(self):
+        return 'Product'
 
     def setUpSubWidgets(self):
         if self._widgets_set_up:
@@ -59,7 +62,7 @@
         fields = [
             Choice(
                 __name__='product', title=u'Project',
-                required=True, vocabulary='Product'),
+                required=True, vocabulary=self.getProductVocabulary()),
             Choice(
                 __name__='distribution', title=u"Distribution",
                 required=True, vocabulary=self.getDistributionVocabulary(),

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2012-12-05 15:58:18 +0000
+++ lib/lp/testing/factory.py	2012-12-06 02:30:30 +0000
@@ -973,7 +973,8 @@
         title=None, summary=None, official_malone=None,
         translations_usage=None, bug_supervisor=None, driver=None, icon=None,
         bug_sharing_policy=None, branch_sharing_policy=None,
-        specification_sharing_policy=None, information_type=None):
+        specification_sharing_policy=None, information_type=None,
+        answers_usage=None):
         """Create and return a new, arbitrary Product."""
         if owner is None:
             owner = self.makePerson()
@@ -1020,6 +1021,8 @@
             naked_product.official_malone = official_malone
         if translations_usage is not None:
             naked_product.translations_usage = translations_usage
+        if answers_usage is not None:
+            naked_product.answers_usage = answers_usage
         if bug_supervisor is not None:
             naked_product.bug_supervisor = bug_supervisor
         if driver is not None: