launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/faq-delete into lp:launchpad.

Commit message:
Allow deletion of unlinked FAQs via the webservice.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/faq-delete/+merge/253008

Allow deletion of unlinked FAQs via the webservice. There's no UI for this, and questions must be unlinked first, but at least it's possible to get rid of them.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/faq-delete into lp:launchpad.

=== modified file 'lib/lp/answers/configure.zcml'
--- lib/lp/answers/configure.zcml	2014-11-24 06:20:03 +0000
+++ lib/lp/answers/configure.zcml	2015-03-15 23:30:41 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -212,12 +212,16 @@
 
   <class class=".model.faq.FAQ">
     <!-- IFAQ-->
-    <allow interface=".interfaces.faq.IFAQ" />
+    <allow interface=".interfaces.faq.IFAQPublic" />
     <require
         permission="launchpad.Edit"
         set_attributes="title keywords content
                         last_updated_by date_last_updated"
         />
+    <require
+        permission="launchpad.Delete"
+        attributes="destroySelf"
+        />
   </class>
 
   <adapter

=== modified file 'lib/lp/answers/interfaces/faq.py'
--- lib/lp/answers/interfaces/faq.py	2013-01-07 02:40:55 +0000
+++ lib/lp/answers/interfaces/faq.py	2015-03-15 23:30:41 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Interface for FAQ document."""
@@ -10,11 +10,20 @@
     'IFAQSet',
     ]
 
+import httplib
+
+from lazr.restful.declarations import (
+    error_status,
+    export_as_webservice_entry,
+    export_destructor_operation,
+    exported,
+    operation_for_version,
+    )
+from lazr.restful.fields import Reference
 from zope.interface import Attribute
 from zope.schema import (
     Datetime,
     Int,
-    Object,
     Text,
     TextLine,
     )
@@ -29,54 +38,75 @@
     )
 
 
-class IFAQ(IHasOwner):
-    """A document containing the answer to a commonly asked question.
-
-    The answer can be in the document itself or can be hosted on a separate
-    web site and referred to by URL.
-    """
-
-    id = Int(
+@error_status(httplib.BAD_REQUEST)
+class CannotDeleteFAQ(Exception):
+    """The FAQ cannnot be deleted."""
+
+
+class IFAQPublic(IHasOwner):
+
+    id = exported(Int(
         title=_('FAQ Number'),
         description=_('The unique number identifying the FAQ in Launchpad.'),
-        required=True, readonly=True)
+        required=True, readonly=True), as_of='devel')
 
-    title = Title(
+    title = exported(Title(
         title=_('Title'),
         description=_('The title describing this FAQ, often a question.'),
-        required=True)
+        required=True), as_of='devel')
 
-    keywords = TextLine(
+    keywords = exported(TextLine(
         title=_('Keywords'),
         description=_('One or more terms that relate to this FAQ.'),
-        required=False)
+        required=False), as_of='devel')
 
-    content = Text(
+    content = exported(Text(
         title=_('Content'),
         description=_(
             'The answer for this FAQ in plain text. You may choose to '
             'include a URL to an external FAQ.'),
-        required=True)
-
-    date_created = Datetime(title=_('Created'), required=True, readonly=True)
-
-    last_updated_by = PublicPersonChoice(
+        required=True), as_of='devel')
+
+    date_created = exported(Datetime(
+        title=_('Created'), required=True, readonly=True), as_of='devel')
+
+    last_updated_by = exported(PublicPersonChoice(
         title=_('Last Updated By'),
         description=_('The last person who modified the document.'),
-        vocabulary='ValidPersonOrTeam', required=False)
-
-    date_last_updated = Datetime(title=_('Last Updated'), required=False)
-
-    target = Object(
+        vocabulary='ValidPersonOrTeam', required=False, readonly=True),
+        as_of='devel')
+
+    date_last_updated = exported(Datetime(
+        title=_('Last Updated'), required=False, readonly=True),
+        as_of='devel')
+
+    target = exported(Reference(
         title=_('Target'),
         description=_('Product or distribution containing this FAQ.'),
-        schema=IFAQTarget,
-        required=True)
+        schema=IFAQTarget, required=True, readonly=True), as_of='devel')
 
     related_questions = Attribute(
         _('The set of questions linked to this FAQ.'))
 
 
+class IFAQ(IFAQPublic):
+    """A document containing the answer to a commonly asked question.
+
+    The answer can be in the document itself or can be hosted on a separate
+    web site and referred to by URL.
+    """
+
+    export_as_webservice_entry('faq', as_of='beta')
+
+    @export_destructor_operation()
+    @operation_for_version('devel')
+    def destroySelf():
+        """Delete this FAQ.
+
+        Any questions linked to this FAQ must be unlinked beforehand.
+        """
+
+
 class IFAQSet(IFAQCollection):
     """`IFAQCollection` of all the FAQs existing in Launchpad.
 

=== modified file 'lib/lp/answers/interfaces/faqtarget.py'
--- lib/lp/answers/interfaces/faqtarget.py	2013-01-07 02:40:55 +0000
+++ lib/lp/answers/interfaces/faqtarget.py	2015-03-15 23:30:41 +0000
@@ -10,12 +10,16 @@
     ]
 
 
+from lazr.restful.declarations import export_as_webservice_entry
+
 from lp.answers.interfaces.faqcollection import IFAQCollection
 
 
 class IFAQTarget(IFAQCollection):
     """An object that can contain a FAQ document."""
 
+    export_as_webservice_entry('faq_target', as_of='beta')
+
     def newFAQ(owner, title, content, keywords=None, date_created=None):
         """Create a new FAQ hosted in this target.
 

=== modified file 'lib/lp/answers/interfaces/webservice.py'
--- lib/lp/answers/interfaces/webservice.py	2012-01-01 02:58:52 +0000
+++ lib/lp/answers/interfaces/webservice.py	2015-03-15 23:30:41 +0000
@@ -10,6 +10,8 @@
 """
 
 __all__ = [
+    'IFAQ',
+    'IFAQTarget',
     'IQuestion',
     'IQuestionSet',
     'IQuestionSubscription',
@@ -17,6 +19,8 @@
 
 from lazr.restful.declarations import LAZR_WEBSERVICE_EXPORTED
 
+from lp.answers.interfaces.faq import IFAQ
+from lp.answers.interfaces.faqtarget import IFAQTarget
 from lp.answers.interfaces.question import IQuestion
 from lp.answers.interfaces.questioncollection import (
     IQuestionSet,

=== modified file 'lib/lp/answers/model/faq.py'
--- lib/lp/answers/model/faq.py	2015-01-29 10:39:32 +0000
+++ lib/lp/answers/model/faq.py	2015-03-15 23:30:41 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """FAQ document models."""
@@ -23,6 +23,7 @@
 from zope.interface import implements
 
 from lp.answers.interfaces.faq import (
+    CannotDeleteFAQ,
     IFAQ,
     IFAQSet,
     )
@@ -93,6 +94,12 @@
         else:
             return self.distribution
 
+    def destroySelf(self):
+        if self.related_questions:
+            raise CannotDeleteFAQ(
+               "Cannot delete FAQ: questions must be unlinked first.")
+        super(FAQ, self).destroySelf()
+
     @staticmethod
     def new(owner, title, content, keywords=keywords, date_created=None,
             product=None, distribution=None):

=== modified file 'lib/lp/answers/tests/test_faq.py'
--- lib/lp/answers/tests/test_faq.py	2012-01-01 02:58:52 +0000
+++ lib/lp/answers/tests/test_faq.py	2015-03-15 23:30:41 +0000
@@ -1,15 +1,21 @@
-# Copyright 2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2015 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for IFAQ"""
 
 __metaclass__ = type
 
+import transaction
 from zope.component import getUtility
 
+from lp.answers.interfaces.faq import CannotDeleteFAQ
+from lp.answers.model.faq import FAQ
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.database.interfaces import IStore
 from lp.services.webapp.authorization import check_permission
 from lp.services.worlddata.interfaces.language import ILanguageSet
 from lp.testing import (
+    admin_logged_in,
     login_person,
     person_logged_in,
     TestCaseWithFactory,
@@ -75,3 +81,41 @@
         nonparticipant = self.factory.makePerson()
         login_person(nonparticipant)
         self.assertCannotEdit(nonparticipant, self.faq)
+
+    def test_direct_answer_contact_cannot_delete(self):
+        # Answer contacts are broad, and deletion is irreversible, so
+        # they cannot do it themselves.
+        direct_answer_contact = self.factory.makePerson()
+        with person_logged_in(direct_answer_contact):
+            self.addAnswerContact(direct_answer_contact)
+            self.assertFalse(check_permission('launchpad.Delete', self.faq))
+
+    def test_registry_can_delete(self):
+        # A member of ~registry can delete any FAQ.
+        expert = self.factory.makePerson(
+            member_of=[getUtility(IPersonSet).getByName('registry')])
+        with person_logged_in(expert):
+            self.assertTrue(check_permission('launchpad.Delete', self.faq))
+
+
+class TestFAQ(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_destroySelf(self):
+        # An FAQ can be deleted.
+        with admin_logged_in():
+            faq = self.factory.makeFAQ()
+            faq.destroySelf()
+            transaction.commit()
+            self.assertIs(None, IStore(FAQ).get(FAQ, faq.id))
+
+    def test_destroySelf_rejected_if_questions_linked(self):
+        # Questions must be unlinked before a FAQ is deleted.
+        with admin_logged_in():
+            faq = self.factory.makeFAQ()
+            question = self.factory.makeQuestion()
+            question.linkFAQ(self.factory.makePerson(), faq, "foo")
+            self.assertRaises(CannotDeleteFAQ, faq.destroySelf)
+            transaction.commit()
+            self.assertEqual(faq, IStore(FAQ).get(FAQ, faq.id))

=== added file 'lib/lp/answers/tests/test_faq_webservice.py'
--- lib/lp/answers/tests/test_faq_webservice.py	1970-01-01 00:00:00 +0000
+++ lib/lp/answers/tests/test_faq_webservice.py	2015-03-15 23:30:41 +0000
@@ -0,0 +1,68 @@
+# Copyright 2015 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from lazr.lifecycle.event import ObjectModifiedEvent
+from testtools.matchers import (
+    Contains,
+    ContainsDict,
+    Equals,
+    MatchesRegex,
+    )
+from zope.component import getUtility
+from zope.event import notify
+
+from lp.registry.interfaces.person import IPersonSet
+from lp.services.webapp.interfaces import OAuthPermission
+from lp.testing import (
+    admin_logged_in,
+    api_url,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import DatabaseFunctionalLayer
+from lp.testing.pages import webservice_for_person
+
+
+class TestFAQWebService(TestCaseWithFactory):
+
+    layer = DatabaseFunctionalLayer
+
+    def test_representation(self):
+        with admin_logged_in():
+            faq = self.factory.makeFAQ(title="Nothing works")
+            faq.keywords = "foo bar"
+            faq.content = "It is all broken."
+            notify(ObjectModifiedEvent(
+                faq, faq, ['keywords', 'content'], user=faq.owner))
+            faq_url = api_url(faq)
+        webservice = webservice_for_person(self.factory.makePerson())
+        repr = webservice.get(faq_url, api_version='devel').jsonBody()
+        with admin_logged_in():
+            self.assertThat(
+                repr,
+                ContainsDict({
+                    "id": Equals(faq.id),
+                    "title": Equals("Nothing works"),
+                    "keywords": Equals("foo bar"),
+                    "content": Equals("It is all broken."),
+                    "date_created": MatchesRegex("\d\d\d\d-\d\d-\d\dT.*"),
+                    "date_last_updated": MatchesRegex("\d\d\d\d-\d\d-\d\dT.*"),
+                    "last_updated_by_link": Contains(
+                        "/devel/~%s" % faq.owner.name),
+                    "target_link": Contains(
+                        "/devel/%s" % faq.target.name),
+                    }))
+
+    def test_delete(self):
+        with admin_logged_in():
+            faq = self.factory.makeFAQ()
+            faq_url = api_url(faq)
+            expert = self.factory.makePerson(
+                member_of=[getUtility(IPersonSet).getByName('registry')])
+        webservice = webservice_for_person(
+            expert, permission=OAuthPermission.WRITE_PRIVATE)
+        response = webservice.delete(faq_url, api_version='devel')
+        self.assertEqual(200, response.status)
+        response = webservice.get(faq_url, api_version='devel')
+        self.assertEqual(404, response.status)

=== modified file 'lib/lp/security.py'
--- lib/lp/security.py	2015-03-06 10:22:08 +0000
+++ lib/lp/security.py	2015-03-15 23:30:41 +0000
@@ -2071,6 +2071,14 @@
         return AppendFAQTarget(self.obj.target).checkAuthenticated(user)
 
 
+class DeleteFAQ(AuthorizationBase):
+    permission = 'launchpad.Delete'
+    usedfor = IFAQ
+
+    def checkAuthenticated(self, user):
+        return user.in_registry_experts or user.in_admin
+
+
 def can_edit_team(team, user):
     """Return True if the given user has edit rights for the given team."""
     if user.in_admin: