launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04578
[Merge] lp:~wallyworld/launchpad/question-contacts-portlet-view into lp:launchpad
Ian Booth has proposed merging lp:~wallyworld/launchpad/question-contacts-portlet-view into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/question-contacts-portlet-view/+merge/71301
The branch is a prerequisite for providing an ajax answer contacts portlet for question targets as per bug 823945.
== Implementation ==
Provide a new view and server side boiler plate to provide the data needed by the javascript client in order to render the answer contacts portlet.
The new view is: lp.answers.browser.questiontarget.QuestionTargetPortletAnswerContactsWithDetails
It is modelled on was is done for the bug subscribers portlet.
Move the canUserAlterAnswerContact() method declaration from IQuestionTargetView to IQuestionTargetPublic so that it can be called by anonymous users.
Change canUserAlterAnswerContact() so that target owners are allowed to add/remove answer contacts. This is required as per the bug report.
The next branch will wire up the new view to the javascript portlet.
As a driveby, remove an allowed attribute "removeAnswerContact" from Question in configure.zcml. This should have been deleted in a previous branch.
== Tests ==
Add new test case to test the view: QuestionTargetPortletAnswerContactsWithDetailsTests
Add new test for the changed canUserAlterAnswerContact behaviour: QuestionTargetAnswerContactTestCase.test_canUserAlterAnswerContact_owner
bin/test -vvct test_questiontarget
== Lint ==
Checking for conflicts and issues in changed files.
Linting changed files:
lib/lp/answers/configure.zcml
lib/lp/answers/browser/configure.zcml
lib/lp/answers/browser/questiontarget.py
lib/lp/answers/browser/tests/test_questiontarget.py
lib/lp/answers/interfaces/questiontarget.py
lib/lp/answers/model/question.py
lib/lp/answers/tests/test_questiontarget.py
--
https://code.launchpad.net/~wallyworld/launchpad/question-contacts-portlet-view/+merge/71301
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/question-contacts-portlet-view into lp:launchpad.
=== modified file 'lib/lp/answers/browser/configure.zcml'
--- lib/lp/answers/browser/configure.zcml 2011-07-28 03:22:54 +0000
+++ lib/lp/answers/browser/configure.zcml 2011-08-12 04:40:27 +0000
@@ -92,6 +92,12 @@
template="../templates/questiontarget-portlet-answercontacts.pt"
/>
<browser:page
+ for="lp.answers.interfaces.question.IQuestionTarget"
+ name="+portlet-answercontacts-details"
+ class="
+ lp.answers.browser.questiontarget.QuestionTargetPortletAnswerContactsWithDetails"
+ permission="zope.Public"/>
+ <browser:page
name="+addquestion"
for="lp.answers.interfaces.questiontarget.IQuestionTarget"
class=".question.QuestionAddView"
=== modified file 'lib/lp/answers/browser/questiontarget.py'
--- lib/lp/answers/browser/questiontarget.py 2011-05-27 21:12:25 +0000
+++ lib/lp/answers/browser/questiontarget.py 2011-08-12 04:40:27 +0000
@@ -17,14 +17,18 @@
'QuestionCollectionOpenCountView',
'QuestionCollectionAnswersMenu',
'QuestionTargetFacetMixin',
+ 'QuestionTargetPortletAnswerContactsWithDetails',
'QuestionTargetTraversalMixin',
'QuestionTargetAnswersMenu',
'UserSupportLanguagesMixin',
]
from operator import attrgetter
+from simplejson import dumps
from urllib import urlencode
+from lazr.restful.interfaces import IWebServiceClientRequest
+
from z3c.ptcompat import ViewPageTemplateFile
from zope.app.form.browser import DropdownWidget
from zope.component import (
@@ -42,6 +46,7 @@
SimpleTerm,
SimpleVocabulary,
)
+from zope.traversing.browser import absoluteURL
from canonical.launchpad import _
from canonical.launchpad.helpers import (
@@ -60,6 +65,7 @@
from canonical.launchpad.webapp.batching import BatchNavigator
from canonical.launchpad.webapp.breadcrumb import Breadcrumb
from canonical.launchpad.webapp.menu import structured
+from canonical.launchpad.webapp.publisher import LaunchpadView
from lp.answers.browser.faqcollection import FAQCollectionMenu
from lp.answers.enums import QuestionStatus
from lp.answers.interfaces.faqcollection import IFAQCollection
@@ -810,6 +816,55 @@
response.addNotification(structured(msgid))
+class QuestionTargetPortletAnswerContactsWithDetails(LaunchpadView):
+ """View returns JSON dump of answer contact details for questiontarget."""
+
+ @cachedproperty
+ def api_request(self):
+ return IWebServiceClientRequest(self.request)
+
+ def answercontact_data(self, questiontarget):
+ """Get the answer contact data.
+
+ This method is isolated from the answercontact_data_js so that query
+ count testing can be done accurately and robustly.
+ """
+ data = []
+ answer_contacts = list(questiontarget.direct_answer_contacts)
+ for person in answer_contacts:
+ can_edit = questiontarget.canUserAlterAnswerContact(
+ person, self.user)
+ if person.private and not can_edit:
+ # Skip private teams user is not a member of.
+ continue
+
+ answer_contact = {
+ 'name': person.name,
+ 'display_name': person.displayname,
+ 'web_link': canonical_url(person, rootsite='mainsite'),
+ 'self_link': absoluteURL(person, self.api_request),
+ 'is_team': person.is_team,
+ 'can_edit': can_edit
+ }
+ record = {
+ 'subscriber': answer_contact,
+ }
+ data.append(record)
+ return data
+
+ @property
+ def answercontact_data_js(self):
+ """Return subscriber_ids in a form suitable for JavaScript use."""
+ questiontarget = IQuestionTarget(self.context)
+ data = self.answercontact_data(questiontarget)
+ return dumps(data)
+
+ def render(self):
+ """Override the default render() to return only JSON."""
+ self.request.response.setHeader('content-type', 'application/json')
+ return self.answercontact_data_js
+
+
class QuestionTargetFacetMixin:
"""Mixin for questiontarget facet definition."""
=== modified file 'lib/lp/answers/browser/tests/test_questiontarget.py'
--- lib/lp/answers/browser/tests/test_questiontarget.py 2011-07-22 14:50:56 +0000
+++ lib/lp/answers/browser/tests/test_questiontarget.py 2011-08-12 04:40:27 +0000
@@ -6,22 +6,36 @@
__metaclass__ = type
import os
+from simplejson import dumps
from urllib import quote
from BeautifulSoup import BeautifulSoup
from zope.component import getUtility
-
+from zope.traversing.browser import absoluteURL
+
+from lazr.restful.interfaces import IWebServiceClientRequest
+
+from canonical.launchpad.webapp import canonical_url
from canonical.launchpad.testing.pages import find_tag_by_id
-from canonical.testing.layers import DatabaseFunctionalLayer
+from canonical.testing.layers import (
+ DatabaseFunctionalLayer,
+ LaunchpadFunctionalLayer,
+ )
from lp.answers.interfaces.questioncollection import IQuestionSet
from lp.app.enums import ServiceUsage
from lp.app.interfaces.launchpad import ILaunchpadCelebrities
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.worlddata.interfaces.language import ILanguageSet
from lp.testing import (
login_person,
person_logged_in,
TestCaseWithFactory,
)
-from lp.testing.views import create_initialized_view
+from lp.testing.sampledata import ADMIN_EMAIL
+from lp.testing.views import (
+ create_initialized_view,
+ create_view,
+ )
class TestSearchQuestionsView(TestCaseWithFactory):
@@ -222,3 +236,192 @@
self.assertIn(picker_vocab, text)
focus_script = "setFocusByName('field.search_text')"
self.assertIn(focus_script, text)
+
+
+class QuestionTargetPortletAnswerContactsWithDetailsTests(
+ TestCaseWithFactory):
+ """Tests for IQuestionTarget:+portlet-answercontacts-details view."""
+ layer = LaunchpadFunctionalLayer
+
+ def test_content_type(self):
+ question = self.factory.makeQuestion()
+
+ # It works even for anonymous users, so no log-in is needed.
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ view.render()
+
+ self.assertEqual(
+ view.request.response.getHeader('content-type'),
+ 'application/json')
+
+ def test_data_no_answer_contacts(self):
+ question = self.factory.makeQuestion()
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ self.assertEqual(dumps([]), view.answercontact_data_js)
+
+ def test_data_person_answercontact(self):
+ # answercontact_data_js returns JSON string of a list
+ # containing all contact information needed for
+ # subscribers_list.js loading.
+ question = self.factory.makeQuestion()
+ contact = self.factory.makePerson(
+ name='user', displayname='Contact Name')
+ contact.addLanguage(getUtility(ILanguageSet)['en'])
+ with person_logged_in(contact):
+ question.target.addAnswerContact(contact, contact)
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ api_request = IWebServiceClientRequest(view.request)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'user',
+ 'display_name': 'Contact Name',
+ 'is_team': False,
+ 'can_edit': False,
+ 'web_link': canonical_url(contact),
+ 'self_link': absoluteURL(contact, api_request)
+ }
+ }
+ self.assertEqual(
+ dumps([expected_result]), view.answercontact_data_js)
+
+ def test_data_team_answer_contact(self):
+ # For a team answer contacts, answercontact_data_js has is_team set
+ # to true.
+ question = self.factory.makeQuestion()
+ teamowner = self.factory.makePerson(
+ name="team-owner", displayname="Team Owner")
+ contact = self.factory.makeTeam(
+ name='team', displayname='Team Name', owner=teamowner)
+ contact.addLanguage(getUtility(ILanguageSet)['en'])
+ with person_logged_in(contact.teamowner):
+ question.target.addAnswerContact(contact, contact)
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ api_request = IWebServiceClientRequest(view.request)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'team',
+ 'display_name': 'Team Name',
+ 'is_team': True,
+ 'can_edit': False,
+ 'web_link': canonical_url(contact),
+ 'self_link': absoluteURL(contact, api_request)
+ }
+ }
+ self.assertEqual(
+ dumps([expected_result]), view.answercontact_data_js)
+
+ def test_data_team_answercontact_owner_looks(self):
+ # For a team subscription, answercontact_data_js has can_edit
+ # set to true for team owner.
+ question = self.factory.makeQuestion()
+ teamowner = self.factory.makePerson(
+ name="team-owner", displayname="Team Owner")
+ contact = self.factory.makeTeam(
+ name='team', displayname='Team Name', owner=teamowner)
+ contact.addLanguage(getUtility(ILanguageSet)['en'])
+ with person_logged_in(contact.teamowner):
+ question.target.addAnswerContact(contact, contact.teamowner)
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ api_request = IWebServiceClientRequest(view.request)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'team',
+ 'display_name': 'Team Name',
+ 'is_team': True,
+ 'can_edit': True,
+ 'web_link': canonical_url(contact),
+ 'self_link': absoluteURL(contact, api_request)
+ }
+ }
+ with person_logged_in(contact.teamowner):
+ self.assertEqual(
+ dumps([expected_result]), view.answercontact_data_js)
+
+ def test_data_team_subscription_member_looks(self):
+ # For a team subscription, answercontact_data_js has can_edit
+ # set to true for team member.
+ question = self.factory.makeQuestion()
+ member = self.factory.makePerson()
+ teamowner = self.factory.makePerson(
+ name="team-owner", displayname="Team Owner")
+ contact = self.factory.makeTeam(
+ name='team', displayname='Team Name', owner=teamowner,
+ members=[member])
+ contact.addLanguage(getUtility(ILanguageSet)['en'])
+ with person_logged_in(contact.teamowner):
+ question.target.addAnswerContact(contact, contact.teamowner)
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ api_request = IWebServiceClientRequest(view.request)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'team',
+ 'display_name': 'Team Name',
+ 'is_team': True,
+ 'can_edit': True,
+ 'web_link': canonical_url(contact),
+ 'self_link': absoluteURL(contact, api_request)
+ }
+ }
+ with person_logged_in(contact.teamowner):
+ self.assertEqual(
+ dumps([expected_result]), view.answercontact_data_js)
+
+ def test_data_target_owner_answercontact_looks(self):
+ # Answercontact_data_js has can_edit set to true for target owner.
+ distro = self.factory.makeDistribution()
+ question = self.factory.makeQuestion(target=distro)
+ contact = self.factory.makePerson(
+ name='user', displayname='Contact Name')
+ contact.addLanguage(getUtility(ILanguageSet)['en'])
+ with person_logged_in(contact):
+ question.target.addAnswerContact(contact, contact)
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ api_request = IWebServiceClientRequest(view.request)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'user',
+ 'display_name': 'Contact Name',
+ 'is_team': False,
+ 'can_edit': True,
+ 'web_link': canonical_url(contact),
+ 'self_link': absoluteURL(contact, api_request)
+ }
+ }
+ with person_logged_in(distro.owner):
+ self.assertEqual(
+ dumps([expected_result]), view.answercontact_data_js)
+
+ def test_data_subscription_lp_admin(self):
+ # For a subscription, answercontact_data_js has can_edit
+ # set to true for a Launchpad admin.
+ question = self.factory.makeQuestion()
+ member = self.factory.makePerson()
+ contact = self.factory.makePerson(
+ name='user', displayname='Contact Name')
+ contact.addLanguage(getUtility(ILanguageSet)['en'])
+ with person_logged_in(member):
+ question.target.addAnswerContact(contact, contact)
+ view = create_view(question.target, '+portlet-answercontacts-details')
+ api_request = IWebServiceClientRequest(view.request)
+
+ expected_result = {
+ 'subscriber': {
+ 'name': 'user',
+ 'display_name': 'Contact Name',
+ 'is_team': False,
+ 'can_edit': True,
+ 'web_link': canonical_url(contact),
+ 'self_link': absoluteURL(contact, api_request)
+ }
+ }
+
+ # Login as admin
+ admin = getUtility(IPersonSet).find(ADMIN_EMAIL).any()
+ with person_logged_in(admin):
+ self.assertEqual(
+ dumps([expected_result]), view.answercontact_data_js)
=== modified file 'lib/lp/answers/configure.zcml'
--- lib/lp/answers/configure.zcml 2011-07-27 13:47:03 +0000
+++ lib/lp/answers/configure.zcml 2011-08-12 04:40:27 +0000
@@ -101,7 +101,7 @@
subscriptions isSubscribed getRecipients
direct_recipients indirect_recipients
getDirectSubscribers getIndirectSubscribers
- getDirectSubscribersWithDetails removeAnswerContact"
+ getDirectSubscribersWithDetails"
/>
<require
permission="launchpad.Owner"
=== modified file 'lib/lp/answers/interfaces/questiontarget.py'
--- lib/lp/answers/interfaces/questiontarget.py 2011-05-21 18:54:42 +0000
+++ lib/lp/answers/interfaces/questiontarget.py 2011-08-12 04:40:27 +0000
@@ -127,6 +127,23 @@
"inherited from other context.)"),
value_type=PublicPersonChoice(vocabulary="ValidPersonOrTeam"))
+ @operation_parameters(
+ person=PublicPersonChoice(
+ title=_('The user or an administered team'), required=True,
+ vocabulary='ValidPersonOrTeam'))
+ @call_with(subscribed_by=REQUEST_USER)
+ @export_read_operation()
+ @operation_for_version('devel')
+ def canUserAlterAnswerContact(person, subscribed_by):
+ """Can the user add or remove the answer contact.
+
+ Users can add or remove themselves or one of the teams they
+ administered. Admins and target owners can add/remove anyone.
+
+ :param person: The `IPerson` that is or will be an answer contact.
+ :param subscribed_by: The `IPerson` making the change.
+ """
+
class IQuestionTargetView(Interface):
"""Methods that logged in user can access."""
@@ -166,23 +183,6 @@
@operation_parameters(
person=PublicPersonChoice(
- title=_('The user or an administered team'), required=True,
- vocabulary='ValidPersonOrTeam'))
- @call_with(subscribed_by=REQUEST_USER)
- @export_read_operation()
- @operation_for_version('devel')
- def canUserAlterAnswerContact(person, subscribed_by):
- """Can the user add or remove the answer contact.
-
- Users can add or remove themselves or one of the teams they
- administered.
-
- :param person: The `IPerson` that is or will be an answer contact.
- :param subscribed_by: The `IPerson` making the change.
- """
-
- @operation_parameters(
- person=PublicPersonChoice(
title=_('The user of an administered team'), required=True,
vocabulary='ValidPersonOrTeam'))
@call_with(subscribed_by=REQUEST_USER)
=== modified file 'lib/lp/answers/model/question.py'
--- lib/lp/answers/model/question.py 2011-08-02 02:54:58 +0000
+++ lib/lp/answers/model/question.py 2011-08-12 04:40:27 +0000
@@ -1351,6 +1351,7 @@
admins = getUtility(ILaunchpadCelebrities).admin
if (person == subscribed_by
or person in subscribed_by.administrated_teams
+ or subscribed_by.inTeam(self.owner)
or subscribed_by.inTeam(admins)):
return True
return False
=== modified file 'lib/lp/answers/tests/test_questiontarget.py'
--- lib/lp/answers/tests/test_questiontarget.py 2011-05-06 03:53:06 +0000
+++ lib/lp/answers/tests/test_questiontarget.py 2011-08-12 04:40:27 +0000
@@ -36,6 +36,12 @@
self.assertTrue(
self.project.canUserAlterAnswerContact(self.user, self.user))
+ def test_canUserAlterAnswerContact_owner(self):
+ login_person(self.user)
+ self.assertTrue(
+ self.project.canUserAlterAnswerContact(
+ self.user, self.project.owner))
+
def test_canUserAlterAnswerContact_other_user(self):
login_person(self.user)
other_user = self.factory.makePerson()
