When RevisionableEntityBundleInterface::shouldCreateNewRevision is TRUE, create a new revision on PATCH requests and add an entry in the revision log.

1. +++ b/jsonapi.services.yml
   @@ -143,7 +143,7 @@ services:
   -    class: \Drupal\jsonapi\Controller\EntityResource
   +    class: Drupal\jsonapi\Controller\EntityResource
   

   🔍🐛 Übernit: I don't see why we're changing this. Can easily be reverted on commit though.

2. +++ b/src/Controller/EntityResource.php
   @@ -299,6 +323,20 @@ class EntityResource {
   +    // Set revision data details for revisionable entities.
   +    if ($entity->getEntityType()->isRevisionable()) {
   +      $bundle_storage = $this->entityTypeManager->getStorage($entity->getEntityType()->getBundleEntityType());
   +      $bundle_entity = $bundle_storage->load($entity->bundle());
   +      if ($bundle_entity instanceof RevisionableEntityBundleInterface) {
   +        $entity->setNewRevision($bundle_entity->shouldCreateNewRevision());
   +      }
   +      if ($entity instanceof RevisionLogInterface) {
   +        $entity->setRevisionUserId($this->user->id());
   +        $entity->setRevisionCreationTime($this->time->getRequestTime());
   +      }
   

   ✅ This is functionally identical to the code I wrote nearly two years ago in #2838395-27: [PP-1] Entity API bug affecting REST: "last changed" timestamp not updated and "create new revision" setting not respected when PATCHing via REST. Looks good :)

3. +++ b/tests/src/Functional/BlockContentTest.php
   @@ -61,7 +61,7 @@ class BlockContentTest extends ResourceTestBase {
          $block_content_type = BlockContentType::create([
            'id' => 'basic',
            'label' => 'basic',
   -        'revision' => TRUE,
   +        'revision' => FALSE,
   

   ✅ block_content.type.basic.yml also has revision: 0, and we're already ensuring the correct expected behavior for Node, no need to do it for multiple entity types. Works for me. 👍

4. +++ b/tests/src/Functional/NodeTest.php
   @@ -42,6 +42,11 @@ class NodeTest extends ResourceTestBase {
   +  protected static $newRevisionsShouldBeAutomatic = TRUE;
   

   👏 Great, we're testing this for Node, where this is most expected to work.

5. +++ b/tests/src/Functional/ResourceTestBase.php
   @@ -2068,7 +2075,7 @@ abstract class ResourceTestBase extends BrowserTestBase {
   -    $original_revision_id = (int) $this->entity->getRevisionId();
   +    $prior_revision_id = (int) $this->entityLoadUnchanged($this->entity->id())->getRevisionId();
   
   @@ -2193,6 +2200,9 @@ abstract class ResourceTestBase extends BrowserTestBase {
   +    $updated_entity = $this->entityLoadUnchanged($this->entity->id());
   

   ✅ We now have to bypass the test runner's static entity cache, to ensure we fetch the latest revision of the just-PATCHed entity. 👍

6. +++ b/tests/src/Functional/ResourceTestBase.php
   @@ -2193,6 +2200,9 @@ abstract class ResourceTestBase extends BrowserTestBase {
   +    $this->assertSame(static::$newRevisionsShouldBeAutomatic, $prior_revision_id < (int) $updated_entity->getRevisionId());
   

   ✅ And we then verify the current revision ID is greater than the prior.

7. +++ b/tests/src/Functional/ResourceTestBase.php
   @@ -3344,4 +3357,18 @@ abstract class ResourceTestBase extends BrowserTestBase {
   +   */
   +  protected function entityLoadUnchanged($id) {
   +    $this->entityStorage->resetCache();
   +    return $this->entityStorage->loadUnchanged($id);
   +  }
   

   ✅ Ah, \Drupal\Core\Entity\ContentEntityStorageBase has a $this->latestRevisionIds static cache, which without this ::resetCache() call would result in the test assertions being unable to correctly assert that a revision is indeed the latest revision. Static caches strike again!

1. +++ b/src/Controller/EntityResource.php
   @@ -299,6 +323,20 @@ class EntityResource {
   +      if ($entity instanceof RevisionLogInterface) {
   +        $entity->setRevisionUserId($this->user->id());
   +        $entity->setRevisionCreationTime($this->time->getRequestTime());
   +      }
   

   Shouldn't this happen only when we create a new revision? I.e. we should add an ::isNewRevision() call. This would also take care of new entities.

2. +++ b/tests/src/Functional/ResourceTestBase.php
   @@ -3344,4 +3357,18 @@ abstract class ResourceTestBase extends BrowserTestBase {
   +  protected function entityLoadUnchanged($id) {
   

   Why do we need this? ::loadUnchanged() will always load data from the storage.

1. I had to ping @effulgentsia to figure out what you meant, but in hindsight it's obvious 😅
2. #4.7 explains the reason. If you comment out that resetCache() you'll see that the test fails.

Test coverage for #6.1.

• Failing patch is #2 + this interdiff (i.e. without the fix in #6).
• Passing patch is #6 + this interdiff.

Looks good to me, if green!

For #6.2, @plach just opened #3038706: Clean up ::entityLoadUnchanged in ResourceTestBase. Adding a @todo to point to that, so we can get rid of that in the future.