fix(dmn): carry rule priority, and pass the uuid as the uuid to deleteObject

lib/Service/AdvisoryBodyService.php

@@ -202,7 +202,7 @@ public function delete(string $id): bool {
 			throw new RuntimeException('Advisory body schema not configured');
 		}
 
-		$objectService->deleteObject($register, $schema, $id);
+		$objectService->deleteObject(uuid: $id, register: $register, schema: $schema);
 
 		$this->logger->info(
 			'Advisory body deleted: ' . $id,

lib/Service/Dmn/DecisionTableService.php

@@ -109,7 +109,11 @@ public function updateTable(string $id, array $data): array {
 	 */
 	public function deleteTable(string $id): void {
 		[$objectService, $register, $schema] = $this->resolve();
-		$objectService->deleteObject($register, $schema, $id);
+		// Named, like every other call in this class. OpenRegister's signature is
+		// deleteObject(uuid, register, schema); passing them positionally in
+		// register/schema/id order transposed all three, so this looked up a
+		// register whose id was really the schema's and 500'd every time.
+		$objectService->deleteObject(uuid: $id, register: $register, schema: $schema);
 	}//end deleteTable()
 
 	/**
@@ -255,6 +259,8 @@ private function validateFields(mixed $raw, string $label): array {
 	 * @return array<int, array<string, mixed>> The validated rules.
 	 *
 	 * @throws OCSBadRequestException When a rule's entry counts don't align with inputs/outputs.
+	 *
+	 * @spec openspec/specs/dmn-decision-tables/spec.md
 	 */
 	private function validateRules(mixed $raw, int $inputCount, int $outputCount): array {
 		if (is_array($raw) === false) {
@@ -289,12 +295,21 @@ private function validateRules(mixed $raw, int $inputCount, int $outputCount): a
 				throw new OCSBadRequestException('Rule ' . $index . ' outputEntries count (' . $got . ') must match outputs count (' . $outputCount . ')');
 			}
 
-			$rules[] = [
+			$built = [
 				'id' => trim((string)($rule['id'] ?? ('r' . ($index + 1)))),
 				'annotation' => trim((string)($rule['annotation'] ?? '')),
 				'inputEntries' => array_map(static fn (mixed $entry): string => (string)$entry, $inputEntries),
 				'outputEntries' => $outputEntries,
 			];
+
+			// PRIORITY ranks by this, and it is only carried when the author
+			// supplied it: writing a default 0 onto every rule of every table
+			// would put a meaningless field on the tables that do not use it.
+			if (array_key_exists('priority', $rule) === true) {
+				$built['priority'] = (int)$rule['priority'];
+			}
+
+			$rules[] = $built;
 		}//end foreach
 
 		return $rules;

lib/Service/Kcc/RoutingRuleService.php

@@ -97,10 +97,12 @@ public function updateRule(string $id, array $data): array {
 	 * @param string $id The rule id.
 	 *
 	 * @return void
+	 *
+	 * @spec openspec/changes/kcc-klantcontact-integratie/tasks.md#TASK-KCC-17
 	 */
 	public function deleteRule(string $id): void {
 		[$objectService, $register, $schema] = $this->resolve(schemaKey: 'routing_rule_schema');
-		$objectService->deleteObject($register, $schema, $id);
+		$objectService->deleteObject(uuid: $id, register: $register, schema: $schema);
 	}//end deleteRule()
 
 	/**

lib/Service/MilestoneService.php

@@ -241,7 +241,7 @@ public function reverseMilestone(
 		foreach ($records as $record) {
 			$recordId = $record['id'] ?? $record['uuid'] ?? '';
 			if ($recordId !== '') {
-				$objectService->deleteObject($register, $schema, $recordId);
+				$objectService->deleteObject(uuid: $recordId, register: $register, schema: $schema);
 			}
 		}
 

lib/Service/TenantSaasService.php

@@ -306,7 +306,7 @@ public function delete(string $tenantId): bool {
 		}
 
 		try {
-			$objectService->deleteObject(register: self::REGISTER, schema: self::SCHEMA_TENANT, id: $tenantId);
+			$objectService->deleteObject(uuid: $tenantId, register: self::REGISTER, schema: self::SCHEMA_TENANT);
 			return true;
 		} catch (Throwable $e) {
 			$this->logger->error('Dossiq: TenantSaasService::delete failed', ['tenantId' => $tenantId, 'exception' => $e->getMessage()]);

lib/Settings/dossiq_mock_register.json

@@ -5514,6 +5514,12 @@
                   "title": "Output Entries",
                   "type": "array",
                   "description": "One literal value per output, positionally aligned to outputs[]"
+                },
+                "priority": {
+                  "title": "Priority",
+                  "type": "integer",
+                  "default": 0,
+                  "description": "Rank for the PRIORITY hit policy. The highest number wins. Equal ranks keep declaration order."
                 }
               }
             }

lib/Settings/register.d/95-dmn-decision-tables.json

@@ -111,6 +111,12 @@
                                     "title": "Output Entries",
                                     "type": "array",
                                     "description": "One literal value per output, positionally aligned to outputs[]"
+                                },
+                                "priority": {
+                                    "title": "Priority",
+                                    "type": "integer",
+                                    "default": 0,
+                                    "description": "Rank for the PRIORITY hit policy. The highest number wins. Equal ranks keep declaration order."
                                 }
                             }
                         }

openspec/changes/dossiq-consumes-shared-dmn/tasks.md

@@ -9,3 +9,8 @@
 - [x] Rework `EvaluateDecisionHandlerTest` onto a mocked evaluator
 - [x] Correct the schema copy: all five hit policies work
 - [x] Run the three seeded tables through the real shared evaluator (all three are placeholder data and fail on their own entries, under either engine)
+- [x] Carry `priority` on a rule, so REQ "PRIORITY returns the highest" is actually
+      satisfiable (dossiq#1564). At the time this change merged it was not: the
+      schema had no `priority` property and `validateRules()` stripped the field,
+      so a PRIORITY table answered 200 with the wrong rule. The e2e found it; the
+      unit tests, which mock the evaluator, could not.

tests/Unit/Service/AdvisoryBodyServiceTest.php

@@ -87,7 +87,11 @@ public function saveObject(string $register, string $schema, array $data, string
 	 *
 	 * @return void
 	 */
-	public function deleteObject(string $register, string $schema, string $id): void;
+	// OpenRegister's real signature is deleteObject(uuid, register, schema). This
+	// fake declared (register, schema, id), so it accepted the transposed
+	// positional call the service was making and no test could ever see the bug.
+	// A fake that agrees with the caller instead of with the callee cannot fail.
+	public function deleteObject(string $uuid, ?string $register = null, ?string $schema = null): void;
 }//end interface
 
 /**
@@ -196,4 +200,45 @@ public function testSaveThrowsWhenObjectServiceUnavailable(): void {
 
 	}//end testSaveThrowsWhenObjectServiceUnavailable()
 
+	/**
+	 * Delete passes the UUID as the uuid, not as the register.
+	 *
+	 * This is the assertion the suite was missing. The service called
+	 * `deleteObject($register, $schema, $id)` positionally against a signature
+	 * of `(uuid, register, schema)`, so all three arguments were transposed and
+	 * every delete looked up a register whose id was really a schema's. It
+	 * 500'd on a live instance every time, and the fake above accepted it
+	 * because the fake had been written to match the caller rather than
+	 * OpenRegister.
+	 *
+	 * The same defect shipped in four other services. InspectionChecklistService
+	 * carries a docblock describing it, found the same way and never swept.
+	 *
+	 * @return void
+	 */
+	public function testDeletePassesTheUuidAsTheUuid(): void {
+		$received = [];
+
+		$objectService = $this->createMock(AdvisoryObjectServiceStub::class);
+		$objectService->method('deleteObject')->willReturnCallback(
+			function (string $uuid, ?string $register = null, ?string $schema = null) use (&$received): void {
+				$received = ['uuid' => $uuid, 'register' => $register, 'schema' => $schema];
+			}
+		);
+
+		$this->settings->method('getObjectService')->willReturn($objectService);
+		$this->settings->method('getConfigValue')->willReturnCallback(
+			static fn (string $key): string => ['register' => 'reg-1', 'advisory_body_schema' => 'schema-9'][$key] ?? ''
+		);
+
+		$this->service->delete('body-uuid-1');
+
+		$this->assertSame(
+			['uuid' => 'body-uuid-1', 'register' => 'reg-1', 'schema' => 'schema-9'],
+			$received,
+			'a transposed positional call would put the register in the uuid slot'
+		);
+
+	}//end testDeletePassesTheUuidAsTheUuid()
+
 }//end class

tests/e2e/changed-surfaces.spec.ts

@@ -181,3 +181,121 @@ test.describe('parafering activation', () => {
 		).not.toBe(200)
 	})
 })
+
+test.describe('decision tables evaluated by OpenRegister', () => {
+	// dossiq deleted its own DMN engine and now injects
+	// `OCA\OpenRegister\Service\Dmn\DecisionTableEvaluator`. Unit tests cannot
+	// prove that resolves: dossiq's suite autoloads `OCA\OpenRegister\` from
+	// tests/Stubs, so it asserts the delegation against a stub by construction.
+	// Only a live instance, where both apps are installed, can say whether the
+	// class is there and whether the container can inject it.
+	test('a PRIORITY table decides, where the old engine refused it', async ({
+		page,
+	}) => {
+		await page.goto(`${BASE_URL}/apps/dossiq/`)
+		await page.waitForLoadState('domcontentloaded')
+
+		const outcome = await page.evaluate(async () => {
+			const token = (window as any).OC?.requestToken ?? ''
+			const headers = {
+				requesttoken: token,
+				'Content-Type': 'application/json',
+			}
+
+			// PRIORITY is the point. dossiq's own engine answered
+			// `hit_policy_not_implemented` for this table, while its schema
+			// offered PRIORITY in the enum the whole time.
+			const table = {
+				// `key` is required by the create endpoint; without it this is a
+				// 400 and the test fails at the fixture rather than the claim.
+				key: `e2e-priority-${Date.now()}`,
+				name: 'e2e priority table',
+				hitPolicy: 'PRIORITY',
+				inputs: [{ name: 'severity', type: 'string' }],
+				outputs: [{ name: 'intervention', type: 'string' }],
+				rules: [
+					{
+						id: 'low',
+						inputEntries: ['gering'],
+						outputEntries: ['brief'],
+						priority: 1,
+					},
+					{
+						id: 'high',
+						inputEntries: ['gering'],
+						outputEntries: ['fine'],
+						priority: 10,
+					},
+				],
+			}
+
+			const created = await fetch('/apps/dossiq/api/decisions', {
+				method: 'POST',
+				headers,
+				body: JSON.stringify(table),
+			})
+			if (!created.ok) {
+				return {
+					stage: 'create',
+					status: created.status,
+					body: await created.text(),
+				}
+			}
+
+			const row = await created.json()
+			const id = row.id ?? row.uuid ?? row['@self']?.id
+			if (!id)
+				return {
+					stage: 'create',
+					status: 200,
+					body: 'no id in the created row',
+				}
+
+			const res = await fetch(`/apps/dossiq/api/decisions/${id}/evaluate`, {
+				method: 'POST',
+				headers,
+				body: JSON.stringify({ severity: 'gering' }),
+			})
+			const body = await res.text()
+
+			// This suite runs against the shared development instance, so the
+			// fixture is removed rather than left behind for the next reader to
+			// wonder about.
+			await fetch(`/apps/dossiq/api/decisions/${id}`, {
+				method: 'DELETE',
+				headers,
+			})
+
+			return { stage: 'evaluate', status: res.status, body }
+		})
+
+		expect(
+			outcome.stage,
+			`could not create the decision table: ${outcome.body}`,
+		).toBe('evaluate')
+
+		// 🔴 What would make this pass wrongly: nothing quiet. If the shared
+		// evaluator were unresolvable the container would fail to build the
+		// controller and this would be a 500; if PRIORITY were still refused it
+		// would be a 4xx carrying `hit_policy_not_implemented`. Both are asserted
+		// against explicitly rather than inferred from a truthy response.
+		expect(outcome.body, 'PRIORITY must no longer be refused').not.toContain(
+			'hit_policy_not_implemented',
+		)
+		expect(
+			outcome.status,
+			`evaluate answered ${outcome.status}: ${outcome.body}`,
+		).toBe(200)
+
+		const decided = JSON.parse(outcome.body)
+
+		// The higher priority wins. Asserting the VALUE, not merely that
+		// something came back, is what separates this from a smoke test: a
+		// delegation that returned the first rule instead would still be a 200.
+		expect(
+			decided.outputs?.intervention,
+			'the rule with priority 10 must win, not the first in declaration order',
+		).toBe('fine')
+		expect(decided.hitPolicy).toBe('PRIORITY')
+	})
+})