Merge lp:~wgrant/launchpad/faq-delete into lp:launchpad

[William Grant]

Allow deletion of unlinked FAQs via the webservice, and let ~registry edit and delete any of them.

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.

[Colin Watson (cjwatson)]

Please add something to lib/lp/services/webservice/wadl-to-refhtml.xsl too so that the URLs are documented. faq is reasonably straightforward, and faq_target could go into the "depends on the underlying entry" set.

review: Approve

=== 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-17 07:19:55 +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/doc/faqtarget.txt'
--- lib/lp/answers/doc/faqtarget.txt	2011-12-24 17:49:30 +0000
+++ lib/lp/answers/doc/faqtarget.txt	2015-03-17 07:19:55 +0000
@@ -71,7 +71,9 @@
 The returned object provides the IFAQ interface:
 
     >>> from lp.answers.interfaces.faq import IFAQ
-    >>> verifyObject(IFAQ, faq)
+    >>> from lp.testing import admin_logged_in
+    >>> with admin_logged_in():
+    ...     verifyObject(IFAQ, faq)
     True
 
 The newFAQ() requires an owner, title, and content parameter. It also

=== 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-17 07:19:55 +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-17 07:19:55 +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	2015-03-13 04:07:31 +0000
+++ lib/lp/answers/interfaces/webservice.py	2015-03-17 07:19:55 +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 (
     IQuestionCollection,

=== 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-17 07:19:55 +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-17 07:19:55 +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,48 @@
         nonparticipant = self.factory.makePerson()
         login_person(nonparticipant)
         self.assertCannotEdit(nonparticipant, self.faq)
+
+    def test_registry_can_edit(self):
+        # A member of ~registry can edit any FAQ.
+        expert = self.factory.makePerson(
+            member_of=[getUtility(IPersonSet).getByName('registry')])
+        login_person(expert)
+        self.assertCanEdit(expert, 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-17 07:19:55 +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-17 07:19:55 +0000
@@ -2045,7 +2045,8 @@
 
     def checkAuthenticated(self, user):
         """Allow people with launchpad.Edit or an answer contact."""
-        if EditByOwnersOrAdmins.checkAuthenticated(self, user):
+        if (EditByOwnersOrAdmins.checkAuthenticated(self, user)
+                or user.in_registry_experts):
             return True
         if IQuestionTarget.providedBy(self.obj):
             # Adapt QuestionTargets to FAQTargets to ensure the correct
@@ -2071,6 +2072,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:

=== modified file 'lib/lp/services/webservice/wadl-to-refhtml.xsl'
--- lib/lp/services/webservice/wadl-to-refhtml.xsl	2015-03-05 16:23:26 +0000
+++ lib/lp/services/webservice/wadl-to-refhtml.xsl	2015-03-17 07:19:55 +0000
@@ -168,6 +168,7 @@
                     <xsl:when test="
                         @id = 'bug_link_target'
                         or @id = 'bug_target'
+                        or @id = 'faq_target'
                         or @id = 'git_target'
                         or @id = 'has_bugs'
                         or @id = 'has_milestones'
@@ -310,6 +311,12 @@
                 <xsl:text>/+email/</xsl:text>
                 <var>&lt;email&gt;</var>
             </xsl:when>
+            <xsl:when test="@id = 'faq'">
+                <xsl:text>/</xsl:text>
+                <var>&lt;target.name&gt;</var>
+                <xsl:text>/+faq/</xsl:text>
+                <var >&lt;faq.id&gt;</var>
+            </xsl:when>
             <xsl:when test="@id = 'git_repository'">
                 <xsl:text>/~</xsl:text>
                 <var>&lt;person.name&gt;</var>