From d6ee4271648baf1412016223adae15184842adb4 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Fri, 5 Jul 2019 17:01:27 +0700 Subject: [PATCH 01/12] fix error with collectionfilter=1 not being included in batch links --- .../collectionfilter/contentfilter.py | 2 +- src/collective/collectionfilter/testing.py | 7 ++++ .../tests/robot/keywords.robot | 10 +++++ .../tests/robot/test_filterportlets.robot | 41 +++++++++++++++++-- .../tests/test_filteritems.py | 21 +++++----- .../collectionfilter/tests/test_maps.py | 2 +- 6 files changed, 67 insertions(+), 16 deletions(-) diff --git a/src/collective/collectionfilter/contentfilter.py b/src/collective/collectionfilter/contentfilter.py index 1c8dc612..1a55052a 100644 --- a/src/collective/collectionfilter/contentfilter.py +++ b/src/collective/collectionfilter/contentfilter.py @@ -9,6 +9,6 @@ def set_content_filter(context, event): req = event.request if 'collectionfilter' not in req.form: return - del req.form['collectionfilter'] + # We leave collectionfilter=1 in req.form so that it gets put into batch links content_filter = make_query(req.form) event.request['contentFilter'] = content_filter 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..7e318c27 100644 --- a/src/collective/collectionfilter/tests/robot/keywords.robot +++ b/src/collective/collectionfilter/tests/robot/keywords.robot @@ -74,3 +74,13 @@ 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}'] + +Set Batch Size + [Arguments] ${batch_size} + + Go to ${PLONE_URL}/testcollection/edit + Input text css=input#form-widgets-ICollection-item_count ${batch_size} + Click element css=input#form-buttons-save + Go to ${PLONE_URL}/testcollection + + diff --git a/src/collective/collectionfilter/tests/robot/test_filterportlets.robot b/src/collective/collectionfilter/tests/robot/test_filterportlets.robot index bbe79f70..c5f9db37 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 @@ -41,3 +41,36 @@ Scenario: Add filter portlets to collection #Clear element text css=.collectionSearch input[name='SearchableText'] #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 //div[contains(@class, 'filterContent')]//li[contains(@class, 'filterItem')] 4 + + +Scenario: Test Batching + + Log in as site owner + Go to ${PLONE_URL}/testcollection + + Click element link=Manage portlets + Element should be visible css=#plone-contentmenu-portletmanager > ul + Click element partial link=Right + + Add filter portlet Subject or checkboxes_dropdowns + Go to ${PLONE_URL}/testcollection + Xpath should match X times //article[@class='entry'] 3 + + Set Batch Size 1 + + Xpath should match X times //article[@class='entry'] 1 + + Click element css=li.filter-super.checkbox input + Wait until keyword succeeds 5s 1s Xpath should match X times //article[@class='entry'] 1 + + capture page screenshot + Xpath should match X times //nav[@class='pagination']//a 2 + + Click element xpath=//nav[@class='pagination']//a[1] + Wait until keyword succeeds 5s 1s Xpath should match X times //article[@class='entry'] 1 + capture page screenshot + + Xpath should match X times //nav[@class='pagination']//a 2 + + ${loc}= get location + should contain ${loc} collectionfilter=1 diff --git a/src/collective/collectionfilter/tests/test_filteritems.py b/src/collective/collectionfilter/tests/test_filteritems.py index 73abafb8..5a6e972d 100644 --- a/src/collective/collectionfilter/tests/test_filteritems.py +++ b/src/collective/collectionfilter/tests/test_filteritems.py @@ -23,17 +23,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 +67,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 +100,9 @@ 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") 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) From f371f771989384fd54289e0e66c90cf637fe97b6 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Sat, 6 Jul 2019 18:36:38 +0700 Subject: [PATCH 02/12] extra check to not call make_query again --- src/collective/collectionfilter/contentfilter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/collective/collectionfilter/contentfilter.py b/src/collective/collectionfilter/contentfilter.py index 1a55052a..0e65a431 100644 --- a/src/collective/collectionfilter/contentfilter.py +++ b/src/collective/collectionfilter/contentfilter.py @@ -7,7 +7,7 @@ def set_content_filter(context, event): parameters to narrow the results of the collection. """ req = event.request - if 'collectionfilter' not in req.form: + if 'collectionfilter' not in req.form or 'contentFilter' in req: return # We leave collectionfilter=1 in req.form so that it gets put into batch links content_filter = make_query(req.form) From 3a7b575f45b2fe27b21afa242de17ba4a765dc30 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Mon, 8 Jul 2019 14:56:30 +0700 Subject: [PATCH 03/12] Fix include to ensure permissions imported --- src/collective/collectionfilter/portlets/configure.zcml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/collective/collectionfilter/portlets/configure.zcml b/src/collective/collectionfilter/portlets/configure.zcml index 474075f5..68160355 100644 --- a/src/collective/collectionfilter/portlets/configure.zcml +++ b/src/collective/collectionfilter/portlets/configure.zcml @@ -6,6 +6,8 @@ xmlns:zcml="http://namespaces.zope.org/zcml" i18n_domain="collective.collectionfilter"> + + Date: Tue, 9 Jul 2019 11:05:22 +0700 Subject: [PATCH 04/12] fix unicode error with urls and tests to proof "and" filter_type works correctly --- .../collectionfilter/filteritems.py | 2 +- .../tests/test_filteritems.py | 100 ++++++++++++++++++ 2 files changed, 101 insertions(+), 1 deletion(-) 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/tests/test_filteritems.py b/src/collective/collectionfilter/tests/test_filteritems.py index 5a6e972d..f94c1f15 100644 --- a/src/collective/collectionfilter/tests/test_filteritems.py +++ b/src/collective/collectionfilter/tests/test_filteritems.py @@ -1,7 +1,11 @@ # -*- coding: utf-8 -*- """Setup tests for this package.""" import unittest +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 @@ -106,3 +110,99 @@ def test_portal_type_filter(self): 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) + + + + 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 result + + + # Test url + self.assertEqual(qs(result, u'Süper'), {'Subject': 'S\xc3\xbcper'}) + + 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': ['S\xc3\xbcper', 'Dokum\xc3\xa4nt']}) + self.assertEqual(qs(result, u'Evänt'), {'Subject_op': 'and', 'Subject': ['S\xc3\xbcper', 'Ev\xc3\xa4nt']}) + + # 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': 'Dokum\xc3\xa4nt'}) + self.assertEqual(qs(result, u'Dokumänt'), {'Subject': 'S\xc3\xbcper'}) + self.assertEqual(qs(result, u'Evänt'), {'Subject': ['S\xc3\xbcper', 'Dokum\xc3\xa4nt', 'Ev\xc3\xa4nt'], '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) From fdaa2dd31fb56d77d3b1f54eee1fd68d7b9fdec2 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 9 Jul 2019 11:06:06 +0700 Subject: [PATCH 05/12] style fixes --- .../tests/test_filteritems.py | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/collective/collectionfilter/tests/test_filteritems.py b/src/collective/collectionfilter/tests/test_filteritems.py index f94c1f15..7096b041 100644 --- a/src/collective/collectionfilter/tests/test_filteritems.py +++ b/src/collective/collectionfilter/tests/test_filteritems.py @@ -16,6 +16,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 result + + class TestFilteritems(unittest.TestCase): layer = COLLECTIVE_COLLECTIONFILTER_INTEGRATION_TESTING @@ -124,18 +134,6 @@ def test_and_filter_type(self): self.assertEqual(get_data_by_val(result, u'Evänt')['count'], 1) self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 2) - - - 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 result - - # Test url self.assertEqual(qs(result, u'Süper'), {'Subject': 'S\xc3\xbcper'}) @@ -152,8 +150,6 @@ def qs(result, index): filter_type="and", cache_enabled=False) - - self.assertEqual(len(result), 4) self.assertEqual(get_data_by_val(result, 'all')['count'], 3) @@ -198,7 +194,6 @@ def qs(result, index): self.assertEqual(qs(result, u'Dokumänt'), {'Subject': 'S\xc3\xbcper'}) self.assertEqual(qs(result, u'Evänt'), {'Subject': ['S\xc3\xbcper', 'Dokum\xc3\xa4nt', 'Ev\xc3\xa4nt'], '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, From b23d13f9a0b7f3949d40b40ff01cac609c6b101d Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 9 Jul 2019 13:55:04 +0700 Subject: [PATCH 06/12] add change log --- CHANGES.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index 336fde2b..ea6170c9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,7 +4,8 @@ Changelog 3.2 (unreleased) ---------------- -- Nothing changed yet. +- Fix bug where filter urls was getting utf encoded then made into unicode again + [djay] 3.1 (2019-06-06) From 36dde6e845d4bc4144f47c5bda78069b7d064e01 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 9 Jul 2019 17:12:53 +0700 Subject: [PATCH 07/12] handle import of urlparse for python3 --- src/collective/collectionfilter/tests/test_filteritems.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/collective/collectionfilter/tests/test_filteritems.py b/src/collective/collectionfilter/tests/test_filteritems.py index 7096b041..a98b20e4 100644 --- a/src/collective/collectionfilter/tests/test_filteritems.py +++ b/src/collective/collectionfilter/tests/test_filteritems.py @@ -1,7 +1,10 @@ # -*- coding: utf-8 -*- """Setup tests for this package.""" import unittest -from urlparse import urlparse, parse_qs +try: + from urllib.parse import urlparse, parse_qs +except: + from urlparse import urlparse, parse_qs from plone.app.contenttypes.interfaces import ICollection From 5271a483e910fce21f1706bbec31be44d8f8e344 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Wed, 10 Jul 2019 00:44:58 +0700 Subject: [PATCH 08/12] py3 unicode fix for tests --- .../collectionfilter/tests/test_filteritems.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/collective/collectionfilter/tests/test_filteritems.py b/src/collective/collectionfilter/tests/test_filteritems.py index a98b20e4..5b3a2a33 100644 --- a/src/collective/collectionfilter/tests/test_filteritems.py +++ b/src/collective/collectionfilter/tests/test_filteritems.py @@ -11,6 +11,7 @@ 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): @@ -26,7 +27,7 @@ def qs(result, index): 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 result + return safe_decode(result) class TestFilteritems(unittest.TestCase): @@ -138,7 +139,7 @@ def test_and_filter_type(self): self.assertEqual(get_data_by_val(result, u'Dokumänt')['count'], 2) # Test url - self.assertEqual(qs(result, u'Süper'), {'Subject': 'S\xc3\xbcper'}) + self.assertEqual(qs(result, u'Süper'), {'Subject': u'Süper'}) catalog_results = ICollection(self.collection).results( batch=False, @@ -165,8 +166,8 @@ def test_and_filter_type(self): 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': ['S\xc3\xbcper', 'Dokum\xc3\xa4nt']}) - self.assertEqual(qs(result, u'Evänt'), {'Subject_op': 'and', 'Subject': ['S\xc3\xbcper', 'Ev\xc3\xa4nt']}) + 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 @@ -193,9 +194,9 @@ def test_and_filter_type(self): 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': 'Dokum\xc3\xa4nt'}) - self.assertEqual(qs(result, u'Dokumänt'), {'Subject': 'S\xc3\xbcper'}) - self.assertEqual(qs(result, u'Evänt'), {'Subject': ['S\xc3\xbcper', 'Dokum\xc3\xa4nt', 'Ev\xc3\xa4nt'], 'Subject_op': 'and'}) + 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( From 67da487bfebe2d9bef7dd6a98fd6aa967a13f9a8 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 9 Jul 2019 11:29:19 +0700 Subject: [PATCH 09/12] fix bug in make_query adding operators where not needed (cherry picked from commit c21400e8503f8001c11fd8351b052b1585da5c0a) --- src/collective/collectionfilter/query.py | 14 +++++++++----- src/collective/collectionfilter/vocabularies.py | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) 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/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 = [ From a5bee2a8f633fa0d52e41d8ce52ab15104e1373e Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Thu, 18 Jul 2019 12:01:35 +0700 Subject: [PATCH 10/12] add changelog for operator bug fix --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index ea6170c9..105151a0 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,8 @@ Changelog - 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] 3.1 (2019-06-06) From 1e3996a9e08d465b1f2552f33cd440a5a9caae89 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Thu, 18 Jul 2019 12:14:39 +0700 Subject: [PATCH 11/12] revert batching collectionfilter=1 fix because its in another PR --- .../collectionfilter/contentfilter.py | 2 +- .../tests/robot/keywords.robot | 8 ----- .../tests/robot/test_filterportlets.robot | 33 ------------------- 3 files changed, 1 insertion(+), 42 deletions(-) diff --git a/src/collective/collectionfilter/contentfilter.py b/src/collective/collectionfilter/contentfilter.py index 0e65a431..836efcbc 100644 --- a/src/collective/collectionfilter/contentfilter.py +++ b/src/collective/collectionfilter/contentfilter.py @@ -9,6 +9,6 @@ def set_content_filter(context, event): req = event.request if 'collectionfilter' not in req.form or 'contentFilter' in req: return - # We leave collectionfilter=1 in req.form so that it gets put into batch links + del req.form['collectionfilter'] content_filter = make_query(req.form) event.request['contentFilter'] = content_filter diff --git a/src/collective/collectionfilter/tests/robot/keywords.robot b/src/collective/collectionfilter/tests/robot/keywords.robot index 7e318c27..36b6396a 100644 --- a/src/collective/collectionfilter/tests/robot/keywords.robot +++ b/src/collective/collectionfilter/tests/robot/keywords.robot @@ -75,12 +75,4 @@ Add filter portlet Click element css=.plone-modal-footer input#form-buttons-add Wait until page contains element xpath://div[contains(@class, 'portletAssignments')]//a[text()='${group_criteria}'] -Set Batch Size - [Arguments] ${batch_size} - - Go to ${PLONE_URL}/testcollection/edit - Input text css=input#form-widgets-ICollection-item_count ${batch_size} - Click element css=input#form-buttons-save - Go to ${PLONE_URL}/testcollection - diff --git a/src/collective/collectionfilter/tests/robot/test_filterportlets.robot b/src/collective/collectionfilter/tests/robot/test_filterportlets.robot index c5f9db37..7bb2cb79 100644 --- a/src/collective/collectionfilter/tests/robot/test_filterportlets.robot +++ b/src/collective/collectionfilter/tests/robot/test_filterportlets.robot @@ -41,36 +41,3 @@ Scenario: Add filter portlets to collection #Clear element text css=.collectionSearch input[name='SearchableText'] #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 //div[contains(@class, 'filterContent')]//li[contains(@class, 'filterItem')] 4 - - -Scenario: Test Batching - - Log in as site owner - Go to ${PLONE_URL}/testcollection - - Click element link=Manage portlets - Element should be visible css=#plone-contentmenu-portletmanager > ul - Click element partial link=Right - - Add filter portlet Subject or checkboxes_dropdowns - Go to ${PLONE_URL}/testcollection - Xpath should match X times //article[@class='entry'] 3 - - Set Batch Size 1 - - Xpath should match X times //article[@class='entry'] 1 - - Click element css=li.filter-super.checkbox input - Wait until keyword succeeds 5s 1s Xpath should match X times //article[@class='entry'] 1 - - capture page screenshot - Xpath should match X times //nav[@class='pagination']//a 2 - - Click element xpath=//nav[@class='pagination']//a[1] - Wait until keyword succeeds 5s 1s Xpath should match X times //article[@class='entry'] 1 - capture page screenshot - - Xpath should match X times //nav[@class='pagination']//a 2 - - ${loc}= get location - should contain ${loc} collectionfilter=1 From f428c31c9d5a904484dba216b0a3ea8f4671fb49 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Thu, 18 Jul 2019 12:28:16 +0700 Subject: [PATCH 12/12] missed other reversion --- src/collective/collectionfilter/contentfilter.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/collective/collectionfilter/contentfilter.py b/src/collective/collectionfilter/contentfilter.py index 836efcbc..1c8dc612 100644 --- a/src/collective/collectionfilter/contentfilter.py +++ b/src/collective/collectionfilter/contentfilter.py @@ -7,7 +7,7 @@ def set_content_filter(context, event): parameters to narrow the results of the collection. """ req = event.request - if 'collectionfilter' not in req.form or 'contentFilter' in req: + if 'collectionfilter' not in req.form: return del req.form['collectionfilter'] content_filter = make_query(req.form)