From 7c4924a78f9667303bd0cf71bd91aa5d2187f502 Mon Sep 17 00:00:00 2001 From: Justas Date: Fri, 10 Jul 2026 09:40:50 +0300 Subject: [PATCH 1/2] feat(SL-371): auto-refresh account payment methods when settings open - Reconcile stored payment methods against the account on page open (persisted, enabled flags preserved, new methods disabled by default) instead of only on save - On API failure, fall back to the last-known stored list instead of an empty page; never wipes stored config (new SaferPayPaymentRepository::getAllPaymentMethodsNames) - Unit test for the refresh reconcile behavior (preserve/added-disabled/removed-dropped + early-return) --- ...dminSaferPayOfficialSettingsController.php | 19 ++- src/Repository/SaferPayPaymentRepository.php | 15 ++ .../SaferPayRefreshPaymentsServiceTest.php | 137 ++++++++++++++++++ 3 files changed, 167 insertions(+), 4 deletions(-) create mode 100644 tests/Unit/Service/SaferPayRefreshPaymentsServiceTest.php diff --git a/controllers/admin/AdminSaferPayOfficialSettingsController.php b/controllers/admin/AdminSaferPayOfficialSettingsController.php index ebad09394..807bce099 100755 --- a/controllers/admin/AdminSaferPayOfficialSettingsController.php +++ b/controllers/admin/AdminSaferPayOfficialSettingsController.php @@ -693,17 +693,28 @@ private function getCurrencies() */ private function getPaymentMethodsData() { + /** @var SaferPayPaymentRepository $paymentRepository */ + $paymentRepository = $this->module->getService(SaferPayPaymentRepository::class); + try { + // Re-read the account and reconcile the stored list when the Payment Methods + // settings open, so methods added/removed on the Saferpay account are reflected + // (and persisted for the front office) without requiring a Save click. Enabled + // flags are preserved by the refresh; newly added methods default to disabled. + /** @var SaferPayRefreshPaymentsService $refreshPaymentsService */ + $refreshPaymentsService = $this->module->getService(SaferPayRefreshPaymentsService::class); + $refreshPaymentsService->refreshPayments(); + /** @var SaferPayObtainPaymentMethods $obtainMethods */ $obtainMethods = $this->module->getService(SaferPayObtainPaymentMethods::class); $paymentMethods = $obtainMethods->obtainPaymentMethodsNamesAsArray(); } catch (SaferPayApiException $exception) { - return ['error' => $this->module->l('Failed to load payment methods. Please verify your API credentials.', self::FILE_NAME)]; + // Account unreachable (bad credentials / offline): keep the last-known stored + // list rather than wiping the page. Credential validity is surfaced separately + // on the Credentials tab. Never clears stored configuration. + $paymentMethods = array_column($paymentRepository->getAllPaymentMethodsNames(), 'name'); } - /** @var SaferPayPaymentRepository $paymentRepository */ - $paymentRepository = $this->module->getService(SaferPayPaymentRepository::class); - /** @var SaferPayLogoRepository $logoRepository */ $logoRepository = $this->module->getService(SaferPayLogoRepository::class); diff --git a/src/Repository/SaferPayPaymentRepository.php b/src/Repository/SaferPayPaymentRepository.php index 9b16e20c6..99c48671b 100644 --- a/src/Repository/SaferPayPaymentRepository.php +++ b/src/Repository/SaferPayPaymentRepository.php @@ -110,6 +110,21 @@ public function getActivePaymentMethodsNames() return $result; } + public function getAllPaymentMethodsNames() + { + $query = new DbQuery(); + $query->select('name'); + $query->from('saferpay_payment'); + + $result = Db::getInstance()->executeS($query); + + if (!$result) { + return []; + } + + return $result; + } + public function truncateTable() { $query = 'TRUNCATE TABLE ' . _DB_PREFIX_ . 'saferpay_payment;'; diff --git a/tests/Unit/Service/SaferPayRefreshPaymentsServiceTest.php b/tests/Unit/Service/SaferPayRefreshPaymentsServiceTest.php new file mode 100644 index 000000000..289d5fabd --- /dev/null +++ b/tests/Unit/Service/SaferPayRefreshPaymentsServiceTest.php @@ -0,0 +1,137 @@ + + *@copyright SIX Payment Services + *@license SIX Payment Services + */ + +namespace Invertus\SaferPay\Tests\Unit\Service; + +use Invertus\SaferPay\Logger\LoggerInterface; +use Invertus\SaferPay\Repository\SaferPayFieldRepository; +use Invertus\SaferPay\Repository\SaferPayPaymentRepository; +use Invertus\SaferPay\Repository\SaferPayRestrictionRepository; +use Invertus\SaferPay\Service\SaferPayObtainPaymentMethods; +use Invertus\SaferPay\Service\SaferPayRefreshPaymentsService; +use PHPUnit\Framework\TestCase; + +class SaferPayRefreshPaymentsServiceTest extends TestCase +{ + public function testReconcilePreservesEnabledAddsNewDisabledDropsRemoved() + { + // Stored: VISA (enabled), AMEX (enabled). Account: VISA (kept), TWINT (added), AMEX removed. + $paymentRepository = $this->mockPaymentRepository([ + ['name' => 'VISA', 'active' => '1'], + ['name' => 'AMEX', 'active' => '1'], + ]); + $fieldRepository = $this->mockFieldRepository(['VISA' => true, 'AMEX' => false]); + $obtainPaymentMethods = $this->mockObtainPaymentMethods(['VISA', 'TWINT']); + + // Both tables are rebuilt. + $paymentRepository->expects($this->once())->method('truncateTable'); + $fieldRepository->expects($this->once())->method('truncateTable'); + + // VISA keeps active=1; TWINT added as active=0; AMEX (removed) is never re-inserted. + $paymentRepository->expects($this->exactly(2)) + ->method('insertPayment') + ->withConsecutive( + [['name' => 'VISA', 'active' => 1]], + [['name' => 'TWINT', 'active' => 0]] + ); + // Custom-form flag preserved for VISA (true -> 1), default 0 for the new TWINT. + $fieldRepository->expects($this->exactly(2)) + ->method('insertField') + ->withConsecutive( + [['name' => 'VISA', 'active' => 1]], + [['name' => 'TWINT', 'active' => 0]] + ); + + $this->makeService($paymentRepository, $obtainPaymentMethods, $fieldRepository)->refreshPayments(); + } + + public function testDoesNothingWhenNoActivePaymentMethodsStored() + { + $paymentRepository = $this->mockPaymentRepository([]); + $fieldRepository = $this->mockFieldRepository([]); + $obtainPaymentMethods = $this->mockObtainPaymentMethods(['VISA']); + + // Early return: no API reconciliation and no destructive rebuild. + $obtainPaymentMethods->expects($this->never())->method('obtainPaymentMethodsNamesAsArray'); + $paymentRepository->expects($this->never())->method('truncateTable'); + $paymentRepository->expects($this->never())->method('insertPayment'); + + $this->makeService($paymentRepository, $obtainPaymentMethods, $fieldRepository)->refreshPayments(); + } + + private function makeService($paymentRepository, $obtainPaymentMethods, $fieldRepository) + { + return new SaferPayRefreshPaymentsService( + $paymentRepository, + $obtainPaymentMethods, + $this->createMockWithMethods(SaferPayRestrictionRepository::class, []), + $fieldRepository, + $this->createMockWithMethods(LoggerInterface::class, []) + ); + } + + private function mockPaymentRepository(array $activePayments) + { + $mock = $this->createMockWithMethods( + SaferPayPaymentRepository::class, + ['getActivePaymentMethods', 'truncateTable', 'insertPayment'] + ); + $mock->method('getActivePaymentMethods')->willReturn($activePayments); + + return $mock; + } + + private function mockFieldRepository(array $activeByName) + { + $mock = $this->createMockWithMethods( + SaferPayFieldRepository::class, + ['isActiveByName', 'truncateTable', 'insertField'] + ); + $mock->method('isActiveByName')->willReturnCallback(function ($name) use ($activeByName) { + return isset($activeByName[$name]) ? $activeByName[$name] : false; + }); + + return $mock; + } + + private function mockObtainPaymentMethods(array $names) + { + $mock = $this->createMockWithMethods( + SaferPayObtainPaymentMethods::class, + ['obtainPaymentMethodsNamesAsArray'] + ); + $mock->method('obtainPaymentMethodsNamesAsArray')->willReturn($names); + + return $mock; + } + + private function createMockWithMethods($class, array $methods) + { + $builder = $this->getMockBuilder($class)->disableOriginalConstructor(); + if (!empty($methods)) { + $builder->setMethods($methods); + } + + return $builder->getMock(); + } +} From 53fc9f257471fa069fecce9c45ca53098a21b7f5 Mon Sep 17 00:00:00 2001 From: Justas Date: Fri, 10 Jul 2026 11:36:42 +0300 Subject: [PATCH 2/2] perf(SL-371): avoid duplicate account API call on settings open Address review feedback: refreshPayments() already persists the account's methods, so read the list back from storage instead of calling obtainPaymentMethodsNamesAsArray() a second time. Falls back to a live fetch only when nothing was persisted (fresh setup / no-active early return). Removes the redundant back-to-back API request. --- .../AdminSaferPayOfficialSettingsController.php | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/controllers/admin/AdminSaferPayOfficialSettingsController.php b/controllers/admin/AdminSaferPayOfficialSettingsController.php index 807bce099..4bd0e3589 100755 --- a/controllers/admin/AdminSaferPayOfficialSettingsController.php +++ b/controllers/admin/AdminSaferPayOfficialSettingsController.php @@ -705,9 +705,17 @@ private function getPaymentMethodsData() $refreshPaymentsService = $this->module->getService(SaferPayRefreshPaymentsService::class); $refreshPaymentsService->refreshPayments(); - /** @var SaferPayObtainPaymentMethods $obtainMethods */ - $obtainMethods = $this->module->getService(SaferPayObtainPaymentMethods::class); - $paymentMethods = $obtainMethods->obtainPaymentMethodsNamesAsArray(); + // The refresh persists the account's methods, so read them back from storage + // instead of calling the API a second time. + $paymentMethods = array_column($paymentRepository->getAllPaymentMethodsNames(), 'name'); + + // refreshPayments() is a no-op when nothing is active yet (e.g. a fresh setup), + // so fall back to the live account list to still surface newly available methods. + if (empty($paymentMethods)) { + /** @var SaferPayObtainPaymentMethods $obtainMethods */ + $obtainMethods = $this->module->getService(SaferPayObtainPaymentMethods::class); + $paymentMethods = $obtainMethods->obtainPaymentMethodsNamesAsArray(); + } } catch (SaferPayApiException $exception) { // Account unreachable (bad credentials / offline): keep the last-known stored // list rather than wiping the page. Credential validity is surfaced separately