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

Patch is deleting subdocument data #144

Closed
imns opened this issue Nov 2, 2016 · 13 comments
Closed

Patch is deleting subdocument data #144

imns opened this issue Nov 2, 2016 · 13 comments

Comments

@imns
Copy link

imns commented Nov 2, 2016

My API uses feathers-mongoose for the db and when I send JSON (via postman) to patch my model I'm losing subdocument (embedded?) data. For example, if I have a model like this

social: {
        facebook: {
            url: String,
            likes: Number
        },
        twitter: {
            username: String,
            followers: Number
        }
}

and I send a patch request with

{
    "social": {
        "twitter": {
            "username": "@the_username"
        }
    }
}

it deletes social.facebook

Expected behavior

It shouldn't delete the sibling subdocuments. In this example social.facebook.

System configuration

Module versions (especially the part that's not working):
[email protected]

NodeJS version:
v6.5.0

@daffl
Copy link
Member

daffl commented Nov 2, 2016

Could this be the Mongoose/MongoDB default behaviour? It's just using Model.update (see https://github.com/feathersjs/feathers-mongoose/blob/master/src/service.js#L217).

@corymsmith
Copy link
Contributor

You would have to use $set to only set that particular property in the subdocument

@corymsmith
Copy link
Contributor

I think you want something like this.

usersService.patch(userId, { $set: { "social.twitter": {username: '@the_username'} } })

@imns
Copy link
Author

imns commented Nov 2, 2016

I agree about using $set. This issue (feathersjs/feathers#323) mentions that $set is being used, but I don't see that anywhere.

@corymsmith I am not using feathers client. I'm sending a patch request directly to the API.

Update

I just tried sending { $set: { "social.twitter": {username: '@the_username'} } } with postman and it worked.

This seems counterintuitive since a patch request by nature needs to use $set.

Update2

$set seems to want dot notation. This works

"$set": { 
        "social.twitter.username": "@test"
    } 

@imns
Copy link
Author

imns commented Nov 2, 2016

Also, I'm not sure about the API of other Database plugins, but doesn't using $set lock you into MongoDB instead of keeping the API DB agnostic?

@corymsmith
Copy link
Contributor

When you send a patch of the following you are effectively saying, set only the social property to whatever I've passed in.

{
    "social": {
        "twitter": {
            "username": "@the_username"
        }
    }
}

The nature of patch is that it patches top level properties, otherwise how would the API know if you wanted to set a new value for social or if you wanted it to merge with the existing values unless you explicitly use $set? The other solution would be to first find the record, grab the social property, add the new twitter key and then issue a patch.

@corymsmith
Copy link
Contributor

I'm not sure if it will work but you could also try a patch with this payload using dot notation:

{
    "social.twitter": {
            "username": "@the_username"
    }
}

@imns
Copy link
Author

imns commented Nov 2, 2016

@corymsmith that second example does work without $set. That's also a good point about the patch behavior.

@corymsmith
Copy link
Contributor

Perfect, I figured it would but didn't test it out before posting that comment :)

@daffl
Copy link
Member

daffl commented Nov 5, 2016

I think this has been figured out right? I think in general Feathers should leave the deep-merge behaviour up to the ORM since some (like all SQL ORMs) don't even support nested objects at all.

@imns
Copy link
Author

imns commented Nov 5, 2016

Ya this is figured out.

@imns imns closed this as completed Nov 5, 2016
@wvezey
Copy link

wvezey commented May 25, 2018

Yes, closed, and yet so helpful. Thank you all.

@FossPrime
Copy link

This is still a problem with AJV... { $set: { "social.twitter": {username: '@the_username'} } } would fail AJV unless you define an object key named "social.twitter", instead of "social" and then "twitter". My workaround is the obvious, overwrite and accept the consequences.

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

5 participants