Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
126 changes: 126 additions & 0 deletions packages/frontend/@n8n/chat/src/__tests__/MessageWithButtons.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
import { waitFor } from '@testing-library/vue';
import { mount } from '@vue/test-utils';
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';

import MessageWithButtons from '../components/MessageWithButtons.vue';

vi.mock('../components/MarkdownRenderer.vue', () => ({
default: {
name: 'MarkdownRenderer',
template: '<div>{{ text }}</div>',
props: ['text'],
},
}));

vi.mock('@n8n/chat/composables', () => ({
useOptions: () => ({
options: {
webhookUrl: 'https://webhook.example.com/webhook/123/chat',
},
}),
}));

const editorOrigin = 'http://localhost:5678';
const webhookOrigin = 'https://webhook.example.com';

const sameDomainButtons = [
{ text: 'Confirm', link: '/api/confirm', type: 'primary' as const },
{ text: 'Cancel', link: '/api/cancel', type: 'secondary' as const },
];

describe('MessageWithButtons', () => {
beforeEach(() => {
vi.stubGlobal('fetch', vi.fn());
Object.defineProperty(window, 'location', {
value: new URL(`${editorOrigin}/chat`),
writable: true,
});
});

afterEach(() => {
vi.restoreAllMocks();
});

it('renders and fetches when the link is on the same origin as the editor', async () => {
vi.mocked(fetch).mockResolvedValue({ ok: true } as Response);

const wrapper = mount(MessageWithButtons, {
props: { text: 'Please confirm', buttons: sameDomainButtons },
});

expect(wrapper.findAll('button')).toHaveLength(2);

await wrapper.findAll('button')[0].trigger('click');

await waitFor(() => expect(fetch).toHaveBeenCalledWith('/api/confirm'));
});

it('renders and fetches when the link is on the configured webhook origin', async () => {
vi.mocked(fetch).mockResolvedValue({ ok: true } as Response);

const webhookButtons = [
{
text: 'Approve',
link: `${webhookOrigin}/webhook/123/approve`,
type: 'primary' as const,
},
];

const wrapper = mount(MessageWithButtons, {
props: { text: 'Please approve', buttons: webhookButtons },
});

expect(wrapper.find('button').exists()).toBe(true);

await wrapper.find('button').trigger('click');

await waitFor(() => expect(fetch).toHaveBeenCalledWith(`${webhookOrigin}/webhook/123/approve`));
});

it('does not render a button when the link points to an unrecognized origin', () => {
const externalButtons = [
{ text: 'Go', link: 'https://broken-link.com/approve', type: 'primary' as const },
];

const wrapper = mount(MessageWithButtons, {
props: { text: 'Click me', buttons: externalButtons },
});

expect(wrapper.find('button').exists()).toBe(false);
expect(fetch).not.toHaveBeenCalled();
});

it('does not render a button for an absolute URL on a different host with the same port', () => {
const externalButtons = [
{ text: 'Go', link: 'http://other-host:5678/api/confirm', type: 'primary' as const },
];

const wrapper = mount(MessageWithButtons, {
props: { text: 'Click me', buttons: externalButtons },
});

expect(wrapper.find('button').exists()).toBe(false);
expect(fetch).not.toHaveBeenCalled();
});

it('renders only valid-origin buttons when the list contains mixed URLs', () => {
const mixedButtons = [
{ text: 'Editor', link: '/api/confirm', type: 'primary' as const },
{
text: 'Webhook',
link: `${webhookOrigin}/webhook/123/approve`,
type: 'secondary' as const,
},
{ text: 'Other', link: 'http://broken-url/approve', type: 'secondary' as const },
];

const wrapper = mount(MessageWithButtons, {
props: { text: 'Choose', buttons: mixedButtons },
});

const rendered = wrapper.findAll('button');
expect(rendered).toHaveLength(2);
expect(rendered[0].text()).toBe('Editor');
expect(rendered[1].text()).toBe('Webhook');
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<script setup lang="ts">
import { useOptions } from '@n8n/chat/composables';

Check failure on line 2 in packages/frontend/@n8n/chat/src/components/MessageWithButtons.vue

View workflow job for this annotation

GitHub Actions / Lint / Lint

There should be at least one empty line between import groups
import { ref } from 'vue';

Check failure on line 3 in packages/frontend/@n8n/chat/src/components/MessageWithButtons.vue

View workflow job for this annotation

GitHub Actions / Lint / Lint

`vue` import should occur before import of `@n8n/chat/composables`

import Button from './Button.vue';
Expand All @@ -13,8 +14,19 @@
}>;
}>();

const chatOptions = useOptions();
const clickedButtonIndex = ref<number | null>(null);

const isValidOrigin = (link: string): boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would maybe make it computed function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did check what the chatOptions are and the result is not reactive, so a computed value would also never be updated

try {
const validOrigins = [window.location.origin, new URL(chatOptions.options.webhookUrl).origin];
const url = new URL(link, window.location.href);
return validOrigins.includes(url.origin);
} catch {
return false;
}
};

const onClick = async (link: string, index: number) => {
if (clickedButtonIndex.value !== null) {
return;
Expand All @@ -33,7 +45,10 @@
<div :class="$style.buttons">
<template v-for="(button, index) in buttons" :key="button.text">
<Button
v-if="clickedButtonIndex === null || index === clickedButtonIndex"
v-if="
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be nice to have clickedButtonIndex === null || index === clickedButtonIndex as a variable with a clear name, because it's not clear what is it for

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is indeed not optimal. I did extract the check into the new function and renamed it to isButtonVisible. The clickedButtonIndex.value === null || index === clickedButtonIndex.value is still there since a proper description would be "if no buttons clicked or current button is clicked" and converting it to multiple conditions would not make it better, imo

isValidOrigin(button.link) &&
(clickedButtonIndex === null || index === clickedButtonIndex)
"
element="button"
:type="button.type"
:disabled="index === clickedButtonIndex"
Expand Down
Loading