Home
The Toolkit for Online Communities
17716 Community Members, 1 member online, 2119 visitors today
Log In Register
OpenACS Home : Forums : OpenACS Q&A : E-commerce 'category-browse-subcategory.tcl'

Forum OpenACS Q&A: E-commerce 'category-browse-subcategory.tcl'

Icon of envelope Request notifications

There is a problem with this code at the moment and I thought I should post here for feedback before logging as a bug and hopefully proposing a fix.

I have installed a clean e-commerce from HEAD and have added a  single category, several subcategories and no subsubcategories. I have added a single product to one of the subcategories which is 'In stock', 'available', and set to be displayed on user search.

If I do a product search, or view 'ALL' products as admin, I see the product as expected - no problem.

However, if browsing the subcategory, no product is displayed. The reason appears to be that the following query returns no rows. The reason that query returns no rows is that it left outer joins to ec_user_session_offer_codes which contains no rows.

I am not clear what causes the ec_user_session_offer_codes to be populated but I presume it to be connected with the process of putting things on sale.

The query is:

  <fullquery name="get_regular_product_list">
    <querytext>
      select p.product_id, p.dirname, p.product_name, p.one_line_description, p.sku, o.offer_code
      from $product_map($sub) m, ec_products_searchable p left outer join ec_user_session_offer_codes o on
    (p.product_id = o.product_id and user_session_id = :user_session_id)
      where p.product_id = m.product_id
      and m.${sub}category_id = :${sub}category_id
      $exclude_subproducts
      order by p.product_name limit :how_many offset :start_row
    </querytext>
  </fullquery>

Whilst waiting for comment I will compare with code from a working 4.6.3 system in search of enlightenment! :-)

Regards
Richard

Collapse
Posted by Torben Brosten on
Ricky,

Try browsing to ecommerce/admin and refreshing the db cache.. see if it becomes available after that.

Collapse
Posted by Richard Hamilton on
Torben, thanks for the reply.

I tried that first of all. Then I connected to the db using psql and issued the queries direct against the tables. With the left outer join to an empty table you get zip back! :-)

R.

Collapse
Posted by Richard Hamilton on
......although of course logically that should not be happening! :-|
Collapse
Posted by Richard Hamilton on
OK, apologies from Bozo central, the left inner join thing is a red herring - caused by a typo in my test query! :-(

However, I still have the problem.

So I have flushed the cache and I still have the problem. Product exists and can be searched and displayed, but the category browse function doesn't work. So this now must be caused by either the 'exclude subproducts' clause or the user session id.

R.

Collapse
Posted by Richard Hamilton on
Got it!

When I compared the query with the earlier working version I noticed the addition of the limit and offset directives. I guess these are to provide the pagination features mentioned the other day.

The problem was a simple fencepost error in the default value for $offset in the category-browse-subcategory.tcl file. The default was set to "1" which means that the first row returned will always be excluded from the product search.

I recommend a change to a default value of "0" which interestingly is what is already set in category-browse-subsubcategory.tcl

Regards
Richard

Collapse
Posted by Brian Fenton on
Hehe - I've never heard of an offset error referred to as a "fencepost" before. I like it! The things you learn on the OpenACS fora. :-)

Brian

Collapse
Posted by Torben Brosten on
Interesting, Ricky. db_multirow specifically states that it uses 1 for the start row ( http://openacs.org/api-doc/proc-view?proc=db_multirow ). There must be another way to handle this, or the calculations will need to be changed on this and the other category-browse pages.
Collapse
Posted by Torben Brosten on
Also, the local versions here are working as expected, ie 1 item in a subcategory shows.. 1 item.

Some of the category browse code caches other values. I'm guessing that a server restart would fix it..

The category browse pages were originally coded to handle all three category levels on one page.. and were then separated to add more level specific options.

The code on these pages needs to be simplified, new features added, and moved into /lib

I'm making this priority for the weekend.

cheers,

Torben

Collapse
Posted by Richard Hamilton on
Torben,

I think we may be at crossed purposes here. I am talking about the OFFSET in the query itself, not the data representation created by db_multirow.

"OFFSET says to skip that many rows before beginning to return rows to the client. OFFSET 0 is the same as omitting an OFFSET clause. If both OFFSET and LIMIT appear, then OFFSET rows are skipped before starting to count the LIMIT rows that are returned."

From: http://www.postgresql.org/docs/7.2/static/queries-limit.html

In subsubcategories there was a default OFFSET of 0 but in subcategories there was a default OFFSET of 1. Both can't be correct, so one way or the other one of these needs to be changed.

I agree that this code is a little obscure and could do with being improved, although as it stands it does work well. Time invested in trying to reduce query times on your large dataset might be well spent and I'd be happy to help with that if you'd like me to.

As far as a complete re-write of this code is concerned, I mentioned before that I would like to contribute to that process.

I have had a look at the new version now and can see lots and lots of evidence of the fantastic work that you have done on this module to bring it up to date. It looks great and you have added some really important new features and support. Congratulations.

Looking forward, I wonder if it would be worth gathering some opinions on the benefits and disadvantages of migrating some of the ecommerce data structures to take advantage of some of the newer core features. This would have to be done with due consideration for performance (we don't want to 'improve' the code structurally only to find the performance degraded).

The ideas that I think we should explore are:

A) Use acs-categories for the product categories instead of dedicated data structures.

Benefits:

1) Would provides ability to have more than the three levels of category.
2) Would allow mapping of more complex reationships.
3) Integrates module more tightly with core system.

Questions/disadvantages:

i) Would deeper category structures degrade performance?
ii) Would ecommerce performance be degraded in systems that grow to large very large numbers of acs_objects?

B) Use list-builder to generate product listings.

Benefits:

1) Should be quick to code, consistent presentation and robust.
2) Provides pagination and other features already.
3) Better abstraction and code structure.
4) Benefits from code performance improvements to list builder and core rather than requiring separate optimisation.

Questions/disadvantages:

i) Would performance be acceptable with lare datasets?
ii) Is display layout flexible enough?

C) Use AMS for re-code of custom product fields.

Benefits:

1) Automatic form generation for use in re-coded product edit pages.
2) Better abstraction and code structure.
3) Tighter integration with core.

Questions/disadvantages:

i) Performance with large datasets and large numbers of custom fields?

These changes, if workable, would be a great first step and would provide the platform on which the rest of the pages could gradually be re-worked and structurally improved. In particular I'd love to lose the spider's web of progressively nested tcl functions that containing display
information. I'd like to separate that out into css for all pages. However, I think to aboid wasted effort we should address the data model questions first.

As a separate suggestion, I also think we should code up some xowiki ecommerce includelets so that once an ecommerce module is in place in a system, users could build shop functionality arbitrarily into user pages.

Some initial ideas for different includelets:

1) Product category tree display with browse and product listing.
2) Single product display with product view and 'add-to-cart' options.
3) Your account/ orders display
4) Shopping cart display
5) Ecommerce utility box with log-in link, items in cart count, value and link to cart display.

What do you think?

Best regards
Richard

Collapse
Posted by Richard Hamilton on
Oh sorry - one other thought. Implicit in what I have already floated is the notion of making products acs_objects. There benefits of this might be:

1) Different product managers could then have admin control over different products and categories of products. This would allow permissions to match organisational product management and sales accountabilities.

2) Categories of product and products themselves could be restricted to specific users and user groups. Perhaps this would be useful for example in a members' or purchasing club where certain products are only available to club members.

3) Built in notification capability through notifications in core. This would make notifying users for any reason (i.e items changing stock status or pre-ordered new products being made available for sale) very straightforward.

4) Better integration with core, better abstraction.

5) Easier to write content provider to site wide search module and would allow us to use sws to replace the ecommerce search.

Regards
Richard

Collapse
Posted by Richard Hamilton on
Oh dear, sorry, have I used the wrong terminology. Just thought it seemed like a good description. :-)
Collapse
Posted by Brian Fenton on
Far from it - you are educating the masses. And you were right:
http://en.wikipedia.org/wiki/Off-by-one_error#Fencepost_error

;-)

Collapse
Posted by Torben Brosten on
Regarding Offset problem, you're right, I mixed up offset and start.. and apparently was a similar mistake when I coded it.

Anyway, fixed now on head for category-browse-subcategory.* and hopefully for category-browse-subsubcategory.* as well. I didn't test the subsub version, it might not use offset, depending on if it shares the db query with category-browse-subcategory-postgresql.xql since they have the same sql named reference.

Collapse
Posted by Torben Brosten on
Ricky, some of the enhancements were actually inspired from seeing how you customized ecommerce.

The ecommerce code isn't yet up-to-date with openacs standards, but importantly it *does* work with much of it.

Regarding the general idea of moving code to OpenACS core features, one can apply these disadvantages:

The more core features are used, the less likely that someone without much experience will be able to maintain/fix it when there are problems, and the more likely a de-stabilized problem with another package will affect ecommerce.

A) Use acs-categories for the product categories instead of dedicated data structures.

i) Would deeper category structures degrade performance?

No degradation. My understanding is that the hierarchal representation is "shadowed" into a column for fast referencing etc.

ii) Would ecommerce performance be degraded in systems that grow to very large numbers of acs_objects?

ec_products is already linked to and an extension of acs_objects. Look at the table definitions in ecommerce/sql. Degradation shouldn't be significant, unless one starts adding per product permissions etc.

B) Use list-builder to generate product listings.

This may be fine, is certainly fine if can be completely defined in ecommerce/lib so that it's easy to switch to another includelet if needed.

Benefits:

1) Should be quick to code, consistent presentation and robust.
2) Provides pagination and other features already.
3) Better abstraction and code structure.
4) Benefits from code performance improvements to list builder and core rather than requiring separate optimisation.

Questions/disadvantages:

i) Would performance be acceptable with large datasets?

I believe so. listbuilder used to handle some db related functions in tcl (which was slow), but since about 5.1 uses the db properly to gain the most in performance.

ii) Is display layout flexible enough?

I'm not sure. if css references can be controlled, then yeah. If the css references are fixed, then any product list display changes would likely affect lists all over the affected website.

C) Use AMS for re-code of custom product fields.

Benefits:

1) Automatic form generation for use in re-coded product edit pages.
2) Better abstraction and code structure.
3) Tighter integration with core.

Questions/disadvantages:

i) Performance with large datasets and large numbers of custom fields?

Automatic form generation is fine until you don't want it automatic. I conceptually understand how ad_form works, but have never gotten one to work ..not even the example notes package. I believe that using ad_form could significantly decrease the qualified labor base, should a company want to hire help to make changes. ad_form has it's own meta language space different than tcl, sql, and adp/html.

ad_form has the limitation of requiring no more than one form per page. Ecommerce tends to have multiple forms per page.

====
Yes, the display code needs to be separated from data. These ec_ procs should be deprecated in lieu of using includelets and formatless procs.

I think the data model has reached it's limit, and needs radical changes, which is one of the reasons for starting ecommerce-g2 project ( http://dekka.com/p2/ ). One main limitation is that ec_items does not have a quantity column. ec is currently not suitable for shops that allow/promote bulk quantity sales.

I agree that ecommerce can benefit by using xowiki or implementing xowiki-like processes. See http://dekka.com/p2/xowiki/online-catalog Xowiki css needs to be standards compliant and the css more cross-browser stable. Try changing some of the DIV background colors to see the differences in display between browsers.
I haven't had a chance to play with adp includelets much in Xowiki, but believe it's going to be straight forward once the ecommerce/www pages have been deconstructed into ecommerce/lib.

There already exists some code in ecommerce to use (an early version of) site-wide search. Hopefully it wouldn't take too much to update it to work with the latest.

cheers,

Torben

Collapse
Posted by Richard Hamilton on
Torben,

Thank you very mmuch for your reply. I think we can safely conclude that we are singing from the same hymn sheet!

There is really only one issue that jumps out at me from your post, and I think it may be rooted in misconception.

"I conceptually understand how ad_form works, but have never gotten one to work ..not even the example notes package. I believe that using ad_form could significantly decrease the qualified labor base, should a company want to hire help to make changes. ad_form has it's own meta language space different than tcl, sql, and adp/html."

I don't agree - here's why......

As I understand it, ad_form is a core proc that is at the heart of the OpenACS forms API. Its facilities are most conveniently accessed using template::form which implements a declarative syntax for form generation. It is easy to use, intuitive, and concise, and is conceptually similar to the way that you would think about manipulating a browser's DOM using javascript. It eliminates repetitive code and provides consistent structure.

The syntax is standard tcl and it no more has its own language space than any API library anywhere. OpenACS is a toolkit , and this is one of the best tools in the kit! :-)

"ad_form has the limitation of requiring no more than one form per page."

This is not so. I don't believe that any code with such a major limitation would ever make it into OpenACS core.

I have knocked up a demo which I have posted the code for below. I have put it up on a server for you to look at but will not post the url here as it will be temporary (there's no point posting a link that will be dead in a week). I'll mail it to you.

The forms API is really easy to use and very elegant. I will be very disappointed if I can't pursuade you to use it in ec-G2!

Regards
Richard

Collapse
Posted by Dave Bauer on
template::form is the original core code and ad_form is built on top of it. Either way, you should not have forms that don't use one of those.

If you don't like ad_form the acs-templating forms API is just as powerful just much more verbose.

In fact there are some things the ad_form apis can't do and when that happens you can just apply the acs-templating forms api where needed.

Collapse
Posted by Torben Brosten on
Yep, thanks Dave. ecks-forms will use template::form
Collapse
Posted by Richard Hamilton on
# TCL PAGE BEGIN - remove me

# Demo of ad_form
ad_page_contract {
    @author Richard Hamilton (richard.hamilton@webteamworks.com)
    @creation-date 21-12-2008
    @cvs-id@ $Id$
    Demonstration of ad_form calls and multiple forms per page
} { 
} -properties {
    context:onevalue
}

set title [ad_quotehtml "Ricky's little ad_form demo"]
set context [list "Demo"]

# The following condition would tel you if the request is for a new form or if the form already exists - i.e. the
# user wants to edit the data already submitted. This is used when implementing an add/edit page.
#
#if {[template::form is_request form_one] } { }
#
# In contrast, the following condition would tell you if the request is for a valid form that is already defined to
# which one may want to add additional fields based upon conditional statements:
#
#if [template::form is_valid form_one] { }
#
# When any of the forms is submitted (and you can specify as many buttons as you like on each form), by default
# the values will be passed back to this tcl file for processing using the introspective facilities provides by
# template::form::get_button or template::form::get_action
#
# The structure of your add/edit page therefore is basically arranged around three modes controlled by conditional
# statements that direct execution according to the current state of the form accessible using the template::form api.


# OK, here we go....let's declare the forms
template::form create form_one
template::form create form_two

# By the way, if you add and -html {name value} to the end of the above you can specify any required form attributes
# such as javascript references and MIME specs as per oacs documentation on form-procs.tcl

# Check whether we arrived here from a previous form submission or if these are to be new forms
if {[template::form is_valid form_one]} {
    set button_clicked "Form One's '[template::form get_button form_one]' Button"
} elseif {[template::form is_valid form_two]} {
    set button_clicked "Form Two's '[template::form get_button form_two]' Button"
} else {
    
    # The forms have not previously been declared so now declare the form elements

    # Set this variable to avoid an error in the template
    set button_clicked "f"
    

    # Start of elements for form_one

    template::element create form_one form_one_id \
        -widget hidden \
        -datatype number \
        -value 1

    template::element create form_one form_one_field_one \
        -datatype text \
        -label "Field 1" \
        -html { size 10 maxlength 10} \
        -value ""

    template::element create form_one form_one_field_two \
        -widget textarea \
        -datatype text \
        -label "Field 2" \
        -html { rows 5 cols 80 wrap hard } \
        -value "You can specify default values"

    template::element create form_one form_one_field_three \
        -widget checkbox \
        -datatype boolean \
        -label "Field 3" \
        -options { { High t } } \
        -value "f"

    # End of elements for form_one


    # Start of elements for form_two

    template::element create form_two form_two_id \
        -widget hidden \
        -datatype number \
        -value 1

    template::element create form_two form_two_field_one \
        -datatype text \
        -label "Field 1" \
        -html { size 10 maxlength 10} \
        -value ""

     template::element create form_two form_two_field_two \
        -widget textarea \
        -datatype text \
        -label "Field 2" \
        -html { rows 8 cols 100 wrap hard } \
        -value "You can also specify style sheet classes or other html on a per element basis"

    template::element create form_two form_two_field_three \
        -widget checkbox \
        -datatype boolean \
        -label "Field 3" \
        -options { { High t } } \
        -value "f"
        
    # End of elements for form_two

}

ad_return_template

# TCL PAGE END - remove me

# ADP PAGE BEGIN - remove me

<master>
<property name="title">@title@</property>
<property name="context">@context@</property>

<h2>Ricky's little ad_form demo</h2>

<if @button_clicked@ eq "f">
  <h3>Form One</h1>
  <formtemplate id="form_one"></formtemplate>
  <h3>Form Two</h1>
  <formtemplate id="form_two"></formtemplate>
</if>
<else>
  <p>
    The following button was clicked:&nbsp;@button_clicked@
  </p>
  <p>
    To try me again click <a href="ad_form_demo">here</a>
  </p>
  <p>
    The documentation for template::form can be found <a href="http://openacs.org/api-doc/proc-view?proc=template::form">here</a>
  </p>
</else>

# ADP PAGE END - remove me
Collapse
Posted by Torben Brosten on
Ricky, you are welcome to use ad_form (with ecommerce). I am not interested in blocking any progress on the code.

I wonder why /register/index and /register/user-new have to be on different pages?

Thank you for the ad_form explanation and demo/example. They *are* helpful. I am still quite wary to start using it, but am saving your info for reference. Perhaps I can tackle ad_form in the context of my familiarity with ecommerce package. I can see how it could be fun to one day re-code ecommerce/www/checkout-one-form*

Emulation is the best way for me to learn this. How would ad_form be coded to handle these common form requirements in ecommerce:

1. block double-clicks or re-submits after the form has been validated and posted
2. handle file-uploads
3. handle multiple date/time fields
4. lists of data in the form, much like the current thread: http://openacs.org/forums/message-view?message_id=2516549 but where the form includes entry for multiple columns as well as rows.

Combining ad_form and template::list seems really complex and yet could be used in many places in ecommerce. That's why I've bee working on code to combine ad_form, template::list, form generation (as web service or multiple browser entry styles or file upload/input), and templates (like AMS) at: http://dekka.com/p2/xowiki/ecks-forms to start with an elegant solution for the most complex which hopefully will make the other cases even easier.

In any case, I'm willing to work with ad_form and follow your lead.

Collapse
Posted by Richard Hamilton on
Torben,

The questions you raise are very good ones, and I wouldn't pretend to have all the answers. My first thoughts though are as follows:

1) My understanding was that one of the benefits of using the API layers is that protection against such basic possibilities would be provided. I am certainly not aware of having had any problems caused by that in the code I have used template::form for.

I can see that with a shop where there is the possibility of placing multiple identical orders, or taking multiple payments, that this is very important indeed.

My, probably naive and complacent, assumption has been that as long as every row that you write from a form to a db has a primary key of some sort, then your protection against this happens where it actually should be happening - inside the RDBMS.

There is an article on Jon Griffin's site that provides an example of ad_form use and mentions that double click protection is built-in by pre-constructing the primary key. Is this just another way of saying what I have just said? It would also be possible to put a select query in the form validation step to look in the target table for the relevant primary key before doing the insert.

http://jongriffin.com/articles/10/

It is quite old and talks about ad_form rather than template::form but may still be useful.

If there is a specific issue that I haven't grasped then please let me know so I can research it.

As far as having to process data in sequential files is concerned, I haven't checked yet, but I'd be astonished if there isn't an option to change the default action to point to another tcl file. Failing that you could do an internal re-direct once you establish that the form has been submitted. In the end, failing all else, for specific cases you could fall back to the old way!

2) Funnily enough, the same Jon Griffin article covers diong this:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
File upload widget

ad_form -name upload_test -html {enctype multipart/form-data} -form {
{upload_file:file {label "Enter a filename or use the browse button"}}
} -on_submit {
set filename [template::util::file::get_property filename $upload_file]
set tmp_filename [template::util::file::get_property tmp_filename
$upload_file]
set mime_type [template::util::file::get_property mime_type $upload_file]
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Clearly this needs to be de-constructed and re-coded using template::form semantics but at least the basics are there.

3) I am not sure why this is seen as a problem. I would expect the built-in date widget to serve the purpose. I am not aware of any restriction on the number of date widgets in a form, or on the number of forms on a page.

http://openacs.org/api-doc/proc-view?proc=template::widget::date

4) I should probably break cover here and own up to the fact that I loathe it when lists of things have editable and submittable form fields on them! :-) YUK!

Dave's suggestion of an Ajax helper is a creative one but I still don't like it. I understand the benefits of minimising clicks, but really, clicking on an edit link in a list is no more effort (or clicks) than clicking in a form field to activate it and give it 'focus'. In any case, on a long list you can end up:

i) with a massive page that is very slow to load

ii) losing track of the fields you've changed as they disappear off the page, and having trouble finding the submit button.

or

iii) if the list is paginated, either having to keep track of pending edits whilst the user pages backwards and forward, or forcing the user to commit edits with one of those horrible 'update cart' links before he can continue paging through the data!

and

iv) where do you stop? Should all fields in all columns be editable? Like a spreadsheet? Over a stateless protocol? This gives your page immense destructive power and makes it very difficult for you to provide a user with a simple and safe way to verify accurately that their intended changes are correctly recorded prior to submission. Your options become quite limited and your user could easily become overloaded and make costly mistakes.

What is wrong with the current design standard where you display a list and provide edit and delete icons on each row for editing them? A simple form is presented which is intuitive, and once dealt with the user is returned to where he came from.

By the way (owning up to vested interest) - I have a 10 year old laptop and am frequently using it on mobile connections, and my browser absolutely hates trying to deal with those mammoth list/form nightmares that I encounter on lots of .asp based websites! :-)

Regards
Richard

Collapse
Posted by Torben Brosten on
Hi Richard, I think we're on the same page:

1) Implementing this is usually where I get stuck. Something about pre-generating a key versus creating a key when the form is first submitted.. Anyway, this just needs to be certain to work. Again, I'll follow your example.

2) re: file upload widget. Great! I've gotten lost in the code when trying to reverse-engineer core versions that upload files.

3) The problem may have to do with how ad_page_contract represents date/time components as lists making a list of dates complicated. ad_form doesn't use ad_page_contract, so maybe this is avoided? Anyway, I would need a working example on how to deal with forms like this (for example when implementing in ecommerce/www/admin/orders/fulfill* .

3b)
Without ad_page_contract, we need to verify that the current code cannot be manipulated. All variables will need to be initialized to clear out any url based tampering of forms.

4) Regarding lists of editable and submittable form fields:
This is more for product, category management and accounting /bookkeeping possibilities. End-users would not see these, and admins would have an option to use file uploads etc.

4iv) You wrote "where do you stop? Should all fields in all columns be editable? Like a spreadsheet?"

yes, think spreadsheet or maybe project-open dyna fields / openacs AMS editor etc: http://dekka.com/p2/xowiki/spreadsheet

Anyway, no worries for ecommerce. This is something for other packages in development, and I only mention it, because I have to consider it in the overall backend UI for ec-g2.

I agree to keep the pages light weight and streamlined. ecommerce (and ecommerce-g2) should work via mobile surfing.

cheers,

Torben

Collapse
Posted by Richard Hamilton on
Torben,

I meant to post a reply to your first point about how to 'pre-generate' a primary key.

In the conditional block:

if {[template::form is_request form_one] } {
**select nextval from your_sequence**
}

Then when you set out the form specification, include a hidden field that contains the key value as the unique identifier for that interaction.

Any double click of the form's buttons will not satisfy the above condition and so will not re-generate the key.

http://openacs.org/api-doc/proc-view?proc=template::form::is_request

Regards
Richard

Collapse
Posted by Richard Hamilton on
Torben,

Re:3b

As I understand it, forms created using the forms API by default have their variables passed as form vars rather than url encoded so url surgery is not an issue. The validation and verification services usually provided by ad_page_contract are also taken care of by the forms API.

It should basically take the sweat out of it and reduce the risks, but you're right to want to test everything thoroughly.

Sorry for getting things muddled. I understood that template::form was the recommended way to access the API (either with or without the 'template::' prefix). I had mis-understood the ad_form/ template::form relationship.

I think that template::form code can be laid out to be really easy to read and understand and may therefore have the edge for maintainability. I certainly found it easier to get my head around initially. I'll have a play with the ad_form method though if I understand Dave correctly it can in certain situations be more limiting.

Thanks for the steer Dave.

R.

Collapse
Posted by Torben Brosten on
Yeah, Richard, I was so focused on the element handling I didn't notice you used template::form instead of ad_form in the demo. This is one explanation of why your code was a breeze to understand! I find other's template::form code much easier to read/debug than ad_form.
Collapse
Posted by Richard Hamilton on
Which is of course a perfectly good reason to elect to code that way, as Phil G put it 'to make it easy for your maintenance programmer'.

I am searching for my electronic copy of Roberto Mello's ad_form cheatsheet which I think you'll find very helpful too.

I'll sent it as soon as I find it.

R.

Collapse
Posted by Torben Brosten on
Thank you, Richard,

I have Mello's ad_form quicksheet (and most all other ad_form references that are publicly available).

The quicksheet is available here: http://openacs.org/storage/view/ad-form-quick-ref.pdf

cheers,

Torben

Collapse
Posted by Torben Brosten on
Hi Richard,

ad_sign and ad_verify_signature are used to verify the form is not generating new records on double click etc.
ad_form and ad_page_contract use them.

As far as I can tell, we will need to replicate their use with template::form also (at least with transactions that are not fully undo-able).

new years cheers,

Torben

Collapse
Posted by Dave Bauer on
Note that the signature is used to confirm the form was not changed. The object_id (called a key) for ad_form is used to generate the signature.
Collapse
Posted by Torben Brosten on
Yeah. The double click protection will be to prevent multiple charges for a single sale etc. ie where the key is already known.
Collapse
Posted by Richard Hamilton on
Torben,

I thought that ad_sign was to verify that:

1) The form has not been changed (as per DB)
2) That no url surgery has taken place

I didn't think that ad_sign had anything to do with double-click protection.

R.

Collapse
Posted by Torben Brosten on
I understand that ad_sign can be used to create a unique key between form and form post. No two form posts have the same key.

This can be handy for posts that activate procedures/callbacks that have a delayed response between call and termination, where one does not want multiple processes activated on one record/key.

Anyway, I'm sure we'll want to find a standard way of handling a more strict double click protection and can start a different thread when we get to that point.

Collapse
Posted by Dave Bauer on
ad_sign can't generate a unique key by itself. You have to pass in the object_id or other unique data. It doesn't really stop a double click since the same signed data can be submitted twice with a double click and it will still be valid.
Collapse
Posted by Richard Hamilton on
Thanks for clarifying that Dave.

I think that one way to implement this is to generate a unique key from an established sequence at form creation time, possibly in the conditional block:

if {[template::form is_request form_one] } {
**select nextval from your_sequence**
}

The key can then be included in the form as a hidden field which means that no matter how many times the form is submitted, the unique key identifies the transaction. If it has already been committed to the db then it can either be quietly absorbed or rejected as duplicate.

There may be other ways of doing this but this seems a logical and elegant option.

R.

Collapse
Posted by Richard Hamilton on
On the subject of taking fine control of layout in the template (and also potentially a way of achieving the desired result in your list/form requirement earlier discussed), some useul info is to be found in this thread:

http://openacs.org/forums/message-view?message_id=156815

Particularly:

"Creating your own layout within the templating system as Don said works like this:

<formtemplate id="myform">
some html stuff ...<br><br><br>
<formwidget id="mywidget1" size="20" ...>
...
</formtemplate>

As soon as the contents of the formtemplate tag are not empty it will use those to render the form instead of rendering it automatically."

Collapse
Posted by Richard Hamilton on
Some info on built-in key handling and therefore potentially useful in building double-click protected forms.

From the OpenACS ad_form documentation here:

http://openacs.org/api-doc/proc-view?proc=ad%5fform

"ad_form defines a "key" pseudotype. Only one element of type "key" is allowed per form, and it is assigned the integer datatype. Only keys which are generated from a database sequence are managed automatically by ad_form. If the sequence name is not specified, the sequence acs_object_id_seq is used to generate new keys. Examples:

    my_key:key
    Define the key "my_key", assigning new values by calling acs_object_id_seq.nextval


    my_key:key(some_sequence_name)
    Define the key "my_key", assigning new values by calling some_sequence_name.nextval"

AND

"Here's an example of a simple page implementing an add/edit form:


    ad_page_contract {


        Simple add/edit form

    } {
        my_table_key:optional
    }

    ad_form -name form_name -export {foo {bar none}} -form {

        my_table_key:key(my_table_sequence)

        {value:text(textarea)            {label "Enter text"}
                                          {html {rows 4 cols 50}}}
    } -select_query {
        select value from my_table where my_table_key = :my_table_key
    } -validate {
        {value
        {[string length $value] >= 3}
        "\"value\" must be a string containing three or more characters"
        }
    } -new_data {
        db_dml do_insert "
            insert into my_table
              (my_table_key, value)
            values
              (:key, :value)"
    } -edit_data {
        db_dml do_update "
            update my_table
            set value = :value
            where my_table_key = :key"
    } -after_submit {
        ad_returnredirect "somewhere"
        ad_script_abort
    }

    In this example, ad_form will first check to see if "my_table_key" was passed to the script. If not, the database will be called to generate a new key value from "my_table_sequence" (the sequence name defaults to acs_object_id_seq). If defined, the query defined by "-select_query" will be used to fill the form elements with existing data (an error will be thrown if the query fails).

"

Collapse
Posted by Brian Fenton on
Great discussion guys! Very educational and informative.

I don't know if this is much use to you, but I find it useful to use Javascript to disable a submit/button after clicking. It's hard to double-click a button that's grayed out. Of course, the user may have Javascript disabled, but in this day and age, it's quite unlikely and anyway, they will probably have other problems too. :-) Of course, it all depends on your accessibility requirements too.

Something like this should do it:

<input type="submit" name="pay" value=" Pay " onclick="this.disabled=true;document.myform.submit() >

Collapse
Posted by Torben Brosten on
Yeah, Brian, the javascript way can work too, though you're right that we want to avoid the dependency.

There's a way to use a hash, much like ad_sign but instead of using the optional expiration parameter, the hash expires on next read automatically. I was mixing up the use of ad_sign versus a proposed (pre-TIPing) addition to it that was never added.

I'll see about making a working example for ecommerce that doesn't affect acs-core.

cheers and happy new years!