launchpad-reviewers team mailing list archive

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

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

Commit message:
Disables creation of new questions on targets without answers setup.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

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

Summary
=======
A last step in disabling questions on products without answers enabled (like
all private projects) is ensuring they cannot be added via the API. This has
to be done at the model level.

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

Implementation
==============
A check is added in newQuestion to ensure the target has answers_usage set to
ServiceUsage.LAUNCHPAD. If it is not, it raises QuestionTargetError, which is
already a valid API error.

Tests
=====
bin/test -vvct TestQuestionCreation

QA
==
Attempt to create a new question over the API on a product without answers
enabled. You will receive the error instead.

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/answers/tests/test_questiontarget.py
  lib/lp/app/widgets/launchpadtarget.py
  lib/lp/answers/model/question.py
  lib/lp/answers/browser/question.py
  lib/lp/answers/errors.py
-- 
https://code.launchpad.net/~jcsackett/launchpad/no-questions-on-disabled-3/+merge/138366
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/no-questions-on-disabled-3 into lp:launchpad.

=== modified file 'lib/lp/answers/errors.py'
--- lib/lp/answers/errors.py	2012-02-21 22:46:28 +0000
+++ lib/lp/answers/errors.py	2012-12-06 02:42:23 +0000
@@ -57,4 +57,4 @@
 
 @error_status(httplib.BAD_REQUEST)
 class QuestionTargetError(ValueError):
-    """The target must be an `IQueastionTarget`."""
+    """The target must be an `IQuestionTarget` with Answers enabled."""

=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py	2012-10-30 16:59:58 +0000
+++ lib/lp/answers/model/question.py	2012-12-06 02:42:23 +0000
@@ -1225,6 +1225,9 @@
     def newQuestion(self, owner, title, description, language=None,
         datecreated=None):
         """See `IQuestionTarget`."""
+        if self.answers_usage != ServiceUsage.LAUNCHPAD:
+            raise QuestionTargetError("The target for this question does not "
+                "have answers enabled.") 
         question = QuestionSet.new(
             title=title, description=description, owner=owner,
             datecreated=datecreated, language=language,

=== modified file 'lib/lp/answers/tests/test_questiontarget.py'
--- lib/lp/answers/tests/test_questiontarget.py	2012-01-01 02:58:52 +0000
+++ lib/lp/answers/tests/test_questiontarget.py	2012-12-06 02:42:23 +0000
@@ -10,6 +10,8 @@
 from zope.component import getUtility
 from zope.security.proxy import removeSecurityProxy
 
+from lp.answers.errors import QuestionTargetError
+from lp.app.enums import ServiceUsage
 from lp.registry.interfaces.distribution import IDistributionSet
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.testing import (
@@ -132,6 +134,25 @@
         # The languages cache has been filled in the correct order.
         self.failUnlessEqual(langs, [u'English', u'Portuguese (Brazil)'])
 
+class TestQuestionCreation(TestCaseWithFactory):
+    """Test newQuestion."""
+
+    layer = DatabaseFunctionalLayer
+
+    def test_target_must_enable_answers(self):
+        #A questiontarget isn't valid if answers isn't enabled.
+        target = self.factory.makeProduct(
+            answers_usage=ServiceUsage.LAUNCHPAD)
+        user = self.factory.makePerson()
+        with person_logged_in(user):
+            question = target.newQuestion(user, 'A question', 'A description')
+        self.assertEqual(u'A question', question.title)
+        
+        with person_logged_in(target.owner):
+            target.answers_usage = ServiceUsage.NOT_APPLICABLE
+        with person_logged_in(user):
+            self.assertRaises(QuestionTargetError,
+                target.newQuestion, user, 'A question', 'A description')
 
 class TestQuestionTargetCreateQuestionFromBug(TestCaseWithFactory):
     """Test the createQuestionFromBug from bug behavior."""