Metadata form needs to be sticky
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
| Silva |
Medium
|
Wim Boucquaert |
Bug Description
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) wrote : | #1 |
Andy Altepeter (aaltepet) wrote : | #2 |
This is still the case in 2.0 branch. Making a note to try and resolve this for the 2.1 release.
Changed in silva: | |
assignee: | jw-infrae → aaltepet |
Andy Altepeter (aaltepet) wrote : | #3 |
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) wrote : | #4 |
Thisfred, any ideas?
Changed in silva: | |
assignee: | aaltepet → thisfred |
Eric Casteleijn (thisfred) wrote : | #5 |
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.
Changed in silva: | |
assignee: | thisfred → aaltepet |
Andy Altepeter (aaltepet) wrote : | #6 |
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.
One possibility would be to "ajax" this form. When you click the save button, an the metadata is serialized and posted to save_metadata_
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_
Elemenent.
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.
Changed in silva: | |
assignee: | aaltepet → thisfred |
Eric Casteleijn (thisfred) wrote : | #7 |
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) wrote : | #8 |
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.
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)
Changed in silva: | |
assignee: | thisfred → aaltepet |
milestone: | 2.1 → 2.2 |
Eric Casteleijn (thisfred) wrote : | #9 |
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.
Changed in silva: | |
assignee: | aaltepet → todd-infrae |
Andy Altepeter (aaltepet) wrote : | #10 |
I've adjusted the feedback on the metadata tab when validation fails.
Kit Blake (kitblake) wrote : | #11 |
Assigning this to Wim (who'll be putting on the functional test hat).
Changed in silva: | |
assignee: | todd (todd-infrae) → Wim Boucquaert (wim-boucquaert) |
Sylvain Viollon (thefunny) wrote : | #12 |
This is fixed in Silva 3.0 with the new UI.
Changed in silva: | |
milestone: | 2.2 → 3.0.1 |
status: | Confirmed → Fix Released |
Deferred until 0.9.4