Problem/Motivation

When requesting a list of entities with an included relation where only the 'view label' operation is allowed, you'll could get an error saying that you try to use a route that does not exist if the public field name is different then the internal field name. This happens because the route name is generated using the internal field name instead of the public field name.

Proposed resolution

Make sure the EntityAccessDeniedHttpException is created with the public field name as $relationship_field parameter instead of the internal field name.

Remaining tasks

None? Maybe add some tests.

User interface changes

None.

API changes

None.

Data model changes

None.

Release notes snippet

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jmeijer created an issue. See original summary.

jmeijer’s picture

jmeijer’s picture

Status: Active » Needs review

Version: 9.0.x-dev » 9.1.x-dev

Drupal 9.0.10 was released on December 3, 2020 and is the final full bugfix release for the Drupal 9.0.x series. Drupal 9.0.x will not receive any further development aside from security fixes. Sites should update to Drupal 9.1.0 to continue receiving regular bugfixes.

Drupal-9-only bug reports should be targeted for the 9.1.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.2.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Kristen Pol’s picture

Version: 9.1.x-dev » 9.2.x-dev
Status: Needs review » Needs work
Issue tags: +Bug Smash Initiative, +Needs reroll

Thanks for the issue and patch. If this is still relevant, it needs a reroll for 9.2.

ankithashetty’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.72 KB
1.68 KB

Rerolled the patch in #2, thanks!

siddhant.bhosale’s picture

Assigned: Unassigned » siddhant.bhosale
siddhant.bhosale’s picture

Assigned: siddhant.bhosale » Unassigned
Status: Needs review » Reviewed & tested by the community

The patch looks good and applies cleanly on 9.2.x-dev.
Looks good to be merged.

catch’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: -Bug Smash Initiative +Bug Smash Initiative. Needs tests

This needs some test coverage.

quietone’s picture

Issue tags: -Bug Smash Initiative. Needs tests +Bug Smash Initiative, +Needs tests

Updating tags.

jibran’s picture

@mohit_aghera asked for pointers on wrtitng a test for this issue in #bugsmash. Just sharing the relevant information here.

We can write a same test as \Drupal\Tests\jsonapi\Kernel\Normalizer\JsonApiDocumentTopLevelNormalizerTest::testNormalizeException() to test this.

mohit_aghera’s picture

Adding test cases and test-only patch.

The last submitted patch, 12: 3171827-12.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 12: test-only-3171827-12.patch, failed testing. View results

mohit_aghera’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
6.43 KB
938 bytes

Trying to fix the test cases. Updated cardinality.
Even with cardinality 1 it was passing on local somehow. Let's see how it goes.

Kristen Pol’s picture

Title: Route not found exception: Route "jsonapi.[entity].[field_name].related" does not exisits » RouteNotFoundException: Route "jsonapi.[entity].[field_name].related" does not exist
Issue summary: View changes

Status: Needs review » Needs work

The last submitted patch, 15: 3171827-15.patch, failed testing. View results

benjifisher’s picture

Please do not ask the testbot to try again until #3207086: [HEAD BROKEN] Consistent failure in MonthDatePluginTest is fixed.

alexpott’s picture

Status: Needs work » Reviewed & tested by the community
alexpott’s picture

Version: 9.2.x-dev » 9.1.x-dev
Status: Reviewed & tested by the community » Fixed

Committed and pushed d84f620c57 to 9.2.x and 282d000ec5 to 9.1.x. Thanks!

diff --git a/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php b/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
index 63f9df98c2..14e73c44fd 100644
--- a/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
+++ b/core/modules/jsonapi/tests/src/Kernel/Normalizer/JsonApiDocumentTopLevelNormalizerTest.php
@@ -423,8 +423,8 @@ public function testNormalizeException() {
    * field values for Label only resource.
    */
   public function testAliasFieldRouteException() {
-    $this->assertSame($this->resourceTypeRepository->getByTypeName('node--article')->getPublicName('uid'), 'uid');
-    $this->assertSame($this->resourceTypeRepository->getByTypeName('user--user')->getPublicName('roles'), 'roles');
+    $this->assertSame('uid', $this->resourceTypeRepository->getByTypeName('node--article')->getPublicName('uid'));
+    $this->assertSame('roles', $this->resourceTypeRepository->getByTypeName('user--user')->getPublicName('roles'));
     $resource_type_field_aliases = [
       'node--article' => [
         'uid' => 'author',
@@ -435,8 +435,8 @@ public function testAliasFieldRouteException() {
     ];
     \Drupal::state()->set('jsonapi_test_resource_type_builder.resource_type_field_aliases', $resource_type_field_aliases);
     Cache::invalidateTags(['jsonapi_resource_types']);
-    $this->assertSame($this->resourceTypeRepository->getByTypeName('node--article')->getPublicName('uid'), 'author');
-    $this->assertSame($this->resourceTypeRepository->getByTypeName('user--user')->getPublicName('roles'), 'user_roles');
+    $this->assertSame('author', $this->resourceTypeRepository->getByTypeName('node--article')->getPublicName('uid'));
+    $this->assertSame('user_roles', $this->resourceTypeRepository->getByTypeName('user--user')->getPublicName('roles'));
 
     // Create the request to fetch the articles and fetch included user.
     list($request, $resource_type) = $this->generateProphecies('node', 'article');

The signature is public static function assertSame($expected, $actual, string $message = ''): void - for the message to make sense the expected value needs to come first. Fixed on commit.

  • alexpott committed d84f620 on 9.2.x
    Issue #3171827 by mohit_aghera, ankithashetty, jmeijer, jibran:...

  • alexpott committed 282d000 on 9.1.x
    Issue #3171827 by mohit_aghera, ankithashetty, jmeijer, jibran:...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.