Skip to content

Conversation

jplhomer
Copy link
Contributor

@jplhomer jplhomer commented Jan 9, 2018

We're seeing an issue where a multi-word relationship key (in Rails) is receiving a write payload with a camelized key. The write fails in this case:

{
  data: {
    id: "1",
    relationships: {
      specialBooks: {
        data: {} // etc
      }
    }
}

This PR adds a test for this behavior, and introduces the decamelize string helper.

Definitely open to more elegant ways to do this without decamelizing a string!

});

// todo test on the way back - id set, attrs updated, isPersisted
// todo remove #destroy? and just save when markwithpersisted? combo? for ombined payload
// todo test unique includes/circular relationshio
it('sends the correct payload', function(done) {
instance.save({ with: { books: 'genre' } }).then((response) => {
instance.save({ with: { books: 'genre', specialBooks: true } }).then((response) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Super minor, but could we make this specialBooks: {}? It better models what we actually expect (and will enforce in TypeScript soon), the true just happens to work here and these tests can be useful as documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh yes - was honestly just a guess on my part 😄 updated!

@richmolj
Copy link
Contributor

@jplhomer one minor comment but this is fantastic, thank you 👏 ! I find it unbelievable I have not come across this before, perhaps one of the minor bumps along the way introduced this. At least now we have a spec in the right place. Very much appreciated! ❤️

(I know @wadetandy has had better luck importing lodash functions in TypeScript for things like decamelize, but prior to merging the 1.x branch they are a bit of a pain in the ass, so I think the explicit function is fine for now)

@richmolj richmolj merged commit 2a4c326 into jsonapi-suite:master Jan 10, 2018
@richmolj
Copy link
Contributor

Released in 0.5.5

wadetandy added a commit to wadetandy/jsorm that referenced this pull request Jan 10, 2018
Pulls in the test and re-implements functionality for jsonapi-suite#40.

Also removes a bunch of superfluous string methods. We had snakeCase
coming from lodash and an internal camelize function. Since we're using
the inflected library for jsonapiType manipulation, we can rely on its
underscore and camelize functions.
@wadetandy wadetandy mentioned this pull request Jan 10, 2018
richmolj pushed a commit that referenced this pull request Jan 10, 2018
Implement #40

Pulls in the test and re-implements functionality for #40.

Also removes a bunch of superfluous string methods. We had snakeCase
coming from lodash and an internal camelize function. Since we're using
the inflected library for jsonapiType manipulation, we can rely on its
underscore and camelize functions.
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.

2 participants