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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

timmillwood created an issue. See original summary.

shruti1803’s picture

Assigned: Unassigned » shruti1803
shruti1803’s picture

Assigned: shruti1803 » Unassigned
timmillwood’s picture

Think this might mean updating \Drupal\content_moderation\Form\BundleModerationConfigurationForm to extend \Drupal\Core\Form\ConfigFormBase instead of \Drupal\Core\Entity\EntityForm.

Hardik2309’s picture

Assigned: Unassigned » Hardik2309
Hardik2309’s picture

Assigned: Hardik2309 » Unassigned
shashikant_chauhan’s picture

overriding actions() method to remove delete link.

shashikant_chauhan’s picture

Status: Active » Needs review
timmillwood’s picture

Was it not possible to switch it to extend \Drupal\Core\Form\ConfigFormBase instead of \Drupal\Core\Entity\EntityForm?

shashikant_chauhan’s picture

Class \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.

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community

I 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.

Sam152’s picture

Issue tags: +Needs screenshots
  1. +++ b/core/modules/content_moderation/src/Form/BundleModerationConfigurationForm.php
    @@ -192,4 +192,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +   * Returns an array of supported actions for the current
    +   * BundleModerationConfigurationForm.
    

    This is a coding standards violation.

  2. +++ b/core/modules/content_moderation/src/Form/BundleModerationConfigurationForm.php
    @@ -192,4 +192,17 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
    +    $actions['submit'] = array(
    

    The rest of the file uses short-hand array syntax.

Screenshots would be helpful here too.

These nits can probably be fixed on commit.

xjm’s picture

Status: Reviewed & tested by the community » Needs review

NW for #12. Thanks! Also, +1 for the suggestion of screenshots.

xjm’s picture

Status: Needs review » Needs work
vanessakovalsky’s picture

Here 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 ?

timmillwood’s picture

Issue tags: -Needs screenshots

The comment should be on one line, and less than 80 characters long.

vanessakovalsky’s picture

@timmillwood : ok thanks for your quick answer, so attach a new patch with correction for the comment

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community

@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:

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 17: 2818267-17.patch, failed testing.

vanessakovalsky’s picture

New version of the patch wihtout new line, which must pass the test this time (I hope)

timmillwood’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 20: 2818267-19.patch, failed testing.

vanessakovalsky’s picture

Status: Needs work » Needs review
FileSize
886 bytes

New one, deleting unuseful line (first patch to the core is not so easy :) )

Status: Needs review » Needs work

The last submitted patch, 23: 2818267-23.patch, failed testing.

vanessakovalsky’s picture

Status: Needs work » Needs review
FileSize
889 bytes

Status: Needs review » Needs work

The last submitted patch, 25: 2818267-25.patch, failed testing.

vanessakovalsky’s picture

Status: Needs work » Needs review
FileSize
886 bytes

I don't understand why this patch failed testing, can somebody explain me why ?

Status: Needs review » Needs work

The last submitted patch, 27: 2818267-27.patch, failed testing.

timmillwood’s picture

timmillwood’s picture

Version: 8.2.x-dev » 8.3.x-dev
FileSize
819 bytes

Here's a 8.3.x version of the patch too.

timmillwood’s picture

Status: Needs review » Reviewed & tested by the community

Moving 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.

The last submitted patch, 15: 2818267-15.patch, failed testing.

timmillwood’s picture

Why is it testing #15?
Going back to RTBC!

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Bugs should have a test no? Just an assertion that the delete link is not there.

timmillwood’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
1.55 KB
771 bytes

Adding a simple test as @alexpott suggested in #35.

Sam152’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
1.5 KB

Looks 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.

diff --git a/core/modules/content_moderation/src/Form/BundleModerationConfigurationForm.php b/core/modules/content_moderation/src/Form/BundleModerationConfigurationForm.php
index 44a2873..917ec5d 100644
--- a/core/modules/content_moderation/src/Form/BundleModerationConfigurationForm.php
+++ b/core/modules/content_moderation/src/Form/BundleModerationConfigurationForm.php
@@ -140,15 +140,16 @@ public function save(array $form, FormStateInterface $form_state) {
   }

   /**
-   * Returns an array of supported actions for the current form.
+   * {@inheritdoc}
    */
   protected function actions(array $form, FormStateInterface $form_state) {
     $actions['submit'] = [
       '#type' => 'submit',
       '#value' => $this->t('Save'),
-      '#submit' => array('::submitForm', '::save'),
+      '#submit' => ['::submitForm', '::save'],
     ];

     return $actions;
   }
+
 }
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed ac5ccef and pushed to 8.3.x. Thanks!

  • alexpott committed ac5ccef on 8.3.x
    Issue #2818267 by vanessakovalsky, timmillwood, shashikant_chauhan,...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.