New payment parameters: #48
Conversation
- chargeUnregulatedCardFees - url_paid - url_cancelled - url_pending + better array shaped phpdocs + add tests with new parameters + phpstan 2.0
WalkthroughUpdated PHPStan configuration with enhanced dependency management and test path inclusion. Refined PHPDoc annotations across multiple entity classes for improved type documentation. Expanded the Payment entity with new properties and accessor methods for card fees and redirect URLs. Added corresponding test coverage for new Payment functionality. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
composer.json(1 hunks)phpstan.neon(1 hunks)src/Entity/AbstractEntity.php(1 hunks)src/Entity/AbstractPayment.php(2 hunks)src/Entity/Payment.php(4 hunks)src/Entity/PaymentStatus.php(1 hunks)src/Entity/RecurringPayment.php(1 hunks)src/Entity/Refund.php(2 hunks)src/Entity/Storno.php(1 hunks)tests/Cases/E2E/payments.php(1 hunks)tests/Cases/Entity/Payment.phpt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/Cases/E2E/payments.php (1)
src/Http/Response.php (1)
getData(34-37)
src/Entity/Payment.php (2)
src/Entity/Codes/LangCode.php (1)
LangCode(5-13)src/Entity/AbstractPayment.php (1)
toArray(134-147)
🔇 Additional comments (13)
tests/Cases/E2E/payments.php (1)
48-48: LGTM! Good improvement from debug code to assertion.Replacing
var_dumpwith a proper assertion that validates the response structure is a solid improvement for the test suite.phpstan.neon (1)
17-17: LGTM! Extends static analysis to test code.Including test cases in PHPStan scanning helps maintain code quality across the entire codebase.
tests/Cases/Entity/Payment.phpt (2)
142-153: LGTM! Comprehensive test for new chargeUnregulatedCardFees parameter.The test properly verifies both the presence of the key in the array and the correct string value conversion.
155-174: LGTM! Thorough test coverage for new URL parameters.The test validates all three URL parameters (paid, cancelled, pending) are correctly set and serialized, ensuring comprehensive coverage of the new functionality.
src/Entity/AbstractEntity.php (1)
9-9: LGTM! Improved type annotation.The more specific
array<string, mixed>type better documents the string-keyed associative array structure and aligns with PHPStan best practices.composer.json (1)
27-31: LGTM! Dev dependency updates aligned with PR objectives.The PHPStan update to ^0.2.0 aligns with the stated goal of updating to PHPStan 2.0, and the restoration of mockery/nette-di ensures test infrastructure remains intact.
src/Entity/Storno.php (1)
31-33: LGTM! Explicit array shape improves static analysis.The structured array type annotation provides clearer documentation and better static analysis support without any runtime changes.
src/Entity/PaymentStatus.php (1)
29-31: LGTM! Explicit array shape improves documentation.The structured array type annotation aligns with the pattern used across other entity classes and provides better static analysis support.
src/Entity/AbstractPayment.php (2)
13-13: LGTM! Helpful reference link for currency codes.Adding the ISO 4217 Wikipedia link provides useful context for the currency field.
122-132: LGTM! Explicit array shape with nullable types.The structured array type annotation clearly documents the return structure and properly reflects that all fields can be null, improving both documentation and static analysis.
src/Entity/RecurringPayment.php (1)
47-61: Excellent PHPDoc improvement!The detailed array shape documentation enhances type safety and provides clear guidance for static analysis tools.
src/Entity/Payment.php (2)
207-221: Good defensive coding withisset()checks.The conditional inclusion of optional fields using
isset()correctly handles uninitialized properties in the serialization path.
173-195: Accurate PHPDoc for array shape.The detailed array shape documentation correctly marks the new fields as optional and documents their serialized string representations.
| private bool $chargeUnregulatedCardFees; | ||
|
|
||
| private string $urlPaid; | ||
|
|
||
| private string $urlCancelled; | ||
|
|
||
| private string $urlPending; | ||
|
|
There was a problem hiding this comment.
Uninitialized typed properties will cause runtime errors.
These properties lack default values and are not initialized in the constructor. Calling their getters (lines 132-170) before their setters will throw a TypeError because PHP typed properties must be initialized before access.
Apply this diff to provide safe defaults:
- private bool $chargeUnregulatedCardFees;
+ private ?bool $chargeUnregulatedCardFees = null;
- private string $urlPaid;
+ private ?string $urlPaid = null;
- private string $urlCancelled;
+ private ?string $urlCancelled = null;
- private string $urlPending;
+ private ?string $urlPending = null;Then update the getter return types accordingly:
- public function isChargeUnregulatedCardFees(): bool
+ public function isChargeUnregulatedCardFees(): ?bool
- public function getUrlPaid(): string
+ public function getUrlPaid(): ?string
- public function getUrlCancelled(): string
+ public function getUrlCancelled(): ?string
- public function getUrlPending(): string
+ public function getUrlPending(): ?stringCommittable suggestion skipped: line range outside the PR's diff.
| * @return array{ | ||
| * transId: string, | ||
| * } | ||
| **/ |
There was a problem hiding this comment.
Fix non-standard docblock closing syntax.
The docblock closing uses **/ instead of the standard */.
Apply this diff:
- **/
+ */📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **/ | |
| */ |
🤖 Prompt for AI Agents
In src/Entity/PaymentStatus.php around line 32, the docblock closing uses
non-standard "**/" instead of the correct "*/"; edit the file and replace the
incorrect closing token with the standard "*/" so the docblock is properly
terminated and tools/parser recognize it.
| /** | ||
| * @return mixed[] | ||
| * @return array{ | ||
| * amount: float, | ||
| * curr: string, | ||
| * transId: string, | ||
| * refId: ?string | ||
| * } |
There was a problem hiding this comment.
Type mismatch in PHPDoc for amount field.
The docblock declares amount: float, but the property is typed as int (Line 11) and is returned as-is without conversion. This creates a documentation inconsistency.
Apply this diff to correct the documentation:
/**
* @return array{
- * amount: float,
+ * amount: int,
* curr: string,
* transId: string,
* refId: ?string
* }
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /** | |
| * @return mixed[] | |
| * @return array{ | |
| * amount: float, | |
| * curr: string, | |
| * transId: string, | |
| * refId: ?string | |
| * } | |
| /** | |
| * @return array{ | |
| * amount: int, | |
| * curr: string, | |
| * transId: string, | |
| * refId: ?string | |
| * } |
🤖 Prompt for AI Agents
In src/Entity/Refund.php around lines 60 to 66, the PHPDoc declares amount as
float while the Refund::$amount property is typed int (line 11) and returned
without conversion; update the PHPDoc to declare amount: int to match the
property (or alternatively cast/convert the property to float at return),
ensuring the docblock and actual return type are consistent.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
composer.json(1 hunks)phpstan.neon(1 hunks)src/Entity/AbstractEntity.php(1 hunks)src/Entity/AbstractPayment.php(2 hunks)src/Entity/Payment.php(4 hunks)src/Entity/PaymentStatus.php(1 hunks)src/Entity/RecurringPayment.php(1 hunks)src/Entity/Refund.php(2 hunks)src/Entity/Storno.php(1 hunks)tests/Cases/E2E/payments.php(1 hunks)tests/Cases/Entity/Payment.phpt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/Cases/E2E/payments.php (1)
src/Http/Response.php (1)
getData(34-37)
src/Entity/Payment.php (2)
src/Entity/Codes/LangCode.php (1)
LangCode(5-13)src/Entity/AbstractPayment.php (1)
toArray(134-147)
🔇 Additional comments (15)
phpstan.neon (1)
17-17: LGTM! Good addition of test coverage to static analysis.Extending PHPStan scanning to include test cases helps maintain type safety and code quality across the entire codebase.
src/Entity/AbstractEntity.php (1)
9-9: LGTM! More precise type annotation.The change from
mixed[]toarray<string, mixed>better reflects thattoArray()returns an associative array with string keys, improving type safety with PHPStan 2.0.src/Entity/PaymentStatus.php (1)
29-32: LGTM! Clear structured array documentation.The precise array shape documentation helps developers understand the return structure and enables better static analysis with PHPStan 2.0.
src/Entity/Refund.php (1)
13-13: LGTM! Cleaner documentation reference.Using
@seewith a link is a good pattern for referencing external standards like ISO 4217.src/Entity/RecurringPayment.php (1)
48-61: LGTM! Comprehensive array shape documentation.The detailed PHPDoc accurately describes the complete structure returned by
toArray(), including both inherited fields and the specificprepareOnlyandinitRecurringIdfields added by this class.tests/Cases/E2E/payments.php (1)
48-48: LGTM! Good defensive assertion.Adding an explicit check for
transIdexistence before accessing it improves test robustness and makes test failures more informative if the API response structure changes.src/Entity/AbstractPayment.php (2)
13-13: LGTM! Cleaner documentation reference.Using
@seewith a link is a good pattern for referencing external standards like ISO 4217, consistent with the approach inRefund.php.
122-132: LGTM! Accurate array shape documentation.The PHPDoc correctly describes all fields returned by
toArray()with their nullable types, matching the property declarations and implementation.composer.json (1)
27-27: Confirm PHPStan 2.0 analysis passes with stricter rules.Upgrading contributte/phpstan to ^0.2.0 brings PHPStan 2.0 with phpstan/phpstan: ^2.0.1, which introduces stricter type checks. Verify that the codebase passes PHPStan analysis at level 9 without introducing new suppressions:
vendor/bin/phpstan analyse --no-progresssrc/Entity/Storno.php (1)
30-34: LGTM! Improved type documentation.The precise array shape documentation enhances static analysis capabilities and aligns with PHPStan 2.0 requirements.
tests/Cases/Entity/Payment.phpt (2)
142-153: LGTM! Test coverage for chargeUnregulatedCardFees.The test correctly verifies that the boolean property is serialized as the string 'true' in the array output, consistent with other boolean property tests in this file.
155-174: LGTM! Comprehensive test coverage for URL parameters.The test thoroughly verifies all three URL setters and their serialization, using defensive assertions to check key presence before value equality.
src/Entity/Payment.php (3)
16-17: Minor docblock improvement.The
@seeannotation provides a more formal reference to the ISO 639-1 standard. This is a good documentation practice.
132-170: Getter/setter implementation follows class patterns.The new accessor methods are consistent with existing code style and naming conventions. However, their correctness depends on fixing the uninitialized property issue flagged above.
196-224: toArray() implementation correctly handles optional fields.The method properly uses
isset()checks before accessing the new properties, avoiding initialization errors. The boolean-to-string conversion is consistent with existing fields, and the array merge preserves parent data.The updated PHPDoc accurately reflects the optional nature of the new fields using the
?syntax for optional keys.
| private bool $chargeUnregulatedCardFees; | ||
|
|
||
| private string $urlPaid; | ||
|
|
||
| private string $urlCancelled; | ||
|
|
||
| private string $urlPending; |
There was a problem hiding this comment.
Uninitialized typed properties will cause fatal errors.
The new properties are declared as non-nullable types without default values, creating uninitialized typed properties. Calling the getter methods (lines 132-170) before the setters will trigger: Error: Typed property must not be accessed before initialization.
This breaks the pattern established by existing properties like prepareOnly, preauth, etc., which all have default values.
Apply this diff to make properties nullable with safe defaults:
- private bool $chargeUnregulatedCardFees;
+ private ?bool $chargeUnregulatedCardFees = null;
- private string $urlPaid;
+ private ?string $urlPaid = null;
- private string $urlCancelled;
+ private ?string $urlCancelled = null;
- private string $urlPending;
+ private ?string $urlPending = null;Then update getter return types to match:
public function isChargeUnregulatedCardFees(): ?bool
{
return $this->chargeUnregulatedCardFees;
}
public function getUrlPaid(): ?string
{
return $this->urlPaid;
}
public function getUrlCancelled(): ?string
{
return $this->urlCancelled;
}
public function getUrlPending(): ?string
{
return $this->urlPending;
}🤖 Prompt for AI Agents
In src/Entity/Payment.php around lines 29 to 35, the new properties are declared
as non-nullable without defaults which creates uninitialized typed properties
and will throw if getters are called before setters; change the property
declarations to nullable types with safe defaults (e.g., ?bool
$chargeUnregulatedCardFees = null; ?string $urlPaid = null; ?string
$urlCancelled = null; ?string $urlPending = null) and update the corresponding
getter return types to nullable (isChargeUnregulatedCardFees(): ?bool,
getUrlPaid(): ?string, getUrlCancelled(): ?string, getUrlPending(): ?string) so
they safely return null until set.
| * @return array{ | ||
| * amount: float, | ||
| * curr: string, | ||
| * transId: string, | ||
| * refId: ?string | ||
| * } |
There was a problem hiding this comment.
Fix type mismatch in PHPDoc.
The PHPDoc specifies amount: float but the property is declared as int (line 11) and returned as int (line 71). This inconsistency could cause confusion and PHPStan warnings.
Apply this diff to fix the type mismatch:
/**
* @return array{
- * amount: float,
+ * amount: int,
* curr: string,
* transId: string,
* refId: ?string
* }
*/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * @return array{ | |
| * amount: float, | |
| * curr: string, | |
| * transId: string, | |
| * refId: ?string | |
| * } | |
| * @return array{ | |
| * amount: int, | |
| * curr: string, | |
| * transId: string, | |
| * refId: ?string | |
| * } |
🤖 Prompt for AI Agents
In src/Entity/Refund.php around lines 61 to 66, the PHPDoc return type declares
amount as float but the property (line 11) and the actual return value (line 71)
are ints; update the PHPDoc to use amount: int so the docblock matches the
property and returned type (ensure the rest of the return shape remains
unchanged).
1 participant
Closes #45 #47
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Chores