Fixed overwrite option: was always true

[leo-nard]

The this.overwrite option is always true in its current form.

[marshallswain]

Thanks for the input. The overwrite:true is actually there by default to conform to the REST spec, which is that on update the object gets replaced. Setting it otherwise would move away from being RESTful, so I think it might be better to leave it so that users need to opt in to overwrite:false.

[marshallswain]

Sending a PATCH request is a great way to get this same behavior.

[ekryski]

Yes @marshallswain is correct. If you don't want that behaviour for your service then you just initialize the service with { overwrite: false }.

Thanks for taking the time to make a PR but this is something that we won't change. 😄

[leo-nard]

Agree with above. Though the current code causes this.overwrite always to evaluate to true. See below, if options.overwrite is false it will take the right part of the || operator. The PR is just a fix, the overwrite will still be opt-in.
this.overwrite = options.overwrite || true;

[ekryski]

Wow. I fail at developing. You are right. Sorry it's been a hectic day.

[ekryski]

We will need to make sure the tests pass before we can merge this. I'll have to take a proper look this evening.

[leo-nard]

No problem. Does the failing of the test has something to do with my PR? Seems harmless :).

[ekryski]

Yup syntax error. It should be: (options.overwrite === false) ? false : true;

[leo-nard]

Sorry. Next time will test this locally :).

[daffl]

I'll take this one and make a new release that includes #70

[ekryski]

Thanks @leo-nard! 😄