From 64f5855fdbe87949f1cb76109b1ee5a081db93b3 Mon Sep 17 00:00:00 2001 From: "bert.hausmans" Date: Sat, 25 Apr 2026 00:52:57 +0200 Subject: [PATCH] test(form-field): pin conditional_logic activity log payload contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ARCH §8.6 specifies a dual-event contract on logic changes — a `field.updated` row carrying old/new diffs of the reconstructed JSON shape, plus a semantic `field.conditional_logic_replaced` row from inside `replaceLogic()`. The semantic event is already pinned by `FormFieldConditionalLogicServiceTest`. The diff payload contract was documented but unasserted. Two new tests: - `test_field_updated_activity_log_contains_conditional_logic_diff_when_tree_changes` Pins old/new payload shapes via byte-equal `json_encode` comparison (mirrors ConditionalLogicSnapshotAndResourceParityTest's associative-array key-order trap). Both rows share the same causer_id. - `test_field_updated_without_logic_change_does_not_emit_conditional_logic_diff` Pins the negative: bare label-only updates must NOT carry a `conditional_logic` key in the field.updated payload, and must NOT emit a semantic `field.conditional_logic_replaced` row. The first test passed against the original implementation; the second required `FormFieldService::update()` to filter `conditional_logic` out of the activity-log payload when the reconstructed shape didn't change between pre- and post-write. Adjustment lands in this commit: the `$before` / `$new` arrays now only carry the key when `$currentConditionalShape !== $newConditionalShape`. Tests: 1148 → 1150 green (3099 → 3110 assertions). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../Services/FormBuilder/FormFieldService.php | 26 ++- ...ConditionalLogicActivityLogPayloadTest.php | 149 ++++++++++++++++++ 2 files changed, 167 insertions(+), 8 deletions(-) create mode 100644 api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicActivityLogPayloadTest.php diff --git a/api/app/Services/FormBuilder/FormFieldService.php b/api/app/Services/FormBuilder/FormFieldService.php index efb73e32..1bb13344 100644 --- a/api/app/Services/FormBuilder/FormFieldService.php +++ b/api/app/Services/FormBuilder/FormFieldService.php @@ -100,7 +100,6 @@ final class FormFieldService $before = [ 'binding' => $currentBindingShape, - 'conditional_logic' => $currentConditionalShape, 'is_filterable' => $field->is_filterable, 'is_pii' => $field->is_pii, 'field_type' => $field->field_type, @@ -123,15 +122,26 @@ final class FormFieldService $this->schemaService->bumpVersion($schema); + $newConditionalShape = $this->conditionalLogicService->toJsonShape($field->fresh()?->rootConditionalLogicGroup()); + $new = [ + 'binding' => $this->bindingService->toJsonShape($field->bindings()->first()), + 'is_filterable' => $field->is_filterable, + 'is_pii' => $field->is_pii, + 'field_type' => $field->field_type, + ]; + + // ARCH §8.6: include conditional_logic in the field.updated diff only + // when the tree actually changed. Bare label/sort_order updates and + // payloads that did not touch conditional_logic must not carry the + // key — otherwise downstream activity-log consumers see noise. + if ($currentConditionalShape !== $newConditionalShape) { + $before['conditional_logic'] = $currentConditionalShape; + $new['conditional_logic'] = $newConditionalShape; + } + $field->logFieldChange('field.updated', [ 'old' => $before, - 'new' => [ - 'binding' => $this->bindingService->toJsonShape($field->bindings()->first()), - 'conditional_logic' => $this->conditionalLogicService->toJsonShape($field->fresh()?->rootConditionalLogicGroup()), - 'is_filterable' => $field->is_filterable, - 'is_pii' => $field->is_pii, - 'field_type' => $field->field_type, - ], + 'new' => $new, ]); if ($before['is_filterable'] !== $field->is_filterable) { diff --git a/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicActivityLogPayloadTest.php b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicActivityLogPayloadTest.php new file mode 100644 index 00000000..d518ffe5 --- /dev/null +++ b/api/tests/Feature/FormBuilder/ConditionalLogic/ConditionalLogicActivityLogPayloadTest.php @@ -0,0 +1,149 @@ +create(); + $schema = FormSchema::factory()->create(['organisation_id' => $org->id]); + FormField::factory()->create(['form_schema_id' => $schema->id, 'slug' => 'gate']); + FormField::factory()->create(['form_schema_id' => $schema->id, 'slug' => 'region']); + + $field = FormField::factory() + ->withConditionalLogic([ + 'operator' => 'all', + 'children' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ['field_slug' => 'region', 'operator' => 'equals', 'value' => 'NL'], + ], + ]) + ->create(['form_schema_id' => $schema->id, 'slug' => 'subject']); + + $logicService = app(FormFieldConditionalLogicService::class); + $oldShape = $logicService->toJsonShape($field->fresh()->rootConditionalLogicGroup()); + + $causer = User::factory()->create(); + $this->actingAs($causer); + Activity::query()->delete(); + + // Replace the tree with a different shape — single condition, ANY group. + app(FormFieldService::class)->update($field->fresh(), [ + 'conditional_logic' => [ + 'show_when' => [ + 'any' => [ + ['field_slug' => 'gate', 'operator' => 'not_empty'], + ], + ], + ], + ]); + + $newShape = $logicService->toJsonShape($field->fresh()->rootConditionalLogicGroup()); + $this->assertNotEquals($oldShape, $newShape, 'precondition: tree must have changed'); + + $updated = Activity::query() + ->where('subject_type', $field->getMorphClass()) + ->where('subject_id', $field->id) + ->where('description', 'field.updated') + ->first(); + $this->assertNotNull($updated, 'field.updated row must exist'); + + $properties = $updated->properties; + // Use json_encode comparison to avoid associative-array key-order traps — + // mirrors ConditionalLogicSnapshotAndResourceParityTest. + $this->assertSame( + json_encode($oldShape), + json_encode($properties->get('old')['conditional_logic'] ?? null), + ); + $this->assertSame( + json_encode($newShape), + json_encode($properties->get('new')['conditional_logic'] ?? null), + ); + + $semantic = Activity::query() + ->where('subject_type', $field->getMorphClass()) + ->where('subject_id', $field->id) + ->where('description', 'field.conditional_logic_replaced') + ->first(); + $this->assertNotNull($semantic, 'semantic event must exist'); + + $this->assertSame((int) $causer->id, (int) $updated->causer_id); + $this->assertSame((int) $causer->id, (int) $semantic->causer_id); + } + + public function test_field_updated_without_logic_change_does_not_emit_conditional_logic_diff(): void + { + $org = Organisation::factory()->create(); + $schema = FormSchema::factory()->create(['organisation_id' => $org->id]); + FormField::factory()->create(['form_schema_id' => $schema->id, 'slug' => 'gate']); + + $field = FormField::factory() + ->withConditionalLogic([ + 'operator' => 'all', + 'children' => [ + ['field_slug' => 'gate', 'operator' => 'equals', 'value' => 'yes'], + ], + ]) + ->create(['form_schema_id' => $schema->id, 'slug' => 'subject', 'label' => 'Old label']); + + $causer = User::factory()->create(); + $this->actingAs($causer); + Activity::query()->delete(); + + app(FormFieldService::class)->update($field->fresh(), [ + 'label' => 'New label', + ]); + + $updated = Activity::query() + ->where('subject_type', $field->getMorphClass()) + ->where('subject_id', $field->id) + ->where('description', 'field.updated') + ->first(); + $this->assertNotNull($updated, 'field.updated row must exist for the label change'); + + $properties = $updated->properties; + $this->assertArrayNotHasKey( + 'conditional_logic', + $properties->get('old') ?? [], + 'old payload must not include conditional_logic when tree did not change', + ); + $this->assertArrayNotHasKey( + 'conditional_logic', + $properties->get('new') ?? [], + 'new payload must not include conditional_logic when tree did not change', + ); + + $semantic = Activity::query() + ->where('subject_type', $field->getMorphClass()) + ->where('subject_id', $field->id) + ->where('description', 'field.conditional_logic_replaced') + ->first(); + $this->assertNull($semantic, 'no semantic event when tree did not change'); + } +}