Dynamically provide action plugins for every moderation state change

I rewritten it the completely written for content_moderartion after forking from daniel repo
Here's the repo https://github.com/naveenvalecha/content_moderation_actions

I think this would be good to get into Content Moderation rather than being a separate module.

The bug in the IS doesn't seem to happen (although "complains" is not particularly specific). :P Assuming the original bug has been fixed in the past year, is this just a feature request now?

When running through the steps in the IS I get a warning "Article content items were skipped as they are under moderation and may not be directly published.".

So yes, this is not technically a bug, because we mitigate an issue by displaying a warning, but it would be nice to provide actions for bulk moderating content.

Ah, the reason I did not run into this in the first place is that the STR were incomplete. Updating the IS with steps that will actually reproduce it.

However, I don't think this is just a feature request; the issue described in the summary is a usability issue.

When content moderation is enabled site admin can't perform bulk update for publishing/unpublishing the nodes so I think this qualifies as a major bug. Also, I'm working on the patch.

This is just action plugin and deriver. Need tests and schema. If this goes in before #2902187: Provide a way for users to moderate content then we can update the new view there if not then we can update that view here. I think we don't need operations.

Corrected some spelling mistakes and text.

Not manually tested, just code review right now.

1. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,135 @@
   +  /**
   +   * The state transitions validator.
   +   *
   +   * @var \Drupal\content_moderation\StateTransitionValidation
   +   */
   +  protected $validation;
   ...
   +    $this->validation = $validation;
   ...
   +      $container->get('content_moderation.state_transition_validation'),
   

   This isn't used anywhere?

2. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,135 @@
   +    if (($moderation_violations = $violations->getByField('moderation_state')) && count($moderation_violations)) {
   

   An empty array is already falsey.

3. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,135 @@
   +        drupal_set_message($violation->getMessage(), 'error');
   

   I thought we had a fancy new service for this ;)

4. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,135 @@
   +    $entity->isDefaultRevision(TRUE);
   

   This should be removed. moderation_state field already sets it correctly.

5. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,135 @@
   +    $workflow = $this->entityTypeManager
   +      ->getStorage('workflow')
   +      ->load($this->pluginDefinition['workflow']);
   

   What about simply Workflow::load. Can remove dependency on entity type manager.

6. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,135 @@
   +    return $return_as_object ? $result : $result->isAllowed();
   

   Is this missing cacheability metadata?

7. +++ b/core/modules/content_moderation/src/Plugin/Derivative/ModerationStateChangeDeriver.php
   @@ -0,0 +1,64 @@
   +    return parent::getDerivativeDefinitions($base_plugin_definition);
   

   We trust parent:: to return $this->derivatives for us? Seems like a break in encapsulation.

Fixed some trivial fails also addressed #18. Thanks for the review @Sam152.

1. Nice catch.
2. Fixed.
3. Actual that is redundant code. access method will never end up in execute if there is some violation.
4. Removed in #17.
5. Removed.
6. Change the logic around this but added cache data where possible.
7. We trust parent:: to return $this->derivatives for us? Yes, this already practiced in core see \Drupal\block_content\Plugin\Derivative\BlockContent::getDerivativeDefinitions.

Do we want to save a new action when new moderations state is created?
Do we want to delete the corresponding action when the moderations state is deleted?

Patch provides a deriver to create the action for all the states in all the workflow for all the moderated entities. Core has no action plugin which uses deriver. Only flag has one in contrib. The only dynamic plugin in core is role add/remove action but it configures action plugin instance via settings form using action configuration settings instead of deriver.

Now, I'm confused which one should I implement core way i.e. role add/remove role or flag way i.e. flag/unflag entity.

Personally, I think we should create deriver to add the entity type id to action plugin and use configuration settings form to create it per state per workflow but that means we have to use ajax to update the configuration settings form i.e. first select workflow if there are more than one for this entity type and then ajax will load the states in that workflow.

Jibran, a quick glance at the code looks like you are hardcoding a set of transitions/states? It's supposed to be dynamic right? Like people can create a workflow with whatever states they like? Have I misunderstood your patch?

Patch is incomplete. Right now, it is adding a functionality to create actions for all the states in all workflows. It is incomplete because we are not creating the instances of those actions. I only added the instances for default states in the editorial workflow.

Either we can add hook_workflow_insert/update to add/remove the instances of the actions or we can add/remove action instances in \Drupal\workflows\Form\WorkflowStateEditForm::save and \Drupal\workflows\Form\WorkflowStateDeleteForm::submitForm

What about using a deriver as in #5?

Yes, this patch has a deriver like #5 which is making all the action plugin instances available for all the states in all the workflows. I was talking about actually making those action plugin instances available for use.

Looking at this it would seem that the core implementation of the bulk forms bypasses the ActionPluginmanager completely which would bloat the implementation of this feature in a pretty bad way.

This core patch: https://www.drupal.org/node/2912390
Should make it a lot simpler (by allowing the already existing transition entities to generate plugins directly without a need to add extra "meta" config entities to muddy the water.
At the cost of being slightly more tied to following up any advances the core action interface makes.

Proposed solution

- Add derived plugins based on the config directly (see core patch) Which then plays nice with the deriver from #5

Optionally nice to have: Should it be possible to define which transitions appear in the bulk actions etc via the transition config? From a workflow perspective it might be odd to have them blanket allowed.

I had a chat with @tim.plunkett and @Sam152 at DC Vianna we agreed on following things.

• Inject type property to the plugin defination using deriver.
• Make workflow and state configurable per action plugin instance. AJAXify the plugin config form if there are more than one workflows attached to the entity.
• Follow-up: dynamically generate action plugin instance for all the states whenever the workflow is updated either from UI or config import.

Inject type property to the plugin definition using deriver.

Done.

Make workflow and state configurable per action plugin instance.

Done.

AJAXify the plugin config form if there are more than one workflows attached to the entity.

Done.

Follow-up: dynamically generate action plugin instance for all the states whenever the workflow is updated either from UI or config import.

Removed default action instances we can add them in follow-up. This should be green.

Moving to optional should also work.

NR for #32.

Interdiff is based on #32.

• Added test for plugin schema.
• Added test for action UI.
• Added test for configuration form AJAX.

Added unit test for the execute and access methods. This is ready for review.

Reviewing per @jibran's request

1. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,233 @@
   +        'callback' => [get_called_class(), 'configurationFormAjax'],
   

   nit: static::class works too

2. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,233 @@
   +      '#value' => t('Change workflow'),
   

   $this->t()

3. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,233 @@
   +    if (!$workflow = $form_state->getValue('workflow')) {
   ...
   +    $input = $form_state->getUserInput();
   +    // Get the workflow value from user input so that we can rebuild the form.
   +    $form_state->setValue('workflow', $input['workflow']);
   

   This should not use setValue(), but set(). Putting raw unsanitized user input into form values is bad

4. +++ b/core/modules/content_moderation/tests/src/Functional/ActionConfigurationTest.php
   @@ -0,0 +1,113 @@
   +    $this->drupalPostForm('admin/config/system/actions', $edit, t('Create'));
   ...
   +    $this->drupalPostForm('admin/config/system/actions/add/' . Crypt::hashBase64('moderation_state_change:node'), $edit, t('Save'));
   ...
   +    $this->clickLink(t('Configure'));
   

   Nit: It's my understanding that t() is no longer used like this in tests.

5. +++ b/core/modules/content_moderation/tests/src/FunctionalJavascript/ActionConfigurationTest.php
   @@ -0,0 +1,87 @@
   +    $workflow->save();
   
   +++ b/core/modules/content_moderation/tests/src/Kernel/ActionConfigSchemaTest.php
   @@ -0,0 +1,91 @@
   +    $node_type->save();
   ...
   +    $workflow->save();
   ...
   +    $action->trustData()->save();
   ...
   +    $action->trustData()->save();
   

   Why the ->trustData() calls in some and not others? If they are necessary, they deserve a comment.

6. +++ b/core/modules/content_moderation/tests/src/Unit/Plugin/Action/ModerationStateChangeTest.php
   @@ -0,0 +1,315 @@
   +    $this->moderationInfo = $this->getMock(ModerationInformationInterface::class);
   

   Nit: Ideally this would be using Prophecy based mocks

Thanks, for the review @tim.plunkett.

1. Fixed.
2. Nice catch. Fixed.
3. Removed it.
4. Test can be multilingual(e.g. http://cgit.drupalcode.org/entityform_block/tree/src/Tests/LocaleTest.ph...) so IMO it is better to use t.
5. Removed trustData.
6. I just copied the existing test. I have nos strong opinion about it.

Some general things I noticed testing this manually:

• The default label is "Change moderation state of Content", which doesn't mention the name of the state. Confusing if you just save the form without typing something in.
• The label of the workflow field is "Workflow(s)", but the user is selecting just one.
• The error message label is "No access to execute Change to Foo on the Content test.", I wonder if we can mention why access was denied. Ie, was it permissions or a missing transition?

Code review as follows:

1. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +    return new static(
   +      $configuration, $plugin_id, $plugin_definition,
   +      $container->get('content_moderation.moderation_information')
   +    );
   

   Is this a CS violation?

2. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +  public function defaultConfiguration() {
   ...
   +      '#options' => $options,
   +      '#default_value' => $this->configuration['workflow'],
   ...
   +    if (!$workflow = $form_state->getValue('workflow')) {
   +      if (!empty($this->configuration['workflow'])) {
   +        $workflow = $this->configuration['workflow'];
   +      }
   +      else {
   +        $workflow = key($options);
   +      }
   +    }
   

   If we're manually using the first option as a "default" in the form, is there no way to just return this in defaultConfiguration so this is setup for us, or should that always be static values?

3. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +      'workflow' => '',
   +      'state' => '',
   

   Maybe NULL is a better way to indicate "missing" than empty string?

4. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +        $options[$workflow->id()] = $workflow->label();
   ...
   +    $state_options = [];
   ...
   +      $state_options[$state->id()] = $this->t('Change moderation state to @state', ['@state' => $state->label()]);
   

   If we have $state_options why not make the other variable $workflow_options

5. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +    if ($state_options) {
   

   Content moderation workflows have 2 required states, should never be empty.

6. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +        '#title' => $this->t('States'),
   

   Should this be "State", we're selecting just one right?

7. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +
   

   Trailing newline.

8. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +    if (!$object) {
   +      $result = AccessResult::forbidden('Not a valid entity.');
   +      return $return_as_object ? $result : $result->isAllowed();
   +    }
   

   If we're checking if $object is a valid entity, why not go all the way and check instanceof ContentEntityInterface?

9. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
   @@ -0,0 +1,231 @@
   +    if ($workflow = $this->moderationInfo->getWorkflowForEntity($object)) {
   +      if ($workflow->id() !== $this->configuration['workflow']) {
   +        $result = AccessResult::forbidden('Not a valid workflow.');
   

   The workflow is valid, just not necessarily for this specific entity right?

10. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
    @@ -0,0 +1,231 @@
    +        $result->addCacheableDependency($object);
    

    I don't think the entity is a dependency here, no change in the entity itself can impact if a workflow is assigned to it or not right? That config exists in the workflow only.

11. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
    @@ -0,0 +1,231 @@
    +      $result = AccessResult::forbidden('The entity is not moderated.');
    +      $result->addCacheableDependency($object);
    +      return $return_as_object ? $result : $result->isAllowed();
    

    If the first part is based on $workflow, shouldn't the else be based on the same thing?

12. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
    @@ -0,0 +1,231 @@
    +    $access = $object->access('update', $account, TRUE);
    

    Can we describe what is taking place in a more formal way or not at all?

13. +++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
    @@ -0,0 +1,231 @@
    +    $from = $workflow->getTypePlugin()->getState($object->moderation_state->value);
    +    // Make sure we can make the transition.
    +    $result = AccessResult::allowedIf($from->canTransitionTo($this->configuration['state']))
    +      ->andIf($access);
    +
    +    return $return_as_object ? $result : $result->isAllowed();
    

    This seems like an access bypass if it doesn't take \Drupal\content_moderation\Permissions::transitionPermissions into consideration?

14. +++ b/core/modules/content_moderation/src/Plugin/Derivative/ModerationStateChangeDeriver.php
    @@ -0,0 +1,78 @@
    + * The moderation state change deriver.
    

    Other doc comments reference moderation_state.

15. +++ b/core/modules/content_moderation/src/Plugin/Derivative/ModerationStateChangeDeriver.php
    @@ -0,0 +1,78 @@
    +   * @param \Drupal\Core\StringTranslation\TranslationInterface $string_translation
    +   *   The string translation service.
    +   * @param \Drupal\content_moderation\ModerationInformationInterface $moderation_information
    +   *   The moderation information service.
    

    Missing one of the params.

16. +++ b/core/modules/content_moderation/src/Plugin/Derivative/ModerationStateChangeDeriver.php
    @@ -0,0 +1,78 @@
    +      $plugin['config_dependencies']['module'] = [
    +        $entity_type->getProvider(),
    +      ];
    

    Is our dependency on content_moderation implicit?

17. +++ b/core/modules/content_moderation/tests/src/Functional/ActionConfigurationTest.php
    @@ -0,0 +1,113 @@
    +class ActionConfigurationTest extends BrowserTestBase {
    
    +++ b/core/modules/content_moderation/tests/src/FunctionalJavascript/ActionConfigurationTest.php
    @@ -0,0 +1,87 @@
    +class ActionConfigurationTest extends JavascriptTestBase {
    

    Should we just fold the coverage for these two things into the same test?

18. +++ b/core/modules/content_moderation/tests/src/Functional/ActionConfigurationTest.php
    @@ -0,0 +1,113 @@
    +    // Make a POST request to admin/config/system/actions.
    

    Is this comment needed, already implied by drupalPostForm.

19. +++ b/core/modules/content_moderation/tests/src/Functional/ActionConfigurationTest.php
    @@ -0,0 +1,113 @@
    +    $edit = [];
    +    $action_label = $this->randomMachineName();
    +    $edit['label'] = $action_label;
    +    $edit['id'] = strtolower($action_label);
    +    $edit['workflow'] = 'editorial';
    +    $edit['state'] = 'draft';
    +    $this->drupalPostForm('admin/config/system/actions/add/' . Crypt::hashBase64('moderation_state_change:node'), $edit, t('Save'));
    +    $assert_session->statusCodeEquals(200);
    

    If the block above lands on the configuration page already, can't we just submitForm?

20. +++ b/core/modules/content_moderation/tests/src/FunctionalJavascript/ActionConfigurationTest.php
    @@ -0,0 +1,87 @@
    +  public function testActionConfigurationXYZ() {
    

    XYZ?

21. +++ b/core/modules/content_moderation/tests/src/Kernel/ActionConfigSchemaTest.php
    @@ -0,0 +1,91 @@
    +  public function testValidActionConfigSchema() {
    

    Don't the functional tests already blow up if the schema isn't right, or does this make assertions beyond that?

22. +++ b/core/modules/content_moderation/tests/src/Unit/Plugin/Action/ModerationStateChangeTest.php
    @@ -0,0 +1,315 @@
    +    $this->node->expects($this->once())
    +      ->method('getEntityTypeId')
    ...
    +    $this->node->expects($this->once())
    +      ->method('id')
    

    Unless it's important this method is called once, this is testing the implementation.

23. +++ b/core/modules/content_moderation/tests/src/Unit/Plugin/Action/ModerationStateChangeTest.php
    @@ -0,0 +1,315 @@
    +    $this->node->expects($this->once())
    +      ->method('save');
    +
    +    $moderation_state = $this
    +      ->getMockBuilder(ModerationStateFieldItemList::class)
    +      ->disableOriginalConstructor()
    +      ->getMock();
    +    $this->node->moderation_state = $moderation_state;
    +
    +    $this->moderationInfo->expects($this->once())
    +      ->method('getLatestRevision')
    +      ->with($entity_type_id, $entity_id)
    +      ->will($this->returnValue($this->node));
    +
    +    $config = ['state' => 'foobar'];
    +    $plugin = new ModerationStateChange($config, 'moderation_state_change', ['type' => 'node'], $this->moderationInfo);
    +
    +    $plugin->execute($this->node);
    

    Where is the actual testing that "foobar" is assigned as the moderation state?

24. +++ b/core/modules/content_moderation/tests/src/Unit/Plugin/Action/ModerationStateChangeTest.php
    @@ -0,0 +1,315 @@
    +  /**
    +   * Tests the access method.
    +   */
    +  public function testAccessModerationStateChange() {
    

    Wow, this is a long test. Can we break it up so the bits which are repetitive use a dataProvider instead?

25. +++ b/core/modules/content_moderation/tests/src/Unit/Plugin/Action/ModerationStateChangeTest.php
    @@ -0,0 +1,315 @@
    +    $moderation_info->expects($this->once())
    +      ->method('getWorkflowForEntity')
    

    Again, why is it important this is only called once?

Thanks, for the excellent review @Sam152.

• Add/remove role has the same problem.
• Fixed it.
• I think it is out of scope for this issue because it is related to \Drupal\system\Plugin\views\field\BulkForm::viewsFormSubmit. We can create a follow-up to have a larger discussion.

1. Fixed.
2. It is better not to depend on config values in defaultConfiguration so NULL is fine in this case.
3. Fixed.
4. Fixed.
5. Fixed.
6. FIxed.
7. Can't find it.
8. Added an additional check.
9. Correct, updated the message.
10. Correct, removed the object dependency.
11. In this case, we don't have a workflow so we can't add the dependency on the workflow but I update the message to make it clearer.
12. I have updated the comment.
13. Wow, nice catch, I don't know why I thought all we need to check is canTransitionTo. Fixed it.
14. Fixed.
15. Fixed.
16. Yeah, action plugin provider is added by default.
17. I wanted to keep JS functionality testing in JS test. *Random fails and all that.*
18. Removed it.
19. Updated it.
20. Debugging code. Fixed it.
21. We have explicit config schema test coverage for all action plugins is core so I added one here as well.
22. I'll update the unit test later.
23. I'll update the unit test later.
24. I'll update the unit test later.
25. I'll update the unit test later.

#2911806: Remove unnecessary Crypt::hashBase64() call from Action UI went in, so you can stop using Crypt::hashBase64() now!

Fixed #44.

Unless it's important this method is called once, this is testing the implementation.

Sorry, I didn't get your point here. Can you please explain?

Where is the actual testing that "foobar" is assigned as the moderation state?

Fixed.

Wow, this is a long test. Can we break it up so the bits which are repetitive use a dataProvider instead?

Done.

Again, why is it important this is only called once?

Can you please explain this as well?

+++ b/core/modules/content_moderation/tests/src/Unit/Plugin/Action/ModerationStateChangeTest.php
@@ -0,0 +1,315 @@
+    $this->node->expects($this->once())
+      ->method('getEntityTypeId')
...
+    $this->node->expects($this->once())
+      ->method('id')

In these examples, you're creating a mock node and saying id will only ever be called one time. To actually deliver the feature, why is it important id isn't called twice? Is there some implementation or additional feature that exists which is a) completely valid and b) calls id a second time? I would probably think so. Hence these kind of assertions aren't actually testing any kind of outcome, but are just duplicating details of the authors implementation in the test. So the code has no more actual test coverage but is now more brittle and harder to maintain.

Thanks, for the explanation @Sam152. Addressed #47.

This is blocked by #2816861: Action configuration form does not support #ajax

@jibran - can you provide some more information on why this is blocked by #2816861: Action configuration form does not support #ajax? As it is blocked by that issue shouldn't we have tests failing?

can you provide some more information on why this is blocked by #2816861: Action configuration form does not support #ajax

I thought title gave it away. :P

The action configuration form in this issue is using AJAX and it is not saving the action configurations because of #2816861: Action configuration form does not support #ajax.

As it is blocked by that issue shouldn't we have tests failing?

I have worked on some failing tests which I can upload here.

Awesome, thanks!

Here you go.

+++ b/core/modules/content_moderation/tests/src/FunctionalJavascript/ActionConfigurationTest.php
@@ -83,4 +84,43 @@ public function testActionConfiguration() {
+    $this->createScreenshot('public://screenshot.jpg');

:/

Fixed #54.

Blocker is in

Fixed the phpcs issue.

Manual UI review,

• On the add form the workflow field shows '- Select -' since no option is yet selected. However the 'State' field shows values for the first valid workflow. The state field should not have selectable values when there is no workflow value. See next point,
• Code to determine the $form['configuration']['workflow']['#default_value'] and $workflow should be consolidated. That is, Move the $workflow logic up, and make default_value use it.
• AJAX should only return the fields being updated (the states field) not the entire container. Affects accessibility, focussed element is re-loaded.

And some integrity issues:

• Workflow field has no options when no workflows apply to the entity type.
• The deriver creates derivates even if no workflows apply to an entity type.
• if a workflow no longer applies to an action plugin:
  • the instance should be updated/deleted. The action is lingering and no longer works.
  • if no workflows apply, the edit form shows all states, none of which apply.
• On action create form, if no workflows apply an exception is thrown (WSOD)

Thanks, for the review.

On the add form the workflow field shows '- Select -' since no option is yet selected. However the 'State' field shows values for the first valid workflow. The state field should not have selectable values when there is no workflow value. See next point,
Code to determine the $form['configuration']['workflow']['#default_value'] and $workflow should be consolidated. That is, Move the $workflow logic up, and make default_value use it.

I think it is a good idea to move default value above. Also, I added an empty value check for state select.

AJAX should only return the fields being updated (the states field) not the entire container. Affects accessibility, focussed element is re-loaded.

Very good point. Fixed it.

Workflow field has no options when no workflows apply to the entity type.
The deriver creates derivates even if no workflows apply to an entity type.

We decided in #28 to move the workflow to the action configuration instead of keeping it in deriver. Yes, you can navigate to add action link but as you said Workflow field has no options so you can't save the add action form anyway so IMO it's fine. We can add warning message with admin/config/workflow/workflows link when there is no workflow on config form.
OTOH if deriver removes the plugin once workflow doesn't apply anymore then we'll start getting PluginNotFoundException. We can properly fix it after #2873287: Dispatch events for changing content moderation states.

if a workflow no longer applies to an action plugin:
the instance should be updated/deleted. The action is lingering and no longer works.

See #28.3. Unfortunately, we can't do that until #2873287: Dispatch events for changing content moderation states.

if no workflows apply, the edit form shows all states, none of which apply.

As discussed above we can add warning message with a link to update the workflow. We can also hide the state select if workflow select has no valid option.

On action create form, if no workflows apply an exception is thrown (WSOD)

Fixed it.

+++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
@@ -0,0 +1,241 @@
+      $result = AccessResult::forbidden('No workflow found for the entity.');

should this add the workflow_list cache tag?

The deriver creates derivates even if no workflows apply to an entity type.

Agree

the instance should be updated/deleted. The action is lingering and no longer works.

The config dependency API should handle this, which indicates we need to add integration and tests for that

if no workflows apply, the edit form shows all states, none of which apply.

That is a major UX issue, I think we should resolve that too

This interdiff solve the following problems:

• Only creates derivates for entity types with workflow(s) attached to them.
• If no workflow applies, the edit form hides the state select.

Now we have the following problem:

• If an action is created, with proper workflow and state configuration, and the user removes the workflow from the entity type then the derivate will not exist anymore and we'll see fatal because of PluginNotFoundException. This means we need an event to be fired when a workflow is removed from entity type. AFAICS #2873287: Dispatch events for changing content moderation states doesn't handle that.

the instance should be updated/deleted. The action is lingering and no longer works.

The config dependency API should handle this, which indicates we need to add integration and tests for that

There are multiple ways to change the workflow configuration:

• Edit with workflow.
• Change the settings during config import/export.
• Change the raw config in hook update.

At DrupalCon Vienna @tim.plunkett, @Sam152 and I agreed that we aren't going to support the 3rd option. If you are messing with raw config then you have to make sure you update the action plugin instances as well. #2873287: Dispatch events for changing content moderation states only handles the 1st option and for the 2nd option, we have to implement config import event.

It all feels like out of scope here so we decided to move it to followup in #28.3. If we want to proceed with that in this issue then #2873287: Dispatch events for changing content moderation states becomes a hard blocker for this issue.

As @Sam152 corrected me today, #2873287: Dispatch events for changing content moderation states deals with content moderation state change not with workflow entity settings change.

To update and delete of the action instance should be done:

• in hook_workflow_insert, hook_workflow_update and hook_workflow_delete when a workflow entity create/update/delete operation happens.
• during \Drupal\Core\Config\ConfigEvents::IMPORT event when workflow entity is updated during config import.
• during \Drupal\Core\Config\ConfigEvents::SAVEand \Drupal\Core\Config\ConfigEvents::DELETE when raw config is changed during hook_update_N.

We also need functional tests for all the above cases and some kind of user notifications to inform about action instance changes. #2902187: Provide a way for users to moderate content is also fixed so we can update the moderated_content admin view with state change actions added here.

I have updated the IS with remaining tasks also re-titled and re-categorized the issue because we are adding whole new functionality and not fixing any buy.

I'm not actively working on this issue anymore so feel free to pick this up.

I wonder if a fallback plugin in the action system could allow us to side-step the complicated things in #65. Views and blocks do this already I think, the downside being it would create orphan action config entities.

Another possible alternative would be to try make the plugins work without requiring a config entity to exist. That is, all the context required to execute the action would be computed as part of a derivative and the config entity wouldn't be required. That could just shift the problem elsewhere, other systems like views wouldn't be able to add actions as config dependencies.

Another option would possibly be returning TRUE in workflowStateHasData and workflowHasData if action config entities exist for a given state. That would require the user go clean up the config entities first, much like they have to clean up their content before deleting a state. We would need to do some follow up to signal to users exactly how to resolve these lingering data issues.

Another option I've thought of (probably like this one the most so far) is have a single action plugin called "Change moderation state". We do what CancelUser and set a custom confirm_form_route_name. Then we simply present the user a list of states they are able to use, then have them hit confirm.

This could simplify some of the access stuff and make the dependency/orphan thing a non-issue.

#67 sounds like https://www.drupal.org/project/bulk_update_fields in core.

I had https://www.drupal.org/project/views_bulk_edit in mind, but yeah, it would borrow heavily from the concepts in those, but obviously heavily reduced in scope and moderation specific.

@Sam152 graciously explained all 5 options.

if a fallback plugin in the action system could allow us to side-step the complicated things in #65. Views and blocks do this already I think, the downside being it would create orphan action config entities.

We also have broken ER selection plugin. I like this one. As @Sam152 said, It avoids fatal errors. And it allows the user to fix it or remove it. It is not a new pattern we have been doing things this way since we had ctools plugins in contrib.

Another possible alternative would be to try make the plugins work without requiring a config entity to exist.

I have no idea how this can be built neither does @Sam152.

Another option would possibly be returning TRUE in workflowStateHasData

I don't like this at all. Historically, term data is used for content and action is a config entity and blocking the changes of a config entity on other config entity doesn't seem right.

We do what CancelUser and set a custom confirm_form_route_name.

This makes life so much easier but it would be UX nightmare. It will be hugely different from what we have right now in HEAD, select content select action(publish/unpublish) apply.

5th option is to just move it all the contrib

Given the scope of changes I explained in #65 I re-categorized the issue as a task but it's a regression nonetheless so I believe it should be fixed in core.

#62 still works quite well on Drupal 8.5.6.

One problem is that the author, and revision dates are not set. Also, since we are creating new revisions, it could be useful to generate a log message for example: Bulk moderation change: Published -> Archived. See for example how it is done in NodeRevisionRevertForm::submitForm.

According with @miroslavbanov, here is a patch for creating a new revision with its own message when the action is executed.

@david4lim

Looking at the last patch. Overall looks good but there are some issues.

There is no interdiff. Please make interdiff to help reviewers.

You are not assuming that setChangedTime method is available, but you are assuming that getChangedTime is available. That's strange.

You should just use the result of the time() call for both create and update times, and get rid of getChangedTime call. It's just good design to make things more simple, rather than convoluted or rely on side effects.

working with @david4lim
#74 works fine but not if the selected content is a translation. if the selected content is a translation then the execute function only apply the action on the original entity and the translation remain intact.
attaching a patch and an interdiff

apologies forgot a return statement in #76

I am still seeing issues when trying to bulk publish/archive a node with translations. if you have a node and 2 translations and attempt to bulk publish all of them, the moderation state gets updated but the status for one translation fails to get updated. See screenshot: https://i.imgur.com/K8b4Joe.png

updating patch to address translation issues.

Fixing most tests, not sure about the entityTypeManager when create a new ModerationStateChange object in tests. Let's see how it fails.

Oops, didn't find this before. :)

There still remain some workflow design issues that need to be worked out. As of now, on the /admin/content page:

• 1. You can change state of content from unpublished (draft/archived) to published which creates a new published default revision.
• 2. You can change the content moderation state *from* published to *draft*. This does not actually affect the published node as it only creates a new draft which is not visible on admin/content. *This is confusing IMO*. Perhaps such a change should be taken out.
• 3. Changing to archive works fine. It creates a new revision and makes it the default "archived"/unpublished revision.
• 4. Changing from *Archived* to *Draft* also works fine. It creates a new revision set to draft.

So far only #2 seems a bit confusing.

Agree, #2 is confusing. What would it be the best way to indicate that a node has been changed to Draft? Do we have to show an indication in admin/content that the node is a Draft even though we have that info already in admin/content/moderated?

Was discussing this with @Berdir, wondering if would be good to add bulk operations by default in the moderated content view. Then the second point on #91 would be clearer since we would have it in a separated view.

@yongt9412 re #93That would be good and looks to be part of the Issue summary proposal.

Update moderation_content admin view with the state actions.

If possible though to keep this issue reviewable that may be better suited as a follow-up issue, what do you think?

While we're waiting on this to make it into Drupal 8 core, as a workaround, you can use the Views Bulk Edit module along with VBO to bulk edit the entities moderation state.

For anyone experimenting with this, here is a reroll of #89 for 8.6.x.

been searching for while for a solution to this, I tried the above patch (the latest), it does work well once it is configured, requires adding a views bulk operation field to the admin/content/moderated view (make sure to remove the 'node bulk operation' field if there is one. Our client has a desire for a solution on the admin/content page rather than the admin/content/moderated page so while this core patch does work and provide a working solution our client wants something easier to use and that is what they will get (because that's what they want).

We're going to do a custom solution (should be easy enough to do).

Otherwise, I did try some other possible solutions, there is a contrib module that appears to do this.
https://www.drupal.org/project/workbench_moderation_actions/releases/8.x...

However (maybe I did not configure it correctly?) I tried it and got an error:

No access to execute Set Content as Draft on the Content
or
No access to execute Set Content as Archived on the Content

Then I tried views_bulk_edit but it seemed to conflict with our configuration.

So, overall this core patch seemed like the best way to go for a non custom solution.

However my client wants a simpler interface on the admin/content view so I'm going to create a custom solution for that which will with one button click for unpublishing ALL revisions (in our case two step, to archived (for current revision) then to draft so that no published revisions remain.
and a button click to set the latest revision to 'published. two options is all my client wants. And we have a javascript solution for a confirmation.

I will share if there is time.

our client wants something easier to use and that is what they will get (because that's what they want)

I feel your pain...

Ok, massive code blitz, here is what our client wants and what I delivered to them.

yml file: cool_admin/config/optional/system.action.unpublish_current.yml

langcode: en
status: true
dependencies:
  module:
    - node
id: unpublish_current
label: 'Unpublish current revision'
type: node
plugin: unpublish_current_revision_action
configuration: {  }

yml file: cool_admin/config/schema/cool_admin.schema.yml

# Schema for the configuration files of the cool_admin module.
# set up the actions for bulk operations for publish and unpublish on /admin/content view.
action.configuration.unpublish_current_revision_action:
  type: action_configuration_default
  label: 'Unpublish current revision'

action.configuration.publish_latest_revision_action:
  type: action_configuration_default
  label: 'Publish latest revision'

yml file: cool_admin/config/optional/system.action.publish_latest.yml

langcode: en
status: true
dependencies:
  module:
    - node
id: publish_latest
label: 'Publish latest revision'
type: node
confirm: true
plugin: publish_latest_revision_action
configuration: {  }

Filename cool_admin/src/AdminModerate.php

<?php

// Filename cool_admin/src/AdminModerate.php
namespace Drupal\cool_admin;

use Drupal\Core\Entity\RevisionLogInterface;
use Drupal\cool_admin\AdminModeration;

/**
 * A Helper Class to assist with the publishing and unpublishing bulk action.
 *   - Called by Publish Latest Revision and Unpublish Current Revision Bulk Operations
 *   - Easy one-stop shop to make modifications to these bulk actions.
 */
class AdminModeration
{
    //set this to true to send to $testEmailList
    private $testMode = false;
    private $entity = null;
    private $nid = 0;
    private $currentRevisionId = 0;
    private $latestRevisionId = 0;
    private $status = 0; // Default is 0, unpublish.

    public function __construct($entity, $status)
    {
        $this->entity = $entity;
        if (!is_null($status)) {
          $this->status = $status;
        }
        $this->nid = $this->entity->id();
    }

    /**
     * Unpublish current revision.
     */
    public function unpublish() {
      //drupal_set_message($message); //deprecated but still okay to use
      \Drupal::logger('AdminModeration-log')->notice(utf8_encode('Unpublish action in cool_admin'));
      \Drupal::Messenger()->addStatus(utf8_encode('Unpublish action in cool_admin'));
      $entity_manager = \Drupal::entityTypeManager();
      $this->entity->set('moderation_state', 'archived');
      if ($this->entity instanceof RevisionLogInterface) {
        // $now = time();
        $this->entity->setRevisionCreationTime(\Drupal::time()->getRequestTime());
        $msg = 'Bulk operation create archived revision';
        $this->entity->setRevisionLogMessage($msg);
        $current_uid = \Drupal::currentUser()->id();
        $this->entity->setRevisionUserId($current_uid);
      }
      $this->entity->save();
      $this->entity = $entity_manager->getStorage($this->entity->getEntityTypeId())->load($this->nid);
      $this->entity->set('moderation_state', 'draft');
      if ($this->entity instanceof RevisionLogInterface) {
        // $now = time();
        $this->entity->setRevisionCreationTime(\Drupal::time()->getRequestTime());
        $msg = 'Bulk operation create draft revision';
        $this->entity->setRevisionLogMessage($msg);
        $current_uid = \Drupal::currentUser()->id();
        $this->entity->setRevisionUserId($current_uid);
      }
      $this->entity->save();
      $this->entity = $entity_manager->getStorage($this->entity->getEntityTypeId())->load($this->nid);
      return $this->entity;
    }

    /**
     * Publish Latest Revision.
     */
    public function publish() {
      //drupal_set_message($message); //deprecated but still okay to use
      \Drupal::logger('AdminModeration-log')->notice(utf8_encode('Publish latest revision bulk operation'));
      \Drupal::Messenger()->addStatus(utf8_encode('Publish latest revision bulk operation'));
      $entity_manager = \Drupal::entityTypeManager();
      $this->entity->set('moderation_state', 'published');
      if ($this->entity instanceof RevisionLogInterface) {
        // $now = time();
        $this->entity->setRevisionCreationTime(\Drupal::time()->getRequestTime());
        $msg = 'Bulk operation publish revision';
        $this->entity->setRevisionLogMessage($msg);
        $current_uid = \Drupal::currentUser()->id();
        $this->entity->setRevisionUserId($current_uid);
      }
      $this->entity->save();
      $entity_manager = \Drupal::entityTypeManager();
      $this->entity = $entity_manager->getStorage('node')->load($this->nid);
      return $this->entity;
    }

    /**
     *
     */
    private function privateSomething() {
        //not sure if need to update both translations or just one
        $this->entity->getTranslation("en")->set('moderation_state', 'published');
        $this->entity->getTranslation("fr")->set('moderation_state', 'published');
        $this->entity->save(); //not needed?
    }
}

?>

file: cool_admin/src/Plugin/Action/PublishLatestRevisionAction.php

namespace Drupal\cool_admin\Plugin\Action;

//use Drupal\views_bulk_operations\Action\ViewsBulkOperationsActionBase;
//use Drupal\views_bulk_operations\Action\ViewsBulkOperationsPreconfigurationInterface;
use Drupal\Core\Action\ActionBase;
use Drupal\Core\Plugin\PluginFormInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\cool_admin\AdminModeration;

/**
 * An example action covering most of the possible options.
 *
 * If type is left empty, action will be selectable for all
 * entity types.
 *
 * @Action(
 *   id = "publish_latest_revision_action",
 *   label = @Translation("Publish Latest Revision"),
 *   type = "node",
 *   confirm = TRUE,
 * )
 */
//only need to add "implements" keywords below if we are goign to add configuration forms to the confirmation step.... not the case here!
class PublishLatestRevisionAction extends ActionBase/*extends ViewsBulkOperationsActionBase implements ViewsBulkOperationsPreconfigurationInterface, PluginFormInterface*/
{



  /**
   * {@inheritdoc}
   */
  public function execute($entity = NULL) {
    /*
     * All config resides in $this->configuration.
     * Passed view rows will be available in $this->context.
     * Data about the view used to select results and optionally
     * the batch context are available in $this->context or externally
     * through the public getContext() method.
     * The entire ViewExecutable object  with selected result
     * rows is available in $this->view or externally through
     * the public getView() method.
     */

    // Do some processing..
    // ...
    // drupal_set_message("Publish Latest Revision for - ".$entity->label()); // Deprecated and improperly used, need placeholders for variables in messages otherwise fills up the locale table with junk.
    \Drupal::Messenger()->addStatus(utf8_encode('Publish bulk operation by cool_admin module'));
    \Drupal::logger('ADMIN_MODERATION')->notice("EXECUTING PUBLISH LATEST REVISION OF ".$entity->label());

    $adminModeration = new AdminModeration($entity, \Drupal\node\NodeInterface::PUBLISHED);
    $entity = $adminModeration->publish();

    // Check if published
    if (!$entity->isPublished()){
        $msg = "Something went wrong, the entity must be published by this point.  Review your content moderation configuration make sure you have archive state which sets current revision and a published state and try again.";
        \Drupal::Messenger()->addError(utf8_encode($msg));
        \Drupal::logger('ADMIN_MODERATION')->warning($msg);
        return $msg;
    }

    return sprintf('Example action (configuration: %s)', print_r($this->configuration, TRUE));
  }

  /**
   * {@inheritdoc}
   */
  /*
  public function buildPreConfigurationForm(array $form, array $values, FormStateInterface $form_state) {
    $form['example_preconfig_setting'] = [
      '#title' => $this->t('Example setting'),
      '#type' => 'textfield',
      '#default_value' => isset($values['example_preconfig_setting']) ? $values['example_preconfig_setting'] : '',
    ];
    return $form;
  }
  */

  /**
   * Configuration form builder.
   *
   * If this method has implementation, the action is
   * considered to be configurable.
   *
   * @param array $form
   *   Form array.
   * @param Drupal\Core\Form\FormStateInterface $form_state
   *   The form state object.
   *
   * @return array
   *   The configuration form.
   */
  /*
  public function buildConfigurationForm(array $form, FormStateInterface $form_state) {
    $form['example_config_setting'] = [
      '#title' => t('Example setting pre-execute'),
      '#type' => 'textfield',
      '#default_value' => $form_state->getValue('example_config_setting'),
    ];
    return $form;
  }
  */

  /**
   * Submit handler for the action configuration form.
   *
   * If not implemented, the cleaned form values will be
   * passed direclty to the action $configuration parameter.
   *
   * @param array $form
   *   Form array.
   * @param Drupal\Core\Form\FormStateInterface $form_state
   *   The form state object.
   */
  /*
  public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {
    // This is not required here, when this method is not defined,
    // form values are assigned to the action configuration by default.
    // This function is a must only when user input processing is needed.
    $this->configuration['example_config_setting'] = $form_state->getValue('example_config_setting');
  }
  */

  /**
   * {@inheritdoc}
   */
  public function access($object, AccountInterface $account = NULL, $return_as_object = FALSE) {
    if ($object->getEntityType() === 'node') {
      $access = $object->access('update', $account, TRUE)
        ->andIf($object->status->access('edit', $account, TRUE));
      return $return_as_object ? $access : $access->isAllowed();
    }

    // Other entity types may have different
    // access methods and properties.
    return TRUE;
  }

}

file: cool_admin/src/Plugin/Action/UnpublishCurrentRevisionAction.php

namespace Drupal\cool_admin\Plugin\Action;

//use Drupal\views_bulk_operations\Action\ViewsBulkOperationsActionBase;
//use Drupal\views_bulk_operations\Action\ViewsBulkOperationsPreconfigurationInterface;
use Drupal\Core\Action\ActionBase;
use Drupal\Core\Plugin\PluginFormInterface;
use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\cool_admin\AdminModeration;

/**
 * An example action covering most of the possible options.
 *
 * If type is left empty, action will be selectable for all
 * entity types.
 *
 * @Action(
 *   id = "unpublish_current_revision_action",
 *   label = @Translation("Unpublish Current Revision"),
 *   type = "node",
 *   confirm = TRUE,
 * )
 */
//only need to add "implements" keywords below if we are goign to add configuration forms to the confirmation step.... not the case here!
class UnpublishCurrentRevisionAction extends ActionBase/*extends ViewsBulkOperationsActionBase implements ViewsBulkOperationsPreconfigurationInterface, PluginFormInterface*/
{



  /**
   * {@inheritdoc}
   */
  public function execute($entity = NULL) {
    /*
     * All config resides in $this->configuration.
     * Passed view rows will be available in $this->context.
     * Data about the view used to select results and optionally
     * the batch context are available in $this->context or externally
     * through the public getContext() method.
     * The entire ViewExecutable object  with selected result
     * rows is available in $this->view or externally through
     * the public getView() method.
     */

    // Do some processing..
    // ...
    // drupal_set_message("Publish Latest Revision for - ".$entity->label()); // Deprecated and improperly used, need placeholders for variables in messages otherwise fills up the locale table with junk.
    \Drupal::Messenger()->addStatus(utf8_encode('Begin unpublish bulk operation by cool_admin module plugin'));
    \Drupal::logger('ADMIN_MODERATION')->notice("EXECUTING PUBLISH LATEST REVISION OF ".$entity->label());

    $adminModeration = new AdminModeration($entity, \Drupal\node\NodeInterface::NOT_PUBLISHED);
    $entity = $adminModeration->unpublish();

    //check if published
    if ($entity->isPublished()){
        $msg = "Something went wrong, the entity must be unpublished by this point.  Review your content moderation configuration make sure you have archive state which sets current revision and a draft state and try again.";
        \Drupal::Messenger()->addError(utf8_encode($msg));
        \Drupal::logger('ADMIN_MODERATION')->warning($msg);
        return $msg;
    }

    return sprintf('Example action (configuration: %s)', print_r($this->configuration, TRUE));
  }

  /**
   * {@inheritdoc}
   */

  /*
  public function buildPreConfigurationForm(array $form, array $values, FormStateInterface $form_state) {
    $form['example_preconfig_setting'] = [
      '#title' => $this->t('Example setting'),
      '#type' => 'textfield',
      '#default_value' => isset($values['example_preconfig_setting']) ? $values['example_preconfig_setting'] : '',
    ];
    return $form;
  }
  */

  /**
   * Configuration form builder.
   *
   * If this method has implementation, the action is
   * considered to be configurable.
   *
   * @param array $form
   *   Form array.
   * @param Drupal\Core\Form\FormStateInterface $form_state
   *   The form state object.
   *
   * @return array
   *   The configuration form.
   */
  /*
  public function buildConfigurationForm(array $form, FormStateInterface $form_state) {
    $form['example_config_setting'] = [
      '#title' => t('Example setting pre-execute'),
      '#type' => 'textfield',
      '#default_value' => $form_state->getValue('example_config_setting'),
    ];
    return $form;
  }
  */

  /**
   * Submit handler for the action configuration form.
   *
   * If not implemented, the cleaned form values will be
   * passed direclty to the action $configuration parameter.
   *
   * @param array $form
   *   Form array.
   * @param Drupal\Core\Form\FormStateInterface $form_state
   *   The form state object.
   */
  /*
  public function submitConfigurationForm(array &$form, FormStateInterface $form_state) {
    // This is not required here, when this method is not defined,
    // form values are assigned to the action configuration by default.
    // This function is a must only when user input processing is needed.
    $this->configuration['example_config_setting'] = $form_state->getValue('example_config_setting');
  }
  */

  /**
   * {@inheritdoc}
   */
  public function access($object, AccountInterface $account = NULL, $return_as_object = FALSE) {
    if ($object->getEntityType() === 'node') {
      $access = $object->access('update', $account, TRUE)
        ->andIf($object->status->access('edit', $account, TRUE));
      return $return_as_object ? $access : $access->isAllowed();
    }

    // Other entity types may have different
    // access methods and properties.
    return TRUE;
  }

}

file: cool_admin/cool_admin.info.yml

name: Cool Admin
description: 'Provides general administration features for a cool website.'
package: Custom
type: module
core: 8.x

Now that wasn't so hard now was it ?

to make my example work in drupal core would have to find out (add logic to ) dynamically determine which moderation states set the current revision state. In our case we have archived, published and draft but others might have needs-review , draft and published, the state that sets the current revision state is configurable , so to make the code work the logic would have to find that. there's only two states that can ever do this, one for publish (obvious) and the other (not so obvious but could make some logic to find out)

These are really the only two things anyone really gives a hoot about for bulk operation, the rest is done when editing a node. like really think about the workflow and process, it's not that complicated, the essentials is they need some way to take something or a bunch of items out of publication quickly or to vice versa publish something easily in a way that makes sense to most people, that's why we put this on the admin/content page .

*** EDIT *** confirmed this patch reroll is needed for 8.7.x

patch 96 needed a reroll for 8.7.x

Patch applies correctly but it doesn't seem to fix the issue for me FYI.

Just tried this patch. We had the same result. It applies but does not appear to resolve any issues. Are there any thoughts on next steps or workarounds for this?

I was successful by applying the code snippet in #100, which does what I expected. Just make sure the files are placed into the proper folders and the class in src folder is named "AdminModeration.php" and the use statement is removed (perform a basic code check on that). Then enable the module and you will see two new options in the Actions window: "Publish latest revision" and "Unpublish current revision". Both are performing the job as expected with content moderation enabled. No additional patch needed.

Perhaps it makes sense to make a contrib module from that in the meantime?

hi @mirsoft, yes I agree, I will take that code and make a contrib module out of it.

What should I call the module ? moderated_content_bulk_publishing? (Moderated content bulk publishing)?
once I decide on a name, I can create the project and publish a release, would be nice as a contrib module.

NOTE: I have an updated version of that code, been hardenned with production use and feedback improving it. Just need to decide on a contrib module name.

Thanks @joseph.olstad, yes, the name looks good to me.

@joseph.olstad I also have a use case for this functionality and would prefer to use a contrib module. You could just drop "ing" from the module name to make it (slightly) shorter, i.e. moderated_content_bulk_publish. Hoping to see a sandbox soon!

Ok I have created the project, will soon (maybe this weekend) add the source code and make a build.
https://www.drupal.org/project/moderated_content_bulk_publish
If you want to help maintain and develop this please let me know and I will grant the permissions for this.

Awesome @joseph.olstad! Please push the source code at your earliest convenience. I tried the example code in #100 but it does not check if the state transitions are valid so an entity can be published from any moderation state. I'm wondering if the revisions in moderated_content_bulk_publish include that check and would be happy to move the discussion over there.

@amanire and @mirsoft, I have pushed the source code and published a release of the new module.
I have not tried this new module yet, but it is based off of a working custom module.
It 'should' work 'knock on wood'.
If you want to join in and make improvements or help get it ready for a beta then RC (release candidate) , please let me know and I will grant co-maintainer priviledges.

Thanks,
Joseph.Olstad

@joseph.olstad, thank you!

I'd been trying for hours to set and save a moderation state on unpublished content after installing Content Moderation on an existing site. I was not about to manually set 662 unpublished nodes to Draft. While the system would display Draft as a moderation state for unpublished content in lists, it was superficial. I couldn't track moderated content anywhere. After running Unpublish current revision now I can actually see all the unpublished the admin/content/moderated.

Hi @nateB , glad you like my module! I hope others find it useful as well.

You can thank northern taxpayers for this module, they paid for it. Now due to open source, we have residual value from what we spend on taxes!

A detail I didn't realize (may or may not be helpful to you, @anavarre + @uxmfdesign): After applying the patch, you have to add the actual actions -- i.e.

1. Apply patch.
2. Go to Actions admin: /admin/config/system/actions
3. Use the "Add advanced action" thing at the top to add new actions for moderation state transitions, as needed.

I thought "dynamically provide action plugins" mean that the actual per-moderation-transition actions would automatically exist after I patch the module, but I guess what it actually meant was that (from the site builder's perspective) new "types of advanced actions" become available to me, for each moderation transition.
(...or, I think that's what it actually meant! 😁 )

...I'm probably missing something, but, the new "moderation state transition) advanced actions I created are available to me on /admin/content, and they seem to work totally fine.......? (I may have misunderstood other commenters, but I thought I'd only be able to use these new advanced actions on /admin/content/moderated )

Just to clarify for others, if you prefer you can just use the contrib module I created instead of patching. The contrib module adds a views bulk operation plugin on the admin/content page for 'publish current revision' and 'unpublish current revision' in bulk.

Is there some way the module could do it dynamically? We have a lot of transitions between states other than just publish/unpublish, so it seems unwieldy if we would have to create an Action plugin for all of them.

@Rob230 , please submit a patch to moderated_content_bulk_publish :

The reason for the plugin is because you can have several revisions in different moderation states. The reason for these plugins is if you want to ensure that the current revision is placed into the desired state as indicated by the label of the plugin.

So most use cases someone would want to publish the newest revision even if a previous revision is already published

same thing for unpublish, to make sure that all revisions are unpublished the plugin does just that and puts the current revision into draft state ensuring that no other revision is published. The plugin allows bulk.

So, if you wanted to make a bulk operations plugin for lets say put the current revision into needs review, you could create a patch with that functionality and push it to the module.

The module also provides bulk sticky/pin and bulk unsticky/unpin

You could create a plugin that bulk sets current revision to a configured/desired state. The tricky part is, anyone can define their own moderation states, so if you want some special state it would have to be configurable so you'd have to also add a configuration form and retrieve that configuration and put it into the plugin, enable/disable configuration for the custom moderation state(s).

Please submit the patch(es) to the project and I will review.

Aren't states and transitions quite specific though? We have things like "submit for approval", "reset to draft" and "stand down". I I don't think they are very relevant to other people. Surely there needs to be a solution which will work for any arbitrary state transition that is created in content moderation workflow, as they can be created in the UI at any time.

@alisonjo315 your comment #115 is a great help. Thanks!

@sastha Oh I'm so glad, you're welcome!

The patch in #102 applies cleanly to drupal/core (8.7.10)

With the Actions module enabled, new options appear in the advanced action list at /configuration/system/actions:

The add action form lists available workflows:

Once saved, the actions appear in the list and function as expected:

I'd mark RTBC, but the patch has failing tests that I assume need to be addressed.

WOW, thank you @thejimbirch! Just what I had hoped to see here. My first comment here overlapped with your excellent screenshots.

[edit]
Ok yes patch works as advertised: I had to enable core module Action to get it going. I had hope this would expose the same set of Advanced Actions to Views Bulk Operations, but it does not. Is there something about either this patch or its newly created contrib module that would help expose its generated Advanced Actions to VBO? In VBO, I can add the root ModerationStateChange action, and it works as it should, but it's not as seamless an end user experience as the Advanced Actions provided by this patch.

When I use core Bulk Actions in my custom View, my new Advanced Action does work perfectly.
[/edit]

Now that (with this patch in #112) we can publish/unpublish and even set other moderation state changes, I found the need to change the patched Action to call $entity->validate() like:
./core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php

  $revision->save();

becomes

    $violations = $revision->validate();
    if ($violations->count() > 0) {
      $this->messenger()->addError($violations[0]->getMessage());
      return;
    }
    $revision->save();

This allows content moderation state change-blocking entity validations (like fields required for publish but found empty or other entity constraints) to be considered before, say, pushing this entity live to the world.

Two things:
While I've greatly enjoyed the use of patch in #102, looks like #85 introduced some failing tests due to \Drupal::entityTypeManager() which I think should be easy enough to mock up swap out.. then we can know if these additional patches are helping or hurting! :D

And the action provided by this patch needs to act on the latest revision of the translation of the entity passed into it. I've updated the patch with the following to do just that (see interdiff). Before it was loading latest revision and then loading translation, not getting latest revision of the translation.

I'm sorry for the noise. Corrected patch here. Note: Still module unit tests fail.

I set the issue status back to Needs work because of #129:

Note: Still module unit tests fail.

Thank you!

The last test failures are weird because it's saying that the underlying Selenium system from Drupal\FunctionalJavascriptTests\WebDriverTestBase::assertSession() doesn't support status codes. But it should. I'm rerunning the tests.

As I understand, it's not weird due to #2827014: Throw an exception when testing status code or response headers in functional JavaScript tests.
I'm adding a new patch trying to remove statusCodeEquals().

Yey! Finally, ready for the review!
Sorry for spam :)

I went through the following steps in the UI with a fresh Drupal install:

1. Enabled the Editorial workflow for the Article content type as specified in the issue description.
2. Applied the patch in #148 and enabled the core Action module.
3. Added "Change moderation state to Published" and "Change moderation state to Archived" advanced actions as described in #124
4. On the content administration view, successfully published several unpublished articles using the new "Change moderation state to Published" bulk action.
5. On the content administration view, successfully unpublished several published articles using the new "Change moderation state to Archived" bulk action.

Additionally, after re-reading the issue description, I checked the moderated_content admin view and:

1. Added the Node operations bulk form field to the view (with the previously added content moderation actions).
2. Successfully applied the "Change moderation state to Published" to bulk publish draft and archived articles.
3. Added the "Change moderation state to Draft" advanced action and successfully applied the "Change moderation state to Draft" to bulk update Archived articles. This doesn't seem terribly important, but it's a conceivable use case.

Note: it's probably worth mentioning (and restating) a few odd scenarios:

- You would most likely never want to enable a bulk moderation state action for changing nodes to a draft state, since this will create an empty new draft revision while preserving the published revision. This could confuse content administrators if they were expecting the node to become unpublished and revert to a draft state.
- "Change moderation state to Archived" is useful for archiving published nodes on admin/content but is unhelpful on the moderated_content view since it does not display nodes with latest revision in the published state. The default workflow does not allow transitions from draft to archived and results in a "No access to execute Change moderation state to Published message.

I haven't had a chance to check actions on translated content yet.

Anyway, the unit tests are passing. Nice work!

I applied the patch in #148.

I have 3 states. draft, published and unpublished.

I can bulk update state of the nodes from draft to published.

But I can't change the state of the published nodes.

@hmdnawaz Have you applied the appropriate transitions from published? Transition states on how I have them setup.

I have tested this on 8.7.11 test site and following comment #124 with the patch from #149. It is working, will test later with newer builds as well.

+++ b/core/modules/content_moderation/src/Plugin/Action/ModerationStateChange.php
@@ -0,0 +1,283 @@
+  /**
+   * {@inheritdoc}
+   */
+  public function calculateDependencies() {
+    if (!empty($this->configuration['workflow'])) {
+      $this->addDependency('config', 'workflows.workflow.' . $this->configuration['workflow']);
+    }
+    return $this->dependencies;
+  }

There was some feedback in #65 onward that needs to be addressed with the entire deriver approach that has been implemented.

The tl;dr is, a dependency on the workflow is insufficient for anything that deals with states, since states can be deleted without deleting the whole workflow. That is, there is no way to add a dependency on an individual workflow state and have that handled throughout the lifecycle of the workflow config.

The only viable implementation I can think of would be a single plugin called "Moderate content", which would work something like:

1. The plugin would iterate the content selected by the user.
2. In an additional confirmation step, the plugin would group entities by their starting state.
3. Each starting state would have a select box of states the current user has access to transition to, with an additional "do nothing" option.
4. Confirming the starting and ending state would then execute the action.

@Sam152: A single plugin actually sounds more useful and flexible to me. Presumably that would mean you could, for example, archive some content and publish other content in the same user action.

From a content admin's point of view that's fewer steps to screw up, they can start by selecting all the things that need a moderation change, and only need to make that selection once.

A dynamic plugin per state change has the slight gotcha of the user being able to select a group of content for which no single state change is globally applicable, right? So you wind up having to make multiple selections and process them separately per starting state.

One use case to support this would be when a the wrong content has gotten published. The content admin can select that, also select the correct but unpublished content, and fix it in one operation.

A single plugin would also result in less clutter in the operations list for sites with many possible workflow states.

Is there any chance of being able to act on non-latest revisions? I haven't looked at the API, so perhaps there's a limitation there, but it strikes me that it would be useful. We, for example, have a need to remind content editors to review their published content, even while they might have forward draft revisions in existence. So we'd have this situation:

* Revision 2: Draft
* Revision 1: Published

And we want to push revision 1 to "Published, needs review" to remind the person who owns the content that they need to maintain their content (we require they update it every 6 months) or else we're going to unpublish it. Unfortunately there's currently no way for us to do that since these plugins can only operate on the latest revision.

Hi,

I am using Drupal Version 8.7.12, I tried with #102 and #116. But there is no luck to fix this issue. I am looking for assistance from the one who has fixed issue for publish/unpublish action plugin for every moderation state change.

@gurpreet.kaur
We're using the https://github.com/naveenvalecha/content_moderation_actions on a client project.

@naveenvalecha, thanks, I will look review your module and look into perhaps bringing some of your ideas into moderated_content_bulk_publish

After adding patch #149 and following the instructions on #124 and #152.
I found that although it worked. It only work if my user was the last one to save the node revision. As an administrator, I had to go into each node and save it again. Currently, the patch seems to only let you change bulk actions if you are the last user who saves the lastest node revision.
It be great if as an administrator I can do bulk actions regardless.

FYI, for those using moderated_content_bulk_publish, I just released 8.x-1.0-beta4 2.0.0 today, if you're on a previous beta I encourage you to upgrade.

Currently, the patch seems to only let you change bulk actions if you are the last user who saves the lastest node revision.

@bernardm28 Are you sure your transition permissions are set properly?

I did not run into this issue (I tested with admin + user with and without bypass access permissions). For me everything works as expected, I tested patch #149 (explanation in #124 definitely helped :)

When testing this patch I found that it didn't work with the latest stable version of Groups, more specifically with the groups content moderation integration. This patch uses the core access checks to do the permissions checks, which groups doesn't hook into anymore. If a user has permission to perform the status transition for content within their group but not the global (outside of groups) status transition permission the user receives an access denied.

To get this to work on my site I re-rolled the patch to instead use Content Moderation's StateTransitionValidation service to perform the permissions check. This service not only checks if the transition is valid, it also checks if the user account has permission to perform the transition. Groups overrides this service and adds its permissions checks in there. I went this route because it seemed more straightforward to update this patch rather than update groups to hook into core's hasPermission call. The downside of this approach is that the valid status transition check is now performed twice (though the code can be refactored to remove that second check).

Thanks for this, Demma10. We ran into the same issue but hadn't yet had the chance to look deeply into the problem. Thanks for the fix. I'll test it and report back.

The patch in 165 is malformed a bit (includes itself in the diff). Here's a fixed version.

I should add: in my initial testing #167 does appear to fix the problem.

Thanks so much for fixing the patch azinck. My git patching skills aren't the best (I think I ended up creating a patch of a patch).

Fixed failed tests of patch #167.