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

publish interface doesn't clone its arguments #1750

Closed
liorst opened this Issue Jan 11, 2014 · 6 comments

Comments

Projects
None yet
3 participants
@liorst

liorst commented Jan 11, 2014

I've tried to replicate the example of the counts-by-room from the meteor docs, but instead of

added: 
    .
    .
    .
    self.changed("collection", uuid, { count: count})

I used

 added:
    vol : {white: 5, green: 0, red: 3}
    self.changed("collection", uuid, {count : vol})

And I've initialized the collection with the same object, as required before calling self.ready().
The collection is indeed initialized properly on the client side, but when the "added" event is called on the published collection, the code fails to update the "counts" collection.
I've beaten around the bush for a couple of hours, before calling this.changed twice - to finally get the "collection" to be updated as expected:

self,changed("collection", uuid, {count : 0})
self,changed("collection", uuid, {count : vol})

I've tried using $set to update the value of a specific color, but it simply created a key called "$set" in the collection, and assigns the vol object to it.

@glasser

This comment has been minimized.

Member

glasser commented Jan 15, 2014

This seems like a bug but you haven't provided enough details to investigate it. Please see https://github.com/meteor/meteor/wiki/Contributing-to-Meteor#reporting-a-bug-in-meteor I'll reopen the issue if you provide a reproduction.

@glasser glasser closed this Jan 15, 2014

@liorst

This comment has been minimized.

liorst commented Jan 19, 2014

@awatson1978

This comment has been minimized.

Contributor

awatson1978 commented Jan 19, 2014

Hi Lior,
So, the issue is probably that you've put an .observeChanges() from within a .publish() functions. If you check out the official docs, you'll notice that it's suppose to be called after a .find() function, usually within a handlebars template helper.

http://docs.meteor.com/#observe_changes

If you're looking for a pattern for incrementing some basic analytics, and keeping a count of how many books are in your collection, take a look at line 52 from the topicsPage controller and line 94 from the postsPage controller in GroupThink BBS.

https://github.com/awatson1978/groupthink/blob/master/client/controllers/workflow/topicsPage.js
https://github.com/awatson1978/groupthink/blob/master/client/controllers/workflow/postPage.js

Instead of .observeChanges, you probably want to be using $inc operators, like so:

// first example
Topics.update(Session.get('forum_topic_id'), {$inc: {replies: 1 }});

// second example
Topics.update(this._id, {$inc: { views: 1 }, $set: { lastPostId: Meteor.userId(), lastPostBy: Meteor.user().profile.name }});

You touched on the right pattern by thinking you'd need a Books-count publication. But you probably will want to follow through the pattern by creating a BooksStats collection, so you can do things like this:

BookStats.update(Session.get('selected_book_id'), {$inc: {views: 1 }});
@liorst

This comment has been minimized.

liorst commented Jan 19, 2014

Hey Abigail,
I appreciate your comment but I wasn't originally looking to just increment
a value, I've supplied this as an example of the bug, as per meteor's bug
report instructions.
If you'd look at the this link:
http://stackoverflow.com/questions/10565654/how-does-the-messages-count-example-in-meteor-docs-work

You'd see that calling observeChanges within a publish() function is what
was recommended by Meteor, and it does serve my purpose of creating a
client side collection which depends on the contents of another
server/client side collection.
Now on the other hand, the bug I've reported, is not the only one existing
in the "this.changed" call, if you do try to increment using $inc, inside a "this.changed" function, you would find that you've created a key in your doc, named "$inc", not quite what you expect.

On Sun, Jan 19, 2014 at 5:09 PM, Abigail Watson notifications@github.comwrote:

Hi Lior,
So, the issue is probably that you've put an .observeChanges() from within
a .publish() functions. If you check out the official docs, you'll notice
that it's suppose to be called after a .find() function, usually within a
handlebars template helper.

http://docs.meteor.com/#observe_changes

If you're looking for a pattern for incrementing some basic analytics, and
keeping a count of how many books are in your collection, take a look at
line 52 from the topicsPage controller and line 94 from the postsPage
controller in GroupThink BBS.

https://github.com/awatson1978/groupthink/blob/master/client/controllers/workflow/topicsPage.js

https://github.com/awatson1978/groupthink/blob/master/client/controllers/workflow/postPage.js

Instead of .observeChanges, you probably want to be using $inc operators,
like so:

// first exampleTopics.update(Session.get('forum_topic_id'), {$inc: {replies: 1 }});
// second exampleTopics.update(this._id, {$inc: { views: 1 }, $set: { lastPostId: Meteor.userId(), lastPostBy: Meteor.user().profile.name }});

You touched on the right pattern by thinking you'd need a Books-countpublication. But you probably will want to follow through the pattern by
creating a BooksStats collection, so you can do things like this:

BookStats.update(Session.get('selected_book_id'), {$inc: {views: 1 }});


Reply to this email directly or view it on GitHubhttps://github.com//issues/1750#issuecomment-32710312
.

@glasser glasser reopened this Feb 17, 2014

@glasser

This comment has been minimized.

Member

glasser commented Feb 17, 2014

Thanks, verified.

@glasser glasser closed this in 764b41a Feb 17, 2014

glasser added a commit that referenced this issue Feb 17, 2014

@glasser

This comment has been minimized.

Member

glasser commented Feb 17, 2014

Fix will be in 0.7.1. Thanks for the reproduction; it made the issue very clear.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment