Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: data.meta ignored in relationships #89

Merged
merged 4 commits into from
Jan 18, 2021

Conversation

BurningDog
Copy link
Contributor

Detailed description

A JSON:API resource linkage in a relationship object may also include a meta member, but json-api-client doesn't check for this, which is a bug.

Specifically, I'm integrating with a Drupal 9.1.0 JSON:API in which I've enabled the Drupal consumer_image_styles module in order to add image style information to an image field in a content entity. The relevant part in the relationships API response looks like this:

"field_header_image": {
  "data": {
    "type": "file--file",
    "id": "bb7912a9-81ed-4c58-8e11-37d4d619e8a5",
    "meta": {
      "alt": "",
      "title": "",
      "width": 1920,
      "height": 1280,
      "imageDerivatives": {
        "links": {
          "header": {
            "href": "https://example.com/sites/default/files/styles/header/public/header/2021-01/about-us-header.jpeg?h=e5aec6c8&itok=qXnhaDF4",
            "meta": {
              "rel": [
                "drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/#derivative"
              ]
            }
          },
          "header_600x431_": {
            "href": "https://example.com/sites/default/files/styles/header_600x431_/public/header/2021-01/about-us-header.jpeg?h=e5aec6c8&itok=qEH6iDvL",
            "meta": {
              "rel": [
                "drupal://jsonapi/extensions/consumer_image_styles/links/relation-types/#derivative"
              ]
            }
          }
        }
      }
    }
  }

Note that meta is a property of data.

The place in the code which should check for this is in the ItemParser's parseRelationshipData().

Instead, the return should be changed to:

        $item = $this->getItemInstance($data->type)->setId($data->id);
        if (property_exists($data, 'meta')) {
            $item->setMeta($this->metaParser->parse($data->meta));
        }
        return $item;

Context

This helps me integrate with a Drupal JSON:API so that I can get various resized images.

My environment

  • PHP 7.4
  • Mac OS 10.14.6 running Valet

src/Item.php Outdated Show resolved Hide resolved
static::assertInstanceOf(Meta::class, $dataMeta);

$image = $dataMeta->imageDerivatives->links->header->href;
$this->assertEquals('https://example.com/image/header/about-us.jpeg', $image);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so we can check we're getting specific information back, and not accidentally returning the other Meta object.

@JaZo JaZo force-pushed the feature/relationships-data-meta branch from 23b57b5 to 8b486ac Compare January 18, 2021 09:25
@JaZo JaZo self-assigned this Jan 18, 2021
@JaZo JaZo self-requested a review January 18, 2021 09:28
Copy link
Member

@JaZo JaZo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, you are absolutely right! I've also added the meta to plural relations in Item::getRelationships and updated the test.

@JaZo JaZo merged commit d0e7564 into swisnl:1.x Jan 18, 2021
@JaZo
Copy link
Member

JaZo commented Jan 18, 2021

Released in 1.3.3!

@BurningDog
Copy link
Contributor Author

Thanks for the quick turn-around!

@BurningDog BurningDog deleted the feature/relationships-data-meta branch January 19, 2021 12:17
@JaZo JaZo added the bug Something isn't working label Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants