Trying to modify the notes package to use ad_form, I stumbled
over the following problem: how does one deal with pages that need to
distinguish wether the current request is
and item-edit or an item-add operation, while displaying the form?

It is of course possible to distinguish that in -edit_data and
-new_data, but they won't be called until the form was sucessfully
committed.

What if I need to set the page title depending on the request type to
"Edit Item" or "New Item", like currently in
packages/notes/www/add-edit.tcl? Also that page does a permission check
before displaying the form - when editing a note it checks for write
on that object, when adding a new one it checks for create on the
package. Can that be done with the current ad_form?

The trick Jon's using in acs-person/www/add-edit.tcl is to define the key as an optional variable in ad_page_contract, as well as the key for ad_form.  Then you can check for its existence to tell whether or not you're building an edit form or new data form before calling ad_form.

I've thought about adding request blocks similar to the submit blocks but I'm concerned about ad_form becoming overly complex and therefore hard to learn to use.  It's quite complex already.

Also ... for the example you're describing - and Jon's example, too - the action (set a page title, set a context bar) doesn't logically belong with the form building code.  ad_form is building a form within a page, it makes sense that you'd have page-building code generating template information that's not part of the form outside the ad_form call.

But ... I'm open to creative ideas as to how it can be extended to make it easier to use!

The trick that Jon uses does not work when the validation fails.

Steps to reproduce: go to acs-person, click "New
Person". The context bar says "Create Person", correct. Enter nothing
and hit the submit button, to produce an invalid form. Now the context bar button says: "Edit Person". This happens because
ad_form has pre-generated an acs_person_id and add-edit.tcl checks for
the existence of that key.

Side note: there is also a bug in the code that sets the page_title, which results in setting it always to "Create Person".

Dave, you meant to write if { [ad_form_new_p -key ...] }, right? Yes that would work, I'll try to write such a procedure later (if you are not faster ...) and see if that would cover my use cases.

Don mailed me that he doesn't like the key-guessing code in
ad_form_new_p. He's right, it's an ugly kludge (it loops through all
available variables, and if there is only one with a name ending in
"_id" then it assumes that one is the key). The problem is that with
the current separation of ad_form and ad_page_contract I don't see an
easy solution for a better key guessing in ad_form_new_p.

Here's what happens currently when using ad_form for new and edit requests in one file:

A new request is usually at first a GET without the key
defined. E.g. linked to from from another page, like this:
<href="note-add-edit">new note</a>.

When entering some values in the new form, and doing a submit that
does not validate, e.g. because a non-optional field was left empty,
then the new form will be redisplayed with an error message next to
the offending fields. This redisplay will be a POST request, with the
key variable set to a pregenerated value, __key_signature will be set
to a non-empty value (not really sure about that), and __new_p will be
set to 1.


An edit request will initially be a GET with the key variable set.

Upon redisplay because of a validation error it will be a POST with
the key variable set as well, and __new_p set to 0 (don't know about
__key_signature here either).

I don't see an easy way to find out if it's a new form without explicitely knowing the name of the key variable, so I will just make -key a required parameter for ad_form_new_p for now.

Rejected? No, it's in CVS already. Jeff even merged it into HEAD, so it's in both branches. I am just preparing one where -key is required.

Could somebody explain how to accomplish the following behavior.  It feels just out of reach with ad_form.

0) a page called "object-edit"

1) A single block of code that includes
  a) sql code or tcl api calls to retrieve a record, update a record, or create a record
  b) description of the fields to display and the html elements to use

2) browsing to "object-edit" displays a form to add an object, with a button labelled "Add" (or using the acs-kernel message key for Add).
2a) After clicking "Add", page redirects to "object-edit?id=47" and shows the object in a read-only version of the form

3) browsing to "object-edit?id=47" shows object 47 in read-only mode

4) browsing to "object-edit?id=47&mode=edit" shows object 47 in edit mode

Sorry. That was the .adp part.

This is a better page:

http://cvs.openacs.org/cvs/openacs-4/packages/curriculum/lib/element-ave.tcl

Hey Joel,

This is a little confusing at times, I know ... You could think about it in the following way: The form builder (and consequently ad_form) has two modes; "edit" and "display". The script has two modes or blocks; "new" and ... well "not new". The "new" block always renders the form in edit mode but the "not new" block renders the form in either edit or display mode.

What the code does first is it determines if a new element should be added or if one already exists (the if/else clause above ad_form). What decides this is whether the element_id query var is provided or not in the request. If the script enters the "new" mode - remember that this is not a form builder mode - the form will go into edit mode. This is the default and would've been used even if I hadn't explicitly told ad_form to use the edit mode.

Now, if in the page request you provide an element_id, and the page is not in "new" mode but in the other mode which I haven't named yet , the form will go into either edit mode or display mode depending on what you tell ad_form. If you don't tell ad_form anything it will default to edit mode (and an editable form will be rendered). But since there is a -mode switch to ad_form you may instruct the form to enter display mode if you so desire (and the form will be rendered read-only).

When the form is rendered in display mode (read-only) an "Edit" button appears below the displayed data (unless the -has_edit switch is being used) and, if pressed, the behavior of form builder is to _force_ itself into edit mode so that you may edit your data.

In case you want to provide a way to choose whether to render the form (for an existing element) read-only or not (the two links example you gave) you can do so by accepting a separate (optional) query var which you could call "mode" or "read_only_p" (say), and let the presence (or absence) of that var have an influence on the $form_mode var in the "not new" block of the script.

(In the element-ave.tcl script I apparently defined the "mode" var for a similar purpose, but as far as I can see it's never used anywhere in my script ...)

Does this make it clearer or dimmer?

/Ola

I'm probably going to work on some additional documentation for ad_form this week while here at University of Galileo.

One problem we face is that three different people have munged with ad_form (at least).  There's the original, relatively simple ad_form that was sketchily but still somewhat comprehensively documented in the Tcl header, and since then a bunch of stuff's been added that wasn't even documented there, much less at the user level ...

Anyway you're summary is useful.  I have to admit that despite being the original author of ad_form, I don't know when I should use "-display", for instance (a feature added by someone else) rather than a confirm template.

My purpose in going to a greater level of detail is that I feel ad_form is currently a "leaky abstraction." It hides details, but not consistently enough, so that its behavior is unpredictable. For example, I wrote code that behaved worked fine until a data field didn't validate, and then ad_form was essentially in a new mode, and it crashed. So I want to drive down past the abstraction, make sure we get it right, and then come back up with a tighter abstraction.

Re: display mode - I think we should go in the opposite direction. That is, we should use ad_form as the recommended, simplest method of displaying single records in detail view. My reasoning is, it's most of the way there already and it's the right approach. The logic for determining which fields to display and in what mode is usually very similar for both edit and display mode. And while ad_form is already pretty complicated, I think that's because of its history and because it's stuck in-between - in particular, in between functional style and declarative style. In some parts you simply describe what you want to see, and in others you have to know the right order of things and what will and won't get pre-processed and when.

Another approach I want to put on the table is creating ad_form2, where we take the best of ad_form and take a clean conceptual approach.

Please see my post earlier in this thread for an enumeration of things I think we should be able to do with ad_form.

BTW I guess I'm now on record as not really liking Ola's approach with the display mode stuff.  I will look at his example to see if I can force myself to love it ...

Template widgets have always known how to display themselves, but that's because they're used in two contexts by the templating system:  form handling for the creation and editing of data, and the displaying of data by the templating system.

Both of which can be auto-generated using the type attribute system.

When I wrote ad_form I never imagined that we'd try to combine the two POVs maintained by the templating system into one humongous wrapper.  My only intention was to simplify the form handling side of things.

Well, while there is no free lunch, display mode in the form builder does give you a lot for free, I guess. For example - not having to write a separate, non-form, object display page in addition to the add/edit page - and consistency in layout among package implementors for another. After all, the data the form handles is _usually_ the same data you want to present, so why not take advantage of this? I kind of like those qualities which follows from Lars' augmenting of the FB. This is not to say I'm not going to like the CR approach taken in CMS, if that ends up the norm. I just haven't seen that stuff in action, I might love it if I do.

The principles in the example script that I pointed to in my first response to Joel has actually been copied, more or less verbatim, from code I found in bug-tracker so please don't let me take any of the glory (and none of the shame)!

There is one distingtive way in which my style of writing form scripts - in the curriculum package, at least - differs from the style used in Bug Tracker, though. BT uses two pages, one for adding a bug and one for displaying/editing it. I figured much would be won (although simplicity would be lost) if I could merge them into one instead, and not have to maintain two basically identical ad_form definitions ... But that's just my oppinion, of course.

Also, by putting the form script under lib/ rather than www/ you'll acheive a better flexibility, I think, in terms of where you can include the form snippet, or, more interestingly, the -display mode piece.

Hell, If someone thinks the element-ave.tcl script that is linked to above is getting hairy you almost definitely do not want to look at curriculum-ave.tcl. There's no two ways about it - the added complexity that comes with a workflow'ed package that's got some fields which are editable while some fields are not, and where all three functions (add, view, edit) are mixed together, really begins to become a problem.


Don't worry Don. Building forms _have_ become _much_ more straight-forward and less error-prone since you created ad_form compared to form builder proper, let alone writing forms and errorhandling, etc, by hand! So, if warranted (and documented) there should still be room for one or two spiffy features, IMHO.

/Ola

>Displaying a record is not the same as building a form, and
>for the life of me I can't see why ad_form would be the
>desired mechanism for doing this.

Displaying a record involves many of the same steps as building a form, especially building a form for edit:
- retrieve a record
- do secondary lookups
- check access permissions

Ad_form already does this, and already has a display mode for showing records that have just been edited.

>I think the templating approach taken by the CR is much
>better for displaying records ... and the CMS/CR +
>templating engine can already build default display
>templates from attribute data.

Is this something we can do with 5.0.0 core content-repository or in the current bcms-ui?

>Continously overloading ad_form as has been done over the
>past few months won't make its use easier to understand,
>quite the opposite.

No, but extending it (or a similar new function) into a more complete abstraction that handles the different paths would make it easier.

>Your approach to documenting it would confuse me, and I
>wrote it! :)  Iterating a matrix of states into a one long
>string of  single-dimensional states is just confusing.

The matrix is the obvious next step, but ad_form doesn't have a matrix of states.  It has (as far as I can tell) a linear algorithm to figure out what to do, and I just tried to document its existing behavior as the first step to documenting the ideal set of states for an edit/display/new function.

I think the dimensions of the matrix should include:
Record status:
  New
  existing record

Behavior:
  display form for new record
  fetch record and display read-only
  fetch record and display for editing
  processes incoming POST and either make changes or display errors

I don't yet see a third dimension in the ideal model but there are some more dimensions in the ad_form matrix because of how it determines what to do next.

>I'd like to see the example you wrote that crashed when a
>field didn't validate.  That sounds like a sloppy
>implementation of some option ... AFAIK that behavior was
>not possible in ad_form before folks started piling on more
>and more special featurettes.

No doubt.  I'll post it in this thread.

>I'm also curious as to which features you'd leave out in an "ad_form2" ...

None.

>Both of which can be auto-generated using the type attribute system.

Is this something that exists now or a new approach we are considering?

Well, my overall reaction is that my nice, simple ad_form idea has been fubar'd beyond belief.  I had no idea that people were turning into a general record display mechanism.

No wonder no one - including me - understands how it works any more!

For the record, Ola ... the WHOLE IDEA behind the form template builder is to combine add/edit scripts into one.  ad_form makes it very easy to do so as it understands the form builder's state information, simplifying things.  So I'm glad you've taken this approach though folding in display, too - YUK.  GROSS.  DISGUSTING.

I'm depressed that folks are writing separate add and edit/view scripts.  The benefit of being able to share a single form declaration for both add and edit functions should be obvious.  This sharing's 90% of the point of taking the form builder approach.  Apparently the adding of the display functionality has made combining add/edit scripts along with display functionality enough of a hassle that people are giving up on that, as has Joel in his example.

There are many fundamental differences between a template that displays data and a form handling page.  The FORM tag comes to mind among other things ...

I called it "ad_form" for a reason ...

Don, can you explain how this functionality can be more easily coded in 5.0.0, using form template builder?

Since everyone else seems to be adding features to ad_form willy-nilly I decided to add a minor one that helps fix a problem with submissions that have errors ...

I've added an "-on_validation_error" block executed when ... well, it is up to you to guess when it is executed.

Here's an example:

ad_form ... -new_request {
    permission::require_permission -object_id $package_id -privilege create
    set page_title "New Note"
} -edit_request {
    permission::require_permission -object_id $note_id -privilege write
    set page_title "Edit Note"
    db_1row note_select {}
} -on_validation_error {
    set page_title "Error in submission"
} -new_data {

Well...the form builder was designed to handle forms and their submission, the very name makes its intended purpose clear.

If it were me I'd probably have written something called "ad_display" that among other things might give me more control over the display format than "ad_form" in display mode does, rather than hack on the form builder and ad_form ...

At some point, when you kludge everthing including the kitchen sink into a single snippet of software, it reaches the point where it becomes nearly impossible to understand, document and maintain.

Hi,

A little OT: the display and usage of CR + templates, etc. is already there all time.  What I just did was to create for myself some procs that I can use to make it easy for me.  So there is nothing new in bcms or bcms-ui-base, its just something more simplified.  This is the same fact that making form was already there prior to ad_form.

Going back to the topic.  I will have to agree what Dave has said several post above.  If ad_form default behaviour is something can't handle then use the form builder.  I do this practice for 2 things:

- I just need a simple form, normally for making none acs objects.  Just a form.  I use the form builder because I want it to use the global form template.  Example are like a search form.  It just needs inputs, it does not create, edit, etc. any object.

- The form processing is very different from ad_form.

I think ad_form should be used 90% of the time.  And that advance form handling should be in the form builder.  I think the main problem is that we would like ad_form to handle 100% of form handling processing.  I think that is possible, but I would prefer that it should be used in the normal way of handling single objects.  Other than that the developer should use the lower layer implementation which is the form builder.

Don't know about the holy grail but my neighbor has an attack rabbit you can have!