launchpad-reviewers team mailing list archive

[Ian Booth <ian.booth@xxxxxxxxxxxxx>]

Ian Booth has proposed merging lp:~wallyworld/launchpad/double-submit-940168 into lp:launchpad.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #940168 in Launchpad itself: "Two merge requests created on double-click"
  https://bugs.launchpad.net/launchpad/+bug/940168

For more details, see:
https://code.launchpad.net/~wallyworld/launchpad/double-submit-940168/+merge/123199

== Implementation ==

In a couple of places where buttons invoke XHR calls, and nothing else is done to prevent double submissions, disable the buttons while the call is in progress.

Fix an error in LaunchpadFormView where errors encountered during action processing for XHR requests were not being handled correctly, resulting in the call failing but no errors being displayed.

== Tests ==

Add test to test_launchpadform

== Lint ==

Linting changed files:
  lib/lp/app/browser/launchpadform.py
  lib/lp/app/browser/tests/test_launchpadform.py
  lib/lp/bugs/javascript/bug_picker.js
  lib/lp/code/javascript/branchmergeproposal.nominate.js

-- 
https://code.launchpad.net/~wallyworld/launchpad/double-submit-940168/+merge/123199
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wallyworld/launchpad/double-submit-940168 into lp:launchpad.

=== modified file 'lib/lp/app/browser/launchpadform.py'
--- lib/lp/app/browser/launchpadform.py	2012-03-27 04:36:24 +0000
+++ lib/lp/app/browser/launchpadform.py	2012-09-07 00:23:25 +0000
@@ -138,6 +138,9 @@
                 self.request.response.redirect(self.next_url)
         if self.request.is_ajax:
             self._processNotifications(self.request)
+            if errors:
+                self.form_result = form_action.failure(data, errors)
+                self._abort()
         self.action_taken = form_action
 
     def _processNotifications(self, request):

=== modified file 'lib/lp/app/browser/tests/test_launchpadform.py'
--- lib/lp/app/browser/tests/test_launchpadform.py	2012-03-27 04:36:24 +0000
+++ lib/lp/app/browser/tests/test_launchpadform.py	2012-09-07 00:23:25 +0000
@@ -207,11 +207,14 @@
     @action('Test', name='test',
         failure=LaunchpadFormView.ajax_failure_handler)
     def test_action(self, action, data):
-        pass
+        single_line_value = data['single_line']
+        if single_line_value == 'success':
+            return
+        self.addError("An action error")
 
     def validate(self, data):
         single_line_value = data['single_line']
-        if single_line_value == 'success':
+        if single_line_value != 'error':
             return
         self.setFieldError('single_line', 'An error occurred')
         self.addError('A form error')
@@ -264,3 +267,20 @@
         view = TestFormView({}, request)
         view.initialize()
         self.assertIsNone(view.form_result)
+
+    def test_ajax_action_failure(self):
+        # When there are errors performing the action, these are recorded.
+        extra = {'HTTP_X_REQUESTED_WITH': 'XMLHttpRequest'}
+        request = LaunchpadTestRequest(
+            method='POST',
+            form={
+                'field.actions.test': 'Test',
+                'field.single_line': 'failure'},
+            **extra)
+        view = TestFormView({}, request)
+        view.initialize()
+        self.assertEqual(
+                {"error_summary": "There is 1 error.",
+                 "errors": {},
+                 "form_wide_errors": ["An action error"]},
+            simplejson.loads(view.form_result))

=== modified file 'lib/lp/bugs/javascript/bug_picker.js'
--- lib/lp/bugs/javascript/bug_picker.js	2012-08-27 20:13:49 +0000
+++ lib/lp/bugs/javascript/bug_picker.js	2012-09-07 00:23:25 +0000
@@ -106,6 +106,7 @@
     _show_bug_spinner: function(node) {
         if( Y.Lang.isValue(node)) {
             node.addClass('update-in-progress-message');
+            node.set('disabled', true);
         }
     },
 
@@ -117,6 +118,7 @@
     _hide_bug_spinner: function(node) {
         if( Y.Lang.isValue(node)) {
             node.removeClass('update-in-progress-message');
+            node.set('disabled', false);
         }
     },
 

=== modified file 'lib/lp/code/javascript/branchmergeproposal.nominate.js'
--- lib/lp/code/javascript/branchmergeproposal.nominate.js	2012-08-30 02:40:04 +0000
+++ lib/lp/code/javascript/branchmergeproposal.nominate.js	2012-09-07 00:23:25 +0000
@@ -147,6 +147,7 @@
     var spinner_node = Y.Node.create(
     '<img class="spinner" src="/@@/spinner" alt="Submitting..." />');
     submit_link.insert(spinner_node, 'after');
+    submit_link.set('disabled', true);
 };
 
 /**
@@ -159,6 +160,7 @@
     if (spinner !== null) {
         spinner.remove(true);
     }
+    submit_link.set('disabled', false);
 };
 
 /**
@@ -200,7 +202,7 @@
     var base_url = LP.cache.context.web_link;
     var submit_url = base_url + "/+register-merge";
     form.on('submit', function(e) {
-        e.preventDefault();
+        e.halt();
         var y_config = {
             method: "POST",
             headers: {'Accept': 'application/json;'},