Discussion:
[Erp5-dev] [patch] Fixing API of Field.render_view and Widget.render_view
Julien Muchembled
2008-04-30 09:34:47 UTC
Permalink
Contrary to {Field,Widget}.render, render_view doesn't take a REQUEST parameter and there are cases where this prevents from accessing 'cell' in TALES. For example:

Let's suppose you have a ListBox with an editable column but you want certain fields not to be editable (by writing a TALES expression for the 'editable' property of the field). If the field is not editable, cell can't be accessed for properties like 'items', and you must write hacks like :
some_expression_will_cell | some_expression_without_cell_that_will_always_contain_the_current_value

What happens:

The behaviour of a listbox is the same for an editable field and a non-editable one so ListBox.py seems ok. In both cases, the following lines are executed:

2136 cell_request = brain.asContext( REQUEST = self.renderer.request
2137 , form = self.renderer.request.form
2138 , cell = brain
2139 )
2140 if editable_field.get_value('enabled', REQUEST=cell_request):
2141 cell_html = editable_field.render( \
2142 value = display_value
2143 , REQUEST = cell_request
2144 , key = key
2145 )
2146 else:
2147 cell_html = ''

with cell_request.cell == brain != None

Then, we have the following calls chain:

* editable field:
File "/home/jm/zope/1/Products/ERP5Form/ListBox.py", line 2150, in render
, key = key
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 69, in Field_render
return self._render_helper(self.generate_field_key(key=key), value, REQUEST)
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 103, in Field_render_helper
return self.widget.render(self, key, value, REQUEST)
File "/home/jm/zope/1/Products/ERP5Form/ProxyField.py", line 75, in __call__
return proxied_method(field, *args, **kw)
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 735, in ListWidget_render
rendered_items = self.render_items(field, key, value, REQUEST)
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 587, in SingleItemsWidget_render_items
items = field.get_value('items', REQUEST=REQUEST, cell=cell)
File "/home/jm/zope/1/Products/ERP5Form/ProxyField.py", line 639, in get_value
return ZMIField.get_value(self, id, **kw)
File "/home/jm/zope/1/Products/ERP5Form/Form.py", line 298, in get_value
return value(field, id, **kw)
File "/home/jm/zope/1/Products/ERP5Form/Form.py", line 167, in __call__
value = self.tales_expr.__of__(field)(**kw)

* non-?ditable field:

File "/home/jm/zope/1/Products/ERP5Form/ListBox.py", line 2145, in render
, key = key
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 69, in Field_render
return self._render_helper(self.generate_field_key(key=key), value, REQUEST)
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 101, in Field_render_helper
return self.widget.render_view(self, value)
File "/home/jm/zope/1/Products/ERP5Form/ProxyField.py", line 75, in __call__
return proxied_method(field, *args, **kw)
File "/home/jm/zope/1/Products/ERP5Form/FormulatorPatch.py", line 755, in ListWidget_render_view
title_list = [x[0] for x in field.get_value("items") if x[1]==value]
File "/home/jm/zope/1/Products/ERP5Form/ProxyField.py", line 639, in get_value
return ZMIField.get_value(self, id, **kw)
File "/home/jm/zope/1/Products/ERP5Form/Form.py", line 294, in get_value
return value(field, id, **kw)
File "/home/jm/zope/1/Products/ERP5Form/Form.py", line 162, in __call__
value = self.tales_expr.__of__(field)(**kw)

The difference is in FormulatorPatch.py, at line 101, where the call to render_view drops out RENDER=cell_request

A solution is to modify the API of render_view to be able to pass a REQUEST parameter, like render.


A strange thing is the comment before the call to brain.asContext (ListBox.py:2136):
# to the field which is being displayed. Since the
# render_view API did not permit this, we pass the line object
# as the REQUEST. But this has side effects since it breaks
# many possibilities. Therefore, the trick is to wrap
# the REQUEST into the brain. In addition, the define a
# cell property on the request itself so that forms may
# use the 'cell' value (refer to get_value method in Form.py)

More precisely: ? Since the render_view API did not permit this, we pass the line object as the REQUEST. ?
Should we read 'render' instead of 'render_view' ?


The following patch fixes {Field,Widget}.render_view so that Listbox's non-editable cells can access 'cell' in TALES:

* add a 'REQUEST' parameter to every render_view:
* one more patch in FormulatorPatch to fix Field.render_view
* for Field.render_view, default value for 'REQUEST' is None
* for widgets, there is no default value
* reorder parameters in OOoChartWidget.render_view
* add a 'REQUEST' parameter to DurationField.render_sub_field_view
* forward REQUEST to field.get_value in:
* ListWidget_render_view (my goal)
* TALESWidget_render_view (why not?)
* PatchedLinkWidget.render_view and
MultiRelationStringFieldWidget.render_view
needn't call get_request anymore if REQUEST isn't None
* PatchedLinkWidget.render_view: change REQUEST.get('cell') into
getattr(REQUEST,'cell',None) since 'cell' may be an attribute of REQUEST


The patch doesn't break any test in ERP5Form/tests and in erp5_pdm_ui_test/erp5_ui_test.

A note about
* PatchedLinkWidget.render_view and
MultiRelationStringFieldWidget.render_view
needn't call get_request anymore if REQUEST isn't None
i.e.:
- REQUEST = get_request()
+ if REQUEST is None:
+ REQUEST = get_request()

Perhaps we don't have to call get_request even if REQUEST is None. Not tested yet though.


JM
-------------- next part --------------
A non-text attachment was scrubbed...
Name: render_view-request-1.patch
Type: text/x-patch
Size: 11293 bytes
Desc: not available
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080430/338d9f70/attachment.bin>
Julien Muchembled
2008-05-14 11:52:37 UTC
Permalink
Post by Julien Muchembled
The patch doesn't break any test in ERP5Form/tests and in erp5_pdm_ui_test/erp5_ui_test.
I ran all tests in ERP5/tests recently and they reveal a few bugs in my patch:
* forgot 2 lines where render_pdf is set to render_view
* MultiItemsWidget.render_view wasn't patched

A business template would also need to be updated now that 'cell' may be None instead of not existing at all:
* erp5_accounting uses "CONTEXTS.get('cell', context)" in the TALES of fields. It wouldn't be correct anymore because we want to use 'context' if 'cell' is None.

Attaching updated patches.

JM
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: render_view-request-2.patch
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080514/4d969507/attachment.asc>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: render_view-request-bt5-2.patch
URL: <http://mail.tiolive.com/pipermail/erp5-dev/attachments/20080514/4d969507/attachment.txt>
Loading...