Merge pull request #5317 from hypothesis/revert-5309-filters

Revert "Add new HiddenFilter to filter annotations which are moderated or belong to a nipsaed user."

h/presenters/annotation_searchindex.py

@@ -44,14 +44,13 @@ def asdict(self):
         # Mark an annotation as hidden if it and all of it's children have been
         # moderated and hidden.
         result['hidden'] = False
-
-        ann_mod_svc = self.request.find_service(name='annotation_moderation')
-        result['hidden'] = ann_mod_svc.hidden(self.annotation)
-
         if self.annotation.thread_ids:
-            are_all_replies_hidden = len(ann_mod_svc.all_hidden(self.annotation.thread_ids)) == len(self.annotation.thread_ids)
+            ann_mod_svc = self.request.find_service(name='annotation_moderation')
+            annotation_hidden = ann_mod_svc.hidden(self.annotation)
+            thread_ids_hidden = len(ann_mod_svc.all_hidden(self.annotation.thread_ids)) == len(self.annotation.thread_ids)
 
-            result['hidden'] = result['hidden'] and are_all_replies_hidden
+            if annotation_hidden and thread_ids_hidden:
+                result['hidden'] = True
 
         return result
 

h/search/core.py

@@ -48,7 +48,7 @@ def __init__(self, request, separate_replies=False, stats=None, _replies_limit=2
                            query.GroupFilter(),
                            query.GroupAuthFilter(request),
                            query.UserFilter(),
-                           query.HiddenFilter(request),
+                           query.NipsaFilter(request),
                            query.AnyMatcher(),
                            query.TagsMatcher(),
                            query.KeyValueMatcher()]

h/search/query.py

@@ -392,18 +392,19 @@ def __call__(self, search, _):
         return search.exclude("exists", field="deleted")
 
 
-class HiddenFilter(object):
-    """Return an Elasticsearch filter for filtering out moderated or NIPSA'd annotations."""
+class NipsaFilter(object):
+    """Return an Elasticsearch filter for filtering out NIPSA'd annotations."""
 
     def __init__(self, request):
         self.group_service = request.find_service(name='group')
         self.user = request.user
 
     def __call__(self, search, _):
-        """Filter out all hidden and NIPSA'd annotations except the current user's."""
-        # If any one of the "should" clauses is true then the annotation will
-        # get through these filter.
-        should_clauses = [Q("bool", must_not=Q("term", nipsa=True), must=Q("term", hidden=False))]
+        """Filter out all NIPSA'd annotations except the current user's."""
+        # If any one of these "should" clauses is true then the annotation will
+        # get through the filter.
+        should_clauses = [Q("bool", must_not=Q("term", nipsa=True)),
+                          Q("exists", field="thread_ids")]
 
         if self.user is not None:
             # Always show the logged-in user's annotations even if they have nipsa.

tests/h/presenters/annotation_searchindex_test.py

@@ -123,9 +123,7 @@ def test_it_does_not_mark_annotation_hidden_when_not_moderated_and_no_replies(se
         assert annotation_dict['hidden'] is False
 
     def test_it_marks_annotation_hidden_when_moderated_and_no_replies(self, pyramid_request, moderation_service):
-        thread_ids = []
-        annotation = mock.MagicMock(userid='acct:luke@hypothes.is',
-                                    thread_ids=thread_ids)
+        annotation = mock.MagicMock(userid='acct:luke@hypothes.is')
 
         moderation_service.hidden.return_value = True
 

tests/h/search/conftest.py

@@ -6,7 +6,6 @@
 
 import h.search.index
 from h.services.group import GroupService
-from h.services.annotation_moderation import AnnotationModerationService
 
 
 @pytest.fixture(autouse=True)
@@ -17,15 +16,6 @@ def group_service(pyramid_config):
     return group_service
 
 
-@pytest.fixture(autouse=True)
-def moderation_service(pyramid_config):
-    svc = mock.create_autospec(AnnotationModerationService, spec_set=True, instance=True)
-    svc.all_hidden.return_value = []
-    svc.hidden.return_value = False
-    pyramid_config.register_service(svc, name='annotation_moderation')
-    return svc
-
-
 @pytest.fixture
 def Annotation(factories, index):
     """Create and index an annotation.

tests/h/search/index_test.py

@@ -10,11 +10,12 @@
 import pytest
 
 import h.search.index
+from h.services.annotation_moderation import AnnotationModerationService
 
 from tests.common.matchers import Matcher
 
 
-@pytest.mark.usefixtures("annotations")
+@pytest.mark.usefixtures("annotations", "moderation_service")
 class TestIndex(object):
     def test_annotation_ids_are_used_as_elasticsearch_ids(self, es_client,
                                                           factories,
@@ -494,3 +495,12 @@ def _get(annotation_id):
             index=es_client.index, doc_type=es_client.mapping_type,
             id=annotation_id)["_source"]
     return _get
+
+
+@pytest.fixture
+def moderation_service(pyramid_config):
+    svc = mock.create_autospec(AnnotationModerationService, spec_set=True, instance=True)
+    svc.all_hidden.return_value = []
+    svc.hidden.return_value = False
+    pyramid_config.register_service(svc, name='annotation_moderation')
+    return svc

tests/h/search/query_test.py

@@ -623,50 +623,13 @@ def search(self, search):
 
 
 @pytest.mark.usefixtures('pyramid_config')
-class TestHiddenFilter(object):
-
-    @pytest.mark.parametrize('nipsa,hidden,should_show_annotation', [
-        # both nipsa and hidden fields are set, so don't show the annotation
-        (True, True, False),
-        # nipsa field is set, so don't show the annotation
-        (True, False, False),
-        # hidden field is set, so don't show  the annotation
-        (False, True, False),
-        # neither field is set, show  the annotation
-        (False, False, True),
-    ])
-    def test_visibility_of_moderated_and_nipsaed_annotations(
-        self, index, Annotation, pyramid_request, search, user,
-        AnnotationSearchIndexPresenter, nipsa, hidden, should_show_annotation
-    ):
-
-        pyramid_request.user = user
-        search.append_modifier(query.HiddenFilter(pyramid_request))
-        presenter = AnnotationSearchIndexPresenter.return_value
-
-        presenter.asdict.return_value = {'id': 'ann1',
-                                         'hidden': hidden,
-                                         'nipsa': nipsa}
-        Annotation(id='ann1')
-
-        presenter.asdict.return_value = {'id': 'ann2',
-                                         'hidden': False,
-                                         'nipsa': False}
-        Annotation(id='ann2', userid=user.userid)
-
-        expected_ids = ['ann2']
-        if should_show_annotation:
-            expected_ids.append('ann1')
-
-        result = search.run({})
-
-        assert sorted(result.annotation_ids) == sorted(expected_ids)
+class TestNipsaFilter(object):
 
     def test_hides_banned_users_annotations_from_other_users(
         self, pyramid_request, search, banned_user, user, Annotation
     ):
         pyramid_request.user = user
-        search.append_modifier(query.HiddenFilter(pyramid_request))
+        search.append_modifier(query.NipsaFilter(pyramid_request))
         Annotation(userid=banned_user.userid)
         expected_ids = [Annotation(userid=user.userid).id]
 
@@ -678,7 +641,7 @@ def test_shows_banned_users_annotations_to_banned_user(
         self, pyramid_request, search, banned_user, user, Annotation
     ):
         pyramid_request.user = banned_user
-        search.append_modifier(query.HiddenFilter(pyramid_request))
+        search.append_modifier(query.NipsaFilter(pyramid_request))
         expected_ids = [Annotation(userid=banned_user.userid).id]
 
         result = search.run(webob.multidict.MultiDict({}))
@@ -691,7 +654,7 @@ def test_shows_banned_users_annotations_in_groups_they_created(
     ):
         pyramid_request.user = user
         group_service.groupids_created_by.return_value = ["created_by_banneduser"]
-        search.append_modifier(query.HiddenFilter(pyramid_request))
+        search.append_modifier(query.NipsaFilter(pyramid_request))
         expected_ids = [Annotation(groupid="created_by_banneduser",
                                    userid=banned_user.userid).id]
 
@@ -975,10 +938,3 @@ def es_dsl_search(pyramid_request):
         using=pyramid_request.es.conn,
         index=pyramid_request.es.index,
     )
-
-
-@pytest.fixture
-def AnnotationSearchIndexPresenter(patch):
-    AnnotationSearchIndexPresenter = patch('h.search.index.presenters.AnnotationSearchIndexPresenter')
-    AnnotationSearchIndexPresenter.return_value.asdict.return_value = {'test': 'val'}
-    return AnnotationSearchIndexPresenter