Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(web): fixed the template editor react-hook-form dirty state #2674

Merged
merged 7 commits into from
Feb 7, 2023
Merged

fix(web): fixed the template editor react-hook-form dirty state #2674

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
e7bf601
fix(web): fixed the template editor react-hook-form dirty state
LetItRock Feb 3, 2023
da53036
Merge branch 'next' into nv-1587-template-editor-react-hook-form-fix
LetItRock Feb 3, 2023
db75d1e
chore(web): small adjustments
LetItRock Feb 3, 2023
1bf7e81
chore(web): the pnpm lock file
LetItRock Feb 3, 2023
38aa5c8
chore(web): fixed failing variables management component test
LetItRock Feb 3, 2023
1628b13
Merge branch 'next' into nv-1587-template-editor-react-hook-form-fix
LetItRock Feb 6, 2023
d8fcb0b
chore(web): added suggestions from the pr #2674
LetItRock Feb 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .cspell.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,8 @@
"codesee",
"ehlo",
"logrocket",
"backoff"
"backoff",
"isequal"
],
"flagWords": [],
"patterns": [
Expand Down
2 changes: 1 addition & 1 deletion apps/web/cypress/tests/changes.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function createNotification() {

cy.getByTestId('emailSubject').type('this is email subject');

cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

template editor: renamed the data-test-id name for the Create/Update button

cy.waitForNetworkIdle(500);
cy.getByTestId('trigger-snippet-btn').click();
}
Expand Down
14 changes: 8 additions & 6 deletions apps/web/cypress/tests/notification-editor/create-notification.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ describe('Creation functionality', function () {
force: true,
});
cy.getByTestId('inAppRedirect').type('/example/test');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.getByTestId('success-trigger-modal').should('be.visible');
cy.getByTestId('success-trigger-modal').getByTestId('trigger-code-snippet').contains('test-notification');
Expand All @@ -34,10 +34,10 @@ describe('Creation functionality', function () {
cy.getByTestId('success-trigger-modal')
.getByTestId('trigger-curl-snippet')
.contains("--header 'Authorization: ApiKey");

cy.getByTestId('success-trigger-modal').getByTestId('trigger-curl-snippet').contains('taskName');

cy.getByTestId('trigger-snippet-btn').click();

cy.location('pathname').should('equal', '/templates');
});

Expand All @@ -63,10 +63,12 @@ describe('Creation functionality', function () {
force: true,
});
cy.getByTestId('inAppRedirect').type('/example/test');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.getByTestId('trigger-snippet-btn').click();

cy.location('pathname').should('equal', '/templates');

// trigger the notification
cy.task('createNotifications', {
identifier: 'test-notification-title',
Expand Down Expand Up @@ -211,7 +213,7 @@ describe('Creation functionality', function () {

cy.getByTestId('emailSubject').type('this is email subject');

cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.getByTestId('success-trigger-modal').should('be.visible');
cy.getByTestId('success-trigger-modal').getByTestId('trigger-code-snippet').contains('test-notification');
Expand Down Expand Up @@ -275,7 +277,7 @@ describe('Creation functionality', function () {
cy.getByTestId('backoff-unit').click();
cy.get('.mantine-Select-dropdown .mantine-Select-item').contains('Minutes').click();

cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('success-trigger-modal').should('be.visible');
cy.getByTestId('trigger-snippet-btn').click();

Expand Down Expand Up @@ -311,7 +313,7 @@ describe('Creation functionality', function () {
cy.getByTestId('submit-category-btn').click();
cy.getByTestId('groupSelector').should('have.value', 'New Test Category');

cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.visit('/templates');
cy.getByTestId('template-edit-link');
Expand Down
51 changes: 40 additions & 11 deletions apps/web/cypress/tests/notification-editor/main-functionality.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,17 @@ describe('Workflow Editor - Main Functionality', function () {
cy.getByTestId('emailPreheader').should('have.value', 'this is email preheader');
});

it('should edit notification', function () {
it('should edit in-app notification', function () {
const template = this.session.templates[0];

cy.visit('/templates/edit/' + template._id);
cy.waitForNetworkIdle(500);

cy.getByTestId('notification-template-submit-btn').should('be.disabled');
cy.getByTestId('title').should('have.value', template.name);

addAndEditChannel('inApp');
cy.getByTestId('notification-template-submit-btn').should('not.be.disabled');

cy.get('.ace_text-layer').first().contains('Test content for <b>{{firstName}}</b>');

Expand All @@ -68,7 +70,8 @@ describe('Workflow Editor - Main Functionality', function () {
.type('new content for notification', {
force: true,
});
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('notification-template-submit-btn').should('be.disabled');

cy.visit('/templates');
cy.waitForNetworkIdle(500);
Expand All @@ -85,10 +88,34 @@ describe('Workflow Editor - Main Functionality', function () {

editChannel('inApp', true);

cy.getByTestId('notification-template-submit-btn').should('be.disabled');
cy.getByTestId('feed-button-1-checked');
cy.getByTestId('create-feed-input').type('test4');
cy.getByTestId('add-feed-button').click();
cy.getByTestId('feed-button-2-checked');
cy.getByTestId('notification-template-submit-btn').should('not.be.disabled');
});

it('should edit email notification', function () {
const template = this.session.templates[0];

cy.visit('/templates/edit/' + template._id);
cy.waitForNetworkIdle(500);

// edit email step
cy.getByTestId('notification-template-submit-btn').should('be.disabled');
clickWorkflow();
editChannel('email');
cy.getByTestId('notification-template-submit-btn').should('be.disabled');

// edit email editor content
cy.getByTestId('email-editor').getByTestId('editor-row').first().click().type('{selectall}{backspace}Hello world!');
LetItRock marked this conversation as resolved.
Show resolved Hide resolved
cy.getByTestId('notification-template-submit-btn').should('not.be.disabled');

// go back and update
goBack();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('notification-template-submit-btn').should('be.disabled');
});

it('should update notification active status', function () {
Expand Down Expand Up @@ -144,13 +171,13 @@ describe('Workflow Editor - Main Functionality', function () {
cy.waitForNetworkIdle(500);

cy.getByTestId('description').type('this is a notification template description');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('title').should('have.class', 'mantine-TextInput-invalid');
fillBasicNotificationDetails('Test SMS Notification Title');
clickWorkflow();
dragAndDrop('inApp');

cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('workflowButton').getByTestId('error-circle').should('be.visible');
cy.getByTestId('settingsButton').getByTestId('error-circle').should('be.visible');
});
Expand All @@ -162,7 +189,7 @@ describe('Workflow Editor - Main Functionality', function () {
fillBasicNotificationDetails();
clickWorkflow();
dragAndDrop('email');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('node-emailSelector').getByTestId('error-circle').should('be.visible');
editChannel('email');
cy.getByTestId('emailSubject').should('have.class', 'mantine-TextInput-invalid');
Expand Down Expand Up @@ -242,7 +269,7 @@ describe('Workflow Editor - Main Functionality', function () {
cy.getByTestId('smsNotificationContent').type('{{firstName}} someone assigned you to {{taskName}}', {
parseSpecialCharSequences: false,
});
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.getByTestId('success-trigger-modal').should('be.visible');
cy.getByTestId('success-trigger-modal').getByTestId('trigger-code-snippet').contains('test-sms-notification');
Expand Down Expand Up @@ -274,7 +301,7 @@ describe('Workflow Editor - Main Functionality', function () {
cy.get('#codeEditor').type('Hello world code {{name}} <div>Test', { parseSpecialCharSequences: false });

cy.intercept('GET', '/v1/notification-templates?page=0&limit=10').as('notification-templates');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('trigger-snippet-btn').click();

cy.wait('@notification-templates', { timeout: 60000 });
Expand All @@ -293,7 +320,7 @@ describe('Workflow Editor - Main Functionality', function () {
});

fillBasicNotificationDetails();
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.wait('@createTemplate').then((res) => {
cy.getByTestId('trigger-snippet-btn').click();
Expand Down Expand Up @@ -347,7 +374,8 @@ describe('Workflow Editor - Main Functionality', function () {
cy.getByTestId('control-add').first().click();
cy.getByTestId('template-container-click-area').eq(0).click();

cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('notification-template-submit-btn').should('be.disabled');

cy.visit('/templates');
cy.waitForNetworkIdle(500);
Expand All @@ -364,12 +392,13 @@ describe('Workflow Editor - Main Functionality', function () {

editChannel('inApp');

cy.getByTestId('notification-template-submit-btn').should('be.disabled');
cy.getByTestId('template-container').first().find('input').should('have.length', 1);

cy.getByTestId('remove-button-icon').click();

cy.getByTestId('submit-btn').click();
cy.waitForNetworkIdle(500);
cy.getByTestId('notification-template-submit-btn').click();
cy.getByTestId('notification-template-submit-btn').should('be.disabled');
LetItRock marked this conversation as resolved.
Show resolved Hide resolved

goBack();

Expand Down
8 changes: 4 additions & 4 deletions apps/web/cypress/tests/notification-editor/steps-actions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ describe('Workflow Editor - Steps Actions', function () {
cy.getByTestId(`node-inAppSelector`).should('not.exist');
cy.get('.react-flow__node').should('have.length', 3);
cy.get('.react-flow__node').first().should('contain', 'Trigger').next().should('contain', 'Email');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.visit('/templates/edit/' + template._id);
cy.waitForNetworkIdle(500);
Expand Down Expand Up @@ -59,7 +59,7 @@ describe('Workflow Editor - Steps Actions', function () {

editChannel('sms');
cy.getByTestId('smsNotificationContent').type('new content for sms');
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.visit('/templates/edit/' + template._id);
cy.waitForNetworkIdle(500);
Expand Down Expand Up @@ -90,7 +90,7 @@ describe('Workflow Editor - Steps Actions', function () {
cy.clickWorkflowNode(`node-inAppSelector`);
cy.getByTestId(`step-active-switch`).get('label').contains('Step is active');
cy.getByTestId(`step-active-switch`).click({ force: true });
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.clickWorkflowNode(`node-inAppSelector`);
cy.getByTestId(`step-active-switch`).get('label').contains('Step is not active');
Expand All @@ -108,7 +108,7 @@ describe('Workflow Editor - Steps Actions', function () {
cy.clickWorkflowNode(`node-inAppSelector`);
cy.getByTestId(`step-should-stop-on-fail-switch`).get('label').contains('Stop workflow if this step fails?');
cy.getByTestId(`step-should-stop-on-fail-switch`).click({ force: true });
cy.getByTestId('submit-btn').click();
cy.getByTestId('notification-template-submit-btn').click();

cy.clickWorkflowNode(`node-inAppSelector`);
cy.getByTestId(`step-should-stop-on-fail-switch`).should('be.checked');
Expand Down
2 changes: 1 addition & 1 deletion apps/web/cypress/tests/onboarding.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ describe('Getting Started Screen', function () {
cy.location('pathname').should('equal', '/templates/create');
cy.getByTestId('title').type('Test Notification Title');
cy.getByTestId('description').type('This is a test description for a test title');
cy.getByTestId('submit-btn').click({ force: true });
cy.getByTestId('notification-template-submit-btn').click({ force: true });
cy.get('.mantine-Notification-root').contains('Template saved successfully');

cy.getByTestId('side-nav-quickstart-link').click({ force: true });
Expand Down
2 changes: 2 additions & 0 deletions apps/web/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@emotion/react": "^11.7.1",
"@emotion/styled": "^11.6.0",
"@handlebars/parser": "^2.1.0",
"@hookform/devtools": "^4.3.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added this library to debug the react-hook-form

"@hookform/resolvers": "^2.9.1",
"@mantine/core": "^5.7.1",
"@mantine/dropzone": "^5.9.2",
Expand Down Expand Up @@ -77,6 +78,7 @@
"lodash.clonedeep": "^4.5.0",
"lodash.get": "^4.3.2",
"lodash.set": "^4.3.2",
"lodash.isequal": "^4.5.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added this library to compare the variables arrays when parsing template fields, more about that later

"logrocket": "^3.0.1",
"logrocket-react": "^5.0.1",
"polished": "^4.1.3",
Expand Down
15 changes: 0 additions & 15 deletions apps/web/src/components/templates/FieldArrayProvider.tsx
View file
Open in desktop

This file was deleted.

5 changes: 3 additions & 2 deletions apps/web/src/components/templates/SaveChangesModal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import { Group } from '@mantine/core';
import { useFormContext } from 'react-hook-form';

import { Button, Title, Text, Modal } from '../../design-system';
import { IForm } from './useTemplateController';
import type { IForm } from './formTypes';
import { errorMessage } from '../../utils/notifications';
import { useFormContext } from 'react-hook-form';

export function SaveChangesModal({
isVisible,
Expand Down
10 changes: 8 additions & 2 deletions apps/web/src/components/templates/TemplateEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import { useFormContext } from 'react-hook-form';
import { ChannelTypeEnum, StepTypeEnum } from '@novu/shared';

import { useActiveIntegrations } from '../../api/hooks';
import { EmailMessagesCards } from './email-editor/EmailMessagesCards';
import { TemplateInAppEditor } from './in-app-editor/TemplateInAppEditor';
import { TemplateSMSEditor } from './TemplateSMSEditor';
import { useTemplateController } from './useTemplateController';
import type { IForm } from './formTypes';
import { ActivePageEnum } from '../../pages/templates/editor/TemplateEditorPage';
import { TemplatePushEditor } from './TemplatePushEditor';
import { TemplateChatEditor } from './chat-editor/TemplateChatEditor';

export const TemplateEditor = ({ activePage, templateId, activeStep }) => {
const { integrations } = useActiveIntegrations();
const { control, errors, watch } = useTemplateController(templateId);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored this hook, there is no sense to hold the react-hook-form functionality inside, instead better to use useFormContext directly as we do in many other places.

const {
control,
formState: { errors },
watch,
} = useFormContext<IForm>();
const steps = watch('steps');

return (
Expand Down
Loading