Skip to content

Handling cases where jsonapi.version isn't set - #378

Open
krainboltgreene wants to merge 3 commits into
mike-north:masterfrom
krainboltgreene:patch-1
Open

Handling cases where jsonapi.version isn't set#378
krainboltgreene wants to merge 3 commits into
mike-north:masterfrom
krainboltgreene:patch-1

Conversation

@krainboltgreene

Copy link
Copy Markdown

#291 is a problem for anyone using the ruby library jsonapi-resources, as it both doesn't support returning /jsonapi/version and it is very annoying to hack in.

I prefer to trust but warn, but failing that I've at least checked for a data attribute.

@ghost

ghost commented Apr 30, 2019

Copy link
Copy Markdown

There were the following issues with this Pull Request

  • Commit: 1069ff1
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

}
function isDocWithData(doc: any): doc is DocWithData {
return isJsonApi(doc) && ['object', 'array'].indexOf(typeOf((doc as DocWithData).data)) >= 0;
function isValidResponse(doc: any): doc is DocWithData {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Maybe I don't understand typescript, but why isn't the return type boolean?

@krainboltgreene

Copy link
Copy Markdown
Author

What is commitlint talking about here?

@ghost

ghost commented Apr 30, 2019

Copy link
Copy Markdown

There were the following issues with this Pull Request

  • Commit: 1069ff1
    • ✖ message may not be empty
    • ✖ type may not be empty
  • Commit: d1c85c9
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@ghost

ghost commented Apr 30, 2019

Copy link
Copy Markdown

There were the following issues with this Pull Request

  • Commit: 1069ff1
    • ✖ message may not be empty
    • ✖ type may not be empty
  • Commit: d1c85c9
    • ✖ message may not be empty
    • ✖ type may not be empty
  • Commit: e44f9e8
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

console.debug("Received a repsonse that doesn't have a /jsonapi/version property")

// Don't even bother if the data propery doesn't exist
return typeof response.data !== 'undefined';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Shouldn't this be doc.data?

@jagthedrummer

Copy link
Copy Markdown

Anything we can do to get this merged? I just ran into this and am trying to work around it like so, but with no luck.

    after(response){
      response.jsonapi = { "version": "1.0" };
      serializeAndPush(this, response);
    }

I'm still getting the console message:

serializeAndPush may only be used with a JSON API document. Ignoring response. Document must have a mandatory JSON API object. See https://jsonapi.org/format/#document-jsonapi-object.

@dbollinger

dbollinger commented Apr 6, 2020

Copy link
Copy Markdown

@jagthedrummer I was running into friction here as well (literally yesterday) -- in the short term, so that I could start using this add-on, I just created my own (very brittle) method:

export default function serializeAndPushRecord(payload) {
  let serializer = this.store.serializerFor(payload.data.type);
  serializer.pushPayload(this.store, payload);
}

YMMV, and I only need this for single members (not collections) right now, but if you're looking for a quick answer while this addon goes through some modernization and maintenance, something like this could be an option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants