Content Moderation missing permission descriptions

+++ b/core/modules/content_moderation/src/Permissions.php
@@ -37,4 +41,21 @@ public function transitionPermissions() {
+  private static function getFromDescription($transition) {

Does this need to be a static function?

Awesome, looks like we already have a test for this, it just needs updating.

+++ b/core/modules/content_moderation/src/Permissions.php
@@ -37,4 +41,21 @@ public function transitionPermissions() {
+  /**
+   * @param \Drupal\workflows\TransitionInterface $transition
+   *   Transition to get From list from.
+   *
+   * @return string
+   *   Names of the From transition states.
+   */
+  private function getFromDescription($transition) {
+    $from = [];
+
+    foreach ($transition->from() as $state) {
+      $from[] = $state->label();
+    }
+
+    return implode($from, ', ');
+  }

This method is missing a docblock describing what it does and should also be 'protected' visibility.

We can actually eliminate the method entirely and turn it into a one-liner:

'description' => $this->t('Move content from %from state to %to state.', [
  '%from' => implode(', ', array_map([State::class, 'labelCallback'], $transition->from())),
  '%to' => $transition->to()->label(),
]),

Where 'State' is imported from Drupal\workflows\State.

We could get probably try to get fancy with conditional plural formatting for state/states, but I don't think it's warranted. This is a big improvement as it stands.

Looks good to me.

Yep this is better but I think we should use ->formatPlural() to cope with the multiple from states. It's not that much of an onus. And it'll mean that we have the correct strings for translators first time.

Also this is more a task than a feature.

+++ b/core/modules/content_moderation/tests/src/Kernel/ContentModerationPermissionsTest.php
@@ -59,9 +59,11 @@ public function permissionsTestCases() {
+            'description' => 'Move content from <em class="placeholder">Draft, Published</em> state to <em class="placeholder">Published</em> state.',
...
+            'description' => 'Move content from <em class="placeholder">Draft, Published</em> state to <em class="placeholder">Draft</em> state.',

Looks like this tests should fail.

Also funnily enough - is the word "state" or "states" that useful? Maybe the message could be "Change state from Draft, Published to Published" hmmm.... but I guess there you'll need an or which is even harder.

Here is the expanded test case. I had a crack at implementing the suggestion in #13, it looked something like:

diff --git a/core/modules/content_moderation/src/Permissions.php b/core/modules/content_moderation/src/Permissions.php
index c02308c17b..dacbf54f3b 100644
--- a/core/modules/content_moderation/src/Permissions.php
+++ b/core/modules/content_moderation/src/Permissions.php
@@ -26,19 +26,20 @@ public function transitionPermissions() {
     /** @var \Drupal\workflows\WorkflowInterface $workflow */
     foreach (Workflow::loadMultipleByType('content_moderation') as $id => $workflow) {
       foreach ($workflow->getTypePlugin()->getTransitions() as $transition) {
+
+        $from_states = $transition->from();
+        $last_from_state = array_pop($from_states);
+        $from_label = count($from_states) ? sprintf('%s %s %s', implode(', ', array_map([State::class,'labelCallback'], $from_states)), $this->t('or'), $last_from_state->label()) : $last_from_state->label();
+
         $permissions['use ' . $workflow->id() . ' transition ' . $transition->id()] = [
           'title' => $this->t('%workflow workflow: Use %transition transition.', [
             '%workflow' => $workflow->label(),
             '%transition' => $transition->label(),
           ]),
-          'description' => $this->formatPlural(
-            count($transition->from()),
-            'Move content from %from state to %to state.',
-            'Move content from %from states to %to state.', [
-              '%from' => implode(', ', array_map([State::class,'labelCallback'], $transition->from())),
-              '%to' => $transition->to()->label(),
-            ]
-          ),
+          'description' => $this->t('Move content from %from to %to.', [
+            '%from' => $from_label,
+            '%to' => $transition->to()->label(),
+          ]),
         ];
       }
     }

and the test cases ended up looking like:

Array (
    'use simple_workflow transition publish' => Array (
        'title' => '<em class="placeholder">Simpl...ition.'
        'description' => 'Move content from <em class="placeholder">Draft or Published</em> to <em class="placeholder">Published</em>.'
    )
    'use simple_workflow transition create_new_draft' => Array (
        'title' => '<em class="placeholder">Simpl...ition.'
        'description' => 'Move content from <em class="placeholder">Draft or Published</em> to <em class="placeholder">Draft</em>.'
    )
    'use simple_workflow transition archive' => Array (
        'title' => '<em class="placeholder">Simpl...ition.'
        'description' => 'Move content from <em class="placeholder">Published</em> to <em class="placeholder">Archived</em>.'
    )
)

I don't really mind either one, I'm not sure the benefit is worth the complexity, but happy to see what others think. One thing to note is, the "or" would always have to be part of the <em> placeholder string, because there is a dynamic number of from states. Doesn't read that nicely to have parts of the sentence classed with "placeholder", given we're trained to treat that as the "variable" part of messages.

LGTM.

Needs some help to get past DrupalCI after nearly three years.

Updated patch as per #24

Thanks for the reroll!

Committed/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!