mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 05:29:44 +02:00
feat(editor): Add Changes diff tab to workflow review detail pane (#35431)
This commit is contained in:
Kai
GitHub
parent
40dac4b90a
commit
6c8197e379
|
|
@ -5400,7 +5400,17 @@
|
|||
"workflowReviews.status.changesRequested": "Changes requested",
|
||||
"workflowReviews.status.approved": "Approved",
|
||||
"workflowReviews.status.closed": "Closed",
|
||||
"workflowReviews.detail.placeholder": "Full review details are coming soon.",
|
||||
"workflowReviews.detail.tabs.activity": "Activity",
|
||||
"workflowReviews.detail.tabs.changes": "Changes",
|
||||
"workflowReviews.detail.activity.noDescription": "No description provided.",
|
||||
"workflowReviews.changes.closed.body": "Changes aren't shown for closed reviews.",
|
||||
"workflowReviews.changes.empty": "This review doesn't contain any workflows.",
|
||||
"workflowReviews.changes.unavailable": "Couldn't load the changes for this review. Refresh to try again.",
|
||||
"workflowReviews.changes.versionUnavailable.body": "The submitted workflow version is no longer available.",
|
||||
"workflowReviews.changes.noChanges": "No changes compared to the published version.",
|
||||
"workflowReviews.changes.sourceLabel": "Published: {version}",
|
||||
"workflowReviews.changes.targetLabel": "In review: {version}",
|
||||
"workflowReviews.changes.firstPublish.sourceEmpty": "This workflow hasn't been published before.",
|
||||
"workflowReviews.detail.notFound.title": "Review not found",
|
||||
"workflowReviews.detail.notFound.body": "This review doesn't exist or you don't have access to it.",
|
||||
"workflowReviews.detail.decision.approve": "Approve",
|
||||
|
|
|
|||
|
|
@ -1,5 +1,11 @@
|
|||
<script lang="ts" setup>
|
||||
const { fullWidth = false } = defineProps<{
|
||||
fullWidth?: boolean;
|
||||
}>();
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<div :class="$style.wrapper">
|
||||
<div :class="[$style.wrapper, fullWidth ? $style.fullWidth : '']">
|
||||
<slot name="header" />
|
||||
<main :class="$style.content">
|
||||
<slot />
|
||||
|
|
@ -23,6 +29,11 @@
|
|||
}
|
||||
}
|
||||
|
||||
.fullWidth {
|
||||
max-width: none;
|
||||
padding-inline: var(--spacing--md);
|
||||
}
|
||||
|
||||
.content {
|
||||
display: grid;
|
||||
height: 100%;
|
||||
|
|
|
|||
|
|
@ -0,0 +1,207 @@
|
|||
import type {
|
||||
WorkflowReviewRequestWorkflowDetail,
|
||||
WorkflowReviewVersionSnapshot,
|
||||
} from '@n8n/api-types';
|
||||
import { createTestingPinia } from '@pinia/testing';
|
||||
import type { INode } from 'n8n-workflow';
|
||||
import { isReactive, reactive } from 'vue';
|
||||
import { createComponentRenderer } from '@/__tests__/render';
|
||||
import type { IWorkflowDb } from '@/Interface';
|
||||
|
||||
import WorkflowReviewChangesSection from './WorkflowReviewChangesSection.vue';
|
||||
|
||||
type DiffViewProps = {
|
||||
sourceWorkflow?: IWorkflowDb;
|
||||
targetWorkflow?: IWorkflowDb;
|
||||
sourceLabel?: string;
|
||||
targetLabel?: string;
|
||||
};
|
||||
|
||||
const { diffViewProps } = vi.hoisted(() => ({
|
||||
diffViewProps: [] as DiffViewProps[],
|
||||
}));
|
||||
|
||||
vi.mock('@/features/workflows/workflowDiff/WorkflowDiffView.vue', () => ({
|
||||
default: {
|
||||
name: 'WorkflowDiffView',
|
||||
props: ['sourceWorkflow', 'targetWorkflow', 'sourceLabel', 'targetLabel'],
|
||||
setup(props: DiffViewProps) {
|
||||
diffViewProps.push(props);
|
||||
},
|
||||
template:
|
||||
'<div data-test-id="workflow-diff-view-stub"><slot name="sourceLabel" /><slot name="targetLabel" /></div>',
|
||||
},
|
||||
}));
|
||||
|
||||
const renderComponent = createComponentRenderer(WorkflowReviewChangesSection);
|
||||
|
||||
function makeNode(overrides: Partial<INode> = {}): INode {
|
||||
return {
|
||||
id: 'node-1',
|
||||
name: 'Node 1',
|
||||
type: 'n8n-nodes-base.noOp',
|
||||
typeVersion: 1,
|
||||
position: [0, 0],
|
||||
parameters: {},
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeSnapshot(
|
||||
overrides: Partial<WorkflowReviewVersionSnapshot> = {},
|
||||
): WorkflowReviewVersionSnapshot {
|
||||
return {
|
||||
versionId: '77b70644-0000-0000-0000-000000000000',
|
||||
nodes: [makeNode()],
|
||||
connections: {},
|
||||
nodeGroups: [],
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeWorkflow(
|
||||
overrides: Partial<WorkflowReviewRequestWorkflowDetail> = {},
|
||||
): WorkflowReviewRequestWorkflowDetail {
|
||||
return {
|
||||
workflowId: 'wf-1',
|
||||
workflowName: 'Payment handler',
|
||||
workflowVersionId: '77b70644-0000-0000-0000-000000000000',
|
||||
pinnedVersion: makeSnapshot(),
|
||||
baselineVersion: makeSnapshot({
|
||||
versionId: '0f123890-0000-0000-0000-000000000000',
|
||||
nodes: [makeNode({ id: 'node-removed', name: 'Removed node' }), makeNode()],
|
||||
}),
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('WorkflowReviewChangesSection', () => {
|
||||
beforeEach(() => {
|
||||
createTestingPinia();
|
||||
diffViewProps.length = 0;
|
||||
});
|
||||
|
||||
it('labels both diff sides with a version status', () => {
|
||||
const { getByTestId } = renderComponent({ props: { workflow: makeWorkflow() } });
|
||||
|
||||
expect(getByTestId('workflow-review-changes-source-label')).toHaveTextContent('0f123890');
|
||||
expect(getByTestId('workflow-review-changes-target-label')).toHaveTextContent('77b70644');
|
||||
});
|
||||
|
||||
it('shows an unavailable state when the pinned version was pruned', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
workflow: makeWorkflow({
|
||||
workflowVersionId: null,
|
||||
pinnedVersion: null,
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-version-unavailable')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-diff-view-stub')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows a no-changes state when pinned and baseline are the same version', () => {
|
||||
const snapshot = makeSnapshot();
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
workflow: makeWorkflow({ pinnedVersion: snapshot, baselineVersion: snapshot }),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-no-changes')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-diff-view-stub')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows a no-changes state when versions differ only by id', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
workflow: makeWorkflow({
|
||||
pinnedVersion: makeSnapshot(),
|
||||
baselineVersion: makeSnapshot({ versionId: 'other-version-id' }),
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-no-changes')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-diff-view-stub')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
// First publish: no baseline, so the diff gets no source workflow — and that
|
||||
// side carries no publish-status label either, since nothing was published.
|
||||
it('renders a sourceless diff with no source label for a first publish', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
workflow: makeWorkflow({ baselineVersion: null }),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-diff-view-stub')).toBeInTheDocument();
|
||||
expect(diffViewProps).toHaveLength(1);
|
||||
expect(diffViewProps[0].sourceWorkflow).toBeUndefined();
|
||||
expect(diffViewProps[0].targetWorkflow?.id).toBe('wf-1');
|
||||
expect(queryByTestId('workflow-review-changes-source-label')).not.toBeInTheDocument();
|
||||
expect(getByTestId('workflow-review-changes-target-label')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('hands the diff detached workflows built from the snapshots', () => {
|
||||
const workflow = makeWorkflow();
|
||||
renderComponent({ props: { workflow } });
|
||||
|
||||
expect(diffViewProps).toHaveLength(1);
|
||||
const { sourceWorkflow, targetWorkflow, sourceLabel, targetLabel } = diffViewProps[0];
|
||||
|
||||
expect(sourceWorkflow?.id).toBe('wf-1');
|
||||
expect(sourceWorkflow?.name).toBe('Payment handler');
|
||||
expect(sourceWorkflow?.versionId).toBe('0f123890-0000-0000-0000-000000000000');
|
||||
expect(sourceWorkflow?.nodes.map((node) => node.id)).toEqual(['node-removed', 'node-1']);
|
||||
|
||||
expect(targetWorkflow?.versionId).toBe('77b70644-0000-0000-0000-000000000000');
|
||||
expect(targetWorkflow?.nodes.map((node) => node.id)).toEqual(['node-1']);
|
||||
|
||||
// Detached copies: not sharing references with the DTO, so the diff
|
||||
// canvases can't feed writes back into store state.
|
||||
expect(targetWorkflow?.nodes).not.toBe(workflow.pinnedVersion?.nodes);
|
||||
|
||||
expect(sourceLabel).toContain('Published');
|
||||
expect(sourceLabel).toContain('0f123890');
|
||||
expect(targetLabel).toContain('In review');
|
||||
expect(targetLabel).toContain('77b70644');
|
||||
});
|
||||
|
||||
// Feed a genuinely reactive prop, The guard matters — a reactive source lets
|
||||
// WorkflowDiffView's position writes drive a hydrate → dispose → hydrate loop
|
||||
// that pins the main thread.
|
||||
it('detaches the handed-off workflows from a reactive source', () => {
|
||||
const workflow = reactive(makeWorkflow());
|
||||
renderComponent({ props: { workflow } });
|
||||
|
||||
expect(diffViewProps).toHaveLength(1);
|
||||
const { sourceWorkflow, targetWorkflow } = diffViewProps[0];
|
||||
|
||||
expect(isReactive(workflow.pinnedVersion)).toBe(true);
|
||||
expect(isReactive(sourceWorkflow)).toBe(false);
|
||||
expect(isReactive(targetWorkflow)).toBe(false);
|
||||
expect(isReactive(targetWorkflow?.nodes)).toBe(false);
|
||||
expect(isReactive(targetWorkflow?.nodes[0])).toBe(false);
|
||||
});
|
||||
|
||||
it('renders the diff when versions differ only by an execution flag', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
workflow: makeWorkflow({
|
||||
pinnedVersion: makeSnapshot({ nodes: [makeNode({ disabled: true })] }),
|
||||
baselineVersion: makeSnapshot({
|
||||
versionId: '0f123890-0000-0000-0000-000000000000',
|
||||
nodes: [makeNode()],
|
||||
}),
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-diff-view-stub')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-no-changes')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,162 @@
|
|||
<script setup lang="ts">
|
||||
import type {
|
||||
WorkflowReviewRequestWorkflowDetail,
|
||||
WorkflowReviewVersionSnapshot,
|
||||
} from '@n8n/api-types';
|
||||
import { N8nCallout, N8nText } from '@n8n/design-system';
|
||||
import { useI18n } from '@n8n/i18n';
|
||||
import { deepCopy } from 'n8n-workflow';
|
||||
import isEqual from 'lodash/isEqual';
|
||||
import omit from 'lodash/omit';
|
||||
import { computed, markRaw } from 'vue';
|
||||
|
||||
import WorkflowDiffView from '@/features/workflows/workflowDiff/WorkflowDiffView.vue';
|
||||
import type { IWorkflowDb } from '@/Interface';
|
||||
|
||||
const props = defineProps<{
|
||||
workflow: WorkflowReviewRequestWorkflowDetail;
|
||||
}>();
|
||||
|
||||
const i18n = useI18n();
|
||||
|
||||
const shortVersion = (versionId: string) => versionId.slice(0, 8);
|
||||
|
||||
/**
|
||||
* A snapshot minus its identity and timestamp — i.e. everything the diff renders.
|
||||
* Derived by omission rather than an explicit field list to avoid drift.
|
||||
*/
|
||||
function contentOf(snapshot: WorkflowReviewVersionSnapshot) {
|
||||
return omit(snapshot, ['versionId', 'createdAt']);
|
||||
}
|
||||
|
||||
const hasChanges = computed(() => {
|
||||
const { pinnedVersion, baselineVersion } = props.workflow;
|
||||
|
||||
if (!pinnedVersion) return false;
|
||||
// No baseline means first publish: everything is a change.
|
||||
if (!baselineVersion) return true;
|
||||
if (pinnedVersion.versionId === baselineVersion.versionId) return false;
|
||||
|
||||
return !isEqual(contentOf(baselineVersion), contentOf(pinnedVersion));
|
||||
});
|
||||
|
||||
// Hand the diff a fully-detached, non-reactive copy.
|
||||
function snapshotToWorkflow(snapshot: WorkflowReviewVersionSnapshot): IWorkflowDb {
|
||||
return markRaw(
|
||||
deepCopy({
|
||||
id: props.workflow.workflowId,
|
||||
name: props.workflow.workflowName,
|
||||
active: false,
|
||||
isArchived: false,
|
||||
createdAt: snapshot.createdAt,
|
||||
updatedAt: snapshot.createdAt,
|
||||
versionId: snapshot.versionId,
|
||||
activeVersionId: null,
|
||||
nodes: snapshot.nodes,
|
||||
connections: snapshot.connections,
|
||||
nodeGroups: snapshot.nodeGroups,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
const sourceWorkflow = computed(() =>
|
||||
props.workflow.baselineVersion ? snapshotToWorkflow(props.workflow.baselineVersion) : undefined,
|
||||
);
|
||||
const targetWorkflow = computed(() =>
|
||||
props.workflow.pinnedVersion ? snapshotToWorkflow(props.workflow.pinnedVersion) : undefined,
|
||||
);
|
||||
|
||||
const sourceLabel = computed(() =>
|
||||
props.workflow.baselineVersion
|
||||
? i18n.baseText('workflowReviews.changes.sourceLabel', {
|
||||
interpolate: { version: shortVersion(props.workflow.baselineVersion.versionId) },
|
||||
})
|
||||
: undefined,
|
||||
);
|
||||
const targetLabel = computed(() =>
|
||||
props.workflow.pinnedVersion
|
||||
? i18n.baseText('workflowReviews.changes.targetLabel', {
|
||||
interpolate: { version: shortVersion(props.workflow.pinnedVersion.versionId) },
|
||||
})
|
||||
: undefined,
|
||||
);
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<N8nCallout
|
||||
v-if="!workflow.pinnedVersion"
|
||||
theme="warning"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-version-unavailable"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.versionUnavailable.body') }}
|
||||
</N8nCallout>
|
||||
<N8nCallout
|
||||
v-else-if="!hasChanges"
|
||||
theme="info"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-no-changes"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.noChanges') }}
|
||||
</N8nCallout>
|
||||
<div v-else :class="$style.diff" data-test-id="workflow-review-changes-diff">
|
||||
<WorkflowDiffView
|
||||
:source-workflow="sourceWorkflow"
|
||||
:target-workflow="targetWorkflow"
|
||||
:source-label="sourceLabel"
|
||||
:target-label="targetLabel"
|
||||
>
|
||||
<!-- Only when a baseline exists: with no prior published version there is
|
||||
no publish status to represent. -->
|
||||
<template v-if="workflow.baselineVersion" #sourceLabel>
|
||||
<span :class="$style.versionBadge" data-test-id="workflow-review-changes-source-label">
|
||||
<span :class="[$style.statusDot, $style.statusDotPublished]" />
|
||||
<N8nText color="text-dark" size="small" compact>{{ sourceLabel }}</N8nText>
|
||||
</span>
|
||||
</template>
|
||||
<template #targetLabel>
|
||||
<span :class="$style.versionBadge" data-test-id="workflow-review-changes-target-label">
|
||||
<span :class="[$style.statusDot, $style.statusDotInReview]" />
|
||||
<N8nText color="text-dark" size="small" compact>{{ targetLabel }}</N8nText>
|
||||
</span>
|
||||
</template>
|
||||
<template #sourceEmptyText>
|
||||
{{ i18n.baseText('workflowReviews.changes.firstPublish.sourceEmpty') }}
|
||||
</template>
|
||||
</WorkflowDiffView>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<style module lang="scss">
|
||||
.callout {
|
||||
max-width: var(--review-callout--max-width, 34rem);
|
||||
}
|
||||
|
||||
.versionBadge {
|
||||
composes: sourceBadge from '../../workflows/workflowDiff/workflowDiff.module.scss';
|
||||
}
|
||||
|
||||
.statusDot {
|
||||
height: var(--spacing--2xs);
|
||||
width: var(--spacing--2xs);
|
||||
border-radius: 50%;
|
||||
display: inline-block;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
|
||||
.statusDotPublished {
|
||||
background-color: var(--color--mint-600);
|
||||
}
|
||||
|
||||
.statusDotInReview {
|
||||
background-color: var(--color--yellow-500);
|
||||
}
|
||||
|
||||
.diff {
|
||||
height: 100%;
|
||||
min-height: 0;
|
||||
border: var(--border-width) var(--border-style) var(--border-color--subtle);
|
||||
border-radius: var(--radius--md);
|
||||
overflow: hidden;
|
||||
}
|
||||
</style>
|
||||
|
|
@ -0,0 +1,299 @@
|
|||
import type {
|
||||
WorkflowReviewInboxItem,
|
||||
WorkflowReviewRequestDetail,
|
||||
WorkflowReviewRequestWorkflowDetail,
|
||||
} from '@n8n/api-types';
|
||||
import { createTestingPinia } from '@pinia/testing';
|
||||
import { createComponentRenderer } from '@/__tests__/render';
|
||||
|
||||
import WorkflowReviewDetailTabs from './WorkflowReviewDetailTabs.vue';
|
||||
|
||||
vi.mock('./WorkflowReviewChangesSection.vue', () => ({
|
||||
default: {
|
||||
name: 'WorkflowReviewChangesSection',
|
||||
props: ['workflow'],
|
||||
template: '<div data-test-id="workflow-review-changes-section" />',
|
||||
},
|
||||
}));
|
||||
|
||||
const renderComponent = createComponentRenderer(WorkflowReviewDetailTabs, {
|
||||
global: {
|
||||
stubs: {
|
||||
N8nTooltip: {
|
||||
props: ['disabled', 'content'],
|
||||
template: `
|
||||
<div data-test-id="workflow-review-decision-tooltip" :data-disabled="disabled" :data-content="content">
|
||||
<slot />
|
||||
</div>`,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
function decisionButtons(getByTestId: (id: string) => HTMLElement) {
|
||||
return [
|
||||
getByTestId('workflow-review-approve-button'),
|
||||
getByTestId('workflow-review-request-changes-button'),
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* The tab bar renders a tooltip per tab, which the N8nTooltip stub matches too,
|
||||
* so walk up from the button rather than querying the test id globally.
|
||||
*/
|
||||
function decisionTooltip(button: HTMLElement) {
|
||||
const tooltip = button.closest('[data-test-id="workflow-review-decision-tooltip"]');
|
||||
if (!tooltip) throw new Error('decision button is not wrapped in a tooltip');
|
||||
return tooltip;
|
||||
}
|
||||
|
||||
function makeInboxItem(overrides: Partial<WorkflowReviewInboxItem> = {}): WorkflowReviewInboxItem {
|
||||
return {
|
||||
id: 'req-1',
|
||||
projectId: 'proj-1',
|
||||
title: 'Needs review',
|
||||
workflowName: 'My workflow',
|
||||
workflowVersionId: null,
|
||||
requester: null,
|
||||
reviewers: [],
|
||||
decision: 'pending',
|
||||
state: 'open',
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
updatedAt: '2024-01-01T00:00:00.000Z',
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeWorkflowDetail(
|
||||
overrides: Partial<WorkflowReviewRequestWorkflowDetail> = {},
|
||||
): WorkflowReviewRequestWorkflowDetail {
|
||||
return {
|
||||
workflowId: 'wf-1',
|
||||
workflowName: 'My workflow',
|
||||
workflowVersionId: 'version-1',
|
||||
pinnedVersion: null,
|
||||
baselineVersion: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeDetail(
|
||||
overrides: Partial<WorkflowReviewRequestDetail> = {},
|
||||
): WorkflowReviewRequestDetail {
|
||||
return {
|
||||
...makeInboxItem(),
|
||||
description: null,
|
||||
workflows: [makeWorkflowDetail()],
|
||||
viewerCanDecide: true,
|
||||
viewerDecisionIneligibilityReason: null,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe('WorkflowReviewDetailTabs', () => {
|
||||
beforeEach(() => {
|
||||
createTestingPinia();
|
||||
});
|
||||
|
||||
it('emits update:tab when a tab is selected', async () => {
|
||||
const { getByText, emitted } = renderComponent({
|
||||
props: { review: makeDetail(), tab: 'activity', deciding: false },
|
||||
});
|
||||
|
||||
getByText('Changes').click();
|
||||
|
||||
expect(emitted('update:tab')).toEqual([['changes']]);
|
||||
});
|
||||
|
||||
describe('activity tab', () => {
|
||||
it('renders the description when present', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({ description: 'Adds retry logic' }),
|
||||
tab: 'activity',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-description')).toHaveTextContent('Adds retry logic');
|
||||
expect(queryByTestId('workflow-review-no-description')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('renders a fallback when the review has no description', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: { review: makeDetail(), tab: 'activity', deciding: false },
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-no-description')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe('changes tab', () => {
|
||||
it('renders one section per workflow', () => {
|
||||
const { getAllByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({
|
||||
workflows: [
|
||||
makeWorkflowDetail(),
|
||||
makeWorkflowDetail({ workflowId: 'wf-2', workflowName: 'Other workflow' }),
|
||||
],
|
||||
}),
|
||||
tab: 'changes',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(getAllByTestId('workflow-review-changes-section')).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('shows an info state instead of diffs for a closed review', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({ state: 'closed', decision: 'approved' }),
|
||||
tab: 'changes',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-closed')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-section')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows an empty state when the detail has no workflows', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({ workflows: [] }),
|
||||
tab: 'changes',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-empty')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows an error state when the detail fetch failed', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: { review: makeInboxItem(), tab: 'changes', deciding: false },
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-unavailable')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-empty')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
describe('decision actions', () => {
|
||||
it('emits decisions from the action buttons', () => {
|
||||
const { getByTestId, emitted } = renderComponent({
|
||||
props: { review: makeDetail(), tab: 'activity', deciding: false },
|
||||
});
|
||||
|
||||
getByTestId('workflow-review-approve-button').click();
|
||||
getByTestId('workflow-review-request-changes-button').click();
|
||||
|
||||
expect(emitted('decide')).toEqual([['approved'], ['changes_requested']]);
|
||||
});
|
||||
|
||||
it('hides the action buttons for a closed review', () => {
|
||||
const { queryByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({ state: 'closed', decision: 'approved' }),
|
||||
tab: 'activity',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(queryByTestId('workflow-review-approve-button')).not.toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-request-changes-button')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('hides the action buttons on a review whose detail payload never loaded', () => {
|
||||
const { queryByTestId } = renderComponent({
|
||||
props: { review: makeInboxItem(), tab: 'activity', deciding: false },
|
||||
});
|
||||
|
||||
expect(queryByTestId('workflow-review-approve-button')).not.toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-request-changes-button')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('disables the action buttons while deciding', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: { review: makeDetail(), tab: 'activity', deciding: true },
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-approve-button')).toBeDisabled();
|
||||
expect(getByTestId('workflow-review-request-changes-button')).toBeDisabled();
|
||||
});
|
||||
|
||||
// Each button carries its own tooltip, so both must agree.
|
||||
it('keeps the buttons enabled and the tooltips off when the viewer can decide', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: { review: makeDetail(), tab: 'activity', deciding: false },
|
||||
});
|
||||
|
||||
for (const button of decisionButtons(getByTestId)) {
|
||||
expect(button).not.toBeDisabled();
|
||||
expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'true');
|
||||
}
|
||||
});
|
||||
|
||||
it('disables the buttons and says why when the viewer contributed a version', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({
|
||||
viewerCanDecide: false,
|
||||
viewerDecisionIneligibilityReason: 'author',
|
||||
}),
|
||||
tab: 'activity',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
for (const button of decisionButtons(getByTestId)) {
|
||||
expect(button).toBeDisabled();
|
||||
expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'false');
|
||||
expect(decisionTooltip(button)).toHaveAttribute(
|
||||
'data-content',
|
||||
'You contributed a version to this review.',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('falls back to the generic permission hint for any other reason', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({
|
||||
viewerCanDecide: false,
|
||||
viewerDecisionIneligibilityReason: 'missing_publish_permission',
|
||||
}),
|
||||
tab: 'activity',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
for (const button of decisionButtons(getByTestId)) {
|
||||
expect(button).toBeDisabled();
|
||||
expect(decisionTooltip(button)).toHaveAttribute(
|
||||
'data-content',
|
||||
'Missing permissions to perform this action',
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not emit a decision for an ineligible viewer', () => {
|
||||
const { getByTestId, emitted } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({
|
||||
viewerCanDecide: false,
|
||||
viewerDecisionIneligibilityReason: 'author',
|
||||
}),
|
||||
tab: 'activity',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
getByTestId('workflow-review-approve-button').click();
|
||||
|
||||
expect(emitted('decide')).toBeUndefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,183 @@
|
|||
<script setup lang="ts">
|
||||
import type { WorkflowReviewInboxItem, WorkflowReviewRequestDetail } from '@n8n/api-types';
|
||||
import { N8nButton, N8nCallout, N8nTabs, N8nText, N8nTooltip } from '@n8n/design-system';
|
||||
import { useI18n } from '@n8n/i18n';
|
||||
import { computed } from 'vue';
|
||||
|
||||
import type { WorkflowReviewDecisionInput } from '../workflowReviews.api';
|
||||
import WorkflowReviewChangesSection from './WorkflowReviewChangesSection.vue';
|
||||
|
||||
export type WorkflowReviewDetailTab = 'activity' | 'changes';
|
||||
|
||||
const props = defineProps<{
|
||||
review: WorkflowReviewInboxItem | WorkflowReviewRequestDetail;
|
||||
tab: WorkflowReviewDetailTab;
|
||||
deciding: boolean;
|
||||
}>();
|
||||
|
||||
const emit = defineEmits<{
|
||||
'update:tab': [tab: WorkflowReviewDetailTab];
|
||||
decide: [decision: WorkflowReviewDecisionInput];
|
||||
}>();
|
||||
|
||||
const i18n = useI18n();
|
||||
|
||||
// The view falls back to the inbox list item while the full detail payload is
|
||||
// missing.
|
||||
const detail = computed<WorkflowReviewRequestDetail | null>(() =>
|
||||
'workflows' in props.review ? props.review : null,
|
||||
);
|
||||
|
||||
const viewerCanDecide = computed(() => detail.value?.viewerCanDecide ?? false);
|
||||
|
||||
const ineligibilityHint = computed(() => {
|
||||
if (!detail.value || detail.value.viewerCanDecide) return '';
|
||||
// Any reason other than 'author' gets the generic permission hint, so new
|
||||
// backend reasons degrade gracefully instead of breaking the UI.
|
||||
return detail.value.viewerDecisionIneligibilityReason === 'author'
|
||||
? i18n.baseText('workflowReviews.detail.decision.ineligible.author')
|
||||
: i18n.baseText('generic.missing.permissions');
|
||||
});
|
||||
|
||||
const tabOptions = computed(() => [
|
||||
{
|
||||
label: i18n.baseText('workflowReviews.detail.tabs.activity'),
|
||||
value: 'activity' as const,
|
||||
},
|
||||
{
|
||||
label: i18n.baseText('workflowReviews.detail.tabs.changes'),
|
||||
value: 'changes' as const,
|
||||
},
|
||||
]);
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<div :class="$style.container" data-test-id="workflow-review-detail-tabs">
|
||||
<div :class="$style.tabRow">
|
||||
<N8nTabs
|
||||
:model-value="tab"
|
||||
:options="tabOptions"
|
||||
variant="modern"
|
||||
data-test-id="workflow-review-detail-tab-bar"
|
||||
@update:model-value="emit('update:tab', $event)"
|
||||
/>
|
||||
|
||||
<!-- Gated on `detail`, not `review`: eligibility only arrives with the
|
||||
detail payload, so the list item alone can't say who may decide. -->
|
||||
<div v-if="detail?.state === 'open'" :class="$style.decisionActions">
|
||||
<N8nTooltip :disabled="!ineligibilityHint" :content="ineligibilityHint" :show-after="300">
|
||||
<N8nButton
|
||||
size="small"
|
||||
:disabled="deciding || !viewerCanDecide"
|
||||
data-test-id="workflow-review-approve-button"
|
||||
@click="emit('decide', 'approved')"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.detail.decision.approve') }}
|
||||
</N8nButton>
|
||||
</N8nTooltip>
|
||||
<N8nTooltip :disabled="!ineligibilityHint" :content="ineligibilityHint" :show-after="300">
|
||||
<N8nButton
|
||||
size="small"
|
||||
type="secondary"
|
||||
:disabled="deciding || !viewerCanDecide"
|
||||
data-test-id="workflow-review-request-changes-button"
|
||||
@click="emit('decide', 'changes_requested')"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.detail.decision.requestChanges') }}
|
||||
</N8nButton>
|
||||
</N8nTooltip>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div
|
||||
v-if="tab === 'activity'"
|
||||
:class="$style.panel"
|
||||
data-test-id="workflow-review-activity-panel"
|
||||
>
|
||||
<N8nText
|
||||
v-if="detail?.description"
|
||||
color="text-base"
|
||||
size="medium"
|
||||
:class="$style.description"
|
||||
data-test-id="workflow-review-description"
|
||||
>
|
||||
{{ detail.description }}
|
||||
</N8nText>
|
||||
<N8nText
|
||||
v-else
|
||||
color="text-light"
|
||||
size="medium"
|
||||
data-test-id="workflow-review-no-description"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.detail.activity.noDescription') }}
|
||||
</N8nText>
|
||||
</div>
|
||||
|
||||
<div v-else :class="$style.panel" data-test-id="workflow-review-changes-panel">
|
||||
<N8nCallout
|
||||
v-if="review.state === 'closed'"
|
||||
theme="info"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-closed"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.closed.body') }}
|
||||
</N8nCallout>
|
||||
<N8nCallout
|
||||
v-else-if="!detail"
|
||||
theme="warning"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-unavailable"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.unavailable') }}
|
||||
</N8nCallout>
|
||||
<template v-else-if="detail.workflows.length > 0">
|
||||
<WorkflowReviewChangesSection
|
||||
v-for="workflow in detail.workflows"
|
||||
:key="workflow.workflowId"
|
||||
:workflow="workflow"
|
||||
/>
|
||||
</template>
|
||||
<N8nText v-else color="text-light" size="medium" data-test-id="workflow-review-changes-empty">
|
||||
{{ i18n.baseText('workflowReviews.changes.empty') }}
|
||||
</N8nText>
|
||||
</div>
|
||||
</div>
|
||||
</template>
|
||||
|
||||
<style module lang="scss">
|
||||
.container {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
height: 100%;
|
||||
min-height: 0;
|
||||
}
|
||||
|
||||
.tabRow {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: space-between;
|
||||
gap: var(--spacing--sm);
|
||||
}
|
||||
|
||||
.panel {
|
||||
flex: 1;
|
||||
min-height: 0;
|
||||
overflow: auto;
|
||||
padding-top: var(--review-tab-bar--gap, calc(var(--spacing--sm) + 11px));
|
||||
}
|
||||
|
||||
.callout {
|
||||
max-width: var(--review-callout--max-width, 34rem);
|
||||
}
|
||||
|
||||
.description {
|
||||
white-space: pre-wrap;
|
||||
}
|
||||
|
||||
.decisionActions {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: var(--spacing--2xs);
|
||||
flex-shrink: 0;
|
||||
}
|
||||
</style>
|
||||
|
|
@ -103,6 +103,7 @@ function onListBackgroundClick() {
|
|||
<N8nTabs
|
||||
:model-value="activeTab"
|
||||
:options="tabOptions"
|
||||
variant="modern"
|
||||
data-test-id="workflow-reviews-tabs"
|
||||
@update:model-value="onTabChange"
|
||||
/>
|
||||
|
|
@ -191,8 +192,7 @@ function onListBackgroundClick() {
|
|||
.sidebar {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
flex: 0 0 35%;
|
||||
min-width: 12rem;
|
||||
flex: 0 0 26rem;
|
||||
height: 100%;
|
||||
border-right: var(--border-width) solid var(--color--foreground--tint-1);
|
||||
}
|
||||
|
|
@ -206,9 +206,10 @@ function onListBackgroundClick() {
|
|||
|
||||
.header {
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: var(--spacing--sm);
|
||||
padding: 0 var(--spacing--md) var(--spacing--md) 0;
|
||||
align-items: center;
|
||||
height: var(--review-tab-bar--height, var(--height--sm));
|
||||
padding-right: var(--spacing--md);
|
||||
margin-bottom: var(--review-tab-bar--gap, calc(var(--spacing--sm) + 11px));
|
||||
}
|
||||
|
||||
.list {
|
||||
|
|
|
|||
|
|
@ -11,6 +11,8 @@ export const WORKFLOW_REVIEW_REQUESTS_VIEW = 'WorkflowReviewRequestsView';
|
|||
* change — reserve the key here, hydrate it in the view, and write it with
|
||||
* router.replace({ query }) preserving params.
|
||||
* - `state`: 'open' | 'closed'. Default 'open' is omitted from the URL.
|
||||
* - `tab`: 'activity' | 'changes' — detail-pane tab. Default 'activity' is
|
||||
* omitted from the URL.
|
||||
* - Reserved for later filters: `q`, `projectId`, `author`, `reviewer`.
|
||||
*/
|
||||
export const REVIEW_INBOX_QUERY_PARAM = { state: 'state' } as const;
|
||||
export const REVIEW_INBOX_QUERY_PARAM = { state: 'state', tab: 'tab' } as const;
|
||||
|
|
|
|||
|
|
@ -23,6 +23,16 @@ vi.mock('@/app/composables/useDocumentTitle', () => ({
|
|||
}),
|
||||
}));
|
||||
|
||||
// The changes sections pull in the whole workflow-diff canvas machinery, which
|
||||
// is far too heavy for these routing/wiring tests.
|
||||
vi.mock('@/features/workflow-reviews/components/WorkflowReviewChangesSection.vue', () => ({
|
||||
default: {
|
||||
name: 'WorkflowReviewChangesSection',
|
||||
props: ['workflow'],
|
||||
template: '<div data-test-id="workflow-review-changes-section" />',
|
||||
},
|
||||
}));
|
||||
|
||||
const showError = vi.fn();
|
||||
const showMessage = vi.fn();
|
||||
|
||||
|
|
@ -169,6 +179,38 @@ describe('WorkflowReviewRequestsView', () => {
|
|||
expect(pushSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('drops the tab when selecting a different review, so it lands on Activity', async () => {
|
||||
await router.replace('/workflow-review-requests/req-2?state=closed&tab=changes');
|
||||
store.probeSettled = true;
|
||||
store.showSidebar = true;
|
||||
const replaceSpy = vi.spyOn(router, 'replace');
|
||||
|
||||
const { getByTestId } = renderComponent();
|
||||
getByTestId('select-review').click();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(replaceSpy).toHaveBeenCalledWith({
|
||||
params: { reviewRequestId: 'req-1' },
|
||||
query: { state: 'closed' },
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps the tab when re-selecting the review already open', async () => {
|
||||
await router.replace('/workflow-review-requests/req-1?tab=changes');
|
||||
store.probeSettled = true;
|
||||
store.showSidebar = true;
|
||||
const replaceSpy = vi.spyOn(router, 'replace');
|
||||
|
||||
const { getByTestId } = renderComponent();
|
||||
getByTestId('select-review').click();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(replaceSpy).toHaveBeenCalledWith({
|
||||
params: { reviewRequestId: 'req-1' },
|
||||
query: { tab: 'changes' },
|
||||
});
|
||||
});
|
||||
|
||||
it('clears the selection back to the bare inbox path', async () => {
|
||||
await router.replace('/workflow-review-requests/req-1?state=closed');
|
||||
store.probeSettled = true;
|
||||
|
|
@ -282,6 +324,61 @@ describe('WorkflowReviewRequestsView', () => {
|
|||
expect(store.setActiveTab).toHaveBeenCalledWith('closed');
|
||||
});
|
||||
|
||||
describe('detail tabs', () => {
|
||||
beforeEach(async () => {
|
||||
await router.replace('/workflow-review-requests/req-1');
|
||||
store.probeSettled = true;
|
||||
store.showSidebar = true;
|
||||
store.detail = createDetail();
|
||||
});
|
||||
|
||||
it('defaults to the activity tab', async () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(getByTestId('workflow-review-activity-panel')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-panel')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('hydrates the changes tab from the query', async () => {
|
||||
await router.replace('/workflow-review-requests/req-1?tab=changes');
|
||||
|
||||
const { getByTestId, queryByTestId } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(getByTestId('workflow-review-changes-panel')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-activity-panel')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('falls back to the activity tab for an invalid tab query', async () => {
|
||||
await router.replace('/workflow-review-requests/req-1?tab=bogus');
|
||||
|
||||
const { getByTestId } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(getByTestId('workflow-review-activity-panel')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('writes the tab to the query preserving selection and state', async () => {
|
||||
await router.replace('/workflow-review-requests/req-1?state=closed');
|
||||
|
||||
const { getByText } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
getByText('Changes').click();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(router.currentRoute.value.query).toEqual({ state: 'closed', tab: 'changes' });
|
||||
expect(router.currentRoute.value.params.reviewRequestId).toBe('req-1');
|
||||
|
||||
getByText('Activity').click();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(router.currentRoute.value.query).toEqual({ state: 'closed' });
|
||||
expect(router.currentRoute.value.params.reviewRequestId).toBe('req-1');
|
||||
});
|
||||
});
|
||||
|
||||
describe('decision actions', () => {
|
||||
const decisionResponse = (
|
||||
overrides: Partial<DecideWorkflowReviewRequestResponse> = {},
|
||||
|
|
@ -511,13 +608,12 @@ describe('WorkflowReviewRequestsView', () => {
|
|||
|
||||
// Each button carries its own tooltip, so both must agree.
|
||||
it('keeps the buttons enabled and the tooltips off when the viewer can decide', async () => {
|
||||
const { getByTestId, getAllByTestId } = renderComponent();
|
||||
const { getByTestId } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(getByTestId('workflow-review-approve-button')).not.toBeDisabled();
|
||||
expect(getByTestId('workflow-review-request-changes-button')).not.toBeDisabled();
|
||||
for (const tooltip of getAllByTestId('workflow-review-decision-tooltip')) {
|
||||
expect(tooltip).toHaveAttribute('data-disabled', 'true');
|
||||
for (const button of decisionButtons(getByTestId)) {
|
||||
expect(button).not.toBeDisabled();
|
||||
expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'true');
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -527,16 +623,13 @@ describe('WorkflowReviewRequestsView', () => {
|
|||
viewerDecisionIneligibilityReason: 'author',
|
||||
});
|
||||
|
||||
const { getByTestId, getAllByTestId } = renderComponent();
|
||||
const { getByTestId } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(getByTestId('workflow-review-approve-button')).toBeDisabled();
|
||||
expect(getByTestId('workflow-review-request-changes-button')).toBeDisabled();
|
||||
const tooltips = getAllByTestId('workflow-review-decision-tooltip');
|
||||
expect(tooltips).toHaveLength(2);
|
||||
for (const tooltip of tooltips) {
|
||||
expect(tooltip).toHaveAttribute('data-disabled', 'false');
|
||||
expect(tooltip).toHaveAttribute(
|
||||
for (const button of decisionButtons(getByTestId)) {
|
||||
expect(button).toBeDisabled();
|
||||
expect(decisionTooltip(button)).toHaveAttribute('data-disabled', 'false');
|
||||
expect(decisionTooltip(button)).toHaveAttribute(
|
||||
'data-content',
|
||||
'You contributed a version to this review.',
|
||||
);
|
||||
|
|
@ -549,12 +642,12 @@ describe('WorkflowReviewRequestsView', () => {
|
|||
viewerDecisionIneligibilityReason: 'missing_publish_permission',
|
||||
});
|
||||
|
||||
const { getByTestId, getAllByTestId } = renderComponent();
|
||||
const { getByTestId } = renderComponent();
|
||||
await waitAllPromises();
|
||||
|
||||
expect(getByTestId('workflow-review-approve-button')).toBeDisabled();
|
||||
for (const tooltip of getAllByTestId('workflow-review-decision-tooltip')) {
|
||||
expect(tooltip).toHaveAttribute(
|
||||
for (const button of decisionButtons(getByTestId)) {
|
||||
expect(button).toBeDisabled();
|
||||
expect(decisionTooltip(button)).toHaveAttribute(
|
||||
'data-content',
|
||||
'Missing permissions to perform this action',
|
||||
);
|
||||
|
|
@ -637,6 +730,23 @@ describe('WorkflowReviewRequestsView', () => {
|
|||
});
|
||||
});
|
||||
|
||||
function decisionButtons(getByTestId: (id: string) => HTMLElement) {
|
||||
return [
|
||||
getByTestId('workflow-review-approve-button'),
|
||||
getByTestId('workflow-review-request-changes-button'),
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* The tab bar renders a tooltip per tab, which the N8nTooltip stub matches too,
|
||||
* so walk up from the button rather than querying the test id globally.
|
||||
*/
|
||||
function decisionTooltip(button: HTMLElement) {
|
||||
const tooltip = button.closest('[data-test-id="workflow-review-decision-tooltip"]');
|
||||
if (!tooltip) throw new Error('decision button is not wrapped in a tooltip');
|
||||
return tooltip;
|
||||
}
|
||||
|
||||
function createInboxItem(): WorkflowReviewInboxItem {
|
||||
return {
|
||||
id: 'req-1',
|
||||
|
|
|
|||
|
|
@ -3,12 +3,14 @@ import { computed, onMounted, onUnmounted, ref, watch } from 'vue';
|
|||
import { storeToRefs } from 'pinia';
|
||||
import type { WorkflowReviewRequestState } from '@n8n/api-types';
|
||||
import { useI18n } from '@n8n/i18n';
|
||||
import { N8nButton, N8nHeading, N8nLoading, N8nText, N8nTooltip } from '@n8n/design-system';
|
||||
import { N8nHeading, N8nLoading, N8nText } from '@n8n/design-system';
|
||||
import { useRoute, useRouter } from 'vue-router';
|
||||
import PageViewLayout from '@/app/components/layouts/PageViewLayout.vue';
|
||||
import { useDocumentTitle } from '@/app/composables/useDocumentTitle';
|
||||
import { useToast } from '@n8n/composables/useToast';
|
||||
|
||||
import WorkflowReviewDetailTabs from '../components/WorkflowReviewDetailTabs.vue';
|
||||
import type { WorkflowReviewDetailTab } from '../components/WorkflowReviewDetailTabs.vue';
|
||||
import WorkflowReviewRequestsSidebar from '../components/WorkflowReviewRequestsSidebar.vue';
|
||||
import { REVIEW_INBOX_QUERY_PARAM, WORKFLOW_REVIEW_REQUESTS_VIEW } from '../constants';
|
||||
import { useReviewInboxStore } from '../reviewInbox.store';
|
||||
|
|
@ -85,7 +87,10 @@ watch(
|
|||
);
|
||||
|
||||
function onSelect(id: string) {
|
||||
void router.replace({ params: { reviewRequestId: id }, query: route.query });
|
||||
// Switching reviews lands on Activity tab. Deep links still win.
|
||||
const query = { ...route.query };
|
||||
if (id !== selectedReviewId.value) delete query[REVIEW_INBOX_QUERY_PARAM.tab];
|
||||
void router.replace({ params: { reviewRequestId: id }, query });
|
||||
}
|
||||
|
||||
function onClearSelection() {
|
||||
|
|
@ -99,6 +104,17 @@ function onActiveTabChange(tab: WorkflowReviewRequestState) {
|
|||
void router.replace({ query });
|
||||
}
|
||||
|
||||
const detailTab = computed<WorkflowReviewDetailTab>(() =>
|
||||
route.query[REVIEW_INBOX_QUERY_PARAM.tab] === 'changes' ? 'changes' : 'activity',
|
||||
);
|
||||
|
||||
function onDetailTabChange(tab: WorkflowReviewDetailTab) {
|
||||
const query = { ...route.query };
|
||||
if (tab === 'changes') query[REVIEW_INBOX_QUERY_PARAM.tab] = tab;
|
||||
else delete query[REVIEW_INBOX_QUERY_PARAM.tab];
|
||||
void router.replace({ query });
|
||||
}
|
||||
|
||||
async function onLoadMore() {
|
||||
try {
|
||||
await store.loadMore();
|
||||
|
|
@ -109,17 +125,6 @@ async function onLoadMore() {
|
|||
|
||||
const deciding = ref(false);
|
||||
|
||||
const viewerCanDecide = computed(() => detail.value?.viewerCanDecide ?? false);
|
||||
|
||||
const ineligibilityHint = computed(() => {
|
||||
if (!detail.value || detail.value.viewerCanDecide) return '';
|
||||
// Any reason other than 'author' gets the generic permission hint, so new
|
||||
// backend reasons degrade gracefully instead of breaking the UI.
|
||||
return detail.value.viewerDecisionIneligibilityReason === 'author'
|
||||
? i18n.baseText('workflowReviews.detail.decision.ineligible.author')
|
||||
: i18n.baseText('generic.missing.permissions');
|
||||
});
|
||||
|
||||
// backend activation messages are inconsistently punctuated
|
||||
function asSentence(message: string) {
|
||||
const trimmed = message.trim();
|
||||
|
|
@ -200,7 +205,7 @@ onUnmounted(() => {
|
|||
</script>
|
||||
|
||||
<template>
|
||||
<PageViewLayout data-test-id="workflow-review-requests-view">
|
||||
<PageViewLayout full-width data-test-id="workflow-review-requests-view">
|
||||
<div :class="$style.content">
|
||||
<WorkflowReviewRequestsSidebar
|
||||
v-if="showSidebar"
|
||||
|
|
@ -257,43 +262,14 @@ onUnmounted(() => {
|
|||
<!-- Must precede the selectedItem branch: on a deep link the review is not
|
||||
in the list yet, so selectedItem is null while the detail loads. -->
|
||||
<N8nLoading v-else-if="selectedReviewId && detailLoading" :loading="true" :rows="3" />
|
||||
<div v-else-if="selectedItem">
|
||||
<N8nText
|
||||
color="text-light"
|
||||
size="medium"
|
||||
data-test-id="workflow-review-request-detail-stub"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.detail.placeholder') }}
|
||||
</N8nText>
|
||||
<div v-if="detail && detail.state === 'open'" :class="$style.decisionActions">
|
||||
<N8nTooltip
|
||||
:disabled="!ineligibilityHint"
|
||||
:content="ineligibilityHint"
|
||||
:show-after="300"
|
||||
>
|
||||
<N8nButton
|
||||
:disabled="deciding || !viewerCanDecide"
|
||||
data-test-id="workflow-review-approve-button"
|
||||
@click="onDecide(detail.id, 'approved')"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.detail.decision.approve') }}
|
||||
</N8nButton>
|
||||
</N8nTooltip>
|
||||
<N8nTooltip
|
||||
:disabled="!ineligibilityHint"
|
||||
:content="ineligibilityHint"
|
||||
:show-after="300"
|
||||
>
|
||||
<N8nButton
|
||||
:disabled="deciding || !viewerCanDecide"
|
||||
data-test-id="workflow-review-request-changes-button"
|
||||
@click="onDecide(detail.id, 'changes_requested')"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.detail.decision.requestChanges') }}
|
||||
</N8nButton>
|
||||
</N8nTooltip>
|
||||
</div>
|
||||
</div>
|
||||
<WorkflowReviewDetailTabs
|
||||
v-else-if="selectedItem"
|
||||
:review="selectedItem"
|
||||
:tab="detailTab"
|
||||
:deciding="deciding"
|
||||
@update:tab="onDetailTabChange"
|
||||
@decide="onDecide(selectedItem.id, $event)"
|
||||
/>
|
||||
<N8nText
|
||||
v-else-if="!showSidebar"
|
||||
color="text-light"
|
||||
|
|
@ -326,6 +302,12 @@ onUnmounted(() => {
|
|||
|
||||
<style lang="scss" module>
|
||||
.content {
|
||||
--review-tab-bar--height: var(--height--sm);
|
||||
--review-tab-bar--indicator-overhang: 11px;
|
||||
--review-tab-bar--gap: calc(var(--spacing--sm) + var(--review-tab-bar--indicator-overhang));
|
||||
|
||||
--review-callout--max-width: 34rem;
|
||||
|
||||
display: flex;
|
||||
width: 100%;
|
||||
min-height: 0;
|
||||
|
|
@ -355,10 +337,4 @@ onUnmounted(() => {
|
|||
min-height: 0;
|
||||
overflow: auto;
|
||||
}
|
||||
|
||||
.decisionActions {
|
||||
display: flex;
|
||||
gap: var(--spacing--2xs);
|
||||
margin-top: var(--spacing--sm);
|
||||
}
|
||||
</style>
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user