Make name a property of Step.args instead of Step

[MAILPOET-4445]
This commit is contained in:
David Remer
2022-08-12 11:50:45 +03:00
committed by Veljko V
parent 2a09646e8d
commit 24849afb7a
16 changed files with 35 additions and 80 deletions

View File

@ -1,21 +1,18 @@
import { Dropdown, TextControl } from '@wordpress/components'; import { Dropdown, TextControl } from '@wordpress/components';
import { edit, Icon } from '@wordpress/icons'; import { edit, Icon } from '@wordpress/icons';
import { dispatch, useSelect } from '@wordpress/data';
import { PlainBodyTitle } from './plain-body-title'; import { PlainBodyTitle } from './plain-body-title';
import { TitleActionButton } from './title-action-button'; import { TitleActionButton } from './title-action-button';
import { store } from '../../store';
import { Step } from '../workflow/types';
type Props = { type Props = {
step: Step; currentName: string;
defaultName: string;
update: (value: string) => void;
}; };
export function StepName({ step }: Props): JSX.Element { export function StepName({
const { stepType } = useSelect( currentName,
(select) => ({ defaultName,
stepType: select(store).getStepType(step.key), update,
}), }: Props): JSX.Element {
[],
);
return ( return (
<Dropdown <Dropdown
className="mailpoet-step-name-dropdown" className="mailpoet-step-name-dropdown"
@ -23,7 +20,7 @@ export function StepName({ step }: Props): JSX.Element {
position="bottom left" position="bottom left"
renderToggle={({ isOpen, onToggle }) => ( renderToggle={({ isOpen, onToggle }) => (
<PlainBodyTitle <PlainBodyTitle
title={step.name && step.name.length > 0 ? step.name : stepType.title} title={currentName.length > 0 ? currentName : defaultName}
> >
<TitleActionButton <TitleActionButton
onClick={onToggle} onClick={onToggle}
@ -38,11 +35,9 @@ export function StepName({ step }: Props): JSX.Element {
<TextControl <TextControl
label="Step name" label="Step name"
className="mailpoet-step-name-input" className="mailpoet-step-name-input"
placeholder={stepType.title} placeholder={defaultName}
value={step.name} value={currentName}
onChange={(value) => { onChange={update}
dispatch(store).updateStepName(step.id, value);
}}
help="Give the automation step a name that indicates its purpose. E.g help="Give the automation step a name that indicates its purpose. E.g
“Abandoned cart recovery”. This name will be displayed only to you and not to the clients." “Abandoned cart recovery”. This name will be displayed only to you and not to the clients."
/> />

View File

@ -36,11 +36,8 @@ function getTitle(step: StepData): string {
if (step.type === 'trigger') { if (step.type === 'trigger') {
return 'Trigger'; return 'Trigger';
} }
if (step.name) {
return step.name;
}
const selectedStepType = select(store).getStepType(step.key); const selectedStepType = select(store).getStepType(step.key);
return selectedStepType.title; return selectedStepType ? selectedStepType.title : '';
} }
function getSubtitle(step: StepData): JSX.Element | string { function getSubtitle(step: StepData): JSX.Element | string {

View File

@ -1,7 +1,6 @@
export type Step = { export type Step = {
id: string; id: string;
type: 'trigger' | 'action'; type: 'trigger' | 'action';
name: string | null;
key: string; key: string;
next_step_id?: string; next_step_id?: string;
args: Record<string, unknown>; args: Record<string, unknown>;

View File

@ -95,11 +95,3 @@ export function updateStepArgs(stepId, name, value) {
value, value,
}; };
} }
export function updateStepName(stepId, name) {
return {
type: 'UPDATE_STEP_NAME',
stepId,
name,
};
}

View File

@ -50,24 +50,6 @@ export function reducer(state: State, action: Action): State {
[action.stepType.key]: action.stepType, [action.stepType.key]: action.stepType,
}, },
}; };
case 'UPDATE_STEP_NAME': {
const step = {
...state.workflowData.steps[action.stepId],
name: action.name,
};
return {
...state,
workflowData: {
...state.workflowData,
steps: {
...state.workflowData.steps,
[action.stepId]: step,
},
},
workflowSaved: false,
selectedStep: step,
};
}
case 'UPDATE_STEP_ARGS': { case 'UPDATE_STEP_ARGS': {
const prevArgs = state.workflowData.steps[action.stepId].args ?? {}; const prevArgs = state.workflowData.steps[action.stepId].args ?? {};

View File

@ -31,16 +31,23 @@ function SingleLineTextareaControl(
} }
export function EmailPanel(): JSX.Element { export function EmailPanel(): JSX.Element {
const { selectedStep } = useSelect( const { selectedStep, selectedStepType } = useSelect(
(select) => ({ (select) => ({
selectedStep: select(store).getSelectedStep(), selectedStep: select(store).getSelectedStep(),
selectedStepType: select(store).getSelectedStepType(),
}), }),
[], [],
); );
return ( return (
<PanelBody opened> <PanelBody opened>
<StepName step={selectedStep} /> <StepName
currentName={(selectedStep.args.name as string) ?? ''}
defaultName={selectedStepType.title}
update={(value) => {
dispatch(store).updateStepArgs(selectedStep.id, 'name', value);
}}
/>
<TextControl <TextControl
label="“From” name" label="“From” name"
placeholder="John Doe" placeholder="John Doe"

View File

@ -7,7 +7,7 @@ export const step: StepType = {
group: 'actions', group: 'actions',
title: 'Send email', title: 'Send email',
description: 'An email will be sent to subscriber', description: 'An email will be sent to subscriber',
subtitle: (data) => `Email ID ${data.args.email_id as string}`, subtitle: (data) => (data.args.name as string) ?? 'Send email',
foreground: '#996800', foreground: '#996800',
background: '#FCF9E8', background: '#FCF9E8',
icon: Icon, icon: Icon,

View File

@ -64,7 +64,6 @@ class AutomationEditor {
'steps' => array_map(function (Step $step) { 'steps' => array_map(function (Step $step) {
return [ return [
'id' => $step->getId(), 'id' => $step->getId(),
'name' => $step->getName(),
'type' => $step->getType(), 'type' => $step->getType(),
'key' => $step->getKey(), 'key' => $step->getKey(),
'next_step_id' => $step->getNextStepId(), 'next_step_id' => $step->getNextStepId(),

View File

@ -36,7 +36,6 @@ class UpdateStepsController {
return new Step( return new Step(
$data['id'], $data['id'],
$data['name'] ?? null,
$data['type'], $data['type'],
$data['key'], $data['key'],
$data['next_step_id'] ?? null, $data['next_step_id'] ?? null,

View File

@ -9,9 +9,6 @@ class Step {
/** @var string */ /** @var string */
private $id; private $id;
/** @var ?string */
private $name;
/** @var string */ /** @var string */
private $type; private $type;
@ -26,14 +23,12 @@ class Step {
public function __construct( public function __construct(
string $id, string $id,
?string $name,
string $type, string $type,
string $key, string $key,
?string $nextStepId = null, ?string $nextStepId = null,
array $args = [] array $args = []
) { ) {
$this->id = $id; $this->id = $id;
$this->name = $name;
$this->type = $type; $this->type = $type;
$this->key = $key; $this->key = $key;
$this->nextStepId = $nextStepId; $this->nextStepId = $nextStepId;
@ -44,10 +39,6 @@ class Step {
return $this->id; return $this->id;
} }
public function getName(): ?string {
return $this->name;
}
public function getType(): string { public function getType(): string {
return $this->type; return $this->type;
} }
@ -71,7 +62,6 @@ class Step {
public function toArray(): array { public function toArray(): array {
return [ return [
'id' => $this->id, 'id' => $this->id,
'name' => $this->name,
'type' => $this->type, 'type' => $this->type,
'key' => $this->key, 'key' => $this->key,
'next_step_id' => $this->nextStepId, 'next_step_id' => $this->nextStepId,

View File

@ -170,7 +170,6 @@ class Workflow {
foreach ($data as $step) { foreach ($data as $step) {
$steps[] = new Step( $steps[] = new Step(
$step['id'], $step['id'],
$step['name'] ?? null,
$step['type'], $step['type'],
$step['key'], $step['key'],
$step['next_step_id'] ?? null, $step['next_step_id'] ?? null,

View File

@ -31,7 +31,6 @@ class WorkflowsPutEndpoint extends Endpoint {
public static function getRequestSchema(): array { public static function getRequestSchema(): array {
$step = Builder::object([ $step = Builder::object([
'id' => Builder::string()->required(), 'id' => Builder::string()->required(),
'name' => Builder::string()->nullable(),
'type' => Builder::string()->required(), 'type' => Builder::string()->required(),
'key' => Builder::string()->required(), 'key' => Builder::string()->required(),
'args' => Builder::object(), 'args' => Builder::object(),
@ -56,7 +55,6 @@ class WorkflowsPutEndpoint extends Endpoint {
'steps' => array_map(function (Step $step) { 'steps' => array_map(function (Step $step) {
return [ return [
'id' => $step->getId(), 'id' => $step->getId(),
'name' => $step->getName(),
'type' => $step->getType(), 'type' => $step->getType(),
'key' => $step->getKey(), 'key' => $step->getKey(),
'next_step_id' => $step->getNextStepId(), 'next_step_id' => $step->getNextStepId(),

View File

@ -68,6 +68,7 @@ class SendEmailAction implements Action {
'reply_to_name' => Builder::string()->default($this->settings->get('reply_to.name')), 'reply_to_name' => Builder::string()->default($this->settings->get('reply_to.name')),
'reply_to_address' => Builder::string()->default($this->settings->get('reply_to.address')), 'reply_to_address' => Builder::string()->default($this->settings->get('reply_to.address')),
'ga_campaign' => Builder::string(), 'ga_campaign' => Builder::string(),
'name' => Builder::string()->default(__('Send email', 'mailpoet')),
]); ]);
} }

View File

@ -78,7 +78,6 @@ class WorkflowBuilder {
private function delayStep(?int $delay, string $delayType): Step { private function delayStep(?int $delay, string $delayType): Step {
return new Step( return new Step(
$this->uniqueId(), $this->uniqueId(),
null,
Step::TYPE_ACTION, Step::TYPE_ACTION,
$this->delayAction->getKey(), $this->delayAction->getKey(),
null, null,
@ -92,7 +91,6 @@ class WorkflowBuilder {
private function segmentSubscribedTriggerStep(?int $segmentId = null): Step { private function segmentSubscribedTriggerStep(?int $segmentId = null): Step {
return new Step( return new Step(
$this->uniqueId(), $this->uniqueId(),
null,
Step::TYPE_TRIGGER, Step::TYPE_TRIGGER,
$this->segmentSubscribedTrigger->getKey(), $this->segmentSubscribedTrigger->getKey(),
null, null,
@ -105,7 +103,6 @@ class WorkflowBuilder {
private function sendEmailActionStep(): Step { private function sendEmailActionStep(): Step {
return new Step( return new Step(
$this->uniqueId(), $this->uniqueId(),
__('Send email', 'mailpoet'),
Step::TYPE_ACTION, Step::TYPE_ACTION,
$this->sendEmailAction->getKey(), $this->sendEmailAction->getKey(),
null, null,

View File

@ -16,7 +16,7 @@ class DelayActionTest extends \MailPoetTest {
*/ */
public function testItCalculatesDelayTypesCorrectly(int $delay, string $type, int $expectation) { public function testItCalculatesDelayTypesCorrectly(int $delay, string $type, int $expectation) {
$step = new Step("1", null, 'core:delay', 'core:delay', 'next-step', [ $step = new Step("1", 'core:delay', 'core:delay', 'next-step', [
'delay' => $delay, 'delay' => $delay,
'delay_type' => $type, 'delay_type' => $type,
]); ]);
@ -81,7 +81,7 @@ class DelayActionTest extends \MailPoetTest {
*/ */
public function testDelayActionInvalidatesOutsideOfBoundaries(int $delay, bool $expectation) { public function testDelayActionInvalidatesOutsideOfBoundaries(int $delay, bool $expectation) {
$step = new Step("1", null, 'core:delay', 'core:delay', 'next-step', [ $step = new Step("1", 'core:delay', 'core:delay', 'next-step', [
'delay' => $delay, 'delay' => $delay,
'delay_type' => "HOURS", 'delay_type' => "HOURS",
]); ]);

View File

@ -60,7 +60,7 @@ class SendEmailActionTest extends \MailPoetTest {
$this->segmentSubject = $this->diContainer->get(SegmentSubject::class); $this->segmentSubject = $this->diContainer->get(SegmentSubject::class);
$this->email = (new Newsletter())->withAutomationType()->create(); $this->email = (new Newsletter())->withAutomationType()->create();
$this->step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $this->email->getId()]); $this->step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $this->email->getId()]);
$this->workflow = new Workflow('test-workflow', []); $this->workflow = new Workflow('test-workflow', []);
} }
@ -78,13 +78,13 @@ class SendEmailActionTest extends \MailPoetTest {
} }
public function testItIsNotValidIfStepHasNoEmail(): void { public function testItIsNotValidIfStepHasNoEmail(): void {
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, []); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, []);
expect($this->action->isValid($this->getSubjects(), $step, $this->workflow))->false(); expect($this->action->isValid($this->getSubjects(), $step, $this->workflow))->false();
} }
public function testItRequiresAutomationEmailType(): void { public function testItRequiresAutomationEmailType(): void {
$newsletter = (new Newsletter())->withPostNotificationsType()->create(); $newsletter = (new Newsletter())->withPostNotificationsType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $newsletter->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $newsletter->getId()]);
expect($this->action->isValid($this->getSubjects(), $step, $this->workflow))->false(); expect($this->action->isValid($this->getSubjects(), $step, $this->workflow))->false();
} }
@ -97,7 +97,7 @@ class SendEmailActionTest extends \MailPoetTest {
$subjects = $this->getLoadedSubjects($subscriber, $segment); $subjects = $this->getLoadedSubjects($subscriber, $segment);
$email = (new Newsletter())->withAutomationType()->create(); $email = (new Newsletter())->withAutomationType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]);
$workflow = new Workflow('some-workflow', [$step]); $workflow = new Workflow('some-workflow', [$step]);
$run = new WorkflowRun(1, 1, 'trigger-key', $subjects); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);
@ -119,7 +119,7 @@ class SendEmailActionTest extends \MailPoetTest {
$subjects = $this->getLoadedSubjects($subscriber, $segment); $subjects = $this->getLoadedSubjects($subscriber, $segment);
$email = (new Newsletter())->withAutomationType()->create(); $email = (new Newsletter())->withAutomationType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]);
$workflow = new Workflow('some-workflow', [$step]); $workflow = new Workflow('some-workflow', [$step]);
$run = new WorkflowRun(1, 1, 'trigger-key', $subjects); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);
@ -151,7 +151,7 @@ class SendEmailActionTest extends \MailPoetTest {
$subjects = $this->getLoadedSubjects($subscriber, $segment); $subjects = $this->getLoadedSubjects($subscriber, $segment);
$email = (new Newsletter())->withAutomationType()->create(); $email = (new Newsletter())->withAutomationType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]);
$workflow = new Workflow('some-workflow', [$step]); $workflow = new Workflow('some-workflow', [$step]);
$run = new WorkflowRun(1, 1, 'trigger-key', $subjects); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);
@ -180,7 +180,7 @@ class SendEmailActionTest extends \MailPoetTest {
$subjects = $this->getLoadedSubjects($subscriber, $segment); $subjects = $this->getLoadedSubjects($subscriber, $segment);
$email = (new Newsletter())->withAutomationType()->create(); $email = (new Newsletter())->withAutomationType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]);
$workflow = new Workflow('some-workflow', [$step]); $workflow = new Workflow('some-workflow', [$step]);
$run = new WorkflowRun(1, 1, 'trigger-key', $subjects); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);
@ -218,7 +218,7 @@ class SendEmailActionTest extends \MailPoetTest {
$subjects = $this->getLoadedSubjects($subscriber, $segment); $subjects = $this->getLoadedSubjects($subscriber, $segment);
$email = (new Newsletter())->withAutomationType()->create(); $email = (new Newsletter())->withAutomationType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]);
$workflow = new Workflow('some-workflow', [$step]); $workflow = new Workflow('some-workflow', [$step]);
$run = new WorkflowRun(1, 1, 'trigger-key', $subjects); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);
@ -247,7 +247,7 @@ class SendEmailActionTest extends \MailPoetTest {
$subjects = $this->getLoadedSubjects($subscriber, $segment); $subjects = $this->getLoadedSubjects($subscriber, $segment);
$email = (new Newsletter())->withAutomationType()->create(); $email = (new Newsletter())->withAutomationType()->create();
$step = new Step('step-id', null, Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]); $step = new Step('step-id', Step::TYPE_ACTION, 'step-key', null, ['email_id' => $email->getId()]);
$workflow = new Workflow('some-workflow', [$step]); $workflow = new Workflow('some-workflow', [$step]);
$run = new WorkflowRun(1, 1, 'trigger-key', $subjects); $run = new WorkflowRun(1, 1, 'trigger-key', $subjects);