Opened 7 years ago

Closed 3 years ago

Last modified 4 months ago

#28889 closed Cleanup/optimization (wontfix)

Use JavaScript to prevent double submission of admin forms

Reported by: Manuel Saelices Owned by: Marcelo Galigniana
Component: contrib.admin Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Manuel Saelices)

For fast clickers.

Change History (19)

comment:1 by Manuel Saelices, 7 years ago

Description: modified (diff)

Fixed on this Pull Request: https://github.com/django/django/pull/9425

comment:2 by Aymeric Augustin, 7 years ago

Triage Stage: UnreviewedAccepted

I think this is an improvement and the implementation looks reasonable.

comment:3 by Tim Graham, 7 years ago

Has patch: set
Needs tests: set
Summary: Prevent double submission on admin formsUse JavaScript to prevent double submission of admin forms
Type: BugCleanup/optimization

I found an article titled JavaScript: Preventing Double Form Submission which might be worth reviewing. In particular, it points out "Rather than simply disabling the button, we can also change the text so that people don't get confused."

Did you consider trying to add some tests for the patch?

I share Nick's concern on the pull request that this may have a good chance of causing some regression in unconsidered edge cases. I'm not sure if it's considered a best practice these days. The article suggests that some browsers (e.g. IE11) treat a double click as a single click. It seems unfortunate if every project has to add something like this.

comment:4 by Karan Bedi, 7 years ago

Owner: changed from nobody to Karan Bedi
Status: newassigned

comment:5 by Karan Bedi, 7 years ago

I have created a PR. Please guide me how to proceeding further.
PR

comment:6 by Carlton Gibson, 7 years ago

Hi Karan,

Thanks for the input. Your fix includes the changes Nick suggested from the original PR. (Great.)

To get this merged we need to address Tim's concerns.

  • Can we add some selenium tests, to exercise the behaviour?
  • What's your assessment of the article Tim linked to? Do we need (still) to do this?
Last edited 7 years ago by Carlton Gibson (previous) (diff)

comment:7 by Karan Bedi, 7 years ago

Hi Carlton,

  • I am unable to reproduce the behavior, probably because till now, I am serving on localhost.
  • In the article, there are different methods of implementing the same. One improvement can be changing the label of the submit button after a click.
  • From the point of usability, there is just a comment by someone, which also doesn't seem concrete.

Please guide me on how to proceed further.

comment:8 by Mariusz Felisiak, 3 years ago

Owner: Karan Bedi removed
Status: assignednew

comment:9 by Marcelo Galigniana, 3 years ago

Has patch: unset
Owner: set to Marcelo Galigniana
Status: newassigned

comment:10 by Marcelo Galigniana, 3 years ago

Has patch: set
Needs tests: unset

comment:11 by Carlton Gibson, 3 years ago

Patch needs improvement: set

comment:12 by Marcelo Galigniana, 3 years ago

For this issue there are currently 2 approaches:

comment:13 by Marcelo Galigniana, 3 years ago

I think thibaudcolas's comment in 1st approach makes sense: https://github.com/django/django/pull/15217#issuecomment-1018935149

So I updated the PR to go for the first option

Last edited 3 years ago by Marcelo Galigniana (previous) (diff)

comment:14 by Carlton Gibson, 3 years ago

Patch needs improvement: unset
Triage Stage: AcceptedReady for checkin

comment:15 by Carlton Gibson <carlton@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In fe7dbef:

Fixed #28889 -- Prevented double submission of admin forms.

Added a JavaScript confirm() to catch double-submissions, when the
change form has already been submitted.

Thanks to Adam Johnson, Claude Paroz, Keryn Knight, and Thibaud Colas
for review.

comment:16 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 0756c61f:

Fixed #33893 -- Reverted "Fixed #28889 -- Prevented double submission of admin forms."

Regression in fe7dbef5867c577995f0fc849d8dfdb8f2e6bbfa.

comment:17 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In 82e9e19:

[4.1.x] Fixed #33893 -- Reverted "Fixed #28889 -- Prevented double submission of admin forms."

Regression in fe7dbef5867c577995f0fc849d8dfdb8f2e6bbfa.

Backport of 0756c61f2ada56e4ae625589099c0141a77737eb from main

comment:18 by Mariusz Felisiak, 2 years ago

Has patch: unset
Resolution: fixedwontfix
Triage Stage: Ready for checkinUnreviewed

Patch was reverted, it's probably not worth complexity.

comment:19 by Ömer Faruk Abacı, 4 months ago

In our production environment, we encountered this issue with inlines. Adding an inline object and mistakenly clicking “Save” multiple times resulted in multiple requests, leading to the insertion of the same inline object multiple times. To resolve this, I disabled the buttons on form submission by overriding admin/submit_line.html as follows:

{% extends "admin/submit_line.html" %}
{% block submit-row %}
    <script>
        if (!$){
            const $ = django.jQuery;
        }
        $(document).ready(function() {
            $('form').on('submit', function(event) {
                $('input[type="submit"], button').prop('disabled', true);
            });
        });
    </script>
    {{ block.super }}
{% endblock %}

I understand the concerns raised in previous discussions about network failures potentially leaving buttons disabled and preventing further user actions. However, I believe we need to weigh which scenario is worse: adding duplicate entries or requiring users to re-enter their changes. Additionally, we should consider which is more likely: mistakenly clicking “Save” twice or experiencing a network failure. IMHO, it might be good to address this issue to enhance the user experience and data integrity.

Version 0, edited 4 months ago by Ömer Faruk Abacı (next)
Note: See TracTickets for help on using tickets.
Back to Top