Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
657 stars 92 forks source link

[server] Does MyCollection.direct.update call MyCollection.update? #73

Closed colllin closed 9 years ago

colllin commented 9 years ago

Or does it skip straight to the Mongo driver's update function?

When I try to update a document by _id, using:

Things.direct.update('254at4wta43t34', modifier, options);

I get the server-side error:

Exception while invoking method '/things/update' Error: selector must be a valid JavaScript object

which to me looks like a Mongo driver error. I expected it to call the Meteor Collection.update function directly, which would allow me to pass the ID instead of a selector object.

matb33 commented 9 years ago

Ah interesting point, you're right, and it's because of: https://github.com/matb33/meteor-collection-hooks/blob/master/collection-hooks.js#L50

I suspect to make this work properly would involve making sure that _super at https://github.com/matb33/meteor-collection-hooks/blob/master/collection-hooks.js#L95 is always referencing the original mutator method and not _collection's

colllin commented 9 years ago

That looks like it! I meant to say Things.direct.update in my original example. I've updated it now, but it seems like you knew what I meant.

Looks like you went out of your way to make the server extend the underlying _collection though. Do you remember why?

matb33 commented 9 years ago

Nope... don't remember, but it was for a good reason. I looked briefly and couldn't recall...

colllin commented 9 years ago

Dug a little more, and it looks like _collection is from your original attempt. Then later you moved to the Meteor.isClient ? self : self._collection logic.

Commit where you changed to that logic: https://github.com/matb33/meteor-collection-hooks/commit/f3b9deb5fea9e66d91085d648032763fe89510d7#diff-25a92cefdc8e3dd82dc7e3b4a90472bbL46

Original commit with the _collection logic: https://github.com/matb33/meteor-collection-hooks/commit/8e5a8f15c8406c6ca45e7db01be893fb4186aff6#diff-25a92cefdc8e3dd82dc7e3b4a90472bbR49

Oh well, at least you can get around it by passing {_id: <id>}

matb33 commented 9 years ago

I believe that logic is important for firing server-defined hooks after allow/deny has been processed.

colllin commented 9 years ago

Great improvement. Thanks for this!