Merge lp:~danilo/launchpad/add-subscription-link into lp:launchpad

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

[r=gmb][bug=752410] Provide 'add subscription' link on target:+subscriptions page along with some refactoring of structural-subscriptions.js.

= Provide 'add subscription' from target:+subscriptions =

Allow creating a target subscription from the subscriptions overview page.
Lots of refactoring of the existing code (mostly moving stuff around and generalizing so we can get all the data we need and render the stuff with the same code).

== Tests ==

No tests atm. Will do them as a separate branch.

== Demo and Q/A ==

https://devpad.canonical.com/~danilo/screencasts/add-subscriptions.ogv

https://launchpad.dev/firefox/+subscriptions

https://launchpad.dev/firefox/ ("Subscribe to bug mail" link keeps working)

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/browser/structuralsubscription.py
  lib/lp/bugs/templates/bugtarget-subscription-list.pt
  lib/lp/registry/javascript/structural-subscription.js

./lib/lp/registry/javascript/structural-subscription.js
     209: Line exceeds 78 characters.
     672: Line exceeds 78 characters.
     710: Line exceeds 78 characters.

[Graham Binns (gmb)]

Just one comment; otherwise this is good to land.

357 + .set('id', 'subscription-filter-'+filter_id.toString());
358 + filter_node.appendChild(Y.Node.create(
359 + '<div style="margin-top: 1em"></div>'));
360 + filter_node.appendChild(Y.Node.create(
361 + '<strong class="filter-name"></strong>'));
362 +
363 + if (!filter_info.subscriber_is_team ||
364 + filter_info.user_is_team_admin) {
365 + // User can edit the subscription.
366 + filter_node.appendChild(Y.Node.create(
367 + '<span style="float: right">'+
368 + '<a href="#" class="sprite modify edit js-action '+
369 + ' edit-subscription">'+
370 + ' Edit this subscription</a> or '+
371 + '<a href="#" class="sprite modify remove js-action '+
372 + ' delete-subscription">'+
373 + ' Unsubscribe</a></span>'));
374 + } else {
375 + // User cannot edit the subscription, because this is a
376 + // team and the user does not have admin privileges.
377 + filter_node.appendChild(Y.Node.create(
378 + '<span style="float: right"><em>'+
379 + 'You do not have privileges to change this subscription'+
380 + '</em></span>'));
381 + }
382 +
383 + var description_node = filter_node.appendChild(
384 + Y.Node.create(
385 + '<div style="padding-left: 1em"></div>')
386 + .set('id', 'filter-description-'+filter_id.toString()));
387 +

Unless there's a reason for setting the style manually, you should create a class for this and add it to the core CSS.

review: Approve (code)

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

As explained on IRC, this is just moved code. structural-subscriptions.js has more of style manipulations, I filed bug 752406 about it.

=== modified file 'lib/lp/bugs/browser/structuralsubscription.py'
--- lib/lp/bugs/browser/structuralsubscription.py	2011-04-04 18:51:17 +0000
+++ lib/lp/bugs/browser/structuralsubscription.py	2011-04-06 16:01:18 +0000
@@ -33,6 +33,7 @@
 from zope.traversing.browser import absoluteURL
 
 from canonical.launchpad.webapp.authorization import check_permission
+from canonical.launchpad.webapp.interfaces import NoCanonicalUrl
 from canonical.launchpad.webapp.menu import (
     enabled_with_permission,
     Link,
@@ -390,10 +391,13 @@
     expose_user_administered_teams_to_js(request, user, context)
     expose_enum_to_js(request, BugTaskImportance, 'importances')
     expose_enum_to_js(request, BugTaskStatus, 'statuses')
-    if subscriptions is None:
+    if subscriptions is None or len(list(subscriptions)) == 0:
         subscriptions = []
+        target = context
+    else:
+        target = None
     expose_user_subscriptions_to_js(
-        user, subscriptions, request)
+        user, subscriptions, request, target)
 
 
 def expose_enum_to_js(request, enum, name):
@@ -421,11 +425,12 @@
             info.append({
                 'link': absoluteURL(team, api_request),
                 'title': team.title,
+                'url': canonical_url(team),
             })
     IJSONRequestCache(request).objects['administratedTeams'] = info
 
 
-def expose_user_subscriptions_to_js(user, subscriptions, request):
+def expose_user_subscriptions_to_js(user, subscriptions, request, target=None):
     """Make the user's subscriptions available to JavaScript."""
     info = {}
     api_request = IWebServiceClientRequest(request)
@@ -433,6 +438,19 @@
         administered_teams = []
     else:
         administered_teams = user.getAdministratedTeams()
+
+    if target is not None:
+        try:
+            # No subscriptions, which means we are on a target
+            # subscriptions page. Let's at least provide target details.
+            target_info = {}
+            target_info['title'] = target.title
+            target_info['url'] = canonical_url(target, rootsite='mainsite')
+            IJSONRequestCache(request).objects['target_info'] = target_info
+        except NoCanonicalUrl:
+            # We export nothing if the target implements no canonical URL.
+            pass
+
     for subscription in subscriptions:
         target = subscription.target
         record = info.get(target)

=== modified file 'lib/lp/bugs/browser/tests/test_expose.py'
--- lib/lp/bugs/browser/tests/test_expose.py	2011-03-31 15:04:53 +0000
+++ lib/lp/bugs/browser/tests/test_expose.py	2011-04-06 16:01:18 +0000
@@ -132,7 +132,7 @@
         self.assertThat(len(team_info), Equals(expected_number_teams))
         # The items info consist of a dictionary with link and title keys.
         for i in range(expected_number_teams):
-            self.assertThat(team_info[i], KeysEqual('link', 'title'))
+            self.assertThat(team_info[i], KeysEqual('link', 'title', 'url'))
         # The link is the title of the team.
         self.assertThat(
             team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
@@ -165,7 +165,7 @@
         self.assertThat(len(team_info), Equals(expected_number_teams))
         # The items info consist of a dictionary with link and title keys.
         for i in range(expected_number_teams):
-            self.assertThat(team_info[i], KeysEqual('link', 'title'))
+            self.assertThat(team_info[i], KeysEqual('link', 'title', 'url'))
         # The link is the title of the team.
         self.assertThat(
             team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))
@@ -193,7 +193,7 @@
         self.assertThat(len(team_info), Equals(expected_number_teams))
         # The items info consist of a dictionary with link and title keys.
         for i in range(expected_number_teams):
-            self.assertThat(team_info[i], KeysEqual('link', 'title'))
+            self.assertThat(team_info[i], KeysEqual('link', 'title', 'url'))
         # The link is the title of the team.
         self.assertThat(
             team_info[0]['title'], Equals(u'Bug Supervisor Sub Team'))

=== modified file 'lib/lp/bugs/templates/bugtarget-subscription-list.pt'
--- lib/lp/bugs/templates/bugtarget-subscription-list.pt	2011-03-29 22:34:04 +0000
+++ lib/lp/bugs/templates/bugtarget-subscription-list.pt	2011-04-06 16:01:18 +0000
@@ -17,9 +17,13 @@
           request/features/malone.advanced-structural-subscriptions.enabled">
       LPS.use('lp.registry.structural_subscription', function(Y) {
           var module = Y.lp.registry.structural_subscription;
+          var config = {
+              content_box: "#structural-subscription-content-box",
+              add_filter_description: true};
           Y.on('domready', function() {
-              module.setup_bug_subscriptions(
-                  {content_box: "#structural-subscription-content-box"})
+              module.setup_bug_subscriptions(config);
+              module.setup_subscription_link(
+                  config, '#create-new-subscription');
           });
       });
     </script>
@@ -31,6 +35,12 @@
 
     <div id="maincontent">
       <div id="nonportlets" class="readable">
+        <div tal:condition="
+            features/malone.advanced-structural-subscriptions.enabled">
+          <a class="sprite add" id="create-new-subscription"
+             href="#">Add a subscription</a>
+        </div>
+
         <div id="subscription-listing"></div>
 
             <div id="structural-subscription-content-box"></div>

=== modified file 'lib/lp/registry/javascript/structural-subscription.js'
--- lib/lp/registry/javascript/structural-subscription.js	2011-04-04 16:22:20 +0000
+++ lib/lp/registry/javascript/structural-subscription.js	2011-04-06 16:01:18 +0000
@@ -164,20 +164,27 @@
  * @method create_structural_subscription filter
  * @param {Object} who Link to the user or team to be subscribed.
  * @param {Object} form_data The data returned from the form submission.
+ * @param {Object} success_callback Function to execute when filter is added.
  */
-function add_bug_filter(who, form_data) {
+function add_bug_filter(who, form_data, success_callback) {
     var config = {
         on: {success: function (bug_filter) {
                 // If we fail to PATCH the new bug filter, DELETE it.
-                var on = {failure: function () {
-                    // We use the namespace binding so tests can override
-                    // these functions.
-                    namespace._delete_filter(bug_filter);
-                    // Call the failure handler to report the original
-                    // error to the user.
-                    overlay_error_handler.getFailureHandler()
-                        .apply(this, arguments);
-                }};
+                var on = {
+                    failure: function () {
+                        // We use the namespace binding so tests can override
+                        // these functions.
+                        namespace._delete_filter(bug_filter);
+                        // Call the failure handler to report the original
+                        // error to the user.
+                        overlay_error_handler.getFailureHandler()
+                            .apply(this, arguments);
+                    },
+                    success: function (bug_filter) {
+                        success_callback(form_data, bug_filter);
+                        subscription_success(bug_filter);
+                    }
+                };
                 patch_bug_filter(bug_filter.getAttrs(), form_data, on);
             },
             failure: overlay_error_handler.getFailureHandler()
@@ -195,28 +202,31 @@
 namespace._add_bug_filter = add_bug_filter;
 
 /**
- * Given the form data from a user, save the subscription.
+ * Create a handler to save the subscription given the form data from a user.
  *
  * @private
- * @method save_subscription
- * @param {Object} form_data The data generated by the form submission.
+ * @method make_add_subscription_handler
+ * @param {Object} success_callback Function to execute on successful addition.
  */
-
-function save_subscription(form_data) {
-    var who;
-    var has_errors = check_for_errors_in_overlay(add_subscription_overlay);
-    if (has_errors) {
-        return false;
-    }
-    if (form_data.recipient[0] === 'user') {
-        who = LP.links.me;
-    } else {
-        // There can be only one.
-        who = form_data.team[0];
-    }
-    add_bug_filter(who, form_data);
+function make_add_subscription_handler(success_callback) {
+    var save_subscription = function(form_data) {
+        var who;
+        var has_errors = check_for_errors_in_overlay(
+            add_subscription_overlay);
+        if (has_errors) {
+            return false;
+        }
+        if (form_data.recipient[0] === 'user') {
+            who = LP.links.me;
+        } else {
+            // There can be only one.
+            who = form_data.team[0];
+        }
+        return add_bug_filter(who, form_data, success_callback);
+    };
+    return save_subscription;
 }
-namespace.save_subscription = save_subscription;
+namespace._make_add_subscription_handler = make_add_subscription_handler;
 
 function check_for_errors_in_overlay(overlay) {
     var has_errors = false;
@@ -245,6 +255,18 @@
 }
 
 /**
+ * Fill the filter name and description.
+ */
+function fill_filter_description(description_node, filter_info, filter) {
+    description_node
+        .empty()
+        .appendChild(create_filter_description(filter));
+    description_node.ancestor('.subscription-filter').one('.filter-name')
+        .empty()
+        .appendChild(render_filter_title(filter_info, filter));
+};
+
+/**
  * Handle the activation of the edit subscription link.
  */
 function edit_subscription_handler(context, form_data) {
@@ -254,14 +276,10 @@
         return false;
     }
     var on = {success: function (new_data) {
+        var description_node = Y.one(filter_id);
         var filter = new_data.getAttrs();
-        var description_node = Y.one(filter_id);
-        description_node
-            .empty()
-            .appendChild(create_filter_description(filter));
-        description_node.ancestor('.subscription-filter').one('.filter-name')
-            .empty()
-            .appendChild(render_filter_title(context.filter_info, filter));
+        fill_filter_description(
+            description_node, context.filter_info, filter);
         add_subscription_overlay.hide();
     }};
     patch_bug_filter(context.filter_info.filter, form_data, on);
@@ -279,7 +297,7 @@
  * Populate the overlay element with the contents of the add/edit form.
  */
 function create_overlay(content_box_id, overlay_id, submit_button,
-        submit_callback) {
+                        submit_callback, success_callback) {
     // Create the overlay.
     add_subscription_overlay = new Y.lazr.FormOverlay({
         headerContent:
@@ -540,14 +558,18 @@
         });
     });
     anim.on("end", function() {
-        node.setStyles({
-            height: 0,
-            visibility: 'hidden',
-            display: null
-            // Don't set display: none because then the node won't be taken
-            // into account and the rendering will sometimes jiggle
-            // horizontally when the node is opened.
-        });
+        if (user_cfg && user_cfg.remove_on_end === true) {
+            node.remove();
+        } else {
+            node.setStyles({
+                height: 0,
+                visibility: 'hidden',
+                display: null
+                // Don't set display: none because then the node won't be taken
+                // into account and the rendering will sometimes jiggle
+                // horizontally when the node is opened.
+            });
+        }
     });
     anim.run();
 }
@@ -881,13 +903,8 @@
     var recipient_label = content_node.one('input[name="recipient"] + span'),
         teams = LP.cache.administratedTeams;
     if (filter_info !== undefined && filter_info.subscriber_is_team) {
-        var i;
-        for (i=0; i<teams.length; i++) {
-            if (teams[i].link === filter_info.subscriber_link){
-                recipient_label.set('text', teams[i].title);
-                break;
-            }
-        }
+        var team = get_team(filter_info.subscriber_link);
+        recipient_label.set('text', team.title);
     } else {
         recipient_label.set('text', 'Yourself');
     }
@@ -1040,25 +1057,50 @@
         var y_config = {
             method: "POST",
             headers: {'X-HTTP-Method-Override': 'DELETE'},
-            on: {success: function(transactionid, response, args){
+            on: {
+                success: function(transactionid, response, args){
+                    var filter_node = Y.one(
+                        '#subscription-filter-'+filter_id.toString());
+                    filter_node.setStyle("margin-top", "0");
                     var subscriber = Y.one(
                         '#subscription-'+subscriber_id.toString());
-                    var to_collapse = subscriber;
                     var filters = subscriber.all('.subscription-filter');
-                    if (!filters.isEmpty()) {
-                        to_collapse = Y.one(
-                            '#subscription-filter-'+filter_id.toString());
-                        }
-                    collapse_node(to_collapse);
-                    },
-                 failure: error_handler.getFailureHandler()
-                }
-            };
+
+                    collapse_node(filter_node, { remove_on_end: true });
+                    if (filters.size() <= 1) {
+                        collapse_node(subscriber, { remove_on_end: true });
+                        LP.cache.subscription_info[subscriber_id].filters = [];
+                    }
+                },
+                failure: error_handler.getFailureHandler()
+            }
+        };
         Y.io(filter.self_link, y_config);
     };
 }
 
 /**
+ * Attach activation (click) handlers to links for a particular filter.
+ */
+function wire_up_edit_links_for_filter(
+    config, subscription, subscription_id, filter_info, filter_id,
+    filter_node) {
+    if (!filter_info.subscriber_is_team ||
+        filter_info.user_is_team_admin) {
+        var node = filter_node || Y.one(
+            '#subscription-filter-'+filter_id.toString());
+        var edit_link = node.one('a.edit-subscription');
+        var edit_handler = make_edit_handler(
+            subscription, filter_info, filter_id, config);
+        edit_link.on('click', edit_handler);
+        var delete_link = node.one('a.delete-subscription');
+        var delete_handler = make_delete_handler(
+            filter_info.filter, filter_id, subscription_id);
+        delete_link.on('click', delete_handler);
+    }
+}
+
+/**
  * Attach activation (click) handlers to all of the edit links on the page.
  */
 function wire_up_edit_links(config) {
@@ -1071,25 +1113,96 @@
         var sub = subscription_info[i];
         for (j=0; j<sub.filters.length; j++) {
             var filter_info = sub.filters[j];
-            if (!filter_info.subscriber_is_team ||
-                filter_info.user_is_team_admin) {
-                var node = Y.one(
-                    '#subscription-filter-'+filter_id.toString());
-                var edit_link = node.one('a.edit-subscription');
-                var edit_handler = make_edit_handler(
-                    sub, filter_info, filter_id, config);
-                edit_link.on('click', edit_handler);
-                var delete_link = node.one('a.delete-subscription');
-                var delete_handler = make_delete_handler(
-                    filter_info.filter, filter_id, i);
-                delete_link.on('click', delete_handler);
-            }
+            wire_up_edit_links_for_filter(
+                config, sub, i, filter_info, filter_id);
             filter_id += 1;
         }
     }
 }
 
 /**
+ * Create filter node to include in the subscription's filter listing.
+ */
+function create_filter_node(filter_id, filter_info, filter) {
+    var filter_node = Y.Node.create(
+        '<div style="margin: 1em 0em 0em 1em"'+
+            '      class="subscription-filter"></div>')
+        .set('id', 'subscription-filter-'+filter_id.toString());
+    filter_node.appendChild(Y.Node.create(
+        '<div style="margin-top: 1em"></div>'));
+    filter_node.appendChild(Y.Node.create(
+        '<strong class="filter-name"></strong>'));
+
+    if (!filter_info.subscriber_is_team ||
+        filter_info.user_is_team_admin) {
+        // User can edit the subscription.
+        filter_node.appendChild(Y.Node.create(
+            '<span style="float: right">'+
+                '<a href="#" class="sprite modify edit js-action '+
+                '    edit-subscription">'+
+                '  Edit this subscription</a> or '+
+                '<a href="#" class="sprite modify remove js-action '+
+                '    delete-subscription">'+
+                '  Unsubscribe</a></span>'));
+    } else {
+        // User cannot edit the subscription, because this is a
+        // team and the user does not have admin privileges.
+        filter_node.appendChild(Y.Node.create(
+            '<span style="float: right"><em>'+
+                'You do not have privileges to change this subscription'+
+                '</em></span>'));
+    }
+
+    var description_node = filter_node.appendChild(
+        Y.Node.create(
+            '<div style="padding-left: 1em"></div>')
+            .set('id', 'filter-description-'+filter_id.toString()));
+
+    fill_filter_description(description_node, filter_info, filter);
+
+    return filter_node;
+}
+
+/**
+ * Create a node with subscription description.
+ */
+function create_subscription_node(serial_id, subscription_data, filter_id) {
+    var node = Y.Node.create(
+        '<div style="margin-top: 2em; padding: 0 1em 1em 1em; '+
+            '      border: 1px solid #ddd;"></div>')
+        .set('id', 'subscription-'+serial_id.toString())
+    node.appendChild(Y.Node.create(
+        '  <span style="float: left; margin-top: -0.6em; '+
+            '      padding: 0 1ex; background-color: #fff;"></a>'))
+        .appendChild('<span>Subscriptions to </span>')
+        .appendChild(Y.Node.create('<a></a>')
+                     .set('href', subscription_data.target_url)
+                     .set('text', subscription_data.target_title));
+
+    // We can remove this once we enforce at least one filter per
+    // subscription.
+    if (subscription_data.filters.length === 0) {
+        node.appendChild(
+            '<div style="clear: both; padding: 1em 0 0 1em"></div>')
+            .appendChild('<strong>All messages</strong>');
+    }
+
+    for (j=0; j<subscription_data.filters.length; j++) {
+        var filter_info = subscription_data.filters[j];
+        var filter = filter_info.filter;
+        // We put the filters in the cache so that the patch mechanism
+        // can automatically find them and update them on a successful
+        // edit.  This makes it possible to open up a filter after an edit
+        // and see the information you expect to see.
+        LP.cache['structural-subscription-filter-'+filter_id.toString()] =
+            filter;
+        node.appendChild(create_filter_node(filter_id, filter_info, filter));
+        filter_id += 1;
+    };
+    return node;
+}
+
+/**
  * Populate the subscription list DOM element with subscription descriptions.
  */
 function fill_in_bug_subscriptions(config) {
@@ -1099,77 +1212,14 @@
     var subscription_info = LP.cache.subscription_info;
     var top_node = Y.Node.create(
         '<div class="yui-g"><div id="structural-subscriptions"></div></div>');
+    var i;
     var filter_id = 0;
-    var i;
-    var j;
     for (i=0; i<subscription_info.length; i++) {
-        var sub = subscription_info[i];
-        var sub_node = top_node.appendChild(Y.Node.create(
-            '<div style="margin-top: 2em; padding: 0 1em 1em 1em; '+
-            '      border: 1px solid #ddd;"></div>')
-            .set('id', 'subscription-'+i.toString()));
-        sub_node.appendChild(Y.Node.create(
-            '  <span style="float: left; margin-top: -0.6em; '+
-            '      padding: 0 1ex; background-color: #fff;"></a>'))
-            .appendChild('<span>Subscriptions to </span>')
-                .appendChild(Y.Node.create('<a></a>')
-                    .set('href', sub.target_url)
-                    .set('text', sub.target_title));
-
-        for (j=0; j<sub.filters.length; j++) {
-            var filter = sub.filters[j].filter;
-            // We put the filters in the cache so that the patch mechanism
-            // can automatically find them and update them on a successful
-            // edit.  This makes it possible to open up a filter after an edit
-            // and see the information you expect to see.
-            LP.cache['structural-subscription-filter-'+filter_id.toString()] =
-                filter;
-            var filter_node = sub_node.appendChild(Y.Node.create(
-                '<div style="margin: 1em 0em 0em 1em"'+
-                '      class="subscription-filter"></div>')
-                .set('id', 'subscription-filter-'+filter_id.toString()))
-                .appendChild(Y.Node.create(
-                    '<div style="margin-top: 1em"></div>'));
-            filter_node.appendChild(Y.Node.create(
-                '<strong class="filter-name"></strong>'))
-                .appendChild(render_filter_title(sub.filters[j], filter));
-
-            if (!sub.filters[j].subscriber_is_team ||
-                sub.filters[j].user_is_team_admin) {
-                // User can edit the subscription.
-                filter_node.appendChild(Y.Node.create(
-                    '<span style="float: right">'+
-                    '<a href="#" class="sprite modify edit js-action '+
-                    '    edit-subscription">'+
-                    '  Edit this subscription</a> or '+
-                    '<a href="#" class="sprite modify remove js-action '+
-                    '    delete-subscription">'+
-                    '  Unsubscribe</a></span>'));
-            } else {
-                // User cannot edit the subscription, because this is a
-                // team and the user does not have admin privileges.
-                filter_node.appendChild(Y.Node.create(
-                    '<span style="float: right"><em>'+
-                    'You do not have privileges to change this subscription'+
-                    '</em></span>'));
-            }
-
-            filter_node.appendChild(Y.Node.create(
-                '<div style="padding-left: 1em"></div>')
-                .set('id', 'filter-description-'+filter_id.toString()))
-                .appendChild(create_filter_description(filter));
-
-            filter_id += 1;
-        }
-
-        // We can remove this once we enforce at least one filter per
-        // subscription.
-        if (subscription_info[i].filters.length === 0) {
-            sub_node.appendChild(
-                '<div style="clear: both; padding: 1em 0 0 1em"></div>')
-                .appendChild('<strong>All messages</strong>');
-        }
+        top_node.appendChild(
+            create_subscription_node(i, subscription_info[i], filter_id));
+        filter_id += subscription_info[i].filters.length;
     }
+
     listing.appendChild(top_node);
 
     wire_up_edit_links(config);
@@ -1203,7 +1253,6 @@
  */
 function create_filter_description(filter) {
     var description = Y.Node.create('<div></div>');
-
     var filter_items = [];
     // Format status conditions.
     if (filter.statuses.length !== 0) {
@@ -1303,20 +1352,153 @@
 }
 
 /**
+ * Get team information.
+ */
+function get_team(url) {
+    var teams = LP.cache.administratedTeams;
+    var i;
+    for (i=0; i<teams.length; i++) {
+        if (teams[i].link === url) {
+            return teams[i];
+        }
+    }
+}
+
+/**
+ * Get team information from the submitted form data.
+ */
+function get_team_info(form_data) {
+    var is_team = (form_data.recipient[0] == "team");
+    var link, team, title, url;
+    if (is_team) {
+        link = form_data.team[0];
+        team = get_team(link);
+        if (team !== undefined) {
+            title = team.title;
+            url = team.url;
+        } else {
+            is_team = false;
+            link = LP.links.me;
+        }
+    }
+    return {
+        is_team: is_team,
+        link: link,
+        title: title,
+        url: url}
+}
+
+/**
+ * Get target details from either target_info or first subscription_info.
+ */
+function get_target_info() {
+    if (LP.cache.target_info !== undefined) {
+        return LP.cache.target_info
+    } else {
+        var info = LP.cache.subscription_info[0];
+        return {
+            title: info.target_title,
+            url: info.target_url}
+    }
+}
+
+/**
+ * Constructs filter info on-the-go.
+ */
+function construct_filter_info(filter, form_data, target_info) {
+    var team_info = get_team_info(form_data);
+
+    var filter_info = {
+        filter : filter,
+        subscriber_is_team: team_info.is_team,
+        user_is_team_admin: team_info.is_team,
+        subscriber_url: team_info.url,
+        subscriber_link: team_info.link,
+        subscriber_title: team_info.title,
+        target_title: target_info.title,
+        target_url: target_info.url
+    };
+    return filter_info;
+}
+
+/**
+ * Return a function that adds the newly created filter to the
+ * subscription's filter list after it has been saved.
+ */
+
+function make_add_subscription_success_handler(config) {
+    return function(form_data, filter) {
+        if (config.add_filter_description === true) {
+            // This way to figure out the ID works only
+            // if the page shows exactly one "target" (eg. Firefox).
+            var subscriber_id = 0;
+            var filter_id;
+            var subscriptions_list = Y.one(
+                '#subscription-' + subscriber_id.toString());
+
+            var filter_data = filter.getAttrs();
+            var target_info = get_target_info();
+            var filter_info = construct_filter_info(
+                filter_data, form_data, target_info);
+
+            /* Node to flash with green on success. */
+            var anim_node;
+
+            var subscription_info;
+            if (subscriptions_list === null) {
+                // No subscriptions are listed at all.
+                filter_id = 0;
+                subscription_info = {
+                    filters: [filter_info],
+                    target_url: target_info.url,
+                    target_title: target_info.title
+                };
+                LP.cache.subscription_info = [subscription_info]
+
+                subscriptions_list = create_subscription_node(
+                    0, subscription_info, 0);
+                var subscriptions = Y.one("#structural-subscriptions");
+                subscriptions.appendChild(subscriptions_list);
+                anim_node = subscriptions_list.one("#subscription-filter-0");
+            } else {
+                // There's at least one filter in the page.
+                subscription_info = LP.cache.subscription_info[0];
+                filter_id = subscription_info.filters.length;
+                subscription_info.filters.push(filter);
+
+                var description_node = create_filter_node(
+                    filter_id, filter_info, filter_data);
+                subscriptions_list.append(description_node);
+                anim_node = description_node;
+            }
+
+            wire_up_edit_links_for_filter(
+                config, subscription_info, 0,
+                filter_info, filter_id, anim_node);
+            Y.lazr.anim.green_flash({node: anim_node}).run();
+        }
+    }
+}
+
+/**
  * Show the overlay for creating a new subscription.
  */
 function show_add_overlay(config) {
-    Y.one(config.content_box).empty();
+    var content_node = Y.one(config.content_box);
+    content_node.empty();
     var overlay_id = setup_overlay(config.content_box);
-    clear_overlay(Y.one(config.content_box), false);
+    clear_overlay(content_node, false);
 
     var submit_button = Y.Node.create(
         '<button type="submit" name="field.actions.create" ' +
         'value="Create subscription" class="lazr-pos lazr-btn" '+
         '>OK</button>');
 
+    var success_callback = make_add_subscription_success_handler(config);
+
+    var save_subscription = make_add_subscription_handler(success_callback);
     create_overlay(config.content_box, overlay_id, submit_button,
-        save_subscription);
+                   save_subscription, success_callback);
     // We need to initialize the help links.  They may have already been
     // initialized except for the ones we added, so setupHelpTrigger
     // is idempotent.  Notice that this is old MochiKit code.
@@ -1337,7 +1519,7 @@
     // Modify the menu-link-subscribe-to-bug-mail link to be visible.
     var link = Y.one(link_id);
     if (!Y.Lang.isValue(link)) {
-        Y.fail('Link to set as the pop-up link not found.');
+        Y.Assert.fail('Link to set as the pop-up link not found.');
     }
     link.removeClass('invisible-link');
     link.addClass('visible-link');
@@ -1347,6 +1529,7 @@
     });
     link.addClass('js-action');
 }                               // setup_subscription_links
+namespace.setup_subscription_link = setup_subscription_link;
 
 /**
  * External entry point for configuring the structural subscription.
@@ -1366,6 +1549,6 @@
 }; // setup
 
 }, '0.1', {requires: [
-        'dom', 'node', 'lazr.anim', 'lazr.formoverlay', 'lazr.overlay',
-        'lazr.effects', 'lp.app.errors', 'lp.client', 'gallery-accordion'
-    ]});
+    'dom', 'node', 'test', 'lazr.anim', 'lazr.formoverlay', 'lazr.overlay',
+    'lazr.effects', 'lp.app.errors', 'lp.client', 'gallery-accordion'
+]});

=== modified file 'lib/lp/registry/javascript/tests/test_structural_subscription.js'
--- lib/lp/registry/javascript/tests/test_structural_subscription.js	2011-04-04 15:59:28 +0000
+++ lib/lp/registry/javascript/tests/test_structural_subscription.js	2011-04-06 16:01:18 +0000
@@ -202,6 +202,10 @@
                 recipient: ['user']
             };
             context = {};
+
+            // Get the save subscription handler with empty success handler.
+            this.save_subscription = module._make_add_subscription_handler(
+                function() {});
         },
 
         tearDown: function() {
@@ -216,7 +220,7 @@
             module.setup(this.configuration);
             module._show_add_overlay(this.configuration);
             this.form_data.recipient = ['user'];
-            module.save_subscription(this.form_data);
+            this.save_subscription(this.form_data);
             Assert.areEqual(
                 LP.links.me,
                 context.config.parameters.subscriber);
@@ -229,7 +233,7 @@
             module._show_add_overlay(this.configuration);
             this.form_data.recipient = ['team'];
             this.form_data.team = ['https://launchpad.dev/api/~super-team'];
-            module.save_subscription(this.form_data);
+            this.save_subscription(this.form_data);
             Assert.areEqual(
                 this.form_data.team[0],
                 context.config.parameters.subscriber);