dev/core#904 - undo part of 13333

[demeritcowboy]

Overview

See https://lab.civicrm.org/dev/core/issues/904. The Save and New button on a new case doesn't work fully after 13333.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

@mattwire @colemanw Can you review this potential regression

[mattwire]

@demeritcowboy This is pretty vague. What Civi version. How does it "break"?

[demeritcowboy]

@mattwire Create a new case and at the bottom of the new case form click "Save and New". It doesn't go to a new case form. I believe this would have the same problem on any form that has more than one submit button. It can be reproduced on dmaster.demo.

[mattwire]

Ok, @demeritcowboy I can reproduce because of the silly way opencase tries to detect the 2nd button. We definitely don't want to remove submitOnce from the "save" button and I feel like not redirecting to a new case is less serious that accidently creating multiple cases... However the actual failure occurs here: https://github.com/civicrm/civicrm-core/blob/master/CRM/Case/Form/Activity/OpenCase.php#L350 We can differentiate between the two buttons by checking for the existence of _qf_Case_upload or _qf_Case_upload_new in the submitValues array of the form object. Happy to review and merge a PR that does that.

[demeritcowboy]

Hi @mattwire I looked but I don't see that there's anything to differentiate in the submitted values either in $form or $params. In js/common.js submitOnce() where it calls form.submit(), I think maybe that's the equivalent of clicking the default button, regardless of what was clicked.

Maybe in the meantime we can compromise and just remove it from the "Save and New" button and leave it on the "Save" button? That way both buttons will still work for now.

[mattwire]

@demeritcowboy We'll run into this issue anywhere else there are more than one submit buttons (just activities currently I think).

Can you try this?

/**
 * Function to change button text and disable one it is clicked
 * @deprecated
 * @param obj object - the button clicked
 * @param formID string - the id of the form being submitted
 * @param string procText - button text after user clicks it
 * @return bool
 */
var submitcount = 0;
/* Changes button label on submit, and disables button after submit for newer browsers.
 Puts up alert for older browsers. */
function submitOnce(obj, formId, procText) {
  // if named button clicked, change text
  if (obj.value != null) {
    cj('input[name=' + obj.name + ']').val(procText + " ...");
  }
  var form = cj(obj).closest('form');
  form.attr('data-warn-changes', 'false');
  if (document.getElementById) { // disable submit button for newer browsers
    cj('input[name=' + obj.name + ']').attr("disabled", true);
    var tmpButtonElement = cj("<input type='hidden'/>");
    // clone the important parts of the button used to submit the form.
    tmpButtonElement
      .attr("name", obj.name)
      .appendTo(form);

    form.submit();
    tmpButtonElement.remove();

    return true;
  }
  else { // for older browsers
    if (submitcount == 0) {
      submitcount++;
      return true;
    }
    else {
      alert("Your request is currently being processed ... Please wait.");
      return false;
    }
  }
}

[demeritcowboy]

I ** AM ** willing to help out here and try some things, but that should probably be against master. All I'm asking for the RC is to remove the thing that made the button stop working until it can be looked at closer. To be honest I'm not sure how this turned into my problem to solve, but I do promise to test that javascript and take a run at it. @mattwire

[eileenmcnaughton]

@mattwire given this is a very recent regression our default response should be to revert in the rc & re-fix the original bug in master. Would doing so here cause problems other than removing the fix until we have a better fix

Also @colemanw

[mattwire]

@demeritcowboy Save/Save and New submitOnce only on "New case" currently but we want to add it elsewhere where we have the same problem eg. "New Membership" so would be good to fix properly. I'm going to merge this based on @eileenmcnaughton comments but would be really happy if you were able to find some time to review the proposed js change and PR it for master so we can have it fixed properly.

[demeritcowboy]

@mattwire The js doesn't work as-is but I have some ideas. Will keep you posted.