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

Fix for included collection with numeric keys #14

Merged
merged 5 commits into from
Jan 26, 2023

Conversation

snellingio
Copy link
Contributor

When using the Resource Collection, if you have a relationship that is a HasOne or BelongsTo, and the keys are numeric, the flattened include map has keys associated with it.

This only happens with numeric keys, and does not happen when the keys are strings (as in all test cases)

Example: User -> HasOne? -> License

Without values() called, you'll end up with a keyed included response:

  "included": {
    "0": {
      "id": "1",
      "type": "basicModels",
      "attributes": {
        "url": "https://example.com/avatar1.png"
      },
      "relationships": {},
      "meta": {},
      "links": {}
    },
    "2": {
      "id": "2",
      "type": "basicModels",
      "attributes": {
        "url": "https://example.com/avatar2.png"
      },
      "relationships": {},
      "meta": {},
      "links": {}
    }
  },

This can be solved by getting only the values() from the collection.

You'll see that this does not occur when making a single resource, because values is already called on the included method:

https://github.com/timacdonald/json-api/blob/main/src/JsonApiResource.php#L170

@timacdonald
Copy link
Owner

Thanks for the PR Sam. I'll be digging back into this package development next week, so I'll review early next week and get this merged for ya. Thank you so much for sending through a test ❤️

@snellingio
Copy link
Contributor Author

No problem.

I was wrong in my ending sentence, and just discovered the same problem on the JsonAPIResource,
https://github.com/timacdonald/json-api/blob/main/src/JsonApiResource.php#L170

I'll update and write another test soon

@snellingio
Copy link
Contributor Author

Fixed, although I believe this one has to do with a circular reference. idk, but it's fixed and tested

@jhonaikerf
Copy link

@timacdonald any update?

@roerjo
Copy link

roerjo commented Oct 27, 2022

Running into this same issue in our project. Re-keying the array index with the Collection's values() method resolves our issues. Looking forward to seeing this merged in.

@timacdonald timacdonald merged commit c42f42a into timacdonald:main Jan 26, 2023
@timacdonald
Copy link
Owner

Thanks @snellingio and sorry for the delay. This will be tagged in the upcoming v1 release. There is beta already tagged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants