launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #03039
[Merge] lp:~jcsackett/launchpad/set-question-message-visibility into lp:launchpad/db-devel
j.c.sackett has proposed merging lp:~jcsackett/launchpad/set-question-message-visibility into lp:launchpad/db-devel.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~jcsackett/launchpad/set-question-message-visibility/+merge/54408
Summary
=======
As part of ongoing Maintenance and spam work, this branch adds a means of hiding comments on questions to silence question spam via the API.
As part of this, the branch partially relies on a branch that exposes Question on the webservice. To get around the noise of those bits, you should look at http://paste.ubuntu.com/583961/ instead of the generated diff.
Implementation
==============
I've added a new method to Question, setCommentVisibility, that's a pretty close copy of the same method on Bug. This method is also exported over the webservice.
It's security is set to launchpad.Moderate, as set in the configure.zcml, with an adapter in the main security.py file.
Tests
=====
bin/test -t answers.*webservice.*Visibililty
QA
==
Hide a comment using the API; it shouldn't be visible on the answers page. Setting visible back to True should show it on the answers page.
Lint
====
= Launchpad lint =
Checking for conflicts and issues in changed files.
Linting changed files:
lib/canonical/launchpad/security.py
lib/canonical/launchpad/browser/message.py
lib/lp/answers/configure.zcml
lib/lp/answers/interfaces/question.py
lib/lp/answers/interfaces/webservice.py
lib/lp/answers/model/question.py
lib/lp/answers/tests/test_question_webservice.py
--
https://code.launchpad.net/~jcsackett/launchpad/set-question-message-visibility/+merge/54408
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jcsackett/launchpad/set-question-message-visibility into lp:launchpad/db-devel.
=== modified file 'lib/canonical/launchpad/browser/message.py'
--- lib/canonical/launchpad/browser/message.py 2009-08-27 07:05:16 +0000
+++ lib/canonical/launchpad/browser/message.py 2011-03-22 19:09:33 +0000
@@ -11,6 +11,16 @@
from canonical.launchpad.webapp.interfaces import ICanonicalUrlData
+class QuestionMessageCanonicalUrlData:
+ """Question messages have a canonical_url within the question."""
+ implements(ICanonicalUrlData)
+ rootsite = 'answers'
+
+ def __init__(self, question, message):
+ self.inside = question
+ self.path = "messages/%d" % list(question.messages).index(message)
+
+
class BugMessageCanonicalUrlData:
"""Bug messages have a canonical_url within the primary bugtask."""
implements(ICanonicalUrlData)
@@ -36,11 +46,18 @@
def message_to_canonical_url_data(message):
- """This factory creates `ICanonicalUrlData` for BugMessage."""
- if IIndexedMessage.providedBy(message):
- return IndexedBugMessageCanonicalUrlData(message)
- else:
- if message.bugs.count() == 0:
- # Will result in a ComponentLookupError
- return None
+ """This factory creates `ICanonicalUrlData` for Message."""
+ # Circular imports
+ from lp.bugs.interfaces.bugmessage import IBugMessage
+ from lp.answers.interfaces.questionmessage import IQuestionMessage
+
+ if IBugMessage.providedBy(message):
+ if IIndexedMessage.providedBy(message):
+ return IndexedBugMessageCanonicalUrlData(message)
+ else:
+ if message.bugs.count() == 0:
+ # Will result in a ComponentLookupError
+ return None
return BugMessageCanonicalUrlData(message.bugs[0], message)
+ if IQuestionMessage.providedBy(message):
+ return QuestionMessageCanonicalUrlData(message.question, message)
=== modified file 'lib/canonical/launchpad/security.py'
--- lib/canonical/launchpad/security.py 2011-03-15 11:50:41 +0000
+++ lib/canonical/launchpad/security.py 2011-03-22 19:09:33 +0000
@@ -1656,6 +1656,15 @@
checkUnauthenticated = _checkAccess
+class SetQuestionCommentVisibility(AuthorizationBase):
+ permission = 'launchpad.Moderate'
+ usedfor = IQuestion
+
+ def checkAuthenticated(self, user):
+ """Admins and registry admins can set bug comment visibility."""
+ return (user.in_admin or user.in_registry_experts)
+
+
class AdminQuestion(AdminByAdminsTeam):
permission = 'launchpad.Admin'
usedfor = IQuestion
=== modified file 'lib/lp/answers/configure.zcml'
--- lib/lp/answers/configure.zcml 2011-01-28 22:59:51 +0000
+++ lib/lp/answers/configure.zcml 2011-03-22 19:09:33 +0000
@@ -1,10 +1,11 @@
<!-- Copyright 2009 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
-
<configure
xmlns="http://namespaces.zope.org/zope";
+ xmlns:browser="http://namespaces.zope.org/browser";
xmlns:i18n="http://namespaces.zope.org/i18n";
+ xmlns:webservice="http://namespaces.canonical.com/webservice";
xmlns:lp="http://namespaces.canonical.com/lp";
i18n_domain="launchpad">
@@ -107,6 +108,10 @@
set_attributes="assignee"
/>
<require
+ permission="launchpad.Moderate"
+ attributes="setCommentVisibility"
+ />
+ <require
permission="launchpad.Admin"
attributes="setStatus"
set_attributes="priority whiteboard"
@@ -256,4 +261,6 @@
for="lp.registry.interfaces.person.IPerson"
factory="lp.answers.browser.questiontarget.AnswersVHostBreadcrumb"
permission="zope.Public"/>
+
+ <webservice:register module="lp.answers.interfaces.webservice" />
</configure>
=== modified file 'lib/lp/answers/interfaces/question.py'
--- lib/lp/answers/interfaces/question.py 2010-08-20 20:31:18 +0000
+++ lib/lp/answers/interfaces/question.py 2011-03-22 19:09:33 +0000
@@ -15,6 +15,19 @@
'IQuestionLinkFAQForm',
]
+
+from lazr.restful.declarations import (
+ call_with,
+ export_as_webservice_entry,
+ exported,
+ export_write_operation,
+ operation_parameters,
+ REQUEST_USER,
+ )
+from lazr.restful.fields import (
+ CollectionField,
+ Reference,
+ )
from zope.interface import (
Attribute,
Interface,
@@ -53,42 +66,44 @@
class IQuestion(IHasOwner):
"""A single question, often a support request."""
- id = Int(
+ export_as_webservice_entry()
+
+ id = exported(Int(
title=_('Question Number'), required=True, readonly=True,
- description=_("The tracking number for this question."))
- title = TextLine(
+ description=_("The tracking number for this question.")))
+ title = exported(TextLine(
title=_('Summary'), required=True, description=_(
- "A one-line summary of the issue or problem."))
- description = Text(
+ "A one-line summary of the issue or problem.")))
+ description = exported(Text(
title=_('Description'), required=True, description=_(
"Include as much detail as possible: what "
u"you\N{right single quotation mark}re trying to achieve, what steps "
- "you take, what happens, and what you think should happen instead."))
- status = Choice(
+ "you take, what happens, and what you think should happen instead.")))
+ status = exported(Choice(
title=_('Status'), vocabulary=QuestionStatus,
- default=QuestionStatus.OPEN, readonly=True)
- priority = Choice(
+ default=QuestionStatus.OPEN, readonly=True))
+ priority = exported(Choice(
title=_('Priority'), vocabulary=QuestionPriority,
- default=QuestionPriority.NORMAL)
+ default=QuestionPriority.NORMAL))
# XXX flacoste 2006-10-28: It should be more precise to define a new
# vocabulary that excludes the English variants.
language = Choice(
title=_('Language'), vocabulary='Language',
description=_('The language in which this question is written.'))
- owner = PublicPersonChoice(
+ owner = exported(PublicPersonChoice(
title=_('Owner'), required=True, readonly=True,
- vocabulary='ValidPersonOrTeam')
- assignee = PublicPersonChoice(
+ vocabulary='ValidPersonOrTeam'))
+ assignee = exported(PublicPersonChoice(
title=_('Assignee'), required=False,
description=_("The person responsible for helping to resolve the "
"question."),
- vocabulary='ValidPersonOrTeam')
- answerer = PublicPersonChoice(
+ vocabulary='ValidPersonOrTeam'))
+ answerer = exported(PublicPersonChoice(
title=_('Answered By'), required=False,
description=_("The person who last provided a response intended to "
"resolve the question."),
- vocabulary='ValidPersonOrTeam')
- answer = Object(
+ vocabulary='ValidPersonOrTeam'))
+ answer = Reference(
title=_('Answer'), required=False,
description=_("The IQuestionMessage that contains the answer "
"confirmed by the owner as providing a solution to his problem."),
@@ -146,13 +161,13 @@
'The set of subscriptions to this question.')
reopenings = Attribute(
"Records of times when this question was reopened.")
- messages = List(
+ messages = exported(List(
title=_("Messages"),
description=_(
"The list of messages that were exchanged as part of this "
"question , sorted from first to last."),
value_type=Object(schema=IQuestionMessage),
- required=True, default=[], readonly=True)
+ required=True, default=[], readonly=True))
# Workflow methods
def setStatus(user, new_status, comment, datecreated=None):
@@ -422,6 +437,7 @@
:return: A list of persons sorted by displayname.
"""
+
def getIndirectSubscribers():
"""Return the persons who are implicitly subscribed to this question.
@@ -455,6 +471,18 @@
notify along the rationale for doing so.
"""
+ @operation_parameters(
+ comment_number=Int(
+ title=_('The number of the comment in the list of messages.'),
+ required=True),
+ visible=Bool(title=_('Show this comment?'), required=True))
+ @call_with(user=REQUEST_USER)
+ @export_write_operation()
+ def setCommentVisibility(user, comment_number, visible):
+ """Set the visible attribute on a bug comment. This is restricted
+ to Launchpad admins, and will return a HTTP Error 401: Unauthorized
+ error for non-admin callers.
+ """
# These schemas are only used by browser/question.py and should really live
# there. See Bug #66950.
=== added file 'lib/lp/answers/interfaces/webservice.py'
--- lib/lp/answers/interfaces/webservice.py 1970-01-01 00:00:00 +0000
+++ lib/lp/answers/interfaces/webservice.py 2011-03-22 19:09:33 +0000
@@ -0,0 +1,16 @@
+# Copyright 2010 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""All the interfaces that are exposed through the webservice.
+
+There is a declaration in ZCML somewhere that looks like:
+ <webservice:register module="lp.answers.interfaces.webservice" />
+
+which tells `lazr.restful` that it should look for webservice exports here.
+"""
+
+__all__ = [
+ 'IQuestion',
+ ]
+
+from lp.answers.interfaces.question import IQuestion
=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py 2011-01-29 16:52:18 +0000
+++ lib/lp/answers/model/question.py 2011-03-22 19:09:33 +0000
@@ -646,6 +646,10 @@
"""See BugLinkTargetMixin."""
return QuestionBug(question=self, bug=bug)
+ def setCommentVisibility(self, user, comment_number, visible):
+ """See `IQuestion`."""
+ message = self.messages[comment_number]
+ message.visible = visible
class QuestionSet:
"""The set of questions in the Answer Tracker."""
=== added file 'lib/lp/answers/tests/test_question_webservice.py'
--- lib/lp/answers/tests/test_question_webservice.py 1970-01-01 00:00:00 +0000
+++ lib/lp/answers/tests/test_question_webservice.py 2011-03-22 19:09:33 +0000
@@ -0,0 +1,152 @@
+# Copyright 2011 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Webservice unit tests related to Launchpad Questions."""
+
+__metaclass__ = type
+
+from BeautifulSoup import BeautifulSoup
+from lazr.restfulclient.errors import HTTPError
+from simplejson import dumps
+import transaction
+from zope.component import getUtility
+from zope.security.management import endInteraction
+
+from canonical.launchpad.testing.pages import LaunchpadWebServiceCaller
+from canonical.testing.layers import DatabaseFunctionalLayer
+from lp.registry.interfaces.person import IPersonSet
+from lp.testing import (
+ TestCaseWithFactory,
+ celebrity_logged_in,
+ launchpadlib_for,
+ person_logged_in,
+ )
+
+
+class TestQuestionRepresentation(TestCaseWithFactory):
+ """Test ways of interacting with Question webservice representations."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestQuestionRepresentation, self).setUp()
+ with celebrity_logged_in('admin'):
+ self.question = self.factory.makeQuestion(
+ title="This is a question")
+
+ self.webservice = LaunchpadWebServiceCaller(
+ 'launchpad-library', 'salgado-change-anything')
+
+ def findQuestionTitle(self, response):
+ """Find the question title field in an XHTML document fragment."""
+ soup = BeautifulSoup(response.body)
+ dt = soup.find('dt', text="title").parent
+ dd = dt.findNextSibling('dd')
+ return str(dd.contents.pop())
+
+ def test_GET_xhtml_representation(self):
+ response = self.webservice.get(
+ '/%s/+question/%d' % (self.question.target.name,
+ self.question.id),
+ 'application/xhtml+xml')
+ self.assertEqual(response.status, 200)
+
+ self.assertEqual(
+ self.findQuestionTitle(response),
+ "<p>This is a question</p>")
+
+ def test_PATCH_xhtml_representation(self):
+ new_title = "No, this is a question"
+
+ question_json = self.webservice.get(
+ '/%s/+question/%d' % (self.question.target.name,
+ self.question.id)).jsonBody()
+
+ response = self.webservice.patch(
+ question_json['self_link'],
+ 'application/json',
+ dumps(dict(title=new_title)),
+ headers=dict(accept='application/xhtml+xml'))
+
+ self.assertEqual(response.status, 209)
+
+ self.assertEqual(
+ self.findQuestionTitle(response),
+ "<p>No, this is a question</p>")
+
+
+class TestSetCommentVisibility(TestCaseWithFactory):
+ """Tests who can successfully set comment visibility."""
+
+ layer = DatabaseFunctionalLayer
+
+ def setUp(self):
+ super(TestSetCommentVisibility, self).setUp()
+ self.person_set = getUtility(IPersonSet)
+ admins = self.person_set.getByName('admins')
+ self.admin = admins.teamowner
+ with person_logged_in(self.admin):
+ self.question = self.factory.makeQuestion()
+ self.message = self.question.addComment(
+ self.admin, 'Some comment')
+ transaction.commit()
+
+ def _get_question_for_user(self, user=None):
+ """Convenience function to get the api question reference."""
+ endInteraction()
+ if user is not None:
+ lp = launchpadlib_for("test", user)
+ else:
+ lp = launchpadlib_for("test")
+
+ question_entry = lp.load(
+ '/%s/+question/%d/' % (
+ self.question.target.name, self.question.id))
+ return question_entry
+
+ def _set_visibility(self, question):
+ """Method to set visibility; needed for assertRaises."""
+ question.setCommentVisibility(
+ comment_number=0,
+ visible=False)
+
+ def test_random_user_cannot_set_visible(self):
+ # Logged in users without privs can't set question comment
+ # visibility.
+ nopriv = self.person_set.getByName('no-priv')
+ question = self._get_question_for_user(nopriv)
+ self.assertRaises(
+ HTTPError,
+ self._set_visibility,
+ question)
+
+ def test_anon_cannot_set_visible(self):
+ # Anonymous users can't set question comment
+ # visibility.
+ question = self._get_question_for_user()
+ self.assertRaises(
+ HTTPError,
+ self._set_visibility,
+ question)
+
+ def test_registry_admin_can_set_visible(self):
+ # Members of registry experts can set question comment
+ # visibility.
+ registry = self.person_set.getByName('registry')
+ person = self.factory.makePerson()
+ with person_logged_in(registry.teamowner):
+ registry.addMember(person, registry.teamowner)
+ question = self._get_question_for_user(person)
+ self._set_visibility(question)
+ self.assertFalse(self.message.visible)
+
+ def test_admin_can_set_visible(self):
+ # Admins can set question comment
+ # visibility.
+ admins = self.person_set.getByName('admins')
+ person = self.factory.makePerson()
+ with person_logged_in(admins.teamowner):
+ admins.addMember(person, admins.teamowner)
+ question = self._get_question_for_user(person)
+ self._set_visibility(question)
+ self.assertFalse(self.message.visible)
