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

How to count comments associated with referenceId? #83

Closed
ritchieng opened this issue Apr 2, 2016 · 6 comments
Closed

How to count comments associated with referenceId? #83

ritchieng opened this issue Apr 2, 2016 · 6 comments

Comments

@ritchieng
Copy link

@boxofrox I've seen your solutions here, it's amazing. Thanks for your help in advance. I'm still struggling to count comments attached to profiles.

I've profiles (same fork as weworkmeteor) where I use comments package (arkham) to attach comments to profiles.

Each comment seems to have a field "referenceId" which contains the profile's id to know which comments is attached to which. I'm trying to count the number of comments for each profile. Very simple case but I'm struggling with undefined errors.

publications.js

Meteor.publish('postCommentCount', function(referenceId) {
    check(referenceId, String);
    Counts.publish(this, 'comments', Comments.getCollection().find({
        referenceId: referenceId
    }));
});

subscriptions.js
Meteor.subscribe('postCommentCount', referenceId);

*profileSmall.html *
{{getPublishedCount 'comments'}}

My repo forked from weworkmeteor shows the changes if you need more information.

@boxofrox
Copy link
Contributor

boxofrox commented Apr 2, 2016

I'm afraid my Meteor-fu is rusty these days.

Your publications.js example references an undefined variable, _id, but I see in your repo that you're using referenceId: referenceId instead. Please confirm that your query selector should not be { _id: referenceId }. I just want to make sure there isn't another typo here.

I cloned your repo, but there are a few things that don't make sense to me here.

  1. How do I navigate to the page in order to view the count/errors? I've created a user. I follow the Developers navbar link which opens /profiles. I see my user with 0 reviews. Am I in the right place?
  2. Where is the Comments collection defined? Under /both/collections/ are files for jobs, profiles, and users, but nothing for comments. Comments in your publications.js example appears to be undefined.
  3. You mention undefined errors, but I see none in either the browser or server consoles. Where do you see your errors? Did you run meteor with options to turn them on, if so what are they?

Also, I recommend reading about unique names and #43 for good measure.

It appears you're trying to publish counts for multiple profiles on the same page. This will be a problem later when the page shows more than one profile. Think of your published count as a global variable that holds one value. Does it hold the review count for the first profile in the list, or the second, third, etc as each profileSmall template is rendered? In other words, you need a separate name for each count you publish. 'comments-' + referenceId is a sufficient naming convention here.

You may want to consider using a meteor call to fetch the counts from the server once. It seems a bit overkill to have the review counts updating in real-time, and may hurt performance (see kadira article link in #76). Here is a simple untested example:

//// server.js
Meteor.methods({
  getReviewCount: function (profileId) {
    return Comments.getCollection().find({ referenceId: profileId }).count();
  }
});

//// client.js
// hmm.... down the rabbit hole.  Not sure where the list of profiles that are
// displayed comes from, so this isn't the optimal way to do it.

Profiles.find({}, { fields: { _id: 1 } }).fetch().forEach(function (profile) {
  // provide a callback to Meteor.call so it doesn't block.  Session will
  // reactively update the page as the counts come in.
  Meteor.call('getReviewCount', profile._id, function (count) {
    Session.set('comments-' + profile._id, count);
  });
});

//// template.js
Template.profiles.helpers({
  reviewCount: function (id) {
    return Session.get('comments-' + id);
  },
});

//// template.html
<span class="label label-primary">
  <i class="fa fa-rocket"></i>
  {{reviewCount profile._id}} Reviews
</span>

The upside is the counts are only computed once for this user, no observers are
created.

The downside is you've dumped a bunch of comment-X values in your
session store, and you've computed counts for all profiles even though
infinit-scroll is only showing a few of them.

You may consider using a null collection on the client to store the counts
in place of Session.

I leave the exercise to you to figure out how to tap into the list of profiles
used by infinite-scroll and fetch the counts on demand.

Cheers

@ritchieng
Copy link
Author

@boxofrox Thanks for your prompt reply:

  1. Yes, you're in the right place. It's zero because by default this package shows 0 if it's unable to retrieve anything.
  2. Comments collection are defined by the comments package, it can be retrieved using Comments.getCollection()
  3. Yes there's no undefined error, my mistake. It's just an issue of the package not working to retrieve the number of comments attached to each profile.

Does this clarify? I've been struggling for so long, really appreciate your help! Thanks.

I tried your code, putting them in the respective areas. The console does not show any errors. However I'm unable to get any counts.

I'm trying to get the "number of comments" to be displayed on "profileSmall" that will be displayed under the route "developers". And I think your solution would work! Just facing some issues implementing your solution now.

alt text

@boxofrox
Copy link
Contributor

boxofrox commented Apr 3, 2016

Okay, I think I found the issue with publish-counts "not working". The problem is subscribing in /client/subscriptions.js. This line is straight from your repo.

//// /client/subscriptions.js
Meteor.subscribe('postCommentCount', "referenceId");

First, you subscribed with the string "referenceId", not with an actual profile id. If I hardcode a string with my test user's id, then I get 1 Reviews--per the single comment I created--in the profileSmall template as you intended.

Second, you kind of subscribed too early. At this point you don't have a list of profiles to load counts against.

Here's a very hackish alteration to /client/subscriptions.js that demonstrates publish-counts can and does work, but I don't recommend going this route. I don't think it'll scale well.

//// /client/subscriptions.js
Meteor.subscribe("userData");
Meteor.subscribe("jobCount");
Meteor.subscribe("developerCount");

Meteor.subscribe('profiles', 0, /* callback when subscribe is ready */ function () {
  // observe profiles and load review counts.
  Profiles.find({}).observe({
    added: function (doc) {
      console.log('profile', doc);
      Meteor.subscribe('postCommentCount', doc._id);
    },
    changed: function (oldDoc, newDoc) {
      Meteor.subscribe('postCommentCount', newDoc._id);
    },
  });
});

I'll see if I can knock out a tested solution using my previous recommendation.

@ritchieng
Copy link
Author

Wow. I get what you mean. Your hackish solution works in the sense that there is a comment count now.

But weirdly, the comment counts a single profile only. So I've a case where:

  1. John: 1 review
  2. Ritchie: 2 reviews

But both are displaying 2 reviews. This is one step in the right direction as I did not manage to even retrieve the counts previously. Thank you so much.

@boxofrox
Copy link
Contributor

boxofrox commented Apr 3, 2016

That wierd part is related to the lack of unique names I mentioned previously.

Here's an alternative solution. Apply it to your repo with

cd repo
curl -L https://gist.github.com/boxofrox/9cf351660c29b8e94265e172d425ec6c/raw/392c37ee5d55c88dc1b110280b0fb27ba0df67b6/publish-counts-issue-83.diff | git apply

It...

  1. ...creates one in-memory-only collection for ReviewCounts, {profileId, count} for every profile.
  2. ...reactively adds ReviewCount docs when new profiles are added. I do not expect the Profile observer to scale well with a large number of profiles. Any performance hit should be confined to initial startup of the server when observe.added processes every profile (see footnote [1]).
  3. ...reactively counts comments as they are added/removed. I also do not expect the Comment observer to scale well with a large dataset (see footnote [1]).
  4. ...pushes the entire ReviewCount collection to every client via pub/sub, and updates reactively.
  5. ...creates a template helper for profileSmall that reactively fetches the count for a given profile from the ReviewCount collection.

This code has much in common with publish-counts. The reasons I did not use publish-counts are...

  1. Organization. Counts stores all counts. Great for unrelated one-off counts, but for a large mass of related counts, I prefer to keep them separate.
  2. Mass publish. Counts.publish works for single values and needs N subscriptions for N counts, plus a subscription to figure out what N is. This solution dumps a collection of counts to each client with a single subscription.

Cons:

  1. Slow startup for large datasets because of observers.
  2. Entire collection of review counts is dumped to every client, whether they need it all or not.

Note that there is probably a better solution than this long term.

For Con 1, if you read the Kadira article I referenced in previous comment, that team recommends using cursor.count() instead of cursor.observe(). You might user a timer as they suggest to refresh the counts for every profile in the ReviewCounts collection; maybe even update 100 review counts every 5 seconds to reduce the load on the server.

For Con 2, you can use Meteor.call in the profileSmall template to fetch its review count from the server. Now you're only pulling those counts that are actually needed by a user.

Footnote [1]: (see http://docs.meteor.com/#/full/observe)

Before observe returns, added (or addedAt) will be called zero or more times to deliver the initial results of the query.

@ritchieng
Copy link
Author

@boxofrox You are amazing, really thanks :D

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

2 participants