Enable Content Moderation integration for taxonomy terms

House cleaning, just postponing on #2899923: UI for publishing/unpublishing taxonomy terms

Guess we can open this back up now.

Blocking issue is in so yes. Triggered the tests on the initial patch just to see where we are with this.

reroll for taxonomy.module hunk

Setting back to Needs Review & Triggering bots.
@n4r3n - can you please post the interdiff as well?

Nits / CS review.

1. +++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermContentModerationTest.php
   @@ -0,0 +1,201 @@
   +    $parent_1 = $this->createTerm($this->vocabulary, ['langcode' => 'en', 'moderation_state' => 'published', 'parent' => $root->id()]);
   +    $parent_2 = $this->createTerm($this->vocabulary, ['langcode' => 'en', 'moderation_state' => 'published', 'parent' => $root->id()]);
   +    $parent_3 = $this->createTerm($this->vocabulary, ['langcode' => 'en', 'moderation_state' => 'published', 'parent' => $root->id()]);
   ...
   +    $child = $this->createTerm($this->vocabulary, ['langcode' => 'en', 'moderation_state' => 'published', 'parent' => $parent_1->id()]);
   

   These arrays should be one item in each line.

2. +++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermContentModerationTest.php
   @@ -0,0 +1,201 @@
   +    $this->assertSession()->pageTextNotContains($validation_message);
   ...
   +    $this->assertSession()->pageTextContains($validation_message);
   ...
   +    $this->assertSession()->pageTextContains($validation_message);
   ...
   +    $this->assertSession()->pageTextContains($validation_message);
   ...
   +    $this->assertSession()->pageTextContains($validation_message);
   ...
   +    //$this->assertSession()->pageTextNotContains($validation_message);
   ...
   +    $this->assertSession()->pageTextContains($validation_message);
   ...
   +    $this->assertSession()->pageTextNotContains($validation_message);
   ...
   +    $this->assertSession()->pageTextContains($default_term_name);
   +    $this->assertSession()->pageTextContains($default_term_description);
   ...
   +    $this->assertSession()->fieldNotExists('revision_log_message[0][value]');
   ...
   +    $this->assertSession()->pageTextContains($pending_term_name);
   +    $this->assertSession()->pageTextContains($pending_term_description);
   +    $this->assertSession()->pageTextNotContains($default_term_description);
   ...
   +    $this->assertSession()->pageTextContains($default_term_name);
   +    $this->assertSession()->pageTextContains($default_term_description);
   

   let's cache this as $assert_session = $this->assertSession();

+++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermContentModerationTest.php
@@ -0,0 +1,201 @@
+    // Check that parents were changed.
...
+    $taxonomy_storage->resetCache();
+    $this->assertEquals([$parent_2->id()], array_keys($taxonomy_storage->loadParents($child->id())));

Since we're checking here that parents were changed, should we do assertNotEqual() instead?

Also, should we uncomment //$this->assertSession()->pageTextNotContains($validation_message); ?

With #2899923: UI for publishing/unpublishing taxonomy terms being closed, is this back under active consideration/development?

Re #15 yes indeed, we are free to work on this now.

Latest patch #9 needed a reroll, so here's a first step forward.

The failing test on #16 is because the test is trying to change the parent before publishing the taxonomy term, I have changed the test to first publish the term, then change the parent. Please let me know if this is not correct and the failure is valid and there is a bug, to me it seems reasonable but I could be wrong.

The rest of changes are essentially tidying things up and addressing my comments earlier on #11 and #13.

Tried patch with JSON:API export and it does not work.
$resource_type->isVersionable() returns false. So draft and published versions of terms are not exposed with JSON:API

Surprisingly, #16 still works with 9.1.9!

Yet i'm wondering how to manage the content moderation view that is only applying to nodes. Though, it's probably out of this issue's scope.

Thanks!

Composer reports patch #18 fails to apply on 9.2.0.

  - Applying patches for drupal/core
    https://www.drupal.org/files/issues/2020-04-17/3047110-18.patch (Enable Content Moderation integration for taxonomy terms)
   Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2020-04-17/3047110-18.patch

Actually the best I can tell section of the patch modifying the taxonomy.module by deleting the .function taxonomy_entity_type_alter isn't needed any more since that bit of code seems to have been deleted in 9.2.0 in issue #3204883: Move exclusion of taxonomy terms from moderation to content_moderation module.

Re-roll the patch while taking into account the changes in #3204883: Move exclusion of taxonomy terms from moderation to content_moderation module.

Content moderation still works with this

Added missing TaxonomyTermContentModerationTest.php

Fixed the coding standards issue from previous patch..

Fixed fail tests, It is related to:
- "drupalPostForm()" & "taxonomy_term_load_multiple_by_name()" are deprecated,
- "void return",
- "$modules" property must be declared protected.

We've been testing this including building out Views as part of a custom moderation UI. Everything was working really well until we ran into #3145661: Users without "administer taxonomy" permission can't see unpublished terms canonical pages. Granting full access to administer taxonomy to users in the role we created to review new terms is problematic for us. In our use case, mistakenly deleting a term would cause a lot of problems.

We're evaluating the time involved in adding custom validation to prevent this, but Content Moderation of Taxonomy Terms still has limitations when compared to moderating Content Types.

Another feature missing from really being able to leverage Content Moderation on Taxonomy is the Most Recent Revision filter in Views. That was rewritten in #2865579: Rewrite the 'Latest revision' views filter and remove the revision_tracker table. It's not clear to me what would need to be done to include support for that filter in the Taxonomy Revision Views, but it is difficult to leverage moderation state without it.

I've applied and tested patch from #32 on Drupal 9.5.2 and it works just great.

@jurgenhaas, thanks for reporting that the patch is working for you and the version of Drupal it is being used on.

The Issue Summary is clear.

Going through the comments I see that the following have not been addressed

• #13 - The commented out line is still in the patch.
• #21

I skimmed the patch and noticed the following items.

1. +++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationStateTest.php
   @@ -278,7 +282,7 @@ public function testContentModerationStateTranslationDataRemoval($entity_type_id
   +      $translation = $entity->addTranslation($langcode, [$entity->getEntityType()->getKey('label') => 'French title test']);
   

   This looks to be out of scope.

2. +++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermContentModerationTest.php
   @@ -0,0 +1,201 @@
   +    // Add a pending revision and change the parent.
   

   It would be easier to follow if parent_1 was used here. The same applies to other comments. As in 'change the parent to parent_2' and so on.

3. +++ b/core/modules/taxonomy/tests/src/Functional/TaxonomyTermContentModerationTest.php
   @@ -0,0 +1,201 @@
   +    //$this->assertSession()->pageTextNotContains($validation_message);
   

   Why is this commented out?

I saw an @todo in the code for this issue. A search of core needs to be done to check for all instances and determine if the todo can be deleted.

Oh sorry for that @quietone - as the issue was NR I thought that all that had been addressed. Will be more cautious next time.

I can't see how this could get merged without resolving #3145661: Users without "administer taxonomy" permission can't see unpublished terms canonical pages. The expectation of enabling Content Moderation on Taxonomy Terms is going to be that the responsibility could be assigned to the same roles as other types of content. Requiring Administer Taxonomy for this seems wrong.

Not sure if the lack of a View filter would get resolved here or in a follow-up issue, but this is another place where it won't work the way most people expect.

Posting a patch from the last MR push because I need an immutable patch.

From what I can tell points in #39 have not been addressed.

Which also mentions other comments not addressed.

Added permission check for view all revisions - @kreynen is correct.

One thing noticed that once taxonomy term published and then moved to Draft, still the term is in published state.

It's the same behavior for nodes.

Yep. The draft version is "ahead" of the currently published version until it is published.

I chose the Node permission "view all revisions" and this is definitely wrong. The issue https://www.drupal.org/project/drupal/issues/2936995 provides revision permissions and should probably be a blocker for this issue.

The current MR solved my original issue, but our use case is only using moderation on a single vocabulary. I agree with @slydevil that it makes more sense to use view terms revisions in $id, but then we just get to the next place where the experience of managing drafts of terms doesn't have feature parity with content.

I added #3359159: Add Status to OverviewTerms.php form and proposed a patch to add Status to the Terms Overview UI. Unlike the Content Overview, admin/structure/taxonomy/manage/[tid[/overview is not a View and there is no support to filter terms by moderation state making it hard to add the UI users are used to seeing in Content.

I'm not sure if the goal is to make incremental progress on this in multiple issues or propose a single META MR that would provide the type of features users will expect to see when enabling moderation on a specific Vocabulary.

We have a project using 9.3.2, Acquia_CMS 1.4.0.
We added node type and/or taxonomy to a workflow item.
We tried enable_content_moderation_for_terms-3047110-49.patch.
We're using 3047110-32.patch to enable moderation for taxonomy_term.
We contacted Acquia Support.
We are failing at UNPUBLISHING either node or taxonomy_term.

Is anyone experience this issue and how to fix it?

I'm thinking that the feature can do this:
- User can specify a state as published.
- When user set the state, the feature will set entity.status to 1.
I'm thinking that the feature CAN NOT do this at the moment:
- User no need to specify states as unpublished.
- When user set a state other than the state-as-published, the feature won't set entity.status to 0.

If someone can help I'm willing to share what more needed detail.
It's been 1/2 year already.
Thanks in advance!

PS:
We're trying to upgrading to D9.4.15 (PHP 7.4) and ACMS 1.5.5 with hope to get the issue resolved but facing many issues while updating DB and importing configs.
- Our Staging environment is blocked by unknown validation when saving content. It's READ-ONLY mode now.
- We got dirty configuration in database.
- We 're facing getConfigDependencyName() on NULL while updating DB by scheduler_content_moderation_integration_update_9002()

Woot! #3359159: Add Status to OverviewTerms.php form was merged into core! Small step, but it improves the core UI when using this patch.

Woot! #3359159: Add Status to OverviewTerms.php form was merged into core! Small step, but it improves the core UI when using this patch.

Hey thanks for that, a good improvement indeed!

Adding related revisions UI issue.

Rerolled with 11.x.

Setting this back to needs review. The issues in #39, #47 seem to have been addressed except no1 in #39 which I don't think is out of scope since we need content moderation to be testing for taxonomy_term entity as well which may have a different entity key for label than other entities which may use "title".

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

@sakthi_dev please do not upload patches, this issue is using an MR and patches are confusing the bot.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Oh! The MR is still against 10.x. We either need it rebased against 11.x, or it's probably easier to create a new branch off 11.x and apply the changes (which seem like they're not applying cleanly according to the bot)

I've hidden the 9.5.x branch

Created a new branch from 11.x and cherrypicked the commits onto it.

We're missing a moderation handler here too, see BlockContentModerationHandler

Great work on the MR, I think now we just need an IS update and a CR.

Change record added https://www.drupal.org/node/3424780 and issue summary updated.

Great job @pavlosdan, thank you for pushing this along - the only thing for me now is whether we have enough test coverage or not. CM is a huge layer of complexity, but the test coverage in the MR (along with existing test coverage that we're enabling for taxonomy terms) seems quite thorough.

I think we're good to go!

Thank you for the reviews and suggestions @acbramley!

I think the test coverage is fine; the actual changes here are minimal and this functionality is already tested, we just need to ensure it's extended for this entity type which has been done here.

Committed and pushed 07473ef250 to 11.x and 2d61a0ef34 to 10.3.x. Thanks!