SL-388 Read checkout payment methods from storage instead of the API - #346
Open
TLabutis wants to merge 2 commits into
Open
SL-388 Read checkout payment methods from storage instead of the API#346TLabutis wants to merge 2 commits into
TLabutis wants to merge 2 commits into
Conversation
added 2 commits
August 27, 2026 12:36
PrestaShop re-renders the payment step over AJAX on every address and carrier change, and hookPaymentOptions called SaferPayObtainPaymentMethods on each render, so one order produced several Management API round trips. The only things the checkout needed from that response beyond the method name were the logo and the supported currencies. Both are now stored on saferpay_payment as logo_url and currencies, written by SaferPayRefreshPaymentsService, and read by the new SaferPayStoredPaymentMethods, which falls back to one account call only when nothing is stored yet. SaferPayRefreshPaymentsService now also reads every stored row rather than the active ones alone, so a method the merchant switched off keeps its flags across a refresh instead of reappearing enabled. The column migration and its backfill go into upgrade/install-2.1.0.php rather than a new 2.1.1 script, because 2.1.0 is still unreleased and PrestaShop skips an upgrade file whose version is above the module version. A backfill failure is logged rather than fatal, since an unreachable account must not break the upgrade and the checkout repopulates the columns by itself. The card brand grouping changes sitting on the same working tree are deliberately left out and ship on their own branch.
A method that disappears from the Saferpay account is dropped from storage by refreshPayments() without leaving any trace, so the merchant finds it gone from both the settings page and the checkout with nothing to explain why, and support cannot tell whether Saferpay stopped offering it or the module lost it. refreshPayments() now diffs the stored names against the account list and logs each removal through logRemovedPayments() before the tables are rebuilt, so a failure part way through the inserts still leaves a record. A removal that was enabled is a warning because it cost the shop a live checkout option, one that was already disabled is a notice. Logger::warning() and Logger::notice() are not gated by SaferPayConfig::isDebugMode(), so the rows reach Advanced Parameters > Logs on a default install. getPaymentName() is extracted so the payment name normalisation is shared by the comparison and the insert. Left out: the reactive refresh on a rejected Initialize, which needs the Saferpay ErrorName for an unavailable method confirmed against the sandbox first.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Self-Checks
JIRA task link
SL-388
Summary
The checkout called the Saferpay Management API on every payment step render to get the payment method list. The logo and the supported currencies are now stored alongside the method, so the list is built locally and the account is only asked when nothing is stored yet.
QA Checklist Labels
QA Checklist
Part of the same working tree as the SL-386 card brand grouping work, which is deliberately not in this PR and ships on its own branch. Nothing here depends on it and nothing here reads
SAFERPAY_GROUP_CARDS.Additional Context
Cause
hookPaymentOptionsresolvedSaferPayObtainPaymentMethodsand calledobtainPaymentMethods()on every render. PrestaShop re-renders the payment step over AJAX on every address and carrier change, so a single order produced several Management API round trips, each one blocking the payment step behind a remote call, and each one able to empty the payment step if the account was briefly unreachable.Everything the checkout actually needed from that response, beyond the method name it already had in
saferpay_payment, was two fields:logoUrlandcurrencies.Changes
saferpay_paymentgainslogo_url VARCHAR(255)andcurrencies VARCHAR(1024), inInstallerfor fresh installs and inupgrade/install-2.1.0.phpfor existing shops.SaferPayRefreshPaymentsServicenow callsobtainPaymentMethods()instead ofobtainPaymentMethodsNamesAsArray()and persists both fields. The currency list is comma joined;pSQL()is applied per value becauseSaferPayPaymentRepository::insertPayment()goes throughDb::insert(), which does not escape.SaferPayStoredPaymentMethods::getPaymentMethods()reads the stored rows and returns the same array shapeobtainPaymentMethods()returned, so nothing downstream in the hook changed. If no row carries a logo it triggers one refresh and re-reads, which is how a shop that has never opened the settings page still recovers instead of showing an empty payment step.SaferPayPaymentRepository::getAllPaymentMethods().SaferPayRefreshPaymentsServicealso reads every stored row rather than only the active ones when it snapshots the current flags. Before this, a refresh dropped the state of any method the merchant had switched off, so it came back enabled. That is a behaviour fix, not a side effect of the caching.Why the migration is in
install-2.1.0.phpand not a new 2.1.1 script2.1.0 is still unreleased (last tag is
v2.0.3), and PrestaShop only runs an upgrade file whenfile_version <= module_version(classes/module/Module.php). Ainstall-2.1.1.phpat module version 2.1.0 would be skipped entirely, which is the same trap #340 fixed on this branch. The module version is therefore left at 2.1.0 and the changelog line goes into the 2.1.0 section.The backfill runs one
refreshPayments()inside atry/catchand logs on failure rather than failing the upgrade. An unreachable account at upgrade time must not roll the version back, and the lazy refresh inSaferPayStoredPaymentMethodscovers that shop on its first checkout.Testing
PS 1.7.6.1 / PHP 7.2.34, live Saferpay test account, 24 payment methods on the account.
Real upgrade path, not a simulated one. Dropped both columns, set
ps_module.versionback to2.0.3, then ranphp bin/console prestashop:module upgrade saferpayofficial:The three rows without currencies are
APPLEPAY,GOOGLEPAYandCLICKTOPAY, exactlySaferPayConfig::WALLET_PAYMENT_METHODS, which the hook overrides with every shop currency. No method is dropped from the checkout because of an empty list. No2.1.0 upgradeentry inps_log, so the backfill did not fall back.Checkout, both grouping states. Called
hookPaymentOptionsagainst a real cart in EUR:The ungrouped path is the one this branch runs on its own, since the grouping default stays at 0 here. Both were checked because the stored rows now feed both.
Lazy refresh. Blanked
logo_urlon every row and rendered the payment step:isPopulated()returned false, one refresh ran, the methods came back. That is the recovery path for a shop whose upgrade backfill could not reach the account.php -lclean on all changed files.php-cs-fixer --dry-runclean, run inside the PHP 7.2 container because the host PHP 8.4 is above the fixer's supported range.Frontend Changes
No visual change. The payment step renders the same options from the same logos; only the source of the data changed.