diff --git a/CHANGES.rst b/CHANGES.rst index 9e622b54..4180719a 100644 --- a/CHANGES.rst +++ b/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) ---------------- diff --git a/src/collective/collectionfilter/filteritems.py b/src/collective/collectionfilter/filteritems.py index 621751f9..c90c8782 100644 --- a/src/collective/collectionfilter/filteritems.py +++ b/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 diff --git a/src/collective/collectionfilter/query.py b/src/collective/collectionfilter/query.py index 07d68588..98b094f4 100644 --- a/src/collective/collectionfilter/query.py +++ b/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: diff --git a/src/collective/collectionfilter/testing.py b/src/collective/collectionfilter/testing.py index 87443610..7908c652 100644 --- a/src/collective/collectionfilter/testing.py +++ b/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 diff --git a/src/collective/collectionfilter/tests/robot/keywords.robot b/src/collective/collectionfilter/tests/robot/keywords.robot index c0dc206d..36b6396a 100644 --- a/src/collective/collectionfilter/tests/robot/keywords.robot +++ b/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}'] + + diff --git a/src/collective/collectionfilter/tests/robot/test_filterportlets.robot b/src/collective/collectionfilter/tests/robot/test_filterportlets.robot index bbe79f70..7bb2cb79 100644 --- a/src/collective/collectionfilter/tests/robot/test_filterportlets.robot +++ b/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 diff --git a/src/collective/collectionfilter/tests/test_filteritems.py b/src/collective/collectionfilter/tests/test_filteritems.py index 73abafb8..5b3a2a33 100644 --- a/src/collective/collectionfilter/tests/test_filteritems.py +++ b/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) diff --git a/src/collective/collectionfilter/tests/test_maps.py b/src/collective/collectionfilter/tests/test_maps.py index dfc85a01..7612277a 100644 --- a/src/collective/collectionfilter/tests/test_maps.py +++ b/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) diff --git a/src/collective/collectionfilter/vocabularies.py b/src/collective/collectionfilter/vocabularies.py index 68cefa60..73d7a175 100644 --- a/src/collective/collectionfilter/vocabularies.py +++ b/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 = [