Easy-digital-downloads: 3.0 - EDD_Customer->get_address method not working

[mintplugins]

Bug Report

Expected behavior

When I have a customer with an address in the Customer Addresses table, and I run this code:

$customer = new EDD_Customer( CUSTOMER_ID_HERE );
print_r( $customer->get_address() );

I expect to see the customer's active address. Note that the "Real world" scenario here is the auto pre-filling of the address upon checkout for logged in customers.

Actual behavior

I get no output.

Steps to reproduce the behavior

1) Purchase a product with taxes enabled, and fill out your address.
2) Attempt to run this code in debug bar console:

$customer = new EDD_Customer( CUSTOMER_ID_HERE );
print_r( $customer->get_address() );

3) You will get no output.

Reason

The address is being queried with the paramater "primary", but that isn't a queryable paramater:
https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/class-edd-customer.php#L1074

Information (if a specific version is affected):

EDD Version (or branch): 3.0

[cklosowski]

The 'type' column is queryable here: https://github.com/easydigitaldownloads/easy-digital-downloads/blob/release/3.0/includes/database/queries/class-customer-address.php#L113

Where are we finding that 'type' isn't querable? We should absolutely be able to get just the primary address, much like we do with email address.

[mintplugins]

I'm not saying that "type" (the column) is un-queryable, but rather that "primary" (the value) is not something used, and thus not queryable.

The type column does not hold a value like "primary" or "secondary" at this time, but a different taxonomy of type.

It holds the value "billing" or "shipping".

[mintplugins]

If we would like to change the taxonomy of the type column from address's purpose to address's priority, then we could use it to query based on the value of primary. But we would have to change the understood meaning of "type" for this table.

[cklosowski]

i have some in my tables using primary not just billing/shipping

5   929 primary active  12345 E Main Rd ""  Queen Creek AZ  85142   US  2019-05-16 21:46:44 2019-05-16 21:46:44 urn:uuid:6198b945-8bab-4f80-a26e-498e12b938ef

[mintplugins]

That's a sign of a data issue to me then. We haven't properly defined what "type" refers to for this table, and it could cause problems in the future.

It needs to either refer to "primary/secondary", or "billing/shipping", but it can't be both. Currently it is used for both.

To demonstrate, purchase a new product with a new customer, with taxes enabled. Notice that the type column is automatically populated with the word "billing".

[cklosowski]

A couple things we need to determine then, and I think this is the approach we should take.

1) Let's make this column always be the purpose, not the priority. That leaves us with billing as the only one from core, and then extensions can use this for things like shipping.
2) We should find a way to indicate the 'primary', and thus calls to get_address() would take in the type billing by default, and get the primary.

Primary could exist as a column on the addresses, or column on the customer. I would prefer it be on the address table, I would propose we do something like:
1) Add a 'primary' column to the customer addresses. This can be a boolean state 1/0.
2) We should add a unique on customer ID, type, and primary, so we can't ever end up with two primaries. It would require setting one to not primary, and then adding primary to the type. By making the unique on the Customer ID, type, and boolean, we'd never be able to set two primaries for a type for a customer.

[cklosowski]

@mintplugins what do you think about that solution above?

[mintplugins]

1. I like that approach, adding a "primary" boolean column to the customer addresses table..

2. I'm not totally sure if understand #2 above. Can you rephrase it again for me?

Edit: By "unique" do you mean to make the schema have UNIQUE for those columns?

[cklosowski]

Ignore the 'UNIQUE' structure, that won't work...however I think we should have the ability to add a column for the 'primary' status of an address....so you could have a primary shipping and primary billing?

[mintplugins]

Yeah I think that makes sense. That way you can have many different address purposes, but we can easily query for the default/primary one to use.

[cklosowski]

So a few things we'll need to work out:
1) A new column for 'primary'
2) A function to set primary for an address (which conversely also unsets another as primary)
3) I think we should default the type to billing for input as well as searching as that's what core uses.

[cklosowski]

Also, I know we've talked about a few of these 'extension' things in the core migrator...but I think we should probably migrate shipping addresses from simple shipping here as well, instead of having that baked into the extensions.

[cklosowski]

Ok I talked with @mintplugins about this a bit, and I think we're going to add a new column for 'primary'. This way we can use a tinyint to store 1 or 0 for if the address is a primary. We can restrict a single primary to a type for a customer ID.

Eventually we can build out a UI for this but for now supporting primary is ideal in this table. It stops us from storing it on customer meta, which would take multiple queries or joins to get the primary address of a customer.

[cklosowski]

I'm going to add an index to this table now that we have the concept of a primary of;
CREATE INDEX customer_primary ON {$this->table_name}(customer_id, type(20), primary);

This will allow a much faster query for a customer's primary address for an address type.

[cklosowski]

Turns out we can't use 'primary' as it creates some issues with creating indexes...b/c primary is a protected word in MySQL. Going to move it to is_primary

[spencerfinnell]

Where are we finding that 'type' isn't querable? We should absolutely be able to get just the primary address, much like we do with email address.

@cklosowski It should be noted that wp_edd_customer_email_addresses use the type column to designate primary and secondary. Perhaps that should be updated to use is_primary as well, and remove the type column.

[ashleyfae]

The original issue as described no longer seems to be a problem for me. At least when I run the test everything prints out as it should. Is there still something we want to do here?

[ashleyfae]

If we use is_primary as 1 or 0 then there are some BerlinDB query considerations. There are/were some issues with explicitly querying for a 0 value. This may have already been fixed in BerlinDB core, but those changes haven't yet been pulled into EDD. Mostly we'll want to query for a 1 value for "primary" so there may not immediately be a use case for querying for a 0 value, just pointing it out!

[cklosowski]

I'm going to continue forward with this one. There is some benefits to the extension ecosystem that having a type column that supports things like billing and shipping is ideal for. This also stops us from 'dumping' data into different areas in extensions.

We can move to is_primary

[robincornett]

I'm encountering this issue on my test site.

I have taxes enabled and am testing license renewals/upgrades (with issue/7545). The logged in customer's address shows in the physical addresses table as "Secondary" (all addresses for all customers appear to be Secondary). When the customer is logged in and tries to check out, no address information is filled in.

I've confirmed this behavior with a customer with two addresses and one with only one.

[robincornett]

Additionally, the [edd_profile_editor] shortcode does not populate the address for a logged in user with only one address from which to choose.

[robincornett]

Something to consider for the checkout screen: in the add/edit order screens, if an existing user is selected, I can select a previously used address from a dropdown field, or I can enter a new address. Could this functionality be implemented for users to select from multiple addresses?

[cklosowski]

@robincornett I think it could but I don't think that is a scope we're ready to do for 3.0. If you want to add it to the 3.2 release we can look at getting it in there.