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

Ignore relationships that don't have data property #778

Merged
merged 5 commits into from
Feb 22, 2023

Conversation

pdw-mb
Copy link
Contributor

@pdw-mb pdw-mb commented Feb 16, 2023

I'm using json-api-merge on a request that only has some relationships included, so I have data that looks like this:

  "relationships": {
    "validation_messages": {
      "links": {
        "related": "[/api/filings/5877/validation_messages](http://localhost:5000/api/filings/5877/validation_messages)"
      }
    },
    "entity": {
      "links": {
        "related": "[/api/entities/213800XB8YZNGJYDEZ97](http://localhost:5000/api/entities/213800XB8YZNGJYDEZ97)"
      },
      "data": {
        "type": "entity",
        "id": "1331"
      }
    }
  }

Note the lack of data property on validation_messages. json-api-merge currently breaks on this input. This PR filters relationships to those which contain the data property.

@char0n char0n self-assigned this Feb 16, 2023
@char0n char0n added the bug Something isn't working label Feb 16, 2023
@char0n char0n self-requested a review February 16, 2023 18:40
@char0n
Copy link
Member

char0n commented Feb 22, 2023

Hi @pdw-mb,

You're right. According to the https://jsonapi.org/format/#document-resource-object-relationships, one of links, meta, data must be contained within relationships. Thus we cannot assume that data is always there.

@char0n
Copy link
Member

char0n commented Feb 22, 2023

Added test for asserting that json-api-merge throws error on the case you described: #788

char0n added a commit that referenced this pull request Feb 22, 2023
src/index.js Outdated Show resolved Hide resolved
@char0n
Copy link
Member

char0n commented Feb 22, 2023

I've fixed linting issue + changed the test to verify your change.

@char0n char0n merged commit 65f0daa into swaggerexpert:master Feb 22, 2023
@char0n
Copy link
Member

char0n commented Feb 22, 2023

Thanks for contributing.

char0n pushed a commit that referenced this pull request Feb 22, 2023
## [1.1.4](v1.1.3...v1.1.4) (2023-02-22)

### Bug Fixes

* ignore relationships that don't have data property ([#778](#778)) ([65f0daa](65f0daa))
@char0n
Copy link
Member

char0n commented Feb 22, 2023

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.

None yet

2 participants