In situations where there isn't an profile id on the profile used in commerce_order_commerce_customer_profile_can_delete, the query generated creates an SQL error:

PDOException: SQLSTATE[22P02]: Invalid text representation: 7 ERROR: invalid input syntax for integer: "" LINE 7: ...r_billing0.commerce_customer_billing_profile_id = '') AND (f... ^: SELECT COUNT(*) AS expression FROM (SELECT 1 AS expression FROM {field_data_commerce_customer_billing} field_data_commerce_customer_billing0 INNER JOIN {commerce_order} commerce_order ON commerce_order.order_id = field_data_commerce_customer_billing0.entity_id WHERE (field_data_commerce_customer_billing0.commerce_customer_billing_profile_id = :db_condition_placeholder_0) AND (field_data_commerce_customer_billing0.deleted = :db_condition_placeholder_1) AND (commerce_order.status NOT IN (:db_condition_placeholder_2, :db_condition_placeholder_3, :db_condition_placeholder_4)) AND (commerce_order.order_id != :db_condition_placeholder_5) AND (field_data_commerce_customer_billing0.entity_type = :db_condition_placeholder_6) ) subquery; Array ( [:db_condition_placeholder_0] => [:db_condition_placeholder_1] => 0 [:db_condition_placeholder_2] => cart [:db_condition_placeholder_3] => checkout_checkout [:db_condition_placeholder_4] => checkout_review [:db_condition_placeholder_5] => 20635 [:db_condition_placeholder_6] => commerce_order ).

I believe the problem is that an empty profile id is treated as a text string which is illegal to use for an integer which the profile id is.

Comments

googletorp’s picture

googletorp commented
Status: Active » Needs review

I have a fix for this in my sandbox: 0bf4350

rszrama’s picture

rszrama commented

How did you get into a situation where you needed to "delete" an unsaved profile? That seems like the deeper issue. : P

googletorp’s picture

googletorp commented

rszrame, I didn't need to delete an unsaved profile, but in commerce_order_field_attach_form the check is performed always. Also when you create an order.

Also applying this patch adds a fail safe, which is nice to have, considering the alternative is a fatal PHP (SQL) error.

rszrama’s picture

rszrama commented
Status: Needs review » Fixed

Ahh, I see what you mean. The Order module is actually in the wrong here; it shouldn't be processing the check unless there's a $profile->profile_id, which is the same logic the Customer module uses when building the form for the profile reference field. I've updated the Order module to operate properly here (and cleaned the code up to just use a $profile variable instead of the full variable name from the form array).

That said, your fix may have merit, but it wouldn't belong in the implementation of the hook. Rather, commerce_customer_profile_can_delete() and commerce_product_can_delete() should both be updated, but instead of returning TRUE if there is no ID I believe they should return FALSE. There's nothing to delete, and therefore calling the associated delete function wouldn't do anything, which means to me the given entity cannot be deleted (semantically I suppose it's "need not be deleted", but as far as the code depending on these functions is concerned, it's functionally equivalent).

Commit: http://drupalcode.org/project/commerce.git/commitdiff/ff421fd

googletorp’s picture

googletorp commented
Status: Fixed » Needs review

Looks like there is a case more, when adding line items to a order that isn't saved yet, fix: 4e0472e

rszrama’s picture

rszrama commented
Status: Needs review » Fixed

Whoops, committed this one during Dries' keynote but forgot to close the issue. ; )

Automatically closed -- issue fixed for 2 weeks with no activity.