diff --git a/openlobby/core/api/schema.py b/openlobby/core/api/schema.py index c2c79abf9e9b1e5355c750b08412067b37f70a37..7d8235b04d1e0961930d22f7770d351c29472e44 100644 --- a/openlobby/core/api/schema.py +++ b/openlobby/core/api/schema.py @@ -36,9 +36,7 @@ class SearchReportsConnection(relay.Connection): def _get_authors_cache(ids): - authors = User.objects.filter(id__in=ids).annotate( - total_reports=Count("report", filter=Q(report__is_draft=False)) - ) + authors = User.objects.with_total_reports().filter(id__in=ids) return {a.id: types.Author.from_db(a) for a in authors} diff --git a/openlobby/core/api/types.py b/openlobby/core/api/types.py index c4932b542b23db4c10d21fa7be5aae84ad1f20a9..de692b88e1611d6fb504023bcc60aebda9c54dcb 100644 --- a/openlobby/core/api/types.py +++ b/openlobby/core/api/types.py @@ -154,9 +154,7 @@ class Author(graphene.ObjectType): @classmethod def get_node(cls, info, id): try: - author = models.User.objects.annotate( - total_reports=Count("report", filter=Q(report__is_draft=False)) - ).get(id=id, is_author=True) + author = models.User.objects.with_total_reports().get(id=id, is_author=True) return cls.from_db(author) except models.User.DoesNotExist: return None @@ -176,7 +174,9 @@ class Author(graphene.ObjectType): return ReportConnection(page_info=page_info, edges=edges, total_count=total) def resolve_total_reports(self, info, **kwargs): - return models.Report.objects.filter(author_id=self.id, is_draft=False).count() + return models.Report.objects.filter( + author_id=self.id, is_draft=False, superseded_by=None + ).count() class LoginShortcut(graphene.ObjectType): diff --git a/openlobby/core/documents.py b/openlobby/core/documents.py index 113c07384553aa74e7b5e5d7c31350b33e90743b..72fd8729f58e0584a3581dde45a73a3f60750f53 100644 --- a/openlobby/core/documents.py +++ b/openlobby/core/documents.py @@ -19,7 +19,7 @@ class ReportDoc(DocType): other_participants = fields.TextField(analyzer=settings.ES_TEXT_ANALYZER) # there is no support for JSONField now, so we serialize it to keyword extra_serialized = fields.KeywordField() - superseded_by = fields.IntegerField() + superseded_by_id = fields.IntegerField() def prepare_extra_serialized(self, instance): return json.dumps(instance.extra) diff --git a/openlobby/core/models.py b/openlobby/core/models.py index 7f528222209bf9a80127351d457a595d8b02830d..97648f705dbeaf8bb8c5a9d1fabcdd5a241d4a02 100644 --- a/openlobby/core/models.py +++ b/openlobby/core/models.py @@ -10,6 +10,14 @@ from django.utils import timezone class CustomUserManager(UserManager): + def with_total_reports(self): + return self.get_queryset().annotate( + total_reports=Count( + "report", + filter=Q(report__is_draft=False) & Q(report__superseded_by=None), + ) + ) + def sorted(self, **kwargs): # inline import intentionally from openlobby.core.api.schema import ( @@ -17,9 +25,7 @@ class CustomUserManager(UserManager): AUTHOR_SORT_TOTAL_REPORTS_ID, ) - qs = self.get_queryset().annotate( - total_reports=Count("report", filter=Q(report__is_draft=False)) - ) + qs = self.with_total_reports() sort_field = kwargs.get("sort", AUTHOR_SORT_LAST_NAME_ID) if sort_field == AUTHOR_SORT_LAST_NAME_ID: diff --git a/openlobby/core/search.py b/openlobby/core/search.py index fcf4bd2d7d0f5b2b862941d13625a1774779819b..56277ed2ac4abdfcd76ef5241080552403da0d50 100644 --- a/openlobby/core/search.py +++ b/openlobby/core/search.py @@ -19,6 +19,7 @@ def query_reports(query, paginator, *, highlight=False): ] s = ReportDoc.search() s = s.exclude("term", is_draft=True) + s = s.exclude("exists", field="superseded_by_id") if query != "": s = s.query("multi_match", query=query, fields=fields) if highlight: @@ -31,6 +32,7 @@ def query_reports(query, paginator, *, highlight=False): def reports_by_author(author_id, paginator): s = ReportDoc.search() s = s.exclude("term", is_draft=True) + s = s.exclude("exists", field="superseded_by_id") s = s.filter("term", author_id=author_id) s = s.sort("-date") s = s[paginator.slice_from : paginator.slice_to] diff --git a/tests/dummy.py b/tests/dummy.py index d17511334fc17fe26ada0f2b61fb5ecf198aea5a..b0effcd184f4c422810e4d29c291c687fe0876d6 100644 --- a/tests/dummy.py +++ b/tests/dummy.py @@ -14,16 +14,16 @@ authors = [ }, { "id": 2, - "username": "sponge", - "first_name": "Spongebob", - "last_name": "Squarepants", + "username": "shaun", + "first_name": "Shaun", + "last_name": "Sheep", "is_author": True, }, { "id": 3, - "username": "shaun", - "first_name": "Shaun", - "last_name": "Sheep", + "username": "sponge", + "first_name": "Spongebob", + "last_name": "Squarepants", "is_author": True, }, ] @@ -92,6 +92,47 @@ reports = [ "other_participants": "", "is_draft": True, }, + { + "id": 6, + "superseded_by_id": 2, + "date": arrow.get(2018, 1, 3).datetime, + "published": arrow.get(2018, 1, 4).datetime, + "edited": arrow.get(2018, 2, 1).datetime, + "title": "The Two Towers", + "body": "Another long story in progress.", + "received_benefit": "Mithrill Jacket", + "provided_benefit": "The Ring", + "our_participants": "Frodo, Gimli, Legolas", + "other_participants": "Saruman, Sauron", + "extra": {"rings": 2}, + }, + { + "id": 7, + "superseded_by_id": 2, + "date": arrow.get(2018, 1, 3).datetime, + "published": arrow.get(2018, 1, 4).datetime, + "edited": arrow.get(2018, 2, 5).datetime, + "title": "The Towels", + "body": "What am I doing?", + "received_benefit": "Jacket", + "provided_benefit": "The Ringo", + "our_participants": "Ringo Starr", + "other_participants": "", + "extra": {"rings": 1}, + }, + { + "id": 8, + "superseded_by_id": 1, + "date": arrow.get(2018, 1, 1).datetime, + "published": arrow.get(2018, 1, 2).datetime, + "edited": arrow.get(2018, 1, 3).datetime, + "title": "The Fellowship of the Ringing Bell", + "body": "The bell is ringing!", + "received_benefit": "bell", + "provided_benefit": "noise", + "our_participants": "", + "other_participants": "", + }, ] @@ -104,6 +145,9 @@ def prepare_reports(): Report.objects.create(author=author1, **reports[2]) Report.objects.create(author=author1, **reports[3]) Report.objects.create(author=author3, **reports[4]) + Report.objects.create(author=author2, **reports[5]) + Report.objects.create(author=author2, **reports[6]) + Report.objects.create(author=author1, **reports[7]) def prepare_author(): diff --git a/tests/schema/snapshots/snap_test_authors.py b/tests/schema/snapshots/snap_test_authors.py index 43fa5c8e6f40bd788bd398abfafade943c68ad92..919b60d05c106addbd3e7b57bdff885d4827ac11 100644 --- a/tests/schema/snapshots/snap_test_authors.py +++ b/tests/schema/snapshots/snap_test_authors.py @@ -17,9 +17,9 @@ snapshots["test_all 1"] = { "extra": None, "firstName": "Shaun", "hasCollidingName": False, - "id": "QXV0aG9yOjM=", + "id": "QXV0aG9yOjI=", "lastName": "Sheep", - "totalReports": 0, + "totalReports": 1, }, }, { @@ -28,9 +28,9 @@ snapshots["test_all 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, }, { @@ -66,9 +66,9 @@ snapshots["test_first 1"] = { "extra": None, "firstName": "Shaun", "hasCollidingName": False, - "id": "QXV0aG9yOjM=", + "id": "QXV0aG9yOjI=", "lastName": "Sheep", - "totalReports": 0, + "totalReports": 1, }, }, { @@ -77,9 +77,9 @@ snapshots["test_first 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, }, ], @@ -104,9 +104,9 @@ snapshots["test_first_after 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, } ], @@ -142,9 +142,9 @@ snapshots["test_last_before 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, } ], @@ -168,19 +168,8 @@ snapshots["test_with_reports 1"] = { "extra": None, "firstName": "Shaun", "hasCollidingName": False, - "id": "QXV0aG9yOjM=", - "lastName": "Sheep", - "reports": {"edges": [], "totalCount": 0}, - "totalReports": 0, - } - }, - { - "node": { - "extra": None, - "firstName": "Spongebob", - "hasCollidingName": False, "id": "QXV0aG9yOjI=", - "lastName": "Squarepants", + "lastName": "Sheep", "reports": { "edges": [ { @@ -206,6 +195,17 @@ snapshots["test_with_reports 1"] = { "totalReports": 1, } }, + { + "node": { + "extra": None, + "firstName": "Spongebob", + "hasCollidingName": False, + "id": "QXV0aG9yOjM=", + "lastName": "Squarepants", + "reports": {"edges": [], "totalCount": 0}, + "totalReports": 0, + } + }, { "node": { "extra": '{"movies": 1}', @@ -270,9 +270,9 @@ snapshots["test_sort_by_last_name 1"] = { "extra": None, "firstName": "Shaun", "hasCollidingName": False, - "id": "QXV0aG9yOjM=", + "id": "QXV0aG9yOjI=", "lastName": "Sheep", - "totalReports": 0, + "totalReports": 1, }, }, { @@ -281,9 +281,9 @@ snapshots["test_sort_by_last_name 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, }, { @@ -330,9 +330,9 @@ snapshots["test_sort_by_last_name_reversed 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, }, { @@ -341,9 +341,9 @@ snapshots["test_sort_by_last_name_reversed 1"] = { "extra": None, "firstName": "Shaun", "hasCollidingName": False, - "id": "QXV0aG9yOjM=", + "id": "QXV0aG9yOjI=", "lastName": "Sheep", - "totalReports": 0, + "totalReports": 1, }, }, ], @@ -377,10 +377,10 @@ snapshots["test_sort_by_total_reports 1"] = { "cursor": "Mg==", "node": { "extra": None, - "firstName": "Spongebob", + "firstName": "Shaun", "hasCollidingName": False, "id": "QXV0aG9yOjI=", - "lastName": "Squarepants", + "lastName": "Sheep", "totalReports": 1, }, }, @@ -388,10 +388,10 @@ snapshots["test_sort_by_total_reports 1"] = { "cursor": "Mw==", "node": { "extra": None, - "firstName": "Shaun", + "firstName": "Spongebob", "hasCollidingName": False, "id": "QXV0aG9yOjM=", - "lastName": "Sheep", + "lastName": "Squarepants", "totalReports": 0, }, }, @@ -415,10 +415,10 @@ snapshots["test_sort_by_total_reports_reversed 1"] = { "cursor": "MQ==", "node": { "extra": None, - "firstName": "Shaun", + "firstName": "Spongebob", "hasCollidingName": False, "id": "QXV0aG9yOjM=", - "lastName": "Sheep", + "lastName": "Squarepants", "totalReports": 0, }, }, @@ -426,10 +426,10 @@ snapshots["test_sort_by_total_reports_reversed 1"] = { "cursor": "Mg==", "node": { "extra": None, - "firstName": "Spongebob", + "firstName": "Shaun", "hasCollidingName": False, "id": "QXV0aG9yOjI=", - "lastName": "Squarepants", + "lastName": "Sheep", "totalReports": 1, }, }, @@ -477,9 +477,9 @@ snapshots["test_sort_by_default_reversed 1"] = { "extra": None, "firstName": "Spongebob", "hasCollidingName": False, - "id": "QXV0aG9yOjI=", + "id": "QXV0aG9yOjM=", "lastName": "Squarepants", - "totalReports": 1, + "totalReports": 0, }, }, { @@ -488,9 +488,9 @@ snapshots["test_sort_by_default_reversed 1"] = { "extra": None, "firstName": "Shaun", "hasCollidingName": False, - "id": "QXV0aG9yOjM=", + "id": "QXV0aG9yOjI=", "lastName": "Sheep", - "totalReports": 0, + "totalReports": 1, }, }, ], diff --git a/tests/schema/snapshots/snap_test_search_reports.py b/tests/schema/snapshots/snap_test_search_reports.py index d817d541ca7c10a66a507c8cadfc013e759d7a3a..6d0f9ce046e7d0d96ba4c8693a7736b2d2ea4d36 100644 --- a/tests/schema/snapshots/snap_test_search_reports.py +++ b/tests/schema/snapshots/snap_test_search_reports.py @@ -41,10 +41,10 @@ snapshots["test_all 1"] = { "node": { "author": { "extra": None, - "firstName": "Spongebob", + "firstName": "Shaun", "hasCollidingName": False, "id": "QXV0aG9yOjI=", - "lastName": "Squarepants", + "lastName": "Sheep", "totalReports": 1, }, "body": "Another long story.", @@ -107,10 +107,10 @@ snapshots["test_query 1"] = { "node": { "author": { "extra": None, - "firstName": "Spongebob", + "firstName": "Shaun", "hasCollidingName": False, "id": "QXV0aG9yOjI=", - "lastName": "Squarepants", + "lastName": "Sheep", "totalReports": 1, }, "body": "Another long story.", diff --git a/tests/test_models.py b/tests/test_models.py index 10469d4d01329b2361deafccadfe9769d174381e..12e54d92581d3cba5544f0d0174107e9bbff942d 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -10,6 +10,8 @@ from openlobby.core.api.schema import ( from openlobby.core.documents import ReportDoc from openlobby.core.models import Report, User, OpenIdClient, LoginAttempt +from .dummy import prepare_reports + pytestmark = [pytest.mark.django_db, pytest.mark.usefixtures("django_es")] @@ -34,6 +36,7 @@ def test_report__is_saved_in_elasticsearch(): date = arrow.get(2018, 1, 1).datetime published = arrow.get(2018, 1, 2).datetime edited = arrow.get(2018, 1, 3).datetime + replacement = Report.objects.create(id=4, author=author, date=date, body="IDDQD") Report.objects.create( id=3, author=author, @@ -48,12 +51,14 @@ def test_report__is_saved_in_elasticsearch(): other_participants="them", extra={"a": 3}, is_draft=False, + superseded_by=replacement, ) docs = list(ReportDoc.search()) - assert len(docs) == 1 - doc = docs[0] + assert len(docs) == 2 + doc = docs[1] assert doc.meta.id == "3" assert doc.author_id == 6 + assert doc.superseded_by_id == 4 assert doc.date == date assert doc.published == published assert doc.edited == edited @@ -141,74 +146,39 @@ def test_user__name_collision_excludes_self_on_update(): assert User.objects.get(username="a").has_colliding_name is False -def test_user__sorted_default(): - User.objects.create( - username="a", is_author=False, first_name="Ryan", last_name="AGosling" - ) - User.objects.create( - username="b", is_author=True, first_name="Ryan", last_name="BGosling" - ) - User.objects.create( - username="c", is_author=False, first_name="Ryan", last_name="CGosling" - ) - assert User.objects.sorted()[0].username == "a" - - -def test_user__sorted_default_reversed(): - User.objects.create( - username="a", is_author=False, first_name="Ryan", last_name="AGosling" - ) - User.objects.create( - username="b", is_author=True, first_name="Ryan", last_name="BGosling" - ) - User.objects.create( - username="c", is_author=False, first_name="Ryan", last_name="CGosling" - ) - assert User.objects.sorted(reversed=True)[0].username == "c" - - -def test_user__sorted_last_name(): - User.objects.create( - username="a", is_author=False, first_name="Ryan", last_name="AGosling" - ) - User.objects.create( - username="b", is_author=True, first_name="Ryan", last_name="BGosling" - ) - User.objects.create( - username="c", is_author=False, first_name="Ryan", last_name="CGosling" - ) - assert User.objects.sorted(sort=AUTHOR_SORT_LAST_NAME_ID)[0].username == "a" - assert ( - User.objects.sorted(sort=AUTHOR_SORT_LAST_NAME_ID, reversed=False)[0].username - == "a" - ) - assert ( - User.objects.sorted(sort=AUTHOR_SORT_LAST_NAME_ID, reversed=True)[0].username - == "c" - ) +@pytest.mark.parametrize( + "params, expected", + [ + ({}, ["Sheep", "Squarepants", "Wolfe"]), + ({"reversed": False}, ["Sheep", "Squarepants", "Wolfe"]), + ({"reversed": True}, ["Wolfe", "Squarepants", "Sheep"]), + ({"sort": AUTHOR_SORT_LAST_NAME_ID}, ["Sheep", "Squarepants", "Wolfe"]), + ( + {"sort": AUTHOR_SORT_LAST_NAME_ID, "reversed": False}, + ["Sheep", "Squarepants", "Wolfe"], + ), + ( + {"sort": AUTHOR_SORT_LAST_NAME_ID, "reversed": True}, + ["Wolfe", "Squarepants", "Sheep"], + ), + ({"sort": AUTHOR_SORT_TOTAL_REPORTS_ID}, ["Wolfe", "Sheep", "Squarepants"]), + ( + {"sort": AUTHOR_SORT_TOTAL_REPORTS_ID, "reversed": False}, + ["Wolfe", "Sheep", "Squarepants"], + ), + ( + {"sort": AUTHOR_SORT_TOTAL_REPORTS_ID, "reversed": True}, + ["Squarepants", "Sheep", "Wolfe"], + ), + ], +) +def test_user__sorted(params, expected): + prepare_reports() + last_names = User.objects.sorted(**params).values_list("last_name", flat=True) + assert list(last_names) == expected -def test_user__sorted_total_reports(): - author = User.objects.create( - username="a", is_author=True, first_name="Ryan", last_name="AGosling" - ) - User.objects.create( - username="b", is_author=True, first_name="Ryan", last_name="BGosling" - ) - date = arrow.get(2018, 1, 1).datetime - Report.objects.create( - id=7, author=author, date=date, published=date, body="Lorem ipsum." - ) - assert User.objects.sorted(sort=AUTHOR_SORT_TOTAL_REPORTS_ID)[0].username == "a" - assert ( - User.objects.sorted(sort=AUTHOR_SORT_TOTAL_REPORTS_ID, reversed=False)[ - 0 - ].username - == "a" - ) - assert ( - User.objects.sorted(sort=AUTHOR_SORT_TOTAL_REPORTS_ID, reversed=True)[ - 0 - ].username - == "b" - ) +def test_user__with_total_reports(): + prepare_reports() + users = User.objects.with_total_reports().values_list("last_name", "total_reports") + assert set(users) == {("Wolfe", 2), ("Sheep", 1), ("Squarepants", 0)}