Metadata form needs to be sticky

[Kit Blake]

If a validation error occurs on submit (such as an invalid email address in
silve-extra) all data in the form is lost.

[Guido Wesdorp (guido-infrae)]

Deferred until 0.9.4

[Andy Altepeter (aaltepet)]

This is still the case in 2.0 branch. Making a note to try and resolve this for the 2.1 release.

[Andy Altepeter (aaltepet)]

OK, not sure where to go here. The standard pattern for a submitted formulator form is to call field.render, which, if REQUEST is passed in, will render the user-submitted value.

The metadata system doesn't work that way, so you can use this feature of Formulator. My thought is to manually retrieve the value from REQUEST, and pass that to renderEdit...just not sure how to really go about this yet.

[Kit Blake (kitblake)]

Thisfred, any ideas?

[Eric Casteleijn (thisfred)]

Not really. I don't want to touch anything in SilvaMetadata if we don't absolutely *have* to, since almost every change there escalates, so if we can do this with a fix in the SMI in Silva it'd be great, if we have to change SilvaMetadata, I'd like to re-evaluate.

[Andy Altepeter (aaltepet)]

I've just re-confirmed that it isn't possible to provide a fix for this in the SMI -- the fix will likely have to go into SilvaMetadata to adjust the API of Element.renderEdit and Binding.renderElementEdit to accept a value as a parameter. These methods currently only use the already saved metadata. As thisfred suggests, we need to reevaluate.

One possibility would be to "ajax" this form. When you click the save button, an the metadata is serialized and posted to save_metadata_validate. The response to this would be any fields with errors and what those errors are. Of course, this means that validation happens twice, since it would also need to be validated within save_metadata for security reasons.

The alternative is to adjust SilvaMetadata. The key problem here is that SilvaMetadata's renderEdit code doesn't know if there is was an error saving in this transaction, and doesn't have the currently value for the element from the REQUEST. Here's one possibility to fix this:

all_errors will contain a dictionary of the fields with errors. This is passed to tab_metadata and could be accessible from macro_metadata_edit (the pt that actually renders a set). If all_errors is not empty, it can be assumed that the current transaction was an attempt to save the metadata, and some values in the request did not pass validation. macro_edit_metadata(SMI) gets the metadata from a helper script, get_metadata(SMI). This script could accept a parameter, fromRequest, which if true indicates that the metadata should be rendered from the values in REQUEST rather than from the saved metadata. get_metadata calls Binding.renderElementEdit. renderElementEdit could accept the request object, which if present will be passed to Element.renderEdit instead of the old/saved metadata value.
Elemenent.renderEdit will be changed to accept two values:
renderEdit(value, REQUEST)
both of these parameters will be passed to the formulator fields render method, which has the following docstring:
        if value and REQUEST are both None, the 'default' property of
        the field will be used for the value.
This is why the saved data will not be sent if there was an error saving.

Thoughts? I understand you're concern about escalation with SilvaMetadata. The changes I'm proposing should not affect the default behavior of SilvaMetadata, and the changes should be well documented, including addition of testing suites.

[Eric Casteleijn (thisfred)]

Ok, I think that sounds like a reasonable solution, so I think we (you ;) should do this. Again, preferably on the trunk, but if it only needs SilvaMetadata changes, it would be excellent if we could make it work with trunk and 2.1 (and 2.0 maybe, even)

[Andy Altepeter (aaltepet)]

Since it's a change in SilvaMetadata, I applied this to the trunk and not the 2.1 branch.

In SM, I only needed to change Binding.renderElementEdit, I didn't have to go as deep as I was originally expecting.

I don't know how to write functional tests, but we should probably have some tests for this new functionality. Who should write them? I'm fine if it's me, I just haven't before.

Additionally, the error message on the metadata screen needs to change (I can do that)

[Eric Casteleijn (thisfred)]

Cool! I will ask functional testmaster Todd to add a functional test for this, so that you can see how it's done for future reference.

[Andy Altepeter (aaltepet)]

I've adjusted the feedback on the metadata tab when validation fails.

[Kit Blake (kitblake)]

Assigning this to Wim (who'll be putting on the functional test hat).

[Sylvain Viollon (thefunny)]

This is fixed in Silva 3.0 with the new UI.