Still on Drupal 7? Security support for Drupal 7 ended on 5 January 2025. Please visit our Drupal 7 End of Life resources page to review all of your options.Problem/Motivation
WorkflowDeleteForm extends EntityConfirmFormBase, not EntityDeleteForm. EntityDeleteForm displays consequences of deleting configuration dependencies. Currently a workflow config entity can be deleted without the site admin being aware of consequential configuration changes.
Steps to reproduce
1. Delete a workflow
2. See confirmation page without config changes
Proposed resolution
Change base class
Remaining tasks
Do it!
User interface changes
Deleting a workflow displays "Configuration updates", if any.
API changes
None
Data model changes
None
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | 3377166-10.patch | 1.12 KB | roshni27 |
| #13 | interdiff-3377166.txt | 536 bytes | roshni27 |
| #3 | 3377166-3.patch | 673 bytes | sidharth_soman |
Comments
Comment #2
mglamanComment #3
sidharth_soman commentedI have changed the base class. Please review.
Comment #4
longwaveThanks - @sidharth_soman please note there is no need to run tests on all available databases, as each test costs the Drupal Association money to run, and it is usually only necessary to run tests once with the default setting by setting the issue to "Needs review".
I think that some of the methods in WorkflowDeleteForm can now also be cleaned up here, as EntityDeleteFormTrait provides them already.
Comment #5
sidharth_soman commented@longwave - I wasn't the one who applied all those tests. I had simply uploaded the patch without adding any tests. Not sure how that happened.
Comment #6
longwaveApologies, I assumed and did not check.
@rushiraval - please see #4 as it appears you added multiple tests here. In most cases just set the status to "Needs review" and a relevant test will be run automatically.
Comment #7
roshni27 commentedHello,
I removed the getQuestion(), getCancelUrl(), and getConfirmText() methods because these methods are already provided by the EntityDeleteFormTrait. There's no need to redefine them unless you want to customize their behavior.
The submitForm method has also been removed because EntityDeleteFormTrait handles entity deletion, message display, and redirection automatically.
Please review the patch.
Comment #8
longwaveComment #9
roshni27 commentedComment #11
roshni27 commentedPlease review.
Comment #12
smustgrave commentedAlso interdiffs should be included.
Comment #13
roshni27 commentedPlease review.
Comment #14
roshni27 commentedComment #15
roshni27 commentedComment #16
roshni27 commentedComment #17
smustgrave commentedApplied patch and confirmed I can still delete workflows without issue
Comment #18
bnjmnmThe issue summary pointed out one of the primary reasons this was a problem
Had there been test coverage to check for this information, the problem would have been caught/addressed much sooner (or never been an issue at all). Lets add tests that validate this fix and prevent it from regressing.