Revisions on relations are not loaded correctly resulting in wrong data in includes

I'm having the same issue. Using ?resourceVersion=rel:working-copy or ?resourceVersion=id:1110 JSON:API returns the correct revision of the node and all the fields, BUT all referenced entities including paragraphs are returned as the "Current Version".

This is our quick fix for this problem, near the line 145, replacing the Entity::loadMultiple, making sure we are using target_revision_id for revisions.

Sorry I have no patch for this, we messed a lot with the file, since we faced also a permissions problem, but this snippet is actually working.

      foreach ($references as $target_type => $targets) {
        $entity_storage = $this->entityTypeManager->getStorage($target_type);
        $targeted_entities = [];
        foreach ($targets as $target) {
          if ($target['type'] == 'target_revision_id') {
            $targeted_entities[] = $entity_storage->loadRevision($target['value']);
          }
          else {
            $targeted_entities[] = $entity_storage->load($target['value']);
          }
        }
      }

I will work on this.

After some digging around I found out that there an issue Add a version negotiation to revisionable resource types

An admin/authenticated doesn't have the proper permissions to get the revision data.

This patch is only a temporary solution but may be of use to people that need something now before the API first initiative can provide the standard entity revision access checks.

This patch allows revisions of included entities to be loaded, so the correct revision based on what is referenced by the revision of the entity being requested.

This patch also provides a new hook which allows module developers the ability to dangerously provide their own custom revision access logic for entities which are not nodes or media. By default other entities, for example the popular paragraphs module, will always come back as an access denied via JSON API unless its referencing the default version.

After applying this patch, you may need to implement your own `hook_jsonapi_revision_view_access()` function. You will see that the patch adds the paragraphs module example into the jsonapi.api.php file which you can copy straight into one of your own custom modules (renaming the function appropriately and clearing caches first of course!)

This patch is for 8.8.x branch.

Another patch which hopefully will stop the 500 errors from the tests (can't seem to run all the tests locally for some reason).

I hate to say this but … 😞

+++ b/core/modules/jsonapi/src/Access/EntityAccessChecker.php
@@ -256,10 +256,22 @@ protected function checkRevisionViewAccess(EntityInterface $entity, AccountInter
+        // Allow modules that know what they are doing a way to provide custom revision access.
+        $custom_access_list = \Drupal::moduleHandler()->invokeAll('jsonapi_revision_view_access', [$entity, $account]);

We specifically don't want to add more JSON:API-specific APIs to Drupal core. We need to add the missing APIs in Entity API.

See #3035113: JSON:API is typehinting LatestRevisionCheck because there is no Entity Revision Access API yet, remove that typehint once such an API exists. This is a duplicate of that AFAICT. Or am I mistaken?

Yeah, I didn't expect this to go in - I just needed to host the patch file for people that need a solution now!

This still doesn't address the comments in #10.

Patch attached improves 9 by allowing sub referenced paragraphs to come through the json API i.e. where you might have a paragraph referencing a paragraph - JSON API by default seems to forget its dealing with revisions after its gone two deep in the include tree. Can't tell if this is Paragraphs fault or not. Paragraphs seem to implement RevisionableInterface but $resource_type->isVersionable() when wrapping a paragraph always says no :(

Alright! I need your help. So I have content types with paragraph types nested inside them. But when I try to access a draft version of the content fields inside the paragraph type it's not returning any data. I came across this thread and applied a few of these patches but I'm not seeing a change in the api call. (I'm using postman for testing this). am I missing something?

Rerolled patch for Drupal 8.9.4 attached.

drup4lNub - did you implement the hook mentioned in the patch?

you'll need something like the code below - note by doing this you are explicitly overriding core behaviour to provide access control to non node / media entities (core blocks access to all revisions of paragraphs via JSON API):-

/**
 * Allow access to revisions of content.
 *
 * Normally JSON API forbids access to all revisions of entities apart from core
 * entity types Node and Media since these have interfaces for properly
 * determining access.
 *
 * If you know what you are doing and understand the risks you can implement
 * this hook to provide custom logic for other Entity types to allow revision
 * access.
 *
 * The example below allows access to revisions of paragraph entities by
 * checking if the user has access to the parent node revision the paragraph
 * revision is on.
 *
 * @return \Drupal\Core\Access\AccessResultInterface
 */
function hook_jsonapi_revision_view_access(\Drupal\Core\Entity\EntityInterface $entity, \Drupal\Core\Session\AccountInterface $account) {
  /** @var \Drupal\paragraphs\Entity\Paragraph $entity */
  if (!$entity instanceof \Drupal\paragraphs\ParagraphInterface) {
    return AccessResult::neutral();
  }

  $parent = $entity->getParentEntity();

  if ($parent instanceof \Drupal\node\NodeInterface) {
    $field_name = $entity->get('parent_field_name')[0]->getValue();

    $database = \Drupal::database();
    $node_revision = $database
      ->select('node_revision__' . $field_name['value'], 'r')
      ->fields('r', ['revision_id'])
      ->condition('r.' . $field_name['value'] . '_target_id', $entity->id())
      ->condition('r.' . $field_name['value'] . '_target_revision_id', $entity->getRevisionId())
      ->execute()
      ->fetch();

    if ($parent->getRevisionId() !== $node_revision->revision_id) {
      $parent = node_revision_load($node_revision->revision_id);
    }

    /** @var \Drupal\node\Access\NodeRevisionAccessCheck $node_revision_access_service */
    $node_revision_access_service = \Drupal::service('access_check.node.revision');

    return AccessResult::allowedIf($node_revision_access_service
      ->checkAccess($parent, $account, 'view'))
      ->cachePerPermissions()
      ->addCacheableDependency($entity);
  }

  return AccessResult::neutral('Paragraphs revisions are only supported on nodes with JSON:API');
}

Thank you johnennew your patch of comment #13 saved my life

Reapplied patch for Drupal version 9.1.4.
Replaced `\Drupal::moduleHandler()` with Module Handler container - added as a service in `EntityAccessChecker.php` class.
Removed `hook_jsonapi_revision_view_access` from `jsonapi.api.php` file and placed in `jsonapi.module` file - in order to be able to invoke the hook.

Along with #3031271: Support version negotiation for any entity type (currently only Node & Media are supported) I was able to get forward revisions of Node -> Paragraphs through JSON:API with just the IncludeResolver changes. I agree that these still need finessing but this is a good step forward.

I can confirm that the patch from #21 within the changes from #3031271: Support version negotiation for any entity type (currently only Node & Media are supported) issue works for my project.

This would be great to get this working as advertised and really happy it already seems to work! To get a clear view of what we are doing here i'd like to see a few things:

1. An update to the issue summary/title that explains this change and what we are trying to do.
2. A nice set of minimal usecases that we can translate into tests, preferably using core functionality.

The use cases might just be something like:

Structure: We have a article with 2 reference fields (foo and bar) to pages.
State: A published node with a forward revision with 1 foo reference to a published page, and 1 bar reference to a published page with a forward revision.
Expect:
When asking for the forward revision jsonapi should send the forward revision of the article, current revision of the foo reference, and the forward revision of the bar reference.
When asking for the current revision jsonapi should senf the current revision of the article, the foo reference and bar reference.

This way we can describe this change, and someone with testing skills could write some tests :)

Seems to be a similar issue and patch but I don't know which is better. Ideally, we'd close one and continue work on the other.

#3291095: Content moderation + json api includes wrong revision issue

Attached rerolled against branch 9.5x,,
Kindly review patch

Thanks for the reroll, but this issue also needs a few updates as mentioned in #24.

The reroll from @Rakhi Soni was not necessary and no credit will be granted.

Here's how to see if a reroll is necessary:
Find the most recent patch and choose "add test/retest"

In the Add test / retest queue choose the most recent branch and click the "queue" button to run the tests against the current dev branch.

If you don't get an error that the patch didn't apply, then a reroll isn't needed, the patch is able to apply to the current dev branch.

Adding some credit from #3291095: Content moderation + json api includes wrong revision issue. There is.

Below the feedback on the code of the related issue.

1. +++ b/core/modules/jsonapi/src/IncludeResolver.php
   @@ -141,12 +142,23 @@ protected function resolveIncludeTree(array $include_tree, Data $data, Data $inc
   +          if ($field_item instanceof EntityReferenceRevisionsItem) {
   +            $references[$target_type]['revision_id'][] = $field_item->get('target_revision_id')->getValue();
   +          }
   +          else {
   +            $references[$target_type]['id'][] = $field_item->get($field_item::mainPropertyName())->getValue();
   +          }
   

   Compared to this issue i like this code better.

We do still need tests, and we will need a change record too.

Thanks for your suggestions @bbrala
I have attached Patch Against Drupal 9.5.x

@pooja saraah, now I think $field_item_properties variable is unused and can be removed.

+++ b/core/modules/jsonapi/src/IncludeResolver.php
@@ -141,12 +141,22 @@ protected function resolveIncludeTree(array $include_tree, Data $data, Data $inc
+          if ($field_item instanceof EntityReferenceRevisionsItem) {

also, the use statement should be there for EntityReferenceRevisionsItem class.

I had this with paragraph references and fixed it before coming across this issue. While the core idea of the fix is the same, the recent patch is missing a 'use' statement and has an unused variable. So I am attaching my patch anyway for review. Thanks.

@varshith thanks for the patch, please provide the interdiff as well.

Adding interdiff

#33 Re-roll against 10.1.x-dev branch. Patch applied successfully, but custom command failing. required NW

I just had a look at #33 and I'm not sure why we are using

+use Drupal\entity_reference_revisions\Plugin\Field\FieldType\EntityReferenceRevisionsItem;

Started to re-roll for 11.x but the changes to \Drupal\jsonapi\IncludeResolver::resolveIncludeTree from #3057545: ResourceTypeRepository wrongly assumes that all entity reference fields have the setting "target_type" have complicated this.

Re-rolled the patch against 10.2.x, still testing needs to be done on this re-rolled patch.

We can't reference classes from entity_reference_revisions in core, this needs a reroll starting from #21

Thanks @acbramley
I have re-rolled it from #21 against 10.2.x

I don't think the re-roll is that simple. Now we're only adding the revision_ids and ids bit to $references if the is_subclass_of condition fails.

Opened MR against 11.x which is a re-roll of #21, but moves bits out of the is_subclass_of condition. This also applies to 10.2.x.

As mentioned in #45, the patch for v10.2.x in #44 doesn't cover both conditions in the if/else. Using that patch, I wasn't seeing the results I expected, so I made this patch that covers both conditions, and now I see the results I expected.

Hiding patches to avoid bot confusion.

Patch #48 works for me, but

list($target_type, $revision_type) = explode(':', $target_type_and_rev);

assumes that the $target_type_and_rev will always have 2 values. This is not the case, so you end up with tons of warnings>
I've changed it to

list($target_type, $revision_type) = array_pad(explode(':', $target_type_and_rev), 2, NULL);

and no more warnings.

Attaching the revised patch.

The line:

list($target_type, $revision_type) = array_pad(explode(':', $target_type_and_rev), 2, NULL);

throws a warning, I added '@' to suppress it since $revision_type is expected sometimes to be NULL.

Applied on Core 10.2.X

I rebased the MR and pushed a new commit with a different approach where I'm using the entity from the field item instead of loading it again from entity storage. Not sure if it has some downsides but it seems to work fine when testing manually.

This approach seems great! Thanks for moving this forward.

We do still need test. Probably best to look at JsonApiRegressionTest and then testDeepNestedIncludeMultiTargetEntityTypeFieldFromIssue2973681 which also creates some nested relations and tests those. It would just need to install content_moderation and make a forward revision.

Awesome, thanks!

Sorry for not addressing the needed tests. I'm not sure if test is needed since we're not changing any of core's behaviour, we're not expecting core's entity reference field to load a future draft(?) So it should already be covered by existing tests? My understanding here is that this allows modules like paragraphs to have a say in what revisions are loaded, but core should still works the same.

I could be wrong and I'd be happy to try and write a test for this. But I'd need some guidance on how to approach it. 🙇

DIsecting the change, you might actually be right. Instead of collecting ID's and then getting the entities we just reference the entity and create the list right away, doing away with the load multiple.

Technically its the same, there is also no risk involved with not load multiple imo since we know we have the entiy already. I cannot see how there could ever be a way to have different entities loaded with loadMultiple than we already know of in the (validated for existing) entity property.

This does assume the entity that is loaded is the correct revision, but that seems like ano brainer.

I want to RTBC, but:

1. IS needs update for new approach.
2. Need a change record describing the change

I've updated the proposed resolution. Assigning the issue to myself to draft a change record.

I've drafted a change record now. I'm no technical writer and haven't authored one before but hope it helps some what to drive this issue forward. 🙂

Thanks! I've updated the CR a little.

I prefer to test this, ive put some time into this to see if i could test it with a kernel test. Seems i can use the resolver properly, just need to do the second level of node and see if i can reproduce this. I keep getting an itch this needs testing.

Very rough work in progress.

class JsonApiDocumentTopLevelNormalizerTest extends JsonapiKernelTestBase {

  use ImageFieldCreationTrait;
  use ContentModerationTestTrait;
  use UserCreationTrait;

  public function testRelationshipVersionIncludes(){

    $this->assertTrue($this->container->get('module_installer')->install(['content_moderation'], TRUE), 'Installed modules.');


    $workflow = $this->createEditorialWorkflow();
    $this->addEntityTypeAndBundleToWorkflow($workflow, 'node', 'article');
    $this->type->getEntityType()->setHandlerClass('moderation', ModerationHandler::class);


    Role::create([
      'id' => RoleInterface::AUTHENTICATED_ID,
      'permissions' => [
        'access content',
        'view any unpublished content',
        'use editorial transition publish',
      ],
      'label' => 'Anonymous',
    ])->save();

    $this->setCurrentUser($this->user);

    $this->includeResolver = $this->container->get('jsonapi.include_resolver');
    $this->resourceTypeRepository = $this->container->get('jsonapi.resource_type.repository');

    $this->createEntityReferenceField(
      'node',
      'article',
      'field_entity_reference',
      'Related entity',
      'node',
      'default',
      ['target_bundles' => []],
      1
    );

    $node = Node::create([
      'title' => 'original node',
      'type' => 'article',
      'uid' => $this->user,
      'body' => [
        'format' => 'plain_text',
        'value' => $this->randomString(),
      ],
      'field_tags' => [
        ['target_id' => $this->term1->id()],
        ['target_id' => $this->term2->id()],
      ],
      'field_image' => [
        [
          'target_id' => $this->file->id(),
          'alt' => 'test alt',
          'title' => 'test title',
          'width' => 10,
          'height' => 11,
        ],
      ],
    ]);

    $nodeWithReference = Node::create([
      'title' => 'original node with reference',
      'type' => 'article',
      'uid' => $this->user,
      'body' => [
        'format' => 'plain_text',
        'value' => $this->randomString(),
      ],
      'field_tags' => [
        ['target_id' => $this->term1->id()],
        ['target_id' => $this->term2->id()],
      ],
      'field_image' => [
        [
          'target_id' => $this->file->id(),
          'alt' => 'test alt',
          'title' => 'test title',
          'width' => 10,
          'height' => 11,
        ],
      ],
      'field_entity_reference' => [
        ['target_id' => $node->id(), 'entity' => $node],
      ],
    ]);
    $nodeWithReference->save();

    $node->set('title', 'updated node');
    $node->setNewRevision(TRUE);
    $node->set('moderation_state', 'draft');
    $node->save();

    $nodeWithReference->set('title', 'updated node with reference');
    $nodeWithReference->setNewRevision();
    $nodeWithReference->set('moderation_state', 'draft');
    $nodeWithReference->save();

    $parentVid = $nodeWithReference->get('vid')->value;
    $childVid = $node->get('vid')->value;

    $resource_type = $this->container->get('jsonapi.resource_type.repository')->get('node', 'article');
    $resource_object = ResourceObject::createFromEntity($resource_type, $nodeWithReference);
    $includes = $this->includeResolver->resolve($resource_object, 'field_entity_reference');

    $jsonapi_doc_object = $this
      ->getNormalizer()
      ->normalize(
        new JsonApiDocumentTopLevel(new ResourceObjectData([$resource_object], 1), $includes, new LinkCollection([])),
        'api_json',
        [
          'resource_type' => $resource_type,
          'account' => NULL,
          'include' => [
            'field_entity_reference',
          ],
        ]
      );
    $normalized = $jsonapi_doc_object->getNormalization();



    // Todo:

    // Node 1  (static) -> reference node 2 -> reference node 3
    // Update node 3 to have a working copy.
    // Ask for working copy. Might need

  }
}

I worked on this during DrupalCon Singapore. I started work on a having mock entity reference revisions field type since entity reference revisions module is in contrib in git merge request. My test is just a mock placeholder. I am finished for today.

Hi @jkdaza
I am using 3088239-50.patch in Drupal 10.4.8 and moderation enabled.
Scenerio:
1. We have a content type.
2. In that we have a paragraph field.
3. Inside paragraph we have content reference field.

So when we pass include=content_blocks.contents&resourceVersion=rel%3Aworking-copy then we get correct version copy for paragraph. However we are not getting the current working copy of referenced content if that referenced content latest state is in draft mode(This was published earlier)
* content_blocks is paragraph field machine name.