makumba / makumba

Makumba helps you rapidly develop data driven web applications. Provides a custom JSP taglib as a main interface, but leaves API open for advanced access. It is implemented in Java.
https://www.makumba.org
GNU Lesser General Public License v2.1
5 stars 2 forks source link

annotate forms with data validation errors #122

Closed ghost closed 16 years ago

ghost commented 21 years ago

Reported by @cristianbogdan on 7 Jan 2003 10:42 UTC at present, when the users enters wrong data in a form, the validation errors are shown in red in the response page, the user has to press "back", find the respective fields by themselves and correct the errors

it is very fashionable lately on the net to show the form again instead of showing the response page, and write the validation errors near the respective fields.

we need to design:

  1. how will the mak:programmer indicate that they want their form annotated (instead of the current behavior)? see below for a suggestion.
  2. how will the mak:programmer indicate the field to which a validation error refers to?
  3. how will makumba know where in the form to show the annotation for a certain field, and how to format it?

SUGGESTION: to solve (1), a new type of exception (MultipleValidationException) could be thrown, which would contain a list of (field, validation_error) pairs. to solve (2), if a MVE is caught during form response, makumba will automatically show the form page instead of the "action" page didn't think much how to solve (3), i guess a small message just before the input control of the field (i.e. at the begining of the content generated by the respective mak:input) can be good enough

Migrated-From: http://trac.makumba.org/ticket/281

ghost commented 21 years ago

Comment by @frederikhabils on 29 May 2003 11:44 UTC i read this interesting related code example: http://javaalmanac.com/egs/javax.servlet.jsp/myform.jsp.html

that demonstrates a general strategy to achieve annotating forms with error messages (based on javabeans).

I tried out what it could be like for a makumba form: https://parade.best.eu.org/fred-k/lbg/member/memberEditBug281.jsp? person=hhi4y8a

Apart from the more complicated code, bug 451 makes it impossible to do this for all datatypes (date, char, ptr is ok, intEnum is not)

The example also puts the error code on top of the page (actually, its forced in the default header)... didn't try putting the error closer to the wrong value.

ghost commented 20 years ago

Comment by @frederikhabils on 27 Jun 2003 20:42 UTC with bug 451 fixed in 0.5.9.3, I could fix the example mentioned in comment 1. It now shows 'primitive' annotated forms in case of data validation errors.

https://parade.best.eu.org/fred-k/lbg/member/memberEditBug281.jsp?person=hhi4y8a

As I wrote on the page, negatives in this concept:

ghost commented 20 years ago

Comment by @cristianbogdan on 30 Jun 2003 08:43 UTC this java almanac thing is very nice, no time to look into the code now, how can we integrate it in makumba? in other words:

comment 1:

Then the url- in-browser should be edit.jsp, which can not be achieved by having view.jsp forward internally to edit.jsp if there was an error.

how about doing a sendRedirect() to the original form in case of error? the

also, maybe without the redirect, i think there is a http header that tells the browser what to show, simply changing that in the response may achieve the same effect?

i put this bug on "doable by me" list because i was concerned about how this will go with multiple forms (bug 37) but i think doing it for simple forms should be a good first step. should i put it on "doable by others"?

ghost commented 20 years ago

Comment by @frederikhabils on 2 Jul 2003 00:03 UTC comment 3: the first question was more or less answered by comment 0.

I thought about it just now, and without having read comment 0 in any recent months, came to same conclusions. Except for one. I don't believe that the errormessage should be shown by mak:input. It must come from its own tag.

The exception class could look like

DataValidationException extends LogicException { private Map errorMessages;
-> we might decide on a specific Class here instead of Map (e.g. org.makumba.ValidationMessages)

DataValidationException(String s, Map errors) {super(s); errorMessages=errors;}

String getErrorMessage(Object key) { if (errorMessages.containsKey(key)) return errorMessage.get(key); else return ""; } }

Then, form handler would do the dataValidation, and for every error found, add a message to a Map. In the end, if errors were found, put the Map, together with global message, in the DataValidationException and throw it.

then on the jsp side:

the form can have a param : dataValidation="true|false"
(other names: annotateOnError, annotate, validateData, ...)

 true: annotate form in case of error (and if the thrown error is type

DataValidationExc) false : just go to response page on error (default)

then the coding can be:

-> global message

<mak:form action=... dataValidation="true" > -> prints the getErrorMessage("email") email : </mak:form>

PROBLEM: the layout of -> user will want to have specific layout tags only if there is a message, example:

email

(or layout layout </mak:response> )

(probably to be considered together with some of the other bugs that discuss response, successful or not)

how about doing a sendRedirect() to the original form in case of error?

SendRedirect tell the browser to re-GET the specified URL. You would lose the entire pageContext (error messages, exception object ...) by the time control passes back to the form.

If you know a smart header that fixes that, do tell. I haven't heard of anything other than forward() and redirect() (cfr. tip 4 on http://java.oreilly.com/news/jsptips_1100.html)

Finally, reviewing the almanac example linked before, is also interesting for one other thing. In it (they use beans), the JSP page determines the actual error message wording (since it's presentation). Those messages are put in a Map, with keys being numeric codes (number-code, msg). Then validation would (on error) put the numeric code in the errorMap (field, number-code), and then when the annotated form comes back, the getError("fieldX") would translate field -> code -> message.

It adds complication, but improves model/view separation. Still, probably overly complicated for makumba.

ghost commented 20 years ago

Comment by @cristianbogdan on 22 Aug 2003 11:41 UTC

I don't believe that the errormessage should be shown by mak:input. It must come from its own tag.

that's clearly better in terms of mak:user control of the interface.

but i would advocate, for lazy users like me, a <mak:input dataValidationMessage="over/under" dataValidationSeparator="
" > etc.

or even being able to specify the position and separation of the data validation message for the whole form! that would also tell the form that it uses form anotation and it should re-display itself in case of data validation error

also, one can think of data validation printed by hand with <mak:response name="..." printAlsoNearField="false" />: should it be printed also near the field (via dataValidationMessage="over") or just on top of the form (or wherever the user puts it)

i would use instead of but i think names of tags are not so important at this stage.


i guess we could adopt the "action=self and re-direct in case of success" technique for such forms. only thing that worries me is what happens if the user presses "back". then the redirect will take place again, right?

ghost commented 20 years ago

Comment by @stefanb on 21 Oct 2003 11:28 UTC

technique for such forms. only thing that worries me is what happens if the user presses "back". then the redirect will take place again, right? That is only the case with html refresh meta tag, but it works nicely with http "location:" header (aka http redirect)

Let's see...

<mak:input name="..." validationMessage="before|after" validationMessageBefore=""
validationMessageAfter="

" />

It would be way handy to be able to specify this for all fields in a form at once, by putting these tag attributes to the <mak:*form> tag as default for all input controls - but user will be able to override those defaults at each mak:input tag.

ghost commented 16 years ago

Comment by @rudolfmayer on 27 Aug 2007 01:42 UTC Design (and almost finished implementation) from Makumba Congress Vienna

further ideas would be to have a specific mak:error tag, that would allow us to habe fain-grained control on where to show the annotation for each field.

ghost commented 16 years ago

Comment by @rudolfmayer on 28 Aug 2007 14:30 UTC still to be decided is

on idea would be to even after makumba detects some validation errors, the BL gets executed. The MultipleValidationException could be put in the attributes, where the programmer can get it from and add his own exceptions to.

after the BL is executed, the controller would check if some exceptions have been gathered in MultipleValidationException and either proceed normally or reload the form page and annotate

ghost commented 16 years ago

Comment by @rudolfmayer on 28 Aug 2007 14:32 UTC another issue to solve is how to deal with (user-thrown) exceptions that cannot be attributed to a specific form field.

the most logical solution seems to display them as the mak:response. therefore, before the form page is reloaded, all gathered exceptions have to be checked if they can be attributed to a field, and those that can't have to be set in the response before the filter is applied.

ghost commented 16 years ago

Comment by @cristianbogdan on 29 Aug 2007 08:27 UTC (In reply to comment #8)

still to be decided is

  • how to combine makumba detected errors (basically invalid types) with programmer-detected errors (semantic checks).

the BL would need to be rewritten to separate field validity checks from other operations.

on idea would be to even after makumba detects some validation errors, the BL gets executed. The MultipleValidationException could be put in the attributes, where the programmer can get it from and add his own exceptions to.

sounds reasonable. an even cleaner solution is to add a validate_XXXX() method to be called before each on_XXXX BL method, if one exists. then you can simply add the MVException parameter to the method without breaking the current BL APIs. since the BL needs rewriting (or moving code around) for this to work well, putting stuff in a separate method will actually improve the its quality.

ghost commented 16 years ago

Comment by @cristianbogdan on 31 Aug 2007 09:31 UTC (In reply to comment #9)

the most logical solution seems to display them as the mak:response. therefore, before the form page is reloaded, all gathered exceptions have to be checked if they can be attributed to a field, and those that can't have to be set in the response before the filter is applied.

i agree in principle with showing non-field-specific exceptions via mak:response

in my opinion, after the mak validation and validateXXX are run, the onXXX or afterXXX should not run if any of the mak validation or validateXXXX have produced an exception. even running validateXXX may be problematic if the mak:validation has generated problems. it can be ran but any runtime exceptions produced there should be ignored as they are most probably due to the previous problems.

the validateXXX can still produce non-field-specific exceptions. these can be put in the MVE, to be exctracted by the responder and put in the error attribute shown by mak:response.

ghost commented 16 years ago

Comment by @rudolfmayer on 9 Sep 2007 01:10 UTC this is basically implemented with the following attributes

still unsolved is to actually rewrite the URL displayed, but this does not change anything design wise, so it can be fixed once we know how to do it...

ghost commented 16 years ago

Comment by @cristianbogdan on 9 Sep 2007 14:35 UTC great!

still unsolved is to actually rewrite the URL displayed, but this does not change anything design wise, so it can be fixed once we know how to do it...

well, i guess we should file that as a separate bug? rewriting will also be needed for view selection, so maybe we can investigate more, maybe it's a tomcat bug, etc.