Skip to content

Commit

Permalink
fix edit/delete for batched crud forms (#8)
Browse files Browse the repository at this point in the history
w/o passing the batch-page to the form-handler, it can not know which
subforms have been rendered and is always using the first batch -
resulting in no items of the other batches being editable or deleteable
  • Loading branch information
frisi committed Sep 18, 2019
1 parent 6cedb1e commit 70dbe39
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 28 deletions.
2 changes: 1 addition & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ New features:

Bug fixes:

- *add item here*
- Fix edit/delete in paginated crud-forms [fRiSi]


1.1.0 (2019-04-09)
Expand Down
70 changes: 44 additions & 26 deletions src/plone/z3cform/crud/README.txt
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ Setup
>>> from plone.z3cform.tests import setup_defaults
>>> setup_defaults()


A simple form
-------------

Expand Down Expand Up @@ -58,8 +59,13 @@ Our simple form looks like this:
This is all that we need to render a combined edit add form containing
all our items:

>>> class FakeContext(object):
... def absolute_url(self):
... return 'http://nohost/context'
>>> fake_context = FakeContext()

>>> from plone.z3cform.tests import TestRequest
>>> print(MyForm(None, TestRequest())()) \
>>> print(MyForm(fake_context, TestRequest())()) \
... # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
<div class="crud-form">...Martha...Peter...</div>

Expand All @@ -81,7 +87,7 @@ form fires for us:
>>> request.form['crud-edit.Peter.widgets.name'] = u'Franz'
>>> request.form['crud-edit.Peter.widgets.age'] = u'16'
>>> request.form['crud-edit.form.buttons.edit'] = u'Apply changes'
>>> html = MyForm(None, request)()
>>> html = MyForm(fake_context, request)()
>>> "Successfully updated" in html
True

Expand All @@ -97,7 +103,7 @@ Two modified events should have been fired:

If we don't make any changes, we'll get a message that says so:

>>> html = MyForm(None, request)()
>>> html = MyForm(fake_context, request)()
>>> "No changes made" in html
True
>>> log
Expand All @@ -122,7 +128,7 @@ Let's rename Martha to Maria. This will give her another key in our
storage:

>>> request.form['crud-edit.Martha.widgets.name'] = u'Maria'
>>> html = MyRenamingForm(None, request)()
>>> html = MyRenamingForm(fake_context, request)()
>>> "Successfully updated" in html
True
>>> log.pop().object == storage['Maria']
Expand All @@ -139,7 +145,7 @@ clicked the 'Apply changes' button:
>>> request.form['crud-edit.Maria.widgets.name'] = u'Maria'
>>> request.form['crud-edit.Maria.widgets.age'] = u'55'
>>> request.form['crud-edit.Maria.widgets.select'] = [u'selected']
>>> html = MyRenamingForm(None, request)()
>>> html = MyRenamingForm(fake_context, request)()
>>> "No changes" in html
True
>>> log
Expand All @@ -148,7 +154,7 @@ clicked the 'Apply changes' button:
And what if we do have changes *and* click the checkbox?

>>> request.form['crud-edit.Maria.widgets.age'] = u'50'
>>> html = MyRenamingForm(None, request)()
>>> html = MyRenamingForm(fake_context, request)()
>>> "Successfully updated" in html
True
>>> log.pop().object == storage['Maria']
Expand All @@ -159,7 +165,7 @@ And what if we do have changes *and* click the checkbox?
If we omit the name, we'll get an error:

>>> request.form['crud-edit.Maria.widgets.name'] = u''
>>> html = MyRenamingForm(None, request)()
>>> html = MyRenamingForm(fake_context, request)()
>>> "There were some errors" in html
True
>>> "Required input is missing" in html
Expand All @@ -180,7 +186,7 @@ clicking the "Delete" button:
>>> request = TestRequest()
>>> request.form['crud-edit.Peter.widgets.select'] = ['selected']
>>> request.form['crud-edit.form.buttons.delete'] = u'Delete'
>>> html = MyForm(None, request)()
>>> html = MyForm(fake_context, request)()
>>> "Successfully deleted items" in html
True
>>> 'Franz' in html
Expand All @@ -199,7 +205,7 @@ Add an item with our form
>>> request.form['crud-add.form.widgets.name'] = u'Daniel'
>>> request.form['crud-add.form.widgets.age'] = u'28'
>>> request.form['crud-add.form.buttons.add'] = u'Add'
>>> html = MyForm(None, request)()
>>> html = MyForm(fake_context, request)()
>>> "Item added successfully" in html
True

Expand All @@ -219,7 +225,7 @@ What if we try to add "Daniel" twice? Our current implementation of
the add form will simply overwrite the data:

>>> save_daniel = storage['Daniel']
>>> html = MyForm(None, request)()
>>> html = MyForm(fake_context, request)()
>>> "Item added successfully" in html
True
>>> save_daniel is storage['Daniel']
Expand All @@ -239,7 +245,7 @@ Let's implement a class that prevents this:
... u"There's already an item with the name '%s'" % name)

>>> save_daniel = storage['Daniel']
>>> html = MyCarefulForm(None, request)()
>>> html = MyCarefulForm(fake_context, request)()
>>> "Item added successfully" in html
False
>>> "There's already an item with the name 'Daniel'" in html
Expand All @@ -262,7 +268,7 @@ wanted the name of our persons to be viewable only in the table:
... view_schema = field.Fields(IPerson).select('name')
... add_schema = IPerson

>>> print(MyAdvancedForm(None, TestRequest())()) \
>>> print(MyAdvancedForm(fake_context, TestRequest())()) \
... # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
<div class="crud-form">...Daniel...Maria...</div>

Expand All @@ -272,7 +278,7 @@ We can still edit the age of our Persons:
>>> request.form['crud-edit.Maria.widgets.age'] = u'40'
>>> request.form['crud-edit.Daniel.widgets.age'] = u'35'
>>> request.form['crud-edit.form.buttons.edit'] = u'Apply Changes'
>>> html = MyAdvancedForm(None, request)()
>>> html = MyAdvancedForm(fake_context, request)()
>>> "Successfully updated" in html
True

Expand All @@ -287,7 +293,7 @@ We can still add a Person using both name and age:
>>> request.form['crud-add.form.widgets.name'] = u'Thomas'
>>> request.form['crud-add.form.widgets.age'] = u'28'
>>> request.form['crud-add.form.buttons.add'] = u'Add'
>>> html = MyAdvancedForm(None, request)()
>>> html = MyAdvancedForm(fake_context, request)()
>>> "Item added successfully" in html
True
>>> len(storage)
Expand All @@ -302,7 +308,7 @@ Our form can also contain links to our items:
... if field == 'name':
... return 'http://en.wikipedia.org/wiki/%s' % item.name

>>> print(MyAdvancedLinkingForm(None, TestRequest())()) \
>>> print(MyAdvancedLinkingForm(fake_context, TestRequest())()) \
... # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
<div class="crud-form">...
...<a href="http://en.wikipedia.org/wiki/Daniel"...
Expand All @@ -318,7 +324,7 @@ What if we wanted the name to be both used for linking to the item
... view_schema = field.Fields(IPerson).select('name')
... add_schema = IPerson

>>> print(MyAdvancedLinkingForm(None, TestRequest())()) \
>>> print(MyAdvancedLinkingForm(fake_context, TestRequest())()) \
... # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
<div class="crud-form">...
...<a href="http://en.wikipedia.org/wiki/Thomas"...Thomas...</a>...
Expand All @@ -334,7 +340,7 @@ Wikipedia link immediately:
>>> request.form['crud-edit.Thomas.widgets.name'] = u'Dracula'
>>> request.form['crud-edit.form.buttons.edit'] = u'Apply Changes'

>>> print(MyAdvancedLinkingForm(None, request)()) \
>>> print(MyAdvancedLinkingForm(fake_context, request)()) \
... # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
<div class="crud-form">...
...<a href="http://en.wikipedia.org/wiki/Dracula"...Dracula...</a>...
Expand All @@ -352,13 +358,13 @@ disappear:

>>> class OnlyEditForm(MyForm):
... addform_factory = crud.NullForm
>>> html = OnlyEditForm(None, TestRequest())()
>>> html = OnlyEditForm(fake_context, TestRequest())()
>>> 'Edit' in html, 'Add' in html
(True, False)

>>> class OnlyAddForm(MyForm):
... editform_factory = crud.NullForm
>>> html = OnlyAddForm(None, TestRequest())()
>>> html = OnlyAddForm(fake_context, TestRequest())()
>>> 'Edit' in html, 'Add' in html
(False, True)

Expand Down Expand Up @@ -397,7 +403,7 @@ than the 'edit' and 'delete' ones:
... return sorted(storage.items(), key=lambda x: x[1].name)

>>> request = TestRequest()
>>> html = MyCustomForm(None, TestRequest())()
>>> html = MyCustomForm(fake_context, TestRequest())()
>>> "Delete" in html, "Apply changes" in html, "Capitalize" in html
(False, False, True)
>>> pprint(storage)
Expand All @@ -407,7 +413,7 @@ than the 'edit' and 'delete' ones:

>>> request.form['crud-edit.Thomas.widgets.select'] = ['selected']
>>> request.form['crud-edit.form.buttons.capitalize'] = u'Capitalize'
>>> html = MyCustomForm(None, request)()
>>> html = MyCustomForm(fake_context, request)()
>>> "Capitalized items" in html
True
>>> pprint(storage)
Expand All @@ -419,7 +425,7 @@ We *cannot* use any of the other buttons:

>>> del request.form['crud-edit.form.buttons.capitalize']
>>> request.form['crud-edit.form.buttons.delete'] = u'Delete'
>>> html = MyCustomForm(None, request)()
>>> html = MyCustomForm(fake_context, request)()
>>> "Successfully deleted items" in html
False
>>> 'Thomas' in storage
Expand Down Expand Up @@ -450,7 +456,7 @@ page template and customize the look of the fields.
... editform_factory = MyCustomEditForm

>>> request = TestRequest()
>>> html = MyCustomFormWithCustomSubForm(None, TestRequest())()
>>> html = MyCustomFormWithCustomSubForm(fake_context, TestRequest())()

Still uses same form as before
>>> "Delete" in html, "Apply changes" in html, "Capitalize" in html
Expand All @@ -470,7 +476,7 @@ as many items displayed per page.
>>> class MyBatchingForm(MyForm):
... batch_size = 2
>>> request = TestRequest()
>>> html = MyBatchingForm(None, request)()
>>> html = MyBatchingForm(fake_context, request)()
>>> "Daniel" in html, "Maria" in html
(True, True)
>>> "THOMAS" in html
Expand All @@ -483,19 +489,31 @@ Make sure, the batch link to the next page is available

Show next page and check content

>>> request = TestRequest(QUERY_STRING='crud-edit.form.page=1')
>>> request.form['crud-edit.form.page'] = '1'
>>> html = MyBatchingForm(None, request)()
>>> html = MyBatchingForm(fake_context, request)()
>>> "Daniel" in html, "Maria" in html
(False, False)
>>> "THOMAS" in html
True

The form action also includes the batch page information so
the correct set of subforms can be processed::

>>> print(html) \
... # doctest: +ELLIPSIS +NORMALIZE_WHITESPACE
<BLANKLINE>
...
<form action="http://nohost/context?crud-edit.form.page=1"
method="post">
...

Let's change Thomas' age on the second page:

>>> request.form['crud-edit.Thomas.widgets.name'] = u'Thomas'
>>> request.form['crud-edit.Thomas.widgets.age'] = '911'
>>> request.form['crud-edit.form.buttons.edit'] = u'Apply changes'
>>> html = MyBatchingForm(None, request)()
>>> html = MyBatchingForm(fake_context, request)()
>>> "Successfully updated" in html
True
>>> "911" in html
Expand Down
2 changes: 1 addition & 1 deletion src/plone/z3cform/crud/crud-table.pt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
tal:condition="view/status" tal:content="view/status">
</div>

<form action="." method="post" tal:attributes="action request/getURL">
<form action="." method="post" tal:attributes="action view/getURL">

<tal:batch tal:replace="structure view/render_batch_navigation" />

Expand Down
21 changes: 21 additions & 0 deletions src/plone/z3cform/crud/crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,27 @@ def selected_items(self):
tuples.append((subform.content_id, subform.content))
return tuples

def getURL(self):
"""Return url of the current page including parameters.
Equivalent to plone_context_state/current_page_url, not using plone
to not need plone stack in testing-setup
"""
current_base_url = self.request.get(
'ACTUAL_URL',
self.request.get(
'VIRTUAL_URL',
self.request.get(
'URL',
self.context.context.absolute_url()
)
)
)
query = self.request.get('QUERY_STRING', None)
if query:
return current_base_url + '?' + query
return current_base_url


class AddForm(form.Form):
template = viewpagetemplatefile.ViewPageTemplateFile('crud-add.pt')
Expand Down

0 comments on commit 70dbe39

Please sign in to comment.