Merge lp:~danilo/launchpad/kill-feedback-requests into lp:launchpad

[Данило Шеган]

Remove all code related to 'Feedback requests' on blueprint pages.

= Bug 1000642: kill 'request feedback' =

'Request feedback' feature (available from individual blueprint pages) has never been complete nor it did what users expected. As the major problem, it never emailed anything (neither when feedback was requested, nor when it was received).

It was also hard to find (feedback requests for you are available only from your blueprints homepage: https://blueprints.launchpad.net/people/+me/+specfeedback

== Proposed fix ==

Francis proposed killing the feature altogether: it never did what people expected it to do, and it's very much incomplete. There will be no resources to complete it, so it's better to remove it altogether.

== Pre-implementation notes ==

== LOC Rationale ==

This is being done by us to help us keep our relative LOC while working on the workitems project a net negative.

This is all strictly code removal.

== Tests ==

Full test suite.

== Demo and Q/A ==

- Browse to a blueprint page: no 'Feedback' section should appear there (before the whiteboard)
- Browse to https://blueprints.launchpad.net/people/+me: no link to 'Feedback requests' in the menu on the right side should be there
- https://blueprints.launchpad.net/people/+me/+specfeedback should not work

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/blueprints/configure.zcml
  lib/lp/blueprints/enums.py
  lib/lp/blueprints/browser/configure.zcml
  lib/lp/blueprints/browser/specification.py
  lib/lp/blueprints/browser/specificationtarget.py
  lib/lp/blueprints/interfaces/specification.py
  lib/lp/blueprints/model/specification.py
  lib/lp/blueprints/stories/standalone/xx-personviews.txt
  lib/lp/blueprints/templates/specification-index.pt
  lib/lp/blueprints/templates/specification-listing-detailed.pt
  lib/lp/registry/browser/configure.zcml
  lib/lp/registry/browser/person.py
  lib/lp/registry/doc/person.txt
  lib/lp/registry/model/person.py

[Данило Шеган (danilo)]

As William suggested, this will have to wait for https://code.launchpad.net/~danilo/launchpad/bug-1000642-db/+merge/106343 to land and be rolled out.

[Jelmer Vernooij (jelmer)]

Another bit of dead code gone, thanks! :-)

The changes look reasonable and complete to me.

review: Approve

=== modified file 'lib/lp/blueprints/browser/configure.zcml'
--- lib/lp/blueprints/browser/configure.zcml	2012-03-28 18:43:36 +0000
+++ lib/lp/blueprints/browser/configure.zcml	2012-06-12 12:14:35 +0000
@@ -274,9 +274,6 @@
             <browser:page
                 name="+portlet-blocked"
                 template="../templates/specification-portlet-blocked.pt"/>
-            <browser:page
-                name="+portlet-feedbackqueue"
-                template="../templates/specification-portlet-feedbackqueue.pt"/>
         </browser:pages>
         <browser:page
             for="lp.blueprints.interfaces.specification.ISpecification"
@@ -314,9 +311,6 @@
             <browser:page
                 name="+listing-detailed"
                 template="../templates/specification-listing-detailed.pt"/>
-            <browser:page
-                name="+listing-feedback"
-                template="../templates/specification-listing-feedback.pt"/>
         </browser:pages>
         <browser:page
             for="lp.blueprints.interfaces.specification.ISpecification"
@@ -337,12 +331,6 @@
             permission="launchpad.Edit"
             template="../templates/specification-subscription-delete.pt"/>
         <browser:page
-            for="lp.blueprints.interfaces.specification.ISpecification"
-            name="+givefeedback"
-            class="lp.blueprints.browser.specificationfeedback.SpecificationFeedbackClearingView"
-            permission="launchpad.AnyPerson"
-            template="../templates/specification-givefeedback.pt"/>
-        <browser:page
             name="+addsubscriber"
             for="lp.blueprints.interfaces.specification.ISpecification"
             class="lp.blueprints.browser.specificationsubscription.SpecificationSubscriptionAddSubscriberView"
@@ -424,13 +412,6 @@
             permission="launchpad.Edit"
             template="../templates/specification-edit.pt"/>
         <browser:page
-            name="+requestfeedback"
-            for="lp.blueprints.interfaces.specification.ISpecification"
-            class="lp.blueprints.browser.specificationfeedback.SpecificationFeedbackAddView"
-            permission="launchpad.AnyPerson"
-            template="../templates/specification-requestfeedback.pt">
-        </browser:page>
-        <browser:page
             name="+linkbug"
             for="lp.blueprints.interfaces.specification.ISpecification"
             class="lp.bugs.browser.buglinktarget.BugLinkView"

=== modified file 'lib/lp/blueprints/browser/specification.py'
--- lib/lp/blueprints/browser/specification.py	2012-03-26 14:23:39 +0000
+++ lib/lp/blueprints/browser/specification.py	2012-06-12 12:14:35 +0000
@@ -418,19 +418,13 @@
     usedfor = ISpecification
     links = ['edit', 'people', 'status', 'priority',
              'whiteboard', 'proposegoal', 'workitems',
-             'milestone', 'requestfeedback', 'givefeedback', 'subscription',
+             'milestone', 'subscription',
              'addsubscriber',
              'linkbug', 'unlinkbug', 'linkbranch',
              'adddependency', 'removedependency',
              'dependencytree', 'linksprint', 'supersede',
              'retarget']
 
-    def givefeedback(self):
-        text = 'Give feedback'
-        enabled = (self.user is not None and
-                   bool(self.context.getFeedbackRequests(self.user)))
-        return Link('+givefeedback', text, icon='edit', enabled=enabled)
-
     @enabled_with_permission('launchpad.Edit')
     def milestone(self):
         text = 'Target milestone'
@@ -447,11 +441,6 @@
         return Link('+priority', text, icon='edit')
 
     @enabled_with_permission('launchpad.AnyPerson')
-    def requestfeedback(self):
-        text = 'Request feedback'
-        return Link('+requestfeedback', text, icon='add')
-
-    @enabled_with_permission('launchpad.AnyPerson')
     def proposegoal(self):
         text = 'Propose as goal'
         if self.context.goal is not None:
@@ -544,12 +533,6 @@
     """Used to render portlets and listing items that need browser code."""
 
     @cachedproperty
-    def feedbackrequests(self):
-        if self.user is None:
-            return []
-        return list(self.context.getFeedbackRequests(self.user))
-
-    @cachedproperty
     def has_dep_tree(self):
         return self.context.dependencies or self.context.blocked_specs
 
@@ -585,11 +568,6 @@
         if not self.user:
             return
 
-        if self.feedbackrequests:
-            msg = "You have %d feedback request(s) on this blueprint."
-            msg %= len(self.feedbackrequests)
-            self.notices.append(msg)
-
     @property
     def approver_widget(self):
         return InlinePersonEditPickerWidget(

=== removed file 'lib/lp/blueprints/browser/specificationfeedback.py'
--- lib/lp/blueprints/browser/specificationfeedback.py	2012-01-01 02:58:52 +0000
+++ lib/lp/blueprints/browser/specificationfeedback.py	1970-01-01 00:00:00 +0000
@@ -1,116 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Views for SpecificationFeedback."""
-
-__metaclass__ = type
-
-from zope.app.form.browser import TextAreaWidget
-from zope.component import getUtility
-from zope.interface import Interface
-
-from lp import _
-from lp.app.browser.launchpadform import (
-    action,
-    custom_widget,
-    LaunchpadFormView,
-    )
-from lp.blueprints.interfaces.specificationfeedback import (
-    ISpecificationFeedback,
-    )
-from lp.registry.interfaces.person import IPersonSet
-from lp.services.helpers import english_list
-from lp.services.webapp import canonical_url
-
-
-__all__ = [
-    'SpecificationFeedbackAddView',
-    'SpecificationFeedbackClearingView',
-    ]
-
-
-class SpecificationFeedbackAddView(LaunchpadFormView):
-
-    schema = ISpecificationFeedback
-
-    field_names = [
-        'reviewer', 'queuemsg',
-        ]
-
-    custom_widget('queuemsg', TextAreaWidget, height=5)
-
-    @property
-    def label(self):
-        return "Request feedback on specification"
-
-    @property
-    def page_title(self):
-        return self.label
-
-    def validate(self, data):
-        reviewer = data.get('reviewer')
-        requester = self.user
-        for request in self.context.getFeedbackRequests(reviewer):
-            if request.requester == requester:
-                self.addError("You've already requested feedback from %s"
-                    % reviewer.displayname)
-        if reviewer == requester:
-            self.addError("You can't request feedback from yourself")
-
-    @action(_("Add"), name="create")
-    def create_action(self, action, data):
-        reviewer = data.get('reviewer')
-        requester = self.user
-        queuemsg = data.get('queuemsg')
-        return self.context.queue(reviewer, requester, queuemsg)
-
-    @property
-    def next_url(self):
-        return canonical_url(self.context)
-
-    @property
-    def cancel_url(self):
-        return self.next_url
-
-
-class SpecificationFeedbackClearingView(LaunchpadFormView):
-
-    schema = Interface
-    field_names = []
-
-    @property
-    def label(self):
-        return _('Give feedback on this blueprint')
-
-    @property
-    def requests(self):
-        """Return the feedback requests made of this user."""
-        if self.user is None:
-            return None
-        return self.context.getFeedbackRequests(self.user)
-
-    @action(_('Save changes'), name='save')
-    def save_action(self, action, data):
-        names = self.request.form_ng.getAll('name')
-        if len(names) == 0:
-            self.request.response.addNotification(
-                'Please select feedback queue items to clear.')
-        else:
-            cleared_from = []
-            for name in names:
-                requester = getUtility(IPersonSet).getByName(name)
-                if requester is not None:
-                    self.context.unqueue(self.user, requester)
-                    cleared_from.append(requester.displayname)
-            self.request.response.addNotification(
-                'Cleared requests from: %s' % english_list(cleared_from))
-
-    @property
-    def next_url(self):
-        if self.context.getFeedbackRequests(self.user).count() == 0:
-            # No more queue items to process; return to the spec.
-            return canonical_url(self.context)
-
-    @property
-    def cancel_url(self):
-        return canonical_url(self.context)

=== modified file 'lib/lp/blueprints/browser/specificationtarget.py'
--- lib/lp/blueprints/browser/specificationtarget.py	2012-02-17 04:09:06 +0000
+++ lib/lp/blueprints/browser/specificationtarget.py	2012-06-12 12:14:35 +0000
@@ -328,8 +328,6 @@
             filter.append(SpecificationFilter.DRAFTER)
         elif role == 'approver':
             filter.append(SpecificationFilter.APPROVER)
-        elif role == 'feedback':
-            filter.append(SpecificationFilter.FEEDBACK)
         elif role == 'subscriber':
             filter.append(SpecificationFilter.SUBSCRIBER)
 

=== modified file 'lib/lp/blueprints/configure.zcml'
--- lib/lp/blueprints/configure.zcml	2012-02-22 21:34:15 +0000
+++ lib/lp/blueprints/configure.zcml	2012-06-12 12:14:35 +0000
@@ -115,15 +115,6 @@
            lazr.lifecycle.interfaces.IObjectModifiedEvent"
       handler="lp.blueprints.mail.notifications.notify_specification_subscription_modified"/>
 
-  <!-- SpecificationFeedback -->
-
-  <class class="lp.blueprints.model.specificationfeedback.SpecificationFeedback">
-    <allow interface="lp.blueprints.interfaces.specificationfeedback.ISpecificationFeedback"/>
-    <require
-        permission="zope.Public"
-        set_schema="lp.blueprints.interfaces.specificationfeedback.ISpecificationFeedback"/>
-  </class>
-
   <!-- SprintAttendance -->
 
   <class class="lp.blueprints.model.sprintattendance.SprintAttendance">

=== modified file 'lib/lp/blueprints/enums.py'
--- lib/lp/blueprints/enums.py	2012-02-15 21:36:02 +0000
+++ lib/lp/blueprints/enums.py	2012-06-12 12:14:35 +0000
@@ -334,13 +334,6 @@
         to which the person has subscribed.
         """)
 
-    FEEDBACK = DBItem(110, """
-        Feedback
-
-        This indicates that the list should include all the specifications
-        which the person has been asked to provide specific feedback on.
-        """)
-
 
 class SpecificationSort(EnumeratedType):
     """The scheme to sort the results of a specifications query.

=== modified file 'lib/lp/blueprints/interfaces/specification.py'
--- lib/lp/blueprints/interfaces/specification.py	2012-04-03 22:12:29 +0000
+++ lib/lp/blueprints/interfaces/specification.py	2012-06-12 12:14:35 +0000
@@ -379,7 +379,6 @@
     subscribers = Attribute('The set of subscribers to this spec.')
     sprints = Attribute('The sprints at which this spec is discussed.')
     sprint_links = Attribute('The entries that link this spec to sprints.')
-    feedbackrequests = Attribute('The set of feedback requests queued.')
     dependencies = exported(
         CollectionField(
             title=_('Specs on which this one depends.'),
@@ -442,11 +441,6 @@
     def getSprintSpecification(sprintname):
         """Get the record that links this spec to the named sprint."""
 
-    def getFeedbackRequests(person):
-        """Return the requests for feedback for a given person on this
-        specification.
-        """
-
     def notificationRecipientAddresses():
         """Return the list of email addresses that receive notifications."""
 
@@ -523,16 +517,6 @@
         If person is None, the return value is always False.
         """
 
-    # queue-related methods
-    def queue(provider, requester, queuemsg=None):
-        """Put this specification into the feedback queue of the given person,
-        with an optional message."""
-
-    def unqueue(provider, requester):
-        """Remove the feedback request by the requester for this spec, from
-        the provider's feedback queue.
-        """
-
     # sprints
     def linkSprint(sprint, user):
         """Put this spec on the agenda of the sprint."""

=== removed file 'lib/lp/blueprints/interfaces/specificationfeedback.py'
--- lib/lp/blueprints/interfaces/specificationfeedback.py	2011-12-24 16:54:44 +0000
+++ lib/lp/blueprints/interfaces/specificationfeedback.py	1970-01-01 00:00:00 +0000
@@ -1,45 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# pylint: disable-msg=E0211,E0213
-
-"""Specification queueing interfaces. It is possible to put a specification
-into somebody's queue, along with a message telling that person what they
-are supposed to do with that spec."""
-
-__metaclass__ = type
-
-__all__ = [
-    'ISpecificationFeedback',
-    ]
-
-from zope.interface import Interface
-from zope.schema import (
-    Int,
-    Text,
-    )
-
-from lp import _
-from lp.services.fields import PublicPersonChoice
-
-
-class ISpecificationFeedback(Interface):
-    """The queue entry for a specification on a person, including a message
-    from the person who put it in their queue."""
-
-    reviewer = PublicPersonChoice(
-        title=_('Feedback From'), required=True,
-        vocabulary='ValidPersonOrTeam', readonly=False,
-        description=_("Select the person who you would like to give you "
-        "some feedback on this specification."))
-    requester = PublicPersonChoice(
-        title=_("The person who requested this feedback."),
-        vocabulary='ValidPersonOrTeam', required=True)
-    specification = Int(title=_('Specification ID'), required=True,
-        readonly=True)
-    queuemsg = Text(title=_("Message"), required=False,
-        description=_("A brief message for the person that you are "
-        "asking to look at this spec. Tell them why you are putting this "
-        "specification in their queue."))
-
-

=== modified file 'lib/lp/blueprints/model/specification.py'
--- lib/lp/blueprints/model/specification.py	2012-03-26 14:23:39 +0000
+++ lib/lp/blueprints/model/specification.py	2012-06-12 12:14:35 +0000
@@ -57,7 +57,6 @@
 from lp.blueprints.model.specificationdependency import (
     SpecificationDependency,
     )
-from lp.blueprints.model.specificationfeedback import SpecificationFeedback
 from lp.blueprints.model.specificationsubscription import (
     SpecificationSubscription,
     )
@@ -191,8 +190,6 @@
         joinColumn='specification', otherColumn='person',
         intermediateTable='SpecificationSubscription',
         orderBy=['displayname', 'name'])
-    feedbackrequests = SQLMultipleJoin('SpecificationFeedback',
-        joinColumn='specification', orderBy='id')
     sprint_links = SQLMultipleJoin('SprintSpecification', orderBy='id',
         joinColumn='specification')
     sprints = SQLRelatedJoin('Sprint', orderBy='name',
@@ -435,12 +432,6 @@
                 return sprintspecification
         return None
 
-    def getFeedbackRequests(self, person):
-        """See ISpecification."""
-        fb = SpecificationFeedback.selectBy(
-            specification=self, reviewer=person)
-        return fb.prejoin(['requester'])
-
     def notificationRecipientAddresses(self):
         """See ISpecification."""
         related_people = [
@@ -699,32 +690,6 @@
 
         return bool(self.subscription(person))
 
-    # queueing
-    def queue(self, reviewer, requester, queuemsg=None):
-        """See ISpecification."""
-        for fbreq in self.feedbackrequests:
-            if (fbreq.reviewer.id == reviewer.id and
-                fbreq.requester == requester.id):
-                # we have a relevant request already, update it
-                fbreq.queuemsg = queuemsg
-                return fbreq
-        # since no previous feedback request existed for this person,
-        # create a new one
-        return SpecificationFeedback(
-            specification=self,
-            reviewer=reviewer,
-            requester=requester,
-            queuemsg=queuemsg)
-
-    def unqueue(self, reviewer, requester):
-        """See ISpecification."""
-        # see if a relevant queue entry exists, and if so, delete it
-        for fbreq in self.feedbackrequests:
-            if (fbreq.reviewer.id == reviewer.id and
-                fbreq.requester.id == requester.id):
-                SpecificationFeedback.delete(fbreq.id)
-                return
-
     # Template methods for BugLinkTargetMixin
     buglinkClass = SpecificationBug
 

=== removed file 'lib/lp/blueprints/model/specificationfeedback.py'
--- lib/lp/blueprints/model/specificationfeedback.py	2011-12-30 06:14:56 +0000
+++ lib/lp/blueprints/model/specificationfeedback.py	1970-01-01 00:00:00 +0000
@@ -1,39 +0,0 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-# pylint: disable-msg=E0611,W0212
-
-__metaclass__ = type
-
-__all__ = ['SpecificationFeedback']
-
-from sqlobject import (
-    ForeignKey,
-    StringCol,
-    )
-from zope.interface import implements
-
-from lp.blueprints.interfaces.specificationfeedback import (
-    ISpecificationFeedback,
-    )
-from lp.registry.interfaces.person import validate_public_person
-from lp.services.database.sqlbase import SQLBase
-
-
-class SpecificationFeedback(SQLBase):
-    """A subscription for person to a spec."""
-
-    implements(ISpecificationFeedback)
-
-    _table = 'SpecificationFeedback'
-    specification = ForeignKey(dbName='specification',
-        foreignKey='Specification', notNull=True)
-    reviewer = ForeignKey(
-        dbName='reviewer', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True)
-    requester = ForeignKey(
-        dbName='requester', foreignKey='Person',
-        storm_validator=validate_public_person, notNull=True)
-    queuemsg = StringCol(notNull=False, default=None)
-
-

=== removed file 'lib/lp/blueprints/stories/blueprints/xx-reviews.txt'
--- lib/lp/blueprints/stories/blueprints/xx-reviews.txt	2011-03-04 19:21:54 +0000
+++ lib/lp/blueprints/stories/blueprints/xx-reviews.txt	1970-01-01 00:00:00 +0000
@@ -1,150 +0,0 @@
-
-Specification Reviews
-=====================
-
-Now, we'll ask someone to review the spec. Let's ask Carlos! We'll pass
-him a message.
-
-First, we load the review request page.
-
-    >>> browser.addHeader('Authorization', 'Basic [e-mail]:test')
-    >>> browser.open(
-    ...     'http://blueprints.launchpad.dev/firefox/+spec/canvas')
-    >>> browser.getLink('Request feedback').click()
-    >>> browser.url
-    'http://blueprints.launchpad.dev/firefox/+spec/canvas/+requestfeedback'
-
-This page contains a cancellation link back to the blueprint page, in case you
-change your mind.
-
-    >>> print browser.getLink('Cancel').url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas
-
-Now we try to POST to the form behind it. We expect to be redirected to the
-spec home page.
-
-    >>> browser.getControl(name="field.reviewer").value = "mark"
-    >>> browser.getControl(
-    ...     name="field.queuemsg").value = "specification, review thyself"
-    >>> browser.getControl("Add").click()
-    >>> print browser.url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas
-
-If we try to request feedback to the same person to the same specification,
-we'll get a nice error message
-
-    >>> browser.open(
-    ...     'http://blueprints.launchpad.dev/firefox/+spec/canvas/'
-    ...     '+requestfeedback')
-    >>> browser.getControl(name="field.reviewer").value = "mark"
-    >>> browser.getControl(
-    ...     name="field.queuemsg").value = "specification, review thyself"
-    >>> browser.getControl("Add").click()
-    >>> print browser.url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas/+requestfeedback
-
-    >>> for error in get_feedback_messages(browser.contents):
-    ...     print error
-    There is 1 error.
-    You've already requested feedback from Mark Shuttleworth
-
-I can't request feedback from myself.
-
-    >>> browser.getControl(name="field.reviewer").value = "name12"
-    >>> browser.getControl(
-    ...     name="field.queuemsg").value = "test spec request"
-    >>> browser.getControl("Add").click()
-    >>> print browser.url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas/+requestfeedback
-    >>> for error in get_feedback_messages(browser.contents):
-    ...     print error
-    There is 1 error.
-    You can't request feedback from yourself
-
-We will see that review request message on the spec page.
-
-    >>> browser.open('http://blueprints.launchpad.dev/firefox/+spec/canvas')
-    >>> print extract_text(
-    ...     find_tag_by_id(browser.contents, 'feedback-requests'))
-    Feedback requests
-    Sample Person requested feedback from Mark Shuttleworth:
-        specification, review thyself
-    ...
-
-We will do another 2 feedback requests for mark with different logged in
-users, first with Carlos.
-
-    >>> carlos_browser = setupBrowser(auth='Basic [e-mail]:test')
-    >>> carlos_browser.open(
-    ...     'http://blueprints.launchpad.dev/firefox/+spec/canvas/'
-    ...     '+requestfeedback')
-    >>> carlos_browser.getControl(name="field.reviewer").value = "mark"
-    >>> carlos_browser.getControl(
-    ...     name="field.queuemsg").value = (
-    ...     "do we really want to maintain compatibility with windows?")
-    >>> carlos_browser.getControl("Add").click()
-
-And then with Foo Bar
-
-    >>> admin_browser.open(
-    ...     'http://blueprints.launchpad.dev/firefox/+spec/canvas/'
-    ...     '+requestfeedback')
-    >>> admin_browser.getControl(name="field.reviewer").value = "mark"
-    >>> admin_browser.getControl(
-    ...     name="field.queuemsg").value = (
-    ...     "please check my grammer and speling")
-    >>> admin_browser.getControl("Add").click()
-
-Now, let's go and clear a review request. Mark has been asked to review spec
-"canvas". Let's view the page, and tell it that we have completed the review.
-First, on the spec home page we see an alert reminding us that we have been
-asked to review this page in particular.
-
-    >>> mark_browser = setupBrowser(auth='Basic [e-mail]:test')
-    >>> mark_browser.open(
-    ...     'http://blueprints.launchpad.dev/firefox/+spec/canvas/')
-    >>> for message in get_feedback_messages(mark_browser.contents):
-    ...     print message
-    You have 3 feedback request(s)...
-
-We also see our name in the "feedback requests" portlet.
-
-    >>> print extract_text(
-    ...     find_tag_by_id(browser.contents, 'feedback-requests'))
-    Feedback requests
-    ...
-    Mark Shuttleworth:
-    ...
-
-And we see the "Give Feedback" menu item.
-
-    >>> give_feedback = mark_browser.getLink("Give feedback")
-
-Let's load the "+givefeedback" page, to tell the system that our work is
-complete. Note that the form posts back to the main spec home page.
-
-    >>> give_feedback.click()
-    >>> print mark_browser.url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas/+givefeedback
-
-Now we POST this page back to itself. First, we clear a single item.
-
-    >>> mark_browser.getControl("Carlos Perelló Marín").selected = True
-    >>> mark_browser.getControl("Save changes").click()
-    >>> print mark_browser.url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas/+givefeedback
-
-    >>> for message in get_feedback_messages(mark_browser.contents):
-    ...     print extract_text(message)
-    Cleared requests from: Carlos ...
-
-And now, the remaining two. This time we expect to be redirected.
-
-    >>> mark_browser.getControl("Sample Person").selected = True
-    >>> mark_browser.getControl("Foo Bar").selected = True
-    >>> mark_browser.getControl("Save changes").click()
-
-    >>> print mark_browser.url
-    http://blueprints.launchpad.dev/firefox/+spec/canvas
-
-

=== modified file 'lib/lp/blueprints/stories/standalone/xx-personviews.txt'
--- lib/lp/blueprints/stories/standalone/xx-personviews.txt	2011-12-23 23:44:59 +0000
+++ lib/lp/blueprints/stories/standalone/xx-personviews.txt	2012-06-12 12:14:35 +0000
@@ -87,16 +87,3 @@
     Team member workload...
     Andrew Bennetts's specifications:...
     Daniel Silverstone has no outstanding specifications...
-
-The 'Feedback requests' link displays a list of the specifications that
-the person was asked to review.
-
-    >>> browser.open('http://blueprints.launchpad.dev/~name16')
-    >>> browser.getLink('Feedback request').click()
-    >>> browser.url
-    '.../~name16/+specfeedback'
-    >>> print browser.title
-    Feature feedback requests...
-    >>> soup = find_main_content(browser.contents)
-    >>> soup('p')
-    [... 1 specification(s) listed...]

=== removed file 'lib/lp/blueprints/templates/person-specfeedback.pt'
--- lib/lp/blueprints/templates/person-specfeedback.pt	2009-09-14 22:45:13 +0000
+++ lib/lp/blueprints/templates/person-specfeedback.pt	1970-01-01 00:00:00 +0000
@@ -1,31 +0,0 @@
-<html
-  xmlns="http://www.w3.org/1999/xhtml"
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  metal:use-macro="view/macro:page/main_only"
-  i18n:domain="launchpad"
->
-
-<body>
-
-<div metal:fill-slot="main" tal:define="specs view/feedback_specs">
-  <p>
-    <tal:no_specs condition="not: specs">
-      No feedback requests to list.
-    </tal:no_specs>
-    <tal:has_specs condition="specs">
-      <span tal:replace="specs/count">7</span> specification(s) listed.
-      Select the specification then choose "Give Feedback" to clear this
-      request from your queue.
-    </tal:has_specs>
-    Anybody can request feedback from anyone else on a particular
-    specification.
-  </p>
-
-  <tal:per_spec repeat="spec specs">
-    <div tal:replace="structure spec/@@+listing-feedback" />
-  </tal:per_spec>
-</div>
-</body>
-</html>

=== removed file 'lib/lp/blueprints/templates/specification-givefeedback.pt'
--- lib/lp/blueprints/templates/specification-givefeedback.pt	2009-09-22 09:19:45 +0000
+++ lib/lp/blueprints/templates/specification-givefeedback.pt	1970-01-01 00:00:00 +0000
@@ -1,42 +0,0 @@
-<specification-give-feedback
-  xmlns="http://www.w3.org/1999/xhtml"
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  metal:use-macro="view/macro:page/main_only"
-  i18n:domain="launchpad">
-
-  <div metal:fill-slot="main">
-    <div class="top-portlet">
-      <h2>Select the requests which you wish to remove from your queue</h2>
-      <div metal:use-macro="context/@@launchpad_form/form">
-        <div metal:fill-slot="extra_info">
-          <p>
-            Select the items that you have addressed in this
-            blueprint, and click "Save changes" to clear those
-            feedback requests from your queue.
-          </p>
-        </div>
-        <div metal:fill-slot="widgets">
-          <table class="form">
-            <tbody>
-              <tr tal:repeat="request view/requests">
-                <td colspan="2">
-                  <input type="checkbox" name="name"
-                         tal:attributes="value request/requester/name;
-                                         id string:req_${request/requester/name}" />
-                  <label tal:content="request/requester/displayname"
-                         tal:attributes="for string:req_${request/requester/name}" />
-                  <div tal:condition="request/queuemsg"
-                       tal:content="request/queuemsg"
-                       class="lesser" />
-                </td>
-              </tr>
-            </tbody>
-          </table>
-        </div>
-      </div>
-    </div>
-  </div>
-
-</specification-give-feedback>

=== modified file 'lib/lp/blueprints/templates/specification-index.pt'
--- lib/lp/blueprints/templates/specification-index.pt	2012-06-04 11:41:47 +0000
+++ lib/lp/blueprints/templates/specification-index.pt	2012-06-12 12:14:35 +0000
@@ -260,22 +260,6 @@
           </div>
         </div>
 
-        <div id="feedback-requests">
-          <h3 tal:condition="not:context/feedbackrequests">Feedback requests</h3>
-
-          <div tal:replace="structure context/@@+portlet-feedbackqueue" />
-
-          <ul class="horizontal">
-            <li tal:define="link context_menu/givefeedback"
-                tal:condition="link/enabled">
-              <a tal:replace="structure link/fmt:link" />
-            </li>
-            <li tal:define="link context_menu/requestfeedback"
-                tal:condition="link/enabled">
-              <a tal:replace="structure link/fmt:link" />
-            </li>
-          </ul>
-        </div>
       </div>
     </div>
 

=== modified file 'lib/lp/blueprints/templates/specification-listing-detailed.pt'
--- lib/lp/blueprints/templates/specification-listing-detailed.pt	2009-07-17 17:59:07 +0000
+++ lib/lp/blueprints/templates/specification-listing-detailed.pt	2012-06-12 12:14:35 +0000
@@ -46,12 +46,6 @@
   </tal:block>
   <i tal:condition="context/whiteboard"
      tal:content="context/whiteboard">white board goes here</i>
-  <tal:fbreqs repeat="fbreq view/feedbackrequests">
-    <b>Your feedback requested</b> by
-    <span tal:replace="fbreq/requester/displayname">Foo Bar</span>.
-    <i tal:content="fbreq/queuemsg"
-       tal:condition="fbreq/queuemsg">Msg goes here</i>
-  </tal:fbreqs>
   <tal:sprints condition="context/sprints">
   <br />
   <b>Sprints</b>:

=== removed file 'lib/lp/blueprints/templates/specification-listing-feedback.pt'
--- lib/lp/blueprints/templates/specification-listing-feedback.pt	2009-09-14 22:45:13 +0000
+++ lib/lp/blueprints/templates/specification-listing-feedback.pt	1970-01-01 00:00:00 +0000
@@ -1,36 +0,0 @@
-<tal:root
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  omit-tag="">
-
-<table class="narrow-listing" style="margin-bottom: 1em;">
-  <tr>
-    <td width="80%">
-      <a tal:replace="structure context/fmt:link" />
-      <img src="/@@/info" alt="Informational"
-           title="Informational specification"
-           tal:condition="context/informational" />
-    </td>
-    <td>
-      <a class="sprite external-link"
-        tal:attributes="href context/specurl"
-        tal:condition="context/specurl">Spec</a>
-    </td>
-  </tr>
-  <tr>
-    <td colspan="2">
-      <span tal:replace="context/summary/fmt:shorten/500">summary</span>.
-    </td>
-  </tr>
-  <tr class="lesser" tal:repeat="fbreq view/feedbackrequests">
-    <td colspan="2">
-      <a tal:replace="structure fbreq/requester/fmt:link">Foo Bar</a>
-      wants your feedback:<br />
-      <span tal:replace="fbreq/queuemsg">
-        Feedback queue message
-      </span>
-    </td>
-  </tr>
-</table>
-</tal:root>

=== removed file 'lib/lp/blueprints/templates/specification-portlet-feedbackqueue.pt'
--- lib/lp/blueprints/templates/specification-portlet-feedbackqueue.pt	2011-03-03 23:45:45 +0000
+++ lib/lp/blueprints/templates/specification-portlet-feedbackqueue.pt	1970-01-01 00:00:00 +0000
@@ -1,28 +0,0 @@
-<tal:root
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  omit-tag="">
-<div
-  tal:define="fbreqs context/feedbackrequests"
-  tal:condition="fbreqs"
-  id="portlet-feedback"
->
-    <h3>Feedback requests</h3>
-    <div>
-      <ul style="list-style: none; padding-left: 0;">
-        <li tal:repeat="fbreq fbreqs">
-        <tal:requester replace="structure fbreq/requester/fmt:link" />
-        requested feedback from
-        <strong tal:content="structure fbreq/reviewer/fmt:link" />:
-        <span
-          tal:condition="fbreq/queuemsg"
-          tal:content="structure fbreq/queuemsg/fmt:text-to-html"
-          style="font-style:italic; text-indent:2em;"
-        />
-        </li>
-      </ul>
-    </div>
-
-</div>
-</tal:root>

=== removed file 'lib/lp/blueprints/templates/specification-requestfeedback.pt'
--- lib/lp/blueprints/templates/specification-requestfeedback.pt	2009-09-22 10:48:09 +0000
+++ lib/lp/blueprints/templates/specification-requestfeedback.pt	1970-01-01 00:00:00 +0000
@@ -1,32 +0,0 @@
-<html
-  xmlns="http://www.w3.org/1999/xhtml"
-  xmlns:tal="http://xml.zope.org/namespaces/tal"
-  xmlns:metal="http://xml.zope.org/namespaces/metal"
-  xmlns:i18n="http://xml.zope.org/namespaces/i18n"
-  metal:use-macro="view/macro:page/main_side"
-  i18n:domain="launchpad"
->
-
-<body>
-
-<div metal:fill-slot="side">
-  <div class="portlet"
-       tal:content="structure context/@@+portlet-feedbackqueue" />
-</div>
-
-<div metal:fill-slot="main">
-
-  <div metal:use-macro="context/@@launchpad_form/form">
-
-    <p metal:fill-slot="extra_info" class="documentDescription">
-      You can request feedback from a specific person,
-      including a brief message to explain what you need them to look at or
-      consider.
-    </p>
-
-  </div>
-
-</div>
-
-</body>
-</html>

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2012-04-06 15:57:35 +0000
+++ lib/lp/registry/browser/configure.zcml	2012-06-12 12:14:35 +0000
@@ -981,13 +981,6 @@
             template="../../blueprints/templates/person-table-specworkload.pt"/>
         <browser:page
             for="lp.registry.interfaces.person.IPerson"
-            name="+specfeedback"
-            facet="specifications"
-            class="lp.registry.browser.person.PersonSpecFeedbackView"
-            permission="zope.Public"
-            template="../../blueprints/templates/person-specfeedback.pt"/>
-        <browser:page
-            for="lp.registry.interfaces.person.IPerson"
             name="+specworkload"
             facet="specifications"
             class="lp.registry.browser.person.PersonSpecWorkloadView"

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2012-05-24 12:05:42 +0000
+++ lib/lp/registry/browser/person.py	2012-06-12 12:14:35 +0000
@@ -43,7 +43,6 @@
     'PersonSetActionNavigationMenu',
     'PersonSetContextMenu',
     'PersonSetNavigation',
-    'PersonSpecFeedbackView',
     'PersonSpecWorkloadTableView',
     'PersonSpecWorkloadView',
     'PersonSpecsMenu',
@@ -148,9 +147,7 @@
     LaunchpadRadioWidget,
     LaunchpadRadioWidgetWithDescription,
     )
-from lp.blueprints.browser.specificationtarget import HasSpecificationsView
 from lp.blueprints.enums import (
-    SpecificationFilter,
     SpecificationWorkItemStatus,
     )
 from lp.bugs.interfaces.bugtask import (
@@ -602,8 +599,7 @@
     usedfor = IPerson
     facet = 'specifications'
     links = ['assignee', 'drafter', 'approver',
-             'subscriber', 'registrant', 'feedback',
-             'workload']
+             'subscriber', 'registrant', 'workload']
 
     def registrant(self):
         text = 'Registrant'
@@ -631,12 +627,6 @@
         text = 'Subscriber'
         return Link('+specs?role=subscriber', text, icon='blueprint')
 
-    def feedback(self):
-        text = 'Feedback requests'
-        summary = 'List specs where feedback has been requested from %s' % (
-            self.context.displayname)
-        return Link('+specfeedback', text, summary, icon='info')
-
     def workload(self):
         text = 'Workload'
         summary = 'Show all specification work assigned'
@@ -1356,17 +1346,6 @@
                 for spec in self.context.specifications()]
 
 
-class PersonSpecFeedbackView(HasSpecificationsView):
-
-    label = 'Feature feedback requests'
-    page_title = label
-
-    @cachedproperty
-    def feedback_specs(self):
-        filter = [SpecificationFilter.FEEDBACK]
-        return self.context.specifications(filter=filter)
-
-
 class PersonVouchersView(LaunchpadFormView):
     """Form for displaying and redeeming commercial subscription vouchers."""
 

=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt	2012-04-16 15:35:47 +0000
+++ lib/lp/registry/doc/person.txt	2012-06-12 12:14:35 +0000
@@ -93,7 +93,6 @@
     ...     '[e-mail]', PersonCreationRationale.POFILEIMPORT,
     ...     comment='when importing the Portuguese translation of firefox',
     ...     hide_email_addresses=True)
-    >>> import transaction
     >>> transaction.commit()
     >>> p.teamowner is None
     True
@@ -1194,16 +1193,15 @@
     ...     SpecificationFilter.COMPLETE]).count()
     0
 
-Next, Carlos has three incomplete specs *related* to him:
+Next, Carlos has two incomplete specs *related* to him:
 
     >>> filter = []
     >>> for spec in carlos.specifications(filter=filter):
     ...     print spec.name, spec.is_complete, spec.informational
     svg-support False False
     extension-manager-upgrades False True
-    media-integrity-check False False
 
-Carlos has 2 specifications assigned to him:
+These 2 specifications are assigned to Carlos:
 
     >>> for spec in carlos.assigned_specs:
     ...     print spec.name
@@ -1225,13 +1223,6 @@
     ...     print spec.name
     extension-manager-upgrades
 
-The Foo Bar person has a single spec which has feedback requested:
-
-    >>> filter = [SpecificationFilter.FEEDBACK]
-    >>> for spec in foobar.specifications(filter=filter):
-    ...     print spec.name
-    e4x
-
 But has registered 5 of them:
 
     >>> filter = [SpecificationFilter.CREATOR]

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2012-06-11 09:17:44 +0000
+++ lib/lp/registry/model/person.py	2012-06-12 12:14:35 +0000
@@ -844,7 +844,6 @@
             SpecificationFilter.ASSIGNEE,
             SpecificationFilter.DRAFTER,
             SpecificationFilter.APPROVER,
-            SpecificationFilter.FEEDBACK,
             SpecificationFilter.SUBSCRIBER])
         for role in roles:
             if role in filter:
@@ -884,10 +883,6 @@
             base += """ OR Specification.id in
                 (SELECT specification FROM SpecificationSubscription
                  WHERE person = %(my_id)d)"""
-        if SpecificationFilter.FEEDBACK in filter:
-            base += """ OR Specification.id in
-                (SELECT specification FROM SpecificationFeedback
-                 WHERE reviewer = %(my_id)d)"""
         base += ') '
 
         # filter out specs on inactive products
@@ -3904,40 +3899,6 @@
             DELETE FROM StructuralSubscription WHERE subscriber=%(from_id)d
             ''' % vars())
 
-    def _mergeSpecificationFeedback(self, cur, from_id, to_id):
-        # Update the SpecificationFeedback entries that will not conflict
-        # and trash the rest.
-
-        # First we handle the reviewer.
-        cur.execute('''
-            UPDATE SpecificationFeedback
-            SET reviewer=%(to_id)d
-            WHERE reviewer=%(from_id)d AND specification NOT IN
-                (
-                SELECT specification
-                FROM SpecificationFeedback
-                WHERE reviewer = %(to_id)d
-                )
-            ''' % vars())
-        cur.execute('''
-            DELETE FROM SpecificationFeedback WHERE reviewer=%(from_id)d
-            ''' % vars())
-
-        # And now we handle the requester.
-        cur.execute('''
-            UPDATE SpecificationFeedback
-            SET requester=%(to_id)d
-            WHERE requester=%(from_id)d AND specification NOT IN
-                (
-                SELECT specification
-                FROM SpecificationFeedback
-                WHERE requester = %(to_id)d
-                )
-            ''' % vars())
-        cur.execute('''
-            DELETE FROM SpecificationFeedback WHERE requester=%(from_id)d
-            ''' % vars())
-
     def _mergeSpecificationSubscription(self, cur, from_id, to_id):
         # Update the SpecificationSubscription entries that will not conflict
         # and trash the rest
@@ -4334,10 +4295,6 @@
         self._mergeStructuralSubscriptions(cur, from_id, to_id)
         skip.append(('structuralsubscription', 'subscriber'))
 
-        self._mergeSpecificationFeedback(cur, from_id, to_id)
-        skip.append(('specificationfeedback', 'reviewer'))
-        skip.append(('specificationfeedback', 'requester'))
-
         self._mergeSpecificationSubscription(cur, from_id, to_id)
         skip.append(('specificationsubscription', 'person'))