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

Comments total at /Admin/Blogs is always 0 #7838

Closed
robertwray opened this Issue Aug 31, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@robertwray

robertwray commented Aug 31, 2017

This issue can be seen by either:

  • Clicking on "Blog" in the /Admin menu (for an Orchard instance where there is more than one blog)
  • Navigating directly to /Admin/Blogs (for an Orchard instance where there's only one blog which results in clicking on "Blog" in the /Admin menu taking you to the posts view for that blog)

The text under the name of the blog will look something like this:

0 comments | Last modified: 7 days ago | By robertwray

The issue is that, irrespective of the number of comments actually present against individual blog posts, the totaliser will always state "0 comments". I think this is due to the fact that CommentsContainerPartDriver (from Orchard.Comments) asks the commentService for the number of comments associated with the ContentItem for the blog, which is 0, rather than asking for the number of comments associated with the blog posts associated with the blog.

Unfortunately I'm too new to the Orchard codebase and how it all hangs together to work out how this could be fixed, other than by simply removing the field that indicates the number of comments against the blog.

@Hazzamanic

This comment has been minimized.

Show comment
Hide comment
@Hazzamanic

Hazzamanic Sep 4, 2017

Contributor

Weirdly, the comments module attaches the comments container part onto the blog content type in it's migrations. I assume someone meant to attach it to the blog post, not that it has any business doing that. So basically it is a mistake, the comments container part (which displays the number of comments), should not be attached to the blog content type and be displayed at all

Contributor

Hazzamanic commented Sep 4, 2017

Weirdly, the comments module attaches the comments container part onto the blog content type in it's migrations. I assume someone meant to attach it to the blog post, not that it has any business doing that. So basically it is a mistake, the comments container part (which displays the number of comments), should not be attached to the blog content type and be displayed at all

@robertwray

This comment has been minimized.

Show comment
Hide comment
@robertwray

robertwray Sep 4, 2017

I can see the utility in having the aggregate number of comments (specifically the number that are pending) for a given blog visible at the blog level, rather than at a post level, so perhaps there was an intention to provide that, albeit one that didn't work?

Is it feasible to have a solution in Orchard that allows something that contains items (e.g. a blog, or a list?) which have comments associated with its distinct member items to report back aggregate totals, without writing the mother of kludges to achieve that?

robertwray commented Sep 4, 2017

I can see the utility in having the aggregate number of comments (specifically the number that are pending) for a given blog visible at the blog level, rather than at a post level, so perhaps there was an intention to provide that, albeit one that didn't work?

Is it feasible to have a solution in Orchard that allows something that contains items (e.g. a blog, or a list?) which have comments associated with its distinct member items to report back aggregate totals, without writing the mother of kludges to achieve that?

@Hazzamanic

This comment has been minimized.

Show comment
Hide comment
@Hazzamanic

Hazzamanic Sep 4, 2017

Contributor

Actually, I was wrong and you were right. It looks like the part being attached is named to be attached to the container i.e. the blog. But yeah it just totally fails to implement that functionality. It also returns the same shape name as that which is used for the CommentsPart count. And the container part is not attachable. Overall, a big fail.

I too have a site with many blogs and this functionality would actually be pretty cool so I just chucked together a quick module for it. Some of the blogs have quite a lot of posts and querying for the comment counts was a bit of a performance hog so I am storing aggregates. The module is here: https://github.com/Hazzamanic/Hazza.Blogging. The module hides the broken container comments. To use enable the feature 'Blog Comment Count' and attach the ContainerCommentsCountPart to your Blog type. The counts will update when a comment is added to any blog post in that blog (or updating the status of a comment in the comment admin area).

I'm not sure if it is worth integrating this into core or removing the broken attempt and letting people use their own solution/this extension.

Note: If you cant install a module or just want to add the functionality to the comments module in place, you can find the queries here: https://github.com/Hazzamanic/Hazza.Blogging/blob/master/Services/ContainerCommentProcessor.cs#L40 and chuck them into the driver Orchard.Comments.Drivers.CommentsContainerPartDriver and replace the funcs to calculate the comment counts with those queries.

Contributor

Hazzamanic commented Sep 4, 2017

Actually, I was wrong and you were right. It looks like the part being attached is named to be attached to the container i.e. the blog. But yeah it just totally fails to implement that functionality. It also returns the same shape name as that which is used for the CommentsPart count. And the container part is not attachable. Overall, a big fail.

I too have a site with many blogs and this functionality would actually be pretty cool so I just chucked together a quick module for it. Some of the blogs have quite a lot of posts and querying for the comment counts was a bit of a performance hog so I am storing aggregates. The module is here: https://github.com/Hazzamanic/Hazza.Blogging. The module hides the broken container comments. To use enable the feature 'Blog Comment Count' and attach the ContainerCommentsCountPart to your Blog type. The counts will update when a comment is added to any blog post in that blog (or updating the status of a comment in the comment admin area).

I'm not sure if it is worth integrating this into core or removing the broken attempt and letting people use their own solution/this extension.

Note: If you cant install a module or just want to add the functionality to the comments module in place, you can find the queries here: https://github.com/Hazzamanic/Hazza.Blogging/blob/master/Services/ContainerCommentProcessor.cs#L40 and chuck them into the driver Orchard.Comments.Drivers.CommentsContainerPartDriver and replace the funcs to calculate the comment counts with those queries.

@sebastienros sebastienros added this to the Orchard 1.10.x milestone Sep 7, 2017

@Hazzamanic

This comment has been minimized.

Show comment
Hide comment
@Hazzamanic

Hazzamanic Sep 10, 2017

Contributor

@sebastienros do you want me to add this to core? or just remove the broken views from core and let people use a module to do this?

@robertwray I added the blog comment overview to the blog management page too, which is a pretty nice way to see comment overviews for your blog. That was a lot of blog

Contributor

Hazzamanic commented Sep 10, 2017

@sebastienros do you want me to add this to core? or just remove the broken views from core and let people use a module to do this?

@robertwray I added the blog comment overview to the blog management page too, which is a pretty nice way to see comment overviews for your blog. That was a lot of blog

jchenga added a commit to jchenga/Orchard that referenced this issue Sep 13, 2017

@jchenga

This comment has been minimized.

Show comment
Hide comment
@jchenga

jchenga Sep 13, 2017

Contributor

Pull request created: #7847

Contributor

jchenga commented Sep 13, 2017

Pull request created: #7847

@Hazzamanic

This comment has been minimized.

Show comment
Hide comment
@Hazzamanic

Hazzamanic Sep 13, 2017

Contributor

Haha good catch, I totally didn't even see that the comments contain that property! Much simpler

Contributor

Hazzamanic commented Sep 13, 2017

Haha good catch, I totally didn't even see that the comments contain that property! Much simpler

@sebastienros

This comment has been minimized.

Show comment
Hide comment
@sebastienros

sebastienros Sep 14, 2017

Member

The PR sounds good, I commented on it as it's a breaking change.

Member

sebastienros commented Sep 14, 2017

The PR sounds good, I commented on it as it's a breaking change.

jchenga added a commit to jchenga/Orchard that referenced this issue Sep 15, 2017

@jchenga

This comment has been minimized.

Show comment
Hide comment
@jchenga

jchenga Sep 16, 2017

Contributor

Pull request created: #7853. The change is based on dev branch as requested.

Contributor

jchenga commented Sep 16, 2017

Pull request created: #7853. The change is based on dev branch as requested.

@sebastienros sebastienros modified the milestones: Orchard 1.10.x, dev Sep 21, 2017

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