Skip to content

Commit

Permalink
feat(editor): Rework banners framework and add email confirmation ban…
Browse files Browse the repository at this point in the history
…ner (#7205)

This PR introduces banner framework overhaul:
First version of the banner framework was built to allow multiple
banners to be shown at the same time. Since that proven to be the case
we don't need and it turned out to be pretty messy keeping only one
banner visible in such setup, this PR reworks it so it renders only one
banner at a time, based on [this priority
list](https://www.notion.so/n8n/Banner-stack-60948c4167c743718fde80d6745258d5?pvs=4#6afd052ec8d146a1b0fab8884a19add7)
that is assembled together with our product & design team.

### How to test banner stack:
1. Available banners and their priorities are registered
[here](https://github.com/n8n-io/n8n/blob/f9f122d46d26565a4cc5dcf63060e7ed9f359e53/packages/editor-ui/src/components/banners/BannerStack.vue#L14)
2. Banners are pushed to stack using `pushBannerToStack` action, for
example:
```
useUIStore().pushBannerToStack('TRIAL');
```
4. Try pushing different banners to stack and check if only the one with
highest priorities is showing up

### How to test the _Email confirmation_ banner:
1. Comment out [this
line](https://github.com/n8n-io/n8n/blob/b80d2e3bec59a9abe141a4c808ea2b7f5d9fecce/packages/editor-ui/src/stores/cloudPlan.store.ts#L59),
so cloud data is always fetched
2. Create an
[override](https://chrome.google.com/webstore/detail/resource-override/pkoacgokdfckfpndoffpifphamojphii)
(URL -> File) that will serve user data that triggers this banner:
- **URL**: `*/rest/cloud/proxy/admin/user/me`
- **File**:
```
{
    "confirmed": false,
    "id": 1,
    "email": "test@test.com",
    "username": "test"
}
```
3. Run n8n
  • Loading branch information
MiloradFilipovic authored Sep 21, 2023
1 parent 2491ccf commit b0e98b5
Show file tree
Hide file tree
Showing 18 changed files with 424 additions and 118 deletions.
4 changes: 0 additions & 4 deletions packages/editor-ui/src/App.vue
Original file line number Diff line number Diff line change
Expand Up @@ -143,9 +143,6 @@ export default defineComponent({
console.log(HIRING_BANNER);
}
},
async initBanners() {
return this.uiStore.initBanners();
},
async checkForCloudPlanData() {
return this.cloudPlanStore.checkForCloudPlanData();
},
Expand Down Expand Up @@ -239,7 +236,6 @@ export default defineComponent({
await this.redirectIfNecessary();
void this.checkForNewVersions();
await this.checkForCloudPlanData();
void this.initBanners();
void this.postAuthenticate();
this.loading = false;
Expand Down
18 changes: 17 additions & 1 deletion packages/editor-ui/src/Interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ import type {
} from 'n8n-workflow';
import type { BulkCommand, Undoable } from '@/models/history';
import type { PartialBy, TupleToUnion } from '@/utils/typeHelpers';
import type { Component } from 'vue';

export * from 'n8n-design-system/types';

Expand Down Expand Up @@ -1084,7 +1085,7 @@ export interface UIState {
addFirstStepOnLoad: boolean;
executionSidebarAutoRefresh: boolean;
bannersHeight: number;
banners: { [key in BannerName]: { dismissed: boolean; type?: 'temporary' | 'permanent' } };
bannerStack: BannerName[];
}

export type IFakeDoor = {
Expand Down Expand Up @@ -1192,6 +1193,7 @@ export interface IVersionsState {
export interface IUsersState {
currentUserId: null | string;
users: { [userId: string]: IUser };
currentUserCloudInfo: Cloud.UserAccount | null;
}

export interface IWorkflowsState {
Expand Down Expand Up @@ -1529,6 +1531,13 @@ export declare namespace Cloud {
length: number;
gracePeriod: number;
}

export type UserAccount = {
confirmed: boolean;
username: string;
email: string;
hasEarlyAccess?: boolean;
};
}

export interface CloudPlanState {
Expand Down Expand Up @@ -1594,3 +1603,10 @@ export type UTMCampaign =
| 'open'
| 'upgrade-users'
| 'upgrade-variables';

export type N8nBanners = {
[key in BannerName]: {
priority: number;
component: Component;
};
};
10 changes: 9 additions & 1 deletion packages/editor-ui/src/api/cloudPlans.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { Cloud, IRestApiContext, InstanceUsage } from '@/Interface';
import { get } from '@/utils';
import { get, post } from '@/utils';

export async function getCurrentPlan(context: IRestApiContext): Promise<Cloud.PlanData> {
return get(context.baseUrl, '/admin/cloud-plan');
Expand All @@ -8,3 +8,11 @@ export async function getCurrentPlan(context: IRestApiContext): Promise<Cloud.Pl
export async function getCurrentUsage(context: IRestApiContext): Promise<InstanceUsage> {
return get(context.baseUrl, '/cloud/limits');
}

export async function getCloudUserInfo(context: IRestApiContext): Promise<Cloud.UserAccount> {
return get(context.baseUrl, '/cloud/proxy/admin/user/me');
}

export async function confirmEmail(context: IRestApiContext): Promise<Cloud.UserAccount> {
return post(context.baseUrl, '/cloud/proxy/admin/user/resend-confirmation-email');
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ import {
import type { IPermissions } from '@/permissions';
import { getWorkflowPermissions } from '@/permissions';
import { createEventBus } from 'n8n-design-system/utils';
import { useCloudPlanStore } from '@/stores';
import { nodeViewEventBus } from '@/event-bus';
import { genericHelpers } from '@/mixins/genericHelpers';
Expand Down Expand Up @@ -223,7 +222,6 @@ export default defineComponent({
useUsageStore,
useWorkflowsStore,
useUsersStore,
useCloudPlanStore,
useSourceControlStore,
),
currentUser(): IUser | null {
Expand Down
113 changes: 66 additions & 47 deletions packages/editor-ui/src/components/__tests__/BannersStack.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { within } from '@testing-library/vue';
import { merge } from 'lodash-es';
import userEvent from '@testing-library/user-event';

Expand All @@ -11,6 +10,7 @@ import { useUIStore } from '@/stores/ui.store';
import { useUsersStore } from '@/stores/users.store';
import type { RenderOptions } from '@/__tests__/render';
import { createComponentRenderer } from '@/__tests__/render';
import { waitFor } from '@testing-library/vue';

let uiStore: ReturnType<typeof useUIStore>;
let usersStore: ReturnType<typeof useUsersStore>;
Expand All @@ -20,11 +20,7 @@ const initialState = {
settings: merge({}, SETTINGS_STORE_DEFAULT_STATE.settings),
},
[STORES.UI]: {
banners: {
V1: { dismissed: false },
TRIAL: { dismissed: false },
TRIAL_OVER: { dismissed: false },
},
bannerStack: ['TRIAL_OVER', 'V1', 'NON_PRODUCTION_LICENSE', 'EMAIL_CONFIRMATION'],
},
[STORES.USERS]: {
currentUserId: 'aaa-bbb',
Expand Down Expand Up @@ -66,62 +62,30 @@ describe('BannerStack', () => {
vi.clearAllMocks();
});

it('should render default configuration', async () => {
const { getByTestId } = renderComponent();

const bannerStack = getByTestId('banner-stack');
expect(bannerStack).toBeInTheDocument();

expect(within(bannerStack).getByTestId('banners-TRIAL')).toBeInTheDocument();
expect(within(bannerStack).getByTestId('banners-TRIAL_OVER')).toBeInTheDocument();
expect(within(bannerStack).getByTestId('banners-V1')).toBeInTheDocument();
});

it('should not render dismissed banners', async () => {
const { getByTestId } = renderComponent({
pinia: createTestingPinia({
initialState: merge(initialState, {
[STORES.UI]: {
banners: {
V1: { dismissed: true },
TRIAL: { dismissed: true },
},
},
}),
}),
});
it('should render banner with the highest priority', async () => {
const { getByTestId, queryByTestId } = renderComponent();

const bannerStack = getByTestId('banner-stack');
expect(bannerStack).toBeInTheDocument();

expect(within(bannerStack).queryByTestId('banners-V1')).not.toBeInTheDocument();
expect(within(bannerStack).queryByTestId('banners-TRIAL')).not.toBeInTheDocument();
expect(within(bannerStack).getByTestId('banners-TRIAL_OVER')).toBeInTheDocument();
// Only V1 banner should be visible
expect(getByTestId('banners-V1')).toBeInTheDocument();
expect(queryByTestId('banners-TRIAL_OVER')).not.toBeInTheDocument();
});

it('should dismiss banner on click', async () => {
const { getByTestId } = renderComponent();
const dismissBannerSpy = vi
.spyOn(useUIStore(), 'dismissBanner')
.mockImplementation(async (banner, mode) => {});
const closeTrialBannerButton = getByTestId('banner-TRIAL_OVER-close');
expect(getByTestId('banners-V1')).toBeInTheDocument();
const closeTrialBannerButton = getByTestId('banner-V1-close');
expect(closeTrialBannerButton).toBeInTheDocument();
await userEvent.click(closeTrialBannerButton);
expect(dismissBannerSpy).toHaveBeenCalledWith('TRIAL_OVER');
expect(dismissBannerSpy).toHaveBeenCalledWith('V1');
});

it('should permanently dismiss banner on click', async () => {
const { getByTestId } = renderComponent({
pinia: createTestingPinia({
initialState: merge(initialState, {
[STORES.UI]: {
banners: {
V1: { dismissed: false },
},
},
}),
}),
});
const { getByTestId } = renderComponent();
const dismissBannerSpy = vi
.spyOn(useUIStore(), 'dismissBanner')
.mockImplementation(async (banner, mode) => {});
Expand All @@ -144,4 +108,59 @@ describe('BannerStack', () => {
});
expect(queryByTestId('banner-confirm-v1')).not.toBeInTheDocument();
});

it('should send email confirmation request from the banner', async () => {
const { getByTestId, getByText } = renderComponent({
pinia: createTestingPinia({
initialState: {
...initialState,
[STORES.UI]: {
bannerStack: ['EMAIL_CONFIRMATION'],
},
},
}),
});
const confirmEmailSpy = vi.spyOn(useUsersStore(), 'confirmEmail');
getByTestId('confirm-email-button').click();
await waitFor(() => expect(confirmEmailSpy).toHaveBeenCalled());
await waitFor(() => {
expect(getByText('Confirmation email sent')).toBeInTheDocument();
});
});

it('should show error message if email confirmation fails', async () => {
const ERROR_MESSAGE = 'Something went wrong';
const { getByTestId, getByText } = renderComponent({
pinia: createTestingPinia({
initialState: {
...initialState,
[STORES.UI]: {
bannerStack: ['EMAIL_CONFIRMATION'],
},
},
}),
});
const confirmEmailSpy = vi.spyOn(useUsersStore(), 'confirmEmail').mockImplementation(() => {
throw new Error(ERROR_MESSAGE);
});
getByTestId('confirm-email-button').click();
await waitFor(() => expect(confirmEmailSpy).toHaveBeenCalled());
await waitFor(() => {
expect(getByText(ERROR_MESSAGE)).toBeInTheDocument();
});
});

it('should render empty banner stack when there are no banners to display', async () => {
const { queryByTestId } = renderComponent({
pinia: createTestingPinia({
initialState: {
...initialState,
[STORES.UI]: {
bannerStack: [],
},
},
}),
});
expect(queryByTestId('banner-stack')).toBeEmptyDOMElement();
});
});
49 changes: 35 additions & 14 deletions packages/editor-ui/src/components/banners/BannerStack.vue
Original file line number Diff line number Diff line change
@@ -1,38 +1,59 @@
<script setup lang="ts">
<script lang="ts">
import NonProductionLicenseBanner from '@/components/banners/NonProductionLicenseBanner.vue';
import TrialOverBanner from '@/components/banners/TrialOverBanner.vue';
import TrialBanner from '@/components/banners/TrialBanner.vue';
import V1Banner from '@/components/banners/V1Banner.vue';
import EmailConfirmationBanner from '@/components/banners/EmailConfirmationBanner.vue';
import type { Component } from 'vue';
import type { N8nBanners } from '@/Interface';
// All banners that can be shown in the app should be registered here.
// This component renders the banner with the highest priority from the banner stack, located in the UI store.
// When registering a new banner, please consult this document to determine it's priority:
// https://www.notion.so/n8n/Banner-stack-60948c4167c743718fde80d6745258d5
export const N8N_BANNERS: N8nBanners = {
V1: { priority: 350, component: V1Banner as Component },
TRIAL_OVER: { priority: 260, component: TrialOverBanner as Component },
EMAIL_CONFIRMATION: { priority: 250, component: EmailConfirmationBanner as Component },
TRIAL: { priority: 150, component: TrialBanner as Component },
NON_PRODUCTION_LICENSE: { priority: 140, component: NonProductionLicenseBanner as Component },
};
</script>

<script setup lang="ts">
import { useUIStore } from '@/stores/ui.store';
import { onMounted, watch } from 'vue';
import { computed, onMounted } from 'vue';
import { getBannerRowHeight } from '@/utils';
import type { BannerName } from 'n8n-workflow';
const uiStore = useUIStore();
function shouldShowBanner(bannerName: BannerName) {
return uiStore.banners[bannerName].dismissed === false;
}
async function updateCurrentBannerHeight() {
const bannerHeight = await getBannerRowHeight();
uiStore.updateBannersHeight(bannerHeight);
}
onMounted(async () => {
await updateCurrentBannerHeight();
const currentlyShownBanner = computed(() => {
void updateCurrentBannerHeight();
if (uiStore.bannerStack.length === 0) return null;
// Find the banner with the highest priority
let banner = N8N_BANNERS[uiStore.bannerStack[0]];
uiStore.bannerStack.forEach((bannerName, index) => {
if (index === 0) return;
const bannerToCompare = N8N_BANNERS[bannerName];
if (bannerToCompare.priority > banner.priority) {
banner = bannerToCompare;
}
});
return banner.component;
});
watch(uiStore.banners, async () => {
onMounted(async () => {
await updateCurrentBannerHeight();
});
</script>

<template>
<div data-test-id="banner-stack">
<trial-over-banner v-if="shouldShowBanner('TRIAL_OVER')" />
<trial-banner v-if="shouldShowBanner('TRIAL')" />
<v1-banner v-if="shouldShowBanner('V1')" />
<non-production-license-banner v-if="shouldShowBanner('NON_PRODUCTION_LICENSE')" />
<component :is="currentlyShownBanner" />
</div>
</template>
12 changes: 11 additions & 1 deletion packages/editor-ui/src/components/banners/BaseBanner.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<script lang="ts" setup>
import { useUIStore } from '@/stores/ui.store';
import { computed, useSlots } from 'vue';
import type { BannerName } from 'n8n-workflow';
interface Props {
Expand All @@ -10,6 +11,7 @@ interface Props {
}
const uiStore = useUIStore();
const slots = useSlots();
const props = withDefaults(defineProps<Props>(), {
theme: 'info',
Expand All @@ -18,6 +20,10 @@ const props = withDefaults(defineProps<Props>(), {
const emit = defineEmits(['close']);
const hasTrailingContent = computed(() => {
return !!slots.trailingContent;
});
async function onCloseClick() {
await uiStore.dismissBanner(props.name);
emit('close');
Expand All @@ -31,7 +37,7 @@ async function onCloseClick() {
:roundCorners="false"
:data-test-id="`banners-${props.name}`"
>
<div :class="$style.mainContent">
<div :class="[$style.mainContent, !hasTrailingContent ? $style.keepSpace : '']">
<slot name="mainContent" />
</div>
<template #trailingContent>
Expand All @@ -56,6 +62,10 @@ async function onCloseClick() {
display: flex;
gap: var(--spacing-4xs);
}
.keepSpace {
padding: 5px 0;
}
.trailingContent {
display: flex;
align-items: center;
Expand Down
Loading

0 comments on commit b0e98b5

Please sign in to comment.