launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/sub-search-ui-bug-656823-2 into lp:launchpad with lp:~allenap/launchpad/sub-search-ui-bug-656823 as a prerequisite.

Requested reviews:
  Gavin Panella (allenap): ui
  Launchpad code reviewers (launchpad-reviewers): ui
Related bugs:
  #656823 Subscribing to a search lacks a UI
  https://bugs.launchpad.net/bugs/656823


This adds a new view for IPerson, registered as
+structural-subscriptions, that displays the structures to which the
user has subscribed. This matches a similar page called +subscriptions
(which is live, but not linked to from anywhere) which shows direct
subscriptions (bugs only so far).

I didn't put this information on the +subscriptions page because that
has a batch navigator over the direct subscriptions. I've found having
a static data set on the same page as batched information to be
confusing in the past.

Instead each page links to the other using an info link at the
top. This looks okay, but there's probably a better way of doing it.

The structural subscriptions are shown in a list, not in tabular form,
because they will be extended later to include subordinate
information, namely bug subscription filters. There can be multiple
filters for each structural subscription. Nested lists seem to be an
obvious way to represent this.

-- 
https://code.launchpad.net/~allenap/launchpad/sub-search-ui-bug-656823-2/+merge/38136
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/sub-search-ui-bug-656823-2 into lp:launchpad.

=== modified file 'lib/lp/registry/browser/configure.zcml'
--- lib/lp/registry/browser/configure.zcml	2010-10-06 18:53:53 +0000
+++ lib/lp/registry/browser/configure.zcml	2010-10-11 16:22:47 +0000
@@ -1085,6 +1085,12 @@
             name="+subscriptions"
             template="../templates/person-subscriptions.pt"/>
         <browser:page
+            for="lp.registry.interfaces.person.IPerson"
+            permission="zope.Public"
+            class="lp.registry.browser.person.PersonStructuralSubscriptionsView"
+            name="+structural-subscriptions"
+            template="../templates/person-structural-subscriptions.pt"/>
+        <browser:page
             for="lp.registry.interfaces.person.ITeam"
             permission="zope.Public"
             class="lp.registry.browser.person.TeamIndexView"

=== modified file 'lib/lp/registry/browser/person.py'
--- lib/lp/registry/browser/person.py	2010-10-11 16:22:45 +0000
+++ lib/lp/registry/browser/person.py	2010-10-11 16:22:47 +0000
@@ -52,6 +52,7 @@
     'PersonSpecWorkloadTableView',
     'PersonSpecWorkloadView',
     'PersonSpecsMenu',
+    'PersonStructuralSubscriptionsView',
     'PersonSubscribedBugTaskSearchListingView',
     'PersonSubscriptionsView',
     'PersonView',
@@ -73,8 +74,8 @@
     'TeamMembershipView',
     'TeamMugshotView',
     'TeamNavigation',
+    'TeamOverviewMenu',
     'TeamOverviewNavigationMenu',
-    'TeamOverviewMenu',
     'TeamReassignmentView',
     'archive_to_person',
     ]
@@ -990,6 +991,16 @@
         enabled = bool(self.person.getOwnedOrDrivenPillars())
         return Link(target, text, enabled=enabled, icon='info')
 
+    def subscriptions(self):
+        target = '+subscriptions'
+        text = 'Direct subscriptions'
+        return Link(target, text, icon='info')
+
+    def structural_subscriptions(self):
+        target = '+structural-subscriptions'
+        text = 'Structural subscriptions'
+        return Link(target, text, icon='info')
+
 
 class PersonMenuMixin(CommonMenuLinks):
 
@@ -1023,14 +1034,34 @@
 
     usedfor = IPerson
     facet = 'overview'
-    links = ['edit', 'branding', 'common_edithomepage',
-             'editemailaddresses', 'editlanguages', 'editwikinames',
-             'editircnicknames', 'editjabberids',
-             'editsshkeys', 'editpgpkeys', 'editlocation', 'memberships',
-             'codesofconduct', 'karma', 'administer', 'administer_account',
-             'projects', 'activate_ppa', 'maintained',
-             'view_ppa_subscriptions', 'ppa', 'oauth_tokens',
-             'related_software_summary', 'view_recipes']
+    links = [
+        'edit',
+        'branding',
+        'common_edithomepage',
+        'editemailaddresses',
+        'editlanguages',
+        'editwikinames',
+        'editircnicknames',
+        'editjabberids',
+        'editsshkeys',
+        'editpgpkeys',
+        'editlocation',
+        'memberships',
+        'codesofconduct',
+        'karma',
+        'administer',
+        'administer_account',
+        'projects',
+        'activate_ppa',
+        'maintained',
+        'view_ppa_subscriptions',
+        'ppa',
+        'oauth_tokens',
+        'related_software_summary',
+        'view_recipes',
+        'subscriptions',
+        'structural_subscriptions',
+        ]
 
     def related_software_summary(self):
         target = '+related-software'
@@ -1370,14 +1401,36 @@
 
     usedfor = ITeam
     facet = 'overview'
-    links = ['edit', 'branding', 'common_edithomepage', 'members', 'mugshots',
-             'add_member', 'proposed_members',
-             'memberships', 'received_invitations',
-             'editemail', 'configure_mailing_list', 'moderate_mailing_list',
-             'editlanguages', 'map', 'polls',
-             'add_poll', 'join', 'leave', 'add_my_teams',
-             'reassign', 'projects', 'activate_ppa', 'maintained', 'ppa',
-             'related_software_summary', 'view_recipes']
+    links = [
+        'edit',
+        'branding',
+        'common_edithomepage',
+        'members',
+        'mugshots',
+        'add_member',
+        'proposed_members',
+        'memberships',
+        'received_invitations',
+        'editemail',
+        'configure_mailing_list',
+        'moderate_mailing_list',
+        'editlanguages',
+        'map',
+        'polls',
+        'add_poll',
+        'join',
+        'leave',
+        'add_my_teams',
+        'reassign',
+        'projects',
+        'activate_ppa',
+        'maintained',
+        'ppa',
+        'related_software_summary',
+        'view_recipes',
+        'subscriptions',
+        'structural_subscriptions',
+        ]
 
 
 class TeamOverviewNavigationMenu(NavigationMenu, TeamMenuMixin):
@@ -2398,6 +2451,22 @@
         return "Subscriptions for %s" % self.context.displayname
 
 
+class PersonStructuralSubscriptionsView(LaunchpadView):
+    """All the structural subscriptions for a person."""
+
+    page_title = 'Structural subscriptions'
+
+    def canUnsubscribeFromBugTasks(self):
+        """Can the current user modify subscriptions for the context?"""
+        return (self.user is not None and
+                self.user.inTeam(self.context))
+
+    @property
+    def label(self):
+        """The header for the structural subscriptions page."""
+        return "Structural subscriptions for %s" % self.context.displayname
+
+
 class PersonVouchersView(LaunchpadFormView):
     """Form for displaying and redeeming commercial subscription vouchers."""
 

=== modified file 'lib/lp/registry/stories/person/xx-person-subscriptions.txt'
--- lib/lp/registry/stories/person/xx-person-subscriptions.txt	2010-10-07 22:25:24 +0000
+++ lib/lp/registry/stories/person/xx-person-subscriptions.txt	2010-10-11 16:22:47 +0000
@@ -1,5 +1,5 @@
 Subscriptions View
-==================
+------------------
 
 XXX: bdmurray 2010-09-24 bug=628411 This story is complete until we publish
 the link that leads to the +subscriptions page.
@@ -129,3 +129,62 @@
     ...     find_main_content(sample_browser.contents))
     >>> "does not have any direct bug subscriptions" in page_text
     True
+
+
+Structural subscriptions
+========================
+
+Leading from the subscriptions view is an overview page of all
+structural subscriptions.
+
+    >>> admin_browser.open("http://launchpad.dev/people/+me/+subscriptions";)
+    >>> admin_browser.getLink("Structural subscriptions").click()
+    >>> admin_browser.url
+    'http://launchpad.dev/~name16/+structural-subscriptions'
+    >>> admin_browser.title
+    'Structural subscriptions : Foo Bar'
+
+The structures to which the user is subscribed are displayed in a
+list. The title of the structure links to the structure itself, and is
+followed by a link to edit the subscription.
+
+    >>> subscriptions = find_tag_by_id(
+    ...     admin_browser.contents, 'structural-subscriptions')
+    >>> for subscription in subscriptions.findAll("li"):
+    ...     structure_link, modify_link = subscription.findAll("a")
+    ...     print "%s <%s>" % (
+    ...         extract_text(structure_link), structure_link.get("href"))
+    ...     print "--> %s" % modify_link.get("href")
+    mozilla-firefox in ubuntu </ubuntu/+source/mozilla-firefox>
+    --> /ubuntu/+source/mozilla-firefox/+subscribe
+    pmount in ubuntu </ubuntu/+source/pmount>
+    --> /ubuntu/+source/pmount/+subscribe
+
+The links to modify subscriptions are only shown when the user has
+permission to modify those subscriptions.
+
+    >>> sample_browser.open(
+    ...     "http://launchpad.dev/~name16/+structural-subscriptions";)
+    >>> subscriptions = find_tag_by_id(
+    ...     sample_browser.contents, 'structural-subscriptions')
+    >>> for subscription in subscriptions.findAll("li"):
+    ...     [structure_link] = subscription.findAll("a")
+    ...     print "%s <%s>" % (
+    ...         extract_text(structure_link), structure_link.get("href"))
+    mozilla-firefox in ubuntu </ubuntu/+source/mozilla-firefox>
+    pmount in ubuntu </ubuntu/+source/pmount>
+
+The structural subscriptions page links back to the direct
+subscriptions page.
+
+    >>> admin_browser.getLink("Direct subscriptions").url
+    'http://launchpad.dev/~name16/+subscriptions'
+
+A simple explanatory message is shown when the user doesn't have any
+structural subscriptions.
+
+    >>> sample_browser.open(
+    ...     "http://launchpad.dev/people/+me/+structural-subscriptions";)
+    >>> print extract_text(find_tag_by_id(
+    ...     sample_browser.contents, "structural-subscriptions"))
+    Sample Person does not have any structural subscriptions.

=== added file 'lib/lp/registry/templates/person-structural-subscriptions.pt'
--- lib/lp/registry/templates/person-structural-subscriptions.pt	1970-01-01 00:00:00 +0000
+++ lib/lp/registry/templates/person-structural-subscriptions.pt	2010-10-11 16:22:47 +0000
@@ -0,0 +1,48 @@
+<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";
+    xml:lang="en"
+    lang="en"
+    dir="ltr"
+    metal:use-macro="view/macro:page/main_only"
+    i18n:domain="launchpad">
+  <body>
+
+    <div metal:fill-slot="main"
+         tal:define="structural_subscriptions context/structural_subscriptions">
+
+      <ul class="horizontal">
+        <li
+            tal:define="link context/menu:overview/subscriptions"
+            tal:condition="link/enabled"
+            tal:content="structure link/fmt:link" />
+      </ul>
+
+      <div class="yui-g">
+        <div class="portlet" id="structural-subscriptions">
+
+          <ul tal:condition="structural_subscriptions">
+            <li tal:repeat="subscription structural_subscriptions">
+              <span tal:replace="structure subscription/target/fmt:link" />
+              <a tal:condition="view/canUnsubscribeFromBugTasks"
+                 tal:attributes="href subscription/target/fmt:url/+subscribe;
+                                 title string:Modify subscription to ${subscription/target/title};">
+                <img src="/@@/edit" />
+              </a>
+            </li>
+          </ul>
+
+          <p tal:condition="not: structural_subscriptions">
+            <tal:person content="context/fmt:displayname" /> does not
+            have any structural subscriptions.
+          </p>
+
+        </div>
+      </div>
+
+    </div>
+
+  </body>
+</html>

=== modified file 'lib/lp/registry/templates/person-subscriptions.pt'
--- lib/lp/registry/templates/person-subscriptions.pt	2010-10-11 16:22:45 +0000
+++ lib/lp/registry/templates/person-subscriptions.pt	2010-10-11 16:22:47 +0000
@@ -15,6 +15,16 @@
                    sub_bugs view/subscribedBugTasks;
                    sub_bug_batch sub_bugs/currentBatch">
 
+    <ul class="horizontal">
+      <li
+          tal:define="link context/menu:overview/structural_subscriptions"
+          tal:condition="link/enabled"
+          tal:content="structure link/fmt:link" />
+    </ul>
+
+    <div class="yui-g">
+      <div class="portlet">
+
     <tal:no_subscribed_bugs condition="not: sub_bug_batch|nothing">
       <p>
         <tal:person content="display_name">Sample
@@ -23,10 +33,6 @@
     </tal:no_subscribed_bugs>
 
     <tal:subscribed_bugs condition="sub_bug_batch|nothing">
-      <p>
-        Direct bug subscriptions for
-        <tal:person content="display_name">Sample Person</tal:person>
-      </p>
       <tal:multipage condition="sub_bugs/has_multiple_pages">
         <tal:navigation
             replace="structure sub_bugs/@@+navigation-links-upper"/>
@@ -72,6 +78,9 @@
       </tal:multipage>
     </tal:subscribed_bugs>
 
+      </div>
+    </div>
+
   </div>
 </body>
 </html>