Skip to content

Commit

Permalink
Fix copy block and duplicate of form fields (#377)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexander-schranz authored Jun 6, 2024
1 parent bf13306 commit 1ada3e6
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 56 deletions.
34 changes: 30 additions & 4 deletions Manager/FormManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ public function updateReceivers(array $data, FormTranslation $translation): void
{
$receiversRepository = $this->entityManager->getRepository(FormTranslationReceiver::class);
$receiverDatas = self::getValue($data, 'receivers', []);
\assert(\is_array($receiverDatas), 'Receivers must be an array.');

// Remove old receivers.
$oldReceivers = $receiversRepository->findBy(['formTranslation' => $translation]);
Expand Down Expand Up @@ -306,14 +307,37 @@ public function updateReceivers(array $data, FormTranslation $translation): void
*/
protected function updateFields(array $data, Form $form, string $locale): void
{
$reservedKeys = \array_column(self::getValue($data, 'fields', []), 'key');
$fields = self::getValue($data, 'fields', []);
\assert(\is_array($fields), 'Fields must be an array.');

$existingIds = [];
$existingKeys = [];
foreach ($fields as $key => $fieldData) { // make id and keys unique when block get copied
if (\in_array($fieldData['id'] ?? null, $existingIds)) {
unset($fields[$key]['id']);
}
if (\in_array($fieldData['key'] ?? null, $existingKeys)) {
unset($fields[$key]['key']);
}

if (isset($fieldData['id'])) {
$existingIds[] = $fieldData['id'];
}

if (isset($fieldData['key'])) {
$existingKeys[] = $fieldData['key'];
}
}

$reservedKeys = \array_column($fields, 'key');

$counter = 0;

foreach (self::getValue($data, 'fields', []) as $fieldData) {
foreach ($fields as $fieldData) {
++$counter;
$fieldType = self::getValue($fieldData, 'type');
$fieldKey = self::getValue($fieldData, 'key');

$field = $form->getField($fieldKey);
$uniqueKey = $this->getUniqueKey($fieldType, $reservedKeys);

Expand All @@ -324,10 +348,12 @@ protected function updateFields(array $data, Form $form, string $locale): void
if (!$field) {
$field = new FormField();
$field->setKey($uniqueKey);
$reservedKeys[] = $uniqueKey;
} elseif ($field->getType() !== $fieldType || !$field->getKey()) {
$field->setKey($uniqueKey);
$reservedKeys[] = $uniqueKey;
}

if (!\in_array($field->getKey(), $reservedKeys)) {
$reservedKeys[] = $field->getKey();
}

$field->setOrder($counter);
Expand Down
2 changes: 1 addition & 1 deletion Tests/Application/.env
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
APP_ENV=test
DATABASE_URL=mysql://[email protected]:3306/su_form_test?serverVersion=5.7
DATABASE_URL=mysql://root:ChangeMe@127.0.0.1:3306/su_form_test?serverVersion=5.7
DATABASE_CHARSET=utf8mb4
DATABASE_COLLATE=utf8mb4_unicode_ci
73 changes: 73 additions & 0 deletions Tests/Functional/Controller/FormControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,79 @@ public function testPutFull(): void
$this->assertFullForm($response);
}

public function testPutDuplicatedField(): void
{
$form = $this->createFullForm();
$this->client->request(
'PUT',
'/admin/api/forms/' . $form->getId(),
[
'locale' => 'en',
'title' => 'Title',
'toEmail' => '[email protected]',
'fromEmail' => '[email protected]',
'fields' => [
[
'key' => 'email',
'type' => 'email',
'title' => 'Title',
'shortTitle' => 'Short Title',
'placeholder' => 'Placeholder',
'defaultValue' => 'Default Value',
'width' => 'full',
'required' => true,
],
[
'key' => 'email', // we are testing here if second email field is correctly interpreted as email1
'type' => 'email',
'title' => 'Title',
'shortTitle' => 'Short Title',
'placeholder' => 'Placeholder',
'defaultValue' => 'Default Value',
'width' => 'full',
'required' => true,
],
[
'key' => 'email', // we are testing here if third email field is correctly created as email2
'type' => 'email',
'title' => 'Title',
'shortTitle' => 'Short Title',
'placeholder' => 'Placeholder',
'defaultValue' => 'Default Value',
'width' => 'full',
'required' => true,
],
],
]
);

$this->assertHttpStatusCode(200, $this->client->getResponse());
$response = \json_decode($this->client->getResponse()->getContent(), true);

$respondedFields = [];
foreach ($response['fields'] ?? [] as $field) {
$respondedFields[] = [
'key' => $field['key'],
'type' => $field['type'],
];
}

$this->assertSame([
[
'key' => 'email',
'type' => 'email',
],
[
'key' => 'email1',
'type' => 'email',
],
[
'key' => 'email2',
'type' => 'email',
],
], $respondedFields);
}

public function testPutNotExist(): void
{
$this->client->request(
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"jackalope/jackalope-doctrine-dbal": "^1.3.2",
"jangregor/phpstan-prophecy": "^1.0",
"massive/search-bundle": "^2.0",
"php-cs-fixer/shim": "^3.57",
"php-cs-fixer/shim": "^3.0",
"phpspec/prophecy": "^1.14",
"phpstan/phpstan": "^1.0",
"phpstan/phpstan-doctrine": "^1.0",
Expand Down
50 changes: 0 additions & 50 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -1140,26 +1140,6 @@ parameters:
count: 1
path: Mail/MailerHelper.php

-
message: "#^Argument of an invalid type mixed supplied for foreach, only iterables are supported\\.$#"
count: 2
path: Manager/FormManager.php

-
message: "#^Cannot access offset 'email' on mixed\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Cannot access offset 'name' on mixed\\.$#"
count: 2
path: Manager/FormManager.php

-
message: "#^Cannot access offset 'type' on mixed\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Cannot call method getId\\(\\) on Sulu\\\\Bundle\\\\FormBundle\\\\Entity\\\\FormFieldTranslation\\|null\\.$#"
count: 1
Expand Down Expand Up @@ -1285,16 +1265,6 @@ parameters:
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$array of function array_column expects array, mixed given\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$data of static method Sulu\\\\Bundle\\\\FormBundle\\\\Manager\\\\FormManager\\:\\:getValue\\(\\) expects array, mixed given\\.$#"
count: 9
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$datetime of class DateTime constructor expects string, mixed given\\.$#"
count: 1
Expand All @@ -1305,11 +1275,6 @@ parameters:
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$email of method Sulu\\\\Bundle\\\\FormBundle\\\\Entity\\\\FormTranslationReceiver\\:\\:setEmail\\(\\) expects string, mixed given\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$id of method Sulu\\\\Bundle\\\\FormBundle\\\\Manager\\\\FormManager\\:\\:findById\\(\\) expects int, int\\|null given\\.$#"
count: 1
Expand All @@ -1325,11 +1290,6 @@ parameters:
count: 3
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$name of method Sulu\\\\Bundle\\\\FormBundle\\\\Entity\\\\FormTranslationReceiver\\:\\:setName\\(\\) expects string, mixed given\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$required of method Sulu\\\\Bundle\\\\FormBundle\\\\Entity\\\\FormField\\:\\:setRequired\\(\\) expects bool, mixed given\\.$#"
count: 1
Expand All @@ -1350,11 +1310,6 @@ parameters:
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$type of method Sulu\\\\Bundle\\\\FormBundle\\\\Entity\\\\FormTranslationReceiver\\:\\:setType\\(\\) expects string, mixed given\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#1 \\$type of method Sulu\\\\Bundle\\\\FormBundle\\\\Manager\\\\FormManager\\:\\:getUniqueKey\\(\\) expects string, mixed given\\.$#"
count: 1
Expand All @@ -1365,11 +1320,6 @@ parameters:
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#2 \\$array of function array_key_exists expects array, mixed given\\.$#"
count: 1
path: Manager/FormManager.php

-
message: "#^Parameter \\#2 \\$locale of class Sulu\\\\Bundle\\\\FormBundle\\\\Domain\\\\Event\\\\FormCreatedEvent constructor expects string, string\\|null given\\.$#"
count: 1
Expand Down

0 comments on commit 1ada3e6

Please sign in to comment.