Forum OpenACS Development: Re: ad_form for new and edit requests

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

    }