Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporarily remove activity submit once pending PR 14519 #14545

Merged

Conversation

demeritcowboy
Copy link
Contributor

@demeritcowboy demeritcowboy commented Jun 14, 2019

Overview

A few months ago, the "submit once" functionality was added to the "New Activity" screen. This protects users in some web-browsers from making a mistake (i.e. clicking the save/submit button multiple times => producing multiple activities). Unfortunately, the change also created multiple problems for other users/configurations.

This PR reverts that change and restores the old behavior of the "New Activity" screen (circa ~5.11). This should be temporary -- the plan is to bring it back in some form, e.g. PR #14519.

Before

https://lab.civicrm.org/dev/core/issues/1045

https://lab.civicrm.org/dev/core/issues/943

https://lab.civicrm.org/dev/core/issues/914

After

None of the above issues.

Comments

I've left in all the other references for now, e.g. CRM/XXX/Task.php and CRM_Core_Form::addDefaultButtons().

ping @mattwire @eileenmcnaughton @colemanw

@civibot
Copy link

civibot bot commented Jun 14, 2019

(Standard links)

@civibot civibot bot added the 5.15 label Jun 14, 2019
@eileenmcnaughton
Copy link
Contributor

@mattwire current feeling with @colemanw & @demeritcowboy is that removing this for now will make the current release better as it will fix https://lab.civicrm.org/dev/core/issues/1045 & at least some of what is discussed in https://lab.civicrm.org/dev/core/issues/943 but it will still be flawed. Full fix is WIP but expected to merge to master or the rc soon - do you have any concerns? We would include in a 5.14.x patch release - planned for (US) Monday at this stage

@eileenmcnaughton
Copy link
Contributor

@colemanw you still thinking this for 5.14.x/I 5.15.x?

@colemanw
Copy link
Member

Yes I think so.

@eileenmcnaughton eileenmcnaughton merged commit cb82206 into civicrm:5.15 Jun 16, 2019
@eileenmcnaughton
Copy link
Contributor

Ok - I've merged so you can re-fix your master over this - will merge through

@totten
Copy link
Member

totten commented Jun 18, 2019

  1. Updated description to give a bit more context. (This will be a hyperlink from the release notes...)

  2. @demeritcowboy and I have done some further testing as part of the 5.14 backport (Backport regression fixes to 5.14 #14567) with various permutations of (a) with Firefox or Chrome (b) with AJAX popup forms enabled or disabled (c) from "View Contact" or "Manage Case" (d) with or without a repeating calendar (e) with normal single-click or double-click, and it seems good there. Normal submission of an activity form (single-click or enter) produces normal results. Abnormal submission (double-click) sometimes yields double-results (though it can be hard to reproduce)

@demeritcowboy demeritcowboy deleted the temp-remove-submit-once branch June 20, 2019 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants