mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-17 13:39:44 +02:00
feat(editor): Show diff for closed review requests (#36539)
This commit is contained in:
parent
a2188869b5
commit
29779f0c5e
|
|
@ -5561,14 +5561,18 @@
|
|||
"workflowReviews.detail.metadata.reviewers": "Reviewers",
|
||||
"workflowReviews.detail.metadata.noReviewers": "No reviewers assigned.",
|
||||
"workflowReviews.detail.metadata.workflow": "Workflow",
|
||||
"workflowReviews.changes.closed.body": "Changes aren't shown for closed reviews.",
|
||||
"workflowReviews.changes.empty": "This review doesn't contain any workflows.",
|
||||
"workflowReviews.changes.workflowUnavailable": "The workflow in this review is no longer available.",
|
||||
"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.closedWithoutApproval": "This review was closed without approval, so there are no changes to compare.",
|
||||
"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.changes.closed.noChanges": "No changes compared to the previously published version.",
|
||||
"workflowReviews.changes.closed.sourceLabel": "Previously published: {version}",
|
||||
"workflowReviews.changes.closed.targetLabel": "Approved: {version}",
|
||||
"workflowReviews.changes.closed.firstPublish.sourceEmpty": "This workflow hadn't been published yet.",
|
||||
"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.trigger": "Review",
|
||||
|
|
|
|||
|
|
@ -1,4 +1,6 @@
|
|||
import type {
|
||||
WorkflowReviewRequestDecision,
|
||||
WorkflowReviewRequestState,
|
||||
WorkflowReviewRequestWorkflowDetail,
|
||||
WorkflowReviewVersionSnapshot,
|
||||
} from '@n8n/api-types';
|
||||
|
|
@ -33,7 +35,16 @@ vi.mock('@/features/workflows/workflowDiff/WorkflowDiffView.vue', () => ({
|
|||
},
|
||||
}));
|
||||
|
||||
const renderComponent = createComponentRenderer(WorkflowReviewChangesSection);
|
||||
const renderSection = createComponentRenderer(WorkflowReviewChangesSection);
|
||||
|
||||
/** Defaults to an open review; closed cases pass `state` and `decision` themselves. */
|
||||
const renderComponent = (options: {
|
||||
props: {
|
||||
workflow: WorkflowReviewRequestWorkflowDetail;
|
||||
state?: WorkflowReviewRequestState;
|
||||
decision?: WorkflowReviewRequestDecision;
|
||||
};
|
||||
}) => renderSection({ props: { state: 'open', decision: 'pending', ...options.props } });
|
||||
|
||||
function makeNode(overrides: Partial<INode> = {}): INode {
|
||||
return {
|
||||
|
|
@ -242,6 +253,75 @@ describe('WorkflowReviewChangesSection', () => {
|
|||
expect(isReactive(targetWorkflow?.nodes[0])).toBe(false);
|
||||
});
|
||||
|
||||
describe('closed reviews', () => {
|
||||
const approved = { state: 'closed', decision: 'approved' } as const;
|
||||
|
||||
it('labels the frozen baseline and the approved version', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
...approved,
|
||||
workflow: makeWorkflow({
|
||||
pinnedVersion: makeSnapshot({ name: 'Version hotpink' }),
|
||||
baselineVersion: makeSnapshot({
|
||||
versionId: '0f123890-0000-0000-0000-000000000000',
|
||||
name: 'Version teal',
|
||||
nodes: [makeNode({ id: 'node-removed', name: 'Removed node' }), makeNode()],
|
||||
}),
|
||||
}),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-source-label')).toHaveTextContent(
|
||||
'Previously published: Version teal',
|
||||
);
|
||||
expect(getByTestId('workflow-review-changes-target-label')).toHaveTextContent(
|
||||
'Approved: Version hotpink',
|
||||
);
|
||||
});
|
||||
|
||||
// Nothing was published when this was approved, so the baseline is empty by
|
||||
// right — not missing. The diff still renders, sourceless.
|
||||
it('renders a sourceless diff when nothing was published at approval', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: { ...approved, workflow: makeWorkflow({ baselineVersion: null }) },
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-diff-view-stub')).toBeInTheDocument();
|
||||
expect(diffViewProps[0].sourceWorkflow).toBeUndefined();
|
||||
expect(
|
||||
queryByTestId('workflow-review-changes-closed-without-approval'),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('phrases the no-changes state in the past tense', () => {
|
||||
const snapshot = makeSnapshot();
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
...approved,
|
||||
workflow: makeWorkflow({ pinnedVersion: snapshot, baselineVersion: snapshot }),
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-no-changes')).toHaveTextContent(
|
||||
'No changes compared to the previously published version.',
|
||||
);
|
||||
});
|
||||
|
||||
// Auto-closed by archiving, transferring or deleting the workflow: approval
|
||||
// is the only thing that freezes a baseline, so there is nothing to diff.
|
||||
it.each(['pending', 'changes_requested'] as const)(
|
||||
'explains that a review closed with decision %s has no changes to compare',
|
||||
(decision) => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: { state: 'closed', decision, workflow: makeWorkflow() },
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-closed-without-approval')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-diff-view-stub')).not.toBeInTheDocument();
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
it('renders the diff when versions differ only by an execution flag', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
props: {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,7 @@
|
|||
<script setup lang="ts">
|
||||
import type {
|
||||
WorkflowReviewRequestDecision,
|
||||
WorkflowReviewRequestState,
|
||||
WorkflowReviewRequestWorkflowDetail,
|
||||
WorkflowReviewVersionSnapshot,
|
||||
} from '@n8n/api-types';
|
||||
|
|
@ -15,10 +17,19 @@ import type { IWorkflowDb } from '@/Interface';
|
|||
|
||||
const props = defineProps<{
|
||||
workflow: WorkflowReviewRequestWorkflowDetail;
|
||||
state: WorkflowReviewRequestState;
|
||||
decision: WorkflowReviewRequestDecision;
|
||||
}>();
|
||||
|
||||
const i18n = useI18n();
|
||||
|
||||
// Only approval freezes a baseline, so an approved review diffs against what was
|
||||
// published back then; a review closed any other way has nothing to diff against.
|
||||
const isApproved = computed(() => props.state === 'closed' && props.decision === 'approved');
|
||||
const isClosedWithoutApproval = computed(
|
||||
() => props.state === 'closed' && props.decision !== 'approved',
|
||||
);
|
||||
|
||||
/**
|
||||
* Label each side the way version history does, falling back to a short id.
|
||||
* Falsy rather than nullish: the publish endpoints accept `name: ""`.
|
||||
|
|
@ -72,25 +83,63 @@ const targetWorkflow = computed(() =>
|
|||
props.workflow.pinnedVersion ? snapshotToWorkflow(props.workflow.pinnedVersion) : undefined,
|
||||
);
|
||||
|
||||
// The dots track publish status, which approval flips: the approved version is
|
||||
// the published one now, and the baseline it replaced no longer is.
|
||||
const sourceDotClass = computed(() =>
|
||||
isApproved.value ? 'statusDotSuperseded' : 'statusDotPublished',
|
||||
);
|
||||
const targetDotClass = computed(() =>
|
||||
isApproved.value ? 'statusDotPublished' : 'statusDotInReview',
|
||||
);
|
||||
|
||||
const noChangesText = computed(() =>
|
||||
i18n.baseText(
|
||||
isApproved.value
|
||||
? 'workflowReviews.changes.closed.noChanges'
|
||||
: 'workflowReviews.changes.noChanges',
|
||||
),
|
||||
);
|
||||
const sourceEmptyText = computed(() =>
|
||||
i18n.baseText(
|
||||
isApproved.value
|
||||
? 'workflowReviews.changes.closed.firstPublish.sourceEmpty'
|
||||
: 'workflowReviews.changes.firstPublish.sourceEmpty',
|
||||
),
|
||||
);
|
||||
|
||||
const sourceLabel = computed(() =>
|
||||
props.workflow.baselineVersion
|
||||
? i18n.baseText('workflowReviews.changes.sourceLabel', {
|
||||
interpolate: { version: versionLabel(props.workflow.baselineVersion) },
|
||||
})
|
||||
? i18n.baseText(
|
||||
isApproved.value
|
||||
? 'workflowReviews.changes.closed.sourceLabel'
|
||||
: 'workflowReviews.changes.sourceLabel',
|
||||
{ interpolate: { version: versionLabel(props.workflow.baselineVersion) } },
|
||||
)
|
||||
: undefined,
|
||||
);
|
||||
const targetLabel = computed(() =>
|
||||
props.workflow.pinnedVersion
|
||||
? i18n.baseText('workflowReviews.changes.targetLabel', {
|
||||
interpolate: { version: versionLabel(props.workflow.pinnedVersion) },
|
||||
})
|
||||
? i18n.baseText(
|
||||
isApproved.value
|
||||
? 'workflowReviews.changes.closed.targetLabel'
|
||||
: 'workflowReviews.changes.targetLabel',
|
||||
{ interpolate: { version: versionLabel(props.workflow.pinnedVersion) } },
|
||||
)
|
||||
: undefined,
|
||||
);
|
||||
</script>
|
||||
|
||||
<template>
|
||||
<N8nCallout
|
||||
v-if="!workflow.pinnedVersion"
|
||||
v-if="isClosedWithoutApproval"
|
||||
theme="info"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-closed-without-approval"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.closedWithoutApproval') }}
|
||||
</N8nCallout>
|
||||
<N8nCallout
|
||||
v-else-if="!workflow.pinnedVersion"
|
||||
theme="warning"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-version-unavailable"
|
||||
|
|
@ -103,7 +152,7 @@ const targetLabel = computed(() =>
|
|||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-no-changes"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.noChanges') }}
|
||||
{{ noChangesText }}
|
||||
</N8nCallout>
|
||||
<div v-else :class="$style.diff" data-test-id="workflow-review-changes-diff">
|
||||
<WorkflowDiffView
|
||||
|
|
@ -116,18 +165,18 @@ const targetLabel = computed(() =>
|
|||
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]" />
|
||||
<span :class="[$style.statusDot, $style[sourceDotClass]]" />
|
||||
<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]" />
|
||||
<span :class="[$style.statusDot, $style[targetDotClass]]" />
|
||||
<N8nText color="text-dark" size="small" compact>{{ targetLabel }}</N8nText>
|
||||
</span>
|
||||
</template>
|
||||
<template #sourceEmptyText>
|
||||
{{ i18n.baseText('workflowReviews.changes.firstPublish.sourceEmpty') }}
|
||||
{{ sourceEmptyText }}
|
||||
</template>
|
||||
</WorkflowDiffView>
|
||||
</div>
|
||||
|
|
@ -136,6 +185,7 @@ const targetLabel = computed(() =>
|
|||
<style module lang="scss">
|
||||
.callout {
|
||||
max-width: var(--review-callout--max-width, 34rem);
|
||||
margin-top: var(--spacing--5xs);
|
||||
}
|
||||
|
||||
.versionBadge {
|
||||
|
|
@ -158,6 +208,10 @@ const targetLabel = computed(() =>
|
|||
background-color: var(--color--yellow-500);
|
||||
}
|
||||
|
||||
.statusDotSuperseded {
|
||||
background-color: var(--color--text--tint-1);
|
||||
}
|
||||
|
||||
.diff {
|
||||
height: 100%;
|
||||
min-height: 0;
|
||||
|
|
|
|||
|
|
@ -11,8 +11,9 @@ import WorkflowReviewDetailTabs from './WorkflowReviewDetailTabs.vue';
|
|||
vi.mock('./WorkflowReviewChangesSection.vue', () => ({
|
||||
default: {
|
||||
name: 'WorkflowReviewChangesSection',
|
||||
props: ['workflow'],
|
||||
template: '<div data-test-id="workflow-review-changes-section" />',
|
||||
props: ['workflow', 'state', 'decision'],
|
||||
template:
|
||||
'<div data-test-id="workflow-review-changes-section" :data-state="state" :data-decision="decision" />',
|
||||
},
|
||||
}));
|
||||
|
||||
|
|
@ -219,8 +220,10 @@ describe('WorkflowReviewDetailTabs', () => {
|
|||
expect(getAllByTestId('workflow-review-changes-section')).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('shows an info state instead of diffs for a closed review', () => {
|
||||
const { getByTestId, queryByTestId } = renderComponent({
|
||||
// A closed review keeps its diff: the backend serves the baseline frozen at
|
||||
// approval, and the section needs the lifecycle to phrase the sides.
|
||||
it('renders the diff for a closed review and forwards its lifecycle', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({ state: 'closed', decision: 'approved' }),
|
||||
tab: 'changes',
|
||||
|
|
@ -228,20 +231,21 @@ describe('WorkflowReviewDetailTabs', () => {
|
|||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-closed')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-section')).not.toBeInTheDocument();
|
||||
const section = getByTestId('workflow-review-changes-section');
|
||||
expect(section).toHaveAttribute('data-state', 'closed');
|
||||
expect(section).toHaveAttribute('data-decision', 'approved');
|
||||
});
|
||||
|
||||
it('shows an empty state when the detail has no workflows', () => {
|
||||
it('shows an unavailable state when the detail has no workflows left', () => {
|
||||
const { getByTestId } = renderComponent({
|
||||
props: {
|
||||
review: makeDetail({ workflows: [] }),
|
||||
review: makeDetail({ state: 'closed', workflows: [] }),
|
||||
tab: 'changes',
|
||||
deciding: false,
|
||||
},
|
||||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-empty')).toBeInTheDocument();
|
||||
expect(getByTestId('workflow-review-changes-workflow-unavailable')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('shows an error state when the detail fetch failed', () => {
|
||||
|
|
@ -250,7 +254,7 @@ describe('WorkflowReviewDetailTabs', () => {
|
|||
});
|
||||
|
||||
expect(getByTestId('workflow-review-changes-unavailable')).toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-empty')).not.toBeInTheDocument();
|
||||
expect(queryByTestId('workflow-review-changes-workflow-unavailable')).not.toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -120,15 +120,7 @@ const tabOptions = computed(() => [
|
|||
|
||||
<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"
|
||||
v-if="!detail"
|
||||
theme="warning"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-unavailable"
|
||||
|
|
@ -140,16 +132,19 @@ const tabOptions = computed(() => [
|
|||
v-for="workflow in detail.workflows"
|
||||
:key="workflow.workflowId"
|
||||
:workflow="workflow"
|
||||
:state="detail.state"
|
||||
:decision="detail.decision"
|
||||
/>
|
||||
</template>
|
||||
<N8nText
|
||||
<!-- No rows left: the workflow was deleted, or the requester lost access to it. -->
|
||||
<N8nCallout
|
||||
v-else
|
||||
color="text-light"
|
||||
size="medium"
|
||||
data-test-id="workflow-review-changes-empty"
|
||||
theme="warning"
|
||||
:class="$style.callout"
|
||||
data-test-id="workflow-review-changes-workflow-unavailable"
|
||||
>
|
||||
{{ i18n.baseText('workflowReviews.changes.empty') }}
|
||||
</N8nText>
|
||||
{{ i18n.baseText('workflowReviews.changes.workflowUnavailable') }}
|
||||
</N8nCallout>
|
||||
</div>
|
||||
|
||||
<WorkflowReviewDetailMetadata :review="review" />
|
||||
|
|
@ -172,6 +167,10 @@ const tabOptions = computed(() => [
|
|||
align-items: center;
|
||||
justify-content: space-between;
|
||||
gap: var(--spacing--sm);
|
||||
|
||||
> :global(.n8n-tabs) {
|
||||
transform: translateY(var(--spacing--xs));
|
||||
}
|
||||
}
|
||||
|
||||
.detailBody {
|
||||
|
|
|
|||
|
|
@ -30,7 +30,7 @@ vi.mock('@/app/composables/useDocumentTitle', () => ({
|
|||
vi.mock('@/features/workflow-reviews/components/WorkflowReviewChangesSection.vue', () => ({
|
||||
default: {
|
||||
name: 'WorkflowReviewChangesSection',
|
||||
props: ['workflow'],
|
||||
props: ['workflow', 'state', 'decision'],
|
||||
template: '<div data-test-id="workflow-review-changes-section" />',
|
||||
},
|
||||
}));
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user