From 8bf711c4170079993ba1d25b919eb5e3e244ec6d Mon Sep 17 00:00:00 2001 From: Kevin Papst Date: Wed, 17 Jun 2026 10:20:50 +0200 Subject: [PATCH] Some random tests and fixes (#6000) * added tests for form models * apply form theme only to the own custom form type * change route order for improves matching --- config/routes.yaml | 50 +++---- src/Form/Model/Configuration.php | 4 +- src/Form/Model/MultiUserTimesheet.php | 4 +- src/Form/Model/TotpActivation.php | 2 +- .../system-configuration/index.html.twig | 2 +- .../system-configuration/section.html.twig | 2 +- tests/Form/Model/ConfigurationTest.php | 125 ++++++++++++++++++ tests/Form/Model/MultiUserTimesheetTest.php | 122 +++++++++++++++++ tests/Form/Model/SystemConfigurationTest.php | 1 - tests/Form/Model/TotpActivationTest.php | 34 +++++ tests/Form/Model/UserContractModelTest.php | 89 +++++++++++++ 11 files changed, 403 insertions(+), 32 deletions(-) create mode 100644 tests/Form/Model/ConfigurationTest.php create mode 100644 tests/Form/Model/MultiUserTimesheetTest.php create mode 100644 tests/Form/Model/TotpActivationTest.php create mode 100644 tests/Form/Model/UserContractModelTest.php diff --git a/config/routes.yaml b/config/routes.yaml index 823b21d3..d9581708 100644 --- a/config/routes.yaml +++ b/config/routes.yaml @@ -1,3 +1,28 @@ +home: + path: / + defaults: + _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction + route: homepage + permanent: true + +# SF DefaultAuthenticationSuccessHandler uses /login as default target +loginFallback: + path: /login + defaults: + _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction + _locale: '%locale%' + route: login + permanent: true + +# SF LogoutListener uses /logout as default target +logoutFallback: + path: /logout + defaults: + _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction + _locale: '%locale%' + route: logout + permanent: true + controllers: resource: ../src/Controller/ type: attribute @@ -31,13 +56,6 @@ kernel: resource: ../src/Kernel.php type: attribute -home: - path: / - defaults: - _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction - route: homepage - permanent: true - homeLocale: path: /{_locale} defaults: @@ -46,24 +64,6 @@ homeLocale: route: homepage permanent: true -# SF DefaultAuthenticationSuccessHandler uses /login as default target -loginFallback: - path: /login - defaults: - _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction - _locale: '%locale%' - route: login - permanent: true - -# SF LogoutListener uses /logout as default target -logoutFallback: - path: /logout - defaults: - _controller: Symfony\Bundle\FrameworkBundle\Controller\RedirectController::redirectAction - _locale: '%locale%' - route: logout - permanent: true - 2fa_login: path: /{_locale}/auth/2fa defaults: diff --git a/src/Form/Model/Configuration.php b/src/Form/Model/Configuration.php index 3635f83b..a316edce 100644 --- a/src/Form/Model/Configuration.php +++ b/src/Form/Model/Configuration.php @@ -156,8 +156,10 @@ final class Configuration return $this->formTheme; } - public function setFormTheme(string $formTheme): void + public function setFormTheme(string $formTheme): Configuration { $this->formTheme = $formTheme; + + return $this; } } diff --git a/src/Form/Model/MultiUserTimesheet.php b/src/Form/Model/MultiUserTimesheet.php index f3dbc804..bc2e60e7 100644 --- a/src/Form/Model/MultiUserTimesheet.php +++ b/src/Form/Model/MultiUserTimesheet.php @@ -51,7 +51,7 @@ final class MultiUserTimesheet extends Timesheet public function removeUser(User $user): void { if ($this->users->contains($user)) { - $this->users->remove($user); + $this->users->removeElement($user); } } @@ -71,7 +71,7 @@ final class MultiUserTimesheet extends Timesheet public function removeTeam(Team $team): void { if ($this->teams->contains($team)) { - $this->teams->remove($team); + $this->teams->removeElement($team); } } } diff --git a/src/Form/Model/TotpActivation.php b/src/Form/Model/TotpActivation.php index 7d0e22ea..b14a4f27 100644 --- a/src/Form/Model/TotpActivation.php +++ b/src/Form/Model/TotpActivation.php @@ -15,7 +15,7 @@ final class TotpActivation { private ?string $code = null; - public function __construct(private User $user) + public function __construct(private readonly User $user) { } diff --git a/templates/system-configuration/index.html.twig b/templates/system-configuration/index.html.twig index 43e7c9f0..1d0edc13 100644 --- a/templates/system-configuration/index.html.twig +++ b/templates/system-configuration/index.html.twig @@ -39,7 +39,7 @@ {% block form_body %} {% for pref in form.children.configuration %} {% if pref.vars.data.formTheme is not null %} - {% form_theme form pref.vars.data.formTheme %} + {% form_theme pref pref.vars.data.formTheme %} {% endif %} {% endfor %} {% for pref in form.children.configuration %} diff --git a/templates/system-configuration/section.html.twig b/templates/system-configuration/section.html.twig index f610c41c..9ef6296b 100644 --- a/templates/system-configuration/section.html.twig +++ b/templates/system-configuration/section.html.twig @@ -13,7 +13,7 @@ {% block form_body %} {% for pref in form.children.configuration %} {% if pref.vars.data.formTheme is not null %} - {% form_theme form pref.vars.data.formTheme %} + {% form_theme pref pref.vars.data.formTheme %} {% endif %} {% endfor %} {% for pref in form.children.configuration %} diff --git a/tests/Form/Model/ConfigurationTest.php b/tests/Form/Model/ConfigurationTest.php new file mode 100644 index 00000000..2d0d59bd --- /dev/null +++ b/tests/Form/Model/ConfigurationTest.php @@ -0,0 +1,125 @@ +getName()); + self::assertNull($sut->getLabel()); + self::assertSame('messages', $sut->getTranslationDomain()); + self::assertNull($sut->getValue()); + self::assertNull($sut->getType()); + self::assertSame([], $sut->getOptions()); + self::assertTrue($sut->isEnabled()); + self::assertTrue($sut->isRequired()); + self::assertNull($sut->getFormTheme()); + self::assertSame([], $sut->getConstraints()); + } + + public function testFluentSetterAndGetter(): void + { + $sut = new Configuration('foo'); + $constraints = [new NotBlank()]; + $options = ['attr' => ['data-test' => 'value'], 'empty_data' => 0]; + + $result = $sut + ->setLabel('Foo label') + ->setTranslationDomain('admin') + ->setType('custom-type') + ->setValue(42.5) + ->setOptions($options) + ->setConstraints($constraints) + ->setEnabled(false) + ->setRequired(false) + ->setFormTheme('@MyBundle/form/test.html.twig'); + + self::assertSame($sut, $result); + self::assertSame('Foo label', $sut->getLabel()); + self::assertSame('admin', $sut->getTranslationDomain()); + self::assertSame('custom-type', $sut->getType()); + self::assertSame(42.5, $sut->getValue()); + self::assertSame($options, $sut->getOptions()); + self::assertSame($constraints, $sut->getConstraints()); + self::assertFalse($sut->isEnabled()); + self::assertFalse($sut->isRequired()); + self::assertSame('@MyBundle/form/test.html.twig', $sut->getFormTheme()); + } + + #[DataProvider('provideScalarValues')] + public function testSetValueKeepsOriginalTypeForNonBooleanField(string|int|null|bool|float $value): void + { + $sut = new Configuration('foo'); + + self::assertSame($sut, $sut->setValue($value)); + self::assertSame($value, $sut->getValue()); + } + + /** + * @return iterable + */ + public static function provideBooleanTypeValues(): iterable + { + yield 'checkbox true string' => [CheckboxType::class, '1', true]; + yield 'checkbox zero string' => [CheckboxType::class, '0', false]; + yield 'checkbox integer zero' => [CheckboxType::class, 0, false]; + yield 'checkbox integer one' => [CheckboxType::class, 1, true]; + yield 'checkbox null' => [CheckboxType::class, null, false]; + yield 'checkbox empty string' => [CheckboxType::class, '', false]; + yield 'yes no text value' => [YesNoType::class, 'yes', true]; + yield 'yes no false bool' => [YesNoType::class, false, false]; + yield 'yes no float' => [YesNoType::class, 3.14, true]; + } + + #[DataProvider('provideBooleanTypeValues')] + public function testSetValueCastsToBoolForBooleanTypes(string $type, string|int|null|bool|float $value, bool $expected): void + { + $sut = new Configuration('foo'); + + self::assertSame($sut, $sut->setType($type)); + self::assertSame($sut, $sut->setValue($value)); + self::assertSame($expected, $sut->getValue()); + } + + public function testChangingTypeAfterSettingValueDoesNotRetroactivelyCastValue(): void + { + $sut = new Configuration('foo'); + + $sut->setValue('1'); + $sut->setType(CheckboxType::class); + + self::assertSame('1', $sut->getValue()); + } + + /** + * @return iterable + */ + public static function provideScalarValues(): iterable + { + yield 'null' => [null]; + yield 'string' => ['hello world']; + yield 'integer' => [123]; + yield 'float' => [123.45]; + yield 'true' => [true]; + yield 'false' => [false]; + } +} diff --git a/tests/Form/Model/MultiUserTimesheetTest.php b/tests/Form/Model/MultiUserTimesheetTest.php new file mode 100644 index 00000000..b2bba260 --- /dev/null +++ b/tests/Form/Model/MultiUserTimesheetTest.php @@ -0,0 +1,122 @@ +getTeams()); + self::assertEmpty($sut->getUsers()); + } + + public function testAddAndRemoveUser(): void + { + $sut = new MultiUserTimesheet(); + $user = $this->createUser('alpha'); + + $sut->addUser($user); + + self::assertCount(1, $sut->getUsers()); + self::assertSame($user, $sut->getUsers()->first()); + + $sut->removeUser($user); + + self::assertCount(0, $sut->getUsers()); + } + + public function testRemovingUnknownUserDoesNothing(): void + { + $sut = new MultiUserTimesheet(); + $user = $this->createUser('alpha'); + + $sut->removeUser($user); + + self::assertCount(0, $sut->getUsers()); + } + + public function testRemovingUserOnlyRemovesOneDuplicateEntry(): void + { + $sut = new MultiUserTimesheet(); + $user = $this->createUser('alpha'); + + $sut->addUser($user); + $sut->addUser($user); + + self::assertCount(2, $sut->getUsers()); + + $sut->removeUser($user); + + self::assertCount(1, $sut->getUsers()); + self::assertSame($user, $sut->getUsers()->first()); + } + + public function testAddAndRemoveTeam(): void + { + $sut = new MultiUserTimesheet(); + $team = new Team('Team Alpha'); + + $sut->addTeam($team); + + self::assertCount(1, $sut->getTeams()); + self::assertSame($team, $sut->getTeams()->first()); + + $sut->removeTeam($team); + + self::assertCount(0, $sut->getTeams()); + } + + public function testRemovingUnknownTeamDoesNothing(): void + { + $sut = new MultiUserTimesheet(); + $team = new Team('Team Alpha'); + + $sut->removeTeam($team); + + self::assertCount(0, $sut->getTeams()); + } + + public function testRemovingTeamOnlyRemovesOneDuplicateEntry(): void + { + $sut = new MultiUserTimesheet(); + $team = new Team('Team Alpha'); + + $sut->addTeam($team); + $sut->addTeam($team); + + self::assertCount(2, $sut->getTeams()); + + $sut->removeTeam($team); + + self::assertCount(1, $sut->getTeams()); + self::assertSame($team, $sut->getTeams()->first()); + } + + private function createUser(string $username): User + { + $user = new User(); + $user->setUsername($username); + $user->setAlias($username); + $user->setEmail($username . '@example.com'); + + return $user; + } +} diff --git a/tests/Form/Model/SystemConfigurationTest.php b/tests/Form/Model/SystemConfigurationTest.php index c70a41a0..e15188c1 100644 --- a/tests/Form/Model/SystemConfigurationTest.php +++ b/tests/Form/Model/SystemConfigurationTest.php @@ -14,7 +14,6 @@ use App\Form\Model\SystemConfiguration; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; -#[CoversClass(Configuration::class)] #[CoversClass(SystemConfiguration::class)] class SystemConfigurationTest extends TestCase { diff --git a/tests/Form/Model/TotpActivationTest.php b/tests/Form/Model/TotpActivationTest.php new file mode 100644 index 00000000..9df96629 --- /dev/null +++ b/tests/Form/Model/TotpActivationTest.php @@ -0,0 +1,34 @@ +getUser()); + self::assertNull($sut->getCode()); + $sut->setCode(''); + self::assertEquals('', $sut->getCode()); + $sut->setCode('jztfztfjzfjhgfjhgfjhgfjtzfiuzgbljv'); + self::assertEquals('jztfztfjzfjhgfjhgfjhgfjtzfiuzgbljv', $sut->getCode()); + $sut->setCode(null); + self::assertNull($sut->getCode()); + } +} diff --git a/tests/Form/Model/UserContractModelTest.php b/tests/Form/Model/UserContractModelTest.php new file mode 100644 index 00000000..b8af2027 --- /dev/null +++ b/tests/Form/Model/UserContractModelTest.php @@ -0,0 +1,89 @@ +__isset('alias')); + self::assertTrue($sut->__isset('unknownPreference')); + } + + public function testSetAndGetExistingUserPropertyUsesUserMethods(): void + { + $user = new User(); + $sut = new UserContractModel($user); + + $sut->__set('alias', 'contract-user'); + + self::assertSame('contract-user', $sut->__get('alias')); + self::assertSame('contract-user', $user->getAlias()); + } + + public function testSetAndGetExistingPreferenceBackedMethodUsesUserMethod(): void + { + $user = new User(); + $sut = new UserContractModel($user); + + $sut->__set('workContractMode', 'default'); + + self::assertSame('default', $sut->__get('workContractMode')); + self::assertSame('default', $user->getWorkContractMode()); + } + + public function testSetAndGetUnknownPropertyUsesPreferenceFallback(): void + { + $user = new User(); + $sut = new UserContractModel($user); + + $sut->__set('customContractField', 'weekly'); + + self::assertSame('weekly', $sut->__get('customContractField')); + self::assertSame('weekly', $user->getPreferenceValue('customContractField')); + } + + public function testSetUnknownPropertyAllowsNullPreferenceValue(): void + { + $user = new User(); + $sut = new UserContractModel($user); + + $sut->__set('customContractField', null); + + self::assertNull($sut->__get('customContractField')); + self::assertNull($user->getPreferenceValue('customContractField')); + } + + public function testGetUnknownPropertyWithoutPreferenceReturnsNull(): void + { + $sut = new UserContractModel(new User()); + + self::assertNull($sut->__get('missingPreference')); + } + + public function testSetUnknownPropertyRejectsNonScalarValues(): void + { + $sut = new UserContractModel(new User()); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid value passed'); + + $sut->__set('customContractField', ['invalid']); + } +}