Order comments by likes

[sirex]

Thanks for the Spirit I really like it and started to migrate ubuntu.lt forum, here is the demo: https://demo.ubuntu.lt

What do you think about adding possibility to sort topic comments by number of given likes?

Usually in forums there are two types of topics: Q&A and just free form discussion. For Q&A it is more important to see the best answer instead of chronological order, so sorting by likes would be really useful.

It would be even better if category would have a sort setting for all it's topics, then if one posts in a Q&A category, comments would be sorted by likes, by default.

Would do you think about this feature. If you think that this is worth adding, thin I could create pull request with the implementation. Other wise I will probably extend Spirit just for my project use.

[nitely]

Thanks for the Spirit I really like it and started to migrate ubuntu.lt forum, here is the demo: https://demo.ubuntu.lt

Awesome 😄 , I'm glad you are considering Spirit for it.

What do you think about adding possibility to sort topic comments by number of given likes?

Seems like it could be a plugin, but since there is no plugin system in place, I think you are better off forking the project and extending it. I would not add such feature as a core feature.

[nitely]

Moreover, I don't know this would work, not even as a plugin. The discussion taking place would not make any sense. For example, let's say you reply, then I want to add something to your reply, so I mention you or quote you, then I get more likes than you do, now my reply comes before yours (?!!). Now, think of this at scale, reply order (the discussion) would not make any sense. For this to sort of work everyone should constraint their reply to the main comment.

StackOverFlow works because they have threaded/nested comments, so you can comment on a reply or make your own reply if it solves the question.

I think microsoft forums have something that could work, comments are ordered by date but there is also a reply marked as the one solving the question.

What I think it may work in your case, and it's a feature a I'd like to implement, is the "best of", after X replies and likes, you can click on a button called "best of" that will filter the comments showing the ones with most likes. I think VanillaForums and Discourse have this.

[sirex]

I agree, that with sorting by number of likes will make sense only without replies.

What I was planning to do is to make sorting by likes optional, and turned on by default only on some categories, where this kind of sorting makes sense (categories with questions and answers). If someone would like to see the discussion, not only best answers, he could change sorting.

At least, this seems to be easiest solution.

Other options:

Adding two levels of replies

I like the way, how StackOverflow does it. It has two levels of replies, first level is direct reply to the topic (first comment in Spirit case), and second level is reply to a comment.

This would solve sorting by likes issue and also makes forum thread more readable, because replies to the topic and replies to the comments will be visually distinguished.

I really miss that feature in most of the forums, and would really like to see in Spirit. It just makes provided information so much better organized compared to endless flat list of comments, where every comment looks the same.

Summarizing topic

This is similar to what Discourse have on button called „Summarize this Topic“ and I guess this is similar to what you proposed with „best of“ button.

Not sure how Discourse does it, but when you press „Summarize this Topic“, it hides some comments. I guess those comments without any replies or likes are hidden, not sure, but it looks that Discourse tracks replies in some way. Here is example how it works:

http://try.discourse.org/t/what-happens-when-a-topic-has-over-1000-replies/301

Hidden comments are shown as expandable items like this:

view 250 hidden replies

Chronological order is preserved, in case you want to follow whole discussion, but also less important comments are hidden, to make whole thread less noisy.

Still, two levels of replies IMHO is much better solution.

[nitely]

What I was planning to do is to make sorting by likes optional, and turned on by default only on some categories, where this kind of sorting makes sense (categories with questions and answers). If someone would like to see the discussion, not only best answers, he could change sorting.

I guess it may make sense in specific cases (out of Spirit scope).

This is similar to what Discourse have on button called „Summarize this Topic“ and I guess this is similar to what you proposed with „best of“ button.

It's what I was talking about, they renamed the feature.It shows the X comments with the most likes, ordered by date. I guess they also think it does not make sense sorting by likes.

Still, two levels of replies IMHO is much better solution.

Spirit won't ever have this, at least not in core. I think a plugin could handle this, having a model for the nested comments and overwritting the comments template.

[sirex]

Still, two levels of replies IMHO is much better solution.

Spirit won't ever have this, at least not in core. I think a plugin could handle this, having a model for the nested comments and overwritting the comments template.

In that case, it would be much easier for plugin authors to have a parent field on Comment model. Overriding templates is easy, but switching models is not.

Having parent, for Spirit it will be possible to add „go to the quoted post“ link as in Discourse, and if parent is in place, adding two levels of replies would be just a simple template and queryset override.

[nitely]

In that case, it would be much easier for plugin authors to have a parent field on Comment model.

As I said, not a Spirit user case. If you want to change the internals just fork it.

Overriding templates is easy, but switching models is not.

I did not suggest to "switch" models, but to add another one that will hold the nested replies.

[sirex]

Is „Go to the quoted post“ link is also not Spirit user case?

[nitely]

That's embedded into the message itself (markdown or preprocessed HTML), nothing to do with nested/threaded replies.

[sirex]

I'm a bit sceptical about thin core and a lot of functionality put in plugins, but I will try to create a plugin and will see how far I can go with it.

I'm planing to go with two levels of replies approach, because it is proven by StackOverflow to work very well.

This will require following extension points:

• Possibility to store parent field in database. This can be achieved by creating new model with OneToOneField reference to Comment, as you suggested. This will add extra join, but for ubuntu.lt low traffic case this will not be an issue.
• Possibility to override (or better extend) comments queryset in spirit.topic.views.detail view, before pagination. At the moment, this is not possible.
• Possibility to add extra context for templates in a view. Not possible at the moment.
• Possibility to add extra fieldsCommentForm and execute some extra code in spirit.comment.views.publish, to store parent field. Not possible at the moment.
• Possibility to add extra fields in category form. I'm planning to add QA type topics just for some categories, configurable in Category form. Not possible at the moment.
• Possibility to extend spirit/comment/templates/spirit/comment/_render_list.html and inject html after each comment, the place where next level of comments will be rendered. Probably it would be nice to have single comment in separate template. Not possible at the moment. Currently it is not possible to extend templates, there is no block in templates, that can be override, the only way is to override whole template which will make templates really hard to maintain.

So far, I see, that a lot have to be done, to make plugins possible. The question is, is it going to happen? Or this kind of extensibility is not planned for Spirit at all?

[nitely]

I'm a bit sceptical about thin core

You can always fork.

Possibility to override (or better extend) comments queryset in spirit.topic.views.detail view, before pagination. At the moment, this is not possible.

Pagination is lazy, so you can modify the query any time before rendering it, so it's not a concern at all. A signal should be added pre_render(comments).

Possibility to add extra context for templates in a view. Not possible at the moment.

To add what?

• Possibility to add extra fieldsCommentForm and execute some extra code in spirit.comment.views.publish, to store parent field. Not possible at the moment.
• Possibility to add extra fields in category form. I'm planning to add QA type topics just for some categories, configurable in Category form. Not possible at the moment.
• Possibility to extend spirit/comment/templates/spirit/comment/_render_list.html and inject html after each comment...

django-djconfig can take care of that.

The question is, is it going to happen?

Only if you send a PR 😄

[sirex]

You can always fork.

„The most important characteristic of a fork is that it spawns competing projects that cannot later exchange code, splitting the potential developer community.“ -- Eric S. Raymond, Jargon File

To add what?

To add second query for second comments level. Since Comment model is not based on MPTT or any other tree like structure, so I can't simply add more parameters to existing query, I think, I will need to do second query, fore second level of comments.

django-djconfig can take care of that.

django-djonfig provides global config, what I need is per-category config.

Only if you send a PR 😄

Before starting to code I would like to know, if this going to be accepted.

[nitely]

You can always fork.

„The most important characteristic of a fork is that it spawns competing projects that cannot later exchange code, splitting the potential developer community.“ -- Eric S. Raymond, Jargon File

You can fork the project and try to add support for every user case out there... good luck with that.

To add what?

To add second query for second comments level. Since Comment model is not based on MPTT or any other tree like structure, so I can't simply add more parameters to existing query, I think, I will need to do second query, fore second level of comments.

Why do you want to modify the context? nested comments should be attached to parent comments, so no need to modify the context there.

django-djconfig can take care of that.
django-djonfig provides global config, what I need is per-category config.

Oops, I meant django-hooks

Before starting to code I would like to know, if this going to be accepted.

You are asking me to predict the future. If all you do is adding hooks then I probably will merge the PR.

[sirex]

Why do you want to modify the context? nested comments should be attached to parent comments, so no need to modify the context there.

To avoid query for each comment.

[nitely]

Why do you want to modify the context? nested comments should be attached to parent comments, so no need to modify the context there.

To avoid query for each comment.

That does not make sense. You don't need to modify the context. Every nested comment will be attached to the parent comment object itself.

[sirex]

Here is my NestedComments model (using django-mptt):

from mptt.models import MPTTModel, TreeForeignKey

class NestedComments(MPTTModel):
    comment = OneToOneField(Comment)
    parent = TreeForeignKey('self', null=True, blank=True, related_name='children', db_index=True)

Here is existing comments queryset:

    comments = Comment.objects\
        .for_topic(topic=topic)\
        .with_likes(user=request.user)\
        .with_polls(user=request.user)\
        .order_by('date')

Since all tree related queryset manages are available only on NestedComments, I can't access them, from comments queryset. That is why I have to do second query, for all children.

[nitely]

You want to do comments.select_related('nested_comment_set') or something like that, better use OneToOneField('spirit_comment.Comment', related_name='nested_comments') and select_related('nested_comments'), that's were pre_render signal would be useful. Then you can access the children through comment.nested_comments.parent.all() (or whatever MPTT query)

Not sure why you are using MPTT, you want more than one level of nesting? StackOVerFlow has only one.

[sirex]

With one level prefetch_related('children') would work. But I want unlimited nesting.

Using MPTT, it is quite easy to produce output, that looks like two levels of nesting, even if database has information about multi level nesting. It is easy to adjust comment rendering, but it is not possible to switch to another rendering if you don't have data.

That is why I would like to store data about all nesting level to not lose meta data, but render only two.

Also, having reference to parent comment, I will be able to add link „Go to the replied message“.

In addition, I'm thinking about rating comments not only by stars, but also by number of replies in case there is no stars. Again, data about who replied what gives me more flexibility to implement better rating algorithm.

[sirex]

In summary, I want to store all data, and have flexibility to render them whatever I like. I can always change rendering, but users will enter data only once.

[nitely]

ok, with django-hooks you can modify the context within a template hook.

[sirex]

As I understand, in a template hook I will receive lazy pagination object, instead of queryset? How can I modify comments queryset to change ordering?

[nitely]

I think comments.object_list will contain the queryset.

[nitely]

I think I'm wrong, you can't change the query since a limit has been applied. So yeah, you will have to put the signal before the pagination.

[nitely]

Apparently you can, try it and let me know or I will try it later and let you know.

Edit: You can not: AssertionError Cannot reorder a query once a slice has been taken..

[sirex]

@nitely for now, I'm just asking around, to know the answers before starting to code. I'm not sure yet, when I will try to implement QA plugin, but I guess this will be this or next weekend.

I will add necessary plugin points to the Spirit and will create pull request.

For now, part with templates, adding extra context variables and how to modify query set looks clear to me. I should use Django Signals and django-hooks for templates.

What is not clear, is how to extend CategoryForm with new fields? Any ideas?

[nitely]

What is not clear, is how to extend CategoryForm with new fields? Any ideas?

You should add a form hook to the category creation view.

[nitely]

So yeah, you will have to put the signal before the pagination.

This won't work, since you won't be able to change the view comments/queryset from the signal. I see two solutions: 1. create the context and pass it to the signal before paginate, 2. re-paginate, within the template or passing the context to a signal after paginate.

# re-paginate example
def signal_handler_pre_render(context, *args, **kwargs):
    comments = context['comments']
    comments.paginator.object_list = comments.paginator.object_list.order_by(...)
    context['comments'] = comments.paginator.page(comments.number)

FIrst one is less weird, but requires reassign the paginated comments to the context. Second one requires almost no changes to the view, but the signal handler is awkward as shown above.

I say go with the first one.

[sirex]

This won't work, since you won't be able to change the view comments/queryset from the signal.

What do you think about this solution:

    comments = Comment.objects\
        .for_topic(topic=topic)\
        .with_likes(user=request.user)\
        .with_polls(user=request.user)\
        .order_by('date')

    for receiver, response in comments_queryset_signal.send(sender=topic, comments=comments):
        if response is not None:
            for method, args, kwars in response:
                  comments = getattr(comments, method)(*args, **kwargs)

    comments = paginate(
        comments,
        per_page=config.comments_per_page,
        page_number=request.GET.get('page', 1)
    )

?

Then handler would look something like this:

@receiver(comments_queryset_signal)
def comments_queryset_signal_handler(sender, **kwargs):
    return [('order_by', ('parent'), {})]

The idea here is to collect all responses, that modify query set, and then apply all those modifications. Probably small wrapper would help to make code more readable.

Anyway, something like get_queryset is really needed for plugins, because need to modify queryset is quite common.

Maybe views should be refactored to class based views? But personally I like signals more, but out of the box, signals does not really let you override things.

[nitely]

Passing the context is cleaner.

El lun., feb. 1, 2016 5:35 AM, sirex notifications@github.com escribió:

This won't work, since you won't be able to change the view
comments/queryset from the signal.

What do you think about this solution:

comments = Comment.objects\
    .for_topic(topic=topic)\
    .with_likes(user=request.user)\
    .with_polls(user=request.user)\
    .order_by('date'

)

for receiver, response in comments_queryset_signal.send(sender=topic, comments=comments):
    if response is not None:
        for method, args, kwars in response:
              comments = getattr(comments, method)(*args, **kwargs)

comments = paginate(
    comments,
    per_page=config.comments_per_page,
    page_number=request.GET.get('page', 1)
)

?

Then handler would look something like this:

@receiver(comments_queryset_signal)def comments_queryset_signal_handler(sender, **kwargs):
return [('order_by', ('parent'), {})]

The idea here is to collect all responses, that modify query set, and then
apply all those modifications. Probably small wrapper would help to make
code more readable.

Anyway, something like get_queryset
https://docs.djangoproject.com/en/1.9/ref/class-based-views/mixins-multiple-object/#django.views.generic.list.MultipleObjectMixin.get_queryset
is really needed for plugins, because need to modify queryset is quite
common.

Maybe views should be refactored to class based views? But personally I
like signals more, but out of the box, signals does not really let you
override things.

—
Reply to this email directly or view it on GitHub
#95 (comment).

[nitely]

Maybe views should be refactored to class based views?

No. That won't solve the issue. You would be able to override the view, but other plugin may also override the view and that would replace your plugin changes. So...
Also, not a fan of CBV.

Just pass the context to the signal.

    comments = Comment.objects\
        .for_topic(topic=topic)\
        .with_likes(user=request.user)\
        .with_polls(user=request.user)\
        .order_by('date')

    context = {
        'topic': topic,
        'comments': comments
    }

    pre_render_topic_detail.send(context=context)

    context['comments'] = paginate(
        context['comments'],
        per_page=config.comments_per_page,
        page_number=request.GET.get('page', 1)
    )

    # ...