Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Webforms can have the following statuses:
/**
* Webform status open.
*/
const STATUS_OPEN = 'open';
/**
* Webform status closed.
*/
const STATUS_CLOSED = 'closed';
/**
* Webform status scheduled.
*/
const STATUS_SCHEDULED = 'scheduled';
And this are used as values for:
/**
* The webform status.
*
* @var bool
*/
protected $status = WebformInterface::STATUS_OPEN;
This is highly problematic because the API of configuration entities is going to do weird things when calling ->disable() and enable() on webforms
Proposed resolution
Use a new property for this. Eventually we'll move status out of the default configuration entity fields but that's going to take until Drupal 9. See #2872149: All configuration entities can be disabled but many don't actually define what, if anything, that that means for more about status and configuration entities.
Remaining tasks
User interface changes
None
API changes
Probably
Data model changes
Definitely
Comments
Comment #2
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedFor the Webform entity I am basically overloading the ::setStatus method to support scheduling. The ::disable and ::enable methods are still supported.
If Drupal core starts allowing configuration status to be controlled via the UI or Views bulk operations, the Webform module could have some serious problems
I am open to creating a new 'status' property. Truth is I am not sure what to call it.
Also, I am using the same type of 'status' property for Webform fields and that would need to be updated as well. Right now, I am still have a hard time writing a stable update hook for altering existing fields. @see #2866957: Updating from beta9 to beta10 fails to beta11 fails
For people,wanting to understand the issue, here is the Webform::setStatus method.
which is overriding the below default implementation.
Comment #3
bucefal91 CreditAttribution: bucefal91 at Websolutions Agency commentedIf anybody is interested in listening to my 2 cents.. I've read through alexpott's core issue he references. Overall I tend to believe webform's status (at least in its current shape) is a different thing from config entity status and hence should be called differently and use different interface because, as alexpott describes, when somebody/something tries to walk all config entities and do something about their "status", it assumes a certain meaning behind that status (and actually all config entities have agreed to obey that meaning by implementing the
ConfigEntityInterface
interface). This assumed meaning turns out to be different when it comes to a webform (but the invoking code, the one that walks all config entities) does not know that. So it will become a pain sooner or later.Alternatively, the "status" in the context of a webform can be simplified down to the level of general "status" meaning for a config entity. I still haven't explored this option in depth, but from a quick study of Webform entity: the status could be turned back into a boolean where TRUE means "open" and FALSE menas "closed". The missing 3rd status "scheduled" can be achieved by checking the value of "open" and "close" properties.
Does anyone have a preference as for which way to go?
Comment #4
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented@bucefal91 I agree with you that status should be reverted back to the default behavior.
This is a major API change that needs to done carefully, especially because the Webform (entity reference) field's status may also need to be updated. Personally, I am procrastinating dealing with this because we have until D9 to fix the problem.
I think we have to fully write out a plan of attack before implementing anything.
Comment #5
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedI am changing this issue's priority to major since it should not block tagging a stable release.
Comment #6
jonathanshawShould this be considered for 6.x?
Comment #7
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedWe could move all the
WebformInterface::setStatus
andWebformInterface::getStatus
code toWebformInterface::setFormStatus
toWebformInterface::getFormStatus
. I think this change will be be a little confusing but we can make sure to include comments in the API docs.I should be able to roll a patch this via PHPStorm without any problems.
This is definitely a 6.x change.
Comment #8
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedThe concept of changing status for formStatus is a lot more work than I expected. I am not sure if I have time available to address this issue.
Comment #9
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedI do want to prematurely start fixing a "none" issue.
I am pretty sure when #2872149: All configuration entities can be disabled but many don't actually define what, if anything, that that means is resolved we will have to make some minor adjustments or workarounds to managing a webform's status. For example, we might be able to keep WebformInterface::setStatus but store the
status
string in aform_status
property.Comment #10
jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commentedI am going to make this as won't fix until there is actually a reason to fix it.