fix(editor): Make expression edit modal read-only in executions view (#…

…10806)
n8n-io · Sep 23, 2024 · 7ff9ac2 · 7ff9ac2
1 parent 3286796
commit 7ff9ac2
Show file tree

Hide file tree

Showing 8 changed files with 157 additions and 54 deletions.
diff --git a/packages/editor-ui/src/components/ExpressionEditModal.test.ts b/packages/editor-ui/src/components/ExpressionEditModal.test.ts
@@ -0,0 +1,77 @@
+import { createComponentRenderer } from '@/__tests__/render';
+import { cleanupAppModals, createAppModals } from '@/__tests__/utils';
+import ExpressionEditModal from '@/components/ExpressionEditModal.vue';
+import { createTestingPinia } from '@pinia/testing';
+import { waitFor, within } from '@testing-library/vue';
+
+vi.mock('vue-router', () => {
+	const push = vi.fn();
+	return {
+		useRouter: () => ({
+			push,
+		}),
+		useRoute: () => ({}),
+		RouterLink: vi.fn(),
+	};
+});
+
+const renderModal = createComponentRenderer(ExpressionEditModal);
+
+describe('ExpressionEditModal', () => {
+	beforeEach(() => {
+		createAppModals();
+	});
+
+	afterEach(() => {
+		cleanupAppModals();
+		vi.clearAllMocks();
+	});
+
+	it('renders correctly', async () => {
+		const pinia = createTestingPinia();
+
+		const { getByTestId } = renderModal({
+			pinia,
+			props: {
+				parameter: { name: 'foo', type: 'string' },
+				path: '',
+				modelValue: 'test',
+				dialogVisible: true,
+			},
+		});
+
+		await waitFor(() => {
+			expect(getByTestId('expression-modal-input')).toBeInTheDocument();
+			expect(getByTestId('expression-modal-output')).toBeInTheDocument();
+
+			const editor = within(getByTestId('expression-modal-input')).getByRole('textbox');
+			expect(editor).toBeInTheDocument();
+			expect(editor).toHaveAttribute('contenteditable', 'true');
+			expect(editor).not.toHaveAttribute('aria-readonly');
+		});
+	});
+
+	it('is read only', async () => {
+		const pinia = createTestingPinia();
+
+		const { getByTestId } = renderModal({
+			pinia,
+			props: {
+				parameter: { name: 'foo', type: 'string' },
+				path: '',
+				modelValue: 'test',
+				dialogVisible: true,
+				isReadOnly: true,
+			},
+		});
+
+		await waitFor(() => {
+			expect(getByTestId('expression-modal-input')).toBeInTheDocument();
+			expect(getByTestId('expression-modal-output')).toBeInTheDocument();
+
+			const editor = within(getByTestId('expression-modal-input')).getByRole('textbox');
+			expect(editor).toBeInTheDocument();
+			expect(editor).toHaveAttribute('aria-readonly', 'true');
+		});
+	});
+});
diff --git a/packages/editor-ui/src/components/ExpressionEditModal.vue b/packages/editor-ui/src/components/ExpressionEditModal.vue
@@ -151,9 +151,9 @@ async function onDrop(expression: string, event: MouseEvent) {
 					:class="$style.schema"
 					:search="appliedSearch"
 					:nodes="parentNodes"
-					mapping-enabled
-					pane-type="input"
+					:mapping-enabled="!isReadOnly"
 					:connection-type="NodeConnectionType.Main"
+					pane-type="input"
 				/>
 			</div>
 

diff --git a/packages/editor-ui/src/components/ParameterInputList.vue b/packages/editor-ui/src/components/ParameterInputList.vue
@@ -472,6 +472,7 @@ function getParameterValue<T extends NodeParameterValueType = NodeParameterValue
 				:node="node"
 				:path="getPath(parameter.name)"
 				:dependent-parameters-values="getDependentParametersValues(parameter)"
+				:is-read-only="isReadOnly"
 				input-size="small"
 				label-size="small"
 				@value-changed="valueChanged"

diff --git a/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue b/packages/editor-ui/src/components/ResourceMapper/MappingFields.vue
@@ -21,23 +21,26 @@ import {
 	parseResourceMapperFieldName,
 } from '@/utils/nodeTypesUtils';
 import { useNodeSpecificationValues } from '@/composables/useNodeSpecificationValues';
+import { N8nIconButton, N8nInputLabel, N8nOption, N8nSelect, N8nTooltip } from 'n8n-design-system';
 
 interface Props {
 	parameter: INodeProperties;
 	path: string;
 	nodeValues: INodeParameters | undefined;
 	fieldsToMap: ResourceMapperField[];
 	paramValue: ResourceMapperValue;
-	labelSize: string;
+	labelSize: 'small' | 'medium';
 	showMatchingColumnsSelector: boolean;
 	showMappingModeSelect: boolean;
 	loading: boolean;
 	refreshInProgress: boolean;
 	teleported?: boolean;
+	isReadOnly?: boolean;
 }
 
 const props = withDefaults(defineProps<Props>(), {
 	teleported: true,
+	isReadOnly: false,
 });
 const FORCE_TEXT_INPUT_FOR_TYPES: FieldType[] = ['time', 'object', 'array'];
 
@@ -285,7 +288,7 @@ defineExpose({
 
 <template>
 	<div class="mt-xs" data-test-id="mapping-fields-container">
-		<n8n-input-label
+		<N8nInputLabel
 			:label="valuesLabel"
 			:underline="true"
 			:size="labelSize"
@@ -300,14 +303,15 @@ defineExpose({
 					:custom-actions="parameterActions"
 					:loading="props.refreshInProgress"
 					:loading-message="fetchingFieldsLabel"
+					:is-read-only="isReadOnly"
 					@update:model-value="onParameterActionSelected"
 				/>
 			</template>
-		</n8n-input-label>
+		</N8nInputLabel>
 		<div v-if="orderedFields.length === 0" class="mt-3xs mb-xs">
-			<n8n-text size="small">{{
+			<N8nText size="small">{{
 				$locale.baseText('fixedCollectionParameter.currentlyNoItemsExist')
-			}}</n8n-text>
+			}}</N8nText>
 		</div>
 		<div
 			v-for="field in orderedFields"
@@ -322,7 +326,7 @@ defineExpose({
 				v-if="resourceMapperMode === 'add' && field.required"
 				:class="['delete-option', 'mt-5xs', $style.parameterTooltipIcon]"
 			>
-				<n8n-tooltip placement="top">
+				<N8nTooltip placement="top">
 					<template #content>
 						<span>{{
 							locale.baseText('resourceMapper.mandatoryField.title', {
@@ -331,7 +335,7 @@ defineExpose({
 						}}</span>
 					</template>
 					<font-awesome-icon icon="question-circle" />
-				</n8n-tooltip>
+				</N8nTooltip>
 			</div>
 			<div
 				v-else-if="
@@ -343,7 +347,7 @@ defineExpose({
 				"
 				:class="['delete-option', 'mt-5xs']"
 			>
-				<n8n-icon-button
+				<N8nIconButton
 					type="tertiary"
 					text
 					size="mini"
@@ -356,16 +360,17 @@ defineExpose({
 							},
 						})
 					"
+					:disabled="isReadOnly"
 					@click="removeField(field.name)"
-				></n8n-icon-button>
+				></N8nIconButton>
 			</div>
 			<div :class="$style.parameterInput">
 				<ParameterInputFull
 					:parameter="field"
 					:value="getParameterValue(field.name)"
 					:display-options="true"
 					:path="`${props.path}.${field.name}`"
-					:is-read-only="refreshInProgress || field.readOnly"
+					:is-read-only="refreshInProgress || field.readOnly || isReadOnly"
 					:hide-issues="true"
 					:node-values="nodeValues"
 					:class="$style.parameterInputFull"
@@ -379,26 +384,26 @@ defineExpose({
 			/>
 		</div>
 		<div :class="['add-option', $style.addOption]" data-test-id="add-fields-select">
-			<n8n-select
+			<N8nSelect
 				:placeholder="
 					locale.baseText('resourceMapper.addFieldToSend', {
 						interpolate: { fieldWord: singularFieldWordCapitalized },
 					})
 				"
 				size="small"
 				:teleported="teleported"
-				:disabled="addFieldOptions.length == 0"
+				:disabled="addFieldOptions.length == 0 || isReadOnly"
 				@update:model-value="addField"
 			>
-				<n8n-option
+				<N8nOption
 					v-for="item in addFieldOptions"
 					:key="item.value"
 					:label="item.name"
 					:value="item.value"
 					:disabled="item.disabled"
 				>
-				</n8n-option>
-			</n8n-select>
+				</N8nOption>
+			</N8nSelect>
 		</div>
 	</div>
 </template>

diff --git a/packages/editor-ui/src/components/ResourceMapper/MappingModeSelect.vue b/packages/editor-ui/src/components/ResourceMapper/MappingModeSelect.vue
@@ -3,20 +3,22 @@ import type { INodePropertyTypeOptions, ResourceMapperFields } from 'n8n-workflo
 import { computed, ref, watch } from 'vue';
 import { i18n as locale } from '@/plugins/i18n';
 import { useNodeSpecificationValues } from '@/composables/useNodeSpecificationValues';
+import { N8nInputLabel, N8nSelect, N8nText } from 'n8n-design-system';
 
 interface Props {
 	initialValue: string;
 	fieldsToMap: ResourceMapperFields['fields'];
-	inputSize: string;
-	labelSize: string;
+	inputSize: 'small' | 'medium';
+	labelSize: 'small' | 'medium';
 	typeOptions: INodePropertyTypeOptions | undefined;
 	serviceName: string;
 	loading: boolean;
 	loadingError: boolean;
 	teleported?: boolean;
+	isReadOnly?: boolean;
 }
 
-const props = defineProps<Props>();
+const props = withDefaults(defineProps<Props>(), { isReadOnly: false });
 const { resourceMapperTypeOptions, pluralFieldWord, singularFieldWord } =
 	useNodeSpecificationValues(props.typeOptions);
 
@@ -103,21 +105,22 @@ defineExpose({
 
 <template>
 	<div data-test-id="mapping-mode-select">
-		<n8n-input-label
+		<N8nInputLabel
 			:label="locale.baseText('resourceMapper.mappingMode.label')"
 			:bold="false"
 			:required="false"
 			:size="labelSize"
 			color="text-dark"
 		>
 			<div class="mt-5xs">
-				<n8n-select
+				<N8nSelect
 					:model-value="selected"
 					:teleported="teleported"
 					:size="props.inputSize"
+					:disabled="isReadOnly"
 					@update:model-value="onModeChanged"
 				>
-					<n8n-option
+					<N8nOption
 						v-for="option in mappingModeOptions"
 						:key="option.value"
 						:value="option.value"
@@ -130,28 +133,28 @@ defineExpose({
 							</div>
 							<div class="option-description" v-html="option.description" />
 						</div>
-					</n8n-option>
-				</n8n-select>
+					</N8nOption>
+				</N8nSelect>
 			</div>
 			<div class="mt-5xs">
-				<n8n-text v-if="loading" size="small">
-					<n8n-icon icon="sync-alt" size="xsmall" :spin="true" />
+				<N8nText v-if="loading" size="small">
+					<N8nIcon icon="sync-alt" size="xsmall" :spin="true" />
 					{{
 						locale.baseText('resourceMapper.fetchingFields.message', {
 							interpolate: {
 								fieldWord: pluralFieldWord,
 							},
 						})
 					}}
-				</n8n-text>
-				<n8n-text v-else-if="errorMessage !== ''" size="small" color="danger">
-					<n8n-icon icon="exclamation-triangle" size="xsmall" />
+				</N8nText>
+				<N8nText v-else-if="errorMessage !== ''" size="small" color="danger">
+					<N8nIcon icon="exclamation-triangle" size="xsmall" />
 					{{ errorMessage }}
-					<n8n-link size="small" theme="danger" :underline="true" @click="onRetryClick">
+					<N8nLink size="small" theme="danger" :underline="true" @click="onRetryClick">
 						{{ locale.baseText('generic.retry') }}
-					</n8n-link>
-				</n8n-text>
+					</N8nLink>
+				</N8nText>
 			</div>
-		</n8n-input-label>
+		</N8nInputLabel>
 	</div>
 </template>