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.

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

feat: Update NPS Value Survey #9638

Merged
merged 72 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
72 commits
Select commit Hold shift + click to select a range
c5923c3
fix: refactor value component
mutdmour May 23, 2024
931ab36
feat: add nps score to value survey
mutdmour May 23, 2024
6295b8d
feat: add value survey endpoints
mutdmour May 23, 2024
a6fdb1e
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour May 24, 2024
213fb7a
fix: add logic to decide to show value survey
mutdmour May 24, 2024
c6e195c
feat: add logic to update survey
mutdmour May 24, 2024
0c4a75e
feat: update logic to show
mutdmour May 24, 2024
1858e28
merge master
mutdmour Jun 3, 2024
08acc4e
reduce changes
mutdmour Jun 3, 2024
e55a241
handle edge case for ignoring users
mutdmour Jun 3, 2024
4a4b9fa
add migration to set for existing users
mutdmour Jun 4, 2024
2a8ce40
prevent if telemetry disabled
mutdmour Jun 4, 2024
b7acea8
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour Jun 5, 2024
7c0c2ff
refactor: add clear state to api, add store
mutdmour Jun 5, 2024
33f5c46
refactor to seperate store
mutdmour Jun 5, 2024
9b31e25
clean up
mutdmour Jun 5, 2024
995946e
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour Jun 5, 2024
7ef0d9d
add tests
mutdmour Jun 5, 2024
e07e054
refactor, add tests
mutdmour Jun 5, 2024
a560053
add tests for controller
mutdmour Jun 5, 2024
29b19f3
fix up package
mutdmour Jun 5, 2024
cc6b2fc
remove import
mutdmour Jun 5, 2024
007cee7
add initial e2e tests
mutdmour Jun 5, 2024
0436667
Add test for email part
mutdmour Jun 5, 2024
e67caad
lint fix
mutdmour Jun 6, 2024
0aec1b4
rename var
mutdmour Jun 6, 2024
135737e
refactor: rename to nps survey
mutdmour Jun 6, 2024
efc7c74
refactor: rename to nps survey
mutdmour Jun 6, 2024
a48601d
fix: update key name
mutdmour Jun 6, 2024
81a29fc
last bit of rename
mutdmour Jun 6, 2024
3841598
refactor to make more restful
mutdmour Jun 6, 2024
6e90a25
update to add vars on top
mutdmour Jun 7, 2024
9cfef22
refactor to reduce dep cycle
mutdmour Jun 7, 2024
61d2ba0
fix lint issue
mutdmour Jun 7, 2024
c333361
refactor continue
mutdmour Jun 7, 2024
87a14ef
override for postgres
mutdmour Jun 7, 2024
10bdd1c
fix migrations
mutdmour Jun 7, 2024
2d5963e
fix migrations
mutdmour Jun 7, 2024
aaf4aa3
merge master
mutdmour Jun 7, 2024
52b4ad0
add tests
mutdmour Jun 7, 2024
26db1df
add more tests
mutdmour Jun 7, 2024
0854acd
fix tests
mutdmour Jun 7, 2024
fcd5998
fix migrations
mutdmour Jun 8, 2024
1403e5e
update unit tests
mutdmour Jun 8, 2024
aa71cc8
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour Jun 8, 2024
cd1224a
fix mysql queries
mutdmour Jun 8, 2024
f846d65
update unit tests
mutdmour Jun 8, 2024
4a5ee8f
clean up
mutdmour Jun 8, 2024
14fa060
fix: bug with e2e
mutdmour Jun 8, 2024
dc7ff61
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour Jun 10, 2024
3200e7e
refactor: add comment
mutdmour Jun 10, 2024
7a3a95b
refactor: address feedback
mutdmour Jun 10, 2024
22ba1d1
fix: remove telemetry check
mutdmour Jun 10, 2024
8058b39
fix: add telemetry config
mutdmour Jun 10, 2024
8ab51ca
fix: e2e
mutdmour Jun 10, 2024
ff9218f
fix: remove console log
mutdmour Jun 10, 2024
c50b9ba
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour Jun 10, 2024
ebc1e86
fix types
mutdmour Jun 10, 2024
9ec6414
fix last type issues
mutdmour Jun 10, 2024
0be328e
fix: bug with retry logic
mutdmour Jun 10, 2024
b60e4cd
merge in masteR
mutdmour Jun 10, 2024
683848a
update to test domains
mutdmour Jun 10, 2024
962a3e8
clean up unused import
mutdmour Jun 10, 2024
e84151d
make tests less flaky
mutdmour Jun 10, 2024
04db697
remove only
mutdmour Jun 10, 2024
51f58b4
lintfix
netroy Jun 10, 2024
aa72281
Merge branch 'master' of github.com:n8n-io/n8n into ado-768
mutdmour Jun 11, 2024
d4d6829
skip failing test
mutdmour Jun 11, 2024
dd03c94
fix lint
mutdmour Jun 11, 2024
f517535
Update cypress/e2e/42-nps-survey.cy.ts
mutdmour Jun 11, 2024
d8a9b47
address feedback
mutdmour Jun 11, 2024
9fb993d
user assert instead of type check
mutdmour Jun 11, 2024
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
50 changes: 50 additions & 0 deletions cypress/e2e/42-value-survey.cy.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import { getValueSurvey, getValueSurveyEmail, getValueSurveyRatings } from '../pages/valueSurvey';
import { WorkflowPage } from '../pages/workflow';

const workflowPage = new WorkflowPage();

const NOW = Date.now();
const ONE_DAY = 24 * 60 * 60 * 1000;
const THREE_DAYS = ONE_DAY * 3;

describe('ValueSurvey', () => {
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
it('shows value survey to recently activated user and can submit email ', () => {
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
cy.intercept('/rest/settings', { middleware: true }, (req) => {
req.on('response', (res) => {
if (res.body.data) {
res.body.data.telemetry = { enabled: true };
}
});
});

cy.intercept('/rest/login', { middleware: true }, (req) => {
req.on('response', (res) => {
if (res.body.data) {
res.body.data.settings = res.body.data.settings || {};
res.body.data.settings.userActivated = true;
res.body.data.settings.userActivatedAt = NOW - THREE_DAYS - 1000;
}
});
});

workflowPage.actions.visit();

workflowPage.actions.saveWorkflowOnButtonClick();

getValueSurvey().should('be.visible');
getValueSurveyRatings().find('button').should('have.length', 11);

getValueSurveyRatings().find('button').first().click();

getValueSurveyEmail().find('input').type('[email protected]');
getValueSurveyEmail().find('button').click();

// test that modal does not show up again
cy.reload();

workflowPage.actions.visit();
workflowPage.actions.saveWorkflowOnButtonClick();

getValueSurvey().should('not.be.visible');
});
});
13 changes: 13 additions & 0 deletions cypress/pages/valueSurvey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* Getters
*/

export const getValueSurvey = () => cy.getByTestId('value-survey-modal');

export const getValueSurveyRatings = () => cy.getByTestId('value-survey-ratings');

export const getValueSurveyEmail = () => cy.getByTestId('value-survey-email');

/**
* Actions
*/
2 changes: 2 additions & 0 deletions packages/cli/src/Server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ import { InvitationController } from './controllers/invitation.controller';
import { OrchestrationService } from '@/services/orchestration.service';
import { ProjectController } from './controllers/project.controller';
import { RoleController } from './controllers/role.controller';
import { ValueSurveyController } from './controllers/valueSurvey.controller';

const exec = promisify(callbackExec);

Expand Down Expand Up @@ -150,6 +151,7 @@ export class Server extends AbstractServer {
ProjectController,
RoleController,
CurlController,
ValueSurveyController,
];

if (
Expand Down
33 changes: 33 additions & 0 deletions packages/cli/src/controllers/valueSurvey.controller.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { Post, RestController } from '@/decorators';
import { ValueSurveyRequest } from '@/requests';
import { UserService } from '@/services/user.service';

@RestController('/value-survey')
export class ValueSurveyController {
constructor(private readonly userService: UserService) {}

@Post('/show')
async valueSurveyShown(req: ValueSurveyRequest.ValueSurveyShown): Promise<void> {
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
await this.userService.updateSettings(req.user.id, {
valueSurveyLastShownAt: Date.now(),
valueSurveyLastResponseState: 'waiting',
valueSurveyIgnoredLastCount: 0,
});
}

@Post('/respond')
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
async valueSurveyResponded(req: ValueSurveyRequest.ValueSurveyResponded): Promise<void> {
await this.userService.updateSettings(req.user.id, {
valueSurveyLastResponseState: 'done',
valueSurveyIgnoredLastCount: 0,
});
}

@Post('/ignore')
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
async valueSurveyIgnored(req: ValueSurveyRequest.ValueSurveyIgnored): Promise<void> {
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
const lastCount = req.user.settings?.valueSurveyIgnoredLastCount ?? 0;
await this.userService.updateSettings(req.user.id, {
valueSurveyIgnoredLastCount: lastCount + 1,
});
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import type { MigrationContext, ReversibleMigration } from '@db/types';

export class AddActivatedAtUserSetting1717498465931 implements ReversibleMigration {
async up({ queryRunner, tablePrefix }: MigrationContext) {
const now = Date.now();
await queryRunner.query(
`UPDATE ${tablePrefix}user
SET settings = JSON_SET(settings, '$.userActivatedAt', ${now})
WHERE JSON_EXTRACT(settings, '$.userActivated') = true;`,
);
}

async down({ queryRunner, tablePrefix }: MigrationContext) {
await queryRunner.query(
`UPDATE ${tablePrefix}user SET settings = JSON_REMOVE(settings, '$.userActivatedAt')`,
);
}
}
2 changes: 2 additions & 0 deletions packages/cli/src/databases/migrations/mysqldb/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import { RemoveFailedExecutionStatus1711018413374 } from '../common/171101841337
import { MoveSshKeysToDatabase1711390882123 } from '../common/1711390882123-MoveSshKeysToDatabase';
import { RemoveNodesAccess1712044305787 } from '../common/1712044305787-RemoveNodesAccess';
import { MakeExecutionStatusNonNullable1714133768521 } from '../common/1714133768521-MakeExecutionStatusNonNullable';
import { AddActivatedAtUserSetting1717498465931 } from '../common/1717498465931-AddActivatedAtUserSetting';

export const mysqlMigrations: Migration[] = [
InitialMigration1588157391238,
Expand Down Expand Up @@ -117,4 +118,5 @@ export const mysqlMigrations: Migration[] = [
RemoveNodesAccess1712044305787,
CreateProject1714133768519,
MakeExecutionStatusNonNullable1714133768521,
AddActivatedAtUserSetting1717498465931,
];
2 changes: 2 additions & 0 deletions packages/cli/src/databases/migrations/postgresdb/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ import { RemoveFailedExecutionStatus1711018413374 } from '../common/171101841337
import { MoveSshKeysToDatabase1711390882123 } from '../common/1711390882123-MoveSshKeysToDatabase';
import { RemoveNodesAccess1712044305787 } from '../common/1712044305787-RemoveNodesAccess';
import { MakeExecutionStatusNonNullable1714133768521 } from '../common/1714133768521-MakeExecutionStatusNonNullable';
import { AddActivatedAtUserSetting1717498465931 } from '../common/1717498465931-AddActivatedAtUserSetting';

export const postgresMigrations: Migration[] = [
InitialMigration1587669153312,
Expand Down Expand Up @@ -115,4 +116,5 @@ export const postgresMigrations: Migration[] = [
RemoveNodesAccess1712044305787,
CreateProject1714133768519,
MakeExecutionStatusNonNullable1714133768521,
AddActivatedAtUserSetting1717498465931,
];
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import { AddActivatedAtUserSetting1717498465931 as BaseMigration } from '../common/1717498465931-AddActivatedAtUserSetting';

export class AddActivatedAtUserSetting1717498465931 extends BaseMigration {
transaction = false as const;
mutdmour marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 2 additions & 0 deletions packages/cli/src/databases/migrations/sqlite/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import { RemoveFailedExecutionStatus1711018413374 } from '../common/171101841337
import { MoveSshKeysToDatabase1711390882123 } from '../common/1711390882123-MoveSshKeysToDatabase';
import { RemoveNodesAccess1712044305787 } from '../common/1712044305787-RemoveNodesAccess';
import { MakeExecutionStatusNonNullable1714133768521 } from '../common/1714133768521-MakeExecutionStatusNonNullable';
import { AddActivatedAtUserSetting1717498465931 } from './1717498465931-AddActivatedAtUserSetting';

const sqliteMigrations: Migration[] = [
InitialMigration1588102412422,
Expand Down Expand Up @@ -111,6 +112,7 @@ const sqliteMigrations: Migration[] = [
RemoveNodesAccess1712044305787,
CreateProject1714133768519,
MakeExecutionStatusNonNullable1714133768521,
AddActivatedAtUserSetting1717498465931,
];

export { sqliteMigrations };
9 changes: 9 additions & 0 deletions packages/cli/src/requests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -601,3 +601,12 @@ export declare namespace ProjectRequest {
>;
type Delete = AuthenticatedRequest<{ projectId: string }, {}, {}, { transferId?: string }>;
}

// ----------------------------------
// /value-survey
// ----------------------------------
export declare namespace ValueSurveyRequest {
type ValueSurveyShown = AuthenticatedRequest;
type ValueSurveyResponded = AuthenticatedRequest;
type ValueSurveyIgnored = AuthenticatedRequest;
}
1 change: 1 addition & 0 deletions packages/cli/src/services/events.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export class EventsService extends EventEmitter {
await Container.get(UserService).updateSettings(owner.id, {
firstSuccessfulWorkflowId: workflowId,
userActivated: true,
userActivatedAt: runData.startedAt.getTime(),
});
}

Expand Down
71 changes: 71 additions & 0 deletions packages/cli/test/unit/controllers/valueSurvey.controller.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import { ValueSurveyController } from '@/controllers/valueSurvey.controller';
import type { ValueSurveyRequest } from '@/requests';
import type { UserService } from '@/services/user.service';
import { mock } from 'jest-mock-extended';

const NOW = 1717607016208;
jest.useFakeTimers({
now: NOW,
});

describe('ValueSurveyController', () => {
const userService = mock<UserService>();
const controller = new ValueSurveyController(userService);

describe('valueSurveyShown', () => {
it('updates user settings, reseting to waiting state', async () => {
const req = mock<ValueSurveyRequest.ValueSurveyShown>();
req.user.id = '1';

await controller.valueSurveyShown(req);

expect(userService.updateSettings).toHaveBeenCalledWith('1', {
valueSurveyIgnoredLastCount: 0,
valueSurveyLastResponseState: 'waiting',
valueSurveyLastShownAt: 1717607016208,
});
});
});

describe('valueSurveyResponded', () => {
it('updates user settings, setting response state to done', async () => {
const req = mock<ValueSurveyRequest.ValueSurveyResponded>();
req.user.id = '1';

await controller.valueSurveyResponded(req);

expect(userService.updateSettings).toHaveBeenCalledWith('1', {
valueSurveyIgnoredLastCount: 0,
valueSurveyLastResponseState: 'done',
});
});
});

describe('valueSurveyIgnored', () => {
it('updates user settings, incrementing ignore count', async () => {
const req = mock<ValueSurveyRequest.ValueSurveyIgnored>();
req.user.id = '1';
req.user.settings = {
valueSurveyIgnoredLastCount: 1,
};

await controller.valueSurveyIgnored(req);

expect(userService.updateSettings).toHaveBeenCalledWith('1', {
valueSurveyIgnoredLastCount: 2,
});
});

it('updates user settings, incrementing ignore count from 0, if not set', async () => {
const req = mock<ValueSurveyRequest.ValueSurveyIgnored>();
req.user.id = '1';
req.user.settings = {};

await controller.valueSurveyIgnored(req);

expect(userService.updateSettings).toHaveBeenCalledWith('1', {
valueSurveyIgnoredLastCount: 2,
});
});
});
});
45 changes: 37 additions & 8 deletions packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -770,14 +770,6 @@ export interface IN8nPrompts {
showValueSurvey: boolean;
}

export interface IN8nValueSurveyData {
[key: string]: string;
}

export interface IN8nPromptResponse {
updated: boolean;
}

export const enum UserManagementAuthenticationMethod {
Email = 'email',
Ldap = 'ldap',
Expand Down Expand Up @@ -1951,4 +1943,41 @@ export type EnterpriseEditionFeatureKey =
| 'WorkerView'
| 'AdvancedPermissions';

export type ModalKey =
| 'about'
| 'chatEmbed'
| 'changePassword'
| 'editCredential'
| 'selectCredential'
| 'deleteUser'
| 'inviteUser'
| 'duplicate'
| 'tagsManager'
| 'versions'
| 'settings'
| 'lmChat'
| 'workflowShare'
| 'personalization'
| 'contactPrompt'
| 'valueSurvey'
| 'activation'
| 'onboardingCallSignup'
| 'communityPackageInstall'
| 'communityPackageManageConfirm'
| 'importCurl'
| 'generateCurl'
| 'settingsLogStream'
| 'sourceControlPush'
| 'sourceControlPull'
| 'debugPaywall'
| 'mfaSetup'
| 'workflowHistoryVersionRestore'
| 'suggestedTemplatePreview'
| 'setupCredentials'
| 'externalSecretsProvider';

export type EnterpriseEditionFeatureValue = keyof Omit<IN8nUISettings['enterprise'], 'projects'>;

export interface IN8nPromptResponse {
updated: boolean;
}
18 changes: 1 addition & 17 deletions packages/editor-ui/src/api/settings.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,4 @@
import type {
IRestApiContext,
IN8nPrompts,
IN8nValueSurveyData,
IN8nPromptResponse,
} from '../Interface';
import type { IRestApiContext, IN8nPrompts, IN8nPromptResponse } from '../Interface';
import { makeRestApiRequest, get, post } from '@/utils/apiUtils';
import { N8N_IO_BASE_URL, NPM_COMMUNITY_NODE_SEARCH_API_URL } from '@/constants';
import type { IN8nUISettings } from 'n8n-workflow';
Expand Down Expand Up @@ -34,17 +29,6 @@ export async function submitContactInfo(
);
}

export async function submitValueSurvey(
instanceId: string,
userId: string,
params: IN8nValueSurveyData,
): Promise<IN8nPromptResponse> {
return await post(N8N_IO_BASE_URL, '/value-survey', params, {
'n8n-instance-id': instanceId,
'n8n-user-id': userId,
});
}

export async function getAvailableCommunityPackageCount(): Promise<number> {
const response = await get(
NPM_COMMUNITY_NODE_SEARCH_API_URL,
Expand Down
14 changes: 14 additions & 0 deletions packages/editor-ui/src/api/valueSurvey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import type { IRestApiContext } from '@/Interface';
import { makeRestApiRequest } from '@/utils/apiUtils';

export async function valueSurveyShown(context: IRestApiContext): Promise<void> {
await makeRestApiRequest(context, 'POST', '/value-survey/show');
}

export async function valueSurveyResponded(context: IRestApiContext): Promise<void> {
await makeRestApiRequest(context, 'POST', '/value-survey/respond');
}

export async function valueSurveyIgnored(context: IRestApiContext): Promise<void> {
await makeRestApiRequest(context, 'POST', '/value-survey/ignore');
}
Loading
Loading