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

issue when saving a null value #31

Closed
jamesdixon opened this issue Sep 16, 2016 · 5 comments
Closed

issue when saving a null value #31

jamesdixon opened this issue Sep 16, 2016 · 5 comments

Comments

@jamesdixon
Copy link

Hello again!

I ran into an issue with the 1.2.1 release. I specifically noticed it when I was trying to save a payload that contained a property that had a value of null. For example, if I was to call model.save(data) where data contains something like:

{
  preferences: null
}

Before saving, I have a method that validates the model, which is tapping into the save hook provided by Bookshelf. When inspecting the model that's passed into that function, I can see that preferences is now a string:

{
  preferences: "null"
}

I'm assuming this is because it's stringified beforehand, but ultimately, this is causing problems on my end.

If I remove preferences as a JSON column, it works as expected. Reverting to v1.1.1 also resolves the issue.

Thanks!

@jamesdixon
Copy link
Author

A couple of additional data points:

  1. This only occurs when { patch: true}
  2. If you run the following test and then look at the value of the foo column in the test table, you'll see that it contains a "null" value.
    it.only('null or undefined values should not be stringified', async () => {
      const model = await Model.forge().save();

      await model.save({ foo: null }, { patch: true });

      should(model.get('foo')).be.null();
    });

Regarding the test, this test actually passes. It appears that either Bookshelf, Knex or the PG driver coerces the null string into an actual null value.

@jamesdixon
Copy link
Author

I believe I have a fix for this: scoutforpets@d91e9f5

Let me know what you think.

The issue now is I'm unsure how to test since something is coercing the null string mentioned above into an actual null value. Any ideas?

@ricardogama
Copy link
Collaborator

Sorry for the delay @jamesdixon, already have a fix for this. Thanks again for contributing!

@jamesdixon
Copy link
Author

Awesome. Thanks!

@ricardogama
Copy link
Collaborator

@jamesdixon Released as 1.2.2, cheers!

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

No branches or pull requests

2 participants