#19 Fix: using patch to update entity in verifier

[skinnyworm]

Summary

Using service.patch to update existing entity.

Closes #19

[marshallswain]

These changes look good to me. Thank you!

[ekryski]

See my comment here: https://github.com/feathersjs/feathers-authentication-oauth2/issues/19#issuecomment-297234695

This is pretty hard to test but I think before we give this a 🚢 we need to ensure that deep object changes on the entity are still respected. If @skinnyworm can add a test for that, that would be great. Other than that this looks good!

[daffl]

True, patch generally does not do a deep merge. I don't remember at the moment what the verifier actually stores. Is ist just the id or the whole object (which usually won't work for SQL dbs anyway right?).

[ekryski]

@daffl the verifier merges in the object and you can register a hook to customize the user object as you need to. It's unopinionated. We just take what the user in the DB has, merge the raw response from the OAuth provider and let you clean it up in your own hook.

So in that case patch probably won't work. That's why I had used update, because we can't possibly know what keys the developer wants to store on their user object but we definitely want to make sure we don't clobber keys.

On that note, I'm wondering if we should always return the existing user and the new data, not merge them, and let the developer do whatever they want in their own hook.

@skinnyworm any chance you can add a test for deep objects?

[xixilive]

I think, in the most of cases, we need patch.

[ekryski]

TL;DR. After discussion, LGTM. Thanks @skinnyworm for the reporting the issue and the fix. Sorry for the delay! :shipit:

This is a breaking change so we'll be releasing this as v0.3.0.

---

Copying the additional discussion around this in Slack for visibility.