Allow 'false' payload to get normalized

[meirish]

I'm coding against an API that returns true for successful update and false for an unsuccessful one, both with a status code of 200.

When I was attempting to customize what happened in the uncessful path, I found that my normalization code wasn't being run at all. I tracked it down to this _commit method in system/store.js file. false is a valid JSON response, so I've updated the guard to use typeof adapterPayload !== 'undefined'.

I'm not sure the test I wrote is in the correct place or if it will even work (the directions on the homepage didn't work for me for running the tests), any pointers / comments about that would be appreciated.

Thanks!

[meirish]

Yeah looks like the interop may not be the best place to test this. I'll dig a bit more later.

[fivetanley]

The code / objective overall looks good to me, and at a glance seems backwards compatible. Ping me tomorrow and I can help you find a better file / way to test.

[bmac]

@meirish do you mind rebasing this pr. I think the failing test may have been fixed by #4256.

[meirish]

@bmac thanks for the ping, looks like it still fails though

[bmac]

Looks like the error is because the JSONSerializer doesn't know how to normalize a response of false. Changing this line to return { data: [] }; should fix the test.

[meirish]

😱 tests are green! Thank for the pointers @bmac! I'll have to figure out why I can't get things installed on my machine.

Initially I thought your suggestion meant that JSONSerializer should know how to handle a false payload as well, but I think that's far enough outside of json-api expectations that implementing your own normalizeResponse is expected. Thanks again for the nudge and help on this!

[pangratz]

So I took a look and I have a question about your initial statement @meirish:

I'm coding against an API that returns true for successful update and false for an unsuccessful one, both with a status code of 200.

When I was attempting to customize what happened in the uncessful path, I found that my normalization code wasn't being run at all.

I am interested in why you want to hit the normalization even though the response is not a success. I think overwriting isSuccess and returning false might be a better option for you in this case. Have you considered that?

---

That being said, I think the change proposed in this PR is 👍. It would be even ok to remove the check as a whole, since this should be a serializer concern. But this might be a breaking change, so that's not an option for now.

@meirish can you update the if statement and also check for adapterPayload not being null.

[pangratz]

@meirish can you update the if statement and also check for adapterPayload not being null.

Or we could use !Ember.isNone(adapterPayload) as well...

[bmac]

Looks like this pr has gone stale. I'm going to close it but feel free to reopen the pr if you get a chance to address @pangratz's comments.