Forum OpenACS Development: ad_form for new and edit requests

Look at acs-person, I discussed these issues with Don and this was the result. In short, ad_form doesn't do everything, just most things.

Don,

We discussed this on IRC. He wants to also set the context and title correctly if there is an error in validation. At that point, the key will be set.

One idea I had was an ad_form_new_p proc that checks for the existence of the key, and also checks for existence of the __new_p variable so that if the request was new, but redirected due to an error in entry, it would still return as a new request. Just checking for 0 in the key won't work for that.

I am thinking of something like this:

if {[ad_form_p -key key_variable_name]} {
    set title for new request
} else {
    set title for edit request
}

ad_form_new_p is now at: patch 485.

Works for me so far.

I noticed your patch was rejected. Do you have a new one?

I am not sure what Don was going to do with ad_form but this is definitly a problem.

I accidently clicked on rejected  instead of accept only. no way to change.  Damn that SDM.

Joel,

Please take a look at /curriculum/lib/element-ave.tcl ("ave" means add-view-edit ... http://cvs.openacs.org/cvs/openacs-4/packages/curriculum/lib/element-ave.adp). It does more or less exactly what you want, I think.

If you have questions on it, I'll try to answer them.

The key issue here is that your "new" proc/function must be provided with the key (e.g, element_id) that ad_form allocated (at your script level) instead of not being provided any, thus having to fetch it itself. The script will need to know the key to be able to redirect to the recently created object's page ...

/Ola

Ola, I've looked through the examples and I'm afraid I still don't understand how the form differentiates between edit mode and view mode.  I can see that the form magically does this, but I don't see how to override it (for example, to make to links to an object, one a view link and one an edit link) or how to get access to this information within the form's code.  I only see where you differentiate between add mode and edit mode.  I see where you figure out that you're in display mode, but that applies to both edit and view, right?

I think I've isolated one of my confusions with ad_form. Ad_form has more actual modes than are documented or named directly controllable, or even described in Ola's last message. It has:

1. displaying form for new entry. Proposed name: new form
2. Handling new mode POST. new post
3. Displaying new mode post with errors (ew form w/errors
4. displaying Edit form for existing entry existing form. May or may not have errors, same mode either way. previously called 'edit'
5. displaying read-only for existing entry existing display, previously called 'display'
6. handling edit mode POST existing post

Did I miss anything? Can we hammer out the actual rules it uses to figure out which mode its in? Then we can make this modes explicit, first in docs, then in standard usage and example code, and then maybe in ad_form itself. A first stab at the rules is

• Was an element_id passed in? (I think this implies that the form defines a 'key' field).
  1. No element_id passed in: we are in New Form.
  2. Yes, element_id passed in. Does the key match a record already in the system (can be tested by ad_form_new_p, but ad_form uses different code to actually determine this?)
     1. No, key is not preexisting: We are in New Post and expecting a submission. Are there any errors?
        1. no validation errors: redirect to the default in aftersubmit (?)
        2. yes, validation errors: we are in New Form w/errors. Display errors in something that looks like mode 1 but now has an id set.
     2. Yes, key already in use: What is the -mode flag for ad_form? (It can be controlled from the url if you code a custom variable to hold it and then pass it in to ad_form, but there is no standard way to do iths)
        1. -mode is Display: we are in Existing Display
        2. -mode is Edit or not set: we are in Existing Edit or Existing Post.

Now can we map this back to the ad_form flags, and maybe straighten out the terminology?

I personally don't see the utility in listing out each possible state rather than listing parallel interacting states independently.

In other words, a submitted form either has errors, or it doesn't, and it doesn't matter if it's a from meant to add a new record or edit an existing one.  In either case the form's  filled with data and error messages which you are intended to edit.  Saying "this is a new form", "this is a new form with errors", "this is an edit form", etc seems more confusing than breaking things down to their fundamentals.

The basic steps in the template form builder, exposed by ad_form, are:

1. Build the initial form, with or without existing data.

2. Submit and verify the form, repeating step one and two with the submitted data if there are errors.

3. Do something with the verified data that has been posted, with new_data and edit_data being shortcuts when the database is involved.

Display mode complicates things.  Forms which display some fields and allow for entry of others complicates things further.  For basic understanding I think both concepts should be avoided in initial documentation and added as "advanced ad_form hacking".

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.

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.  Continously overloading ad_form as has been done over the past few months won't make its use easier to understand, quite the opposite.

We need to 1) simplify the use (something Dave, Jun and I are discussing and plan to work on together in the next few weeks) and extend this functionality to plain old objects.

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.

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.

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

If you don't use ad_form to build a display mode for a single record, what else is as simple (the state stuff notwithstanding?)  Also, there are basically two ways to look at a record: as part of a list (with sort, filter, paginate) and one record at a time (adding, editing, viewing).  So why not have, in the simple case, one declation of data, one for the list, one for the singe record?

I think the holy grail is this (in mangled pseudocode)

In the .sql creation file:

perform content_type__create_type(
    'mt_song',                    -- content_type
    'content_revision',          -- supertype
    'song',                      -- pretty_name,
    'id',                        -- id_column
);

perform content_type__create_attribute(
    'mt_song',                    -- content_type
    'artist',                      -- attribute_name
    'string',                      -- datatype
    'textbox(20)',                -- display mode
    'Artist',                      -- pretty_name
);

song_list.tcl
template::list::create_from_cr -name mt_songs -single_record_page song

song_list.adp:
<listtemplate .../>

song.tcl
template::form::create_from_cr -name mt_songs

song.adp
<formtemplate .../>

And, for added measure, we should automate url-name creation and put the index.vuh in by default so that mysite/song/jump is an alias for mysite/song?title=jump.

<blockquote>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.
</blockquote>

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.

<blockquote>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.
</blockquote>

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

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

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

<blockquote>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.
</blockquote>

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.

<blockquote>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.
</blockquote>

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

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

None.

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

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

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

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,

Let me see if I understand what you're saying...

form builder should be used to add and edit data, but not viewing.

CR/CMS should be used to display data, but currently there's no Tcl API (or is there in bcms-ui?)

Did I get this right? If I did, what's the recommended way to display data that one would perhaps also want to edit, in 5.0?

-Roberto

The one place I *do* like the display capability being enabled on forms is where one wants to disable editing of a particular field while enabling editing of others, as is done in the bug tracker.

Joel, one of the goals that Jun, Dave and I have is to expose the template/cr/cms triad's functionality in a well-designed Tcl API using Jun's stuff as a base.  We should be able to apply a bunch of the template/attribute work to base objects, too.

Auto-generated forms as well as auto-generated display templates are possible using the attribute metadata.  The display code just walks through the attributes and asks the widgets to display them, much as is being done in ad_form's display mode.  Much of this is buried in the CMS package and needs to be exposed ... and Jun's done some work in this area in the BCMS code as well.

Custom forms and display templates will still be with us, though.  That doesn't mean they aren't different and doesn't mean both should be handled by ad_form, though.

To some extent all of this falls apart a bit in the presence of LOBs/CLOBs which need custom handling compared to normal fields and of course differ with each RDBMS.

Anyway, I'm busy at the University of Galileo until the 24th and won't be able to put much time into the whole CR/template/object/metadata/etc problem until then, but intend to work on that and to complete my portals package stuff in March.  Meanwhile Dave and Jun are working on the CR Tcl API stuff as we speak ...

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.

Thinking about this some more I want to make clear (as I said above) that the use of display mode in the bugtracker seems sensible ... my problem is with comments that ad_form become the preferred mechanism for displaying data in the general case.

For instance I don't see why we'd want ad_form to be the mechanism for publishing articles in a CMS.

Having said this I still regret the complexity that's crept into ad_form and the form builder to support a variety of features.  I'm really concerned about the maintainability of the beast.