diff --git a/src/app/models/flow.ts b/src/app/models/flow.ts index 2e2e7ab..ebc64fa 100644 --- a/src/app/models/flow.ts +++ b/src/app/models/flow.ts @@ -229,6 +229,14 @@ export type FlowValidationError = { relatedNodeIds?: string[]; }; +export type GroupedFlowValidation = { + flowLevel: FlowValidationError[]; + byContainer: Record; +}; + 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 + : {}; + const byContainerRaw = value['byContainer']; + const byContainer = byContainerRaw && typeof byContainerRaw === 'object' && !Array.isArray(byContainerRaw) + ? Object.fromEntries( + Object.entries(byContainerRaw as Record).map(([containerId, grouped]) => { + const group = grouped && typeof grouped === 'object' && !Array.isArray(grouped) + ? grouped as Record + : {}; + 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[] { const explicitNodeIds = Array.isArray(item['relatedNodeIds']) ? item['relatedNodeIds'].map((value) => String(value)).filter((value) => value.length > 0) diff --git a/src/app/services/flows/flows-call.base.ts b/src/app/services/flows/flows-call.base.ts index f2f1b2b..384f2f2 100644 --- a/src/app/services/flows/flows-call.base.ts +++ b/src/app/services/flows/flows-call.base.ts @@ -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; + abstract getGroupedFlowValidation(flowId: string) : Observable; + } diff --git a/src/app/services/flows/flows-call.fake.ts b/src/app/services/flows/flows-call.fake.ts index 885971d..847d7ad 100644 --- a/src/app/services/flows/flows-call.fake.ts +++ b/src/app/services/flows/flows-call.fake.ts @@ -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 { return defer(() => of(this.requireFlow(flowId).validationErrors ?? [])); } + + override getGroupedFlowValidation(flowId: string): Observable { + return defer(() => of(groupedFlowValidationFromErrors(this.requireFlow(flowId).validationErrors))); + } } const testDataFlow ={ "id": "testFlow", diff --git a/src/app/services/flows/flows-call.ts b/src/app/services/flows/flows-call.ts index 4cf8339..f097399 100644 --- a/src/app/services/flows/flows-call.ts +++ b/src/app/services/flows/flows-call.ts @@ -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(`${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(`${environment.apiUrl}/flows/${encodedId}/validation/grouped`) + .pipe(map((raw) => normalizeGroupedFlowValidation(raw))); + } } diff --git a/src/app/services/flows/flows.spec.ts b/src/app/services/flows/flows.spec.ts index d3816c4..0f0e8ce 100644 --- a/src/app/services/flows/flows.spec.ts +++ b/src/app/services/flows/flows.spec.ts @@ -29,6 +29,7 @@ describe('FlowsService', () => { updatePublished: ReturnType; finalizeFlow: ReturnType; getFlowValidation: ReturnType; + getGroupedFlowValidation: ReturnType; getFlowById: ReturnType; createNewFlow: ReturnType; }; @@ -42,6 +43,7 @@ describe('FlowsService', () => { updatePublished: vi.fn(), finalizeFlow: vi.fn(), getFlowValidation: vi.fn(), + getGroupedFlowValidation: vi.fn(), getFlowById: vi.fn(), createNewFlow: vi.fn() }; diff --git a/src/app/services/flows/flows.ts b/src/app/services/flows/flows.ts index a93274e..6ac2b96 100644 --- a/src/app/services/flows/flows.ts +++ b/src/app/services/flows/flows.ts @@ -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): Observable { return this.createFlow({ name: `${flow.name} (cloned)`, diff --git a/src/app/stores/flow-editor.spec.ts b/src/app/stores/flow-editor.spec.ts index 773f0e7..c6bd67f 100644 --- a/src/app/stores/flow-editor.spec.ts +++ b/src/app/stores/flow-editor.spec.ts @@ -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 }; let authSpy: { loggedInUser: ReturnType }; - let flowsServiceSpy: { getFlowValidation: ReturnType; updateFlow: ReturnType }; + let flowsServiceSpy: { getGroupedFlowValidation: ReturnType; updateFlow: ReturnType }; 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: { diff --git a/src/app/stores/flow-editor.ts b/src/app/stores/flow-editor.ts index 16666aa..568aa59 100644 --- a/src/app/stores/flow-editor.ts +++ b/src/app/stores/flow-editor.ts @@ -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([]); readonly draggingSelectedBlockIds = signal([]); - readonly flowValidationErrors = signal([]); + private readonly groupedFlowValidation = signal(emptyGroupedFlowValidation()); + private readonly localFlowValidationErrors = signal([]); + 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([]); readonly validationRequiresSave = signal(false); readonly activeSubflow = signal(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; +}