Forum OpenACS Q&A: More ecommerce bugs (solutions included)

I've found some bugs in the ecommerce module (and the fixes).

Could you please tell me if i should put them in the sdm or just post them here? I think that putting them in the sdm is better (what is the sdm for, else?), but i think Ben Adida (the sdm mantainer) is busy enough with the OpenACS4 project, and here another developper can see the bug and see if the fix is good enough to get into the CVS.

So i will post them here now, but if i'm wrong, please tell me.

1. /www/admin/ecommerce/products/sale-price-expire-2.tcl.
   Change sysdate for sysdate():
   

   ns_db dml $db "update ec_sale_prices set sale_ends=sysdate,
   last_modified=sysdate ...
    
   for
    
   ns_db dml $db "update ec_sale_prices set sale_ends=sysdate(),
   last_modified=sysdate() ...
   

2. /www/ecommerce/product.tcl
   Actual code:

   # valid offer codes must be <= 20 characters, so if it's more than
   20 characters, pretend
   # it isn't there
   if { [info exists offer_code] && [string length $offer_code] <= 20
   } {
   

   Change for

   if { [info exists offer_code] && [string length $offer_code]  >
    20 } {
   

3. /www/ecommerce/process-order-quantity-shipping.tcl
   This queries for items in an order, and gets products in ec_user_session_offer_codes, union products not in ec_user_session_offer_codes:

   set selection [ns_db select $db "
   (select i.item_id, i.product_id, u.offer_code
   from ec_items i, ec_user_session_offer_codes u
   where i.product_id=u.product_id
   and u.user_session_id= $user_session_id
   and i.order_id=$order_id) union
   (select i.item_id, i.product_id, null as offer_code
   from ec_items i
   where 0=(select count(*) from ec_user_session_offer_codes where
   user_session_id= $user_session_id)
   and i.order_id=$order_id)"]
   

   The problem is that the second select isn't completed. If an order has various products, (only one of them in ec_user_session_offer_codes), the query won't return the other products:

   Fix:

   set selection [ns_db select $db "
   (select i.item_id, i.product_id, u.offer_code
   from ec_items i, ec_user_session_offer_codes u
   where i.product_id=u.product_id
   and u.user_session_id= $user_session_id
   and i.order_id=$order_id) union
   (select i.item_id, i.product_id, null as offer_code
   from ec_items i
   where 0=(select count(*) from ec_user_session_offer_codes where
   user_session_id= $user_session_id
            and product_id = i.product_id)
   and i.order_id=$order_id)"]
   

4. /www/admin/ecommerce/products/upload-2.tcl
   Missing close bracket, line 68:

   append values_sql ", [db_postgres_null_sql [DoubleApos [lindex
   $elements $i]]"
   

   for

   append values_sql ", [db_postgres_null_sql [DoubleApos [lindex
   $elements $i]]]"
   

5. /www/ecommerce/shipping-address-2.tcl
   We europeans are always complaining that americans don't make thing easy for us (internationalization, and stuff like that) and then i sent this fix to Dan Wickstrom in the shipping-address-international-2.tcl version without checking the american one ;-)

   Here is the wrong code:

   if { [ad_ssl_available_p] } {
       ns_returnredirect "https://[ns_config ns/server/[ns_info
   server]/module/nsssl Hostname]/ecommerce/checkout-2.tcl"
   } else {
       ns_returnredirect "http://[ns_config ns/server/[ns_info
   server]/module/nssock Hostname]/ecommerce/checkout-2.tcl"
   } 
   

   You have to include the port numbers. If you are running in port 80, you shouldn't include it, because the server gets confused with the cookies (myserver.com cookies don't get read by myserver.com:80).

   if { [ad_ssl_available_p] } {
       ns_returnredirect "https://[ns_config ns/server/[ns_info
   server]/module/nsssl Hostname]:[ns_config ns/server/[ns_info
   server]/module/nssock port]/ecommerce/checkout-2.tcl"
   } else {
       set insecure_port [ns_config ns/server/[ns_info
   server]/module/nssock port]
       if { $insecure_port == "80" } {
           ns_returnredirect "http://[ns_config ns/server/[ns_info
   server]/module/nssock Hostname]/ecommerce/checkout-2.tcl"
       } else {
           ns_returnredirect "http://[ns_config ns/server/[ns_info
   server]/module/nssock
   Hostname]:$insecure_port/ecommerce/checkout-2.tcl"
       }
   }
   

So ... I agree with Dan that posting into the SDM is probably uneccessary if you have a patch available.  Post information here and e-mail Roberto a diff-generated patch and it should get into the tree.

# valid offer codes must be <= 20 characters, so if it's more than 20 characters, pretend
# it isn't there
if { [info exists offer_code] && [string length $offer_code] <= 20 } {
    ad_return_complaint 1 "<li> You need to have cookies turned on in order to have special ..."
    return
} 

And, instead of the silly thing i wrote above, it should read (i've checked it against Oracle ACS):

# valid offer codes must be <= 20 characters, so if it's more than 20 characters, pretend
# it isn't there
if { ![info exists user_session_id] } {
    if { [info exists offer_code] && [string length $offer_code] <= 20 } {
        ad_return_complaint 1 "<li>You need to have cookies turned on in order to have special ..."
        return
    }
} 

I'll send the new diff to Roberto.

Sorry about that, guys!