Merge lp:~michael.nelson/launchpad/409187-trivial-ui-fixes-for-p3a-access into lp:launchpad

[Michael Nelson]

[Michael Nelson (michael.nelson)]

= Summary =

This branch deals with a number of UI issues related to private PPA
subscriptions. Please refer tot bug 409187 for the details.

== Proposed fix ==

== Pre-implementation notes ==

All before-after screenshots and notes are on bug 409187, as well as a
screencast of the new interaction.

== Implementation details ==

I have not added a windmill test for the new interaction yet, but will
do soon. If I need to do it now I will, but otherwise I'll focus on
blueprint templates for the rest of today.

== Tests ==

bin/test -vv -t archivesubscription-views.txt -t
TestArchiveSubscriptionBreadcrumb -t stories/ppa

== Demo and Q/A ==

For screenshots/casts, see bug 409187.

To demo locally, run the following in a harness:

ppa = getUtility(IPersonSet).getByName('cprov').archive
ppa.private = True
ppa.buildd_secret = 'blah'
import transaction;transaction.commit()

and then navigate to:
https://launchpad.dev/~cprov/+archive/ppa

Click on 'Manage access', add yourself (Celso) and others. Try editing.

Next go to:
https://launchpad.dev/~cprov and click on 'View your private ppa
subscriptions'. Click on the subscription listed there etc.

For QA, the soyuz-team ppa can be used.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/soyuz/templates/person-archive-subscriptions.pt
  lib/lp/soyuz/templates/archive-subscribers.pt
  lib/canonical/launchpad/pagetitles.py
  lib/lp/soyuz/templates/person-archive-subscription.pt
  lib/lp/soyuz/browser/tests/test_breadcrumbs.py
  lib/lp/soyuz/browser/tests/archivesubscription-views.txt
  lib/lp/soyuz/interfaces/archivesubscriber.py
  lib/lp/soyuz/configure.zcml
  lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt
  lib/canonical/launchpad/icing/style-3-0.css
  lib/lp/soyuz/browser/archivesubscription.py
  lib/lp/soyuz/browser/configure.zcml
  lib/lp/soyuz/templates/archive-subscriber-edit.pt

== Pylint notices ==

lib/lp/soyuz/interfaces/archivesubscriber.py
    20: [F0401] Unable to import 'lazr.enum' (No module named enum)
    26: [F0401] Unable to import 'lazr.restful.declarations' (No module
named restful)
    27: [F0401] Unable to import 'lazr.restful.fields' (No module named
restful)

--
Michael

[Guilherme Salgado (salgado)]

Hi Michael,

Your changes look good. I have a few suggestions and I'd like somebody
else to review the JS changes, as I don't feel comfortable doing so.

 review approve

> === modified file 'lib/lp/soyuz/browser/archivesubscription.py'
> --- lib/lp/soyuz/browser/archivesubscription.py 2009-08-12 08:40:41 +0000
> +++ lib/lp/soyuz/browser/archivesubscription.py 2009-09-14 08:08:51 +0000
> @@ -9,6 +9,8 @@
>
> __all__ = [
> 'ArchiveSubscribersView',
> + 'PersonalArchiveSubscriptionBreadcrumb',
> + 'PersonArchiveSubscriptionView',
> 'PersonArchiveSubscriptionsView',
> 'traverse_archive_subscription_for_subscriber'
> ]
> @@ -34,10 +36,11 @@
> IArchiveAuthTokenSet)
> from lp.soyuz.interfaces.archivesubscriber import (
> IArchiveSubscriberSet, IPersonalArchiveSubscription)
> +from canonical.launchpad.webapp.breadcrumb import Breadcrumb
> from canonical.launchpad.webapp.launchpadform import (
> action, custom_widget, LaunchpadFormView, LaunchpadEditFormView)
> from canonical.launchpad.webapp.publisher import (
> - canonical_url, LaunchpadView)
> + canonical_url, LaunchpadView, Navigation)
> from canonical.widgets import DateWidget
> from canonical.widgets.popup import PersonPickerWidget
>
> @@ -65,6 +68,20 @@
> """See `IPersonalArchiveSubscription`."""
> return "Access to %s" % self.archive.displayname
>
> + @property
> + def title(self):

I know of page_title and label, which are used for this purpose, did you mean
one of them or am I not aware of this third one?

> + """Required for default headings in templates."""
> + return self.displayname
> +
> +
> +class PersonalArchiveSubscriptionNavigation(Navigation):
> + """Navigation for `IPersonalArchiveSubscription`.
> +
> + Without this, a breadcrumb will not be created for personal
> + archive subscription objects.

This is no longer needed, actually -- a fix for the bug (423898) is included
in my breadcrumbs branch that is on PQM now.

> + """
> + usedfor = IPersonalArchiveSubscription
> +
> def traverse_archive_subscription_for_subscriber(subscriber, archive_id):
> """Return the subscription for a subscriber to an archive."""
> subscription = None
> @@ -79,6 +96,14 @@
> return PersonalArchiveSubscription(subscriber, archive)
>
>
> +class PersonalArchiveSubscriptionBreadcrumb(Breadcrumb):
> + """Builds a breadcrumb for `PersonalArchiveSubscription`."""
> +
> + @property
> + def text(self):
> + return self.context.displayname

You can use webapp.breadcrumbs.DisplaynameBreadcrumb in the zcml and get rid
of this adapter altogether. It'd be great if you could do a quick check for
other places where that generic adapter could replace other specific ones. :)

> +
> +
> class IArchiveSubscriberUI(Interface):
> """A custom interface for user interaction with archive subscriptions.
>
> === modified file 'lib/lp/soyuz/browser/tests/test_breadcrumbs.py'
> --- lib/lp/soyuz/browser/tests/test_breadcrumbs.py 2009-09-02 16:32:06 +0000
> +++ lib/lp/soyuz/browser/tests/test_breadcrumbs.py 2009-09-11 10:52:54 +0000
> @@ -11,6 +11,8 @@
> from canonical.launc...

review: Approve

[Barry Warsaw (barry)]

Looks very good, and the movie really helped! The only thing that's weird for me is leaving the date blank to mean never expire. There's no need to hold up the branch for this, so I'll just ramble a bit.

If possible, the date picker maybe should have a "Never expire" button. You'd use this both in the initial date choice but also if you then decide to extend a user's access to "forever". Also, if access never expires, it would be nice if the date field actually said that, instead of being blank.

With these changes, you'd be able to get rid of the helpful text explaining what a blank field means.

If you think these are worthwhile changes to make, file a bug and fix it later. But the ui as it is looks otherwise great.

review: Approve (ui*)

[Michael Nelson (michael.nelson)]

Guilherme Salgado wrote:
> Review: Approve
> Hi Michael,
>
> Your changes look good. I have a few suggestions and I'd like somebody
> else to review the JS changes, as I don't feel comfortable doing so.
>

Great, thanks Salgado.

> review approve
>
>> === modified file 'lib/lp/soyuz/browser/archivesubscription.py'
>> --- lib/lp/soyuz/browser/archivesubscription.py 2009-08-12 08:40:41 +0000
>> +++ lib/lp/soyuz/browser/archivesubscription.py 2009-09-14 08:08:51 +0000
>> @@ -9,6 +9,8 @@
>>
>> __all__ = [
>> 'ArchiveSubscribersView',
>> + 'PersonalArchiveSubscriptionBreadcrumb',
>> + 'PersonArchiveSubscriptionView',
>> 'PersonArchiveSubscriptionsView',
>> 'traverse_archive_subscription_for_subscriber'
>> ]
>> @@ -34,10 +36,11 @@
>> IArchiveAuthTokenSet)
>> from lp.soyuz.interfaces.archivesubscriber import (
>> IArchiveSubscriberSet, IPersonalArchiveSubscription)
>> +from canonical.launchpad.webapp.breadcrumb import Breadcrumb
>> from canonical.launchpad.webapp.launchpadform import (
>> action, custom_widget, LaunchpadFormView, LaunchpadEditFormView)
>> from canonical.launchpad.webapp.publisher import (
>> - canonical_url, LaunchpadView)
>> + canonical_url, LaunchpadView, Navigation)
>> from canonical.widgets import DateWidget
>> from canonical.widgets.popup import PersonPickerWidget
>>
>> @@ -65,6 +68,20 @@
>> """See `IPersonalArchiveSubscription`."""
>> return "Access to %s" % self.archive.displayname
>>
>> + @property
>> + def title(self):
>
> I know of page_title and label, which are used for this purpose, did you mean
> one of them or am I not aware of this third one?

No, the helper class to which this belongs (PersonalArchiveSubscription)
is used as the context for these views, and so like normal content
classes, it needs a title attribute for when the base template tries to
access context.title.

>
>> + """Required for default headings in templates."""
>> + return self.displayname
>> +
>> +
>> +class PersonalArchiveSubscriptionNavigation(Navigation):
>> + """Navigation for `IPersonalArchiveSubscription`.
>> +
>> + Without this, a breadcrumb will not be created for personal
>> + archive subscription objects.
>
> This is no longer needed, actually -- a fix for the bug (423898) is included
> in my breadcrumbs branch that is on PQM now.

Great, removed.

>
>> + """
>> + usedfor = IPersonalArchiveSubscription
>> +
>> def traverse_archive_subscription_for_subscriber(subscriber, archive_id):
>> """Return the subscription for a subscriber to an archive."""
>> subscription = None
>> @@ -79,6 +96,14 @@
>> return PersonalArchiveSubscription(subscriber, archive)
>>
>>
>> +class PersonalArchiveSubscriptionBreadcrumb(Breadcrumb):
>> + """Builds a breadcrumb for `PersonalArchiveSubscription`."""
>> +
>> + @property
>> + def text(self):
>> + return self.context.displayname
>
> You can use webapp.breadcrumbs.DisplaynameBreadcrumb in the zcml and get rid
> of this adapter altogether. It'd be great if you could do a quick check for
> other places where that generic adapter could replace other speci...

[Graham Binns (gmb)]

Approved with the changes requested in IRC w.r.t comments in the tests.

review: Approve (js)

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2009-09-23 10:29:52 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2009-09-30 14:18:17 +0000
@@ -366,6 +366,9 @@
     vertical-align: bottom;
     }
 
+form {
+    margin-bottom: 1em;
+}
 form h1 {
     margin-bottom: 1em;
     }

=== added file 'lib/canonical/launchpad/javascript/soyuz/archivesubscribers_index.js'
--- lib/canonical/launchpad/javascript/soyuz/archivesubscribers_index.js	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/soyuz/archivesubscribers_index.js	2009-09-30 14:18:17 +0000
@@ -0,0 +1,54 @@
+/* Copyright 2009 Canonical Ltd.  This software is licensed under the
+ * GNU Affero General Public License version 3 (see the file LICENSE).
+ *
+ * Enhancements for adding ppa subscribers.
+ *
+ * @module ArchiveSubscribersIndex
+ * @requires  event, node, oop
+ */
+YUI.add('soyuz.archivesubscribers_index', function(Y) {
+
+var soyuz = Y.namespace('soyuz');
+
+/*
+ * Setup the style and click handler for the add subscriber link.
+ *
+ * @method setup_archivesubscribers_index
+ */
+Y.soyuz.setup_archivesubscribers_index = function() {
+    // If there are no errors then we hide the add-subscriber row and
+    // potentially the whole table if there are no subscribers.
+    if (Y.Lang.isNull(Y.get('p.error.message'))) {
+
+        // Hide the add-subscriber row.
+        var add_subscriber_row = Y.get(
+            '#archive-subscribers .add-subscriber');
+        add_subscriber_row.setStyle('display', 'none');
+
+        // If there are no subscribers, then hide the complete section.
+        var subscribers = Y.get('#subscribers');
+        if (Y.Lang.isObject(Y.get('#no-subscribers'))) {
+            subscribers.setStyle('display', 'none');
+        }
+    }
+
+    // Add a link to open the add-subscriber row.
+    var placeholder = Y.get('#add-subscriber-placeholder');
+    placeholder.set(
+        'innerHTML',
+        '<a class="js-action sprite add" href="#">Add access</a>');
+
+    // Unfortunately we can't use the lazr slider, as it uses display:block
+    // which breaks table rows (they use display:table-row).
+    function show_add_subscriber(e) {
+        e.preventDefault();
+        subscribers.setStyle('display', 'block');
+        add_subscriber_row.setStyle('display', 'table-row');
+    }
+
+    Y.on('click', show_add_subscriber,
+         '#add-subscriber-placeholder a');
+};
+
+}, '0.1', {requires: ['oop', 'node', 'event']});
+

=== added file 'lib/canonical/launchpad/javascript/soyuz/tests/archivesubscribers_index.html'
--- lib/canonical/launchpad/javascript/soyuz/tests/archivesubscribers_index.html	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/soyuz/tests/archivesubscribers_index.html	2009-09-30 14:18:17 +0000
@@ -0,0 +1,46 @@
+<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
+<html>
+  <head>
+  <title>Launchpad ArchiveSubscriberIndex</title>
+
+  <!-- YUI 3.0 Setup -->
+  <script type="text/javascript" src="../../../icing/yui/current/build/yui/yui.js"></script>
+  <link rel="stylesheet" href="../../../icing/yui/current/build/cssreset/reset.css"/>
+  <link rel="stylesheet" href="../../../icing/yui/current/build/cssfonts/fonts.css"/>
+  <link rel="stylesheet" href="../../../icing/yui/current/build/cssbase/base.css"/>
+  <link rel="stylesheet" href="../../test.css" />
+
+  <!-- The module under test -->
+  <script type="text/javascript" src="../archivesubscribers_index.js"></script>
+
+  <!-- The test suite -->
+  <script type="text/javascript" src="archivesubscribers_index.js"></script>
+</head>
+<body class="yui-skin-sam">
+  <h1>Testing the ArchiveSubscribersIndex javascript</h1>
+
+  <h2>Errors</h2>
+  <div id="errors">
+  </div>
+
+  <h2>Add subscriber place-holder</h2>
+  <div id="add-subscriber-placeholder">
+  </div>
+
+  <h2>Subscribers</h2>
+  <div id="subscribers">
+    <table id="archive-subscribers">
+      <thead>
+        <tr>
+          <th>Name</th>
+          <th>Expires</th>
+          <th colspan="2">Comment</th>
+        </tr>
+      </thead>
+      <tbody>
+      </tbody>
+    </table>
+  </div>
+  <div id="log"></div>
+</body>
+</html>

=== added file 'lib/canonical/launchpad/javascript/soyuz/tests/archivesubscribers_index.js'
--- lib/canonical/launchpad/javascript/soyuz/tests/archivesubscribers_index.js	1970-01-01 00:00:00 +0000
+++ lib/canonical/launchpad/javascript/soyuz/tests/archivesubscribers_index.js	2009-09-30 14:18:17 +0000
@@ -0,0 +1,147 @@
+/* Copyright 2009 Canonical Ltd.  This software is licensed under the
+   GNU Affero General Public License version 3 (see the file LICENSE). */
+
+YUI({
+    base: '../../../icing/yui/current/build/',
+    filter: 'raw',
+    combine: false
+    }).use(
+        'yuitest', 'console', 'soyuz.archivesubscribers_index', function(Y) {
+
+var Assert = Y.Assert;  // For easy access to isTrue(), etc.
+
+var suite = new Y.Test.Suite("ArchiveSubscriber Tests");
+
+suite.add(new Y.Test.Case({
+
+    name: 'add-subscriber',
+
+    setUp: function() {
+        this.add_subscriber_placeholder = Y.get(
+            '#add-subscriber-placeholder');
+        this.archive_subscribers_table_body = Y.get(
+            '#archive-subscribers').query('tbody');
+        this.error_div = Y.get('#errors');
+        this.subscribers_div = Y.get('#subscribers');
+
+
+        // Ensure there are no errors displayed.
+        this.error_div.set('innerHTML', '');
+
+        // Ensure the add subscriber place-holder is empty.
+        this.add_subscriber_placeholder.set('innerHTML', '');
+
+        // Ensure the table has the correct structure.
+        this.archive_subscribers_table_body.set(
+            'innerHTML', [
+                '<tr class="add-subscriber">',
+                '<td>New 1</td>',
+                '<td>New 2</td>',
+                '<td>New 3</td>',
+                '<td>Add</td>',
+                '</tr>',
+                '<tr>',
+                '<td>Existing 1</td>',
+                '<td>Existing 2</td>',
+                '<td>Existing 3</td>',
+                '<td>Edit</td>',
+                '</tr>'
+                ].join(''));
+
+        this.add_subscriber_row = Y.get(
+            '#archive-subscribers .add-subscriber');
+    },
+
+    test_add_row_displayed_by_default: function() {
+        // The add subscriber row is displayed when the JS is not run.
+        Assert.areEqual(
+            'table-row', this.add_subscriber_row.getStyle('display'),
+            'The add subscriber row should display when the js is not run.');
+    },
+
+    test_subscribers_displayed_by_default: function() {
+        // The subscribers section is displayed when the js is not run.
+        Assert.areEqual(
+            'block', this.subscribers_div.getStyle('display'),
+            'The subscribers section should display without js.');
+    },
+
+    test_add_row_hidden_after_setup: function() {
+        // The add subscriber row is hidden during setup.
+        Y.soyuz.setup_archivesubscribers_index();
+        Assert.areEqual(
+            'none', this.add_subscriber_row.getStyle('display'),
+            'The add subscriber row should be hidden during setup.');
+    },
+
+    test_subscribers_section_displayed_after_setup: function() {
+        // The subscribers div normally remains displayed after setup.
+        Y.soyuz.setup_archivesubscribers_index();
+        Assert.areEqual(
+            'block', this.subscribers_div.getStyle('display'),
+            'The subscribers div should remain displayed after setup.');
+    },
+
+    test_subscribers_section_hidden_when_no_subscribers: function() {
+        // The subscribers div is hidden when there are no subscribers.
+
+        // Add a paragraph with the no-subscribers id.
+        this.error_div.set('innerHTML', '<p id="no-subscribers">blah</p>');
+        Y.soyuz.setup_archivesubscribers_index();
+        Assert.areEqual(
+            'none', this.subscribers_div.getStyle('display'),
+            'The subscribers div should be hidden when there are ' +
+            'no subscribers.');
+    },
+
+    test_add_row_displayed_when_errors_present: function() {
+        // The add subscriber row is not hidden if there are validation
+        // errors.
+
+        // Add an error paragraph.
+        this.error_div.set('innerHTML', '<p class="error message">Blah</p>');
+        Y.soyuz.setup_archivesubscribers_index();
+        Assert.areEqual(
+            'table-row', this.add_subscriber_row.getStyle('display'),
+            'The add subscriber row should not be hidden if there are ' +
+            'errors present.');
+    },
+
+    test_add_access_link_added_after_setup: function() {
+        // The 'Add access' link is created during setup.
+
+        Y.soyuz.setup_archivesubscribers_index();
+        Assert.areEqual(
+            '<a class="js-action sprite add" href="#">Add access</a>',
+            this.add_subscriber_placeholder.get('innerHTML'),
+            "The 'Add access' link should be created during setup.")
+    },
+
+    test_click_add_access_displays_add_row: function() {
+        // The add subscriber row is displayed after clicking 'Add access'.
+        Y.soyuz.setup_archivesubscribers_index();
+        var link_node = this.add_subscriber_placeholder.query('a');
+        Assert.areEqual(
+            'Add access', link_node.get('innerHTML'));
+
+        Y.Event.simulate(Y.Node.getDOMNode(link_node), 'click');
+
+        Assert.areEqual(
+            'table-row', this.add_subscriber_row.getStyle('display'),
+            "The add subscriber row should be displayed after clicking " +
+            "'Add access'");
+    }
+}));
+
+Y.Test.Runner.add(suite);
+
+var yconsole = new Y.Console({
+    newestOnTop: false
+});
+yconsole.render('#log');
+
+Y.on('domready', function() {
+    Y.Test.Runner.run();
+});
+
+});

=== modified file 'lib/canonical/launchpad/pagetitles.py'
--- lib/canonical/launchpad/pagetitles.py	2009-09-23 14:58:12 +0000
+++ lib/canonical/launchpad/pagetitles.py	2009-09-30 14:18:17 +0000
@@ -128,10 +128,6 @@
 
 archive_edit_dependencies = ContextDisplayName('Edit dependencies for %s')
 
-archive_subscriber_edit = ContextDisplayName('Edit %s')
-
-archive_subscribers = ContextDisplayName('Manage access to %s')
-
 bazaar_all_branches = 'All branches in the Launchpad Bazaar'
 
 bazaar_index = 'Launchpad Branches'
@@ -562,10 +558,6 @@
 
 people_requestmerge_multiple = 'Merge Launchpad accounts'
 
-person_archive_subscription = ContextDisplayName('%s')
-
-person_archive_subscriptions = 'Private PPA access'
-
 person_answer_contact_for = ContextDisplayName(
     'Projects for which %s is an answer contact')
 

=== modified file 'lib/lp/soyuz/browser/archivesubscription.py'
--- lib/lp/soyuz/browser/archivesubscription.py	2009-08-12 08:40:41 +0000
+++ lib/lp/soyuz/browser/archivesubscription.py	2009-09-30 14:18:17 +0000
@@ -9,6 +9,7 @@
 
 __all__ = [
     'ArchiveSubscribersView',
+    'PersonArchiveSubscriptionView',
     'PersonArchiveSubscriptionsView',
     'traverse_archive_subscription_for_subscriber'
     ]
@@ -34,10 +35,11 @@
     IArchiveAuthTokenSet)
 from lp.soyuz.interfaces.archivesubscriber import (
     IArchiveSubscriberSet, IPersonalArchiveSubscription)
+from canonical.launchpad.webapp.breadcrumb import Breadcrumb
 from canonical.launchpad.webapp.launchpadform import (
     action, custom_widget, LaunchpadFormView, LaunchpadEditFormView)
 from canonical.launchpad.webapp.publisher import (
-    canonical_url, LaunchpadView)
+    canonical_url, LaunchpadView, Navigation)
 from canonical.widgets import DateWidget
 from canonical.widgets.popup import PersonPickerWidget
 
@@ -65,6 +67,12 @@
         """See `IPersonalArchiveSubscription`."""
         return "Access to %s" % self.archive.displayname
 
+    @property
+    def title(self):
+        """Required for default headings in templates."""
+        return self.displayname
+
+
 def traverse_archive_subscription_for_subscriber(subscriber, archive_id):
     """Return the subscription for a subscriber to an archive."""
     subscription = None
@@ -111,6 +119,11 @@
     custom_widget('subscriber', PersonPickerWidget,
         header="Select the subscriber")
 
+    @property
+    def label(self):
+        """Return a label for the view's main heading."""
+        return "Manage access to " + self.context.title
+
     def initialize(self):
         """Ensure that we are dealing with a private archive."""
         # If this archive is not private, then we should not be
@@ -210,6 +223,11 @@
     custom_widget('description', TextWidget, displayWidth=40)
     custom_widget('date_expires', CustomWidgetFactory(DateWidget))
 
+    @property
+    def label(self):
+        """Return a label for the view's main heading."""
+        return "Edit " + self.context.displayname
+
     def validate_update_subscription(self, action, data):
         """Ensure that the date of expiry is not in the past."""
         form.getWidgetsData(self.widgets, 'field', data)
@@ -241,12 +259,12 @@
             self.context.subscriber.displayname)
         self.request.response.addNotification(notification)
 
-    @action(u'Cancel access', name='cancel')
+    @action(u'Revoke access', name='cancel')
     def cancel_subscription(self, action, data):
         """Cancel the context subscription."""
         self.context.cancel(self.user)
 
-        notification = "You have cancelled %s's subscription to %s." % (
+        notification = "You have revoked %s's access to %s." % (
             self.context.subscriber.displayname,
             self.context.archive.displayname)
         self.request.response.addNotification(notification)
@@ -265,6 +283,8 @@
 class PersonArchiveSubscriptionsView(LaunchpadView):
     """A view for displaying a persons archive subscriptions."""
 
+    label = "Private PPA access"
+
     @cachedproperty
     def subscriptions_with_tokens(self):
         """Return all the persons archive subscriptions with the token
@@ -294,6 +314,11 @@
     tokens.
     """
 
+    @property
+    def label(self):
+        """Return the label for the view's main heading."""
+        return self.context.title
+
     def initialize(self):
         """Process any posted actions."""
         super(PersonArchiveSubscriptionView, self).initialize()

=== modified file 'lib/lp/soyuz/browser/distroarchseriesbinarypackage.py'
--- lib/lp/soyuz/browser/distroarchseriesbinarypackage.py	2009-09-03 15:17:24 +0000
+++ lib/lp/soyuz/browser/distroarchseriesbinarypackage.py	2009-09-30 14:18:17 +0000
@@ -4,7 +4,6 @@
 __metaclass__ = type
 
 __all__ = [
-    'DistroArchSeriesBinaryPackageBreadcrumb',
     'DistroArchSeriesBinaryPackageNavigation',
     'DistroArchSeriesBinaryPackageView',
     ]
@@ -16,14 +15,6 @@
 from canonical.lazr.utils import smartquote
 
 
-class DistroArchSeriesBinaryPackageBreadcrumb(Breadcrumb):
-    """A breadcrumb for `DistroArchSeriesBinaryPackage`."""
-
-    @property
-    def text(self):
-        return self.context.name
-
-
 class DistroArchSeriesBinaryPackageOverviewMenu(ApplicationMenu):
 
     usedfor = IDistroArchSeriesBinaryPackage

=== modified file 'lib/lp/soyuz/browser/tests/archivesubscription-views.txt'
--- lib/lp/soyuz/browser/tests/archivesubscription-views.txt	2009-08-13 19:03:36 +0000
+++ lib/lp/soyuz/browser/tests/archivesubscription-views.txt	2009-09-30 14:18:17 +0000
@@ -30,11 +30,16 @@
     >>> transaction.commit()
     >>> logout()
 
+The view includes a label property.
+
+    >>> login('[e-mail]')
+    >>> view = create_initialized_view(cprov.archive, name="+subscriptions")
+    >>> print view.label
+    Manage access to PPA for Celso Providelo
+
 Initially the view does not display any subscribers, as can be seen
 using the has_subscriptions property:
 
-    >>> login('[e-mail]')
-    >>> view = create_initialized_view(cprov.archive, name="+subscriptions")
     >>> view.has_subscriptions
     False
 
@@ -166,8 +171,8 @@
 
 == ArchiveSubscriptionEditView ==
 
-The ArchiveSubscriptionEditView presents the expiry and description ready
-for editing, together with Update and Cancel actions:
+The ArchiveSubsriptionEditView includes a view label for the views main
+title.
 
     >>> login('[e-mail]')
     >>> from lp.soyuz.interfaces.archivesubscriber import (
@@ -175,12 +180,18 @@
     >>> spiv_subscription = getUtility(IArchiveSubscriberSet).getByArchive(
     ...     cprov.archive).one()
     >>> view = create_initialized_view(spiv_subscription, name="+edit")
+    >>> print view.label
+    Edit Andrew Bennetts's access to PPA for Celso Providelo
+
+The ArchiveSubscriptionEditView presents the expiry and description ready
+for editing, together with Update and Cancel actions:
+
     >>> view.field_names
     ['date_expires', 'description']
     >>> for action in view.actions:
     ...     print action.label
     Save
-    Cancel access
+    Revoke access
 
 The ArchiveSubscriptionEditView has a next_url helper property.
 
@@ -241,7 +252,7 @@
 
     >>> for notification in view.request.notifications:
     ...     print notification.message
-    You have cancelled Andrew Bennetts's subscription to PPA for
+    You have revoked Andrew Bennetts's access to PPA for
     Celso Providelo.
 
 Just uncancel the subscription before continuing on.
@@ -306,13 +317,18 @@
 This view displays a single subscription of a person, as well as the
 corresponding token information.
 
-Initially the subscription does not have an active token:
+The view includes a label to denifen its main heading.
 
     >>> from lp.soyuz.browser.archivesubscription import (
     ...     PersonalArchiveSubscription)
     >>> spiv_subscription = PersonalArchiveSubscription(
     ...     spiv_subscription.subscriber, spiv_subscription.archive)
     >>> view = create_initialized_view(spiv_subscription, name="+index")
+    >>> print view.label
+    Access to PPA for Celso Providelo
+
+Initially the subscription does not have an active token:
+
     >>> print view.active_token
     None
 

=== modified file 'lib/lp/soyuz/browser/tests/test_breadcrumbs.py'
--- lib/lp/soyuz/browser/tests/test_breadcrumbs.py	2009-09-02 16:32:06 +0000
+++ lib/lp/soyuz/browser/tests/test_breadcrumbs.py	2009-09-30 14:18:17 +0000
@@ -11,6 +11,8 @@
 from canonical.launchpad.webapp.tests.breadcrumbs import (
     BaseBreadcrumbTestCase)
 from lp.registry.interfaces.distribution import IDistributionSet
+from lp.soyuz.browser.archivesubscription import PersonalArchiveSubscription
+from lp.testing import login, login_person
 
 
 class TestDistroArchSeriesBreadcrumb(BaseBreadcrumbTestCase):
@@ -55,5 +57,37 @@
         self.assertEquals(texts[-1], "0.1-1")
 
 
+class TestArchiveSubscriptionBreadcrumb(BaseBreadcrumbTestCase):
+
+    def setUp(self):
+        super(TestArchiveSubscriptionBreadcrumb, self).setUp()
+
+        # Create a private ppa
+        self.ppa = self.factory.makeArchive()
+        login('[e-mail]')
+        self.ppa.private = True
+        self.ppa.buildd_secret = 'secret'
+
+        owner = self.ppa.owner
+        login_person(owner)
+        self.ppa_subscription = self.ppa.newSubscription(owner, owner)
+        self.ppa_token = self.ppa.newAuthToken(owner)
+        self.personal_archive_subscription = PersonalArchiveSubscription(
+            owner, self.ppa)
+
+    def test_personal_archive_subscription(self):
+        self.traversed_objects = [
+            self.root, self.ppa.owner, self.personal_archive_subscription]
+        subscription_url = canonical_url(self.personal_archive_subscription)
+
+        urls = self._getBreadcrumbsURLs(
+            subscription_url, self.traversed_objects)
+        texts = self._getBreadcrumbsTexts(
+            subscription_url, self.traversed_objects)
+
+        self.assertEquals(subscription_url, urls[-1])
+        self.assertEquals(
+            "Access to %s" % self.ppa.displayname, texts[-1])
+
 def test_suite():
     return unittest.TestLoader().loadTestsFromName(__name__)

=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml	2009-09-04 18:18:39 +0000
+++ lib/lp/soyuz/configure.zcml	2009-09-30 14:18:17 +0000
@@ -582,7 +582,7 @@
     <adapter
         for="lp.soyuz.interfaces.distroarchseriesbinarypackage.IDistroArchSeriesBinaryPackage"
         provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
-        factory="lp.soyuz.browser.distroarchseriesbinarypackage.DistroArchSeriesBinaryPackageBreadcrumb"
+        factory="canonical.launchpad.webapp.breadcrumb.NameBreadcrumb"
         permission="zope.Public" />
 
     <!-- PublishedPackage -->
@@ -647,6 +647,11 @@
         for="lp.soyuz.interfaces.archivesubscriber.IArchiveSubscriber"
         provides="lp.soyuz.browser.archivesubscription.IArchiveSubscriberUI"
         factory="lp.soyuz.browser.archivesubscription.archive_subscription_ui_adapter"/>
+    <adapter
+        provides="canonical.launchpad.webapp.interfaces.IBreadcrumb"
+        for="lp.soyuz.interfaces.archivesubscriber.IPersonalArchiveSubscription"
+        factory="canonical.launchpad.webapp.breadcrumb.DisplaynameBreadcrumb"
+        permission="zope.Public"/>
     <subscriber
         for="lp.soyuz.interfaces.archivesubscriber.IArchiveSubscriber
              lazr.lifecycle.interfaces.IObjectCreatedEvent"

=== modified file 'lib/lp/soyuz/interfaces/archivesubscriber.py'
--- lib/lp/soyuz/interfaces/archivesubscriber.py	2009-07-17 00:26:05 +0000
+++ lib/lp/soyuz/interfaces/archivesubscriber.py	2009-09-30 14:18:17 +0000
@@ -185,3 +185,6 @@
 
     displayname = TextLine(title=_("Subscription displayname"),
         required=False)
+
+    title = TextLine(title=_("Subscription title"),
+        required=False)

=== modified file 'lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt'
--- lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt	2009-08-13 19:03:36 +0000
+++ lib/lp/soyuz/stories/ppa/xx-private-ppa-subscription-stories.txt	2009-09-30 14:18:17 +0000
@@ -176,7 +176,7 @@
 
     >>> for msg in get_feedback_messages(cprov_browser.contents):
     ...     print msg
-    You have cancelled Launchpad Developers's subscription to PPA
+    You have revoked Launchpad Developers's access to PPA
     for Celso Providelo.
 
 
@@ -242,10 +242,10 @@
     >>> regeneration_info = find_tag_by_id(
     ...     joe_browser.contents, 'regenerate_token')
     >>> print(extract_text(regeneration_info))
-    Password security
+    Reset password
     If you believe...
+    Reset password
     Note: after ...
-    Generate new password
 
 When Joe clicks on the 'Generate new personal subscription' link then
 the page is redisplayed with new sources.list entries and a notification.

=== modified file 'lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt'
--- lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt	2009-08-13 19:03:36 +0000
+++ lib/lp/soyuz/stories/ppa/xx-private-ppa-subscriptions.txt	2009-09-30 14:18:17 +0000
@@ -54,6 +54,7 @@
     ...
     http://launchpad.dev/+icing/.../build/lp/calendar.js
     http://launchpad.dev/+icing/.../yui_2.7.0b/build/calendar/assets/skins/sam/calendar.css
+    ...
 
 Initially there are no subscriptions for a newly privatized PPA (although,
 this may need to change, to add the owner/team). A heading is displayed

=== modified file 'lib/lp/soyuz/templates/archive-subscriber-edit.pt'
--- lib/lp/soyuz/templates/archive-subscriber-edit.pt	2009-08-05 10:02:22 +0000
+++ lib/lp/soyuz/templates/archive-subscriber-edit.pt	2009-09-30 14:18:17 +0000
@@ -12,12 +12,6 @@
       use-macro="context/@@launchpad_widget_macros/yui2calendar-dependencies" />
   </metal:block>
 
-  <div metal:fill-slot="heading">
-      <h1 tal:content="CONTEXTS/fmt:pagetitle">
-        Edit access for Joe Smith
-      </h1>
-  </div>
-
   <div metal:fill-slot="main">
     <div class="top-portlet">
       <p>You can update the expiry or description for the granted access,

=== modified file 'lib/lp/soyuz/templates/archive-subscribers.pt'
--- lib/lp/soyuz/templates/archive-subscribers.pt	2009-08-05 10:02:22 +0000
+++ lib/lp/soyuz/templates/archive-subscribers.pt	2009-09-30 14:18:17 +0000
@@ -10,24 +10,26 @@
   <metal:block fill-slot="head_epilogue">
     <metal:yui-dependencies
       use-macro="context/@@launchpad_widget_macros/yui2calendar-dependencies" />
+
+    <tal:devmode condition="devmode">
+        <script type="text/javascript"
+                tal:attributes="src string:${lp_js}/soyuz/archivesubscribers_index.js"></script>
+    </tal:devmode>
   </metal:block>
 
-  <div metal:fill-slot="heading">
-    <h1 tal:content="CONTEXTS/fmt:pagetitle">
-      Manage access to Blah PPA
-    </h1>
-  </div>
   <div metal:fill-slot="main">
     <div class="top-portlet">
       <p>You can grant access to people or teams to install software
          from your PPA.
       </p>
 
-      <div class="subscribers">
-        <p tal:condition="not: view/has_subscriptions" id="no-subscribers">
-          No one has access to install software from this PPA.
-        </p>
-
+      <p tal:condition="not: view/has_subscriptions" id="no-subscribers">
+        No one has access to install software from this PPA.
+      </p>
+
+      <div id="add-subscriber-placeholder"></div>
+
+      <div id="subscribers">
         <p class="error message" tal:condition="view/errors"
            tal:content="view/error_count" />
 
@@ -44,13 +46,13 @@
                id="archive-subscribers" class="listing">
           <thead>
             <tr class="archive_subscriber_row">
-              <th>Name</th>
+              <th style="width:30%">Name</th>
               <th>Expires</th>
               <th colspan="2">Comment</th>
             </tr>
           </thead>
           <tbody>
-            <tr>
+            <tr class="add-subscriber" style="background-color:#eeeeff;">
               <tal:single-row-form define="widgets widgets|view/widgets"
                                    repeat="widget widgets">
 
@@ -95,8 +97,12 @@
         </table>
         </form>
       </div><!-- class="portlet" -->
+      <script type="text/javascript" id="setup-archivesubscribers-index">
+          YUI().use('soyuz.archivesubscribers_index', function(Y) {
+              Y.soyuz.setup_archivesubscribers_index();
+          });
+      </script>
     </div>
-
   </div>
 </body>
 </html>

=== modified file 'lib/lp/soyuz/templates/person-archive-subscription.pt'
--- lib/lp/soyuz/templates/person-archive-subscription.pt	2009-08-05 10:02:22 +0000
+++ lib/lp/soyuz/templates/person-archive-subscription.pt	2009-09-30 14:18:17 +0000
@@ -8,12 +8,6 @@
   >
 <body>
 
-  <div metal:fill-slot="heading">
-    <h1 tal:content="CONTEXTS/fmt:pagetitle">
-      Access to Joe Smith's private archive
-    </h1>
-  </div>
-
   <div metal:fill-slot="main">
 
   <tal:activate condition="not: view/active_token">
@@ -54,22 +48,23 @@
       </div> <!-- signing-key -->
     </div>
     <div id="regenerate_token" class="portlet" style="clear:both">
-      <h2>Password security</h2>
+      <h2>Reset password</h2>
       <p>If you believe the security of your password for this access
-         has been compromised, you can generate a new one. After you've
+         has been compromised, you reset your password. After you've
          requested a new password, you'll see new "sources.list" entries
          on this page. You'll need to update them on your computer.
       </p>
-      <p>Note: after requesting a new password for your personal
-         access &mdash; and updating your sources.list with the new
-         entries &mdash; you may need to wait for up to ten minutes before
-         you can download using the new password.
-      </p>
       <form action="" method="post">
         <button type="submit" name="regenerate" value="1">
-          Generate new password
+          Reset password
         </button>
       </form>
+      <p class="discreet">
+         Note: after resetting the password for your personal
+         access &mdash; and updating your sources.list with the new
+         entries &mdash; you may need to wait for up to ten minutes before
+         you can download using the new password.
+      </p>
     </div>
   </tal:active>
   </div>

=== modified file 'lib/lp/soyuz/templates/person-archive-subscriptions.pt'
--- lib/lp/soyuz/templates/person-archive-subscriptions.pt	2009-08-05 10:02:22 +0000
+++ lib/lp/soyuz/templates/person-archive-subscriptions.pt	2009-09-30 14:18:17 +0000
@@ -7,24 +7,20 @@
   i18n:domain="launchpad"
   >
 <body>
-  <div metal:fill-slot="heading">
-    <h1 tal:content="CONTEXTS/fmt:pagetitle">
-      Private PPA access
-    </h1>
-  </div>
 
   <div metal:fill-slot="main">
     <div class="top-portlet">
       <tal:has_subscriptions condition="view/subscriptions_with_tokens">
         <div id="current_subscriptions">
-          <p>Here's a list of all the private archives to which you have
-             been granted access.
+          <p>All the private archives to which you have
+             been granted access are listed below.
           </p>
           <table summary="CONTEXTS/fmt:pagetitle" id="archive-subscriptions"
-            class="listing">
+            class="listing" style="width:70%">
             <thead>
               <tr class="archive-subscription-row">
                 <th>Archive</th>
+                <th></th>
               </tr>
             </thead>
             <tbody>

=== added directory 'lib/lp/soyuz/windmill'
=== added file 'lib/lp/soyuz/windmill/__init__.py'
=== added file 'lib/lp/soyuz/windmill/testing.py'
--- lib/lp/soyuz/windmill/testing.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/windmill/testing.py	2009-09-30 14:18:17 +0000
@@ -0,0 +1,18 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Soyuz-specific testing infrastructure for Windmill."""
+
+__metaclass__ = type
+__all__ = [
+    'SoyuzWindmillLayer',
+    ]
+
+
+from canonical.testing.layers import BaseWindmillLayer
+
+
+class SoyuzWindmillLayer(BaseWindmillLayer):
+    """Layer for Soyuz Windmill tests."""
+
+    base_url = 'http://launchpad.dev:8085/'

=== added directory 'lib/lp/soyuz/windmill/tests'
=== added file 'lib/lp/soyuz/windmill/tests/__init__.py'
=== added file 'lib/lp/soyuz/windmill/tests/test_archivesubscribersindex.py'
--- lib/lp/soyuz/windmill/tests/test_archivesubscribersindex.py	1970-01-01 00:00:00 +0000
+++ lib/lp/soyuz/windmill/tests/test_archivesubscribersindex.py	2009-09-30 14:18:17 +0000
@@ -0,0 +1,96 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test for the archive subscribers index page.."""
+
+__metaclass__ = type
+__all__ = []
+
+import transaction
+import unittest
+
+from windmill.authoring import WindmillTestClient
+
+from zope.component import getUtility
+
+from canonical.launchpad.ftests import login, logout
+from canonical.launchpad.windmill.testing.lpuser import LaunchpadUser
+from lp.registry.interfaces.distribution import IDistributionSet
+from lp.soyuz.windmill.testing import SoyuzWindmillLayer
+from lp.testing import TestCaseWithFactory
+
+WAIT_PAGELOAD = u'30000'
+WAIT_ELEMENT_COMPLETE = u'30000'
+WAIT_CHECK_CHANGE = u'1000'
+ADD_ACCESS_LINK = u'//a[@class="js-action sprite add"]'
+CHOOSE_SUBSCRIBER_LINK = u'//a[@id="show-widget-field-subscriber"]'
+SUBSCRIBER_SEARCH_FIELD = (
+    u'//div[@id="yui-pretty-overlay-modal"]//input[@name="search"]')
+SUBSCRIBER_SEARCH_BUTTON = u'//div[@id="yui-pretty-overlay-modal"]//button'
+FIRST_SUBSCRIBER_RESULT = (
+    u'//div[@id="yui-pretty-overlay-modal"]'
+     '//span[@class="yui-picker-result-title"]')
+
+
+class TestArchiveSubscribersIndex(TestCaseWithFactory):
+
+    layer = SoyuzWindmillLayer
+
+    def setUp(self):
+        """Create a private PPA."""
+        super(TestArchiveSubscribersIndex, self).setUp()
+
+        user = self.factory.makePerson(
+            name='joe-bloggs', email='[e-mail]', password='joe',
+            displayname='Joe Bloggs')
+        ubuntu = getUtility(IDistributionSet)['ubuntu']
+        self.ppa = self.factory.makeArchive(
+            owner=user, name='myppa', distribution=ubuntu)
+
+        login('[e-mail]')
+        self.ppa.private = True
+        self.ppa.buildd_secret = 'secret'
+        logout()
+        transaction.commit()
+
+        self.lpuser = LaunchpadUser(
+            'Joe Bloggs', '[e-mail]', 'joe')
+
+    def test_add_subscriber(self):
+        """Test adding a private PPA subscriber.."""
+        client = WindmillTestClient('Adding private PPA subscribers.')
+        
+        self.lpuser.ensure_login(client)
+
+        client.open(url='http://launchpad.dev:8085/~joe-bloggs/'
+                        '+archive/myppa/+subscriptions')
+        client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
+
+        # Click on the JS add access action.
+        client.waits.forElement(xpath=ADD_ACCESS_LINK)
+        client.click(xpath=ADD_ACCESS_LINK)
+
+        # Open the picker, search for 'launchpad' and choose the first
+        # result
+        client.click(xpath=CHOOSE_SUBSCRIBER_LINK)
+        client.type(xpath=SUBSCRIBER_SEARCH_FIELD, text='launchpad')
+        client.click(xpath=SUBSCRIBER_SEARCH_BUTTON)
+
+        client.waits.forElement(xpath=FIRST_SUBSCRIBER_RESULT)
+        client.click(xpath=FIRST_SUBSCRIBER_RESULT)
+
+        # Add the new subscriber.
+        client.click(id='field.actions.add')
+        client.waits.forPageLoad(timeout=WAIT_PAGELOAD)
+
+        # And verify that the correct informational message is displayed.
+        # It would be nice if we could use ... here.
+        client.asserts.assertText(
+            xpath=u'//div[@class="informational message"]',
+            validator='You have granted access for Launchpad Developers '
+                      'to install software from PPA named myppa for Joe '
+                      'Bloggs. Members of Launchpad Developers will be '
+                      'notified of the access  via email.')
+
+def test_suite():
+    return unittest.TestLoader().loadTestsFromName(__name__)