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(editor): Make PDF and Audio binary-data viewable in the UI #7367

Merged
merged 6 commits into from
Oct 9, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 6 additions & 1 deletion packages/editor-ui/src/components/BinaryDataDisplayEmbed.vue
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
<source :src="embedSource" :type="binaryData.mimeType" />
{{ $locale.baseText('binaryDataDisplay.yourBrowserDoesNotSupport') }}
</video>
<audio v-if="binaryData.fileType === 'audio'" controls autoplay>
<source :src="embedSource" :type="binaryData.mimeType" />
{{ $locale.baseText('binaryDataDisplay.yourBrowserDoesNotSupport') }}
</audio>
<vue-json-pretty
v-else-if="binaryData.fileType === 'json'"
:data="jsonData"
Expand Down Expand Up @@ -92,7 +96,8 @@ export default defineComponent({
max-width: calc(100% - 1em);
}

&.other {
&.other,
&.pdf {
height: calc(100% - 1em);
width: calc(100% - 1em);
}
Expand Down
8 changes: 4 additions & 4 deletions packages/editor-ui/src/components/RunData.vue
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@
v-for="(binaryData, key) in binaryDataEntry"
:key="index + '_' + key"
>
<div>
<div :data-test-id="'ndv-binary-data_' + index">
<div :class="$style.binaryHeader">
{{ key }}
</div>
Expand Down Expand Up @@ -432,15 +432,15 @@
v-if="isViewable(index, key)"
size="small"
:label="$locale.baseText('runData.showBinaryData')"
class="binary-data-show-data-button"
data-test-id="ndv-view-binary-data"
@click="displayBinaryData(index, key)"
/>
<n8n-button
v-if="isDownloadable(index, key)"
size="small"
type="secondary"
:label="$locale.baseText('runData.downloadBinaryData')"
class="binary-data-show-data-button"
data-test-id="ndv-download-binary-data"
@click="downloadBinaryData(index, key)"
/>
</div>
Expand Down Expand Up @@ -1320,7 +1320,7 @@ export default defineComponent({
},
isViewable(index: number, key: string): boolean {
const { fileType } = this.binaryData[index][key];
return !!fileType && ['image', 'video', 'text', 'json'].includes(fileType);
return !!fileType && ['image', 'audio', 'video', 'text', 'json', 'pdf'].includes(fileType);
},
isDownloadable(index: number, key: string): boolean {
const { mimeType, fileName } = this.binaryData[index][key];
Expand Down
145 changes: 95 additions & 50 deletions packages/editor-ui/src/components/__tests__/RunData.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,102 @@ import RunData from '@/components/RunData.vue';
import { STORES, VIEWS } from '@/constants';
import { SETTINGS_STORE_DEFAULT_STATE } from '@/__tests__/utils';
import { createComponentRenderer } from '@/__tests__/render';

const renderComponent = createComponentRenderer(RunData, {
props: {
nodeUi: {
name: 'Test Node',
},
},
data() {
return {
canPinData: true,
};
},
global: {
mocks: {
$route: {
name: VIEWS.WORKFLOW,
},
},
},
});
import type { IRunDataDisplayMode } from '@/Interface';

describe('RunData', () => {
it('should render data correctly even when "item.json" has another "json" key', async () => {
const { html, getByText, getAllByTestId, getByTestId } = renderComponent({
const { getByText, getAllByTestId, getByTestId } = render(
[
{
json: {
id: 1,
name: 'Test 1',
json: {
data: 'Json data 1',
},
},
},
{
json: {
id: 2,
name: 'Test 2',
json: {
data: 'Json data 2',
},
},
},
],
'schema',
);

await userEvent.click(getByTestId('ndv-pin-data'));
await waitFor(() => getAllByTestId('run-data-schema-item'), { timeout: 1000 });
expect(getByText('Test 1')).toBeInTheDocument();
expect(getByText('Json data 1')).toBeInTheDocument();
});

it('should render view and download buttons for PDFs', async () => {
const { getByTestId } = render(
[
{
json: {},
binary: {
data: {
fileName: 'test.pdf',
fileType: 'pdf',
mimeType: 'application/pdf',
},
},
},
],
'binary',
);
expect(getByTestId('ndv-view-binary-data')).toBeInTheDocument();
expect(getByTestId('ndv-download-binary-data')).toBeInTheDocument();
expect(getByTestId('ndv-binary-data_0')).toBeInTheDocument();
});

it('should not render a view button for unknown content-type', async () => {
const { getByTestId, queryByTestId } = render(
[
{
json: {},
binary: {
data: {
fileName: 'test.xyz',
mimeType: 'application/octet-stream',
},
},
},
],
'binary',
);
expect(queryByTestId('ndv-view-binary-data')).not.toBeInTheDocument();
expect(getByTestId('ndv-download-binary-data')).toBeInTheDocument();
expect(getByTestId('ndv-binary-data_0')).toBeInTheDocument();
});

const render = (outputData: unknown[], displayMode: IRunDataDisplayMode) =>
createComponentRenderer(RunData, {
props: {
nodeUi: {
name: 'Test Node',
},
},
data() {
return {
canPinData: true,
showData: true,
};
},
global: {
mocks: {
$route: {
name: VIEWS.WORKFLOW,
},
},
},
})({
props: {
nodeUi: {
id: '1',
Expand All @@ -49,7 +121,7 @@ describe('RunData', () => {
},
[STORES.NDV]: {
output: {
displayMode: 'schema',
displayMode,
},
activeNodeName: 'Test Node',
},
Expand Down Expand Up @@ -89,28 +161,7 @@ describe('RunData', () => {
startTime: new Date().getTime(),
executionTime: new Date().getTime(),
data: {
main: [
[
{
json: {
id: 1,
name: 'Test 1',
json: {
data: 'Json data 1',
},
},
},
{
json: {
id: 2,
name: 'Test 2',
json: {
data: 'Json data 2',
},
},
},
],
],
main: [outputData],
},
source: [null],
},
Expand All @@ -123,10 +174,4 @@ describe('RunData', () => {
},
}),
});

await userEvent.click(getByTestId('ndv-pin-data'));
await waitFor(() => getAllByTestId('run-data-schema-item'), { timeout: 1000 });
expect(getByText('Test 1')).toBeInTheDocument();
expect(getByText('Json data 1')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
"fileExtension": "pdf",
"fileName": "sample-encrypted.pdf",
"fileSize": "18.9 kB",
"fileType": "pdf",
"mimeType": "application/pdf"
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"fileExtension": "pdf",
"fileName": "sample.pdf",
"fileSize": "17.8 kB",
"fileType": "pdf",
"mimeType": "application/pdf"
}
},
Expand Down
2 changes: 1 addition & 1 deletion packages/workflow/src/Interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type IAllExecuteFunctions =
| ITriggerFunctions
| IWebhookFunctions;

export type BinaryFileType = 'text' | 'json' | 'image' | 'video';
export type BinaryFileType = 'text' | 'json' | 'image' | 'audio' | 'video' | 'pdf';
Copy link
Member Author

Choose a reason for hiding this comment

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

should we use document instead of pdf here? are there any other document types that browsers can easily render via the <embed> tag, that are also relevant to us?

Copy link
Member Author

Choose a reason for hiding this comment

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

text/html could be rendered in <embed>, but that might be a security risk, as the html file could be used for arbitrary code execution.

Copy link
Contributor

Choose a reason for hiding this comment

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

I can see more documents being needed.. but as long as users can download them, it's fine as is. or until a user requests it

export interface IBinaryData {
[key: string]: string | undefined;
data: string;
Expand Down
4 changes: 3 additions & 1 deletion packages/workflow/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,10 @@ export const sleep = async (ms: number): Promise<void> =>
export function fileTypeFromMimeType(mimeType: string): BinaryFileType | undefined {
if (mimeType.startsWith('application/json')) return 'json';
if (mimeType.startsWith('image/')) return 'image';
if (mimeType.startsWith('audio/')) return 'audio';
if (mimeType.startsWith('video/')) return 'video';
if (mimeType.startsWith('text/')) return 'text';
if (mimeType.startsWith('text/') || mimeType.startsWith('application/javascript')) return 'text';
if (mimeType.startsWith('application/pdf')) return 'pdf';
return;
}

Expand Down
40 changes: 39 additions & 1 deletion packages/workflow/test/utils.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { jsonParse, jsonStringify, deepCopy, isObjectEmpty } from '@/utils';
import { jsonParse, jsonStringify, deepCopy, isObjectEmpty, fileTypeFromMimeType } from '@/utils';

describe('isObjectEmpty', () => {
it('should handle null and undefined', () => {
Expand Down Expand Up @@ -190,3 +190,41 @@ describe('deepCopy', () => {
expect(copy.deep.arr.slice(-1)[0]).not.toBe(object);
});
});

describe('fileTypeFromMimeType', () => {
it('should recognize json', () => {
netroy marked this conversation as resolved.
Show resolved Hide resolved
expect(fileTypeFromMimeType('application/json')).toEqual('json');
});

it('should recognize image', () => {
expect(fileTypeFromMimeType('image/jpeg')).toEqual('image');
expect(fileTypeFromMimeType('image/png')).toEqual('image');
expect(fileTypeFromMimeType('image/avif')).toEqual('image');
expect(fileTypeFromMimeType('image/webp')).toEqual('image');
});

it('should recognize audio', () => {
expect(fileTypeFromMimeType('audio/wav')).toEqual('audio');
expect(fileTypeFromMimeType('audio/webm')).toEqual('audio');
expect(fileTypeFromMimeType('audio/ogg')).toEqual('audio');
expect(fileTypeFromMimeType('audio/mp3')).toEqual('audio');
});

it('should recognize video', () => {
expect(fileTypeFromMimeType('video/mp4')).toEqual('video');
expect(fileTypeFromMimeType('video/webm')).toEqual('video');
expect(fileTypeFromMimeType('video/ogg')).toEqual('video');
});

it('should recognize text', () => {
expect(fileTypeFromMimeType('text/plain')).toEqual('text');
expect(fileTypeFromMimeType('text/css')).toEqual('text');
expect(fileTypeFromMimeType('text/html')).toEqual('text');
expect(fileTypeFromMimeType('text/javascript')).toEqual('text');
expect(fileTypeFromMimeType('application/javascript')).toEqual('text');
});

it('should recognize pdf', () => {
expect(fileTypeFromMimeType('application/pdf')).toEqual('pdf');
});
});
Loading