1.6.0: a shipping method a group is shown belongs to that group
Kristian checked out with an address in no group and was offered both free delivery methods set up for the Netic group. Measured on the shop before touching anything: one group, four rules, two of them "show" on the free delivery methods he saw. It was not a regression. A test asserted the behaviour as intended — "an unmatched customer sees every method" — so a "show" rule was a whitelist FOR THE GROUP and nothing more. The plugin did half of what its own description promises: it filtered what the group sees and never reserved anything from anyone else. On a shop that takes money the effect is that free delivery arranged for one company is available to every customer. A method named in any group's show rules is now reserved for the groups that name it. Answering "does it work for other discounts too", which found the hole the first fix left: it did not. A group with NO rules of its own returned every rate untouched, and so did a group carrying only "hide" rules, because neither engages the whitelist — so their members saw another group's reserved methods. Both now have other groups' reserved methods removed. The "never leave the customer without shipping" guard is kept and asserted: a shop that reserved everything it offers must still let an ordinary customer check out, because that failure is worse than the leak. Four assertions, each proven red. Two of them caught my own mistakes rather than the product's: the hide-only fixture originally left nothing for the customer, so the guard fired and returned every rate — which reads exactly like the leak it was written to detect. A third rate makes the behaviour observable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d9de751e3f
commit
6ef4ee8fe5
+5
-1
@@ -4,7 +4,7 @@ Tags: woocommerce, discount, email, shipping, coupon
|
||||
Requires at least: 5.8
|
||||
Tested up to: 7.1
|
||||
Requires PHP: 7.4
|
||||
Stable tag: 1.5.4
|
||||
Stable tag: 1.6.0
|
||||
WC requires at least: 6.0
|
||||
WC tested up to: 11.1.0
|
||||
License: GPLv2 or later
|
||||
@@ -96,6 +96,10 @@ The most specific match wins: exact email, then domain, then partial match.
|
||||
|
||||
== Changelog ==
|
||||
|
||||
= 1.6.0 =
|
||||
* A shipping method a group is shown is now reserved for that group: customers who match no group no longer see it. Previously a method set up for approved addresses stayed on the checkout for everyone, so free delivery meant for one company could be chosen by anybody. Customers are never left with no shipping method at all.
|
||||
|
||||
|
||||
= 1.4.6 =
|
||||
* Housekeeping: the plugin's source moved to a shorter address. Updates continue to arrive as before, and nothing needs doing.
|
||||
|
||||
|
||||
@@ -507,6 +507,25 @@ class WC_EDM_DB {
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Every shipping method any group reserves for itself.
|
||||
*
|
||||
* A "show" rule means the group sees that method. It also has to mean
|
||||
* nobody else does — otherwise reserving a free-delivery method for an
|
||||
* approved group leaves it on the checkout for every other customer, which
|
||||
* is what a shop notices as lost shipping revenue rather than as a bug.
|
||||
*
|
||||
* @return string[] Rate ids, e.g. "free_shipping:15".
|
||||
*/
|
||||
public static function restricted_shipping_methods() {
|
||||
global $wpdb;
|
||||
$table = self::table( 'shipping_rules' );
|
||||
|
||||
$ids = $wpdb->get_col( "SELECT DISTINCT shipping_method_id FROM {$table} WHERE action = 'show'" ); // phpcs:ignore WordPress.DB.PreparedSQL.InterpolatedNotPrepared
|
||||
|
||||
return is_array( $ids ) ? $ids : array();
|
||||
}
|
||||
|
||||
/**
|
||||
* Replace all shipping rules for a group.
|
||||
*
|
||||
|
||||
@@ -330,15 +330,25 @@ class WC_EDM_Frontend {
|
||||
public function filter_shipping_rates( $rates, $package ) {
|
||||
$match = $this->get_current_match();
|
||||
|
||||
if ( ! $match ) {
|
||||
return $rates;
|
||||
}
|
||||
|
||||
if ( 'group' === $match->assignment_type && $match->group_id ) {
|
||||
// $match is null for a customer who matches nothing — the common case,
|
||||
// and the one this whole method exists to constrain now.
|
||||
if ( $match && 'group' === $match->assignment_type && $match->group_id ) {
|
||||
$rates = $this->apply_group_shipping_rules( $rates, (int) $match->group_id );
|
||||
} else {
|
||||
// 🔴 Everyone else — including a matched customer whose assignment
|
||||
// is not a group — must not see a method some group reserved.
|
||||
//
|
||||
// This used to return $rates untouched, and a test asserted that
|
||||
// as intended: "an unmatched customer sees every method". It made
|
||||
// a "show" rule a whitelist FOR THE GROUP and nothing more, so
|
||||
// reserving free delivery for approved addresses left it on the
|
||||
// checkout for every other customer. Reported from a live shop,
|
||||
// where a gmail address could pick a free-delivery method meant
|
||||
// for one company.
|
||||
$rates = $this->remove_restricted_rates( $rates );
|
||||
}
|
||||
|
||||
if ( ! empty( $match->free_shipping ) ) {
|
||||
if ( $match && ! empty( $match->free_shipping ) ) {
|
||||
foreach ( $rates as $rate ) {
|
||||
$rate->set_cost( 0 );
|
||||
$rate->set_taxes( array() );
|
||||
@@ -348,6 +358,33 @@ class WC_EDM_Frontend {
|
||||
return $rates;
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove every method some group reserved for itself.
|
||||
*
|
||||
* @param array $rates Available shipping rates.
|
||||
* @return array
|
||||
*/
|
||||
private function remove_restricted_rates( $rates ) {
|
||||
$restricted = WC_EDM_DB::restricted_shipping_methods();
|
||||
|
||||
if ( empty( $restricted ) ) {
|
||||
return $rates;
|
||||
}
|
||||
|
||||
$filtered = array();
|
||||
|
||||
foreach ( $rates as $rate_id => $rate ) {
|
||||
if ( ! in_array( $rate_id, $restricted, true ) ) {
|
||||
$filtered[ $rate_id ] = $rate;
|
||||
}
|
||||
}
|
||||
|
||||
// The same guard the group rules carry: a shop that reserved every
|
||||
// method it offers would otherwise leave an ordinary customer unable
|
||||
// to check out at all, which is worse than the leak being fixed.
|
||||
return empty( $filtered ) ? $rates : $filtered;
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply a group's show/hide shipping rules.
|
||||
*
|
||||
@@ -362,8 +399,14 @@ class WC_EDM_Frontend {
|
||||
private function apply_group_shipping_rules( $rates, $group_id ) {
|
||||
$rules = WC_EDM_DB::get_shipping_rules( $group_id );
|
||||
|
||||
// 🔴 Not `return $rates` — a group with no rules of its own must still
|
||||
// not see what ANOTHER group reserved. Same for a group carrying only
|
||||
// "hide" rules: its own shown-set is empty, so the whitelist below
|
||||
// never engages and every other group's reserved method would survive.
|
||||
// The leak this fixes was reported for customers matching nothing; it
|
||||
// existed for these two shapes as well.
|
||||
if ( empty( $rules ) ) {
|
||||
return $rates;
|
||||
return $this->remove_restricted_rates( $rates );
|
||||
}
|
||||
|
||||
$show = array();
|
||||
@@ -379,6 +422,9 @@ class WC_EDM_Frontend {
|
||||
|
||||
$filtered = array();
|
||||
|
||||
// Reserved by some group, but not by THIS one.
|
||||
$others = array_diff( WC_EDM_DB::restricted_shipping_methods(), $show );
|
||||
|
||||
foreach ( $rates as $rate_id => $rate ) {
|
||||
// Whitelist mode: only explicitly shown methods survive.
|
||||
if ( ! empty( $show ) && ! in_array( $rate_id, $show, true ) ) {
|
||||
@@ -389,6 +435,10 @@ class WC_EDM_Frontend {
|
||||
continue;
|
||||
}
|
||||
|
||||
if ( in_array( $rate_id, $others, true ) ) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$filtered[ $rate_id ] = $rate;
|
||||
}
|
||||
|
||||
|
||||
+70
-1
@@ -194,9 +194,78 @@ WC()->customer->set_billing_email( 'someone@company.com' );
|
||||
$filtered = wc_edm()->frontend->filter_shipping_rates( $rates, array() );
|
||||
wc_edm_is( 'a "show" rule hides every method it does not name', array( 'flat_rate:1' ), array_keys( $filtered ) );
|
||||
|
||||
// 🔴 This used to assert "an unmatched customer sees every method", which is
|
||||
// what made a reserved free-delivery method visible to everybody on a live
|
||||
// shop. A "show" rule now means the method belongs to that group.
|
||||
WC()->customer->set_billing_email( 'nobody@nowhere.test' );
|
||||
$filtered = wc_edm()->frontend->filter_shipping_rates( $rates, array() );
|
||||
wc_edm_is( 'an unmatched customer sees every method', 2, count( $filtered ) );
|
||||
wc_edm_is(
|
||||
'an unmatched customer does not see a method a group reserved',
|
||||
array( 'local_pickup:2' ),
|
||||
array_keys( $filtered )
|
||||
);
|
||||
|
||||
// The guard that keeps the fix from being worse than the leak: a shop that
|
||||
// reserved everything it offers must still let an ordinary customer check out.
|
||||
WC_EDM_DB::replace_shipping_rules(
|
||||
(int) $fixtures['group_id'],
|
||||
array(
|
||||
array( 'shipping_method_id' => 'flat_rate:1', 'action' => 'show', 'sort_order' => 0 ),
|
||||
array( 'shipping_method_id' => 'local_pickup:2', 'action' => 'show', 'sort_order' => 1 ),
|
||||
)
|
||||
);
|
||||
$filtered = wc_edm()->frontend->filter_shipping_rates( $rates, array() );
|
||||
wc_edm_is( 'but is never left with no shipping at all', 2, count( $filtered ) );
|
||||
|
||||
WC_EDM_DB::replace_shipping_rules(
|
||||
(int) $fixtures['group_id'],
|
||||
array( array( 'shipping_method_id' => 'flat_rate:1', 'action' => 'show', 'sort_order' => 0 ) )
|
||||
);
|
||||
|
||||
// A SECOND group, with no rules of its own. Its members must not see what the
|
||||
// first group reserved — the leak was reported for customers matching nothing,
|
||||
// but it existed for this shape too, and for a group carrying only "hide"
|
||||
// rules, because neither engages the whitelist.
|
||||
$wc_edm_other = WC_EDM_DB::insert_group( array( 'name' => 'Ruleless', 'discount_percent' => 5 ) );
|
||||
WC_EDM_DB::insert_email(
|
||||
array(
|
||||
'email_pattern' => 'ruleless@elsewhere.test',
|
||||
'assignment_type' => 'group',
|
||||
'group_id' => (int) $wc_edm_other,
|
||||
'coupon_code' => null,
|
||||
'custom_discount' => null,
|
||||
'is_active' => 1,
|
||||
'free_shipping' => 0,
|
||||
)
|
||||
);
|
||||
|
||||
WC()->customer->set_billing_email( 'ruleless@elsewhere.test' );
|
||||
$filtered = wc_edm()->frontend->filter_shipping_rates( $rates, array() );
|
||||
wc_edm_is(
|
||||
'a group with no rules still does not see another group reserved method',
|
||||
array( 'local_pickup:2' ),
|
||||
array_keys( $filtered )
|
||||
);
|
||||
|
||||
// A third rate, deliberately: with only two, hiding one and reserving the
|
||||
// other leaves nothing, the "never leave the customer without shipping" guard
|
||||
// fires, and every rate comes back — which reads as a leak but is the guard
|
||||
// working. The fixture has to leave something for the customer to be given.
|
||||
$wc_edm_rates3 = $rates;
|
||||
$wc_edm_rates3['flat_rate:9'] = new WC_Shipping_Rate( 'flat_rate:9', 'Another flat rate', 12, array(), 'flat_rate' );
|
||||
|
||||
WC_EDM_DB::replace_shipping_rules(
|
||||
(int) $wc_edm_other,
|
||||
array( array( 'shipping_method_id' => 'local_pickup:2', 'action' => 'hide', 'sort_order' => 0 ) )
|
||||
);
|
||||
$filtered = wc_edm()->frontend->filter_shipping_rates( $wc_edm_rates3, array() );
|
||||
wc_edm_ok(
|
||||
'and neither does a group carrying only hide rules',
|
||||
! in_array( 'flat_rate:1', array_keys( $filtered ), true ),
|
||||
'saw: ' . implode( ', ', array_keys( $filtered ) )
|
||||
);
|
||||
|
||||
WC()->customer->set_billing_email( 'someone@company.com' );
|
||||
|
||||
WC_EDM_DB::replace_shipping_rules( (int) $fixtures['group_id'], array() );
|
||||
|
||||
|
||||
@@ -4,7 +4,7 @@
|
||||
* Plugin URI: https://gitea.nolimit.dk/kw/woo-discount-manager
|
||||
* Update URI: https://gitea.nolimit.dk/kw/nolimit-discounts-dist
|
||||
* Description: Apply discounts and show/hide shipping methods based on the customer's email address (full address, domain or partial match).
|
||||
* Version: 1.5.4
|
||||
* Version: 1.6.0
|
||||
* Author: Kristian Wind
|
||||
* Author URI: https://gitea.nolimit.dk/kw
|
||||
* License: GPL-2.0-or-later
|
||||
@@ -28,7 +28,7 @@ if ( ! defined( 'ABSPATH' ) ) {
|
||||
exit;
|
||||
}
|
||||
|
||||
define( 'WC_EDM_VERSION', '1.5.4' );
|
||||
define( 'WC_EDM_VERSION', '1.6.0' );
|
||||
define( 'WC_EDM_PLUGIN_FILE', __FILE__ );
|
||||
define( 'WC_EDM_PLUGIN_DIR', plugin_dir_path( __FILE__ ) );
|
||||
define( 'WC_EDM_PLUGIN_URL', plugin_dir_url( __FILE__ ) );
|
||||
|
||||
Reference in New Issue
Block a user