From cef011d461f02178a6f5dd35e566c93b6c3b2c2d Mon Sep 17 00:00:00 2001 From: Kirill Hramov Date: Sat, 4 Jul 2026 13:06:21 +0300 Subject: [PATCH 1/2] Cleanup and optimize nullable method arguments, ensure strict type declarations across tests and core files. --- src/AggregateRoot.php | 2 ++ .../UseCase/OnAppInstall/Handler.php | 4 +--- src/Console/ApplicationSettingsListCommand.php | 6 +++--- ...SettingsItemRepositoryInterfaceContractTest.php | 2 +- .../UseCase/Update/HandlerTest.php | 14 +++----------- .../Doctrine/ContactPersonRepositoryTest.php | 4 +++- .../Entity/ApplicationInstallationTest.php | 3 +-- .../Services/DefaultSettingsInstallerTest.php | 4 ++-- .../Services/SettingsFetcherTest.php | 2 +- 9 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/AggregateRoot.php b/src/AggregateRoot.php index abb2be66..08724a4e 100644 --- a/src/AggregateRoot.php +++ b/src/AggregateRoot.php @@ -1,5 +1,7 @@ bitrix24AccountRepository->findByMemberId( $memberId, - Bitrix24AccountStatus::active, - null, - null + Bitrix24AccountStatus::active ); // Filter for master accounts only diff --git a/src/Console/ApplicationSettingsListCommand.php b/src/Console/ApplicationSettingsListCommand.php index 0f308421..7b4d84cc 100644 --- a/src/Console/ApplicationSettingsListCommand.php +++ b/src/Console/ApplicationSettingsListCommand.php @@ -130,13 +130,13 @@ protected function execute(InputInterface $input, OutputInterface $output): int $allSettings = $this->applicationSettingRepository->findAllForInstallation($installationId); if ($globalOnly || (null === $userId && null === $departmentId)) { - $settings = array_filter($allSettings, fn ($setting): bool => $setting->isGlobal()); + $settings = array_filter($allSettings, fn (\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $applicationSettingsItem): bool => $applicationSettingsItem->isGlobal()); $scope = 'Global'; } elseif (null !== $userId) { - $settings = array_filter($allSettings, fn ($setting): bool => $setting->isPersonal() && $setting->getB24UserId() === $userId); + $settings = array_filter($allSettings, fn (\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $applicationSettingsItem): bool => $applicationSettingsItem->isPersonal() && $applicationSettingsItem->getB24UserId() === $userId); $scope = sprintf('Personal (User ID: %d)', $userId); } else { - $settings = array_filter($allSettings, fn ($setting): bool => $setting->isDepartmental() && $setting->getB24DepartmentId() === $departmentId); + $settings = array_filter($allSettings, fn (\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $applicationSettingsItem): bool => $applicationSettingsItem->isDepartmental() && $applicationSettingsItem->getB24DepartmentId() === $departmentId); $scope = sprintf('Departmental (Department ID: %d)', $departmentId); } diff --git a/tests/Contract/ApplicationSettings/Infrastructure/ApplicationSettingsItemRepositoryInterfaceContractTest.php b/tests/Contract/ApplicationSettings/Infrastructure/ApplicationSettingsItemRepositoryInterfaceContractTest.php index 1dfdd08f..137cb531 100644 --- a/tests/Contract/ApplicationSettings/Infrastructure/ApplicationSettingsItemRepositoryInterfaceContractTest.php +++ b/tests/Contract/ApplicationSettings/Infrastructure/ApplicationSettingsItemRepositoryInterfaceContractTest.php @@ -374,7 +374,7 @@ public function testRepositoryHandlesDifferentScopes(): void $this->assertCount(3, $results); // Verify each scope is present - $values = array_map(fn($s): string => $s->getValue(), $results); + $values = array_map(fn(\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $applicationSettingsItem): string => $applicationSettingsItem->getValue(), $results); $this->assertContains('global', $values); $this->assertContains('personal', $values); $this->assertContains('departmental', $values); diff --git a/tests/Functional/ApplicationSettings/UseCase/Update/HandlerTest.php b/tests/Functional/ApplicationSettings/UseCase/Update/HandlerTest.php index 01faa279..778c0d0f 100644 --- a/tests/Functional/ApplicationSettings/UseCase/Update/HandlerTest.php +++ b/tests/Functional/ApplicationSettings/UseCase/Update/HandlerTest.php @@ -51,10 +51,7 @@ public function testCanUpdateExistingSetting(): void $uuidV7, 'update.test', 'initial_value', - false, - null, - null, - null + false ); $this->repository->save($applicationSettingsItem); EntityManagerFactory::get()->flush(); @@ -112,9 +109,7 @@ public function testCanUpdatePersonalSetting(): void 'personal.test', 'user_value', false, - 123, - null, - null + 123 ); $this->repository->save($applicationSettingsItem); EntityManagerFactory::get()->flush(); @@ -126,7 +121,6 @@ public function testCanUpdatePersonalSetting(): void key: 'personal.test', value: 'new_user_value', b24UserId: 123, - b24DepartmentId: null, changedByBitrix24UserId: 456 ); $this->handler->handle($updateCommand); @@ -157,8 +151,7 @@ public function testCanUpdateDepartmentalSetting(): void 'dept_value', false, null, - 456, - null + 456 ); $this->repository->save($applicationSettingsItem); EntityManagerFactory::get()->flush(); @@ -169,7 +162,6 @@ public function testCanUpdateDepartmentalSetting(): void applicationInstallationId: $uuidV7, key: 'dept.test', value: 'new_dept_value', - b24UserId: null, b24DepartmentId: 456, changedByBitrix24UserId: 789 ); diff --git a/tests/Functional/ContactPersons/Infrastructure/Doctrine/ContactPersonRepositoryTest.php b/tests/Functional/ContactPersons/Infrastructure/Doctrine/ContactPersonRepositoryTest.php index 50b34310..90b6fa0a 100644 --- a/tests/Functional/ContactPersons/Infrastructure/Doctrine/ContactPersonRepositoryTest.php +++ b/tests/Functional/ContactPersons/Infrastructure/Doctrine/ContactPersonRepositoryTest.php @@ -1,5 +1,7 @@ Date: Sat, 4 Jul 2026 18:34:38 +0300 Subject: [PATCH 2/2] fix feature/issue-111-install-contact-person-mobile-phone-validation --- .../UseCase/InstallContactPerson/Handler.php | 23 ++++++++++--------- .../InstallContactPerson/HandlerTest.php | 16 +++++++++++-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/ApplicationInstallations/UseCase/InstallContactPerson/Handler.php b/src/ApplicationInstallations/UseCase/InstallContactPerson/Handler.php index 7f2e4666..2e09a908 100644 --- a/src/ApplicationInstallations/UseCase/InstallContactPerson/Handler.php +++ b/src/ApplicationInstallations/UseCase/InstallContactPerson/Handler.php @@ -37,18 +37,19 @@ public function handle(Command $command): void ]); $createdContactPersonId = ''; - - try { - if (null !== $command->mobilePhoneNumber) { - try { - $this->guardMobilePhoneNumber($command->mobilePhoneNumber); - } catch (InvalidArgumentException) { - // Ошибка уже залогирована внутри гарда. - // Прерываем создание контакта, но не останавливаем установку приложения. - return; - } + $mobilePhoneNumber = $command->mobilePhoneNumber; + + if (null !== $mobilePhoneNumber) { + try { + $this->guardMobilePhoneNumber($mobilePhoneNumber); + } catch (InvalidArgumentException) { + // Ошибка уже залогирована внутри гарда. + // Отбрасываем невалидный номер: контакт создаётся без мобильного телефона. + $mobilePhoneNumber = null; } + } + try { $applicationInstallation = $this->applicationInstallationRepository->getById($command->applicationInstallationId); assert($applicationInstallation instanceof AggregateRootEventsEmitterInterface); @@ -61,7 +62,7 @@ public function handle(Command $command): void $command->fullName, $command->email, null, - $command->mobilePhoneNumber, + $mobilePhoneNumber, null, $command->comment, $command->externalId, diff --git a/tests/Functional/ApplicationInstallations/UseCase/InstallContactPerson/HandlerTest.php b/tests/Functional/ApplicationInstallations/UseCase/InstallContactPerson/HandlerTest.php index 076c62b6..2144b428 100644 --- a/tests/Functional/ApplicationInstallations/UseCase/InstallContactPerson/HandlerTest.php +++ b/tests/Functional/ApplicationInstallations/UseCase/InstallContactPerson/HandlerTest.php @@ -242,6 +242,8 @@ public function testInstallContactPersonWithInvalidPhone(string $phoneNumber, st $this->bitrix24accountRepository->save($bitrix24Account); $applicationInstallation = (new ApplicationInstallationBuilder()) + ->withBitrix24PartnerContactPersonId(null) + ->withContactPersonId(null) ->withBitrix24AccountId($bitrix24Account->getId()) ->withApplicationToken($applicationToken) ->withApplicationStatus(new ApplicationStatus('F')) @@ -274,9 +276,19 @@ public function testInstallContactPersonWithInvalidPhone(string $phoneNumber, st ) ); - // Проверяем, что контакт не был создан + // Контакт создан, но невалидный мобильный телефон отброшен (null) + $dispatchedEvents = $this->eventDispatcher->getOrphanedEvents(); + $this->assertContains(ContactPersonCreatedEvent::class, $dispatchedEvents); + $this->assertContains(ApplicationInstallationContactPersonLinkedEvent::class, $dispatchedEvents); + $foundInstallation = $this->applicationInstallationRepository->getById($applicationInstallation->getId()); - $this->assertNull($foundInstallation->getBitrix24PartnerId()); + $contactPersonId = $foundInstallation->getContactPersonId(); + $this->assertNotNull($contactPersonId); + + $foundContactPerson = $this->repository->getById($contactPersonId); + $this->assertNull($foundContactPerson->getMobilePhone()); + $this->assertSame($contactPerson->getEmail(), $foundContactPerson->getEmail()); + $this->assertEquals($contactPerson->getFullName(), $foundContactPerson->getFullName()); } public static function invalidPhoneProvider(): array