Cavemanon/piratepoet
7
Fork 0
Code Pull Requests Actions Releases 16 Activity

Remove backward compatibility code for email action filters

Browse Source 
This commit cleans up code that is no longer needed after email actions filter data migration.
[MAILPOET-3951]
This commit is contained in:
Rostislav Wolny
2022-01-14 10:54:02 +01:00
committed by Veljko V
parent dc55aaa566
commit 75ab2338d6
9 changed files with 12 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
mailpoet/assets/js/src/segments/dynamic/dynamic_segments_filters/email.tsx
View File

@ -56,7 +56,6 @@ const componentsMap = {
[EmailActionTypes.OPENS_ABSOLUTE_COUNT]: EmailOpensAbsoluteCountFields,
[EmailActionTypes.MACHINE_OPENS_ABSOLUTE_COUNT]: EmailOpensAbsoluteCountFields,
[EmailActionTypes.CLICKED]: EmailClickStatisticsFields,
[EmailActionTypes.NOT_CLICKED]: EmailClickStatisticsFields,
[EmailActionTypes.OPENED]: EmailOpenStatisticsFields,
[EmailActionTypes.MACHINE_OPENED]: EmailOpenStatisticsFields,
[EmailActionTypes.CLICKED_ANY]: null,

6
mailpoet/assets/js/src/segments/dynamic/dynamic_segments_filters/email_statistics_clicks.tsx
View File

@ -89,11 +89,7 @@ export const EmailClickStatisticsFields: React.FunctionComponent<Props> = ({ fil
) {
updateSegmentFilter({ operator: AnyValueTypes.ANY }, filterIndex);
}
// Temporary BC fix
if (segment.link_id && !segment.link_ids) {
updateSegmentFilter({ link_ids: [Number(segment.link_id)] }, filterIndex);
}
}, [segment.link_ids, segment.link_id, segment.operator, filterIndex, updateSegmentFilter]);
}, [segment.operator, filterIndex, updateSegmentFilter]);
return (
<>

4
mailpoet/assets/js/src/segments/dynamic/types.ts
View File

@ -13,7 +13,6 @@ export enum EmailActionTypes {
MACHINE_OPENED = 'machineOpened',
CLICKED = 'clicked',
CLICKED_ANY = 'clickedAny',
NOT_CLICKED = 'notClicked',
}
export enum SubscriberActionTypes {
@ -90,8 +89,7 @@ export interface WooCommerceSubscriptionFormItem extends FormItem {
export interface EmailFormItem extends FormItem {
newsletter_id?: string;
newsletters: number[]
link_id?: string;
newsletters?: number[];
link_ids?: string[];
operator?: string;
opens?: string;

22
mailpoet/lib/API/JSON/ResponseBuilders/DynamicSegmentsResponseBuilder.php
View File

@ -5,7 +5,6 @@ namespace MailPoet\API\JSON\ResponseBuilders;
use MailPoet\Entities\DynamicSegmentFilterData;
use MailPoet\Entities\SegmentEntity;
use MailPoet\Entities\SubscriberEntity;
use MailPoet\Segments\DynamicSegments\Filters\EmailAction;
use MailPoet\Segments\SegmentDependencyValidator;
use MailPoet\Segments\SegmentSubscribersRepository;
use MailPoet\Subscribers\SubscribersCountsController;
@ -61,28 +60,9 @@ class DynamicSegmentsResponseBuilder {
// new filters are always array, they support multiple values, the old didn't convert old filters to new format
$filter['wordpressRole'] = [$filter['wordpressRole']];
}
if (($filter['segmentType'] === DynamicSegmentFilterData::TYPE_EMAIL)) {
// compatibility with older filters
if ((($filter['action'] === EmailAction::ACTION_OPENED) || ($filter['action'] === EmailAction::ACTION_NOT_OPENED) || ($filter['action'] === EmailAction::ACTION_MACHINE_OPENED))) {
if (isset($filter['newsletter_id']) && !isset($filter['newsletters'])) {
// make sure the newsletters are an array
$filter['newsletters'] = [intval($filter['newsletter_id'])];
unset($filter['newsletter_id']);
}
} else {
if (($filter['segmentType'] === DynamicSegmentFilterData::TYPE_EMAIL) && isset($filter['newsletter_id'])) {
$filter['newsletter_id'] = intval($filter['newsletter_id']);
}
if ($filter['action'] === EmailAction::ACTION_NOT_OPENED) {
// convert not opened
$filter['action'] = EmailAction::ACTION_OPENED;
$filter['operator'] = DynamicSegmentFilterData::OPERATOR_NONE;
}
if ($filter['action'] === EmailAction::ACTION_NOT_CLICKED) {
// convert not clicked
$filter['action'] = EmailAction::ACTION_CLICKED;
$filter['operator'] = DynamicSegmentFilterData::OPERATOR_NONE;
}
}
$filters[] = $filter;
}
$data['filters'] = $filters;

19
mailpoet/lib/Segments/DynamicSegments/Filters/EmailAction.php
View File

@ -52,7 +52,7 @@ class EmailAction implements Filter {
$action = $filterData->getAction();
$parameterSuffix = (string)($filter->getId() ?? Security::generateRandomString());
if (in_array($action, self::CLICK_ACTIONS, true)) {
if ($action === self::ACTION_CLICKED) {
return $this->applyForClickedActions($queryBuilder, $filterData, $parameterSuffix);
} else {
return $this->applyForOpenedActions($queryBuilder, $filterData, $parameterSuffix);
@ -63,11 +63,9 @@ class EmailAction implements Filter {
$operator = $filterData->getParam('operator') ?? DynamicSegmentFilterData::OPERATOR_ANY;
$action = $filterData->getAction();
$newsletterId = $filterData->getParam('newsletter_id');
// Temporary backward compatibility for segments saved with link_id
$linkId = $filterData->getParam('link_id') ? (int)$filterData->getParam('link_id') : null;
$linkIds = $filterData->getParam('link_ids');
if (!is_array($linkIds)) {
$linkIds = $linkId ? [$linkId] : [];
$linkIds = [];
}
$statsSentTable = $this->entityManager->getClassMetadata(StatisticsNewsletterEntity::class)->getTableName();
@ -129,20 +127,7 @@ class EmailAction implements Filter {
private function applyForOpenedActions(QueryBuilder $queryBuilder, DynamicSegmentFilterData $filterData, string $parameterSuffix) {
$operator = $filterData->getParam('operator') ?? DynamicSegmentFilterData::OPERATOR_ANY;
$action = $filterData->getAction();
if ($action === self::ACTION_NOT_OPENED) {
// for backward compatibility with old segments
$action = self::ACTION_OPENED;
$operator = DynamicSegmentFilterData::OPERATOR_NONE;
}
$newsletterId = $filterData->getParam('newsletter_id');
if ($newsletterId) {
// for backward compatibility with old segments
$newsletters = [(int)$newsletterId];
} else {
$newsletters = $filterData->getParam('newsletters');
}
$statsSentTable = $this->entityManager->getClassMetadata(StatisticsNewsletterEntity::class)->getTableName();
$subscribersTable = $this->entityManager->getClassMetadata(SubscriberEntity::class)->getTableName();

5
mailpoet/tasks/phpstan/phpstan-7-baseline.neon
View File

@ -1052,11 +1052,6 @@ parameters:
count: 1
path: ../../lib/Segments/DynamicSegments/FilterDataMapper.php
-
message: "#^Cannot cast mixed to int\\.$#"
count: 2
path: ../../lib/Segments/DynamicSegments/Filters/EmailAction.php
-
message: "#^Parameter \\#1 \\$var of function count expects array\\|Countable, mixed given\\.$#"
count: 1

5
mailpoet/tasks/phpstan/phpstan-8-baseline.neon
View File

@ -1050,11 +1050,6 @@ parameters:
count: 1
path: ../../lib/Segments/DynamicSegments/FilterDataMapper.php
-
message: "#^Cannot cast mixed to int\\.$#"
count: 2
path: ../../lib/Segments/DynamicSegments/Filters/EmailAction.php
-
message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#"
count: 1

5
mailpoet/tasks/phpstan/phpstan-8.1-baseline.neon
View File

@ -1050,11 +1050,6 @@ parameters:
count: 1
path: ../../lib/Segments/DynamicSegments/FilterDataMapper.php
-
message: "#^Cannot cast mixed to int\\.$#"
count: 2
path: ../../lib/Segments/DynamicSegments/Filters/EmailAction.php
-
message: "#^Parameter \\#1 \\$value of function count expects array\\|Countable, mixed given\\.$#"
count: 1

57
mailpoet/tests/integration/Segments/DynamicSegments/Filters/EmailActionTest.php
View File

@ -101,7 +101,7 @@ class EmailActionTest extends \MailPoetTest {
public function testGetOpened() {
$segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_OPENED, [
'newsletter_id' => (int)$this->newsletter->getId(),
'newsletters' => [$this->newsletter->getId()],
]);
$statement = $this->emailAction->apply($this->getQueryBuilder(), $segmentFilter)->execute();
$this->assertInstanceOf(Statement::class, $statement);
@ -115,19 +115,6 @@ class EmailActionTest extends \MailPoetTest {
expect($subscriber2->getEmail())->equals('opened_not_clicked@example.com');
}
public function testNotOpened() {
$segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_NOT_OPENED, [
'newsletter_id' => (int)$this->newsletter->getId(),
]);
$statement = $this->emailAction->apply($this->getQueryBuilder(), $segmentFilter)->execute();
$this->assertInstanceOf(Statement::class, $statement);
$result = $statement->fetchAllAssociative();
expect(count($result))->equals(1);
$subscriber1 = $this->entityManager->find(SubscriberEntity::class, $result[0]['id']);
$this->assertInstanceOf(SubscriberEntity::class, $subscriber1);
expect($subscriber1->getEmail())->equals('not_opened@example.com');
}
public function testGetOpenedOperatorAny() {
$segmentFilter = $this->getSegmentFilter(
EmailAction::ACTION_OPENED,
@ -294,11 +281,12 @@ class EmailActionTest extends \MailPoetTest {
expect($subscriber2->getEmail())->equals('not_opened@example.com');
}
public function testGetNotClickedWithLink() {
public function testGetClickedWithNoneAndNoSavedLinks() {
$this->createClickedLink('http://example.com', $this->newsletter, $this->subscriberOpenedClicked); // id 1
$segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_NOT_CLICKED, [
$segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_CLICKED, [
'newsletter_id' => (int)$this->newsletter->getId(),
'link_ids' => [1],
'link_ids' => [],
'operator' => DynamicSegmentFilterData::OPERATOR_NONE,
]);
$statement = $this->emailAction->apply($this->getQueryBuilder(), $segmentFilter)->execute();
$this->assertInstanceOf(Statement::class, $statement);
@ -312,41 +300,6 @@ class EmailActionTest extends \MailPoetTest {
expect($subscriber2->getEmail())->equals('not_opened@example.com');
}
public function testGetNotClickedWithWrongLink() {
$segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_NOT_CLICKED, [
'newsletter_id' => (int)$this->newsletter->getId(),
'link_ids' => [2],
]);
$statement = $this->emailAction->apply($this->getQueryBuilder(), $segmentFilter)->execute();
$this->assertInstanceOf(Statement::class, $statement);
$result = $statement->fetchAllAssociative();
expect(count($result))->equals(3);
$subscriber1 = $this->entityManager->find(SubscriberEntity::class, $result[0]['id']);
$this->assertInstanceOf(SubscriberEntity::class, $subscriber1);
$subscriber2 = $this->entityManager->find(SubscriberEntity::class, $result[1]['id']);
$this->assertInstanceOf(SubscriberEntity::class, $subscriber2);
$subscriber3 = $this->entityManager->find(SubscriberEntity::class, $result[2]['id']);
$this->assertInstanceOf(SubscriberEntity::class, $subscriber3);
expect($subscriber1->getEmail())->equals('opened_clicked@example.com');
expect($subscriber2->getEmail())->equals('opened_not_clicked@example.com');
expect($subscriber3->getEmail())->equals('not_opened@example.com');
}
public function testGetNotClickedWithoutLink() {
$this->createClickedLink('http://example.com', $this->newsletter, $this->subscriberOpenedClicked); // id 1
$segmentFilter = $this->getSegmentFilter(EmailAction::ACTION_NOT_CLICKED, ['newsletter_id' => (int)$this->newsletter->getId()]);
$statement = $this->emailAction->apply($this->getQueryBuilder(), $segmentFilter)->execute();
$this->assertInstanceOf(Statement::class, $statement);
$result = $statement->fetchAllAssociative();
expect(count($result))->equals(2);
$subscriber1 = $this->entityManager->find(SubscriberEntity::class, $result[0]['id']);
$this->assertInstanceOf(SubscriberEntity::class, $subscriber1);
$subscriber2 = $this->entityManager->find(SubscriberEntity::class, $result[1]['id']);
$this->assertInstanceOf(SubscriberEntity::class, $subscriber2);
expect($subscriber1->getEmail())->equals('opened_not_clicked@example.com');
expect($subscriber2->getEmail())->equals('not_opened@example.com');
}
public function testOpensNotIncludeMachineOpens() {
$subscriberOpenedMachine = $this->createSubscriber('opened_machine@example.com');
$this->createStatsNewsletter($subscriberOpenedMachine, $this->newsletter);