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

Hi!

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"
        }
    }
    
And that's all! I hope it helps someone.
Collapse
Posted by Dan Wickstrom on
If you have a fix available, it's probably not necssary to necessary to post the problem on the sdm.  Just let us know via the bboards, and make a patch available if possible.  A patch is nice, because it's easy to review a patch before applying it, and it makes it easier for us, as we don't have to hunt down individual files and fix them.
Collapse
Posted by Don Baccus on
Also, Roberto Mello has offered to handle patches from folks, testing them and committing them to the tree.  I was doing this informally for  awhile, but I'm a bit busy.  Dan has too and may still want to. Roberto's always, and officially, available to broker patches into the  tree.

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.

Collapse
Posted by Ben Adida on
soon soon, the patch feature in the SDM will be available. In the meantime, Roberto and Dan are the OpenACS saviors.
Collapse
Posted by Gaizka Villate on
Hi! I sent the diff file to Roberto, but luckily he hasn't included them into the CVS. I was wrong in bug #2 (of the list above). The original code reads:
# 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!

Collapse
Posted by Gaizka Villate on
Dammit!

Now i've post the code i was testing.

Instead of if { ![info exists user_session_id] } { it should read:

if { $user_session_id == 0 } {
    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
    }
} 

This is triple-tested. So it's correct now. I'm really sorry (again). I think i should get some sleep and stay away from the forums for a while 😊