Merge branch 'master' into djay/fix_geo_portlets

CHANGES.rst

@@ -4,10 +4,13 @@ Changelog
 3.2 (unreleased)
 ----------------
 
+- Fix bug where filter urls was getting utf encoded then made into unicode again
+  [djay]
+- Fix 5.2 where operators should not be used on all index types
+  [djay]
 - Fix bug where portlets didn't work without GeoLocation dependencies
   [djay]
 
-
 3.1 (2019-06-06)
 ----------------
 

src/collective/collectionfilter/filteritems.py

@@ -184,7 +184,7 @@ def get_filter_items(
                 _urlquery[idx] = filter_value
 
             query_param = urlencode(safe_encode(_urlquery), doseq=True)
-            url = u'/'.join([it for it in [
+            url = '/'.join([it for it in [
                 collection_url,
                 view_name,
                 '?' + query_param if query_param else None

src/collective/collectionfilter/query.py

@@ -24,11 +24,15 @@ def make_query(params_dict):
             crit = idx_mod(crit) if idx_mod else safe_decode(crit)
 
             # filter operator
-            op = params_dict.get(idx + '_op', 'or')
-            if op not in ['and', 'or']:
-                op = 'or'
-            # add filter query
-            query_dict[idx] = {'operator': op, 'query': crit}
+            op = params_dict.get(idx + '_op', None)
+            if op is None:
+                # add filter query
+                query_dict[idx] = {'query': crit}
+            else:
+                if op not in ['and', 'or']:
+                    op = 'or'
+                # add filter query
+                query_dict[idx] = {'operator': op, 'query': crit}
 
     for idx in GEOLOC_IDX:
         if idx in params_dict:

src/collective/collectionfilter/testing.py

@@ -60,6 +60,13 @@ def setUpPloneSite(self, portal):
                 text=RichTextValue(u'Ein heißes Test Dokument'),
                 subject=[u'Süper', u'Dokumänt'],
             )
+            portal.invokeFactory(
+                'Document',
+                id='testdoc2',
+                title=u'Page 😉',
+                text=RichTextValue(u'Ein heißes Test Dokument'),
+                subject=[u'Dokumänt'],
+            )
             doc = portal['testdoc']
             # doc.geolocation.latitude = 47.4048832
             # doc.geolocation.longitude = 9.7587760701108

src/collective/collectionfilter/tests/robot/keywords.robot

@@ -74,3 +74,5 @@ Add filter portlet
     Select from List by value  css=select#form-widgets-input_type  ${input_type}
     Click element  css=.plone-modal-footer input#form-buttons-add
     Wait until page contains element  xpath://div[contains(@class, 'portletAssignments')]//a[text()='${group_criteria}']
+
+

src/collective/collectionfilter/tests/robot/test_filterportlets.robot

@@ -23,14 +23,14 @@ Scenario: Add filter portlets to collection
     Add search portlet
     Add filter portlet  Subject  or  checkboxes_dropdowns
 
-    Click link  css=a.link-parent
-    Xpath should match X times  //article[@class='entry']  2
+    Go to  ${PLONE_URL}/testcollection
+    Xpath should match X times  //article[@class='entry']  3
 
     Click element  css=li.filter-dokumant.checkbox input
-    Wait until keyword succeeds  5s  1s  Xpath should match X times  //article[@class='entry']  1
+    Wait until keyword succeeds  5s  1s  Xpath should match X times  //article[@class='entry']  2
 
     Click element  css=li.filter-all.checkbox input
-    Wait until keyword succeeds  5s  1s  Xpath should match X times  //article[@class='entry']  2
+    Wait until keyword succeeds  5s  1s  Xpath should match X times  //article[@class='entry']  3
 
     Input text  css=.collectionSearch input[name='SearchableText']  Docu
     Wait until keyword succeeds  5s  1s  Xpath should match X times  //article[@class='entry']  1

src/collective/collectionfilter/tests/test_filteritems.py

@@ -1,9 +1,17 @@
 # -*- coding: utf-8 -*-
 """Setup tests for this package."""
 import unittest
+try:
+    from urllib.parse import urlparse, parse_qs
+except:
+    from urlparse import urlparse, parse_qs
 
+from plone.app.contenttypes.interfaces import ICollection
+
+from collective.collectionfilter.query import make_query
 from collective.collectionfilter.testing import COLLECTIVE_COLLECTIONFILTER_INTEGRATION_TESTING  # noqa
 from collective.collectionfilter.filteritems import get_filter_items
+from collective.collectionfilter.utils import safe_decode
 
 
 def get_data_by_val(result, val):
@@ -12,6 +20,16 @@ def get_data_by_val(result, val):
             return r
 
 
+def qs(result, index):
+    url = get_data_by_val(result, index)['url']
+    _, _, _, _, query, _ = urlparse(url)
+    result = parse_qs(query)
+    del result['collectionfilter']
+    # Quick hack to get single values back from being lists
+    result.update(dict([(k, v[0]) for k, v in result.items() if len(v) == 1]))
+    return safe_decode(result)
+
+
 class TestFilteritems(unittest.TestCase):
 
     layer = COLLECTIVE_COLLECTIONFILTER_INTEGRATION_TESTING
@@ -23,17 +41,17 @@ def setUp(self):
         self.collection_uid = self.collection.UID()
 
     def test_filteritems(self):
-        self.assertEqual(len(self.collection.results()), 2)
+        self.assertEqual(len(self.collection.results()), 3)
 
         result = get_filter_items(
             self.collection_uid, 'Subject', cache_enabled=False)
 
         self.assertEqual(len(result), 4)
-        self.assertEqual(get_data_by_val(result, 'all')['count'], 2)
+        self.assertEqual(get_data_by_val(result, 'all')['count'], 3)
         self.assertEqual(get_data_by_val(result, 'all')['selected'], True)
         self.assertEqual(get_data_by_val(result, u'Süper')['count'], 2)
         self.assertEqual(get_data_by_val(result, u'Evänt')['count'], 1)
-        self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 1)
+        self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 2)
 
         result = get_filter_items(
             self.collection_uid, 'Subject',
@@ -67,20 +85,20 @@ def test_filteritems(self):
             get_data_by_val(narrowed_down_result, u'Dokumänt')['selected'], True,  # noqa
             msg=u"Test that 'Dokumänt' is selected, matching the query")
         self.assertEqual(
-            get_data_by_val(narrowed_down_result, u'all')['count'], 2,
-            msg=u"Test that there are 2 Subjects")
+            get_data_by_val(narrowed_down_result, u'all')['count'], 3,
+            msg=u"Test that there are 3 results if unselected")
 
     def test_portal_type_filter(self):
-        self.assertEqual(len(self.collection.results()), 2)
+        self.assertEqual(len(self.collection.results()), 3)
 
         result = get_filter_items(
             self.collection_uid, 'portal_type', cache_enabled=False)
 
         self.assertEqual(len(result), 3)
-        self.assertEqual(get_data_by_val(result, 'all')['count'], 2)
+        self.assertEqual(get_data_by_val(result, 'all')['count'], 3)
         self.assertEqual(get_data_by_val(result, 'all')['selected'], True)
         self.assertEqual(get_data_by_val(result, u'Event')['count'], 1)
-        self.assertEqual(get_data_by_val(result, u'Document')['count'], 1)
+        self.assertEqual(get_data_by_val(result, u'Document')['count'], 2)
 
         result = get_filter_items(
             self.collection_uid, 'portal_type',
@@ -100,8 +118,90 @@ def test_portal_type_filter(self):
 
         self.assertEqual(len(result), 2)
         self.assertEqual(
-            get_data_by_val(result, u'all')['count'], 2,
-            msg=u"Test that the number of portal_types in the collection is 2")
+            get_data_by_val(result, u'all')['count'], 3,
+            msg=u"Test that the number of results if unselected is 3")
+
         self.assertEqual(
             get_data_by_val(result, u'Event')['selected'], True,
             msg=u"Test that Event portal_type is selected matching the query")
+
+    def test_and_filter_type(self):
+        self.assertEqual(len(self.collection.results()), 3)
+
+        result = get_filter_items(
+            self.collection_uid, 'Subject', cache_enabled=False)
+
+        self.assertEqual(len(result), 4)
+        self.assertEqual(get_data_by_val(result, 'all')['count'], 3)
+        self.assertEqual(get_data_by_val(result, 'all')['selected'], True)
+        self.assertEqual(get_data_by_val(result, u'Süper')['count'], 2)
+        self.assertEqual(get_data_by_val(result, u'Evänt')['count'], 1)
+        self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 2)
+
+        # Test url
+        self.assertEqual(qs(result, u'Süper'), {'Subject': u'Süper'})
+
+        catalog_results = ICollection(self.collection).results(
+            batch=False,
+            brains=True,
+            custom_query=make_query(qs(result, u'Süper'))
+        )
+        self.assertEqual(len(catalog_results), 2)
+
+        result = get_filter_items(
+            self.collection_uid, 'Subject',
+            request_params={'Subject': u'Süper'},
+            filter_type="and",
+            cache_enabled=False)
+
+        self.assertEqual(len(result), 4)
+        self.assertEqual(get_data_by_val(result, 'all')['count'], 3)
+
+        # TODO: I'm not sure these counts are correct. It should represent how many results you will get if you click so should be smaller than this
+        # but I guess you need to turn on narrow down for that?
+        self.assertEqual(get_data_by_val(result, u'Süper')['count'], 2)
+        self.assertEqual(get_data_by_val(result, u'Evänt')['count'], 1)
+        self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 2)
+
+        self.assertEqual(get_data_by_val(result, u'Süper')['selected'], True)
+
+        self.assertEqual(qs(result, u'Süper'), {})
+        self.assertEqual(qs(result, u'Dokumänt'), {'Subject_op': 'and', 'Subject': [u'Süper', u'Dokumänt']})
+        self.assertEqual(qs(result, u'Evänt'), {'Subject_op': 'and', 'Subject': [u'Süper', u'Evänt']})
+
+        # Narrow down by 2
+
+        catalog_results = ICollection(self.collection).results(
+            batch=False,
+            brains=True,
+            custom_query=make_query(qs(result, u'Dokumänt'))
+        )
+        self.assertEqual(len(catalog_results), 1)
+
+        result = get_filter_items(
+            self.collection_uid, 'Subject',
+            request_params={'Subject': [u'Süper', u'Dokumänt']},
+            filter_type="and",
+            cache_enabled=False)
+
+        self.assertEqual(len(result), 4)
+        self.assertEqual(get_data_by_val(result, 'all')['count'], 3)
+        self.assertEqual(get_data_by_val(result, u'Süper')['count'], 2)
+
+        self.assertEqual(get_data_by_val(result, u'Evänt')['count'], 1)
+        self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 2)
+
+        self.assertEqual(get_data_by_val(result, u'Süper')['selected'], True)
+        self.assertEqual(get_data_by_val(result, u'Dokumänt')['selected'], True)
+
+        self.assertEqual(qs(result, u'Süper'), {'Subject': u'Dokumänt'})
+        self.assertEqual(qs(result, u'Dokumänt'), {'Subject': u'Süper'})
+        self.assertEqual(qs(result, u'Evänt'), {'Subject': [u'Süper', u'Dokumänt', u'Evänt'], 'Subject_op': 'and'})
+
+        # Clicking on Event we should get 0 results as none will be in common
+        catalog_results = ICollection(self.collection).results(
+            batch=False,
+            brains=True,
+            custom_query=make_query(qs(result, u'Evänt'))
+        )
+        self.assertEqual(len(catalog_results), 0)

src/collective/collectionfilter/tests/test_maps.py

@@ -15,4 +15,4 @@ def setUp(self):
         self.collection = self.portal['testcollection']
 
     def test_locationfilter(self):
-        self.assertEqual(len(self.portal['testcollection'].results()), 2)
+        self.assertEqual(len(self.portal['testcollection'].results()), 3)

src/collective/collectionfilter/vocabularies.py

@@ -116,6 +116,7 @@ def GroupByCriteriaVocabulary(context):
     return SimpleVocabulary(items)
 
 
+# TODO: this should depend on the index type, or be validated against it
 @provider(IVocabularyFactory)
 def FilterTypeVocabulary(context):
     items = [