migrate credit system for env usage

[sveneld]

PR Type

Enhancement, Tests, Configuration changes

---

Description

• Migrated credit system configuration to environment variables.

• Updated CreditSystem class to use strongly typed properties and constructor parameters.

• Refactored CreditSystemFactory to simplify credit system initialization.

• Enhanced unit tests to reflect the new credit system configuration.

---

Changes walkthrough 📝

	Relevant files
Configuration changes	3 files services.php Updated service bindings for credit system configuration. +9/-3      .env.dev Added environment variables for credit system configuration. +9/-1      .env.dist Added default credit system configuration to `.env.dist`. +19/-2
Enhancement	5 files CreditSystem.php Refactored CreditSystem to use environment variables and strict typing. +45/-79  CreditSystemFactory.php Simplified CreditSystemFactory to use environment-based configuration. +4/-7      CreditSystemInterface.php Updated interface methods to use strict typing.                    +21/-49  DisabledCreditSystem.php Updated `DisabledCreditSystem` to use strict typing.          +13/-11  PdoDb.php Removed logger dependency from `PdoDb` constructor.            +4/-9
Tests	2 files CreditSystemFactoryTest.php Updated tests for `CreditSystemFactory` to reflect new configuration. +4/-6      CreditSystemTest.php Enhanced tests for CreditSystem with new parameters and exception handling. +71/-45

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[qodo-merge-pro-for-open-source]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Runtime Exception The constructor throws a RuntimeException when credit system is disabled, but this behavior should be handled by the factory instead. The factory is already responsible for choosing between CreditSystem and DisabledCreditSystem. if (!$isEnabled) { throw new \RuntimeException('Use DisabledCreditSystem instead'); } Type Consistency The getCreditCurrency() method returns an empty string while other methods return numeric values (0). Consider returning a default currency string for consistency. public function getCreditCurrency(): string { return ''; }

[qodo-merge-pro-for-open-source]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Add input validation to prevent invalid credit system configurations Add type validation for the credit-related environment variables to ensure they are within valid ranges and prevent potential issues with negative or invalid values. src/Credit/CreditSystem.php [33-43] public function __construct( bool $isEnabled, string $creditCurrency, float $minBalanceCredit, float $rentalFee, int $priceCycle, float $longRentalFee, float $limitIncreaseFee, float $violationFee, DbInterface $db -) +) { + if (!$isEnabled) { + throw new \RuntimeException('Use DisabledCreditSystem instead'); + } + if ($minBalanceCredit < 0 || $rentalFee < 0 || $longRentalFee < 0 || + $limitIncreaseFee < 0 || $violationFee < 0) { + throw new \InvalidArgumentException('Credit values cannot be negative'); + } + if (!in_array($priceCycle, [0, 1, 2])) { + throw new \InvalidArgumentException('Invalid price cycle value'); + } + $this->isEnabled = $isEnabled; + $this->creditCurrency = $creditCurrency; + $this->minBalanceCredit = $minBalanceCredit; + $this->rentalFee = $rentalFee; + $this->priceCycle = $priceCycle; + $this->longRentalFee = $longRentalFee; + $this->limitIncreaseFee = $limitIncreaseFee; + $this->violationFee = $violationFee; + $this->db = $db; +} Apply this suggestion Suggestion importance[1-10]: 8 Why: The suggestion adds crucial validation for credit-related values to prevent negative amounts and invalid price cycle values, which could cause serious issues in the credit system's operation.	8
	Add database error handling to prevent silent failures Add proper error handling for database queries in getUserCredit() method to catch and handle potential database exceptions. src/Credit/CreditSystem.php [58-66] public function getUserCredit($userId): float { - $result = $this->db->query('SELECT credit FROM credit WHERE userId = :userId', ['userId' => $userId]); - if ($result->rowCount() == 0) { - return 0; + try { + $result = $this->db->query('SELECT credit FROM credit WHERE userId = :userId', ['userId' => $userId]); + if ($result->rowCount() == 0) { + return 0; + } + return $result->fetchAssoc()['credit']; + } catch (\PDOException $e) { + throw new \RuntimeException('Failed to retrieve user credit: ' . $e->getMessage()); } - return $result->fetchAssoc()['credit']; } Apply this suggestion Suggestion importance[1-10]: 7 Why: Adding proper exception handling for database operations is important for system reliability and debugging, preventing silent failures that could mask serious issues.	7
General	Return a default currency symbol instead of an empty string to maintain interface consistency The getCreditCurrency() method should return a default currency symbol instead of an empty string to maintain consistency with the interface contract and prevent potential display issues. src/Credit/DisabledCreditSystem.php [29-32] public function getCreditCurrency(): string { - return ''; + return '€'; } Apply this suggestion Suggestion importance[1-10]: 3 Why: While the suggestion improves interface consistency, it's a minor enhancement as the DisabledCreditSystem is only used when credits are disabled, making the currency symbol less critical.	3