From 1184932451994ac1a66e1d8c7406e0b65a02203c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Bedna=C5=99=C3=ADk?= <jan.bednarik@gmail.com>
Date: Sun, 25 Feb 2018 23:51:01 +0100
Subject: [PATCH] Exclude draft Reports from search and totals.

---
 openlobby/core/api/schema.py                |  7 +-
 openlobby/core/api/types.py                 |  5 +-
 openlobby/core/search.py                    |  2 +
 tests/dummy.py                              | 24 ++++---
 tests/schema/snapshots/snap_test_authors.py | 74 +++++++++++++++++++--
 tests/schema/snapshots/snap_test_node.py    |  8 +--
 tests/schema/test_authors.py                | 14 ++--
 tests/schema/test_node.py                   | 15 ++---
 8 files changed, 105 insertions(+), 44 deletions(-)

diff --git a/openlobby/core/api/schema.py b/openlobby/core/api/schema.py
index 7d08210..ffeaa6a 100644
--- a/openlobby/core/api/schema.py
+++ b/openlobby/core/api/schema.py
@@ -1,6 +1,6 @@
 import graphene
 from graphene import relay
-from django.db.models import Count
+from django.db.models import Count, Q
 
 from . import types
 from ..models import OpenIdClient
@@ -25,7 +25,8 @@ class SearchReportsConnection(relay.Connection):
 
 
 def _get_authors_cache(ids):
-    authors = User.objects.filter(id__in=ids).annotate(total_reports=Count('report'))
+    authors = User.objects.filter(id__in=ids)\
+        .annotate(total_reports=Count('report', filter=Q(report__is_draft=False)))
     return {a.id: types.Author.from_db(a) for a in authors}
 
 
@@ -55,7 +56,7 @@ class Query:
 
         total = User.objects.filter(is_author=True).count()
         authors = User.objects.filter(is_author=True)\
-            .annotate(total_reports=Count('report'))\
+            .annotate(total_reports=Count('report', filter=Q(report__is_draft=False)))\
             .order_by('last_name', 'first_name')[
             paginator.slice_from:paginator.slice_to]
 
diff --git a/openlobby/core/api/types.py b/openlobby/core/api/types.py
index c311f39..a6e9c6e 100644
--- a/openlobby/core/api/types.py
+++ b/openlobby/core/api/types.py
@@ -1,4 +1,4 @@
-from django.db.models import Count
+from django.db.models import Count, Q
 from elasticsearch import NotFoundError
 import graphene
 from graphene import relay
@@ -142,7 +142,8 @@ class Author(graphene.ObjectType):
     @classmethod
     def get_node(cls, info, id):
         try:
-            author = models.User.objects.annotate(total_reports=Count('report'))\
+            author = models.User.objects\
+                .annotate(total_reports=Count('report', filter=Q(report__is_draft=False)))\
                 .get(id=id, is_author=True)
             return cls.from_db(author)
         except models.User.DoesNotExist:
diff --git a/openlobby/core/search.py b/openlobby/core/search.py
index 27ef112..77371bc 100644
--- a/openlobby/core/search.py
+++ b/openlobby/core/search.py
@@ -12,6 +12,7 @@ def query_reports(query, paginator, *, highlight=False):
     fields = ['title', 'body', 'received_benefit', 'provided_benefit',
         'our_participants', 'other_participants']
     s = ReportDoc.search()
+    s = s.exclude('term', is_draft=True)
     if query != '':
         s = s.query('multi_match', query=query, fields=fields)
     if highlight:
@@ -23,6 +24,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.filter('term', author_id=author_id)
     s = s.sort('-published')
     s = s[paginator.slice_from:paginator.slice_to]
diff --git a/tests/dummy.py b/tests/dummy.py
index dbdb7dc..e5ce2a0 100644
--- a/tests/dummy.py
+++ b/tests/dummy.py
@@ -63,6 +63,18 @@ reports = [
         'our_participants': 'Aragorn',
         'other_participants': 'Sauron',
     },
+    {
+        'id': 4,
+        'date': arrow.get(2018, 1, 9).datetime,
+        'published': arrow.get(2018, 1, 11).datetime,
+        'title': 'The Silmarillion',
+        'body': 'Not finished yet.',
+        'received_benefit': '',
+        'provided_benefit': '',
+        'our_participants': '',
+        'other_participants': '',
+        'is_draft': True,
+    },
 ]
 
 
@@ -72,13 +84,5 @@ def prepare_reports():
     Report.objects.create(author=author1, **reports[0])
     Report.objects.create(author=author2, **reports[1])
     Report.objects.create(author=author1, **reports[2])
-
-
-def prepare_report():
-    author = User.objects.create(**authors[0])
-    Report.objects.create(author=author, **reports[0])
-
-
-def prepare_authors():
-    for author in authors:
-        User.objects.create(**author)
+    Report.objects.create(author=author1, **reports[3])
+    User.objects.create(**authors[2])
diff --git a/tests/schema/snapshots/snap_test_authors.py b/tests/schema/snapshots/snap_test_authors.py
index b4ca426..5bd5ce2 100644
--- a/tests/schema/snapshots/snap_test_authors.py
+++ b/tests/schema/snapshots/snap_test_authors.py
@@ -30,7 +30,7 @@ snapshots['test_all 1'] = {
                         'hasCollidingName': False,
                         'id': 'QXV0aG9yOjI=',
                         'lastName': 'Squarepants',
-                        'totalReports': 0
+                        'totalReports': 1
                     }
                 },
                 {
@@ -41,7 +41,7 @@ snapshots['test_all 1'] = {
                         'hasCollidingName': False,
                         'id': 'QXV0aG9yOjE=',
                         'lastName': 'Wolfe',
-                        'totalReports': 0
+                        'totalReports': 2
                     }
                 }
             ],
@@ -79,7 +79,7 @@ snapshots['test_first 1'] = {
                         'hasCollidingName': False,
                         'id': 'QXV0aG9yOjI=',
                         'lastName': 'Squarepants',
-                        'totalReports': 0
+                        'totalReports': 1
                     }
                 }
             ],
@@ -106,7 +106,7 @@ snapshots['test_first_after 1'] = {
                         'hasCollidingName': False,
                         'id': 'QXV0aG9yOjI=',
                         'lastName': 'Squarepants',
-                        'totalReports': 0
+                        'totalReports': 1
                     }
                 }
             ],
@@ -150,7 +150,7 @@ snapshots['test_last_before 1'] = {
                         'hasCollidingName': False,
                         'id': 'QXV0aG9yOjI=',
                         'lastName': 'Squarepants',
-                        'totalReports': 0
+                        'totalReports': 1
                     }
                 }
             ],
@@ -169,6 +169,51 @@ snapshots['test_with_reports 1'] = {
     'data': {
         'authors': {
             'edges': [
+                {
+                    'node': {
+                        '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',
+                        'reports': {
+                            'edges': [
+                                {
+                                    'cursor': 'MQ==',
+                                    'node': {
+                                        'body': 'Another long story.',
+                                        'date': '2018-01-05 00:00:00+00:00',
+                                        'extra': '{"rings": 1}',
+                                        'id': 'UmVwb3J0OjI=',
+                                        'otherParticipants': 'Saruman, Sauron',
+                                        'ourParticipants': 'Frodo, Gimli, Legolas',
+                                        'providedBenefit': '',
+                                        'published': '2018-01-10 00:00:00+00:00',
+                                        'receivedBenefit': 'Mithrill Jacket',
+                                        'title': 'The Two Towers'
+                                    }
+                                }
+                            ],
+                            'totalCount': 1
+                        },
+                        'totalReports': 1
+                    }
+                },
                 {
                     'node': {
                         'extra': '{"movies": 1}',
@@ -180,6 +225,21 @@ snapshots['test_with_reports 1'] = {
                             'edges': [
                                 {
                                     'cursor': 'MQ==',
+                                    'node': {
+                                        'body': 'Aragorn is the King. And we have lost the Ring.',
+                                        'date': '2018-01-07 00:00:00+00:00',
+                                        'extra': None,
+                                        'id': 'UmVwb3J0OjM=',
+                                        'otherParticipants': 'Sauron',
+                                        'ourParticipants': 'Aragorn',
+                                        'providedBenefit': 'The Ring',
+                                        'published': '2018-01-08 00:00:00+00:00',
+                                        'receivedBenefit': '',
+                                        'title': 'The Return of the King'
+                                    }
+                                },
+                                {
+                                    'cursor': 'Mg==',
                                     'node': {
                                         'body': 'Long story short: we got the Ring!',
                                         'date': '2018-01-01 00:00:00+00:00',
@@ -194,9 +254,9 @@ snapshots['test_with_reports 1'] = {
                                     }
                                 }
                             ],
-                            'totalCount': 1
+                            'totalCount': 2
                         },
-                        'totalReports': 1
+                        'totalReports': 2
                     }
                 }
             ]
diff --git a/tests/schema/snapshots/snap_test_node.py b/tests/schema/snapshots/snap_test_node.py
index 9ebb2dc..7b2e280 100644
--- a/tests/schema/snapshots/snap_test_node.py
+++ b/tests/schema/snapshots/snap_test_node.py
@@ -19,12 +19,12 @@ snapshots['test_login_shortcut 1'] = {
 snapshots['test_author 1'] = {
     'data': {
         'node': {
-            'extra': '{"x": 1}',
+            'extra': '{"movies": 1}',
             'firstName': 'Winston',
             'hasCollidingName': False,
-            'id': 'QXV0aG9yOjU=',
+            'id': 'QXV0aG9yOjE=',
             'lastName': 'Wolfe',
-            'totalReports': 0
+            'totalReports': 2
         }
     }
 }
@@ -44,7 +44,7 @@ snapshots['test_report 1'] = {
                 'hasCollidingName': False,
                 'id': 'QXV0aG9yOjE=',
                 'lastName': 'Wolfe',
-                'totalReports': 1
+                'totalReports': 2
             },
             'body': 'Long story short: we got the Ring!',
             'date': '2018-01-01 00:00:00+00:00',
diff --git a/tests/schema/test_authors.py b/tests/schema/test_authors.py
index afdf4fc..9de53bc 100644
--- a/tests/schema/test_authors.py
+++ b/tests/schema/test_authors.py
@@ -2,14 +2,14 @@ import pytest
 
 from openlobby.core.models import User
 
-from ..dummy import prepare_authors, prepare_report
+from ..dummy import prepare_reports
 
 
 pytestmark = [pytest.mark.django_db, pytest.mark.usefixtures('django_es')]
 
 
 def test_all(client, snapshot):
-    prepare_authors()
+    prepare_reports()
     User.objects.create(id=4, is_author=False, username='x')
     res = client.post('/graphql', {'query': """
     query {
@@ -39,7 +39,7 @@ def test_all(client, snapshot):
 
 
 def test_first(client, snapshot):
-    prepare_authors()
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {
         authors (first: 2) {
@@ -68,7 +68,7 @@ def test_first(client, snapshot):
 
 
 def test_first_after(client, snapshot):
-    prepare_authors()
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {
         authors (first: 1, after: "MQ==") {
@@ -97,7 +97,7 @@ def test_first_after(client, snapshot):
 
 
 def test_last(client, snapshot):
-    prepare_authors()
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {
         authors (last: 2) {
@@ -126,7 +126,7 @@ def test_last(client, snapshot):
 
 
 def test_last_before(client, snapshot):
-    prepare_authors()
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {
         authors (last: 1, before: "Mw==") {
@@ -155,7 +155,7 @@ def test_last_before(client, snapshot):
 
 
 def test_with_reports(client, snapshot):
-    prepare_report()
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {
         authors {
diff --git a/tests/schema/test_node.py b/tests/schema/test_node.py
index 915b59d..c1ccbf1 100644
--- a/tests/schema/test_node.py
+++ b/tests/schema/test_node.py
@@ -4,7 +4,7 @@ from graphql_relay import to_global_id
 from openlobby.core.auth import create_access_token
 from openlobby.core.models import OpenIdClient, User
 
-from ..dummy import prepare_report
+from ..dummy import prepare_reports
 
 
 pytestmark = [pytest.mark.django_db, pytest.mark.usefixtures('django_es')]
@@ -26,14 +26,7 @@ def test_login_shortcut(client, snapshot):
 
 
 def test_author(client, snapshot):
-    User.objects.create(
-        id=5,
-        is_author=True,
-        openid_uid='TheWolf',
-        first_name='Winston',
-        last_name='Wolfe',
-        extra={'x': 1},
-    )
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {{
         node (id:"{id}") {{
@@ -47,7 +40,7 @@ def test_author(client, snapshot):
             }}
         }}
     }}
-    """.format(id=to_global_id('Author', 5))})
+    """.format(id=to_global_id('Author', 1))})
     snapshot.assert_match(res.json())
 
 
@@ -66,7 +59,7 @@ def test_author__returns_only_if_is_author(client, snapshot):
 
 
 def test_report(client, snapshot):
-    prepare_report()
+    prepare_reports()
     res = client.post('/graphql', {'query': """
     query {{
         node (id:"{id}") {{
-- 
GitLab