fix(validation): scope errors to active subflows
This commit is contained in:
parent
4d92cbc51c
commit
b6474def51
|
|
@ -229,6 +229,14 @@ export type FlowValidationError = {
|
|||
relatedNodeIds?: string[];
|
||||
};
|
||||
|
||||
export type GroupedFlowValidation = {
|
||||
flowLevel: FlowValidationError[];
|
||||
byContainer: Record<string, {
|
||||
body: FlowValidationError[];
|
||||
guard: FlowValidationError[];
|
||||
}>;
|
||||
};
|
||||
|
||||
export const FLOW_DEPENDANT_PORT_KEY = '__dependant';
|
||||
export const FLOW_DEPENDENCY_PORT_KEY = '__dependency';
|
||||
export const FLOW_DEPENDENCY_SOCKET_TYPE = '__FLOW_DEPENDENCY__';
|
||||
|
|
@ -248,6 +256,45 @@ export function normalizeFlowValidationErrors(raw: unknown): FlowValidationError
|
|||
}));
|
||||
}
|
||||
|
||||
export function normalizeGroupedFlowValidation(raw: unknown): GroupedFlowValidation {
|
||||
const value = raw && typeof raw === 'object' && !Array.isArray(raw)
|
||||
? raw as Record<string, unknown>
|
||||
: {};
|
||||
const byContainerRaw = value['byContainer'];
|
||||
const byContainer = byContainerRaw && typeof byContainerRaw === 'object' && !Array.isArray(byContainerRaw)
|
||||
? Object.fromEntries(
|
||||
Object.entries(byContainerRaw as Record<string, unknown>).map(([containerId, grouped]) => {
|
||||
const group = grouped && typeof grouped === 'object' && !Array.isArray(grouped)
|
||||
? grouped as Record<string, unknown>
|
||||
: {};
|
||||
return [containerId, {
|
||||
body: normalizeFlowValidationErrors(group['body']),
|
||||
guard: normalizeFlowValidationErrors(group['guard'])
|
||||
}];
|
||||
})
|
||||
)
|
||||
: {};
|
||||
|
||||
return {
|
||||
flowLevel: normalizeFlowValidationErrors(value['flowLevel']),
|
||||
byContainer
|
||||
};
|
||||
}
|
||||
|
||||
export function groupedFlowValidationFromErrors(errors: unknown): GroupedFlowValidation {
|
||||
return {
|
||||
flowLevel: normalizeFlowValidationErrors(errors),
|
||||
byContainer: {}
|
||||
};
|
||||
}
|
||||
|
||||
export function flattenGroupedFlowValidation(validation: GroupedFlowValidation): FlowValidationError[] {
|
||||
return [
|
||||
...validation.flowLevel,
|
||||
...Object.values(validation.byContainer).flatMap((group) => [...group.body, ...group.guard])
|
||||
];
|
||||
}
|
||||
|
||||
function normalizeRelatedNodeIds(item: Record<string, unknown>): string[] {
|
||||
const explicitNodeIds = Array.isArray(item['relatedNodeIds'])
|
||||
? item['relatedNodeIds'].map((value) => String(value)).filter((value) => value.length > 0)
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Flow, FlowValidationError } from "@models/flow";
|
||||
import { Flow, FlowValidationError, GroupedFlowValidation } from "@models/flow";
|
||||
import { Observable } from "rxjs";
|
||||
|
||||
export abstract class FlowsCallServiceBase {
|
||||
|
|
@ -21,4 +21,6 @@ export abstract class FlowsCallServiceBase {
|
|||
|
||||
abstract getFlowValidation(flowId: string) : Observable<FlowValidationError[]>;
|
||||
|
||||
abstract getGroupedFlowValidation(flowId: string) : Observable<GroupedFlowValidation>;
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { Flow, FlowValidationError } from "@models/flow";
|
||||
import { Flow, FlowValidationError, GroupedFlowValidation, groupedFlowValidationFromErrors } from "@models/flow";
|
||||
import { FlowsCallServiceBase } from "./flows-call.base";
|
||||
import { defer, Observable, of } from "rxjs";
|
||||
import { Authorization } from "@services/authorization/authorization";
|
||||
|
|
@ -122,6 +122,10 @@ export class FlowsCallServiceFake extends FlowsCallServiceBase {
|
|||
override getFlowValidation(flowId: string): Observable<FlowValidationError[]> {
|
||||
return defer(() => of(this.requireFlow(flowId).validationErrors ?? []));
|
||||
}
|
||||
|
||||
override getGroupedFlowValidation(flowId: string): Observable<GroupedFlowValidation> {
|
||||
return defer(() => of(groupedFlowValidationFromErrors(this.requireFlow(flowId).validationErrors)));
|
||||
}
|
||||
}
|
||||
const testDataFlow ={
|
||||
"id": "testFlow",
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
import { HttpClient } from '@angular/common/http';
|
||||
import { inject } from '@angular/core';
|
||||
import { environment } from '@environment';
|
||||
import { Flow, normalizeFlowValidationErrors } from '@models/flow';
|
||||
import { Flow, normalizeFlowValidationErrors, normalizeGroupedFlowValidation } from '@models/flow';
|
||||
import { map, Observable } from 'rxjs';
|
||||
import { flowFromApi, toFlowCreateRequest } from './flow-mapper';
|
||||
import { FlowsCallServiceBase } from './flows-call.base';
|
||||
|
|
@ -81,4 +81,11 @@ export class FlowsCallService extends FlowsCallServiceBase {
|
|||
.get<unknown>(`${environment.apiUrl}/flows/${encodedId}/validation`)
|
||||
.pipe(map((raw) => normalizeFlowValidationErrors((raw as any)?.errors ?? raw)));
|
||||
}
|
||||
|
||||
override getGroupedFlowValidation(flowId: string) {
|
||||
const encodedId = encodeURIComponent(flowId);
|
||||
return this.http
|
||||
.get<unknown>(`${environment.apiUrl}/flows/${encodedId}/validation/grouped`)
|
||||
.pipe(map((raw) => normalizeGroupedFlowValidation(raw)));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -29,6 +29,7 @@ describe('FlowsService', () => {
|
|||
updatePublished: ReturnType<typeof vi.fn>;
|
||||
finalizeFlow: ReturnType<typeof vi.fn>;
|
||||
getFlowValidation: ReturnType<typeof vi.fn>;
|
||||
getGroupedFlowValidation: ReturnType<typeof vi.fn>;
|
||||
getFlowById: ReturnType<typeof vi.fn>;
|
||||
createNewFlow: ReturnType<typeof vi.fn>;
|
||||
};
|
||||
|
|
@ -42,6 +43,7 @@ describe('FlowsService', () => {
|
|||
updatePublished: vi.fn(),
|
||||
finalizeFlow: vi.fn(),
|
||||
getFlowValidation: vi.fn(),
|
||||
getGroupedFlowValidation: vi.fn(),
|
||||
getFlowById: vi.fn(),
|
||||
createNewFlow: vi.fn()
|
||||
};
|
||||
|
|
|
|||
|
|
@ -160,6 +160,15 @@ export class FlowsService {
|
|||
);
|
||||
}
|
||||
|
||||
getGroupedFlowValidation(flowId: string) {
|
||||
return this.flowsCallService.getGroupedFlowValidation(flowId).pipe(
|
||||
catchError(err => {
|
||||
console.error('Retrieve grouped flow validation failed', err);
|
||||
return throwError(() => err);
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
cloneFlow(flow: Pick<Flow, 'name' | 'description' | 'data' | 'status'>): Observable<Flow> {
|
||||
return this.createFlow({
|
||||
name: `${flow.name} (cloned)`,
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
import { TestBed } from '@angular/core/testing';
|
||||
import { Authorization } from '@services/authorization/authorization';
|
||||
import { ConfirmDialogService } from '@services/dialogs/confirm-dialog';
|
||||
import { Flow, FlowData } from '@models/flow';
|
||||
import { FlowsService } from '@services/flows/flows';
|
||||
import { Flow, FlowData, GroupedFlowValidation } from '@models/flow';
|
||||
import { of } from 'rxjs';
|
||||
import { vi } from 'vitest';
|
||||
import { EditorStateHolder } from './flow-editor';
|
||||
|
|
@ -25,7 +26,7 @@ describe('EditorStateHolder', () => {
|
|||
let service: EditorStateHolder;
|
||||
let confirmSpy: { open: ReturnType<typeof vi.fn> };
|
||||
let authSpy: { loggedInUser: ReturnType<typeof vi.fn> };
|
||||
let flowsServiceSpy: { getFlowValidation: ReturnType<typeof vi.fn>; updateFlow: ReturnType<typeof vi.fn> };
|
||||
let flowsServiceSpy: { getGroupedFlowValidation: ReturnType<typeof vi.fn>; updateFlow: ReturnType<typeof vi.fn> };
|
||||
|
||||
beforeEach(() => {
|
||||
confirmSpy = {
|
||||
|
|
@ -35,7 +36,7 @@ describe('EditorStateHolder', () => {
|
|||
loggedInUser: vi.fn().mockReturnValue({ username: 'testuser', email: null, role: 'USER' })
|
||||
};
|
||||
flowsServiceSpy = {
|
||||
getFlowValidation: vi.fn(),
|
||||
getGroupedFlowValidation: vi.fn(),
|
||||
updateFlow: vi.fn()
|
||||
};
|
||||
|
||||
|
|
@ -43,12 +44,13 @@ describe('EditorStateHolder', () => {
|
|||
providers: [
|
||||
EditorStateHolder,
|
||||
{ provide: ConfirmDialogService, useValue: confirmSpy },
|
||||
{ provide: Authorization, useValue: authSpy }
|
||||
{ provide: Authorization, useValue: authSpy },
|
||||
{ provide: FlowsService, useValue: flowsServiceSpy }
|
||||
]
|
||||
});
|
||||
service = TestBed.inject(EditorStateHolder);
|
||||
service.flowsService = flowsServiceSpy as any;
|
||||
flowsServiceSpy.getFlowValidation.mockReturnValue(of([]));
|
||||
flowsServiceSpy.getGroupedFlowValidation.mockReturnValue(of({ flowLevel: [], byContainer: {} }));
|
||||
});
|
||||
|
||||
it('should be created', () => {
|
||||
|
|
@ -117,7 +119,7 @@ describe('EditorStateHolder', () => {
|
|||
|
||||
await service.openDocument(flow);
|
||||
|
||||
expect(flowsServiceSpy.getFlowValidation).not.toHaveBeenCalled();
|
||||
expect(flowsServiceSpy.getGroupedFlowValidation).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -233,6 +235,47 @@ describe('EditorStateHolder', () => {
|
|||
expect(service.structureNavigationRequest()).toBe(previousRequest + 1);
|
||||
});
|
||||
|
||||
it('shows only the active subflow validation errors', async () => {
|
||||
const validation: GroupedFlowValidation = {
|
||||
flowLevel: [{ message: 'Root flow error', code: 'ROOT' }],
|
||||
byContainer: {
|
||||
'container-1': {
|
||||
body: [{ message: 'Body error', code: 'BODY' }],
|
||||
guard: [{ message: 'Guard error', code: 'GUARD' }]
|
||||
}
|
||||
}
|
||||
};
|
||||
flowsServiceSpy.getGroupedFlowValidation.mockReturnValue(of(validation));
|
||||
const flow = makeFlow({
|
||||
data: {
|
||||
blocks: [],
|
||||
containers: [{
|
||||
id: 'container-1',
|
||||
name: 'Loop',
|
||||
typeName: 'LoopContainer',
|
||||
nodeFamily: 'container',
|
||||
inputs: [],
|
||||
outputs: [],
|
||||
specificConfiguration: {
|
||||
subFlow: { blocks: [], containers: [], connections: [], dependencies: [] },
|
||||
guardSubFlow: { blocks: [], containers: [], connections: [], dependencies: [] }
|
||||
}
|
||||
}],
|
||||
connections: [],
|
||||
dependencies: []
|
||||
}
|
||||
});
|
||||
|
||||
await service.openDocument(flow);
|
||||
expect(service.flowValidationErrors().map((error) => error.message)).toEqual(['Root flow error']);
|
||||
|
||||
expect(service.openSubflow([{ containerId: 'container-1', configurationPath: 'subFlow' }])).toBe(true);
|
||||
expect(service.flowValidationErrors().map((error) => error.message)).toEqual(['Body error']);
|
||||
|
||||
expect(service.openSubflow([{ containerId: 'container-1', configurationPath: 'guardSubFlow' }])).toBe(true);
|
||||
expect(service.flowValidationErrors().map((error) => error.message)).toEqual(['Guard error']);
|
||||
});
|
||||
|
||||
it('saves annotations in the full flow payload and adopts server-generated ids', async () => {
|
||||
const flow = makeFlow({
|
||||
data: {
|
||||
|
|
|
|||
|
|
@ -1,5 +1,13 @@
|
|||
import { computed, inject, Injectable, signal } from '@angular/core';
|
||||
import { Flow, FlowData, FlowValidationError, normalizeFlowValidationErrors } from '@models/flow';
|
||||
import {
|
||||
Flow,
|
||||
FlowData,
|
||||
FlowValidationError,
|
||||
GroupedFlowValidation,
|
||||
flattenGroupedFlowValidation,
|
||||
groupedFlowValidationFromErrors,
|
||||
normalizeFlowValidationErrors
|
||||
} from '@models/flow';
|
||||
import { Authorization } from '@services/authorization/authorization';
|
||||
import { ConfirmDialogService } from '@services/dialogs/confirm-dialog';
|
||||
import { FlowsService } from '@services/flows/flows';
|
||||
|
|
@ -23,7 +31,23 @@ export class EditorStateHolder {
|
|||
readonly isDirty = signal(false);
|
||||
readonly selectedBlockIds = signal<string[]>([]);
|
||||
readonly draggingSelectedBlockIds = signal<string[]>([]);
|
||||
readonly flowValidationErrors = signal<FlowValidationError[]>([]);
|
||||
private readonly groupedFlowValidation = signal<GroupedFlowValidation>(emptyGroupedFlowValidation());
|
||||
private readonly localFlowValidationErrors = signal<FlowValidationError[]>([]);
|
||||
readonly flowValidationErrors = computed(() => {
|
||||
const validation = this.groupedFlowValidation();
|
||||
const activeSubflow = this.activeSubflow();
|
||||
if (!activeSubflow) {
|
||||
return [...validation.flowLevel, ...this.localFlowValidationErrors()];
|
||||
}
|
||||
|
||||
const activeStep = activeSubflow.locator.at(-1);
|
||||
if (!activeStep) return [];
|
||||
const containerErrors = validation.byContainer[activeStep.containerId];
|
||||
if (!containerErrors) return [];
|
||||
return isGuardSubflow(activeStep.configurationPath)
|
||||
? containerErrors.guard
|
||||
: containerErrors.body;
|
||||
});
|
||||
readonly highlightedValidationNodeIds = signal<string[]>([]);
|
||||
readonly validationRequiresSave = signal(false);
|
||||
readonly activeSubflow = signal<FlowSubflowEntry | null>(null);
|
||||
|
|
@ -77,7 +101,7 @@ export class EditorStateHolder {
|
|||
this.activeSubflow.set(null);
|
||||
this.isDirty.set(false);
|
||||
this.validationRequiresSave.set(false);
|
||||
this.applyFlowValidationErrors(doc.validationErrors ?? [], doc);
|
||||
this.applyGroupedFlowValidation(groupedFlowValidationFromErrors(doc.validationErrors), doc);
|
||||
this.ensureValidationForFlow(doc);
|
||||
this.clearBlockSelection();
|
||||
return true;
|
||||
|
|
@ -99,7 +123,7 @@ export class EditorStateHolder {
|
|||
this.activeSubflow.set(null);
|
||||
this.isDirty.set(false);
|
||||
this.validationRequiresSave.set(false);
|
||||
this.applyFlowValidationErrors([], null);
|
||||
this.applyGroupedFlowValidation(emptyGroupedFlowValidation(), null);
|
||||
this.lastValidationFetchKey = null;
|
||||
this.clearBlockSelection();
|
||||
}
|
||||
|
|
@ -110,7 +134,7 @@ export class EditorStateHolder {
|
|||
this.activeSubflow.set(null);
|
||||
this.isDirty.set(options?.markDirty === true);
|
||||
this.validationRequiresSave.set(options?.markDirty === true);
|
||||
this.applyFlowValidationErrors(flow.validationErrors ?? [], flow);
|
||||
this.applyGroupedFlowValidation(groupedFlowValidationFromErrors(flow.validationErrors), flow);
|
||||
this.ensureValidationForFlow(flow);
|
||||
this.clearBlockSelection();
|
||||
}
|
||||
|
|
@ -132,7 +156,7 @@ export class EditorStateHolder {
|
|||
const nextFlow = { ...current, data: rootData };
|
||||
this.currentFlow.set(nextFlow);
|
||||
this.markDirty();
|
||||
this.applyFlowValidationErrors(current.validationErrors ?? [], nextFlow);
|
||||
this.refreshValidationPresentation(nextFlow);
|
||||
if (options?.structural !== false) {
|
||||
this.validationRequiresSave.set(true);
|
||||
}
|
||||
|
|
@ -151,12 +175,15 @@ export class EditorStateHolder {
|
|||
? replaceFlowSubflow(current.data, active.locator, data)
|
||||
: data;
|
||||
if (!rootData) return;
|
||||
this.currentFlow.set({ ...current, data: rootData });
|
||||
const nextFlow = { ...current, data: rootData };
|
||||
this.currentFlow.set(nextFlow);
|
||||
this.refreshValidationPresentation(nextFlow);
|
||||
}
|
||||
|
||||
openRootFlow() {
|
||||
if (!this.currentFlow() || !this.activeSubflow()) return;
|
||||
this.activeSubflow.set(null);
|
||||
this.setHighlightedValidationNodes([]);
|
||||
this.clearBlockSelection();
|
||||
}
|
||||
|
||||
|
|
@ -167,6 +194,7 @@ export class EditorStateHolder {
|
|||
const entry = this.availableSubflows().find((candidate) => candidate.key === key);
|
||||
if (!entry) return false;
|
||||
this.activeSubflow.set(entry);
|
||||
this.setHighlightedValidationNodes([]);
|
||||
this.clearBlockSelection();
|
||||
return true;
|
||||
}
|
||||
|
|
@ -239,11 +267,12 @@ export class EditorStateHolder {
|
|||
return save$.pipe(
|
||||
switchMap((savedFlow) => {
|
||||
const validation$ = savedFlow.status !== 'EXECUTABLE'
|
||||
? this.flowsService.getFlowValidation(savedFlow.id)
|
||||
: of([]);
|
||||
? this.flowsService.getGroupedFlowValidation(savedFlow.id)
|
||||
: of(emptyGroupedFlowValidation());
|
||||
|
||||
return validation$.pipe(
|
||||
tap((validationErrors) => {
|
||||
tap((validation) => {
|
||||
const validationErrors = flattenGroupedFlowValidation(validation);
|
||||
const nextFlow = {
|
||||
...savedFlow,
|
||||
validationErrors
|
||||
|
|
@ -254,18 +283,18 @@ export class EditorStateHolder {
|
|||
this.activeSubflow.set(null);
|
||||
}
|
||||
this.lastValidationFetchKey = this.validationFetchKey(nextFlow);
|
||||
this.applyFlowValidationErrors(validationErrors, nextFlow);
|
||||
this.applyGroupedFlowValidation(validation, nextFlow);
|
||||
this.markSaved();
|
||||
this.validationRequiresSave.set(false);
|
||||
}),
|
||||
switchMap(() => of({
|
||||
...savedFlow,
|
||||
validationErrors: this.flowValidationErrors()
|
||||
validationErrors: this.currentFlow()?.validationErrors ?? []
|
||||
}))
|
||||
);
|
||||
}),
|
||||
catchError((error) => {
|
||||
this.applyFlowValidationErrors(this.extractValidationErrors(error), this.currentFlow());
|
||||
this.applyGroupedFlowValidation(groupedFlowValidationFromErrors(this.extractValidationErrors(error)), this.currentFlow());
|
||||
return throwError(() => error);
|
||||
})
|
||||
)
|
||||
|
|
@ -285,13 +314,16 @@ export class EditorStateHolder {
|
|||
return JSON.stringify(left) === JSON.stringify(right);
|
||||
}
|
||||
|
||||
private applyFlowValidationErrors(errors: FlowValidationError[], flow?: Flow | null) {
|
||||
const normalized = Array.isArray(errors) ? errors : [];
|
||||
const derived = flow ? this.deriveGlobalInputReferenceErrors(flow) : [];
|
||||
const merged = [...normalized, ...derived];
|
||||
this.flowValidationErrors.set(merged);
|
||||
private applyGroupedFlowValidation(validation: GroupedFlowValidation, flow?: Flow | null) {
|
||||
this.groupedFlowValidation.set(validation);
|
||||
this.refreshValidationPresentation(flow);
|
||||
}
|
||||
|
||||
private refreshValidationPresentation(flow?: Flow | null) {
|
||||
this.localFlowValidationErrors.set(flow ? this.deriveGlobalInputReferenceErrors(flow) : []);
|
||||
const errors = this.flowValidationErrors();
|
||||
this.highlightedValidationNodeIds.set(Array.from(new Set(
|
||||
merged.flatMap((error) => Array.isArray(error.relatedNodeIds) ? error.relatedNodeIds : [])
|
||||
errors.flatMap((error) => Array.isArray(error.relatedNodeIds) ? error.relatedNodeIds : [])
|
||||
)));
|
||||
}
|
||||
|
||||
|
|
@ -309,13 +341,13 @@ export class EditorStateHolder {
|
|||
if (!flow) return;
|
||||
if (flow.id.startsWith(EditorStateHolder.ASSISTANT_DRAFT_PREFIX)) {
|
||||
this.lastValidationFetchKey = this.validationFetchKey(flow);
|
||||
this.applyFlowValidationErrors(flow.validationErrors ?? [], flow);
|
||||
this.applyGroupedFlowValidation(groupedFlowValidationFromErrors(flow.validationErrors), flow);
|
||||
return;
|
||||
}
|
||||
|
||||
if (flow.status === 'EXECUTABLE') {
|
||||
this.lastValidationFetchKey = this.validationFetchKey(flow);
|
||||
this.applyFlowValidationErrors(flow.validationErrors ?? [], flow);
|
||||
this.applyGroupedFlowValidation(groupedFlowValidationFromErrors(flow.validationErrors), flow);
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -323,15 +355,16 @@ export class EditorStateHolder {
|
|||
if (this.lastValidationFetchKey === fetchKey) return;
|
||||
this.lastValidationFetchKey = fetchKey;
|
||||
|
||||
this.flowsService.getFlowValidation(flow.id).pipe(take(1)).subscribe({
|
||||
next: (validationErrors) => {
|
||||
this.flowsService.getGroupedFlowValidation(flow.id).pipe(take(1)).subscribe({
|
||||
next: (validation) => {
|
||||
const current = this.currentFlow();
|
||||
if (!current || current.id !== flow.id) return;
|
||||
const validationErrors = flattenGroupedFlowValidation(validation);
|
||||
this.currentFlow.set({
|
||||
...current,
|
||||
validationErrors
|
||||
});
|
||||
this.applyFlowValidationErrors(validationErrors, {
|
||||
this.applyGroupedFlowValidation(validation, {
|
||||
...current,
|
||||
validationErrors
|
||||
});
|
||||
|
|
@ -417,3 +450,11 @@ function extractReferencedGlobalNames(content: string): string[] {
|
|||
|
||||
return Array.from(names);
|
||||
}
|
||||
|
||||
function emptyGroupedFlowValidation(): GroupedFlowValidation {
|
||||
return { flowLevel: [], byContainer: {} };
|
||||
}
|
||||
|
||||
function isGuardSubflow(configurationPath: string): boolean {
|
||||
return configurationPath.split('.').at(-1)?.toLowerCase().includes('guard') ?? false;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue