CiviCRM Case, if you submit a new Case and click back then try to submit again an error is shown

@justinfreeman could you test this against the https://dmaster.demo.civicrm.org? and maybe related to #904 (closed)

added git:civicrm-core sig:bug triaged labels

This bug is reproducible on CiviCRM 5.16.alpha

Link to video https://app.hyfy.io/v/ab4Kdb7dV8o/

changed the description

@seamuslee has kindly submitted a PR to fix this one https://github.com/civicrm/civicrm-core/pull/14966

Question: It sounds like the expectation is that clicking back and making changes would function the same way as the "Save and New" button. Would the average person expect that it would edit the case they just created? And/or should this be consistent with how other forms work? Would someone consider that creating a new case is a bug because it lets you create duplicates if you do it unintentionally?

For example if I do the same with a standalone activity, I get a slightly friendlier non-fatal error "We could not find a contact id". There's also some warnings/notices but that's separate.

How should it work?

So the one thing i have noticed is that when you click back and then try and submit the form again the preProcess function doesn't operate.

My understanding from Justin is that he has a client or two where they are creating multiple cases for the same person over and over and over. So they find it easy to go back as that has the data already there and just need to make minor tweaks rather than filling in the form all over again.

Maybe the solution should actually be when the form is submitted with the Save and Add New that the data that was submitted is then passed through as default values when the form is re-loaded perhaps?

Seamus

The problem at the moment is that when you go back the form looses the contact id value for the variable _currentlyViewedContactId which then means when it tries to do the postProcessing on the openCase activity which generates the error here https://github.com/civicrm/civicrm-core/blob/master/CRM/Case/Form/Activity/OpenCase.php#L279

My understanding from Justin is that he has a client or two where they are creating multiple cases for the same person over and over and over. So they find it easy to go back as that has the data already there and just need to make minor tweaks rather than filling in the form all over again.

Ah.
Is it a drupal site? I'm wondering if webform can do this? I haven't looked.

But in any case the current fatal error isn't good, whether there's some other solution or not.

Maybe the solution should actually be when the form is submitted with the Save and Add New that the data that was submitted is then passed through as default values when the form is re-loaded perhaps?

Makes some sense, but is maybe a little non-standard / inconsistent? Would a fourth button be unwieldy? "Save and New Copy"? I'm just tossing ideas.

Hi @justinfreeman is this extension any use to you? https://gitlab.com/physician-health-program-of-bc/saveandnewcopy

I was doing some non-civi stuff with copying and pasting between forms and realized the same type of code could be civi-fied. (Civicized? Encivied?)

Betty and I are reviewing PR's and we came across this issue and the related PR from @seamuslee

We tried to reproduce it on a civiCRM version 5.23 but where not able to do so.

However on a 5.22 version we were able to reproduce it. So it seems that this bug is been fixed in the mean time.

If so could we close this PR and the issue?

ps. We have tested it on 5.19, 5.20, 5.22, 5.23 and dmaster (5.24alpha1)

Interesting. It made me wonder what changed and I'm noticing statusBounce isn't working, for everything not just case. I'm not 100% sure it's related but e.g. as a regular user who doesn't have delete activity permission, try visiting the delete activity url for a regular activity. It should bounce with an error and instead just silently goes to the home page. I'll open a separate ticket for that. Ignore the last part - was seeing multiple things.

It looks like whatever fixed this in 5.23 now makes it possible for clicking back/save to create duplicate cases with no check or deduping. This is going to cause significant pain/issues for existing heavy CiviCase users who have a much easier way for accidental duplicates to be created now.

Can we review what caused the change and whether we want some additional validation to catch duplicate case submissions (@DaveD has an extension request that solves the underlying issue but I feel we should fix this in core.)

Extension is at https://lab.civicrm.org/extensions/preventduplicatecases

Submitting forms multiple times because of use of browser back/forward buttons is one of the most common user errors I come across - it's pretty bad when it relates to payments because it can result in duplicate payments. We had issues with older versions of "Add Case" (and Add activity) where duplicates were being created because the form allowed you to press the submit button multiple times - this was fixed (by @DaveD) after quite a bit of experimentation/development.

We really should be fixing this in core. If people want to create multiple cases with the same details they can either click "Save and New", use an importer or develop a case clone function. Not relying on (unreliable) browser back/forward which could also be triggered accidently if there are network issues etc.

Coleman did the work on the submit-once, it was just me who wouldn't stop bugging you about it. There is a "Save and new copy" function at https://gitlab.com/physician-health-program-of-bc/saveandnewcopy, which is a bit hokey, but worked the last time I checked.

To take @mattwire 's thought a bit further, I personally believe that the use of back/forward buttons should be highly discouraged programatically for any pages that have any kind of form. Again, personally, I feel that the back button should be closer to a CTRL+Z than simple navigation. It seems to be a terrible habit for users to get into and by programming around it, IT has been enabling bad habits. (Just a "me too" pile on, really.)

Posting some findings. One reason I stopped looking earlier and did an extension instead was because it seems time-consuming to track down what has changed.

I tried again and it has something to do with the session. In 5.22 and earlier, the session does NOT have the context during post when you click back and save. In 5.23 it does. Since OpenCase takes some branches based on context, this is the difference that leads to a fatal in 5.22, but potential duplicates in 5.23.

This context retrieval ultimately calls this session variable lookup and in 5.22 $_SESSION['CiviCRM']['CRM_Case_Form_Case_{big long string}'] is mostly empty at the point where you've clicked back and then save. In 5.23 at the same point it has some things including context.

The most obvious session-related change is this, but that doesn't seem to be it: https://github.com/civicrm/civicrm-core/pull/16177/files

Also nothing seems to have changed regarding session storage or caching or what the Case.php and OpenCase.php forms put in there.

While looking into #1675 (closed) there's a line in the DB package that's new in 5.23 and changes what transactions cover: $DB->simpleQuery('BEGIN'). And if I remove it then the save+back+save mostly goes back to being like in 5.22 but it's a different fatal error, so there's probably still something else involved with session/cache, but that DB change seems to also be involved.

closed