ansonphong / postworld

Wordpress Theme Development Framework
GNU General Public License v2.0
7 stars 0 forks source link

Comments Sorting #81

Closed ansonphong closed 8 years ago

ansonphong commented 10 years ago

Hi Michel, I noticed on the comments that I couldn't get the sorting to work with comment points. Have you managed to make that work for you?

It would be fine if we were sorting with angular on the client-side without needing to re-query the comments on the server-side. Especially, since we haven't developed a comment query in PHP which supports ordering by comments, so the only way to do it would be on the client-side.

Did we develop this yet?

michelhabib commented 10 years ago

yes it is indeed implemented and tested before. it looks like the way the drop down is changed to read the order options is affecting how the load-comments directive is reading the order option, it is always reading it as comment_date, instead of comment_points. i will need to look at it and get back to you/

ansonphong commented 10 years ago

ok.

On Tue, Nov 26, 2013 at 10:35 AM, michelhabib notifications@github.comwrote:

yes it is indeed implemented and tested before. it looks like the way the drop down is changed to read the order options is affecting how the load-comments directive is reading the order option, it is always reading it as comment_date, instead of comment_points. i will need to look at it and get back to you/

— Reply to this email directly or view it on GitHubhttps://github.com/phongmedia/postworld/issues/81#issuecomment-29318687 .

michelhabib commented 10 years ago

completed, pushed to both postworld and RSV2 Theme. Actually, it is not doing it client side, because of the current optimized implementation of the comments tree doesn't support dynamic re-ordering of the comments list. doing so would slow down the rendering terribly, especially with hundreds of comments - if you remember the old conversation about rendering the comments?

So, i noticed it is not implemented on the server side. i am wondering how big is this?

michelhabib commented 10 years ago

referring to my previous comment, i might have been mistaken about this, i said that i tested it before, i thought i tested the sorting of comments by karma, but what i actually tested was filtering the karma [min_points], i mixed that with sorting. if you want me to find a workaround or fix for that, please let me know and i will start on it.

ansonphong commented 10 years ago

Right I recall the conversation. Implimenting it server side would require adding rank_score querying / sorting to get_comments function, which would be additional for for Haidy.

If this is the only current way, then we should approach it in a future phase of development.

Is there a way in Angular where we could segregate the sorting by one step for performance increase. For instance, when defining the orderBy filter, have it only re-assess on an emit, broadcast or watch triggered command? (Rather than ever digest cycle)

On 2013-12-01, at 8:15 AM, michelhabib notifications@github.com wrote:

completed, pushed to both postworld and RSV2 Theme. Actually, it is not doing it client side, because of the current optimized implementation of the comments tree doesn't support dynamic re-ordering of the comments list. doing so would slow down the rendering terribly, especially with hundreds of comments - if you remember the old conversation about rendering the comments?

So, i noticed it is not implemented on the server side. i am wondering how big is this?

— Reply to this email directly or view it on GitHub.

ansonphong commented 10 years ago

It seems to me as though there should be an efficient way to sort them so any performance issues only happen at the time or re-ordering.

We are at the wnd of our budget for now, so if you want to do it as an open source contribution that would be great. Although if you're not available for that, we should wait for a future development cycle.

On 2013-12-01, at 8:17 AM, michelhabib notifications@github.com wrote:

referring to my previous comment, i might have been mistaken about this, i said that i tested it before, i thought i tested the sorting of comments by karma, but what i actually tested was filtering the karma [min_points], i mixed that with sorting. if you want me to find a workaround or fix for that, please let me know and i will start on it.

— Reply to this email directly or view it on GitHub.

michelhabib commented 10 years ago

Because we are using a caching technique to speed up the loading of items, it is hard to use the standard order by filter of AngularJS without having to re-populate the cache. You are right the performance issue would happen only at reloading of the items. The most elegant solution on the client side is to re-order the array of comment objects in javascript and re-inject that back to the directive [some property types would require special handling in sorting, we will need to handle string,number, date at least]. It sounds like doable to me this way, but i thought a server refresh would be easier to implement, as sorting feature is more powerful in php/mysql.

I guess you created the pw_get_comments function, but I know Haidy can update it if needed.

We will do it anyway - regardless of the budget - since it is a major feature that we were supposed to handle completely, just let me know your preference here.

ansonphong commented 10 years ago

OK thanks Michel for handling that.

Due to future growth potential (handling thousands of comments, and wanting to limit returns) I would go with your idea to do the sorting on the server-side. This will allow us more flexibility when scaling, and save performance on the front-end.

So lets go ahead and do sorting server-side as you originally suggested.

I will actually be quite happy to see the comment sorting by points in there - I was missing that. :)

michelhabib commented 10 years ago

Sounds good. Are there specific fields that we need to handle in pw_get_comments? i know the following:-

what else?

michelhabib commented 10 years ago

@hmikhail this is the thread for comments sorting. As we discussed we probably need to do the sorting before creating the tree, as the tree should not mess with the sorting order - this is an assumption. @phongmedia Phong will help us know which fields need to be considered for sorting the comments per the above comment.

hmikhail commented 10 years ago

Done in commit c445b58e0df8999288092ae49b558eb5b819c711 I added a new function pw_new_get_comments($args='') , which overrides the behavior of get_comments() function and supports order by comment_points.

ansonphong commented 10 years ago

OK --- sounds good. All we have in current scope is ordering by date or points. It would be nice to offer ordering options for that, such as "ASC" / "DESC", mainly for the date sorting - we could have it in the same drop down. I would also like to be able to save the user's settings for that somehow. This should be easy to 'set' the option in the usermeta, although I'm unclear how we could 'load' the user's saved settings into Angular before it queries on the comments. If this is too complex, we can do this in a future phase.

Michel have you tested implementation of the pw_new_get_comments from the angular side?