diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dc3c0b4..93b848e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ Lines marked **Upgrade step** need you to act after updating. - The forms API (`GET /api/forms`) returns when each form last got a submission (`latest_submission_at`) and can sort by it, newest first. Sorting now works only by `name`, `created_at`, `updated_at` or `latest_submission_at`. Other sort values that worked before, like `published_at`, `id` or `uuid`, now give the default order. A dashboard link with an unknown filter or sort now shows your forms. Before, it showed no forms at all. (#242) - Search your forms by name, also without accents ("muller" finds "Müller"), and press "/" to jump to the search. The form list has status tabs with counts and sorts by latest submission, last edit, newest or name. Your choice stays in the address, so Back undoes it and a link shows the same list. Dashboard links with the old `?filter=` show all forms. (#248) - The form list shows when each form last got a submission, with a blue dot for the last 24 hours, and now opens sorted by latest submission. Its date column shows the date the list is sorted by. (#250) +- The API can copy a question (`POST /api/blocks/{block}/duplicate`) with its text, settings, answers and Required, but not its rules or its Identifier, so links that fill in answers and webhooks can't mix the two up. The copy goes right after the original. (#262) ### Changed diff --git a/app/Http/Controllers/Api/FormBlockController.php b/app/Http/Controllers/Api/FormBlockController.php index 96def200..d44bb831 100644 --- a/app/Http/Controllers/Api/FormBlockController.php +++ b/app/Http/Controllers/Api/FormBlockController.php @@ -75,6 +75,20 @@ public function update(FormBlockUpdateRequest $request, FormBlock $block) return response()->json($block, 200); } + /** + * Duplicate a form block + * + * This endpoint copies a form block with its text, type, settings and interactions, but not its logic rules or its `title` (the Identifier). The copy goes right after the original, and later blocks move down one. A group can't be duplicated. + */ + public function duplicate(FormBlock $block) + { + $this->authorize('update', $block); + + abort_if($block->type === FormBlockType::group, 422, 'A group can\'t be duplicated, only a question.'); + + return response()->json($block->duplicate()->fresh(), 201); + } + /** * Delete a form block * diff --git a/app/Models/FormBlock.php b/app/Models/FormBlock.php index 90413b61..88ba99a4 100644 --- a/app/Models/FormBlock.php +++ b/app/Models/FormBlock.php @@ -5,6 +5,7 @@ use App\Scopes\Sequence; use Illuminate\Support\Str; use App\Enums\FormBlockType; +use Illuminate\Support\Facades\DB; use App\Models\FormBlockLogic; use App\Enums\FormBlockInteractionType; use Illuminate\Database\Eloquent\Factories\HasFactory; @@ -162,6 +163,30 @@ public function updateInteractionSequence(array $sequence) } } + public function duplicate(): FormBlock + { + return DB::transaction(function () { + // duplicates at the same time wait here, else they renumber over each other or deadlock. + // A write, not lockForUpdate: SQLite ignores that and fails a read that turns into a write + $this->form()->touch(); + + $blocks = $this->form->formBlocks()->get(); + + // the Identifier names the answer in prefill links and webhooks, so the copy has none + $copy = $this->form->formBlocks()->create([...$this->only(self::TEMPLATE_ATTRIBUTES), 'title' => null]); + + $this->formBlockInteractions->each( + fn ($interaction) => $copy->formBlockInteractions()->create($interaction->only(FormBlockInteraction::TEMPLATE_ATTRIBUTES)) + ); + + // renumber the whole form, as older forms can have gaps and two blocks on one position + $blocks->splice($blocks->search(fn ($block) => $block->is($this)) + 1, 0, [$copy]); + $blocks->each(fn ($block, $position) => $block->update(['sequence' => $position])); + + return $copy; + }); + } + public function toTemplate() { $attributes = $this->only(self::TEMPLATE_ATTRIBUTES); diff --git a/routes/api.php b/routes/api.php index 9c48b501..4fa24513 100644 --- a/routes/api.php +++ b/routes/api.php @@ -97,6 +97,7 @@ $router->post('{form}/blocks', [FormBlockController::class, 'create'])->name('api.blocks.create'); $router->post('blocks/{block}', [FormBlockController::class, 'update'])->whereNumber('block')->name('api.blocks.update'); $router->delete('blocks/{block}', [FormBlockController::class, 'delete'])->whereNumber('block')->name('api.blocks.delete'); + $router->post('blocks/{block}/duplicate', [FormBlockController::class, 'duplicate'])->whereNumber('block')->name('api.blocks.duplicate'); // Interaction API Routes $router->post('{block}/interactions', [FormBlockInteractionController::class, 'create'])->whereNumber('block')->name('api.interactions.create'); diff --git a/tests/Feature/CrossTeamAccessTest.php b/tests/Feature/CrossTeamAccessTest.php index 3e18b58e..20f840cf 100644 --- a/tests/Feature/CrossTeamAccessTest.php +++ b/tests/Feature/CrossTeamAccessTest.php @@ -88,6 +88,7 @@ 'create block' => ['post', fn () => route('api.blocks.create', $this->form), [], 403], 'update block' => ['post', fn () => route('api.blocks.update', $this->block), ['message' => 'Hijacked'], 403], 'delete block' => ['delete', fn () => route('api.blocks.delete', $this->block), [], 403], + 'duplicate block' => ['post', fn () => route('api.blocks.duplicate', $this->block), [], 403], 'sort blocks' => ['post', fn () => route('api.blocks.sequence', $this->form), ['sequence' => []], 403], // interactions diff --git a/tests/Feature/DuplicateBlockTest.php b/tests/Feature/DuplicateBlockTest.php new file mode 100644 index 00000000..ae4a8b55 --- /dev/null +++ b/tests/Feature/DuplicateBlockTest.php @@ -0,0 +1,144 @@ +form = Form::factory()->create(); + + // positions with gaps, as deleting a block leaves them + $this->first = FormBlock::factory()->for($this->form)->create(['sequence' => 0]); + $this->block = FormBlock::factory()->for($this->form)->create([ + 'sequence' => 4, + 'type' => FormBlockType::radio, + 'title' => 'Pick one', + 'message' => 'Which color do you like?', + 'options' => ['randomize' => true], + 'is_required' => true, + 'is_disabled' => false, + ]); + $this->last = FormBlock::factory()->for($this->form)->create(['sequence' => 9]); + + FormBlockInteraction::factory()->for($this->block)->create(['label' => 'Red', 'sequence' => 0]); + FormBlockInteraction::factory()->for($this->block)->customResponse()->create(['label' => 'Other', 'sequence' => 1, 'options' => ['max_chars' => 20]]); + FormBlockLogic::factory()->for($this->block)->create(); + + $this->order = fn () => $this->form->formBlocks()->pluck('uuid')->all(); +}); + +test('duplicating a question copies it right after the original, without its rules', function () { + $response = $this->actingAs($this->form->user) + ->json('post', route('api.blocks.duplicate', $this->block)) + ->assertCreated() + ->assertJson([ + 'form_id' => $this->form->id, + 'type' => 'radio', + // the Identifier names the answer in prefill links and webhooks, so the copy has none + 'title' => null, + 'message' => 'Which color do you like?', + 'options' => ['randomize' => true], + 'is_required' => true, + 'is_disabled' => false, + 'parent_block' => null, + 'logics' => [], + ]); + + $copy = FormBlock::where('uuid', $response->json('uuid'))->firstOrFail(); + + expect($copy->id)->not->toBe($this->block->id) + ->and($this->block->fresh()->title)->toBe('Pick one') + ->and(($this->order)())->toBe([$this->first->uuid, $this->block->uuid, $copy->uuid, $this->last->uuid]) + ->and($copy->formBlockLogics)->toHaveCount(0) + ->and($this->block->fresh()->formBlockLogics)->toHaveCount(1); + + // the answers are copies with their own ids, in the same order + $answers = fn (FormBlock $block) => $block->formBlockInteractions + ->map->only(['type', 'name', 'label', 'options', 'sequence', 'is_editable', 'is_disabled']) + ->all(); + + expect($answers($copy))->toBe($answers($this->block->fresh())) + ->and($copy->formBlockInteractions->pluck('uuid')->intersect($this->block->formBlockInteractions->pluck('uuid')))->toBeEmpty() + ->and($response->json('interactions.*.label'))->toBe(['Red', 'Other']) + ->and($response->json('interactions.1.type'))->toBe(FormBlockInteractionType::button->value); +}); + +test('the copy goes right after the original when older questions share a position', function () { + // creating a block after a delete can give two blocks the same position + $twin = FormBlock::factory()->for($this->form)->create(['sequence' => 4]); + + $response = $this->actingAs($this->form->user) + ->json('post', route('api.blocks.duplicate', $this->block)) + ->assertCreated(); + + $order = ($this->order)(); + $sequences = $this->form->formBlocks()->pluck('sequence')->all(); + + expect($order[array_search($this->block->uuid, $order) + 1])->toBe($response->json('uuid')) + ->and($order)->toContain($twin->uuid) + ->and($sequences)->toBe(range(0, 4)); +}); + +test('a question in a group is copied into the same group', function () { + $group = FormBlock::factory()->for($this->form)->create(['sequence' => 10, 'type' => FormBlockType::group]); + $child = FormBlock::factory()->for($this->form)->create(['sequence' => 11, 'parent_block' => $group->uuid]); + $nextChild = FormBlock::factory()->for($this->form)->create(['sequence' => 12, 'parent_block' => $group->uuid]); + + $response = $this->actingAs($this->form->user) + ->json('post', route('api.blocks.duplicate', $child)) + ->assertCreated() + ->assertJson(['parent_block' => $group->uuid]); + + expect(array_slice(($this->order)(), -4))->toBe([$group->uuid, $child->uuid, $response->json('uuid'), $nextChild->uuid]); +}); + +test('a group can not be duplicated', function () { + $group = FormBlock::factory()->for($this->form)->create(['sequence' => 10, 'type' => FormBlockType::group]); + $before = $this->form->formBlocks()->get()->toArray(); + + $this->actingAs($this->form->user) + ->json('post', route('api.blocks.duplicate', $group)) + ->assertUnprocessable(); + + expect($this->form->formBlocks()->get()->toArray())->toBe($before); +}); + +test('a failed duplicate leaves the form unchanged', function () { + $before = [ + $this->form->formBlocks()->get()->toArray(), + FormBlockInteraction::count(), + FormBlockLogic::count(), + ]; + + // the second answer can't be copied + FormBlockInteraction::creating(function ($interaction) { + if ($interaction->label === 'Other') { + throw new RuntimeException('disk full'); + } + }); + + $this->actingAs($this->form->user) + ->json('post', route('api.blocks.duplicate', $this->block)) + ->assertServerError(); + + expect([ + $this->form->formBlocks()->get()->toArray(), + FormBlockInteraction::count(), + FormBlockLogic::count(), + ])->toBe($before); +}); + +test('a duplicate writes to the form first, so duplicates at the same time keep one position each', function () { + DB::enableQueryLog(); + + $this->block->duplicate(); + + expect(DB::getQueryLog()[0]['query'])->toMatch('/^update [`"]forms[`"] set /'); +}); diff --git a/tests/Feature/FormTemplateTest.php b/tests/Feature/FormTemplateTest.php index 28a7b8db..7a182a69 100644 --- a/tests/Feature/FormTemplateTest.php +++ b/tests/Feature/FormTemplateTest.php @@ -104,10 +104,12 @@ 'type' => FormBlockType::group, ]); + // its own position, else the database may list it before the group FormBlock::factory()->create([ 'form_id' => $form->id, 'type' => FormBlockType::none, 'parent_block' => $groupBlock->uuid, + 'sequence' => 1, ]); $importTemplateString = $this->actingAs($form->user)