Consider not returning FALSE when saving a non-default entity revision

Just hit this in the exact same way as #12

Like others, just chiming in to say I ran into this exact issue in 11.1. In my case, I have a half dozen custom content entities that I recently added content moderation for (states of data, don't think of them like Nodes).

In any event, I made them using Drush 13. One of the things Drush generates is an entity form with boilerplate like this:

final class ResultForm extends ContentEntityForm {

  /**
   * {@inheritdoc}
   */
  public function save(array $form, FormStateInterface $form_state): int {
    $result = parent::save($form, $form_state);

    $message_args = ['%label' => $this->entity->toLink()->toString()];
    $logger_args = [
      '%label' => $this->entity->label(),
      'link' => $this->entity->toLink($this->t('View'))->toString(),
    ];

    switch ($result) {
      case SAVED_NEW:
        $this->messenger()->addStatus($this->t('New result %label has been created.', $message_args));
        $this->logger('cci_result')->notice('New result %label has been created.', $logger_args);
        break;

      case SAVED_UPDATED:
        $this->messenger()->addStatus($this->t('The result %label has been updated.', $message_args));
        $this->logger('cci_result')->notice('The result %label has been updated.', $logger_args);
        break;

      default:
        throw new \LogicException('Could not save the entity.');
    }

    $form_state->setRedirectUrl($this->entity->toUrl('collection'));

    return $result;
  }

}

Upon adding content moderation and workflow against them, they started failing when testing "Published" -> "Draft" state. Its because parent::save returns false (which core modules consider okay) and that causes the exception to be thrown here.

Updating my save method to this, mimicing core Media/Node, made it work:

  /**
   * {@inheritdoc}
   */
  public function save(array $form, FormStateInterface $form_state) {
    $result = parent::save($form, $form_state);

    $message_args = ['%label' => $this->entity->toLink()->toString()];
    $logger_args = [
      '%label' => $this->entity->label(),
      'link' => $this->entity->toLink($this->t('View'))->toString(),
    ];

    switch ($result) {
      case SAVED_NEW:
        $this->messenger()->addStatus($this->t('New result %label has been created.', $message_args));
        $this->logger('cci_messages')->notice('New result %label has been created.', $logger_args);
        break;

      default:
        $this->messenger()->addStatus($this->t('The result %label has been updated.', $message_args));
        $this->logger('cci_messages')->notice('The result %label has been updated.', $logger_args);
        break;
    }

    $form_state->setRedirectUrl($this->entity->toUrl());

    return $result;
  }

It was disorienting because the false value returned made me think the save failed - but it doesn't fail. Its just not the default revision. I think this should be addressed as soon as possible, it seems like the code comments are incorrect as mentioned.

And yes, core and specifically entity forms are doing a very bad job at that.

Given that Drupal can make an excellent API platform for controlling and managing data beyond the "Node" type, there must be something that can be done here to improve clarity. The diff looks pretty simple - does it really matter if the saved entity was the default revision or not?