Exactly as my title describes, if Addressfield is attached to Entities which are not customer_profiles, this will crash the Commerce "/checkout" page with PDO errors.

CommentFileSizeAuthor
#14 commerce_addressbook-112211-final.patch2.89 KBtscadfx
#13 commerce_addressbook-112211.patch2.89 KBtscadfx
#12 commerce_addressbook-112211.patch3.25 KBtscadfx
#3 checkout-crash-fix-use-billing-address-only.patch1.81 KBSpindles
#2 checkout-crashes-if-address-field-attached-to-entities-which-are-not-customer-profiles-1310072-3.patch858 byteswjaspers

Comments

wjaspers’s picture

wjaspers commented

I believe the source of the problem stems from this function:
Cause of problem is here, in commerce_addressbook_get_saved_addresses:

$query
        ->entityCondition('entity_type', $entity_type)
        ->entityCondition('bundle', $bundle_names, 'IN')
        ->propertyCondition('status', 1)
        ->propertyCondition('uid', $uid)
        ->propertyOrderBy('profile_id', 'DESC');

Profile ID is only available to commerce_customer_profile Entities. Why not sort by something on the address?

Looks like this block could be improved, too:

commerce_addressbook_get_bundles_with_field.
In this block of code:

foreach ($field_info as $field) {
      if ($field['type'] == $field_type) {
        foreach ($field['bundles'] as $entity_type => $bundles) {
          if ($bundles) {
            foreach ($bundles as $bundle_name) {
              // @TODO: support the case where there are multiple addressfields for one bundle
              // Although that's probably not that common
              $result[$entity_type][$bundle_name] = $field['field_name'];
            }
          }
        }
      }
}
wjaspers’s picture

wjaspers commented
Status: Active » Needs review
StatusFileSize
new checkout-crashes-if-address-field-attached-to-entities-which-are-not-customer-profiles-1310072-3.patch858 bytes

Patch available.
I'm sure this can be improved, too, but simple enough.

Spindles’s picture

Spindles commented
StatusFileSize
new checkout-crash-fix-use-billing-address-only.patch1.81 KB

Further to the problem above I was still getting errors even with the above patch.

To remedy the issue I have modified commerce_addressbook_get_bundles_with_field() to only use bundles where the bundle_name is 'billing'.

Patch provided.

wjaspers’s picture

wjaspers commented
Status: Needs review » Needs work

Don't restrict the bundle to just "billing" profiles.

The purpose of Addressbook is to allow any addressfield on any source owned by the current user to be made available.
This would be especially troublesome for people wanting to be able to select "Shipping" profile addresses if the bundle type is restricted.

If you were still seeing errors, please report them here.

Spindles’s picture

Spindles commented

Without the 'billing' restriction this is the error I'm getting:

PDOException: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'commerce_option.uid' in 'where clause': SELECT commerce_option.option_id AS entity_id, commerce_option.set_id AS bundle, :entity_type AS entity_type, NULL AS revision_id FROM {commerce_option} commerce_option WHERE (commerce_option.uid = :db_condition_placeholder_0) AND (commerce_option.status = :db_condition_placeholder_1) AND (set_id IN (:db_condition_placeholder_2)) ; Array ( [:db_condition_placeholder_0] => 37 [:db_condition_placeholder_1] => 1 [:db_condition_placeholder_2] => advert_general [:entity_type] => commerce_option ) in EntityFieldQuery->execute() (line 1119 of /var/www/vhosts/marketing.dev.timberwindows.com/drupal/includes/entity.inc). Backtrace:

Now I haven't been involved in the development of this site from the start, but from what I can figure out the addressfield is being used with Commerce Product Option to insert an address into an order for advertising space. When the module is grabbing the details of the option it doesn't contain the correct fields.

Maybe modifying the function header to add a parameter for bundle name might work?

wjaspers’s picture

wjaspers commented

Are you attaching an AddressField to a product or to a Node (being used as the product display)?

If you have the commerce_option module enabled, I think this is where the PDO error (in your case is stemming from). If you have other errors (or something with a nearby timestamp) from your DB log, it may help, too.

Spindles’s picture

Spindles commented

Nothing else in the dblog I'm afraid.

The AddressField is part of a product option set which is then attached to a product rather than a product display node.

wjaspers’s picture

wjaspers commented

@Spindles, creating a simple if() check against entities/bundles works, but what we may find is that we'd need to create one for every instance where it breaks. ...

Possible solutions:
1. A settings page for "Commerce Addressbook" could allow the site admin to select which Entities / Bundles to pull Addressfield data from.
2. Force commerce_addressbook to only work with customer_profile entities (Known bundles: billing profile, shipping profile, etc...)
3. Add some kind of hook_info_alter() to an API, and expect that modules which want to supply Addressfield data will indicate so...

rodych’s picture

rodych commented

I attach address field to user profile and have PDO error

Humphreybas’s picture

Humphreybas commented

I just installed commerce addressbook and got the PDOexception when checking out. Tried to apply the patch but got 'error: No changes' was found (I guess it was already commited to the git?). Anyone knows how to proceed?

wjaspers’s picture

wjaspers commented

@Humphreybas, The patch in #3 isn't in a git format or p0 format, so it likely will not apply as easily as others.
Perhaps the answer is just to provide an admin settings form that lets the addressbook define its sources...

tscadfx’s picture

tscadfx commented
StatusFileSize
new commerce_addressbook-112211.patch3.25 KB

I've corrected some of the code and made a git patch. Hope this helps some of you out. Let me know if you notice any issues with it and I'll work on fixing them. It works in my environment as is with extra fields.

tscadfx’s picture

tscadfx commented
StatusFileSize
new commerce_addressbook-112211.patch2.89 KB

Use this patch instead. Sorry

tscadfx’s picture

tscadfx commented
StatusFileSize
new commerce_addressbook-112211-final.patch2.89 KB

I really should have been in bed hours ago. I swear this is the final version of the patch and the one that should be used.

So sorry!!!

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Reviewed & tested by the community

Patch from #14 works for me.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented

Patch #14 was now committed, minus the last change (add an if clause for billing addresses) for the reasons specified by wjaspers.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented

After some further investigation, the errors make sense.

We are fetching all addresses that belong to a specific user. Though this currently most likely only makes sense for addressfields that were attached to customer_profiles, since those have a user ID. If you were to attach an addressfield to a random entity type that doesn't have a relationship with a user, this results in all kinds of odd behavior and errors.

So I changed the code to focus on customer_profiles right now, and perhaps user entities later on, but leave all other entities alone (unless someone explicitly would add support for it).

That will solve this issue in the upcoming first release of this module.

svendecabooter’s picture

svendecabooter
he/him
Primary language Dutch
Location Gent
commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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