While using Rules Forms on a specific site I support, I noticed that certain actions were being fired when they weren't supposed to.
Debugging using Drupal Revisions and Rules debugger, I came through the following scenario:
1- Open a form active on Rules Forms
2- Change whichever fields needed
3- Open another form that contain the same ID (may be another content of the same type, for example)
4- The $_SESSION['rules_forms_form_values'][$form_id] will be overridden by the values of the second form
5- Submit the first form. Notice that any conditions that any condition that check the previous values is now comparing with the values for the second form instead.
The cause of this is that even if for different content, all forms of the same content type have the same ID, so the session only stores the values for one form at a time.
Comments
Comment #2
caminadaf CreditAttribution: caminadaf at CI&T for Pfizer, Inc. commentedI believe that setting every visitted form on the session would make it unsustainable (specially if they visit many forms but exit them instead of submitting). This would make the array values form the session not being removed.
As an alternative, I will study using the $form_state instead of using the SESSION, this way each for build values will be related to that form visit only.
Comment #3
caminadaf CreditAttribution: caminadaf at CI&T for Pfizer, Inc. commentedPatched the suggested changes. I'll leave this patch here for now since it has the risk of impacting other people's work.
Comment #4
caminadaf CreditAttribution: caminadaf at CI&T for Pfizer, Inc. commentedAdded back the submit assetion on the test file.
Comment #5
caminadaf CreditAttribution: caminadaf at CI&T for Pfizer, Inc. commentedComment #6
dfranca CreditAttribution: dfranca at CI&T for Pfizer, Inc. commentedTested the scenario described on the patch, with and without it, and the issue is fixed.
Also tested with other different rules to look for side effects, and all of them are working exactly as before.
Comment #10
caminadaf CreditAttribution: caminadaf at CI&T for Pfizer, Inc. commentedCommited based on @dfranca's tests