From 63d893b1ef2c41b1f580ef9ed334abf65692906a Mon Sep 17 00:00:00 2001 From: Pavel Kvach Date: Tue, 2 Apr 2024 21:14:50 +0300 Subject: [PATCH] Add sorting option for comments Add sorting and pagination support for comments, including new API query parameters for sort order and offset. Adjust comment fetching logic accordingly. Closes https://github.com/isso-comments/isso/issues/4 --- docs/docs/reference/client-config.rst | 14 +++ isso/db/comments.py | 15 +-- isso/js/app/api.js | 6 +- isso/js/app/default_config.js | 1 + isso/js/app/isso.js | 33 +++---- isso/js/embed.js | 13 ++- isso/tests/test_comments.py | 128 ++++++++++++++++++++++++++ isso/views/comments.py | 34 ++++++- 8 files changed, 206 insertions(+), 38 deletions(-) diff --git a/docs/docs/reference/client-config.rst b/docs/docs/reference/client-config.rst index 891e2abc7..973eacf93 100644 --- a/docs/docs/reference/client-config.rst +++ b/docs/docs/reference/client-config.rst @@ -13,6 +13,7 @@ preferably in the script tag which embeds the JS: data-isso-max-comments-top="10" data-isso-max-comments-nested="5" data-isso-reveal-on-click="5" + data-isso-sorting="newest" data-isso-avatar="true" data-isso-avatar-bg="#f0f0f0" data-isso-avatar-fg="#9abf88 #5698c4 #e279a3 #9163b6 ..." @@ -255,6 +256,19 @@ data-isso-reply-notifications-default-enabled .. versionadded:: 0.13 +.. _data-isso-sorting: + +data-isso-sorting + A thread sorting method that are applied in the specified order. + + Possible sorting methods: + + - `newest`: Bring newest comments to the top + - `oldest`: Bring oldest comments to the top + - `upvotes`: Bring most liked comments to the top + + Default sorting is `oldest`. + Deprecated Client Settings -------------------------- diff --git a/isso/db/comments.py b/isso/db/comments.py index 9bed6f0b5..de1347a64 100644 --- a/isso/db/comments.py +++ b/isso/db/comments.py @@ -215,7 +215,7 @@ def fetchall(self, mode=5, after=0, parent='any', order_by='id', yield dict(zip(fields_comments + fields_threads, item)) def fetch(self, uri, mode=5, after=0, parent='any', - order_by='id', asc=1, limit=None): + order_by='id', asc=1, limit=None, offset=0): """ Return comments for :param:`uri` with :param:`mode`. """ @@ -240,7 +240,11 @@ def fetch(self, uri, mode=5, after=0, parent='any', if not asc: sql.append(' DESC') - if limit: + if offset and limit: + sql.append('LIMIT ?,?') + sql_args.append(offset) + sql_args.append(limit) + elif limit: sql.append('LIMIT ?') sql_args.append(limit) @@ -327,7 +331,7 @@ def vote(self, upvote, id, remote_addr): return {'likes': likes + 1, 'dislikes': dislikes} return {'likes': likes, 'dislikes': dislikes + 1} - def reply_count(self, url, mode=5, after=0): + def reply_count(self, url, mode=5): """ Return comment count for main thread and all reply threads for one url. """ @@ -335,11 +339,10 @@ def reply_count(self, url, mode=5, after=0): sql = ['SELECT comments.parent,count(*)', 'FROM comments INNER JOIN threads ON', ' threads.uri=? AND comments.tid=threads.id AND', - ' (? | comments.mode = ?) AND', - ' comments.created > ?', + ' (? | comments.mode = ?)', 'GROUP BY comments.parent'] - return dict(self.db.execute(sql, [url, mode, mode, after]).fetchall()) + return dict(self.db.execute(sql, [url, mode, mode]).fetchall()) def count(self, *urls): """ diff --git a/isso/js/app/api.js b/isso/js/app/api.js index 90efa4926..95df73f8d 100644 --- a/isso/js/app/api.js +++ b/isso/js/app/api.js @@ -138,12 +138,14 @@ var view = function(id, plain) { return deferred.promise; }; -var fetch = function(tid, limit, nested_limit, parent, lastcreated) { +var fetch = function(tid, limit, nested_limit, parent, sort, offset) { if (typeof(limit) === 'undefined') { limit = "inf"; } + if (typeof(offset) === 'undefined') { offset = 0; } + if (typeof(sort) === 'undefined') { sort = ""; } if (typeof(nested_limit) === 'undefined') { nested_limit = "inf"; } if (typeof(parent) === 'undefined') { parent = null; } - var query_dict = {uri: tid || location(), after: lastcreated, parent: parent}; + var query_dict = {uri: tid || location(), sort: sort, parent: parent, offset: offset}; if(limit !== "inf") { query_dict['limit'] = limit; diff --git a/isso/js/app/default_config.js b/isso/js/app/default_config.js index 68dd9fecc..6a872aedf 100644 --- a/isso/js/app/default_config.js +++ b/isso/js/app/default_config.js @@ -13,6 +13,7 @@ var default_config = { "max-comments-top": "inf", "max-comments-nested": 5, "reveal-on-click": 5, + "sorting": "oldest", "gravatar": false, "avatar": true, "avatar-bg": "#f0f0f0", diff --git a/isso/js/app/isso.js b/isso/js/app/isso.js index fbf958a3d..5fbec1ec8 100644 --- a/isso/js/app/isso.js +++ b/isso/js/app/isso.js @@ -114,7 +114,7 @@ var Postbox = function(parent) { notification: $("[name=notification]", el).checked() ? 1 : 0, }).then(function(comment) { $(".isso-textarea", el).value = ""; - insert(comment, true); + insert(comment, true, 0); if (parent !== null) { el.onsuccess(); @@ -125,7 +125,7 @@ var Postbox = function(parent) { return el; }; -var insert_loader = function(comment, lastcreated) { +var insert_loader = function(comment, offset) { var entrypoint; if (comment.id === null) { entrypoint = $("#isso-root"); @@ -143,22 +143,19 @@ var insert_loader = function(comment, lastcreated) { api.fetch($("#isso-thread").getAttribute("data-isso-id"), config["reveal-on-click"], config["max-comments-nested"], comment.id, - lastcreated).then( + config["sorting"], + offset).then( function(rv) { if (rv.total_replies === 0) { return; } - var lastcreated = 0; rv.replies.forEach(function(commentObject) { - insert(commentObject, false); - if(commentObject.created > lastcreated) { - lastcreated = commentObject.created; - } + insert(commentObject, false, 0); }); if(rv.hidden_replies > 0) { - insert_loader(rv, lastcreated); + insert_loader(rv, offset + rv.replies.length); } }, function(err) { @@ -167,7 +164,7 @@ var insert_loader = function(comment, lastcreated) { }); }; -var insert = function(comment, scrollIntoView) { +var insert = function(comment, scrollIntoView, offset) { var el = $.htmlify(template.render("comment", {"comment": comment})); // update datetime every 60 seconds @@ -381,19 +378,13 @@ var insert = function(comment, scrollIntoView) { show($("a.isso-reply", footer).detach()); } - if(comment.hasOwnProperty('replies')) { - var lastcreated = 0; - comment.replies.forEach(function(replyObject) { - insert(replyObject, false); - if(replyObject.created > lastcreated) { - lastcreated = replyObject.created; - } - + if (comment.hasOwnProperty('replies')) { + comment.replies.forEach(function (replyObject) { + insert(replyObject, false, offset + 1); }); - if(comment.hidden_replies > 0) { - insert_loader(comment, lastcreated); + if (comment.hidden_replies > 0) { + insert_loader(comment, offset + comment.replies.length); } - } }; diff --git a/isso/js/embed.js b/isso/js/embed.js index 6c7c0c61a..732c2659f 100644 --- a/isso/js/embed.js +++ b/isso/js/embed.js @@ -124,7 +124,10 @@ function fetchComments() { api.fetch(isso_thread.getAttribute("data-isso-id") || location.pathname, config["max-comments-top"], - config["max-comments-nested"]).then( + config["max-comments-nested"], + null, + config["sorting"], + 0).then( function (rv) { if (rv.total_replies === 0) { @@ -132,19 +135,15 @@ function fetchComments() { return; } - var lastcreated = 0; var count = rv.total_replies; rv.replies.forEach(function(comment) { - isso.insert(comment, false); - if (comment.created > lastcreated) { - lastcreated = comment.created; - } + isso.insert(comment, false, 0); count = count + comment.total_replies; }); heading.textContent = i18n.pluralize("num-comments", count); if (rv.hidden_replies > 0) { - isso.insert_loader(rv, lastcreated); + isso.insert_loader(rv, rv.replies.length); } if (window.location.hash.length > 0 && diff --git a/isso/tests/test_comments.py b/isso/tests/test_comments.py index ed4b86550..64b1a33a7 100644 --- a/isso/tests/test_comments.py +++ b/isso/tests/test_comments.py @@ -240,6 +240,134 @@ def testGetLimitedNested(self): rv = loads(r.data) self.assertEqual(len(rv['replies']), 10) + def testGetWithOffset(self): + for i in range(5): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + + r = self.get('/?uri=test&limit=3&offset=2') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + self.assertEqual( + [comment['id'] for comment in rv['replies']], + [3, 4, 5] + ) + + def testGetWithOffsetIgnoredWithoutLimit(self): + for i in range(5): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + + r = self.get('/?uri=test&offset=2') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + self.assertEqual( + [comment['id'] for comment in rv['replies']], + [1, 2, 3, 4, 5] + ) + + def testGetNestedWithOffset(self): + + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + for i in range(5): + self.post('/new?uri=test', + data=json.dumps({'text': '...', 'parent': 1})) + + r = self.get('/?uri=test&parent=1&limit=3&offset=2') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + self.assertEqual( + [comment['id'] for comment in rv['replies']], + [4, 5, 6] + ) + + def testGetSortedByOldest(self): + for i in range(5): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + + r = self.get('/?uri=test&sort=oldest') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + # assert order of comments is oldest first + self.assertEqual( + [comment['id'] for comment in rv['replies']], + [1, 2, 3, 4, 5] + ) + + def testGetSortedByNewest(self): + for i in range(5): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + + r = self.get('/?uri=test&sort=newest') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + # assert order of comments is newest first + self.assertEqual( + [comment['id'] for comment in rv['replies']], + [5, 4, 3, 2, 1] + ) + + def testGetSortedByUpvotes(self): + for i in range(5): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + + # update the likes for some comments + self.app.db.execute( + 'UPDATE comments SET likes = id WHERE id IN (2, 4)' + ) + + r = self.get('/?uri=test&sort=upvotes') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + # assert order of comments is by upvotes + self.assertEqual( + [comment['id'] for comment in rv['replies']], + [4, 2, 1, 3, 5] + ) + + def testGetSortedByNewestWithNested(self): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + for i in range(5): + self.post('/new?uri=test', + data=json.dumps({'text': '...', 'parent': 1})) + + r = self.get('/?uri=test&sort=newest') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + self.assertEqual(len(rv['replies']), 1) + # assert order of nested comments is newest first + self.assertEqual( + [comment['id'] for comment in rv['replies'][0]['replies']], + [6, 5, 4, 3, 2] + ) + + def testGetSortedByUpvotesWithNested(self): + self.post('/new?uri=test', data=json.dumps({'text': '...'})) + for i in range(5): + self.post('/new?uri=test', + data=json.dumps({'text': '...', 'parent': 1})) + + # update the likes for some comments + self.app.db.execute( + 'UPDATE comments SET likes = id WHERE id IN (3, 6)' + ) + + r = self.get('/?uri=test&sort=upvotes') + self.assertEqual(r.status_code, 200) + + rv = loads(r.data) + self.assertEqual(len(rv['replies']), 1) + # assert order of nested comments is newest first + self.assertEqual( + [comment['id'] for comment in rv['replies'][0]['replies']], + [6, 3, 2, 4, 5] + ) + def testUpdate(self): self.post('/new?uri=%2Fpath%2F', diff --git a/isso/views/comments.py b/isso/views/comments.py index 3e31f2293..95272426e 100644 --- a/isso/views/comments.py +++ b/isso/views/comments.py @@ -775,6 +775,10 @@ def moderate(self, environ, request, id, action, key): The maximum number of returned nested comments per comment. Omit for unlimited results. @apiQuery {Number} [after] Includes only comments were added after the provided UNIX timestamp. + @apiQuery {String} sort + The sorting order of the comments. Possible values are `newest`, `oldest`, `upvotes`. + @apiQuery {Number} [offset] + Offset the returned comments by this number. Used for pagination. Works only in combination with `limit`. @apiSuccess {Number} id Id of the comment `replies` is the list of replies of. `null` for the list of top-level comments. @@ -851,6 +855,23 @@ def fetch(self, environ, request, uri): 'after': request.args.get('after', 0) } + # map sort query parameter + valid_sort_options = ['newest', 'oldest', 'upvotes'] + sort = request.args.get('sort') + + if sort and sort not in valid_sort_options: + return BadRequest("Invalid sort option. Must be one of: 'newest', 'oldest', 'upvotes'") + + if sort == 'newest': + args['order_by'] = 'created' + args['asc'] = 0 + elif sort == 'oldest': + args['order_by'] = 'created' + args['asc'] = 1 + elif sort == 'upvotes': + args['order_by'] = 'likes' + args['asc'] = 0 + try: args['limit'] = int(request.args.get('limit')) except TypeError: @@ -858,6 +879,13 @@ def fetch(self, environ, request, uri): except ValueError: return BadRequest("limit should be integer") + try: + args['offset'] = int(request.args.get('offset', 0)) + if args['offset'] < 0: + return BadRequest("offset should not be negative") + except ValueError: + return BadRequest("offset should be integer") + if request.args.get('parent') is not None: try: args['parent'] = int(request.args.get('parent')) @@ -870,7 +898,7 @@ def fetch(self, environ, request, uri): plain = request.args.get('plain', '0') == '0' - reply_counts = self.comments.reply_count(uri, after=args['after']) + reply_counts = self.comments.reply_count(uri) if args['limit'] == 0: root_list = [] @@ -890,7 +918,7 @@ def fetch(self, environ, request, uri): rv = { 'id': root_id, 'total_replies': reply_counts[root_id], - 'hidden_replies': reply_counts[root_id] - len(root_list), + 'hidden_replies': reply_counts[root_id] - len(root_list) - args['offset'], 'replies': self._process_fetched_list(root_list, plain), 'config': self.public_conf } @@ -903,6 +931,8 @@ def fetch(self, environ, request, uri): if nested_limit > 0: args['parent'] = comment['id'] args['limit'] = nested_limit + # Reset offset to 0 for nested comments to ensure correct pagination + args['offset'] = 0 replies = list(self.comments.fetch(**args)) else: replies = []