PayPal, Stripe, Robokassa and YooKassa merged into one branch#5
Conversation
…changes in common interfaces + integration tests for PayPal and Robokassa + README updates.
| { | ||
| private const INVOICE_API_BASE_URI = 'https://services.robokassa.ru/InvoiceServiceWebApi/api'; | ||
| private const REFUND_API_BASE_URI = 'https://services.robokassa.ru/RefundService/Refund'; | ||
| private const XML_API_BASE_URI = 'https://auth.robokassa.ru/Merchant/WebService/Service.asmx'; |
There was a problem hiding this comment.
возможно лучше добавить способ переопределять эти значения без обновления версии библиотеки
There was a problem hiding this comment.
Для единообразия - вынес endpoint’ы в отдельные endpoints-объекты с значениями по-умолчанию + возможностью переопределять передачей в конструктор конкретного Gateway.
| $xmlString = (string) $response->getBody(); | ||
|
|
||
| $xml = @simplexml_load_string($xmlString); | ||
| if ($xml === false) { |
There was a problem hiding this comment.
Тут потеряем причину ошибки и сам payload, будет сложновато дебажить
There was a problem hiding this comment.
Да, согласен, добавил обработку/логирование.
| return 'CANCELLED'; | ||
| } | ||
|
|
||
| return 'UNKNOWN'; |
There was a problem hiding this comment.
Стоит ли оформить в виде констант, чтобы избежать возможных опечаток?
There was a problem hiding this comment.
вынес статусы в константы
| ], | ||
| ]); | ||
| } catch (PaymentException $e) { | ||
| return PaymentIntent::fromArray([ |
There was a problem hiding this comment.
Тут по возвращаемому payment intent не совсем очевидно, прошла ли отмена на самом деле
There was a problem hiding this comment.
Поправил: в metadata теперь есть cancel_confirmed=true/false; при ошибке деактивации можно получить текущий статус через retrievePaymentIntent() и вернуть состояние в metadata.
| final class PayPalGateway extends AbstractGateway | ||
| { | ||
| private const SANDBOX_BASE_URI = 'https://api-m.sandbox.paypal.com'; | ||
| private const LIVE_BASE_URI = 'https://api-m.paypal.com'; |
There was a problem hiding this comment.
возможно лучше добавить способ переопределять эти значения без обновления версии библиотек
There was a problem hiding this comment.
Аналогично с робокассой - переделал на общий интерфейс работы с эндпоинтами для всех платежек.
- Introduce typed endpoint configuration objects for PayPal, Stripe, Robokassa, and YooKassa to allow overriding default URLs and remove duplicated strings. - Replace magic status strings with named constants. - Improve XML parsing error handling and logging in Robokassa. - Return clearer cancellation results for cancelPaymentIntent() with cancel_confirmed metadata. - Update README and code comments to match current behaviour.
| } | ||
| ``` | ||
| #### Step 7: Webhooks (optional) | ||
| This library does not include webhook signature verification or event parsing. |
There was a problem hiding this comment.
Is verification common? If so, can we abstract it out somehow?
There was a problem hiding this comment.
Pull request overview
This PR consolidates PayPal (REST API v2), Stripe, Robokassa, and YooKassa work into a single branch and expands the test/docs surface accordingly.
Changes:
- Added new gateway implementations (Robokassa, YooKassa) and refactored PayPal to API v2 flows; introduced endpoint value objects for configurable base URLs.
- Expanded test support utilities and added/updated unit + integration tests (PayPal/Robokassa), plus config templates for integration credentials.
- Updated documentation (README) and dev dependencies (Symfony HTTP client) to support integration testing.
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
src/Gateways/PayPalGateway.php |
Reworked PayPal to REST API v2; adds endpoint configuration + new flow helpers. |
src/Gateways/RobokassaGateway.php |
New Robokassa gateway implementation (invoice, status, refund APIs). |
src/Gateways/YooKassaGateway.php |
New YooKassa gateway implementation (payments/refunds). |
src/Gateways/StripeGateway.php |
Adds configurable endpoints injection. |
src/Endpoints/PayPalEndpoints.php |
New endpoints value object. |
src/Endpoints/RobokassaEndpoints.php |
New endpoints value object. |
src/Endpoints/StripeEndpoints.php |
New endpoints value object. |
src/Endpoints/YooKassaEndpoints.php |
New endpoints value object. |
tests/Support/TestHttpClient.php |
Enhances test HTTP client with a response queue + raw responses. |
tests/Support/IntegrationConfig.php |
Loads integration secrets from local ignored config files. |
tests/Integration/PayPalGatewayIntegrationTest.php |
Adds PayPal integration tests (skipped if not configured). |
tests/Integration/RobokassaGatewayIntegrationTest.php |
Adds Robokassa integration tests (skipped if not configured). |
tests/Gateways/PayPalGatewayTest.php |
Updates unit tests to PayPal v2 order/refund flows. |
tests/Gateways/RobokassaGatewayTest.php |
Adds Robokassa unit tests. |
tests/Gateways/StripeGatewayCaptureMethodTest.php |
Adds Stripe capture-method behavior test. |
tests/Gateways/YooKassaGatewayTest.php |
Adds YooKassa unit tests. |
tests/config/paypal.php.dist |
Integration config template for PayPal. |
tests/config/robokassa.php.dist |
Integration config template for Robokassa. |
composer.json |
Adds symfony/http-client to require-dev. |
README.md |
Updates supported gateways and documents endpoints + testing. |
.gitignore |
Ignores local integration configs while allowing *.dist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private ?PayPalEndpoints $endpoints = new PayPalEndpoints() | ||
| ) { |
There was a problem hiding this comment.
Constructor uses a new PayPalEndpoints() default value for a parameter. In PHP, default parameter values must be constant expressions, so new ... here will cause a fatal error. Make the parameter default to null and assign $this->endpoints = $endpoints ?? new PayPalEndpoints(); (and consider making the property non-nullable once initialized).
| private ?PayPalEndpoints $endpoints = new PayPalEndpoints() | |
| ) { | |
| private ?PayPalEndpoints $endpoints = null | |
| ) { | |
| $this->endpoints = $endpoints ?? new PayPalEndpoints(); |
| public function retrieveCustomer(string $customerId): Customer | ||
| { | ||
| throw new \RuntimeException('YooKassa API does not support retrieving customer'); | ||
| } | ||
|
|
||
| public function updateCustomer(Customer $customer): Customer | ||
| { | ||
| throw new \RuntimeException('YooKassa API does not support updating customer'); | ||
| } | ||
|
|
||
| public function deleteCustomer(string $customerId): void | ||
| { | ||
| throw new \RuntimeException('YooKassa API does not support delete customer'); | ||
| } |
There was a problem hiding this comment.
Customer operations throw \RuntimeException, while other gateways (and the library’s error model) use PaymentException for gateway/API limitations. To keep exception handling consistent for consumers, prefer throwing Yiisoft\Payments\Exceptions\PaymentException (or implement the same local no-op/placeholder strategy as PayPal/Robokassa).
| $data = [ | ||
| 'payment_id' => $paymentId, | ||
| 'amount' => $this->createAmountValue($params['amount'], $params['currency']), |
There was a problem hiding this comment.
createRefund() reads $params['amount'] and $params['currency'] unconditionally, which will raise undefined-index notices if a caller omits them. Please validate required keys and throw a PaymentException with a clear message when missing/invalid (or make the refund amount optional if the provider supports full-refund by default).
| $data = [ | |
| 'payment_id' => $paymentId, | |
| 'amount' => $this->createAmountValue($params['amount'], $params['currency']), | |
| if (!array_key_exists('amount', $params) || !array_key_exists('currency', $params)) { | |
| throw new PaymentException('Refund "amount" and "currency" parameters are required.'); | |
| } | |
| $amount = $params['amount']; | |
| $currency = $params['currency']; | |
| if (!is_int($amount) || $amount <= 0) { | |
| throw new PaymentException('Refund "amount" must be a positive integer representing minor currency units.'); | |
| } | |
| if (!is_string($currency) || $currency === '') { | |
| throw new PaymentException('Refund "currency" must be a non-empty string.'); | |
| } | |
| $data = [ | |
| 'payment_id' => $paymentId, | |
| 'amount' => $this->createAmountValue($amount, $currency), |
| public function createRefund(string $paymentIntentId, int $amount = null, array $params = []): array | ||
| { | ||
| $captureId = (string) ($params['capture_id'] ?? $paymentIntentId); | ||
|
|
||
| $data = []; |
There was a problem hiding this comment.
createRefund() signature is not compatible with PaymentGatewayInterface::createRefund(string $paymentIntentId, array $params = []): array and also declares int $amount = null (non-nullable type with null default), which is a fatal error. Keep the interface signature and pass amount via $params['amount'] (or add a separate helper), and if you keep an amount parameter it must be ?int $amount = null and still be signature-compatible.
| public function createRefund(string $paymentIntentId, int $amount = null, array $params = []): array | |
| { | |
| $captureId = (string) ($params['capture_id'] ?? $paymentIntentId); | |
| $data = []; | |
| public function createRefund(string $paymentIntentId, array $params = []): array | |
| { | |
| $captureId = (string) ($params['capture_id'] ?? $paymentIntentId); | |
| $data = []; | |
| $amount = array_key_exists('amount', $params) ? (int) $params['amount'] : null; |
| /** | ||
| * Adds authorization and request id headers. | ||
| */ | ||
| protected function createRequest(string $method, string $endpoint, array $data = null): \Psr\Http\Message\RequestInterface |
There was a problem hiding this comment.
createRequest() overrides AbstractGateway::createRequest(string $method, string $endpoint, array $data = []), but here it is declared as array $data = null and then forwards $data to the parent. This is signature-incompatible (and null cannot be the default for a non-nullable array), which will cause a fatal error. Match the parent signature (array $data = []) and avoid passing null into the parent.
| protected function createRequest(string $method, string $endpoint, array $data = null): \Psr\Http\Message\RequestInterface | |
| protected function createRequest(string $method, string $endpoint, array $data = []): \Psr\Http\Message\RequestInterface |
| amount: 1000, | ||
| params: ['op_key' => 'OP-123'] |
There was a problem hiding this comment.
This test calls RobokassaGateway::createRefund() with an amount: argument, but PaymentGatewayInterface::createRefund() only accepts (string $paymentIntentId, array $params = []). Unless the interface is changed (breaking change), adjust the test to pass amount via $params and keep the gateway signature interface-compatible.
| amount: 1000, | |
| params: ['op_key' => 'OP-123'] | |
| params: [ | |
| 'amount' => 1000, | |
| 'op_key' => 'OP-123', | |
| ], |
| * All metadata is passed to Robokassa as-is (except reserved keys shown above). | ||
| */ | ||
| public function createPaymentIntent(PaymentIntent $paymentIntent): PaymentIntent | ||
| { |
There was a problem hiding this comment.
PaymentIntent::$amount and $currency are nullable in the model, but this code calls self::formatAmount($paymentIntent->amount) and strtoupper($paymentIntent->currency) unconditionally, which will throw a TypeError if either is null. Validate required fields (amount/currency) and throw a PaymentException/InvalidArgumentException with a clear message when missing.
| { | |
| { | |
| if ($paymentIntent->amount === null) { | |
| throw new PaymentException('PaymentIntent amount must not be null when creating a Robokassa invoice.'); | |
| } | |
| if ($paymentIntent->currency === null) { | |
| throw new PaymentException('PaymentIntent currency must not be null when creating a Robokassa invoice.'); | |
| } |
| public function createRefund(string $paymentIntentId, int $amount = null, array $params = []): array | ||
| { |
There was a problem hiding this comment.
createRefund() signature is not compatible with PaymentGatewayInterface::createRefund(string $paymentIntentId, array $params = []): array and it also declares int $amount = null (non-nullable type with null default), which is a fatal error. Keep the interface signature and move amount into $params['amount'] (or provide a separate helper method).
| * - cancel_url: URL where PayPal will redirect the payer if they cancel (web flow) | ||
| */ | ||
| public function createPaymentIntent(PaymentIntent $paymentIntent): PaymentIntent | ||
| { |
There was a problem hiding this comment.
PaymentIntent::$currency and $amount are nullable in the model, but this code calls strtoupper($paymentIntent->currency) and self::formatAmount($paymentIntent->amount) unconditionally, which will throw a TypeError if either is null. Consider validating required fields up front and throwing a PaymentException/InvalidArgumentException with a clear message when amount/currency are missing.
| { | |
| { | |
| if ($paymentIntent->currency === null || $paymentIntent->amount === null) { | |
| throw new PaymentException('Payment currency and amount are required to create a PayPal payment intent.'); | |
| } |
| public function createCustomer(Customer $customer): Customer | ||
| { | ||
| return $customer; | ||
| } | ||
|
|
||
| public function retrieveCustomer(string $customerId): Customer | ||
| { | ||
| throw new \RuntimeException('YooKassa API does not support retrieving customer'); | ||
| } | ||
|
|
||
| public function updateCustomer(Customer $customer): Customer | ||
| { | ||
| throw new \RuntimeException('YooKassa API does not support updating customer'); | ||
| } | ||
|
|
||
| public function deleteCustomer(string $customerId): void | ||
| { | ||
| throw new \RuntimeException('YooKassa API does not support delete customer'); | ||
| } | ||
|
|
||
| public function createPaymentMethod(PaymentMethod $paymentMethod): PaymentMethod | ||
| { | ||
| return $paymentMethod; | ||
| } |
There was a problem hiding this comment.
createCustomer() / createPaymentMethod() currently return the input model unchanged, which can leave id as null and makes it hard for callers to reference the created resource consistently. Other “no-op” gateways in this repo (e.g. PayPal/Robokassa) generate a synthetic ID when missing—consider doing the same here for consistency.
| /.phpunit.cache | ||
| /.phpunit.result.cache | ||
|
|
||
| # Integration test configs (do not commit secrets) |
There was a problem hiding this comment.
Suggest move it to separate .gitignore into /tests/config.
| self::assertHttpsUri($this->baseUri); | ||
| } | ||
|
|
||
| private static function assertHttpsUri(string $uri): void |
There was a problem hiding this comment.
Extract assertHttpsUri to separate class, Seems, this code is equal in all endpoints classes.
| * - return_url: URL where PayPal will redirect the payer after approval (web flow) | ||
| * - cancel_url: URL where PayPal will redirect the payer if they cancel (web flow) | ||
| */ | ||
| public function createPaymentIntent(PaymentIntent $paymentIntent): PaymentIntent |
There was a problem hiding this comment.
| public function createPaymentIntent(PaymentIntent $paymentIntent): PaymentIntent | |
| public function createPaymentIntent(PaymentIntent $intent): PaymentIntent |
| * For PayPal web flows, payer approval happens outside of the API (via approval link). | ||
| * This method simply re-fetches the current order state. | ||
| */ | ||
| public function confirmPaymentIntent(string $paymentIntentId, array $params = []): PaymentIntent |
There was a problem hiding this comment.
| public function confirmPaymentIntent(string $paymentIntentId, array $params = []): PaymentIntent | |
| public function confirmPaymentIntent(string $intentId, array $params = []): PaymentIntent |
yookassa-draft.README.mddocumentation was updated and expanded.