From f20878f7f36c6767d804546e4d3d13688b9862e2 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Tue, 13 Jun 2023 12:22:17 -0300 Subject: [PATCH] Update the queries of the 'used shipping method' filter This commit changes the logic of the code that builds the queries for the 'used shipping method' filter. The initial implementation relied only on the shipping method name that is stored in wp_woocommerce_order_items. This implementation did not work as the name is not unique. The new implementation use the shipping method id and the instance id that are stored in wp_woocommerce_order_itemmeta and the combination of both fields is unique. [MAILPOET-4992] --- .../Filters/WooCommerceUsedShippingMethod.php | 87 ++++++++++++---- .../WooCommerceUsedShippingMethodTest.php | 99 ++++++++++--------- 2 files changed, 120 insertions(+), 66 deletions(-) diff --git a/mailpoet/lib/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethod.php b/mailpoet/lib/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethod.php index a00d44e9c9..8b417d24a6 100644 --- a/mailpoet/lib/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethod.php +++ b/mailpoet/lib/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethod.php @@ -41,14 +41,15 @@ class WooCommerceUsedShippingMethod implements Filter { public function apply(QueryBuilder $queryBuilder, DynamicSegmentFilterEntity $filter): QueryBuilder { $filterData = $filter->getFilterData(); $operator = $filterData->getParam('operator'); - $shippingMethods = $filterData->getParam('shipping_methods'); + list($shippingMethodIds, $instanceIds) = $this->extractShippingMethodIdsAndInstanceIds((array)$filterData->getParam('shipping_methods')); + $days = $filterData->getParam('used_shipping_method_days'); if (!is_string($operator) || !in_array($operator, self::VALID_OPERATORS, true)) { throw new InvalidFilterException('Invalid operator', InvalidFilterException::MISSING_OPERATOR); } - if (!is_array($shippingMethods) || count($shippingMethods) < 1) { + if (!is_array($shippingMethodIds) || empty($shippingMethodIds) || !is_array($instanceIds) || empty($instanceIds)) { throw new InvalidFilterException('Missing shipping methods', InvalidFilterException::MISSING_VALUE); } @@ -65,58 +66,106 @@ class WooCommerceUsedShippingMethod implements Filter { switch ($operator) { case DynamicSegmentFilterData::OPERATOR_ANY: - $this->applyForAnyOperator($queryBuilder, $includedStatuses, $shippingMethods, $date); + $this->applyForAnyOperator($queryBuilder, $includedStatuses, $shippingMethodIds, $instanceIds, $date); break; case DynamicSegmentFilterData::OPERATOR_ALL: - $this->applyForAllOperator($queryBuilder, $includedStatuses, $shippingMethods, $date); + $this->applyForAllOperator($queryBuilder, $includedStatuses, $shippingMethodIds, $instanceIds, $date); break; case DynamicSegmentFilterData::OPERATOR_NONE: - $this->applyForNoneOperator($queryBuilder, $includedStatuses, $shippingMethods, $date); + $this->applyForNoneOperator($queryBuilder, $includedStatuses, $shippingMethodIds, $instanceIds, $date); break; } return $queryBuilder; } - private function applyForAnyOperator(QueryBuilder $queryBuilder, array $includedStatuses, array $shippingMethods, Carbon $date): void { + private function applyForAnyOperator(QueryBuilder $queryBuilder, array $includedStatuses, array $shippingMethodIds, array $instanceIds, Carbon $date): void { $dateParam = $this->filterHelper->getUniqueParameterName('date'); - $shippingMethodParam = $this->filterHelper->getUniqueParameterName('shippingMethod'); + $shippingMethodsParam = $this->filterHelper->getUniqueParameterName('shippingMethods'); + $instanceIdsParam = $this->filterHelper->getUniqueParameterName('instanceIds'); $orderItemsTable = $this->filterHelper->getPrefixedTable('woocommerce_order_items'); $orderItemsTableAlias = 'orderItems'; + $orderItemMetaTable = $this->filterHelper->getPrefixedTable('woocommerce_order_itemmeta'); + $orderItemMetaTableAlias1 = 'orderItemMeta1'; + $orderItemMetaTableAlias2 = 'orderItemMeta2'; $orderStatsAlias = $this->wooFilterHelper->applyOrderStatusFilter($queryBuilder, $includedStatuses); $queryBuilder ->innerJoin($orderStatsAlias, $orderItemsTable, $orderItemsTableAlias, "$orderStatsAlias.order_id = $orderItemsTableAlias.order_id") + ->innerJoin($orderItemsTableAlias, $orderItemMetaTable, $orderItemMetaTableAlias1, "$orderItemsTableAlias.order_item_id = $orderItemMetaTableAlias1.order_item_id") + ->innerJoin($orderItemsTableAlias, $orderItemMetaTable, $orderItemMetaTableAlias2, "$orderItemsTableAlias.order_item_id = $orderItemMetaTableAlias2.order_item_id") ->andWhere("$orderStatsAlias.date_created >= :$dateParam") - ->andWhere("$orderItemsTableAlias.order_item_name IN (:$shippingMethodParam)") ->andWhere("$orderItemsTableAlias.order_item_type = 'shipping'") + ->andWhere("$orderItemMetaTableAlias1.meta_key = 'method_id'") + ->andWhere("$orderItemMetaTableAlias1.meta_value IN (:$shippingMethodsParam)") + ->andWhere("$orderItemMetaTableAlias2.meta_key = 'instance_id'") + ->andWhere("$orderItemMetaTableAlias2.meta_value IN (:$instanceIdsParam)") ->setParameter($dateParam, $date->toDateTimeString()) - ->setParameter($shippingMethodParam, $shippingMethods, Connection::PARAM_STR_ARRAY); + ->setParameter($shippingMethodsParam, $shippingMethodIds, Connection::PARAM_STR_ARRAY) + ->setParameter($instanceIdsParam, $instanceIds, Connection::PARAM_STR_ARRAY); } - private function applyForAllOperator(QueryBuilder $queryBuilder, array $includedStatuses, array $shippingMethods, Carbon $date): void { + private function applyForAllOperator(QueryBuilder $queryBuilder, array $includedStatuses, array $shippingMethodIds, array $instanceIds, Carbon $date): void { $dateParam = $this->filterHelper->getUniqueParameterName('date'); $orderItemTypeParam = $this->filterHelper->getUniqueParameterName('orderItemType'); - $shippingMethodsParam = $this->filterHelper->getUniqueParameterName('shippingMethod'); + $shippingMethodsParam = $this->filterHelper->getUniqueParameterName('shippingMethods'); + $instanceIdsParam = $this->filterHelper->getUniqueParameterName('instanceIds'); $orderItemsTable = $this->filterHelper->getPrefixedTable('woocommerce_order_items'); - $orderItemsAlias = 'orderItems'; + $orderItemsTableAlias = 'orderItems'; + $orderItemMetaTable = $this->filterHelper->getPrefixedTable('woocommerce_order_itemmeta'); + $orderItemMetaTableAlias1 = 'orderItemMeta1'; + $orderItemMetaTableAlias2 = 'orderItemMeta2'; $orderStatsAlias = $this->wooFilterHelper->applyOrderStatusFilter($queryBuilder, $includedStatuses); + $queryBuilder - ->innerJoin($orderStatsAlias, $orderItemsTable, $orderItemsAlias, "$orderStatsAlias.order_id = $orderItemsAlias.order_id") + ->innerJoin($orderStatsAlias, $orderItemsTable, $orderItemsTableAlias, "$orderStatsAlias.order_id = $orderItemsTableAlias.order_id") + ->innerJoin($orderItemsTableAlias, $orderItemMetaTable, $orderItemMetaTableAlias1, "$orderItemsTableAlias.order_item_id = $orderItemMetaTableAlias1.order_item_id") + ->innerJoin($orderItemsTableAlias, $orderItemMetaTable, $orderItemMetaTableAlias2, "$orderItemsTableAlias.order_item_id = $orderItemMetaTableAlias2.order_item_id") ->andWhere("$orderStatsAlias.date_created >= :$dateParam") - ->andWhere("$orderItemsAlias.order_item_type = :$orderItemTypeParam") - ->andWhere("$orderItemsAlias.order_item_name IN (:$shippingMethodsParam)") + ->andWhere("$orderItemsTableAlias.order_item_type = :$orderItemTypeParam") + ->andWhere("$orderItemMetaTableAlias1.meta_key = 'method_id'") + ->andWhere("$orderItemMetaTableAlias1.meta_value IN (:$shippingMethodsParam)") + ->andWhere("$orderItemMetaTableAlias2.meta_key = 'instance_id'") + ->andWhere("$orderItemMetaTableAlias2.meta_value IN (:$instanceIdsParam)") ->setParameter($dateParam, $date->toDateTimeString()) ->setParameter($orderItemTypeParam, 'shipping') - ->setParameter($shippingMethodsParam, $shippingMethods, Connection::PARAM_STR_ARRAY) - ->groupBy('inner_subscriber_id')->having("COUNT(DISTINCT $orderItemsAlias.order_item_name) = " . count($shippingMethods)); + ->setParameter($shippingMethodsParam, $shippingMethodIds, Connection::PARAM_STR_ARRAY) + ->setParameter($instanceIdsParam, $instanceIds, Connection::PARAM_STR_ARRAY) + ->groupBy('inner_subscriber_id') + ->having("COUNT(DISTINCT(CONCAT($orderItemMetaTableAlias1.meta_value, $orderItemMetaTableAlias2.meta_value))) = " . count($shippingMethodIds)); } - private function applyForNoneOperator(QueryBuilder $queryBuilder, array $includedStatuses, array $shippingMethods, Carbon $date): void { + private function applyForNoneOperator(QueryBuilder $queryBuilder, array $includedStatuses, array $shippingMethodIds, array $instanceIds, Carbon $date): void { $subQuery = $this->filterHelper->getNewSubscribersQueryBuilder(); - $this->applyForAnyOperator($subQuery, $includedStatuses, $shippingMethods, $date); + $this->applyForAnyOperator($subQuery, $includedStatuses, $shippingMethodIds, $instanceIds, $date); $subscribersTable = $this->filterHelper->getSubscribersTable(); $queryBuilder->andWhere($queryBuilder->expr()->notIn("$subscribersTable.id", $this->filterHelper->getInterpolatedSQL($subQuery))); } + + /** + * Extracts shipping method ids and instance ids from the given array of strings. + * The format of each shipping method string is "shippingMethod:instanceId". For example, + * "flat_rate:1" or "local_pickup:2". + * + * @param array $shippingMethodStrings + * @return array[] + */ + private function extractShippingMethodIdsAndInstanceIds(array $shippingMethodStrings): array { + $shippingMethodIds = []; + $instanceIds = []; + + foreach ($shippingMethodStrings as $shippingMethodString) { + if (preg_match('/^\w+:\d+$/', $shippingMethodString)) { + $parts = preg_split('/:/', $shippingMethodString); + + if (is_array($parts) && is_string($parts[0]) && is_string($parts[1])) { + $shippingMethodIds[] = $parts[0]; + $instanceIds[] = $parts[1]; + } + } + } + + return [$shippingMethodIds, $instanceIds]; + } } diff --git a/mailpoet/tests/integration/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethodTest.php b/mailpoet/tests/integration/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethodTest.php index 41453ba66e..8142109973 100644 --- a/mailpoet/tests/integration/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethodTest.php +++ b/mailpoet/tests/integration/Segments/DynamicSegments/Filters/WooCommerceUsedShippingMethodTest.php @@ -23,99 +23,104 @@ class WooCommerceUsedShippingMethodTest extends \MailPoetTest { $customerId1 = $this->tester->createCustomer('c1@e.com'); $customerId2 = $this->tester->createCustomer('c2@e.com'); $customerId3 = $this->tester->createCustomer('c3@e.com'); + $customerId4 = $this->tester->createCustomer('c4@e.com'); - $this->createOrder($customerId1, Carbon::now(), 'Flat rate'); - $this->createOrder($customerId2, Carbon::now(), 'Local pickup'); - $this->createOrder($customerId3, Carbon::now(), 'Flat rate'); - $this->createOrder($customerId3, Carbon::now(), 'Free shipping'); + $this->createOrder($customerId1, Carbon::now(), 'flat_rate', 1); + $this->createOrder($customerId2, Carbon::now(), 'local_pickup', 2); + $this->createOrder($customerId2, Carbon::now(), 'flat_rate', 4); + $this->createOrder($customerId3, Carbon::now(), 'flat_rate', 1); + $this->createOrder($customerId3, Carbon::now(), 'free_shipping', 3); + $this->createOrder($customerId4, Carbon::now(), 'flat_rate', 4); - $this->assertFilterReturnsEmails('any', ['Flat rate'], 1, ['c1@e.com', 'c3@e.com']); - $this->assertFilterReturnsEmails('any', ['Local pickup'], 1, ['c2@e.com']); - $this->assertFilterReturnsEmails('any', ['Nonexistent method'], 1000, []); + $this->assertFilterReturnsEmails('any', ['flat_rate:1'], 1, ['c1@e.com', 'c3@e.com']); + $this->assertFilterReturnsEmails('any', ['local_pickup:2'], 1, ['c2@e.com']); + $this->assertFilterReturnsEmails('any', ['local_pickup:2', 'flat_rate:1'], 1, ['c1@e.com', 'c2@e.com', 'c3@e.com']); + $this->assertFilterReturnsEmails('any', ['nonexistent_method:1'], 1000, []); } public function testItWorksWithAllOperator(): void { $customerId1 = $this->tester->createCustomer('c1@e.com'); - $this->createOrder($customerId1, Carbon::now(), 'Flat rate'); - $this->createOrder($customerId1, Carbon::now(), 'Flat rate'); + $this->createOrder($customerId1, Carbon::now(), 'flat_rate', 1); + $this->createOrder($customerId1, Carbon::now(), 'flat_rate', 1); $customerId2 = $this->tester->createCustomer('c2@e.com'); - $this->createOrder($customerId2, Carbon::now(), 'Free shipping'); + $this->createOrder($customerId2, Carbon::now(), 'free_shipping', 2); $customerId3 = $this->tester->createCustomer('c3@e.com'); - $this->createOrder($customerId3, Carbon::now(), 'Free shipping'); - $this->createOrder($customerId3, Carbon::now(), 'Flat rate'); + $this->createOrder($customerId3, Carbon::now(), 'free_shipping', 2); + $this->createOrder($customerId3, Carbon::now(), 'flat_rate', 1); - $this->assertfilterreturnsemails('all', ['Flat rate'], 1, ['c1@e.com', 'c3@e.com']); - $this->assertFilterReturnsEmails('all', ['Free shipping'], 1, ['c2@e.com', 'c3@e.com']); - $this->assertFilterReturnsEmails('all', ['Free shipping', 'Flat rate'], 1, ['c3@e.com']); - $this->assertFilterReturnsEmails('all', ['Nonexistent method', 'Flat rate'], 1000, []); + $this->assertfilterreturnsemails('all', ['flat_rate:1'], 1, ['c1@e.com', 'c3@e.com']); + $this->assertFilterReturnsEmails('all', ['free_shipping:2'], 1, ['c2@e.com', 'c3@e.com']); + $this->assertFilterReturnsEmails('all', ['free_shipping:2', 'flat_rate:1'], 1, ['c3@e.com']); + $this->assertFilterReturnsEmails('all', ['nonexistent_method:1', 'flat_rate:1'], 1000, []); } public function testItWorksWithNoneOperator(): void { $customerId1 = $this->tester->createCustomer('c1@e.com'); - $this->createOrder($customerId1, Carbon::now(), 'Flat rate'); - $this->createOrder($customerId1, Carbon::now(), 'Flat rate'); + $this->createOrder($customerId1, Carbon::now(), 'flat_rate', 1); + $this->createOrder($customerId1, Carbon::now(), 'flat_rate', 1); $customerId2 = $this->tester->createCustomer('c2@e.com'); - $this->createOrder($customerId2, Carbon::now(), 'Free shipping'); + $this->createOrder($customerId2, Carbon::now(), 'free_shipping', 2); $customerId3 = $this->tester->createCustomer('c3@e.com'); - $this->createOrder($customerId3, Carbon::now(), 'Free shipping'); - $this->createOrder($customerId3, Carbon::now(), 'Flat rate'); + $this->createOrder($customerId3, Carbon::now(), 'free_shipping', 2); + $this->createOrder($customerId3, Carbon::now(), 'flat_rate', 1); (new Subscriber)->withEmail('sub@e.com')->create(); - $this->assertFilterReturnsEmails('none', ['Flat rate'], 1, ['sub@e.com', 'c2@e.com']); - $this->assertFilterReturnsEmails('none', ['Free shipping'], 1, ['sub@e.com', 'c1@e.com']); - $this->assertFilterReturnsEmails('none', ['Nonexistent method'], 1000, ['sub@e.com', 'c1@e.com', 'c2@e.com', 'c3@e.com']); - $this->assertFilterReturnsEmails('none', ['Flat rate', 'Free shipping'], 1, ['sub@e.com']); + $this->assertFilterReturnsEmails('none', ['flat_rate:1'], 1, ['sub@e.com', 'c2@e.com']); + $this->assertFilterReturnsEmails('none', ['free_shipping:2'], 1, ['sub@e.com', 'c1@e.com']); + $this->assertFilterReturnsEmails('none', ['nonexistent_method:1'], 1000, ['sub@e.com', 'c1@e.com', 'c2@e.com', 'c3@e.com']); + $this->assertFilterReturnsEmails('none', ['flat_rate:1', 'free_shipping:2'], 1, ['sub@e.com']); } public function testItWorksWithDateRanges(): void { $customerId1 = $this->tester->createCustomer('c1@e.com'); - $this->createOrder($customerId1, Carbon::now()->subDays(2)->addMinute(), 'Flat rate'); - $this->createOrder($customerId1, Carbon::now()->subDays(5)->addMinute(), 'Free shipping'); + $this->createOrder($customerId1, Carbon::now()->subDays(2)->addMinute(), 'flat_rate', 1); + $this->createOrder($customerId1, Carbon::now()->subDays(5)->addMinute(), 'free_shipping', 2); $customerId2 = $this->tester->createCustomer('c2@e.com'); - $this->createOrder($customerId2, Carbon::now()->subDays(100)->addMinute(), 'Local pickup'); - $this->assertFilterReturnsEmails('any', ['Flat rate'], 1, []); - $this->assertFilterReturnsEmails('any', ['Flat rate'], 2, ['c1@e.com']); - $this->assertFilterReturnsEmails('any', ['Free shipping'], 4, []); - $this->assertFilterReturnsEmails('any', ['Free shipping'], 5, ['c1@e.com']); - $this->assertFilterReturnsEmails('any', ['Local pickup'], 99, []); - $this->assertFilterReturnsEmails('any', ['Local pickup'], 100, ['c2@e.com']); - $this->assertFilterReturnsEmails('any', ['Local pickup', 'Flat rate'], 100, ['c1@e.com', 'c2@e.com']); + $this->createOrder($customerId2, Carbon::now()->subDays(100)->addMinute(), 'local_pickup', 3); + $this->assertFilterReturnsEmails('any', ['flat_rate:1'], 1, []); + $this->assertFilterReturnsEmails('any', ['flat_rate:1'], 2, ['c1@e.com']); + $this->assertFilterReturnsEmails('any', ['free_shipping:2'], 4, []); + $this->assertFilterReturnsEmails('any', ['free_shipping:2'], 5, ['c1@e.com']); + $this->assertFilterReturnsEmails('any', ['local_pickup:3'], 99, []); + $this->assertFilterReturnsEmails('any', ['local_pickup:3'], 100, ['c2@e.com']); + $this->assertFilterReturnsEmails('any', ['local_pickup:3', 'flat_rate:1'], 100, ['c1@e.com', 'c2@e.com']); - $this->assertFilterReturnsEmails('all', ['Flat rate'], 1, []); - $this->assertFilterReturnsEmails('all', ['Flat rate'], 2, ['c1@e.com']); - $this->assertFilterReturnsEmails('all', ['Flat rate', 'Free shipping'], 2, []); - $this->assertFilterReturnsEmails('all', ['Flat rate', 'Free shipping'], 5, ['c1@e.com']); + $this->assertFilterReturnsEmails('all', ['flat_rate:1'], 1, []); + $this->assertFilterReturnsEmails('all', ['flat_rate:1'], 2, ['c1@e.com']); + $this->assertFilterReturnsEmails('all', ['flat_rate:1', 'free_shipping:2'], 2, []); + $this->assertFilterReturnsEmails('all', ['flat_rate:1', 'free_shipping:2'], 5, ['c1@e.com']); - $this->assertFilterReturnsEmails('none', ['Flat rate'], 1, ['c1@e.com', 'c2@e.com']); - $this->assertFilterReturnsEmails('none', ['Flat rate'], 2, ['c2@e.com']); - $this->assertFilterReturnsEmails('none', ['Free shipping'], 2, ['c1@e.com', 'c2@e.com']); - $this->assertFilterReturnsEmails('none', ['Free shipping'], 5, ['c2@e.com']); + $this->assertFilterReturnsEmails('none', ['flat_rate:1'], 1, ['c1@e.com', 'c2@e.com']); + $this->assertFilterReturnsEmails('none', ['flat_rate:1'], 2, ['c2@e.com']); + $this->assertFilterReturnsEmails('none', ['free_shipping:2'], 2, ['c1@e.com', 'c2@e.com']); + $this->assertFilterReturnsEmails('none', ['free_shipping:2'], 5, ['c2@e.com']); } - private function assertFilterReturnsEmails(string $operator, array $shippingMethods, int $days, array $expectedEmails): void { + private function assertFilterReturnsEmails(string $operator, array $shippingMethodStrings, int $days, array $expectedEmails): void { $filterData = new DynamicSegmentFilterData(DynamicSegmentFilterData::TYPE_WOOCOMMERCE, WooCommerceUsedShippingMethod::ACTION, [ 'operator' => $operator, - 'shipping_methods' => $shippingMethods, + 'shipping_methods' => $shippingMethodStrings, 'used_shipping_method_days' => $days, ]); $emails = $this->tester->getSubscriberEmailsMatchingDynamicFilter($filterData, $this->filter); $this->assertEqualsCanonicalizing($expectedEmails, $emails); } - private function createOrder(int $customerId, Carbon $createdAt, string $shippingMethodTitle): int { + private function createOrder(int $customerId, Carbon $createdAt, string $shippingMethodId, $shippingInstanceId): int { $order = $this->tester->createWooCommerceOrder(); $order->set_customer_id($customerId); $order->set_date_created($createdAt->toDateTimeString()); $order->set_status('wc-completed'); $shippingItem = new \WC_Order_Item_Shipping(); - $shippingItem->set_method_title($shippingMethodTitle); + $shippingItem->set_method_id($shippingMethodId); + $shippingItem->set_instance_id($shippingInstanceId); $order->add_item($shippingItem); $order->save();