launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging lp:~cjwatson/launchpad/limit-faq-editing into lp:launchpad.

Commit message:
Prevent answer contacts from editing FAQs.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/limit-faq-editing/+merge/303658

Prevent answer contacts from editing FAQs.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/limit-faq-editing into lp:launchpad.

=== modified file 'lib/lp/answers/stories/faq-edit.txt'
--- lib/lp/answers/stories/faq-edit.txt	2016-01-26 15:47:37 +0000
+++ lib/lp/answers/stories/faq-edit.txt	2016-08-23 08:24:12 +0000
@@ -4,9 +4,8 @@
 FAQ. To do this, the user goes to the FAQ that they want to modify and
 clicks the 'Edit FAQ' action.
 
-That action is only available to project owners and answer contacts.
-That's why the link doesn't appear for the anonymous user nor
-No Privileges Person:
+That action is only available to project owners. That's why the link doesn't
+appear for the anonymous user nor No Privileges Person:
 
     >>> from lp.services.helpers import backslashreplace
     >>> anon_browser.open('http://answers.launchpad.dev/firefox/+faq/7')

=== modified file 'lib/lp/answers/stories/question-edit.txt'
--- lib/lp/answers/stories/question-edit.txt	2015-12-01 05:26:11 +0000
+++ lib/lp/answers/stories/question-edit.txt	2016-08-23 08:24:12 +0000
@@ -2,8 +2,8 @@
 
 To edit the title and description of question, one uses the 'Edit
 Question' menu item. You need to be logged in to see the edit form, and
-only the question creator or an answer contact can change the title and
-description.
+only the question creator or an owner of the question target can change the
+title and description.
 
     >>> anon_browser.open('http://launchpad.dev/firefox/+question/2')
     >>> anon_browser.getLink('Edit question').click()

=== modified file 'lib/lp/answers/tests/test_faq.py'
--- lib/lp/answers/tests/test_faq.py	2015-03-16 00:04:39 +0000
+++ lib/lp/answers/tests/test_faq.py	2016-08-23 08:24:12 +0000
@@ -56,16 +56,16 @@
         login_person(self.owner)
         self.assertCanEdit(self.owner, self.faq)
 
-    def test_direct_answer_contact_can_edit(self):
-        # A direct answer contact for an FAQ target can edit its FAQs.
+    def test_direct_answer_contact_cannot_edit(self):
+        # A direct answer contact for an FAQ target cannot edit its FAQs.
         direct_answer_contact = self.factory.makePerson()
         login_person(direct_answer_contact)
         self.addAnswerContact(direct_answer_contact)
-        self.assertCanEdit(direct_answer_contact, self.faq)
+        self.assertCannotEdit(direct_answer_contact, self.faq)
 
-    def test_indirect_answer_contact_can_edit(self):
+    def test_indirect_answer_contact_cannot_edit(self):
         # A indirect answer contact (a member of a team that is an answer
-        # contact) for an FAQ target can edit its FAQs.
+        # contact) for an FAQ target cannot edit its FAQs.
         indirect_answer_contact = self.factory.makePerson()
         direct_answer_contact = self.factory.makeTeam()
         with person_logged_in(direct_answer_contact.teamowner):
@@ -73,11 +73,10 @@
                 indirect_answer_contact, direct_answer_contact.teamowner)
             self.addAnswerContact(direct_answer_contact)
         login_person(indirect_answer_contact)
-        self.assertCanEdit(indirect_answer_contact, self.faq)
+        self.assertCannotEdit(indirect_answer_contact, self.faq)
 
     def test_nonparticipating_user_cannot_edit(self):
-        # A user that is neither an owner of, or answer contact for, an
-        # FAQ target's cannot edit a its FAQs.
+        # A user that is not an owner of an FAQ target cannot edit its FAQs.
         nonparticipant = self.factory.makePerson()
         login_person(nonparticipant)
         self.assertCannotEdit(nonparticipant, self.faq)

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2016-08-23 03:49:28 +0000
+++ lib/lp/security.py	2016-08-23 08:24:12 +0000
@@ -2093,9 +2093,10 @@
     usedfor = IFAQ
 
     def checkAuthenticated(self, user):
-        """Everybody who has launchpad.Append on the FAQ target is allowed.
-        """
-        return AppendFAQTarget(self.obj.target).checkAuthenticated(user)
+        """Allow only admins and owners of the FAQ target."""
+        return (
+            user.in_admin or user.in_registry_experts or
+            user.inTeam(self.obj.target.owner))
 
 
 class DeleteFAQ(AuthorizationBase):