SL-350 Settings UX/logic fixes (SL-366/367/370)#334
Conversation
- SL-367: filter MPO/SPG terminals from selection (defensive Type/TerminalType read, case-insensitive) + unit test - SL-366: hide password show/hide toggle until a new password is typed; keep saved secret masked - SL-370: split 'Card Display & Saving' into 'Card Display' and 'Card Saving for Customers' sections Stored config keys and save flow unchanged. Compiled bundle is gitignored (built in CI).
![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 introduces terminal filtering in the SaferPay service to exclude specific terminal types, updates translation strings for payment settings, and refactors the payment processing UI to separate card display and saving settings into distinct cards. Additionally, it adds a conditional password visibility toggle in the API credentials component. The review feedback suggests implementing scalar checks for terminal type properties to improve defensive programming and updating PHPUnit mock methods to use onlyMethods for better compatibility.
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 (isset($terminal->Type)) { | ||
| return strtoupper((string) $terminal->Type); | ||
| } | ||
|
|
||
| if (isset($terminal->TerminalType)) { | ||
| return strtoupper((string) $terminal->TerminalType); | ||
| } |
There was a problem hiding this comment.
To ensure robust defensive programming, we should verify that the Type or TerminalType properties are scalar values (e.g., strings or numbers) before casting them to string and calling strtoupper(). If the API response unexpectedly contains an array or an object for these fields, casting them directly to string will trigger a TypeError or Error in PHP 8.0+.
if (isset($terminal->Type) && is_scalar($terminal->Type)) {
return strtoupper((string) $terminal->Type);
}
if (isset($terminal->TerminalType) && is_scalar($terminal->TerminalType)) {
return strtoupper((string) $terminal->TerminalType);
}|
|
||
| $mock = $this->getMockBuilder(GetTerminalsService::class) | ||
| ->disableOriginalConstructor() | ||
| ->setMethods(['getTerminals']) |
There was a problem hiding this comment.
The setMethods() method is deprecated in PHPUnit 8/9 and has been completely removed in PHPUnit 10. Since getTerminals is an existing method on GetTerminalsService, you should use onlyMethods() instead to ensure compatibility with newer PHPUnit versions.
->onlyMethods(['getTerminals'])Address review feedback: verify Type/TerminalType are scalar before casting to string, so an unexpected array/object in the API response can't trigger a TypeError.
…to SL-350/settings-ux-fixes # Conflicts: # views/js/admin/settings-app/src/components/settings/api-credentials.tsx
justelis22
merged commit 9b979c8
into
feature/react-admin-settings
1 participant
Part of SL-350 settings redesign. Phase 2 — small logic / UX fixes.
Tickets
Type/TerminalType, case-insensitive) since the live API property was unconfirmed.Tests
New unit test
tests/Unit/Service/SaferPayGetTerminalsTest.php(4 tests) covering MPO/SPG exclusion, case-insensitivity, both property names, and the returned shape.Verification
Unit tests green; settings-app build passes;
php -lclean.