fix(editor): Change the checkbox logic for log streaming event selection (#17653)

packages/frontend/editor-ui/src/components/SettingsLogStreaming/EventSelection.ee.vue

@@ -73,7 +73,7 @@ export default defineComponent({
 			<!-- <template #header> -->
 			<Checkbox
 				:model-value="group.selected"
-				:indeterminate="!group.selected && group.indeterminate"
+				:indeterminate="group.indeterminate"
 				:disabled="readonly"
 				@update:model-value="onInput"
 				@change="onCheckboxChecked(group.name, $event)"
@@ -108,15 +108,11 @@ export default defineComponent({
 			</Checkbox>
 			<!-- </template> -->
 			<ul :class="$style.eventList">
-				<li
-					v-for="event in group.children"
-					:key="event.name"
-					:class="`${$style.eventListItem} ${group.selected ? $style.eventListItemDisabled : ''}`"
-				>
+				<li v-for="event in group.children" :key="event.name" :class="`${$style.eventListItem}`">
 					<Checkbox
 						:model-value="event.selected || group.selected"
 						:indeterminate="event.indeterminate"
-						:disabled="group.selected || readonly"
+						:disabled="readonly"
 						@update:model-value="onInput"
 						@change="onCheckboxChecked(event.name, $event)"
 					>

packages/frontend/editor-ui/src/stores/logStreaming.store.test.ts

@@ -0,0 +1,101 @@
+import { setActivePinia } from 'pinia';
+import { useLogStreamingStore } from './logStreaming.store';
+import { createTestingPinia } from '@pinia/testing';
+
+describe('LogStreamingStore', () => {
+	let logStreamingStore: ReturnType<typeof useLogStreamingStore>;
+
+	beforeAll(() => {
+		setActivePinia(createTestingPinia({ stubActions: false }));
+		logStreamingStore = useLogStreamingStore();
+	});
+
+	describe('addEventName', () => {
+		it('should add a new event name', () => {
+			logStreamingStore.addEventName('n8n.node.started');
+			logStreamingStore.addEventName('n8n.node.success');
+			logStreamingStore.addEventName('n8n.node.failed');
+		});
+	});
+
+	describe('addDestination', () => {
+		it('should add a new destination', () => {
+			logStreamingStore.addDestination({
+				id: 'destinationId',
+				label: 'Test Destination',
+				enabled: true,
+				subscribedEvents: [],
+				anonymizeAuditMessages: false,
+			});
+			expect(logStreamingStore.items.destinationId).toBeDefined();
+			expect(logStreamingStore.items.destinationId.destination.label).toBe('Test Destination');
+			expect(logStreamingStore.items.destinationId.eventGroups).toHaveLength(2);
+			const nodeEventGroup = logStreamingStore.items.destinationId.eventGroups.find(
+				(group) => group.name === 'n8n.node',
+			);
+			expect(nodeEventGroup).toBeDefined();
+			expect(nodeEventGroup!.children).toHaveLength(3);
+			expect(nodeEventGroup!.children.every((c) => !c.selected)).toBe(true);
+		});
+	});
+
+	describe('setSelectedInGroup', () => {
+		it('should select the group and unselect all children', () => {
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node', true);
+			const nodeEventGroup = logStreamingStore.items.destinationId.eventGroups.find(
+				(group) => group.name === 'n8n.node',
+			);
+			expect(nodeEventGroup).toBeDefined();
+			expect(nodeEventGroup!.selected).toBe(true);
+			expect(nodeEventGroup!.children.every((e) => !e.selected)).toBe(true);
+		});
+
+		it('should select an event in a group and mark the group as indeterminate', () => {
+			logStreamingStore.addSelectedEvent('destinationId', 'n8n.node.started');
+			const nodeEventGroup = logStreamingStore.items.destinationId.eventGroups.find(
+				(group) => group.name === 'n8n.node',
+			);
+			expect(nodeEventGroup).toBeDefined();
+			expect(nodeEventGroup!.indeterminate).toBe(true);
+			const startedEvent = nodeEventGroup!.children.find((e) => e.name === 'n8n.node.started');
+			expect(startedEvent?.selected).toBe(true);
+		});
+
+		it('should select the group if all children are selected', () => {
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.started', true);
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.success', true);
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.failed', true);
+			const nodeEventGroup = logStreamingStore.items.destinationId.eventGroups.find(
+				(group) => group.name === 'n8n.node',
+			);
+			expect(nodeEventGroup).toBeDefined();
+			expect(nodeEventGroup!.selected).toBe(true);
+			expect(nodeEventGroup!.indeterminate).toBe(false);
+			expect(nodeEventGroup!.children.every((e) => !e.selected)).toBe(true);
+		});
+
+		it('should deselect the group if any child is deselected', () => {
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.success', true);
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.failed', true);
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.started', false);
+			const nodeEventGroup = logStreamingStore.items.destinationId.eventGroups.find(
+				(group) => group.name === 'n8n.node',
+			);
+			expect(nodeEventGroup).toBeDefined();
+			expect(nodeEventGroup!.selected).toBe(false);
+			expect(nodeEventGroup!.indeterminate).toBe(true);
+		});
+
+		it('should unset the group indeterminate state if no children are selected', () => {
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.success', false);
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.failed', false);
+			logStreamingStore.setSelectedInGroup('destinationId', 'n8n.node.started', false);
+			const nodeEventGroup = logStreamingStore.items.destinationId.eventGroups.find(
+				(group) => group.name === 'n8n.node',
+			);
+			expect(nodeEventGroup).toBeDefined();
+			expect(nodeEventGroup!.selected).toBe(false);
+			expect(nodeEventGroup!.indeterminate).toBe(false);
+		});
+	});
+});

packages/frontend/editor-ui/src/stores/logStreaming.store.ts

@@ -33,20 +33,75 @@ export interface DestinationSettingsStore {
 	[key: string]: DestinationStoreItem;
 }
 
+const eventGroupFromEventName = (eventName: string): string | undefined => {
+	const matches = eventName.match(/^[\w\s]+\.[\w\s]+/);
+	if (matches && matches?.length > 0) {
+		return matches[0];
+	}
+	return undefined;
+};
+
+const prettifyEventName = (label: string, group = ''): string => {
+	label = label.replace(group + '.', '');
+	if (label.length > 0) {
+		label = label[0].toUpperCase() + label.substring(1);
+		label = label.replaceAll('.', ' ');
+	}
+	return label;
+};
+
+const eventGroupsFromStringList = (
+	dottedList: Set<string>,
+	selectionList: Set<string> = new Set(),
+) => {
+	const result = [] as EventSelectionGroup[];
+	const eventNameArray = Array.from(dottedList.values());
+
+	const groups: Set<string> = new Set<string>();
+
+	// since a Set returns iteration items on the order they were added, we can make sure workflow and nodes come first
+	groups.add('n8n.workflow');
+	groups.add('n8n.node');
+
+	for (const eventName of eventNameArray) {
+		const matches = eventName.match(/^[\w\s]+\.[\w\s]+/);
+		if (matches && matches?.length > 0) {
+			groups.add(matches[0]);
+		}
+	}
+
+	for (const group of groups) {
+		const collection: EventSelectionGroup = {
+			children: [],
+			label: group,
+			name: group,
+			selected: selectionList.has(group),
+			indeterminate: false,
+		};
+		const eventsOfGroup = eventNameArray.filter((e) => e.startsWith(group));
+		for (const event of eventsOfGroup) {
+			if (!collection.selected && selectionList.has(event)) {
+				collection.indeterminate = true;
+			}
+			const subCollection: EventSelectionItem = {
+				label: prettifyEventName(event, group),
+				name: event,
+				selected: selectionList.has(event),
+				indeterminate: false,
+			};
+			collection.children.push(subCollection);
+		}
+		result.push(collection);
+	}
+	return result;
+};
+
 export const useLogStreamingStore = defineStore('logStreaming', () => {
 	const items = ref<DestinationSettingsStore>({});
 	const eventNames = ref(new Set<string>());
 
 	const rootStore = useRootStore();
 
-	const addDestination = (destination: MessageEventBusDestinationOptions) => {
-		if (destination.id && items.value[destination.id]) {
-			items.value[destination.id].destination = destination;
-		} else {
-			setSelectionAndBuildItems(destination);
-		}
-	};
-
 	const setSelectionAndBuildItems = (destination: MessageEventBusDestinationOptions) => {
 		if (destination.id) {
 			if (!items.value[destination.id]) {
@@ -70,6 +125,14 @@ export const useLogStreamingStore = defineStore('logStreaming', () => {
 		}
 	};
 
+	const addDestination = (destination: MessageEventBusDestinationOptions) => {
+		if (destination.id && items.value[destination.id]) {
+			items.value[destination.id].destination = destination;
+		} else {
+			setSelectionAndBuildItems(destination);
+		}
+	};
+
 	const getDestination = (destinationId: string) => {
 		if (items.value[destinationId]) {
 			return items.value[destinationId].destination;
@@ -79,10 +142,9 @@ export const useLogStreamingStore = defineStore('logStreaming', () => {
 	};
 
 	const getAllDestinations = () => {
-		const destinations: MessageEventBusDestinationOptions[] = [];
-		for (const key of Object.keys(items)) {
-			destinations.push(items.value[key].destination);
-		}
+		const destinations: MessageEventBusDestinationOptions[] = Object.values(items.value).map(
+			(item) => item.destination,
+		);
 		return destinations;
 	};
 
@@ -102,6 +164,45 @@ export const useLogStreamingStore = defineStore('logStreaming', () => {
 		eventNames.value.clear();
 	};
 
+	const setSelectedInGroup = (destinationId: string, name: string, isSelected: boolean) => {
+		if (!items.value[destinationId]) return;
+
+		const groupName = eventGroupFromEventName(name);
+		const group = items.value[destinationId].eventGroups.find((e) => e.name === groupName);
+		if (!group) return;
+
+		const children = group.children;
+		if (groupName === name) {
+			group.selected = isSelected;
+			group.indeterminate = false;
+			// if the whole group is toggled, all children are unselected
+			children.forEach((e) => (e.selected = false));
+			return;
+		}
+
+		const event = children.find((e) => e.name === name);
+		if (!event) return;
+
+		event.selected = isSelected;
+
+		// If whole group is selected and the event is unselected,
+		// we unselect the group but keep other children selected
+		if (!isSelected && group.selected) {
+			group.selected = false;
+			group.children.filter((e) => e !== event).forEach((e) => (e.selected = true));
+		}
+
+		// If all children are selected, we select the group
+		// and unselect all children
+		const selectedChildren = children.filter((e) => e.selected);
+		if (isSelected && selectedChildren.length === children.length) {
+			group.selected = true;
+			group.children.forEach((e) => (e.selected = false));
+		}
+
+		group.indeterminate = selectedChildren.length > 0 && selectedChildren.length < children.length;
+	};
+
 	const addSelectedEvent = (id: string, name: string) => {
 		items.value[id]?.selectedEvents?.add(name);
 		setSelectedInGroup(id, name, true);
@@ -112,44 +213,6 @@ export const useLogStreamingStore = defineStore('logStreaming', () => {
 		setSelectedInGroup(id, name, false);
 	};
 
-	const setSelectedInGroup = (destinationId: string, name: string, isSelected: boolean) => {
-		if (items.value[destinationId]) {
-			const groupName = eventGroupFromEventName(name);
-			const groupIndex = items.value[destinationId].eventGroups.findIndex(
-				(e) => e.name === groupName,
-			);
-
-			if (groupIndex > -1) {
-				if (groupName === name) {
-					items.value[destinationId].eventGroups[groupIndex].selected = isSelected;
-				} else {
-					const eventIndex = items.value[destinationId].eventGroups[groupIndex].children.findIndex(
-						(e) => e.name === name,
-					);
-					if (eventIndex > -1) {
-						items.value[destinationId].eventGroups[groupIndex].children[eventIndex].selected =
-							isSelected;
-						if (isSelected) {
-							items.value[destinationId].eventGroups[groupIndex].indeterminate = isSelected;
-						} else {
-							let anySelected = false;
-							for (
-								let i = 0;
-								i < items.value[destinationId].eventGroups[groupIndex].children.length;
-								i++
-							) {
-								anySelected =
-									anySelected ||
-									items.value[destinationId].eventGroups[groupIndex].children[i].selected;
-							}
-							items.value[destinationId].eventGroups[groupIndex].indeterminate = anySelected;
-						}
-					}
-				}
-			}
-		}
-	};
-
 	const removeDestinationItemTree = (id: string) => {
 		delete items.value[id];
 	};
@@ -194,7 +257,7 @@ export const useLogStreamingStore = defineStore('logStreaming', () => {
 			await saveDestinationToDb(rootStore.restApiContext, destination, selectedEvents);
 			updateDestination(destination);
 			return true;
-		} catch (e) {
+		} catch {
 			return false;
 		}
 	};
@@ -247,66 +310,3 @@ export const useLogStreamingStore = defineStore('logStreaming', () => {
 		items,
 	};
 });
-
-export const eventGroupFromEventName = (eventName: string): string | undefined => {
-	const matches = eventName.match(/^[\w\s]+\.[\w\s]+/);
-	if (matches && matches?.length > 0) {
-		return matches[0];
-	}
-	return undefined;
-};
-
-const prettifyEventName = (label: string, group = ''): string => {
-	label = label.replace(group + '.', '');
-	if (label.length > 0) {
-		label = label[0].toUpperCase() + label.substring(1);
-		label = label.replaceAll('.', ' ');
-	}
-	return label;
-};
-
-export const eventGroupsFromStringList = (
-	dottedList: Set<string>,
-	selectionList: Set<string> = new Set(),
-) => {
-	const result = [] as EventSelectionGroup[];
-	const eventNameArray = Array.from(dottedList.values());
-
-	const groups: Set<string> = new Set<string>();
-
-	// since a Set returns iteration items on the order they were added, we can make sure workflow and nodes come first
-	groups.add('n8n.workflow');
-	groups.add('n8n.node');
-
-	for (const eventName of eventNameArray) {
-		const matches = eventName.match(/^[\w\s]+\.[\w\s]+/);
-		if (matches && matches?.length > 0) {
-			groups.add(matches[0]);
-		}
-	}
-
-	for (const group of groups) {
-		const collection: EventSelectionGroup = {
-			children: [],
-			label: group,
-			name: group,
-			selected: selectionList.has(group),
-			indeterminate: false,
-		};
-		const eventsOfGroup = eventNameArray.filter((e) => e.startsWith(group));
-		for (const event of eventsOfGroup) {
-			if (!collection.selected && selectionList.has(event)) {
-				collection.indeterminate = true;
-			}
-			const subCollection: EventSelectionItem = {
-				label: prettifyEventName(event, group),
-				name: event,
-				selected: selectionList.has(event),
-				indeterminate: false,
-			};
-			collection.children.push(subCollection);
-		}
-		result.push(collection);
-	}
-	return result;
-};