← Back to team overview

launchpad-reviewers team mailing list archive

lp:~wallyworld/launchpad/notify-reviewer-subscribed-to-branches into lp:launchpad

 

Ian Booth has proposed merging lp:~wallyworld/launchpad/notify-reviewer-subscribed-to-branches into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #925219 in Launchpad itself: "Can request a review from someone who can't see the merge proposal - part 2"
  https://bugs.launchpad.net/launchpad/+bug/925219

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/notify-reviewer-subscribed-to-branches/+merge/91645

This branch warns the user if they create a mp where the reviewer can't see the source and/or target branches. What is does:

1. Changes the mp "Propose Merge" form submission to an XHR call. If the mp is created and there is no need to subscribe the reviewer to source/target branch, the page redirects as normal to the mp overview page.
If the reviewer would need to be subscribed to any source/target branch in order for them to see the branches, a confirmation popup is shown asking the user to confirm they really want to subscribe the reviewer. If they answer yes, the mp is created and the page redirects as normal. If no, the mp is not created and the user stays on the register mp page.

2. If the mp is created and subscriptions are required to be made, an Informational message is displayed to the user telling them what branches the reviewer has been subscribed to.

If there is no Javascript available, the user is not prompted to confirm but still gets to see the informational message. IOW, normal html form submissions do not abort if any subscriptions are required to be made. XHR submissions do.

If there are server-side validation errors (eg missing field error), then normally the html form submission causes the page to be re-rendered with the error(s) highlighted. So this behaviour is also replicated here - if the content type returned from the XHR call is html, then the document is replaced with the html and the errors are correctly highlighted.

== Demo and QA ==

Screenshot of confirmation popup when Proposal Merge is clicked:
http://people.canonical.com/~ianb/confirm-mp-register.png

== Tests ==

Add new tests to TestRegisterBranchMergeProposalView:
- test_register_ajax_request
- test_branch_visibility_notification

New yui tests were written. There was a mistake in the MockIO implementation that needed to be fixed also. The start and end callback for io operations weren't being invoked correctly. start needs to be invoked when the io request is first made, not after a response is received. And end wasn't being invoked at all.

== Lint ==

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/app/javascript/testing/mockio.js
  lib/lp/app/javascript/testing/tests/test_mockio.js
  lib/lp/code/browser/branch.py
  lib/lp/code/browser/tests/test_branchmergeproposal.py
  lib/lp/code/javascript/branchmergeproposal.nominate.js
  lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.html
  lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.js
-- 
https://code.launchpad.net/~wallyworld/launchpad/notify-reviewer-subscribed-to-branches/+merge/91645
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/notify-reviewer-subscribed-to-branches into lp:launchpad.
=== modified file 'lib/lp/app/javascript/testing/mockio.js'
--- lib/lp/app/javascript/testing/mockio.js	2011-10-17 19:17:00 +0000
+++ lib/lp/app/javascript/testing/mockio.js	2012-02-07 11:55:24 +0000
@@ -68,8 +68,8 @@
             response = this.response = new MockHttpResponse(response_config);
 
         // See the Y.io utility documentation for the signatures.
-        if (this.config.on.start !== undefined) {
-            this.config.on.start.call(context, tId, args);
+        if (this.config.on.end !== undefined) {
+            this.config.on.end.call(tId, args);
         }
         if (this.config.on.complete !== undefined) {
             this.config.on.complete.call(context, tId, response, args);
@@ -105,6 +105,10 @@
 
     /* Save the Y.io() arguments. */
     MockIo.prototype.io = function(url, config) {
+        // See the Y.io utility documentation for the signatures.
+        if (config.on.start !== undefined) {
+            config.on.start.call('mockTId', config['arguments'] || []);
+        }
         this.last_request = new MockHttpRequest(url, config);
         this.requests.push(this.last_request);
         return this;  // Usually this isn't used, except for logging.

=== modified file 'lib/lp/app/javascript/testing/tests/test_mockio.js'
--- lib/lp/app/javascript/testing/tests/test_mockio.js	2011-08-24 15:25:06 +0000
+++ lib/lp/app/javascript/testing/tests/test_mockio.js	2012-02-07 11:55:24 +0000
@@ -28,6 +28,7 @@
         this.test_config = {
             on: {
                 start: make_call_recorder(),
+                end: make_call_recorder(),
                 complete: make_call_recorder(),
                 success: make_call_recorder(),
                 failure: make_call_recorder()
@@ -46,8 +47,9 @@
     test_respond_success: function() {
         // The success handler is called on success.
         var mockio = this._make_mockio();
+        Y.Assert.areEqual(1, this.test_config.on.start.call_count);
         mockio.respond({status: 200});
-        Y.Assert.areEqual(1, this.test_config.on.start.call_count);
+        Y.Assert.areEqual(1, this.test_config.on.end.call_count);
         Y.Assert.areEqual(1, this.test_config.on.complete.call_count);
         Y.Assert.areEqual(1, this.test_config.on.success.call_count);
         Y.Assert.areEqual(0, this.test_config.on.failure.call_count);
@@ -56,8 +58,9 @@
     test_respond_failure: function() {
         // The failure handler is called on failure.
         var mockio = this._make_mockio();
+        Y.Assert.areEqual(1, this.test_config.on.start.call_count);
         mockio.respond({status: 500});
-        Y.Assert.areEqual(1, this.test_config.on.start.call_count);
+        Y.Assert.areEqual(1, this.test_config.on.end.call_count);
         Y.Assert.areEqual(1, this.test_config.on.complete.call_count);
         Y.Assert.areEqual(0, this.test_config.on.success.call_count);
         Y.Assert.areEqual(1, this.test_config.on.failure.call_count);

=== modified file 'lib/lp/code/browser/branch.py'
--- lib/lp/code/browser/branch.py	2012-02-03 22:03:43 +0000
+++ lib/lp/code/browser/branch.py	2012-02-07 11:55:24 +0000
@@ -48,6 +48,7 @@
 from lazr.restful.utils import smartquote
 from lazr.uri import URI
 import pytz
+import simplejson
 from zope.app.form import CustomWidgetFactory
 from zope.app.form.browser import TextAreaWidget
 from zope.app.form.browser.boolwidgets import CheckBoxWidget
@@ -115,6 +116,7 @@
     )
 from lp.code.interfaces.branch import (
     IBranch,
+    IBranchSet,
     user_has_special_branch_access,
     )
 from lp.code.interfaces.branchcollection import IAllBranches
@@ -135,7 +137,10 @@
     BranchFeedLink,
     FeedsMixin,
     )
-from lp.services.helpers import truncate_text
+from lp.services.helpers import (
+    english_list,
+    truncate_text,
+    )
 from lp.services.propertycache import cachedproperty
 from lp.services.webapp import (
     canonical_url,
@@ -1345,6 +1350,20 @@
         if reviewer is not None:
             review_requests.append((reviewer, review_type))
 
+        branch_names = [branch.unique_name
+                        for branch in [source_branch, target_branch]]
+        visibility_info = getUtility(IBranchSet).getBranchVisibilityInfo(
+            self.user, reviewer, branch_names)
+        visible_branches = list(visibility_info['visible_branches'])
+        if self.request.is_ajax and len(visible_branches) < 2:
+            self.request.response.setHeader(
+                'Content-Type', 'application/json')
+            return simplejson.dumps({
+                'person_name': visibility_info['person_name'],
+                'branches_to_check': branch_names,
+                'visible_branches': visible_branches,
+            })
+
         try:
             proposal = source_branch.addLandingTarget(
                 registrant=registrant, target_branch=target_branch,
@@ -1354,6 +1373,14 @@
                 review_requests=review_requests,
                 commit_message=data.get('commit_message'))
             self.next_url = canonical_url(proposal)
+            if len(visible_branches) < 2:
+                invisible_branches = [branch.unique_name
+                            for branch in [source_branch, target_branch]
+                            if branch.unique_name not in visible_branches]
+                self.request.response.addNotification(
+                    'To ensure visibility, %s is now subscribed to: %s'
+                    % (visibility_info['person_name'],
+                       english_list(invisible_branches)))
         except InvalidBranchMergeProposal, error:
             self.addError(str(error))
 

=== modified file 'lib/lp/code/browser/tests/test_branchmergeproposal.py'
--- lib/lp/code/browser/tests/test_branchmergeproposal.py	2012-02-01 19:39:13 +0000
+++ lib/lp/code/browser/tests/test_branchmergeproposal.py	2012-02-07 11:55:24 +0000
@@ -16,6 +16,7 @@
 
 from lazr.restful.interfaces import IJSONRequestCache
 import pytz
+import simplejson
 from soupmatchers import (
     HTMLContains,
     Tag,
@@ -412,9 +413,9 @@
         self.user = self.factory.makePerson()
         login_person(self.user)
 
-    def _makeTargetBranch(self):
+    def _makeTargetBranch(self, **kwargs):
         return self.factory.makeProductBranch(
-            product=self.source_branch.product)
+            product=self.source_branch.product, **kwargs)
 
     def _makeTargetBranchWithReviewer(self):
         albert = self.factory.makePerson(name='albert')
@@ -422,10 +423,11 @@
             reviewer=albert, product=self.source_branch.product)
         return target_branch, albert
 
-    def _createView(self):
+    def _createView(self, request=None):
         # Construct the view and initialize it.
-        view = RegisterBranchMergeProposalView(
-            self.source_branch, LaunchpadTestRequest())
+        if not request:
+            request = LaunchpadTestRequest()
+        view = RegisterBranchMergeProposalView(self.source_branch, request)
         view.initialize()
         return view
 
@@ -463,6 +465,31 @@
         self.assertOnePendingReview(proposal, target_branch.owner)
         self.assertIs(None, proposal.description)
 
+    def test_register_ajax_request(self):
+        # Ajax submits return json data containing info about what the visible
+        # branches are if they are not all visible to the reviewer.
+
+        # Make a branch the reviewer cannot see.
+        owner = self.factory.makePerson()
+        target_branch = self._makeTargetBranch(owner=owner, private=True)
+        reviewer = self.factory.makePerson()
+        extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+        request = LaunchpadTestRequest(
+            method='POST', principal=owner, **extra)
+        view = self._createView(request=request)
+        with person_logged_in(owner):
+            branches_to_check = [self.source_branch.unique_name,
+                target_branch.unique_name]
+            expected_data = {
+                'person_name': reviewer.displayname,
+                'branches_to_check': branches_to_check,
+                'visible_branches': [self.source_branch.unique_name]}
+            result_data = view.register_action.success(
+                {'target_branch': target_branch,
+                 'reviewer': reviewer,
+                 'needs_review': True})
+        self.assertEqual(expected_data, simplejson.loads(result_data))
+
     def test_register_work_in_progress(self):
         # The needs review checkbox can be unchecked to create a work in
         # progress proposal.
@@ -584,6 +611,26 @@
         matcher = Not(HTMLContains(reviewer.within(extra)))
         self.assertThat(browser.contents, matcher)
 
+    def test_branch_visibility_notification(self):
+        # If the reviewer cannot see the source and/or target branches, a
+        # notification message is displayed.
+        owner = self.factory.makePerson()
+        target_branch = self._makeTargetBranch(
+            private=True, owner=owner)
+        reviewer = self.factory.makePerson()
+        with person_logged_in(owner):
+            view = self._createView()
+            view.register_action.success(
+                {'target_branch': target_branch,
+                 'reviewer': reviewer,
+                 'needs_review': True})
+
+        (notification,) = view.request.response.notifications
+        self.assertThat(
+            notification.message, MatchesRegex(
+                'To ensure visibility, .* is now subscribed to:.*'))
+        self.assertEqual(BrowserNotificationLevel.INFO, notification.level)
+
 
 class TestBranchMergeProposalResubmitView(TestCaseWithFactory):
     """Test BranchMergeProposalResubmitView."""

=== modified file 'lib/lp/code/javascript/branchmergeproposal.nominate.js'
--- lib/lp/code/javascript/branchmergeproposal.nominate.js	2012-02-03 00:29:01 +0000
+++ lib/lp/code/javascript/branchmergeproposal.nominate.js	2012-02-07 11:55:24 +0000
@@ -12,6 +12,35 @@
 var lp_client;
 
 /**
+ * Helper method to make the confirmation prompt for branch visibility.
+ * @param branches_to_check
+ * @param branch_info
+ */
+var _make_confirm_propmpt = function(branches_to_check, branch_info) {
+    var visible_branches = branch_info.visible_branches;
+    var invisible_branches = branches_to_check.filter(function(i) {
+        return visible_branches.indexOf(i) < 0;
+    });
+    return Y.lp.mustache.to_html([
+    "<p class='large-warning' style='padding:12px 2px 0 36px;'>",
+    "{{person_name}} does not currently have permission to ",
+    "view branches:",
+    "<ul style='margin-left: 50px'>",
+    "    {{#invisible_branches}}",
+    "        <li>{{.}}</li>",
+    "    {{/invisible_branches}}",
+    "</ul>",
+    "</p>",
+    "<p>If you proceed, {{person_name}} will be subscribed to the " +
+    "branches.</p>",
+    "<p>Do you really want to nominate them as a reviewer?</p>"
+    ].join(''), {
+        invisible_branches: invisible_branches,
+        person_name: branch_info.person_name
+    });
+};
+
+/**
  * Picker validation callback which confirms that the nominated reviewer can
  * be given visibility to the specified branches.
  * @param branches_to_check
@@ -25,26 +54,8 @@
     var visible_branches = branch_info.visible_branches;
     if (Y.Lang.isArray(visible_branches)
             && visible_branches.length !== branches_to_check.length) {
-        var invisible_branches = branches_to_check.filter(function(i) {
-            return visible_branches.indexOf(i) < 0;
-        });
-        var yn_content = Y.lp.mustache.to_html([
-        "<p class='large-warning' style='padding:12px 2px 0 36px;'>",
-        "{{person_name}} does not currently have permission to ",
-        "view branches:",
-        "<ul style='margin-left: 50px'>",
-        "    {{#invisible_branches}}",
-        "        <li>{{.}}</li>",
-        "    {{/invisible_branches}}",
-        "</ul>",
-        "</p>",
-        "<p>If you proceed, {{person_name}} will be subscribed to the " +
-        "branches.</p>",
-        "<p>Do you really want to nominate them as a reviewer?</p>"
-        ].join(''), {
-            invisible_branches: invisible_branches,
-            person_name: branch_info.person_name
-        });
+        var yn_content = _make_confirm_propmpt(
+            branches_to_check, branch_info);
         Y.lp.app.picker.yesno_save_confirmation(
                 picker, yn_content, "Nominate", "Choose Again",
                 save_fn, cancel_fn);
@@ -55,6 +66,13 @@
     }
 };
 
+    /**
+     * The validation plugin for the reviewer picker.
+     * @param picker
+     * @param value
+     * @param save_fn
+     * @param cancel_fn
+     */
 var check_reviewer_can_see_branches = function(picker, value, save_fn,
                                                cancel_fn) {
     if (value === null || !Y.Lang.isValue(value.api_uri)) {
@@ -93,6 +111,119 @@
     lp_client.named_get("/branches", "getBranchVisibilityInfo", y_config);
 };
 
+/**
+ * Display a confirmation prompt asking the user if the really want to grant
+ * visibility to the source and/or target branches for the nominated reviewer.
+ * If the user answers 'yes', we cause a form submission as for a regular
+ * merge proposal registration.
+ * @param branch_info the result of making the getBranchVisibilityInfo call.
+ */
+var confirm_reviewer_nomination = function(branch_info) {
+    var branches_to_check = branch_info.branches_to_check;
+    var yn_content = _make_confirm_propmpt(branches_to_check, branch_info);
+    var co = new Y.lp.app.confirmationoverlay.ConfirmationOverlay({
+        submit_fn: function() {
+            var form = Y.one("[name='launchpadform']");
+            var dispatcher = Y.Node.create('<input>')
+                .set('type', 'hidden')
+                .addClass('hidden-input')
+                .set('name', 'field.actions.register')
+                .set('value', 'Propose Merge');
+            form.append(dispatcher);
+            form.submit();
+        },
+        form_content: yn_content,
+        headerContent: '<h2>Confirm reviewer nomination</h2>'
+    });
+    co.show();
+};
+
+/**
+ * Show the submit spinner.
+ *
+ * @method _showSubmitSpinner
+ */
+var _showSubmitSpinner = function(submit_link) {
+    var spinner_node = Y.Node.create(
+    '<img class="spinner" src="/@@/spinner" alt="Deleting..." />');
+    submit_link.insert(spinner_node, 'after');
+};
+
+/**
+ * Hide the submit spinner.
+ *
+ * @method _hideSubmitSpinner
+ */
+var _hideSubmitSpinner = function(submit_link) {
+    var spinner = submit_link.get('parentNode').one('.spinner');
+    if (spinner !== null) {
+        spinner.remove(true);
+    }
+};
+
+/**
+ * Replace the entire document contents with the new html.
+ * @param html
+ */
+var render_new_page = function(html) {
+    document.open();
+    document.write(html);
+    document.close();
+};
+
+/**
+ * Wire up the register mp submit button.
+ * @param io_provider
+ */
+var setup_nominate_submit = function(io_provider) {
+    Y.lp.client.remove_notifications();
+    var error_handler = new Y.lp.client.ErrorHandler();
+    var submit_link = Y.one("[name='field.actions.register']");
+    error_handler.showError = Y.bind(function (error_msg) {
+        _hideSubmitSpinner(submit_link);
+        Y.lp.app.errors.display_error(undefined, error_msg);
+    }, this);
+
+    var base_url = LP.cache.context.web_link;
+    var submit_url = base_url + "/+register-merge";
+    var form = Y.one("[name='launchpadform']");
+    form.on('submit', function(e) {
+        e.preventDefault();
+        var y_config = {
+            method: "POST",
+            headers: {'Accept': 'application/json;'},
+            on: {
+                start:
+                    Y.bind(_showSubmitSpinner, namespace, submit_link),
+                end:
+                    Y.bind(_hideSubmitSpinner, namespace, submit_link),
+                failure: error_handler.getFailureHandler(),
+                success:
+                    function(id, response) {
+                        var content_type
+                            = response.getResponseHeader('Content-Type');
+                        // If we get html back, that means there's been an
+                        // error detected on the server and so we need to
+                        // display the error page.
+                        if (content_type.indexOf('application/json') < 0) {
+                            namespace.render_new_page(response.responseText);
+                        } else {
+                            var branch_info = Y.JSON.parse(response.responseText);
+                            namespace.confirm_reviewer_nomination(branch_info);
+                        }
+                    }
+            }
+        };
+        var form_data = {};
+        form.all("[name^='field.']").each(function(field) {
+            form_data[field.get('name')] = field.get('value');
+        });
+        form_data.id = form;
+        y_config.form = form_data;
+        io_provider.io(submit_url, y_config);
+    });
+};
+
 var setup_reviewer_confirmation = function() {
     var validation_namespace = Y.namespace('lp.app.picker.validation');
     var widget_id = 'show-widget-field-reviewer';
@@ -106,6 +237,9 @@
 namespace.setup_reviewer_confirmation = setup_reviewer_confirmation;
 namespace.check_reviewer_can_see_branches = check_reviewer_can_see_branches;
 namespace.confirm_reviewer = confirm_reviewer;
+namespace.confirm_reviewer_nomination = confirm_reviewer_nomination;
+namespace.setup_nominate_submit = setup_nominate_submit;
+namespace.render_new_page = render_new_page;
 
 // We want to disable the review_type field if no reviewer is
 // specified. In such cases, the reviewer will be set by the back end
@@ -118,6 +252,7 @@
 
 namespace.setup = function(conf) {
     lp_client = new Y.lp.client.Launchpad(conf);
+    var io_provider = Y.lp.client.get_configured_io_provider(conf);
     Y.on('blur',
       function(e) {
         reviewer_changed(e.target.get('value'));
@@ -127,7 +262,9 @@
     reviewer_changed(f.value);
 
     setup_reviewer_confirmation();
+    setup_nominate_submit(io_provider);
 };
 
-}, "0.1", {"requires": ["io", "substitute", "dom", "node",
-   "event", "lp.client", "lp.mustache", "lp.app.picker"]});
+}, "0.1", {"requires": ["io", "substitute", "dom", "node", "json",
+   "event", "lp.client", "lp.mustache", "lp.app.picker",
+   "lp.app.confirmationoverlay"]});

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.html'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.html	2012-02-03 00:29:01 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.html	2012-02-07 11:55:24 +0000
@@ -41,10 +41,11 @@
 <body class="yui3-skin-sam">
     <div id="fixture"></div>
     <script type="text/x-template" id="form-template">
-      <form>
+      <form name='launchpadform'>
           <input type="text" name="field.target_branch.target_branch" id="field.target_branch.target_branch" value=""/>
           <input type="text" name="field.reviewer" id="field.reviewer" value=""/>
           <input type="text" name="field.review_type" id="field.review_type" value="" disabled="disabled"/>
+          <input type="submit" class="button" value="Propose Merge" name="field.actions.register" id="field.actions.register"/>
       </form>
     </script>
 </body>

=== modified file 'lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.js'
--- lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.js	2012-02-03 00:29:01 +0000
+++ lib/lp/code/javascript/tests/test_branchmergeproposal.nominate.js	2012-02-07 11:55:24 +0000
@@ -19,10 +19,7 @@
  *
  */
 
-suite.add(new Y.Test.Case({
-
-    name: 'branchmergeproposal-nominate-tests',
-
+var TestMixin = {
     setUp: function() {
         this.fixture = Y.one('#fixture');
         var form = Y.Node.create(Y.one('#form-template').getContent());
@@ -32,6 +29,7 @@
             links: {me : "/~user"},
             cache: {
                 context: {
+                    web_link: 'https://code.launchpad.dev/~someone/b2',
                     unique_name: 'b2'
                 }
             }
@@ -46,7 +44,12 @@
         delete this.fixture;
         delete this.mockio;
         delete window.LP;
-    },
+    }
+};
+
+suite.add(new Y.Test.Case(Y.merge(TestMixin, {
+    name: 'branchmergeproposal-nominate-reviewer-picker-tests',
+
 
     // The module setup function works as expected.
     test_setup: function() {
@@ -83,10 +86,12 @@
     test_check_reviewer_can_see_branches: function() {
         var orig_confirm_reviewer = module.confirm_reviewer;
         var dummy_picker = {};
+        var dummy_save_fn = function() {};
         var confirm_reviewer_called = false;
         module.confirm_reviewer = function(
             branches_to_check, branch_info, picker, save_fn, cancel_fn) {
             Y.Assert.areEqual(dummy_picker, picker);
+            Y.Assert.areEqual(dummy_save_fn, save_fn);
             Y.Assert.areEqual('Fred', branch_info.person_name);
             Y.Assert.areEqual('b2', branch_info.visible_branches[0]);
             Y.Assert.areEqual('b2', branches_to_check[0]);
@@ -96,14 +101,9 @@
         var selected_value = {
             api_uri: '~fred'
         };
-        var target_branch_field =
-            Y.DOM.byId('field.target_branch.target_branch');
-        target_branch_field.value = 'b1';
-        var save_fn = function() {
-
-        };
+        Y.DOM.byId('field.target_branch.target_branch').value = 'b1';
         module.check_reviewer_can_see_branches(
-            dummy_picker, selected_value, save_fn);
+            dummy_picker, selected_value, dummy_save_fn);
         this.mockio.success({
             responseText:
                 '{"person_name": "Fred", "visible_branches": ["b2"]}',
@@ -183,8 +183,104 @@
         Y.Assert.isFalse(result.save_called);
         Y.Assert.isTrue(result.yesno_called);
     }
-}));
-
+})));
+
+suite.add(new Y.Test.Case(Y.merge(TestMixin, {
+
+    name: 'branchmergeproposal-nominate-propose-merge-tests',
+
+    // Test that the Propose Merge submit button is re-wired to perform an XHR
+    // call and that the correct data is passed and the expected callback is
+    // invoked wit hthe returned data.
+    test_setup_nominate_submit: function() {
+        var orig_confirm_reviewer_nomination
+            = module.confirm_reviewer_nomination;
+        var confirm_reviewer_nomination_called = false;
+        module.confirm_reviewer_nomination = function(branch_info) {
+            Y.Assert.areEqual('Fred', branch_info.person_name);
+            confirm_reviewer_nomination_called = true;
+        };
+
+        module.setup_nominate_submit(this.mockio);
+        Y.DOM.byId('field.target_branch.target_branch').value = 'b1';
+        Y.DOM.byId('field.reviewer').value = 'mark';
+        Y.DOM.byId('field.review_type').value = 'code';
+
+        var submit_button = Y.one("[name='field.actions.register']");
+        submit_button.simulate('click');
+        Y.Assert.isNotNull(this.fixture.one('.spinner'));
+        Y.Assert.areEqual(
+            'https://code.launchpad.dev/~someone/b2/+register-merge',
+            this.mockio.last_request.url);
+        var form = Y.one("[name='launchpadform']");
+        var self = this;
+        form.all("[name^='field.']").each(function(field) {
+            Y.Assert.areEqual(
+                field.get('value'),
+                self.mockio.last_request.config.form[field.get('name')]);
+        });
+        this.mockio.success({
+            responseText: '{"person_name": "Fred"}',
+            responseHeaders: {'Content-Type': 'application/json'}});
+        Y.Assert.isTrue(confirm_reviewer_nomination_called);
+        module.confirm_reviewer_nomination = orig_confirm_reviewer_nomination;
+    },
+
+    // Test the confirmation prompt when a mp is submitted and the reviewer
+    // needs to be subscribed to the source and/or target branch.
+    test_confirm_reviewer_nomination: function() {
+        var branch_info = {
+            branches_to_check: ['b1', 'b2'],
+            visible_branches: ['b1'],
+            person_name: 'Fred'
+        };
+        module.confirm_reviewer_nomination(branch_info);
+        var confirmation_overlay_node
+            = Y.one('.yui3-lp-app-confirmationoverlay-content');
+        var confirmation_content_node
+            = confirmation_overlay_node.one('p.large-warning');
+        var confirmation_content = confirmation_content_node.get('text');
+        Y.Assert.isTrue(
+            confirmation_content.indexOf(
+                'Fred does not currently have permission to view branches:')
+                >= 0);
+        var form_submitted = false;
+        var form = Y.one("[name='launchpadform']");
+        var orig_submit = form.submit;
+        form.submit = function(e) {
+            form_submitted = true;
+        };
+        var ok_button = Y.one('.yui3-lazr-formoverlay-actions .ok-btn');
+        ok_button.simulate('click');
+        Y.Assert.isTrue(form_submitted);
+        form.submit = orig_submit;
+    },
+
+    // Test that when an validation error occurs during the io call, the html
+    // returned by the call is properly rendered.
+    test_confirm_reviewer_nomination_error: function() {
+        var orig_render_new_page
+            = module.render_new_page;
+        var render_page_called = false;
+        module.render_new_page = function(html) {
+            Y.Assert.areEqual('<html>Hello World</html>', html);
+            render_page_called = true;
+        };
+
+        module.setup_nominate_submit(this.mockio);
+        Y.DOM.byId('field.target_branch.target_branch').value = 'b1';
+        Y.DOM.byId('field.reviewer').value = 'mark';
+        Y.DOM.byId('field.review_type').value = 'code';
+
+        var submit_button = Y.one("[name='field.actions.register']");
+        submit_button.simulate('click');
+        this.mockio.success({
+            responseText: '<html>Hello World</html>',
+            responseHeaders: {'Content-Type': 'text/html'}});
+        Y.Assert.isTrue(render_page_called);
+        module.render_new_page = orig_render_new_page;
+    }
+})));
 
 Y.lp.testing.Runner.run(suite);