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
On for the "Manage moderation" page (for example /admin/structure/types/manage/article/moderation) there is a delete link, this link deletes the bundle. It shouldn't be there.
Proposed resolution
Remove the delete link
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#37 | 2818267-37.patch | 1.5 KB | Sam152 |
#36 | interdiff.txt | 771 bytes | timmillwood |
#36 | 2818267-36.patch | 1.55 KB | timmillwood |
Comments
Comment #2
shruti1803 CreditAttribution: shruti1803 at Iksula commentedComment #3
shruti1803 CreditAttribution: shruti1803 at Iksula commentedComment #4
timmillwoodThink this might mean updating
\Drupal\content_moderation\Form\BundleModerationConfigurationForm
to extend\Drupal\Core\Form\ConfigFormBase
instead of\Drupal\Core\Entity\EntityForm
.Comment #5
Hardik2309 CreditAttribution: Hardik2309 commentedComment #6
Hardik2309 CreditAttribution: Hardik2309 commentedComment #7
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commentedoverriding actions() method to remove delete link.
Comment #8
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commentedComment #9
timmillwoodWas it not possible to switch it to extend
\Drupal\Core\Form\ConfigFormBase
instead of\Drupal\Core\Entity\EntityForm
?Comment #10
shashikant_chauhan CreditAttribution: shashikant_chauhan as a volunteer and at Iksula commentedClass \Drupal\Core\Form\ConfigFormBase is an abstract class.
It contains 2 abstract methods and must therefore be declared abstract or implement the remaining methods (Drupal\Core\Form\FormInterface::getFormId, Drupal\Core\Form\ConfigFormBase::getEditableConfigNames) in class BundleModerationConfigurationForm.
Which requires more work than overriding actions() method to remove delete link.
So I preferred overriding actions() method.
Comment #11
timmillwoodI think this is a good solution for 8.2.x
We may find once the whole workflow system is in 8.3.x that the "manage moderation" form is very different, but we can cross this bridge then. At least this will give us a better content moderation module in 8.2.x.
Comment #12
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedThis is a coding standards violation.
The rest of the file uses short-hand array syntax.
Screenshots would be helpful here too.
These nits can probably be fixed on commit.
Comment #13
xjmNW for #12. Thanks! Also, +1 for the suggestion of screenshots.
Comment #14
xjmComment #15
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commentedHere a new patch, with short array syntax, and two screenshot, one before the patch and after applying the patch.
Not sure to understand why the comment is a violation standard, could you explain to me ?
Comment #16
timmillwoodThe comment should be on one line, and less than 80 characters long.
Comment #17
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commented@timmillwood : ok thanks for your quick answer, so attach a new patch with correction for the comment
Comment #18
timmillwood@vanessakovalsky - Thanks, only small nit pick it could do with a period at the end of the comment, but xjm might be kind enough to do this on commit, so RTBCing.
Here's the screenshots for clarity.
Before:
After:
Comment #20
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commentedNew version of the patch wihtout new line, which must pass the test this time (I hope)
Comment #21
timmillwoodComment #23
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commentedNew one, deleting unuseful line (first patch to the core is not so easy :) )
Comment #25
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commentedComment #27
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commentedI don't understand why this patch failed testing, can somebody explain me why ?
Comment #29
vanessakovalsky CreditAttribution: vanessakovalsky as a volunteer commentedComment #30
timmillwoodinterdiff from #15.
Comment #31
timmillwoodHere's a 8.3.x version of the patch too.
Comment #32
timmillwoodMoving back to RTBC.
I'd already RTBC'd in #18 but because @vanessakovalsky's patch didn't get generated correctly I helped by generating a 8.2.x version in #30 and 8.3.x in #31. Hope this is ok.
Comment #34
timmillwoodWhy is it testing #15?
Going back to RTBC!
Comment #35
alexpottBugs should have a test no? Just an assertion that the delete link is not there.
Comment #36
timmillwoodAdding a simple test as @alexpott suggested in #35.
Comment #37
Sam152 CreditAttribution: Sam152 as a volunteer and at PreviousNext commentedLooks good to me. Minor changes, not enough to warrant a new review; inheritdoc because the comment is the same as the parent, consistent array style and a cs violation.
Comment #38
alexpottCommitted ac5ccef and pushed to 8.3.x. Thanks!