From f5a0b67e9ff1d19c982ef643eb2c95f5c9973d63 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 28 Sep 2023 12:05:02 -0300 Subject: [PATCH] Fix message displayed when a new segment is created There was a bug and the code was displaying `Segment succesfully updated!` instead of `Segment succesfull added!` when the user created a new segment. This commit fixes this problem. The issue was that Number(match.params.id) returns `NaN` instead of `undefined` when there is no segment ID. So the check inside handleSave() was always calling messages.onUpdate(). Since the segment ID was never used. I replaced it with a boolean and renamed the variable from `segmentId` to `isNewSegment` to better indicate how it is used. [MAILPOET-5615] --- mailpoet/assets/js/src/segments/dynamic/editor.tsx | 5 ++++- mailpoet/assets/js/src/segments/dynamic/form.tsx | 6 +++--- mailpoet/assets/js/src/segments/dynamic/store/actions.ts | 8 ++++---- .../Segments/CreateSegmentEmailAbsoluteCountCest.php | 2 +- mailpoet/tests/acceptance/Segments/ManageSegmentsCest.php | 2 +- 5 files changed, 13 insertions(+), 10 deletions(-) diff --git a/mailpoet/assets/js/src/segments/dynamic/editor.tsx b/mailpoet/assets/js/src/segments/dynamic/editor.tsx index 3e1e4b3fa2..1bbd8fd77a 100644 --- a/mailpoet/assets/js/src/segments/dynamic/editor.tsx +++ b/mailpoet/assets/js/src/segments/dynamic/editor.tsx @@ -27,6 +27,9 @@ export function Editor(): JSX.Element { }; }, [match.params.id, pageLoaded, pageUnloaded]); + const isNewSegment = + match.params.id === undefined || Number.isNaN(Number(match.params.id)); + return ( <> @@ -42,7 +45,7 @@ export function Editor(): JSX.Element { -
+ ); } diff --git a/mailpoet/assets/js/src/segments/dynamic/form.tsx b/mailpoet/assets/js/src/segments/dynamic/form.tsx index ab4182fbb4..6d27777a94 100644 --- a/mailpoet/assets/js/src/segments/dynamic/form.tsx +++ b/mailpoet/assets/js/src/segments/dynamic/form.tsx @@ -27,7 +27,7 @@ import { } from './types'; interface Props { - segmentId?: number; + isNewSegment: boolean; } const FiltersBefore = Hooks.applyFilters( @@ -43,7 +43,7 @@ const FilterAfter = Hooks.applyFilters( (): JSX.Element =>
, ); -export function Form({ segmentId }: Props): JSX.Element { +export function Form({ isNewSegment }: Props): JSX.Element { const segment: Segment = useSelect( (select) => select(storeName).getSegment(), [], @@ -194,7 +194,7 @@ export function Form({ segmentId }: Props): JSX.Element { type="submit" onClick={(e): void => { e.preventDefault(); - void handleSave(segmentId); + void handleSave(isNewSegment); }} isDisabled={ !isFormValid(segment.filters) || diff --git a/mailpoet/assets/js/src/segments/dynamic/store/actions.ts b/mailpoet/assets/js/src/segments/dynamic/store/actions.ts index 27589cc63a..24ade428e7 100644 --- a/mailpoet/assets/js/src/segments/dynamic/store/actions.ts +++ b/mailpoet/assets/js/src/segments/dynamic/store/actions.ts @@ -131,7 +131,7 @@ const messages = { }, }; -export function* handleSave(segmentId?: number): Generator<{ +export function* handleSave(isNewSegment: boolean): Generator<{ type: string; segment?: AnyFormItem; }> { @@ -147,10 +147,10 @@ export function* handleSave(segmentId?: number): Generator<{ if (success) { window.location.href = 'admin.php?page=mailpoet-segments#/segments'; - if (segmentId !== undefined) { - messages.onUpdate(); - } else { + if (isNewSegment) { messages.onCreate(segment); + } else { + messages.onUpdate(); } } else { yield setErrors(error as string[]); diff --git a/mailpoet/tests/acceptance/Segments/CreateSegmentEmailAbsoluteCountCest.php b/mailpoet/tests/acceptance/Segments/CreateSegmentEmailAbsoluteCountCest.php index 9905200ecb..bd657dca9d 100644 --- a/mailpoet/tests/acceptance/Segments/CreateSegmentEmailAbsoluteCountCest.php +++ b/mailpoet/tests/acceptance/Segments/CreateSegmentEmailAbsoluteCountCest.php @@ -64,7 +64,7 @@ class CreateSegmentEmailAbsoluteCountCest { $i->fillField('[data-automation-id="segment-number-of-days"]', 3); $i->waitForText('This segment has 2 subscribers'); $i->click('Save'); - $i->waitForNoticeAndClose('Segment successfully updated!'); + $i->waitForNoticeAndClose('Segment successfully added!'); $i->wantTo('Edit the segment'); $i->amOnMailpoetPage('Segments'); diff --git a/mailpoet/tests/acceptance/Segments/ManageSegmentsCest.php b/mailpoet/tests/acceptance/Segments/ManageSegmentsCest.php index dddded8323..4cb33edc50 100644 --- a/mailpoet/tests/acceptance/Segments/ManageSegmentsCest.php +++ b/mailpoet/tests/acceptance/Segments/ManageSegmentsCest.php @@ -92,7 +92,7 @@ class ManageSegmentsCest { $i->waitForText('This segment has 3 subscribers'); $i->waitForElementClickable('button[type="submit"]'); $i->click('Save'); - $i->waitForNoticeAndClose('Segment successfully updated!'); + $i->waitForNoticeAndClose('Segment successfully added!'); $i->waitForText($segmentTitle); $i->wantTo('Edit existing segment');