Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
215 changes: 215 additions & 0 deletions TASKS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
# SaferPay Official Module - Improvement Tasks

## Performance Optimizations

### 1. Database Query Optimization
**Priority: High** | **Effort: Medium** | **Impact: High**

- **Issue**: Multiple database queries without proper indexing and some inefficient queries
- **Location**: `src/Repository/` classes, especially `SaferPayCardAliasRepository.php` and `SaferPayOrderRepository.php`
- **Improvements**:
- Add database indexes for frequently queried columns (id_customer, payment_method, id_order, id_cart)
- Optimize queries in `getSavedValidCardsByUserIdAndPaymentMethod()` to use JOINs instead of multiple WHERE clauses
- Implement query result caching for static data like payment methods and configurations
- Add LIMIT clauses to queries that don't need all results

### 2. Service Container Optimization
**Priority: Medium** | **Effort: Low** | **Impact: Medium**

- **Issue**: Services are instantiated multiple times in the same request
- **Location**: Main module class `getService()` method
- **Improvements**:
- Implement service singleton pattern or dependency injection container caching
- Cache frequently used services like `LoggerInterface`, `Configuration`, and repository classes
- Reduce service instantiation in hooks and controllers

### 3. Configuration Access Optimization
**Priority: Medium** | **Effort: Low** | **Impact: Medium**

- **Issue**: Configuration values are fetched multiple times per request
- **Location**: Throughout the module, especially in payment processing
- **Improvements**:
- Cache configuration values in memory for the duration of the request
- Implement lazy loading for configuration values
- Group related configuration calls

## Code Quality Improvements

### 4. Type Declarations Enhancement
**Priority: Medium** | **Effort: Medium** | **Impact: Medium**

- **Issue**: Missing return type declarations and inconsistent type hints
- **Location**: Service classes, repositories, and controllers
- **Improvements**:
- Add strict return type declarations to all public methods
- Implement proper type hints for array parameters and return values
- Add PHPDoc blocks with proper @param and @return annotations
- Use PHP 7.4+ typed properties where applicable

### 5. Exception Handling Standardization
**Priority: High** | **Effort: Medium** | **Impact: High**

- **Issue**: Inconsistent exception handling patterns across the module
- **Location**: Controllers and service classes
- **Improvements**:
- Standardize exception handling in all controllers using a common pattern
- Implement proper exception hierarchy with specific exception types
- Add consistent error logging with context information
- Improve error messages for better user experience

### 6. Code Duplication Reduction
**Priority: Medium** | **Effort: Medium** | **Impact: Medium**

- **Issue**: Repeated code patterns in controllers and services
- **Location**: Front controllers, especially validation and return controllers
- **Improvements**:
- Extract common validation logic into shared service classes
- Create base controller methods for common operations (redirects, error handling)
- Implement shared utilities for URL generation and parameter handling
- Consolidate similar database query patterns

## Security Enhancements

### 7. Input Validation Strengthening
**Priority: High** | **Effort: Low** | **Impact: High**

- **Issue**: Some user inputs lack proper validation and sanitization
- **Location**: Front controllers and admin controllers
- **Improvements**:
- Add comprehensive input validation for all user-provided data
- Implement proper SQL injection prevention (already using pSQL but ensure consistency)
- Add CSRF token validation for admin operations
- Validate file uploads and external data sources

### 8. Sensitive Data Protection
**Priority: High** | **Effort: Low** | **Impact: High**

- **Issue**: Sensitive configuration data handling could be improved
- **Location**: Configuration storage and display
- **Improvements**:
- Implement proper password masking in admin forms
- Add encryption for sensitive configuration values
- Improve logging to avoid exposing sensitive data
- Implement proper session security

## Maintainability Improvements

### 9. Configuration Management Enhancement
**Priority: Medium** | **Effort: Medium** | **Impact: Medium**

- **Issue**: Configuration handling is scattered and could be more organized
- **Location**: `SaferPayConfig.php` and admin settings controller
- **Improvements**:
- Group related configuration options logically
- Implement configuration validation on save
- Add configuration migration system for future updates
- Create configuration backup/restore functionality

### 10. Logging System Enhancement
**Priority: Medium** | **Effort: Low** | **Impact: Medium**

- **Issue**: Logging could be more structured and informative
- **Location**: `Logger.php` and throughout the module
- **Improvements**:
- Implement structured logging with consistent format
- Add log level configuration per operation type
- Implement log rotation and cleanup automation
- Add performance metrics logging for critical operations

### 11. Error Message Localization
**Priority: Low** | **Effort: Medium** | **Impact: Medium**

- **Issue**: Some error messages are not properly localized
- **Location**: Exception services and controllers
- **Improvements**:
- Ensure all user-facing messages are properly translated
- Add missing translation keys
- Implement fallback messages for missing translations
- Standardize message formatting

## User Experience Improvements

### 12. Admin Interface Optimization
**Priority: Low** | **Effort: Low** | **Impact: Medium**

- **Issue**: Admin interface could be more user-friendly
- **Location**: Admin settings controller and templates
- **Improvements**:
- Add configuration validation feedback
- Implement auto-save for non-critical settings
- Add help tooltips and documentation links
- Improve form layout and organization

### 13. Frontend Error Handling
**Priority: Medium** | **Effort: Low** | **Impact: Medium**

- **Issue**: Frontend error messages could be more user-friendly
- **Location**: Front controllers and templates
- **Improvements**:
- Implement graceful error handling for payment failures
- Add retry mechanisms for temporary failures
- Improve error message clarity for end users
- Add progress indicators for long-running operations

## Technical Debt Reduction

### 14. Legacy Code Cleanup
**Priority: Low** | **Effort: Medium** | **Impact: Low**

- **Issue**: Some legacy code patterns and unused code
- **Location**: Throughout the module
- **Improvements**:
- Remove unused imports and methods
- Update deprecated PrestaShop API usage
- Consolidate similar functionality
- Remove commented-out code

### 15. Testing Infrastructure
**Priority: Low** | **Effort: High** | **Impact: High**

- **Issue**: Limited test coverage for critical functionality
- **Location**: Missing comprehensive tests
- **Improvements**:
- Add unit tests for service classes
- Implement integration tests for payment flows
- Add automated testing for admin functionality
- Create test data fixtures for consistent testing

## Implementation Priority

### Phase 1 (High Priority - Security & Performance)
1. Database Query Optimization (#1)
2. Exception Handling Standardization (#5)
3. Input Validation Strengthening (#7)
4. Sensitive Data Protection (#8)

### Phase 2 (Medium Priority - Quality & UX)
5. Type Declarations Enhancement (#4)
6. Code Duplication Reduction (#6)
7. Service Container Optimization (#2)
8. Configuration Access Optimization (#3)
9. Frontend Error Handling (#13)

### Phase 3 (Low Priority - Polish & Maintenance)
10. Configuration Management Enhancement (#9)
11. Logging System Enhancement (#10)
12. Admin Interface Optimization (#12)
13. Error Message Localization (#11)
14. Legacy Code Cleanup (#14)
15. Testing Infrastructure (#15)

## Notes

- All improvements should maintain backward compatibility
- Changes should be thoroughly tested in both test and live environments
- Performance improvements should be measured and documented
- Security enhancements should be reviewed by security experts
- User experience improvements should be tested with real users

## Estimated Total Effort
- **Phase 1**: 3-4 weeks
- **Phase 2**: 4-5 weeks
- **Phase 3**: 3-4 weeks
- **Total**: 10-13 weeks for complete implementation

Each task includes specific file locations and implementation guidance to ensure efficient development.
23 changes: 19 additions & 4 deletions src/Install/Installer.php
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,9 @@ private function installSaferPayOrderTable()
`refunded` tinyint(1) DEFAULT 0,
`canceled` tinyint(1) DEFAULT 0,
`authorized` tinyint(1) DEFAULT 0,
`pending` tinyint(1) DEFAULT 0
`pending` tinyint(1) DEFAULT 0,
INDEX `idx_token` (`token`),
INDEX `idx_status` (`captured`, `refunded`, `canceled`, `authorized`, `pending`)
) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci'
);
}
Expand Down Expand Up @@ -254,7 +256,13 @@ private function installSaferPayAssertTable()
`card_number` VARCHAR(64) NOT NULL,
`dcc_value` INTEGER(32) DEFAULT NULL,
`dcc_currency_code` VARCHAR(64) DEFAULT NULL,
`authorized` tinyint(1) DEFAULT 0
`authorized` tinyint(1) DEFAULT 0,
INDEX `idx_status` (`status`),
INDEX `idx_payment_id` (`payment_id`),
INDEX `idx_merchant_reference` (`merchant_reference`),
INDEX `idx_authorized` (`authorized`),
INDEX `idx_currency_code` (`currency_code`),
INDEX `idx_order_status` (`id_saferpay_order`, `status`)
) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci'
);
}
Expand All @@ -272,7 +280,10 @@ private function installSaferPayCardAlias()
`payment_method` VARCHAR(64) NOT NULL,
`valid_till` datetime NOT NULL,
`date_add` datetime NOT NULL,
`date_upd` datetime NOT NULL
`date_upd` datetime NOT NULL,
INDEX `idx_success` (`success`),
INDEX `idx_customer_success` (`id_customer`, `success`),
INDEX `idx_customer_valid_till` (`id_customer`, `valid_till`)
) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci'
);
}
Expand Down Expand Up @@ -304,7 +315,11 @@ private function installOrderRefundTable()
`transaction_id` VARCHAR(64) NOT NULL,
`amount` INTEGER(20) NOT NULL,
`currency` VARCHAR(64) NOT NULL,
`status` VARCHAR(64) NOT NULL
`status` VARCHAR(64) NOT NULL,
INDEX `idx_status` (`status`),
INDEX `idx_currency` (`currency`),
INDEX `idx_order_status` (`id_order`, `status`),
INDEX `idx_saferpay_order_status` (`id_saferpay_order`, `status`)
) ENGINE=' . _MYSQL_ENGINE_ . ' DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci'
);
}
Expand Down
8 changes: 4 additions & 4 deletions src/Repository/SaferPayCardAliasRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public function getSavedValidCardsByUserIdAndPaymentMethod($userId, $paymentMeth
$query = new DbQuery();
$query->select('`id_saferpay_card_alias`, `card_number`');
$query->from('saferpay_card_alias');
$query->where('id_customer = "' . (int) $userId . '"');
$query->where('id_customer = ' . (int) $userId);
$query->where('payment_method = "' . pSQL($paymentMethod) . '"');
$query->where('valid_till > "' . pSQL($currentDate) . '"');

Expand All @@ -49,7 +49,7 @@ public function getSavedCardAliasFromId($id)
$query = new DbQuery();
$query->select('`alias_id`');
$query->from('saferpay_card_alias');
$query->where('id_saferpay_card_alias = "' . (int) $id . '"');
$query->where('id_saferpay_card_alias = ' . (int) $id);

return Db::getInstance()->getValue($query);
}
Expand All @@ -59,7 +59,7 @@ public function getSavedCardIdByCustomerIdAndAliasId($customerId, $aliasId)
$query = new DbQuery();
$query->select('`id_saferpay_card_alias`');
$query->from('saferpay_card_alias');
$query->where('id_customer = "' . (int) $customerId . '"');
$query->where('id_customer = ' . (int) $customerId);
$query->where('alias_id = "' . pSQL($aliasId) . '"');

return Db::getInstance()->getValue($query);
Expand All @@ -70,7 +70,7 @@ public function getSavedCardsByCustomerId($customerId)
$query = new DbQuery();
$query->select('*');
$query->from('saferpay_card_alias');
$query->where('id_customer = "' . (int) $customerId . '"');
$query->where('id_customer = ' . (int) $customerId);

return Db::getInstance()->executeS($query);
}
Expand Down
11 changes: 6 additions & 5 deletions src/Repository/SaferPayOrderRepository.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public function getIdByOrderId($orderId)
$query = new DbQuery();
$query->select('`id_saferpay_order`');
$query->from('saferpay_order');
$query->where('id_order = "' . (int) $orderId . '"');
$query->where('id_order = ' . (int) $orderId);
$query->orderBy('`id_saferpay_order` DESC');

return Db::getInstance()->getValue($query);
Expand All @@ -60,7 +60,7 @@ public function getIdByCartId($cartId)
$query = new DbQuery();
$query->select('`id_saferpay_order`');
$query->from('saferpay_order');
$query->where('id_cart = "' . (int) $cartId . '"');
$query->where('id_cart = ' . (int) $cartId);
$query->orderBy('`id_saferpay_order` DESC');

return Db::getInstance()->getValue($query);
Expand All @@ -70,7 +70,7 @@ public function getAssertIdBySaferPayOrderId($saferPayOrderId)
$query = new DbQuery();
$query->select('`id_saferpay_assert`');
$query->from('saferpay_assert');
$query->where('id_saferPay_order = "' . (int) $saferPayOrderId . '"');
$query->where('id_saferPay_order = ' . (int) $saferPayOrderId);
$query->orderBy('id_saferpay_assert DESC');

return Db::getInstance()->getValue($query);
Expand All @@ -86,7 +86,8 @@ public function getOrderRefunds($saferPayOrderId)
$query = new DbQuery();
$query->select('*');
$query->from('saferpay_order_refund');
$query->where('id_saferPay_order = "' . (int) $saferPayOrderId . '"');
$query->where('id_saferPay_order = ' . (int) $saferPayOrderId);
$query->orderBy('id_saferpay_order_refund DESC');

return Db::getInstance()->executeS($query);
}
Expand All @@ -96,7 +97,7 @@ public function getPaymentBrandBySaferpayOrderId($saferpayOrderId)
$query = new DbQuery();
$query->select('`brand`');
$query->from('saferpay_assert');
$query->where('id_saferpay_order = "' . (int) $saferpayOrderId . '"');
$query->where('id_saferpay_order = ' . (int) $saferpayOrderId);

return Db::getInstance()->getValue($query);
}
Expand Down
Loading