fixup! [erp5_crm] Activate actions for ERP5JS

bt5/erp5_crm/SkinTemplateItem/portal_skins/erp5_crm/Ticket_updateNewEventDialog.py

@@ -35,5 +35,11 @@ if notification_message:
   request.form['your_text_content'] = temp_event.getTextContent()
   request.form['your_content_type'] = temp_event.getContentType()
   request.form['your_resource'] = temp_event.getResource()
+  # BBB for legacy UI
+  request.set('your_notification_message', '')
+  request.set('your_title', title)
+  request.set('your_text_content', temp_event.getTextContent())
+  request.set('your_content_type', temp_event.getContentType())
+  request.set('your_resource', temp_event.getResource())
 
 return context.Base_renderForm(dialog_id)

[Jérome Perrin]

I also noticed this bug and I was working on fixing it. At the moment I am still updating the tests, so that we have test coverage for this also in erp5_xhtml_style.

I was wondering how to fix, Base_renderForm supports a keep_items argument that would also work for erp5_xhtml_style, but not for ERP5JS.

The description of keep_items does not seem correct, so I'm a bit confused here.

> :param keep_items: {dict} items to be available in the next call. They will be either added as hidden fields to the
>                   rendered form or in case of "portal_status_message" just displayed to the user

/cc @romain

[Jérome Perrin]

At the moment I am still updating the tests, so that we have test coverage for this also in erp5_xhtml_style.

see !1397 (merged)

[Romain Courteaud]

Hum. It is a bit hard for me to believe xhtml was broken without doing request.set, as I probably manually tested it. And I said to people to use request.form in their fast input to be compatible with both UI, and I heard it was correctly working like this.

Doing both ways as is will only lead to not maintainable code. We should introduce an API to stop directly modifying the REQUEST and use it everywhere. Modifying the REQUEST directly must be a coding crime.

[Romain Courteaud]

Base_renderForm propagate the keep_items to ERP5Document_getHateoas as extra_param_json, which is set to the REQUEST

But not in form mode. We could probably had this behaviour to form mode.

And get rid of the direct REQUEST manipulation in the crm fast input.

[Jérome Perrin]

Thanks !

It's also a bit mysterious for me. Event_viewCreateResponseDialog / Event_updateCreateResponse have the same logic of using request.form, but it works there. This seem to be depending on the "Form enctype" from the dialog's Settings in ZMI. When it's set to multipart/form-data this trick only work with request.set(k, v), but not request.form[k] = v (edit: in legacy UI). This behaviour might be something new. I thought both ways were equivalent.

I like this idea of having a higher level API and keep_items seems to be OK for this. So the usage here would be something like this, right ?

keep_items = {
  'your_notification_message': '',
  'your_title': title,
  'your_text_content': temp_event.getTextContent(),
  'your_content_type': temp_event.getContentType(),
  'your_resource': temp_event.getResource(),
}

return context.Base_renderForm(dialog_id, keep_items=keep_items)

[Romain Courteaud]

Yes. It seems much cleaner like this.

We should probably add new tests reproducing the different "Form enctype" configurations on a Foo document to ensure Base_renderForm will work in both UI.

[Jérome Perrin]

Thanks. I don't think I can work on this in the short time, but at least we have an API.

About "Form enctype", it reminds me we have this guideline erp5-Guideline.Set.Form.Enctype.To.Multipart.Form-data.When.Using.File.Uploads

The form enctype is multipart/form-data ( actually only important if you have file uploads )

the first module guidelines was saying "always use multipart/form-data". We added "only with file uploads", probably because we did not want to change all the forms. I don't really know what would be wrong to always use multipart/form-data.

Anyway, for a first step, having a test for each enctype might be the best

[Romain Courteaud]

Please add the crm tests for xhtml and keep the scripts as is for now if enough.

I'll add the removal of @kazuhiko fixup on my todo list to make the changes/tests for Base_renderForm

[Jérome Perrin]

Please add the crm tests for xhtml and keep the scripts as is for now if enough.

Thanks, the tests are in !1397 (merged) I'll finalize it as we discussed tomorrow.