Forum OpenACS Development: ad_form for new and edit requests

Collapse
Posted by Tilmann Singer on
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?

Collapse
Posted by Jon Griffin on
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.
Collapse
Posted by Don Baccus on
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!

Collapse
Posted by Dave Bauer on
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
}

Collapse
Posted by Tilmann Singer on
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.

Collapse
Posted by Tilmann Singer on
ad_form_new_p is now at: patch 485.

Works for me so far.

Collapse
Posted by Tilmann Singer on
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.

Collapse
Posted by Jon Griffin on
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.

Collapse
Posted by Tilmann Singer on
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.
Collapse
Posted by Jeff Davis on
I accidently clicked on rejected  instead of accept only. no way to change.  Damn that SDM.
Collapse
Posted by Joel Aufrecht on
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

Collapse
Posted by Ola Hansson on
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

Collapse
Posted by Ola Hansson on
Sorry. That was the .adp part.

This is a better page:

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

Collapse
Posted by Joel Aufrecht on
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?
Collapse
Posted by Ola Hansson on
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

Collapse
Posted by Joel Aufrecht on
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?
Collapse
Posted by Don Baccus on
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.

Collapse
Posted by Don Baccus on
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".

Collapse
Posted by Joel Aufrecht on
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.

Collapse
Posted by Don Baccus on
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" ...

Collapse
Posted by Don Baccus on
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.

Collapse
Posted by Joel Aufrecht on
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.

Collapse
Posted by Jade Rubick on
How far away are we from the holy grail? God that would be really nice...
Collapse
Posted by Joel Aufrecht on
<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?

Collapse
Posted by Joel Aufrecht on
Okay, here's an example of a view/edit page built on ad_form. A functional description would be:
  • Record: word
  • Fields
    • word:text
    • locale:lookup to ad_locale.
      • default to system locale for new records.
    • ipa_phonetic:text
    • local_phonetic:text
      • only show this field if word's locale has a "local phonetic alphabet" defined.
      • If creating a new word, determine based on system locale
      • Field title is dynamic, based on locale

And the code (actual running code; I've just omitted some irrelevant bits)

ad_page_contract {
    
    Form for editing individual words
    
} {
    id:integer,optional
    {return_url ""}
}

set page_title "Edit"
set context [list $page_title]
set package_id [ad_conn package_id]

# catch missing local_phonetic when submitting from a form
# where that field was hidden
if {![exists_and_not_null local_phonetic] } {
    set local_phonetic ""
}

set working_locale [lang::user::locale]
# pre-query for the object's locale, which can override working_locale
# yes, we're querying the database twice for the same thing
# that's still faster than hacking ad_form
if { [exists_and_not_null id]} {
    set working_locale [db_string get_locale "
                            select locale
                              from vocab_word
                             where id = :id
" -default ""]
}

set phonetic_alphabet [vocab::phonetic_alphabet_for_locale -locale $working_locale]
set locale_options [vocab::locale_list -all all]
set form_mode [ad_decode [ad_form_new_p -key id] 1 "edit" "display"]

ad_form \
    -name word \
    -mode $form_mode \
    -form {

	{id:key}
	{word:text 
	    {label Word}
	}
	{locale:text(select)
	    {label Locale}
	    {options $locale_options}
	}
	{ipa_phonetic:text,optional
	    {label "IPA"}
	}
    }

if { ![string equal $phonetic_alphabet none]} {
    ad_form -extend -name word -form {
	{local_phonetic:text,optional
	    {label "[set phonetic_alphabet]"}
	}
    }
}

ad_form -extend -name word \
     -new_request {
	set page_title "Add a Word"
	set context [list $page_title]
        # this pre-sets the form locale
        set locale $working_locale

    } -edit_request {

	db_1row word_select {
	    select word,
                   locale,
                   ipa_phonetic,
                   local_phonetic
              from vocab_word
             where id = :id
	}
	set page_title "$word"
	set context [list $page_title]

        # this may be redundant with the prefetch
	set working_locale $locale
	set phonetic_alphabet [vocab::phonetic_alphabet_for_locale -locale $working_locale]

    } -new_data {

	set user_id [auth::require_login]
	set new_id [db_string word_insert "
        select vocab_word__new(:package_id, :word, :locale, :ipa_phonetic, :local_phonetic, :user_id,null) 
    "]

	if {![exists_and_not_null return_url]} {
	#TODO: new_id is the wrong thing to redirect to, so this breaks
	#    set return_url [export_vars -base word-edit {{id $new_id}}]
	    set return_url "."
	}
	ad_returnredirect $return_url

    } -edit_data {

	auth::require_login
	db_dml word_edit {
	    update vocab_word
	       set word   = :word,
    	           locale = :locale,
	           ipa_phonetic = :ipa_phonetic,
	           local_phonetic = :local_phonetic
	     where id = :id
	}

    } -after_submit {

	ad_returnredirect $return_url
	ad_script_abort

    }

Collapse
Posted by Ola Hansson on
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

Collapse
Posted by Dave Bauer on
Joel,

You have the key in your last post. The templating system controls display of an object, not ad_form, which is only mean t to be a wrapper to quickly build forms.

All the time you will run into issues that ad_form does not handle, and will need to use the standard form builder and templating procedures to get the job done.

Collapse
Posted by Don Baccus on
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 ...

Collapse
Posted by Joel Aufrecht on
Don, can you explain how this functionality can be more easily coded in 5.0.0, using form template builder?
Collapse
Posted by Roberto Mello on
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

Collapse
Posted by Don Baccus on
set form_mode [ad_decode [ad_form_new_p -key id] 1 "edit" "display"]
See, now I can't even understand the code being written and I wrote ad_form_new_p and ad_form! You seem to be saying set the form to edit mode if it is a new request, display otherwise but apparently it acts as a view script if ad_form_new_p returns false?

I must be missing something.

And, yes, the template mapping to content type code works, you can even map your (ugh) ad_form-driven display template to it if you want ...

Is it easy to use at the moment? No. I'm getting more and more motivated with each post in this thread to help Jun and Daveb make this stuff easy to use in the next couple of months, though ...

Collapse
Posted by Don Baccus on
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 ...

Collapse
Posted by Don Baccus on
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 {
Collapse
Posted by Don Baccus on
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.

Collapse
Posted by Jeff Davis on
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.
Then you start adding things from in the yard and left overs from thanksgiving and name it "ad_form"...

:), well sort of anyway.

I do think one problem with having ad_form be the primary display mechanism for objects is that designers have very little control over presentation. Obviously in some contexts (objects that are dynamically defined) there is no easy way to do that, but for things where the fields are know it's probably better to have a simple select and display tcl/adp pair.

Collapse
Posted by Jun Yamog on
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.

Collapse
Posted by Don Baccus on
Don't know about the holy grail but my neighbor has an attack rabbit you can have!
Collapse
Posted by Don Baccus on
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.