Skip to content

Commit 2e17cb6

Browse files
authored
Merge pull request mautic#15564 from aarohiprasad/prevent-duplicate-project-names
Prevent duplicate project names
2 parents 2d61136 + 0d4769f commit 2e17cb6

File tree

11 files changed

+290
-6
lines changed

11 files changed

+290
-6
lines changed
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Mautic\CoreBundle\Form\Extension;
6+
7+
use Symfony\Component\Form\AbstractTypeExtension;
8+
use Symfony\Component\Form\Extension\Core\Type\FormType;
9+
use Symfony\Component\Form\FormBuilderInterface;
10+
use Symfony\Component\Form\FormEvent;
11+
use Symfony\Component\Form\FormEvents;
12+
use Symfony\Component\OptionsResolver\OptionsResolver;
13+
14+
class NormalizeFormExtension extends AbstractTypeExtension
15+
{
16+
/**
17+
* @param array<mixed> $options
18+
*/
19+
public function buildForm(FormBuilderInterface $builder, array $options): void
20+
{
21+
if (!$options['normalize_whitespaces']) {
22+
return;
23+
}
24+
25+
$builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
26+
$data = $event->getData();
27+
28+
if (!\is_string($data)) {
29+
return;
30+
}
31+
32+
$event->setData(preg_replace('/\s+/', ' ', $data));
33+
});
34+
}
35+
36+
public function configureOptions(OptionsResolver $resolver): void
37+
{
38+
$resolver->setDefaults([
39+
'normalize_whitespaces' => false,
40+
]);
41+
}
42+
43+
/**
44+
* @return iterable<class-string>
45+
*/
46+
public static function getExtendedTypes(): iterable
47+
{
48+
return [FormType::class];
49+
}
50+
}

app/bundles/ProjectBundle/Controller/AjaxController.php

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,8 @@ public function addProjectsAction(Request $request, ProjectModel $projectModel,
5959
foreach ($newProjectNames as $projectName) {
6060
$project = new Project();
6161
$project->setName($projectName);
62-
$projectModel->saveEntity($project);
63-
$existingProjectIds[] = $project->getId();
62+
63+
$existingProjectIds[] = $this->createProjectIfNotExists(trim($projectName), $projectModel, $projectRepository);
6464
}
6565
}
6666

@@ -75,4 +75,17 @@ public function addProjectsAction(Request $request, ProjectModel $projectModel,
7575

7676
return $this->sendJsonResponse(['projects' => $projectOptions]);
7777
}
78+
79+
private function createProjectIfNotExists(string $name, ProjectModel $projectModel, ProjectRepository $projectRepository): int
80+
{
81+
if ($project = $projectRepository->findOneBy(['name' => $name])) {
82+
return $project->getId();
83+
}
84+
85+
$project = new Project();
86+
$project->setName($name);
87+
$projectModel->saveEntity($project);
88+
89+
return $project->getId();
90+
}
7891
}

app/bundles/ProjectBundle/Entity/Project.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use Mautic\CoreBundle\Entity\FormEntity;
1313
use Mautic\CoreBundle\Entity\UuidInterface;
1414
use Mautic\CoreBundle\Entity\UuidTrait;
15+
use Mautic\ProjectBundle\Validator\Constraints\UniqueName;
1516
use Symfony\Component\Serializer\Annotation\Groups;
1617
use Symfony\Component\Validator\Constraints\NotBlank;
1718
use Symfony\Component\Validator\Mapping\ClassMetadata;
@@ -82,7 +83,7 @@ public static function loadMetadata(OrmClassMetadata $metadata): void
8283

8384
$builder->setTable(self::TABLE_NAME)
8485
->setCustomRepositoryClass(ProjectRepository::class)
85-
->addIndex(['name'], 'project_name');
86+
->addUniqueConstraint(['name'], 'unique_project_name');
8687

8788
$builder->addIdColumns();
8889

@@ -115,6 +116,7 @@ public static function loadValidatorMetadata(ClassMetadata $metadata): void
115116
'name',
116117
new NotBlank(['message' => 'mautic.core.name.required'])
117118
);
119+
$metadata->addConstraint(new UniqueName());
118120
}
119121

120122
public function getId(): ?int

app/bundles/ProjectBundle/Entity/ProjectRepository.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,4 +22,20 @@ public function getTableAlias(): string
2222
{
2323
return 'p';
2424
}
25+
26+
public function checkProjectNameExists(string $name, ?int $ignoredId = null): bool
27+
{
28+
$q = $this->createQueryBuilder($this->getTableAlias());
29+
$q->select('1');
30+
$q->where($this->getTableAlias().'.name = :name');
31+
$q->setParameter('name', $name);
32+
$q->setMaxResults(1);
33+
34+
if (null !== $ignoredId) {
35+
$q->andWhere($q->expr()->neq($this->getTableAlias().'.id', ':ignoredId'));
36+
$q->setParameter('ignoredId', $ignoredId);
37+
}
38+
39+
return !empty($q->getQuery()->getResult());
40+
}
2541
}

app/bundles/ProjectBundle/Form/Type/ProjectEntityType.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
2020
'name',
2121
TextType::class,
2222
[
23-
'label' => 'mautic.core.name',
24-
'label_attr' => ['class' => 'control-label'],
25-
'attr' => ['class' => 'form-control'],
23+
'label' => 'mautic.core.name',
24+
'label_attr' => ['class' => 'control-label'],
25+
'attr' => ['class' => 'form-control'],
26+
'normalize_whitespaces' => true,
2627
]
2728
);
2829

app/bundles/ProjectBundle/Tests/Functional/Controller/AjaxControllerTest.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,4 +60,56 @@ static function (string $projectName) use ($projectModel) {
6060
$payload['projects']
6161
);
6262
}
63+
64+
public function testCreatingDuplicateProject(): void
65+
{
66+
$projectModel = self::getContainer()->get('mautic.project.model.project');
67+
\assert($projectModel instanceof ProjectModel);
68+
69+
$this->assertCount(
70+
0,
71+
$this->em->getRepository(Project::class)->findAll(),
72+
'There should be no projects at the beginning of the test.'
73+
);
74+
75+
$project = new Project();
76+
$project->setName('Yellow Project');
77+
$projectModel->saveEntity($project);
78+
79+
$this->assertCount(
80+
1,
81+
$this->em->getRepository(Project::class)->findAll(),
82+
'There should be 1 project after creating the first one.'
83+
);
84+
85+
$this->client->request(
86+
'POST',
87+
'/s/ajax?action=project:addProjects',
88+
[
89+
'newProjectNames' => json_encode(['yellow project']),
90+
'existingProjectIds' => json_encode([$project->getId()]),
91+
]
92+
);
93+
94+
$this->assertCount(
95+
1,
96+
$this->em->getRepository(Project::class)->findAll(),
97+
'There should be still 1 project after an attempt to create a duplicate project.'
98+
);
99+
100+
$this->client->request(
101+
'POST',
102+
'/s/ajax?action=project:addProjects',
103+
[
104+
'newProjectNames' => json_encode(['green project']),
105+
'existingProjectIds' => json_encode([$project->getId()]),
106+
]
107+
);
108+
109+
$this->assertCount(
110+
2,
111+
$this->em->getRepository(Project::class)->findAll(),
112+
'There should be 2 projects after an attempt to create a unique project.'
113+
);
114+
}
63115
}
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Mautic\ProjectBundle\Tests\Functional\Validator;
6+
7+
use Mautic\CoreBundle\Test\MauticMysqlTestCase;
8+
use Mautic\ProjectBundle\Entity\Project;
9+
use Symfony\Component\HttpFoundation\Request;
10+
11+
final class UniqueNameValidatorFunctionalTest extends MauticMysqlTestCase
12+
{
13+
public function testDuplicateProjectName(): void
14+
{
15+
$project = new Project();
16+
$project->setName('qwerty');
17+
$this->em->persist($project);
18+
$this->em->flush();
19+
20+
$this->assertCount(1, $this->em->getRepository(Project::class)->findBy(['name' => $project->getName()]));
21+
22+
$crawler = $this->client->request(Request::METHOD_GET, '/s/projects/new');
23+
$buttonCrawler = $crawler->selectButton('Save & Close');
24+
$form = $buttonCrawler->form();
25+
$form['project_entity[name]']->setValue('QWERTY');
26+
$this->client->submit($form);
27+
$this->assertTrue($this->client->getResponse()->isOk());
28+
29+
$this->assertStringContainsString(
30+
'A project with this name already exists.',
31+
$this->client->getResponse()->getContent()
32+
);
33+
34+
$this->assertCount(1, $this->em->getRepository(Project::class)->findBy(['name' => $project->getName()]));
35+
}
36+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
project.name.already.exists="A project with this name already exists."
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Mautic\ProjectBundle\Validator\Constraints;
6+
7+
use Symfony\Component\Validator\Constraint;
8+
9+
final class UniqueName extends Constraint
10+
{
11+
public string $message = 'project.name.already.exists';
12+
13+
public function getTargets(): string
14+
{
15+
return self::CLASS_CONSTRAINT;
16+
}
17+
}
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Mautic\ProjectBundle\Validator\Constraints;
6+
7+
use Mautic\ProjectBundle\Entity\Project;
8+
use Mautic\ProjectBundle\Entity\ProjectRepository;
9+
use Symfony\Component\Validator\Constraint;
10+
use Symfony\Component\Validator\ConstraintValidator;
11+
use Symfony\Component\Validator\Exception\UnexpectedTypeException;
12+
13+
final class UniqueNameValidator extends ConstraintValidator
14+
{
15+
public function __construct(
16+
private ProjectRepository $projectRepository,
17+
) {
18+
}
19+
20+
public function validate(mixed $project, Constraint $constraint): void
21+
{
22+
if (!$project instanceof Project) {
23+
throw new UnexpectedTypeException($project, Project::class);
24+
}
25+
26+
if (!$constraint instanceof UniqueName) {
27+
throw new UnexpectedTypeException($constraint, UniqueName::class);
28+
}
29+
30+
$projectName = $project->getName();
31+
32+
if (!$projectName) {
33+
return;
34+
}
35+
36+
$ignoredId = $project->isNew() ? null : $project->getId();
37+
38+
if ($this->projectRepository->checkProjectNameExists($projectName, $ignoredId)) {
39+
$this->context->buildViolation($constraint->message)
40+
->atPath('name')
41+
->addViolation();
42+
}
43+
}
44+
}

0 commit comments

Comments
 (0)