Release 2.0.3 #304
Release 2.0.3 #304
Conversation
This reverts commit f0df09a.
…mprove performance - Remove unused constant DISABLE_CACHE - Remove unreachable return statement in hookActionEmailSendBefore - Remove unused variable $activePaymentMethods in hookPaymentOptions - Remove redundant validation check in hookActionObjectOrderPaymentAddAfter - Add service container caching to prevent re-instantiation - Extract services from loop in hookPaymentOptions for better performance - Simplify install method by removing unnecessary intermediate variables - Improve variable naming clarity ($isCreditCardSavingEnabledForUser) - Add proper PHPDoc for getService method
- Add return types to VersionUtility methods (all return bool or string) - Add return type to PriceUtility::convertToCents (returns int) - Add return types to SaferPayExceptionService methods - Add return type to SaferPayCartService::isCurrencyAvailable (returns bool) - Add return types to LegacyTranslator methods - Add return types to PaymentRestrictionValidation interface and implementations - Add PHPDoc annotations to SaferPayOrderRepository methods - Add PHPDoc property annotations to SaferPayObtainPaymentMethods Improves type safety and IDE autocomplete support throughout the module.
- Add return type hint ?: Cart to LegacyContext::getCart() - Add return type hints ?object to all API request service methods - Add return type hint AssertBody to AuthorizationService::createObjectsFromAuthorizationResponse() - Fix PHPDoc for ApiRequest::get() to correctly indicate object|null instead of array|null All changes follow Gemini code review suggestions from PR #300
- Move configuration restoration to finally blocks to guarantee execution even when exceptions occur - Update validateTerminalId to use credentials from form input instead of saved configuration - Simplify terminal data parsing by removing redundant object format checks - Remove HOW_TEST.md documentation file These changes fix critical issues identified in code review where configuration state could become inconsistent during API calls. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Set proper permissions for saferpayofficial module: - Directories: 755 (rwxr-xr-x) - Files: 644 (rw-r--r--) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
SL-331 remove JS library from UI & permissions fix
…into SL-330/terminal-retrieval
- Add proper error logging using SaferPay LoggerInterface in validateTerminalId() and getTerminalsForEnvironment() - Optimize validateTerminalId() to fetch terminals only once instead of making two separate API calls - Replace empty catch blocks with proper error handling 🤖 Generated with [Claude Code](https://claude.com/claude-code)
SL-330 terminal retrieval
Enhanced type safety across multiple classes by adding missing parameter and return type hints as identified in Gemini code review. This improves code quality and catches potential type-related bugs at compile time. Changes: - SaferPayOrderRepository: Added int parameter types and SaferPayOrder/array return types - SaferPayOrderBuilder: Added object parameter types for body parameters - PaymentTypeProvider: Added string parameter types to all methods - LegacyTranslator: Added string parameter type to translate method 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Add type hints to TranslatorInterface::translate() to match LegacyTranslator implementation. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
The object type hint was introduced in PHP 7.2. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
- Set platform PHP version to 7.1 - Add PHP >=7.1 requirement - Pin dev dependencies to PHP 7.1 compatible versions 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Pin php-cs-fixer to ^2.16.7 and add symfony/process ^3.4 constraint. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Advisory PKSA-wws7-mr54-jsny affects symfony/process but newer versions require PHP 7.2.5+, which is incompatible with our PHP 7.1 requirement. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
SL-307 refactoring 2
SL-307 refactoring 1
Summary of ChangesHello @MarijusDilys, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request prepares for release 2.0.3 by upgrading the module's PHP compatibility, enhancing the administrative experience with dynamic terminal management, and boosting application responsiveness through targeted database indexing. It also includes a comprehensive code quality pass to improve the codebase's robustness and clarity. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 several valuable improvements for the 2.0.3 release, including database performance optimizations through new indexes, dynamic terminal selection, and significant code quality enhancements with the addition of type hinting across many files. The refactoring to improve performance in saferpayofficial.php is also a welcome change.
However, there are a few critical issues that need to be addressed. A security vulnerability is being ignored in composer.json, which poses a significant risk. More importantly, the upgrade script for version 2.0.2 has been incorrectly overwritten with changes for 2.0.3, which could cause serious problems for users upgrading the module. I've also noted some minor issues, such as typos in the changelog and opportunities to improve SQL query formatting.
| function upgrade_module_2_0_2(SaferPayOfficial $module) | ||
| { | ||
| Configuration::updateValue('SAFERPAY_SEND_ORDER_CONF_MAIL', 0); | ||
| Configuration::updateValue('SAFERPAY_GROUP_CARDS', 0); | ||
| $db = Db::getInstance(); | ||
| $success = true; | ||
|
|
||
| return true; | ||
| // Add indexes for saferpay_order table | ||
| $orderIndexes = [ | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_id_order` (`id_order`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_id_cart` (`id_cart`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_id_customer` (`id_customer`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_transaction_id` (`transaction_id`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order` ADD INDEX `idx_status_flags` (`authorized`, `captured`, `pending`)", | ||
| ]; | ||
|
|
||
| foreach ($orderIndexes as $indexSql) { | ||
| try { | ||
| $result = $db->execute($indexSql); | ||
| if (!$result) { | ||
| $error = $db->getMsgError(); | ||
| if (strpos($error, 'Duplicate key name') === false) { | ||
| $success = false; | ||
| PrestaShopLogger::addLog('SaferPay: Failed to add order index - ' . $error, 3, null, 'SaferPayOrder'); | ||
| } | ||
| } | ||
| } catch (Exception $e) { | ||
| PrestaShopLogger::addLog('SaferPay: Order index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayOrder'); | ||
| } | ||
| } | ||
|
|
||
| // Add indexes for saferpay_card_alias table | ||
| $cardAliasIndexes = [ | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_id_customer` (`id_customer`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_payment_method` (`payment_method`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_customer_payment` (`id_customer`, `payment_method`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_valid_till` (`valid_till`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_card_alias` ADD INDEX `idx_alias_id` (`alias_id`)", | ||
| ]; | ||
|
|
||
| foreach ($cardAliasIndexes as $indexSql) { | ||
| try { | ||
| $result = $db->execute($indexSql); | ||
| if (!$result) { | ||
| $error = $db->getMsgError(); | ||
| if (strpos($error, 'Duplicate key name') === false) { | ||
| $success = false; | ||
| PrestaShopLogger::addLog('SaferPay: Failed to add card alias index - ' . $error, 3, null, 'SaferPayCardAlias'); | ||
| } | ||
| } | ||
| } catch (Exception $e) { | ||
| PrestaShopLogger::addLog('SaferPay: Card alias index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayCardAlias'); | ||
| } | ||
| } | ||
|
|
||
| // Add indexes for saferpay_assert table | ||
| $assertIndexes = [ | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_assert` ADD INDEX `idx_id_saferpay_order` (`id_saferpay_order`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_assert` ADD INDEX `idx_payment_method` (`payment_method`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_assert` ADD INDEX `idx_brand` (`brand`)", | ||
| ]; | ||
|
|
||
| foreach ($assertIndexes as $indexSql) { | ||
| try { | ||
| $result = $db->execute($indexSql); | ||
| if (!$result) { | ||
| $error = $db->getMsgError(); | ||
| if (strpos($error, 'Duplicate key name') === false) { | ||
| $success = false; | ||
| PrestaShopLogger::addLog('SaferPay: Failed to add assert index - ' . $error, 3, null, 'SaferPayAssert'); | ||
| } | ||
| } | ||
| } catch (Exception $e) { | ||
| PrestaShopLogger::addLog('SaferPay: Assert index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayAssert'); | ||
| } | ||
| } | ||
|
|
||
| // Add indexes for saferpay_order_refund table | ||
| $refundIndexes = [ | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order_refund` ADD INDEX `idx_id_saferpay_order` (`id_saferpay_order`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order_refund` ADD INDEX `idx_id_order` (`id_order`)", | ||
| "ALTER TABLE `" . _DB_PREFIX_ . "saferpay_order_refund` ADD INDEX `idx_transaction_id` (`transaction_id`)", | ||
| ]; | ||
|
|
||
| foreach ($refundIndexes as $indexSql) { | ||
| try { | ||
| $result = $db->execute($indexSql); | ||
| if (!$result) { | ||
| $error = $db->getMsgError(); | ||
| if (strpos($error, 'Duplicate key name') === false) { | ||
| $success = false; | ||
| PrestaShopLogger::addLog('SaferPay: Failed to add refund index - ' . $error, 3, null, 'SaferPayOrderRefund'); | ||
| } | ||
| } | ||
| } catch (Exception $e) { | ||
| PrestaShopLogger::addLog('SaferPay: Refund index creation skipped - ' . $e->getMessage(), 1, null, 'SaferPayOrderRefund'); | ||
| } | ||
| } | ||
|
|
||
| if ($success) { | ||
| PrestaShopLogger::addLog('SaferPay: Database indexes added successfully', 1, null, 'SaferPayOptimization'); | ||
| } | ||
|
|
||
| return $success; | ||
| } |
There was a problem hiding this comment.
This change overwrites the upgrade script for version 2.0.2 with logic intended for 2.0.3 (database index optimization). This is a critical issue as it will prevent users upgrading from versions prior to 2.0.2 from receiving the correct updates for that version, and could cause issues if they try to upgrade again.
A new upgrade script, upgrade/install-2.0.3.php, should be created with a corresponding upgrade_module_2_0_3() function for these changes. The upgrade/install-2.0.2.php file should be reverted to its original state.
| "audit": { | ||
| "ignore": ["PKSA-wws7-mr54-jsny"] | ||
| } |
There was a problem hiding this comment.
Ignoring the security audit for PKSA-wws7-mr54-jsny is a security risk. This vulnerability in vlucas/phpdotenv versions before 5.0.0 can lead to the exposure of your .env file if the document root is misconfigured. It is highly recommended to update vlucas/phpdotenv to a secure version (e.g., ^5.0) instead of ignoring the audit.
changelog.md
Outdated
| - Dynamic termimal selection | ||
| - Removed uneccesary inputs from admin settings |
There was a problem hiding this comment.
There are a couple of typos in the changelog entry for version 2.0.3.
termimalshould beterminal.uneccesaryshould beunnecessary.
| - Dynamic termimal selection | |
| - Removed uneccesary inputs from admin settings | |
| - Dynamic terminal selection | |
| - Removed unnecessary inputs from admin settings |
| // Services used in the loop - initialized once for performance | ||
| /** @var SaferPayCardAliasRepository $cardAliasRepository */ | ||
| $cardAliasRepository = $this->getService(SaferPayCardAliasRepository::class); | ||
| /** @var PaymentRedirectionProvider $paymentRedirectionProvider */ | ||
| $paymentRedirectionProvider = $this->getService(PaymentRedirectionProvider::class); | ||
| /** @var LegacyTranslator $translator */ | ||
| $translator = $this->getService(LegacyTranslator::class); | ||
|
|
||
| $isBusinessLicenseEnabled = Configuration::get(SaferPayConfig::BUSINESS_LICENSE . SaferPayConfig::getConfigSuffix()); | ||
| $isCreditCardSavingEnabled = Configuration::get(SaferPayConfig::CREDIT_CARD_SAVE); | ||
|
|
There was a problem hiding this comment.
Moving the service and configuration lookups out of the foreach loop is a great performance improvement. This avoids redundant calls on each iteration.
Introduced interface for request DTOs to improve code consistency and maintainability. Converted remaining TODO comments to documentation notes. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Added return type hints and parameter type hints to SaferPayOrderStatusService and RequestObjectCreator for improved code quality and IDE support. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This reverts commit c084aec.
SL-332 code refactoring
Replaced abandoned container-interop/container-interop with psr/container by upgrading league/container from 2.5.0 to ^3.0 (PHP 7.1 compatible). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Upgrade league/container to 3.x to remove abandoned dependency
- Add missing Smarty variables before rendering payment_method_all.tpl - Add error message when no payment methods are available
…ions-payment-all BUGFIX /fix undefined country options payment all
Self-Checks
JIRA task link
Summary
QA Checklist Labels
QA Checklist
Additional Context
Frontend Changes