From 8a5484d0b77db4bb168e5831c9026a9bddae3e14 Mon Sep 17 00:00:00 2001 From: Julian Dehm Date: Tue, 1 Aug 2023 14:30:47 +0200 Subject: [PATCH] comments_async: make fetchComments take arguments instead of relying on state variables. Should fix the issue with the scrolling not working when an anchor is passed. --- adhocracy4/comments_async/api.py | 9 +++++ .../static/comments_async/comment_box.jsx | 35 +++++++------------ adhocracy4/models/query.py | 1 + changelog/0004.md | 7 ++++ tests/comments/test_async_api.py | 2 +- tests/comments/test_async_api_org_terms.py | 6 ++-- 6 files changed, 33 insertions(+), 27 deletions(-) create mode 100644 changelog/0004.md diff --git a/adhocracy4/comments_async/api.py b/adhocracy4/comments_async/api.py index f9b6d8c2e..34198406e 100644 --- a/adhocracy4/comments_async/api.py +++ b/adhocracy4/comments_async/api.py @@ -1,6 +1,7 @@ from django.apps import apps from django.conf import settings from django.contrib.contenttypes.models import ContentType +from django.db.models import Count from django.urls import reverse from django.urls.exceptions import NoReverseMatch from django.utils.translation import gettext_lazy as _ @@ -37,10 +38,18 @@ def list(self, request, content_type=None, object_pk=None): Attention: No super, be careful with order of inheritance! """ queryset = self.filter_queryset(self.get_queryset()) + child_comment_count = queryset.aggregate( + child_comment_count=Count("child_comments") + ) page = self.paginate_queryset(queryset) if page is not None: serializer = self.get_serializer(page, many=True) response = self.get_paginated_response(serializer.data) + # add a comment count including the child comments as the paginator doesn't + # include them + response.data["comment_count"] = ( + response.data["count"] + child_comment_count["child_comment_count"] + ) if "commentID" in request.query_params: try: commentID = int(request.query_params["commentID"]) diff --git a/adhocracy4/comments_async/static/comments_async/comment_box.jsx b/adhocracy4/comments_async/static/comments_async/comment_box.jsx index b80f5e222..71deea191 100644 --- a/adhocracy4/comments_async/static/comments_async/comment_box.jsx +++ b/adhocracy4/comments_async/static/comments_async/comment_box.jsx @@ -83,7 +83,7 @@ export const CommentBox = (props) => { window.removeEventListener('scroll', handleScroll) } // eslint-disable-next-line react-hooks/exhaustive-deps - }, [loading, nextComments]) + }, [loading, nextComments, comments, anchoredCommentParentId]) useEffect(() => { window.addEventListener('agreedTos', handleTermsOfUse) @@ -109,7 +109,7 @@ export const CommentBox = (props) => { translated.entries = django.ngettext('entry', 'entries', data.count) setComments(data.results) setNextComments(data.next) - setCommentCount(calculateCommentCount(data.results)) + setCommentCount(data.comment_count) setHasCommentingPermission(data.has_commenting_permission) setProjectIsPublic(data.project_is_public) setUseTermsOfUse(data.use_org_terms_of_use) @@ -117,10 +117,10 @@ export const CommentBox = (props) => { setOrgTermsUrl(data.org_terms_url) if (props.anchoredCommentId && data.comment_found) { setAnchoredCommentParentId(data.comment_parent) - if (findAnchoredComment(data.results, data.comment_parent)) { + if (findAnchoredComment(data.results, data.comment_parent) || !data.next) { setLoading(false) } else { - fetchComments() + fetchComments(data.next, data.results, data.comment_parent) } } else { if (props.anchoredCommentId) { @@ -293,7 +293,7 @@ export const CommentBox = (props) => { const data = result setComments(data.results) setNextComments(data.next) - setCommentCount(calculateCommentCount(data.results)) + setCommentCount(data.comment_count) setFilter(filter) setFilterDisplay(displayFilter) setLoadingFilter(false) @@ -322,7 +322,7 @@ export const CommentBox = (props) => { const data = result setComments(data.results) setNextComments(data.next) - setCommentCount(calculateCommentCount(data.results)) + setCommentCount(data.comment_count) setSort(order) setLoadingFilter(false) }) @@ -348,7 +348,7 @@ export const CommentBox = (props) => { const data = result setComments(data.results) setNextComments(data.next) - setCommentCount(calculateCommentCount(data.results)) + setCommentCount(data.comment_count) setSearch(search) setLoadingFilter(false) }) @@ -370,18 +370,18 @@ export const CommentBox = (props) => { return true } - function fetchComments () { + function fetchComments (nextComments, comments, anchoredCommentParentId) { fetch(nextComments) .then((response) => response.json()) .then((data) => { const newComments = comments.concat(data.results) setComments(newComments) setNextComments(data.next) - setCommentCount(commentCount + calculateCommentCount(data.results)) - if (findAnchoredComment(newComments, anchoredCommentParentId)) { + setCommentCount(data.comment_count) + if (findAnchoredComment(newComments, anchoredCommentParentId) || !data.next) { setLoading(false) } else { - fetchComments() + fetchComments(data.next, newComments, anchoredCommentParentId) } return null }).catch(error => { @@ -389,17 +389,6 @@ export const CommentBox = (props) => { }) } - function calculateCommentCount (comments) { - let count = 0 - comments.forEach((comment) => { - count++ - if (comment.child_comments.length > 0) { - count += comment.child_comments.length - } - }) - return count - } - function handleScroll () { const html = document.documentElement if ( @@ -408,7 +397,7 @@ export const CommentBox = (props) => { ) { if (nextComments && !loading) { setLoading(true) - fetchComments() + fetchComments(nextComments, comments, anchoredCommentParentId) } } } diff --git a/adhocracy4/models/query.py b/adhocracy4/models/query.py index b0affddb6..40b31aeff 100644 --- a/adhocracy4/models/query.py +++ b/adhocracy4/models/query.py @@ -32,4 +32,5 @@ def annotate_comment_count(self): "comments", distinct=True, # needed to combine with other count annotations ) + + models.Count("comments__child_comments", distinct=True) ) diff --git a/changelog/0004.md b/changelog/0004.md new file mode 100644 index 000000000..defa5c8c7 --- /dev/null +++ b/changelog/0004.md @@ -0,0 +1,7 @@ +### Fixed + +- fixed comment infinite scrolling not working +- fixed anchor links and scrollTo not working when comment was not on the first + pagination page +- fix comment count not including child comments on module tiles, lists, map + pins and detail view diff --git a/tests/comments/test_async_api.py b/tests/comments/test_async_api.py index 769c844df..ec938fe4f 100644 --- a/tests/comments/test_async_api.py +++ b/tests/comments/test_async_api.py @@ -356,7 +356,7 @@ def test_fields(user, apiclient, question_ct, question): response = apiclient.get(url) - assert len(response.data) == 8 + assert len(response.data) == 9 assert "count" in response.data assert "next" in response.data assert "previous" in response.data diff --git a/tests/comments/test_async_api_org_terms.py b/tests/comments/test_async_api_org_terms.py index eb68a5cbb..381ca3f3b 100644 --- a/tests/comments/test_async_api_org_terms.py +++ b/tests/comments/test_async_api_org_terms.py @@ -245,7 +245,7 @@ def test_agreement_info( ) response = apiclient.get(url) - assert len(response.data) == 10 + assert len(response.data) == 11 assert "use_org_terms_of_use" in response.data assert response.data["use_org_terms_of_use"] assert "user_has_agreed" in response.data @@ -256,13 +256,13 @@ def test_agreement_info( # user has agreed apiclient.force_authenticate(user=user) response = apiclient.get(url) - assert len(response.data) == 10 + assert len(response.data) == 11 assert response.data["user_has_agreed"] # another_user has not agreed apiclient.force_authenticate(user=another_user) response = apiclient.get(url) - assert len(response.data) == 10 + assert len(response.data) == 11 assert not response.data["user_has_agreed"]