Commit 1041a64a authored by Romain Courteaud's avatar Romain Courteaud

[erp5_hal_json_style] Manually count result when no count method is defined

Only use hardcoded limit when rendering a listbox.

Always try to fetch the first 1000 documents in such case, and split the sql result in python.
This means that it is not possible to paginate to the 1001th document.
parent 5b6d91c8
...@@ -64,6 +64,7 @@ from Products.ZSQLCatalog.SQLCatalog import Query, ComplexQuery ...@@ -64,6 +64,7 @@ from Products.ZSQLCatalog.SQLCatalog import Query, ComplexQuery
from collections import OrderedDict from collections import OrderedDict
MARKER = [] MARKER = []
COUNT_LIMIT = 1000
if REQUEST is None: if REQUEST is None:
REQUEST = context.REQUEST REQUEST = context.REQUEST
...@@ -1677,6 +1678,18 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1677,6 +1678,18 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# in case we have custom list method # in case we have custom list method
catalog_kw = {} catalog_kw = {}
# form field issuing this search
source_field = portal.restrictedTraverse(form_relative_url) if form_relative_url else None
source_field_meta_type = source_field.meta_type if source_field is not None else ""
if source_field_meta_type == "ProxyField":
source_field_meta_type = source_field.getRecursiveTemplateField().meta_type
is_rendering_listbox = (source_field is not None) and (source_field_meta_type == "ListBox")
count_method = ""
if is_rendering_listbox:
count_method = source_field.get_value('count_method')
has_listbox_a_count_method = (count_method != "") and (count_method.getMethodName() != list_method)
# hardcoded responses for site and portal objects (which are not Documents!) # hardcoded responses for site and portal objects (which are not Documents!)
# we let the flow to continue because the result of a list_method call can # we let the flow to continue because the result of a list_method call can
# be similar - they can in practice return anything # be similar - they can in practice return anything
...@@ -1702,9 +1715,6 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1702,9 +1715,6 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
if query: if query:
catalog_kw["full_text"] = query catalog_kw["full_text"] = query
if limit:
catalog_kw["limit"] = limit
if selection_domain is not None: if selection_domain is not None:
selection_domain_dict = ensureDeserialized( selection_domain_dict = ensureDeserialized(
byteify(json.loads(selection_domain))) byteify(json.loads(selection_domain)))
...@@ -1740,6 +1750,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1740,6 +1750,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
else: else:
catalog_kw['sort_on'] = [parseSortOn(sort_on), ] catalog_kw['sort_on'] = [parseSortOn(sort_on), ]
if limit:
catalog_kw["limit"] = limit
if is_rendering_listbox and not has_listbox_a_count_method:
# When rendering a listbox without count method, add a dummy manual count
# by fetching more documents than requested
catalog_kw["limit"] = [0, COUNT_LIMIT]
# Some search scripts impertinently grab their arguments from REQUEST # Some search scripts impertinently grab their arguments from REQUEST
# instead of being nice and specify them as their input parameters. # instead of being nice and specify them as their input parameters.
# #
...@@ -1760,19 +1777,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1760,19 +1777,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
elif same_type(select_list, ""): elif same_type(select_list, ""):
select_list = [select_list] select_list = [select_list]
# form field issuing this search
source_field = portal.restrictedTraverse(form_relative_url) if form_relative_url else None
# extract form field definition into `editable_field_dict` # extract form field definition into `editable_field_dict`
editable_field_dict = {} editable_field_dict = {}
url_column_dict = {} url_column_dict = {}
listbox_form = None listbox_form = None
listbox_field_id = None listbox_field_id = None
source_field_meta_type = source_field.meta_type if source_field is not None else ""
if source_field_meta_type == "ProxyField":
source_field_meta_type = source_field.getRecursiveTemplateField().meta_type
if source_field is not None and source_field_meta_type == "ListBox": if is_rendering_listbox:
listbox_field_id = source_field.id listbox_field_id = source_field.id
listbox_form = getattr(traversed_document, source_field.aq_parent.id) listbox_form = getattr(traversed_document, source_field.aq_parent.id)
...@@ -1807,8 +1818,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1807,8 +1818,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
editable_field_dict[select] = proxy_form.get_field(proxy_field_name, include_disabled=1) editable_field_dict[select] = proxy_form.get_field(proxy_field_name, include_disabled=1)
# handle the case when list-scripts are ignoring `limit` - paginate for them # handle the case when list-scripts are ignoring `limit` - paginate for them
if limit is not None and isinstance(limit, (tuple, list)): if limit is not None:
start, num_items = map(int, limit) if isinstance(limit, (tuple, list)):
start, num_items = map(int, limit)
elif isinstance(limit, int):
start, num_items = 0, limit
else:
start, num_items = 0, int(limit)
if len(search_result_iterable) <= num_items: if len(search_result_iterable) <= num_items:
# the limit was most likely taken into account thus we don't need to slice # the limit was most likely taken into account thus we don't need to slice
start, num_items = 0, len(search_result_iterable) start, num_items = 0, len(search_result_iterable)
...@@ -1969,14 +1985,11 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1969,14 +1985,11 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# Compute statistics if the search issuer was ListBox # Compute statistics if the search issuer was ListBox
# or in future if the stats (SUM) are required by JIO call # or in future if the stats (SUM) are required by JIO call
source_field_meta_type = source_field.meta_type if source_field is not None else ""
if source_field_meta_type == "ProxyField":
source_field_meta_type = source_field.getRecursiveTemplateField().meta_type
# Lets mingle with editability of fields here # Lets mingle with editability of fields here
# Original Listbox.py modifies editability during field rendering (method `render`), # Original Listbox.py modifies editability during field rendering (method `render`),
# which is done on frontend side, so we overwrite result's own editability # which is done on frontend side, so we overwrite result's own editability
if source_field is not None and source_field_meta_type == "ListBox": if is_rendering_listbox:
editable_column_set = set(name for name, _ in source_field.get_value("editable_columns")) editable_column_set = set(name for name, _ in source_field.get_value("editable_columns"))
for line in result_dict['_embedded']['contents']: for line in result_dict['_embedded']['contents']:
for select in line: for select in line:
...@@ -1986,11 +1999,9 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -1986,11 +1999,9 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
if isinstance(line[select], dict) and 'field_gadget_param' in line[select]: if isinstance(line[select], dict) and 'field_gadget_param' in line[select]:
line[select]['field_gadget_param']['editable'] = False line[select]['field_gadget_param']['editable'] = False
if source_field is not None and source_field_meta_type == "ListBox":
# Trigger count method if exist # Trigger count method if exist
# XXX No need to count if no pagination # XXX No need to count if no pagination
count_method = source_field.get_value('count_method') if has_listbox_a_count_method:
if count_method != "" and count_method.getMethodName() != list_method:
count_kw = dict(catalog_kw) count_kw = dict(catalog_kw)
# Drop not needed parameters # Drop not needed parameters
count_kw.pop('selection', None) count_kw.pop('selection', None)
...@@ -2007,6 +2018,8 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None, ...@@ -2007,6 +2018,8 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# and just pass. This also ensures we have compatibilty with how old # and just pass. This also ensures we have compatibilty with how old
# UI behave in these cases. # UI behave in these cases.
log('Invalid count method %s' % error, level=800) log('Invalid count method %s' % error, level=800)
else:
result_dict['_embedded']['count'] = ensureSerializable(len(search_result_iterable))
contents_stat_list = [] contents_stat_list = []
# in case the search was issued by listbox we can provide results of # in case the search was issued by listbox we can provide results of
......
...@@ -1300,6 +1300,8 @@ return '%s/Base_viewMetadata?reset:int=1' % context.getRelativeUrl() ...@@ -1300,6 +1300,8 @@ return '%s/Base_viewMetadata?reset:int=1' % context.getRelativeUrl()
self.portal.foo_module.FooModule_viewFooList.listbox.ListBox_setPropertyList( self.portal.foo_module.FooModule_viewFooList.listbox.ListBox_setPropertyList(
field_url_columns = '') field_url_columns = '')
self.assertEqual(result_dict['_embedded']['count'], 1)
@simulate('Base_getRequestUrl', '*args, **kwargs', @simulate('Base_getRequestUrl', '*args, **kwargs',
'return "http://example.org/bar"') 'return "http://example.org/bar"')
@simulate('Base_getRequestHeader', '*args, **kwargs', @simulate('Base_getRequestHeader', '*args, **kwargs',
...@@ -1571,6 +1573,8 @@ return context.getPortalObject().portal_catalog(portal_type='Foo', sort_on=[('id ...@@ -1571,6 +1573,8 @@ return context.getPortalObject().portal_catalog(portal_type='Foo', sort_on=[('id
"""Test that count method also uses the query parameters. """Test that count method also uses the query parameters.
""" """
fake_request = do_fake_request("GET") fake_request = do_fake_request("GET")
# Reset the listbox
self.portal.foo_module.FooModule_viewFooList.listbox.ListBox_setPropertyList()
result = self.portal.web_site_module.hateoas.ERP5Document_getHateoas( result = self.portal.web_site_module.hateoas.ERP5Document_getHateoas(
REQUEST=fake_request, REQUEST=fake_request,
mode="search", mode="search",
...@@ -1613,6 +1617,60 @@ return context.getPortalObject().portal_catalog(portal_type='Foo', sort_on=[('id ...@@ -1613,6 +1617,60 @@ return context.getPortalObject().portal_catalog(portal_type='Foo', sort_on=[('id
self.assertEqual(len(result_dict['_embedded']['contents']), 1) self.assertEqual(len(result_dict['_embedded']['contents']), 1)
self.assertEqual(result_dict['_embedded']['count'], 1) self.assertEqual(result_dict['_embedded']['count'], 1)
@simulate('Base_getRequestUrl', '*args, **kwargs', 'return "http://example.org/bar"')
@simulate('Base_getRequestHeader', '*args, **kwargs', 'return "application/hal+json"')
@createIndexedDocument(quantity=2)
@changeSkin('Hal')
def test_getHateoas_length_as_count_method(self, document_list):
"""Test that erp5 listbox manually count result length.
"""
fake_request = do_fake_request("GET")
# Reset the listbox
# self.portal.foo_module.FooModule_viewFooList.listbox.ListBox_setPropertyList()
self.portal.foo_module.FooModule_viewFooList.listbox.ListBox_setPropertyList(
field_count_method = '')
result = self.portal.web_site_module.hateoas.ERP5Document_getHateoas(
REQUEST=fake_request,
mode="search",
relative_url='foo_module',
list_method="searchFolder",
form_relative_url='portal_skins/erp5_ui_test/FooModule_viewFooList/listbox'
)
result_dict = json.loads(result)
self.assertEqual(len(result_dict['_embedded']['contents']), 2)
self.assertEqual(result_dict['_embedded']['count'], 2)
# Check that limiting the number of result doesn't impact count
fake_request = do_fake_request("GET")
result = self.portal.web_site_module.hateoas.ERP5Document_getHateoas(
REQUEST=fake_request,
mode="search",
relative_url='foo_module',
list_method="searchFolder",
query='portal_type:"Foo"',
limit=1,
form_relative_url='portal_skins/erp5_ui_test/FooModule_viewFooList/listbox'
)
result_dict = json.loads(result)
# There is a count method on this listbox
self.assertEqual(len(result_dict['_embedded']['contents']), 1)
self.assertEqual(result_dict['_embedded']['count'], 2)
# Check that query parameters are passed to the count method
fake_request = do_fake_request("GET")
result = self.portal.web_site_module.hateoas.ERP5Document_getHateoas(
REQUEST=fake_request,
mode="search",
relative_url='foo_module',
list_method="searchFolder",
query='id:"%s"' % self.portal.foo_module.contentIds()[0],
form_relative_url='portal_skins/erp5_ui_test/FooModule_viewFooList/listbox'
)
result_dict = json.loads(result)
# There is a count method on this listbox
self.assertEqual(len(result_dict['_embedded']['contents']), 1)
self.assertEqual(result_dict['_embedded']['count'], 1)
class TestERP5Person_getHateoas_mode_search(ERP5HALJSONStyleSkinsMixin): class TestERP5Person_getHateoas_mode_search(ERP5HALJSONStyleSkinsMixin):
"""Test HAL_JSON operations on cataloged Persons and other allowed content types of Person Module.""" """Test HAL_JSON operations on cataloged Persons and other allowed content types of Person Module."""
......
...@@ -50,7 +50,7 @@ a damn about limits so it is perfect adept for testing. ...@@ -50,7 +50,7 @@ a damn about limits so it is perfect adept for testing.
<td>${listbox}//a[@data-i18n="Next"]</td><td></td></tr> <td>${listbox}//a[@data-i18n="Next"]</td><td></td></tr>
<tr><td>assertElementNotPresent</td><!-- "Next" link must be enabled --> <tr><td>assertElementNotPresent</td><!-- "Next" link must be enabled -->
<td>${listbox}//a[@data-i18n="Next" and contains(@class, "ui-disabled")]</td><td></td></tr> <td>${listbox}//a[@data-i18n="Next" and contains(@class, "ui-disabled")]</td><td></td></tr>
<tal:block tal:define="pagination_configuration python: {'header': '(1 - 3)', 'footer': 'Records 1 - 3'}"> <tal:block tal:define="pagination_configuration python: {'header': '(1 - 3 / 8)', 'footer': 'Records 1 - 3 / 8'}">
<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/check_listbox_pagination_text" /> <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/check_listbox_pagination_text" />
</tal:block> </tal:block>
...@@ -61,7 +61,7 @@ a damn about limits so it is perfect adept for testing. ...@@ -61,7 +61,7 @@ a damn about limits so it is perfect adept for testing.
<td>${listbox}//a[@data-i18n="Next"]</td><td></td></tr> <td>${listbox}//a[@data-i18n="Next"]</td><td></td></tr>
<tr><td>assertElementNotPresent</td><!-- "Next" link must be enabled --> <tr><td>assertElementNotPresent</td><!-- "Next" link must be enabled -->
<td>${listbox}//a[@data-i18n="Next" and contains(@class, "ui-disabled")]</td><td></td></tr> <td>${listbox}//a[@data-i18n="Next" and contains(@class, "ui-disabled")]</td><td></td></tr>
<tal:block tal:define="pagination_configuration python: {'header': '(4 - 6)', 'footer': 'Records 4 - 6'}"> <tal:block tal:define="pagination_configuration python: {'header': '(4 - 6 / 8)', 'footer': 'Records 4 - 6 / 8'}">
<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/check_listbox_pagination_text" /> <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/check_listbox_pagination_text" />
</tal:block> </tal:block>
...@@ -72,7 +72,7 @@ a damn about limits so it is perfect adept for testing. ...@@ -72,7 +72,7 @@ a damn about limits so it is perfect adept for testing.
<td>${listbox}//a[@data-i18n="Next"]</td><td></td></tr> <td>${listbox}//a[@data-i18n="Next"]</td><td></td></tr>
<tr><td>assertElementPresent</td><!-- "Next" link must be disabled because we are at the end --> <tr><td>assertElementPresent</td><!-- "Next" link must be disabled because we are at the end -->
<td>${listbox}//a[@data-i18n="Next" and contains(@class, "ui-disabled")]</td><td></td></tr> <td>${listbox}//a[@data-i18n="Next" and contains(@class, "ui-disabled")]</td><td></td></tr>
<tal:block tal:define="pagination_configuration python: {'header': '(7 - 8)', 'footer': 'Records 7 - 8'}"> <tal:block tal:define="pagination_configuration python: {'header': '(7 - 8 / 8)', 'footer': 'Records 7 - 8 / 8'}">
<tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/check_listbox_pagination_text" /> <tal:block metal:use-macro="here/Zuite_CommonTemplateForRenderjsUi/macros/check_listbox_pagination_text" />
</tal:block> </tal:block>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment