Refuse rules that point to a missing question, list the rules that use each question

CHANGELOG.md

@@ -53,6 +53,7 @@ Lines marked **Upgrade step** need you to act after updating.
 -   On small phones, the form editor and submissions pages no longer scroll sideways. On phones, the submissions page's "Delete All" and "Download CSV" buttons now sit in their own row. (#260)
 -   Questions, answers and template imports now also refuse these values with a message that names the field, and nothing is changed: a question text over 65,535 bytes (a letter like "ä" counts as 2), a webhook address or answer name over 255 characters, a required, hidden or editable value that isn't yes or no, a question or answer position in a template that isn't a whole number, is below zero or is too big, and a template whose answers or logic rules aren't a list. Before, most of these failed with a server error on MySQL and MariaDB, or were saved anyway. A form setting in a template that is only spaces now counts as empty, like on the settings pages, instead of failing. (#261)
 -   While the form editor loads your questions, it now shows grey placeholder rows instead of the "Welcome to the Editor" text. When the questions can't load, it says so and offers Try again. Before, a spinner turned forever. (#263)
+-   A logic rule that uses or goes to a deleted question, or one of another form, is now refused with a message that names the field. Before, it saved and looked like it worked. The blocks API now lists, for each question, the rules that use it (`used_by`). (#264)
 
 ### Security
 

app/Http/Controllers/Api/FormBlockController.php

@@ -8,6 +8,7 @@
 use App\Models\Form;
 use App\Models\FormBlock;
 use Illuminate\Http\Request;
+use Illuminate\Support\Arr;
 use Knuckles\Scribe\Attributes\Authenticated;
 use Knuckles\Scribe\Attributes\BodyParam;
 use Knuckles\Scribe\Attributes\Group;
@@ -20,6 +21,8 @@ class FormBlockController extends Controller
      * Get all form blocks
      *
      * This endpoint returns all form blocks of a form. A form block is like a single page on the form. Each block has a type that can contain different block interactions based on that type.
+     *
+     * Each block lists in `used_by` the logic rules that use it: `id` and `name` of the rule, `block` with the `uuid` of the block the rule is on, and `as` with `source` (a condition uses its answer) and/or `target` (the rule goes to it).
      */
     public function index(Request $request, Form $form)
     {
@@ -32,7 +35,24 @@ public function index(Request $request, Form $form)
             $blocks->each->setAppends(['session_count', 'interactions']);
         }
 
-        return response()->json($blocks->values()->toArray());
+        $rules = $blocks->flatMap(fn ($block) => $block->formBlockLogics->map(fn ($rule) => [
+            'id' => $rule->id,
+            'name' => $rule->name,
+            'block' => $block->uuid,
+            'sources' => collect($rule->conditions)->pluck('source'),
+            'target' => $rule->action === 'goto' ? $rule->action_payload : null,
+        ]));
+
+        return response()->json($blocks->map(fn ($block) => [
+            ...$block->toArray(),
+            'used_by' => $rules->map(fn ($rule) => [
+                ...Arr::only($rule, ['id', 'name', 'block']),
+                'as' => array_keys(array_filter([
+                    'source' => $rule['sources']->containsStrict($block->uuid),
+                    'target' => $rule['target'] === $block->uuid,
+                ])),
+            ])->filter(fn ($use) => $use['as'])->values(),
+        ])->values());
     }
 
     /**

app/Http/Requests/FormBlockLogicRequest.php

@@ -4,6 +4,7 @@
 
 use App\Enums\FormBlockType;
 use App\Models\FormBlock;
+use Closure;
 use Illuminate\Foundation\Http\FormRequest;
 use Illuminate\Validation\Validator;
 
@@ -17,9 +18,21 @@ public function authorize(): bool
         return $this->user()->can('update', $this->block());
     }
 
-    protected function block(): FormBlock
+    // null while Scribe reads the rules without a route
+    protected function block(): ?FormBlock
     {
-        return $this->route('block') ?? $this->route('logic')->formBlock;
+        return $this->route('block') ?? $this->route('logic')?->formBlock;
+    }
+
+    // the template import reuses these rules without a block, and drops a rule that points to a missing block itself
+    protected function inThisForm(string $message, bool $when = true): Closure
+    {
+        /** The `uuid` of a block of this form. */
+        return function (string $attribute, mixed $value, Closure $fail) use ($message, $when) {
+            if ($when && $this->block() && ! $this->block()->form->formBlocks->pluck('uuid')->containsStrict($value)) {
+                $fail($message);
+            }
+        };
     }
 
     /**
@@ -32,12 +45,13 @@ public function rules(): array
         return [
             'name' => 'required|string|max:255',
             'conditions' => 'required|array',
-            'conditions.*.source' => 'required|string',
+            'conditions.*.source' => ['required', 'string', $this->inThisForm('The question of your #:position condition is not in this form.')],
             'conditions.*.operator' => 'required|string|in:equals,equalsNot,contains,containsNot,isLowerThan,isGreaterThan',
             'conditions.*.value' => 'required|string',
             'conditions.*.chainOperator' => 'required|string|in:and,or',
             'action' => 'required|string|in:hide,show,goto',
-            'action_payload' => 'nullable|string',
+            // only a goto rule uses its payload, a hide or show rule can keep an old target from the editor
+            'action_payload' => ['nullable', 'string', $this->inThisForm('The question to go to is not in this form.', when: $this->input('action') === 'goto')],
             'evaluate' => 'required|string|in:before,after',
         ];
     }

tests/Feature/FormBlockLogicTest.php

@@ -110,3 +110,57 @@
 
     expect(FormBlockLogic::pluck('name', 'id')->all())->toBe([$logic->id => 'Old']);
 })->with(['xlf', '.0', 'e0']);
+
+test('a rule that points to a deleted question or one of another form is refused and changes nothing', function (string $method, string $field) {
+    $block = FormBlock::factory()->create();
+    $question = FormBlock::factory()->for($block->form)->create();
+    $logic = FormBlockLogic::factory()->for($block)->create(['name' => 'Old']);
+    $deleted = FormBlock::factory()->for($block->form)->create();
+    $deleted->delete();
+    $otherForm = FormBlock::factory()->create();
+
+    $route = $method === 'create' ? route('api.logics.create', $block->id) : route('api.logics.update', $logic->id);
+    $rule = fn (string $source, string $target) => [
+        'name' => 'New',
+        'conditions' => [['source' => $source, 'operator' => 'equals', 'value' => 'yes', 'chainOperator' => 'and']],
+        'action' => 'goto',
+        'action_payload' => $target,
+        'evaluate' => 'after',
+    ];
+
+    $this->actingAs($block->form->user);
+
+    foreach ([$deleted->uuid, $otherForm->uuid] as $missing) {
+        $payload = $field === 'action_payload' ? $rule($question->uuid, $missing) : $rule($missing, $question->uuid);
+
+        $this->json('post', $route, $payload)
+            ->assertStatus(422)
+            ->assertJsonValidationErrors([$field => 'is not in this form'])
+            ->assertJsonMissingValidationErrors($field === 'action_payload' ? 'conditions.0.source' : 'action_payload');
+    }
+
+    expect(FormBlockLogic::pluck('name')->all())->toBe(['Old']);
+
+    $this->json('post', $route, $rule($question->uuid, $question->uuid))->assertSuccessful();
+})->with(['create', 'update'])->with(['conditions.0.source', 'action_payload']);
+
+test('a hide or show rule that kept an old target from a deleted question saves as before', function (string $method, string $action) {
+    $block = FormBlock::factory()->create();
+    $question = FormBlock::factory()->for($block->form)->create();
+    $logic = FormBlockLogic::factory()->for($block)->create(['name' => 'Old']);
+    $deleted = FormBlock::factory()->for($block->form)->create();
+    $deleted->delete();
+
+    // the editor keeps the old target when a rule switches from goto to hide or show
+    $this->actingAs($block->form->user)
+        ->json('post', $method === 'create' ? route('api.logics.create', $block->id) : route('api.logics.update', $logic->id), [
+            'name' => 'New',
+            'conditions' => [['source' => $question->uuid, 'operator' => 'equals', 'value' => 'yes', 'chainOperator' => 'and']],
+            'action' => $action,
+            'action_payload' => $deleted->uuid,
+            'evaluate' => 'before',
+        ])
+        ->assertSuccessful();
+
+    expect(FormBlockLogic::where('name', 'New')->value('action'))->toBe($action);
+})->with(['create', 'update'])->with(['hide', 'show']);

tests/Feature/ManageBlocksTest.php

@@ -371,3 +371,36 @@
 
     expect($block->fresh()->is_disabled)->toBeFalse();
 });
+
+test('the blocks list names the rules that use each question', function () {
+    $form = Form::factory()->create();
+    $first = FormBlock::factory()->for($form)->create(['sequence' => 0]);
+    $second = FormBlock::factory()->for($form)->create(['sequence' => 1]);
+    $third = FormBlock::factory()->for($form)->create(['sequence' => 2]);
+    $unused = FormBlock::factory()->for($form)->create(['sequence' => 3]);
+
+    $condition = fn (FormBlock $source) => ['source' => $source->uuid, 'operator' => 'equals', 'value' => 'yes', 'chainOperator' => 'and'];
+    $skip = FormBlockLogic::factory()->for($first)->create([
+        'name' => 'Skip', 'conditions' => [$condition($third)], 'action' => 'goto', 'action_payload' => $third->uuid, 'evaluate' => 'after',
+    ]);
+    $jump = FormBlockLogic::factory()->for($second)->create([
+        'name' => 'Jump', 'conditions' => [$condition($first)], 'action' => 'goto', 'action_payload' => $third->uuid, 'evaluate' => 'after',
+    ]);
+    // the editor keeps the old target when a rule switches from goto to hide, so this rule doesn't go to $unused
+    $hide = FormBlockLogic::factory()->for($third)->create([
+        'name' => 'Hide', 'conditions' => [$condition($first), $condition($second), $condition($first)], 'action' => 'hide', 'action_payload' => $unused->uuid,
+    ]);
+
+    $usedBy = fn (FormBlockLogic $rule, FormBlock $block, array $as) => ['id' => $rule->id, 'name' => $rule->name, 'block' => $block->uuid, 'as' => $as];
+
+    $response = $this->actingAs($form->user)
+        ->json('get', route('api.blocks.index', $form->uuid))
+        ->assertOk();
+
+    expect($response->json('*.used_by'))->toBe([
+        [$usedBy($jump, $second, ['source']), $usedBy($hide, $third, ['source'])],
+        [$usedBy($hide, $third, ['source'])],
+        [$usedBy($skip, $first, ['source', 'target']), $usedBy($jump, $second, ['target'])],
+        [],
+    ]);
+});