Possible to configure in such a way that the node form has no "Save" button

1. +++ b/workbench_moderation.module
   @@ -235,3 +235,37 @@ function workbench_moderation_views_data_alter(array &$data) {
   +function wdc_user_node_create_access(AccountInterface $account, array $context, $entity_bundle) {
   

   This should be implemented on behalf of the workbench_moderation module.

   It should be noted that by solving this via a node-specific hook, this solution will only be applicable to nodes. I haven't done further investigation to determine if there is a more general solution, but this should be fine for the time being.

2. +++ b/workbench_moderation.module
   @@ -235,3 +235,37 @@ function workbench_moderation_views_data_alter(array &$data) {
   +    $definitions = \Drupal::config('node.type.' . $entity_bundle)->get('third_party_settings');
   +    $default_moderation_state = $definitions['workbench_moderation']['default_moderation_state'];
   

   Rather than directly load the setting from config, I think it would be preferable here to load the config entity (in this case it would be a NodeTypeInterface) via the entity type manager, and then the getThirdPartySetting($module, $key, $default = NULL) method could be used.

1. +++ b/src/ModerationInformationInterface.php
   @@ -208,4 +209,20 @@ interface ModerationInformationInterface {
   +   * @param $entity_type_id
   ...
   +   * @param $entity_bundle
   

   Parameter type is missing here.

2. +++ b/src/ModerationInformationInterface.php
   @@ -208,4 +209,20 @@ interface ModerationInformationInterface {
   +  public function isModerationBundleCreateValid(AccountInterface $account, $entity_type_id, $entity_bundle);
   

   Perhaps rename this method to hasValidTransitions()? It could then be used for the edit operation too (which still needs to be implemented).

3. +++ b/workbench_moderation.module
   @@ -180,7 +180,7 @@ function workbench_moderation_node_access(NodeInterface $entity, $operation, Acc
   +    return !empty($transition_validation->getValidTransitionTargets($entity, $account))
   

   Is this change necessary?

4. +++ b/workbench_moderation.module
   @@ -235,3 +235,16 @@ function workbench_moderation_views_data_alter(array &$data) {
   + * Implements hook_node_create_access().
   

   This is still node-specific. Can the same be achieved more generally via the entity hook mentioned above?

These changes are complex enough that I think some tests would help ensure this situation doesn't happen again (those tests can also be used to demonstrate the current bug this is fixing).

This is looking really good.

@pericxc pointed out that there isn't really a generic entity access hook, so for the time being, this will need to be mostly node-specific.

I'm going to mark this RTBC to get a few more eyes on for review.

@kevin.dutra you're totally right. When I spoke with @pericxc whatever search I was using to find docs on those seemed to not find them.

So given that, I think the node-specific access hooks should be refactored, and one additional test should be added that uses a non-node entity type (EntityTest probably).

The module is using hook_node_access for the 'view' and 'update' operation

Indeed it is. So perhaps this patch can continue to utilize node-specific access, but we could open a follow-up to generalize this logic where possible. Note the isPublished() method utilized in workbench_moderation_node_access(), while used on several core entity types, does not appear to be part of a more general interface.

1. +++ b/src/Tests/ModerationStateTransitionsValidationTest.php
   @@ -0,0 +1,155 @@
   +
   ...
   +
   

   Nit: extra line breaks here.

2. +++ b/workbench_moderation.module
   @@ -167,22 +167,25 @@ function workbench_moderation_entity_view(array &$build, EntityInterface $entity
   +  $type = $entity->getEntityType();
   

   This variable doesn't appear to be used.

3. +++ b/workbench_moderation.module
   @@ -167,22 +167,25 @@ function workbench_moderation_entity_view(array &$build, EntityInterface $entity
   +    } elseif ($operation == 'update' && $moderation_info->isModeratableEntity($entity) && $entity->moderation_information && $entity->moderation_information->target_id) {
   +      /** @var \Drupal\workbench_moderation\StateTransitionValidation $transition_validation */
   +      $transition_validation = \Drupal::service('workbench_moderation.state_transition_validation');
   +
   +      return !empty($transition_validation->getValidTransitionTargets($entity, $account))
   +        ? AccessResult::neutral()
   +        : AccessResult::forbidden();
   

   Unless I'm missing something, this part isn't node specific at all, so could be moved to apply to all entities? If that is the case, the non-node test should test the update operation too.

This is looking really close.

+++ b/workbench_moderation.module
@@ -168,24 +168,21 @@
+  elseif ($operation == 'update' && $moderation_info->isModeratableEntity($entity) && $entity->moderation_information && $entity->moderation_information->target_id) {

Can you update the non-node test to check for the 'update' operation and expected access?

This looks good. I manually tested as well to confirm. I also like that it takes a further step toward non-node-specific entity moderation.