Country updates don't always clear old region values

[JeffPyeBrook]

_wpsc_updated_visitor_meta_billingcountry and probably the corresponding shipping country, region and state functions check the current region setting when a country update is processed.

If the old country had regions, and the new country does not have regions, the old invalid region value is not cleared.

I think what needs to happen is that if a country is changed, the region and stare values need to be checked and cleared.

Because region & state are typically updated immediately after the country is updated, I don't think this is a huge issue UNLESS a checkout form has a non-standard configuration of shipping same as billing and country/start/region fields

[JustinSainton]

And this becomes a bit of a non-issue when we switch data stores from the database to a filterable array, via #1865?

[JeffPyeBrook]

No, totally unrelated.

The risk for this is that if there is a code path that updates a billing or shipping country, that doesn't clear out the region or state on change, an invalid address could be present.

I think it is low risk, but the intent was for the code to protect against it, so it should to be fixed in the repo.

[JustinSainton]

Ah, so you're talking about running an update and their being existing, live values in carts and things like that?

[JustinSainton]

Or are you talking about the store's base region being checked? Can you elaborate on a real-world use case this would solve?

[JeffPyeBrook]

Need to add a snippets like this one I added to _wpsc_updated_visitor_meta_shippingcountry

        if ( ! empty ( $old_shipping_region ) &&  !$wpsc_country->has_regions() ) {
            wpsc_delete_visitor_meta( $visitor_id, 'shippingregion' );
        }

        if ( ! empty ( $old_shipping_state ) &&  !$wpsc_country->has_regions() ) {
            wpsc_delete_visitor_meta( $visitor_id, 'shippingstate' );
        }

[JeffPyeBrook]

No use case, it's just a bug.

I can make it happen in my test environment by changing the country from uk to usa in the checkout form and clicking submit really fast before the meta is updated. Causes a shipping quote calculation error instead of missing required checkout item error

I suspect there are more likely failure paths than that test case in codebase.

[JustinSainton]

Cool. So, starting in 4.0, we're going to require unit tests for any bug fixes that can be unit tested. We've obviously been super lax on it so far - but I think for things like this, that are sort of esoteric and hard to fully comprehend - it would be great to have a unit test that fails currently, and is fixed by your impending PR.

Thanks Jeff!

[JeffPyeBrook]

In this case it's easy to construct the unit test. Are we already/or going to use one of the standard harnesses?

[JustinSainton]

I'd just add it here: https://github.com/wp-e-commerce/WP-e-Commerce/tree/master/tests, if you're adding tests to any of those existing classes...obviously, if you're adding tests for a new class/function, follow the conventions there.

[JeffPyeBrook]

Do those tests have access to all of the WordPress functionality?

[JustinSainton]

I believe so - though I think some fail because we might be instantiating the country/region database in a weird way...I need to run them again and see for sure...at one point, they all worked and were passing :8ball:

[JeffPyeBrook]

are there directions on how to run them?

[JustinSainton]

As long as you have PHPUnit installed (VVV does by default, fwiw. SSH in and you have it) - just run phpunit on the CLI in the WPEC directory.