From 1e381aa0ef6bd9984e22181c2c33ea81681e83b8 Mon Sep 17 00:00:00 2001
From: Lucio Lelii <lucio.lelii@isti.cnr.it>
Date: Fri, 4 Sep 2026 12:22:00 +0200
Subject: [PATCH] Cover the array item modal before refactoring it

This round trip - item schema to dialog fields and back to an object - is the
machinery an optional-group modal wants to reuse, and nothing covered it: the
dialog mock in this spec resolved null, so no test ever reached the builder or
the parser.

Six characterisation tests pin what it does today: which fields it builds and
with what labels and types, that it writes the parsed item into the array,
that it edits in place rather than appending, and that a cancelled dialog
changes nothing.

One of them pins behaviour I intend to change and deliberately does not endorse:
an emptied required number becomes 0. Writing it down is the point - the optional
case has to differ, and the difference should be visible as a changed assertion
rather than as a silent shift.

584 frontend tests green.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---
 .../nodes/generic-node/generic-node.spec.ts   | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)

diff --git a/src/app/shared/nodes/generic-node/generic-node.spec.ts b/src/app/shared/nodes/generic-node/generic-node.spec.ts
index 94ab650..d821b0a 100644
--- a/src/app/shared/nodes/generic-node/generic-node.spec.ts
+++ b/src/app/shared/nodes/generic-node/generic-node.spec.ts
@@ -127,6 +127,96 @@ describe('GenericNodeComponent', () => {
     });
   });
 
+  describe('the array item modal, as it behaves today', () => {
+    /**
+     * Characterisation tests. This round trip - schema to dialog fields and back to an object - is
+     * the machinery an optional-group modal wants to reuse, and nothing covered it: the dialog mock
+     * resolved null, so no test ever reached the builder or the parser. These pin what it does now,
+     * so extracting it cannot change it by accident.
+     */
+    const itemSchema = {
+      type: 'object',
+      required: ['name', 'weight'],
+      properties: {
+        name: { type: 'string', 'x-ui-label': 'Skill name' },
+        weight: { type: 'integer' },
+        enabled: { type: 'boolean' }
+      }
+    };
+
+    function withArrayField() {
+      const component = fixture.componentInstance as any;
+      component.arrayFieldDefinitions = [{
+        path: 'skills',
+        label: 'Skills',
+        itemSchema,
+        uniqueBy: null,
+        ui: { structural: false, visibleWhen: [], enabledWhen: [] }
+      }];
+      return component;
+    }
+
+    it('builds one dialog field per item property, honouring labels and types', async () => {
+      const component = withArrayField();
+      const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
+      open.mockResolvedValue(null);
+
+      await component.addArrayItem('skills');
+
+      const dialog = open.mock.calls.at(-1)?.[0];
+      expect(dialog.fields.map((field: any) => [field.key, field.type])).toEqual([
+        ['name', 'text'], ['weight', 'text'], ['enabled', 'checkbox']
+      ]);
+      expect(dialog.fields[0].label).toBe('Skill name');
+    });
+
+    it('writes the parsed item into the array', async () => {
+      const component = withArrayField();
+      const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
+      open.mockResolvedValue({ name: 'summarise', weight: '3', enabled: true });
+
+      await component.addArrayItem('skills');
+
+      const config = component.ensureBlockConfiguration();
+      expect(config['skills']).toEqual([{ name: 'summarise', weight: 3, enabled: true }]);
+    });
+
+    it('turns an emptied required number into 0, which is what it has always done', async () => {
+      // Not an endorsement: it is the behaviour a refactor must not change silently. The optional
+      // case is the one that has to differ, and it differs deliberately.
+      const component = withArrayField();
+      const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
+      open.mockResolvedValue({ name: 'x', weight: '', enabled: false });
+
+      await component.addArrayItem('skills');
+
+      expect(component.ensureBlockConfiguration()['skills'][0].weight).toBe(0);
+    });
+
+    it('edits an existing item in place rather than appending', async () => {
+      const component = withArrayField();
+      const config = component.ensureBlockConfiguration();
+      config['skills'] = [{ name: 'first', weight: 1, enabled: false }];
+      const open = TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>;
+      open.mockResolvedValue({ name: 'renamed', weight: '2', enabled: true });
+
+      await component.editArrayItem('skills', 0);
+
+      expect(config['skills']).toEqual([{ name: 'renamed', weight: 2, enabled: true }]);
+    });
+
+    it('leaves the array untouched when the dialog is cancelled', async () => {
+      const component = withArrayField();
+      const config = component.ensureBlockConfiguration();
+      config['skills'] = [{ name: 'first', weight: 1, enabled: false }];
+      (TestBed.inject(NodeSettingsDialogService).open as ReturnType<typeof vi.fn>).mockResolvedValue(null);
+
+      await component.addArrayItem('skills');
+
+      expect(config['skills']).toEqual([{ name: 'first', weight: 1, enabled: false }]);
+    });
+  });
+
   it('should create', () => {
     expect(component).toBeTruthy();
   });