Problem/Motivation
In the previous issue
🐛
Field workflow value condition doesn't properly handle workflow creation state
Active
we added support for detecting Creation State correctly. I've since noticed that this detection will fail when the ECA model is being evaluated in the context of an AJAX request where the node is new, but the form state value for the Workflow field is already set to the default "first state" (which is not the same as the creation state).
Steps to reproduce
- Add an ECA model that uses a Workflow ECA condition to check if the entity is in creation state, and an Action you can detect.
- Add a field that triggers AJAX (file upload widget, entityreference autocomplete widget, etc) to the relevant node form
- Go to /node/add/example-node-type
- See that your Condition and Action trigger successfully when first loading the node form
- Trigger the AJAX behaviour, and see the Condition fails
These steps might be a little tricky to observe, because it appears the model gets evaluated twice in the context of an AJAX request (at least in my project). The way we noticed this is because the model is used to allow or deny access to fields on the node form. When the condition fails during the AJAX request, it creates a knock-on effect of causing the file upload to fail because access is now denied. This turns out to be the result of checking the form_state for the value of the field, which has now changed because the form is fully rendered (I gather).
Proposed resolution
Our "getFieldValues" method in the Condition plugin looks like it was cargo-culted from the example (or parent?) plugin we modeled it after. It naively looks up the field value without awareness of what it's trying to achieve. In "normal" cases of the Creation State, we return an empty array of "expected values" which the logic later in our code treats as the creation state (NULL).
Thus, I propose to introduce an earlier check in the getFieldValues method that will check whether the entity is new, and if so ignore the field values altogether. In a quick test in my local setup this seems to work well. I'm not certain we have tests that will demonstrate this as yet, but I'll post a patch that appears to correct things for now.
I won't likely get a chance to work up a proper test on this in short order, but I should have a pretty good indication of whether it's working when I can run the full test suite in the project where I'm using this. This module was built for that project, so if it covers the cases we need it for there, I'll likely commit this unless/until I hear from other users(!) that this creates problems. I believe we have another issue in the queue for test coverage.
Remaining tasks
Roll a patch
Add tests to illustrate the failure