Feature/issue 111 install contact person mobile phone validation#113
Conversation
…clarations across tests and core files.
# Conflicts: # src/ApplicationInstallations/UseCase/OnAppInstall/Handler.php # src/Console/ApplicationSettingsListCommand.php # tests/Contract/ApplicationSettings/Infrastructure/ApplicationSettingsItemRepositoryInterfaceContractTest.php # tests/Unit/ApplicationSettings/Services/DefaultSettingsInstallerTest.php # tests/Unit/ApplicationSettings/Services/SettingsFetcherTest.php
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the contact person installation handler to allow contact creation to proceed without a mobile phone number if the provided number is invalid, updating the corresponding functional tests. It also cleans up test files by refactoring mock setups and removing redundant arguments. However, the reviewer notes that removing the ApplicationSettingsItemInterface type hints from closure parameters in ApplicationSettingsListCommand.php and its contract test reduces type safety and static analysis precision, recommending that these type hints be restored.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| if ($globalOnly || (null === $userId && null === $departmentId)) { | ||
| $settings = array_filter($allSettings, fn (ApplicationSettingsItemInterface $setting): bool => $setting->isGlobal()); | ||
| $settings = array_filter($allSettings, fn ($setting): bool => $setting->isGlobal()); |
There was a problem hiding this comment.
Removing the ApplicationSettingsItemInterface type hint from the closure parameters reduces type safety and makes static analysis (like PHPStan or Psalm) less precise. It is highly recommended to keep explicit type hints for better maintainability and IDE autocompletion.
Please consider restoring the import of ApplicationSettingsItemInterface and keeping the type hints in the closures at lines 133, 136, and 139.
$settings = array_filter($allSettings, fn (\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $setting): bool => $setting->isGlobal());|
|
||
| // Verify each scope is present | ||
| $values = array_map(fn(\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $s): string => $s->getValue(), $results); | ||
| $values = array_map(fn($s): string => $s->getValue(), $results); |
There was a problem hiding this comment.
Removing the ApplicationSettingsItemInterface type hint from the closure parameter reduces type safety and makes static analysis less precise. It is highly recommended to keep explicit type hints for better maintainability and IDE autocompletion.
$values = array_map(fn(\Bitrix24\Lib\ApplicationSettings\Entity\ApplicationSettingsItemInterface $s): string => $s->getValue(), $results);
KarlsonComplete
deleted the
feature/issue-111-install-contact-person-mobile-phone-validation
branch
1 participant
Fixes #111