Use prefixed SumUp SDK on PHP 8.2+#29
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: wcpos/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
📝 Walkthrough<review_stack_artifact> </review_stack_artifact> ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e51e4ccf9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
Addressed the actionable review feedback in one commit.
Excluded threads: none. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@includes/Gateway.php`:
- Around line 154-155: The call to $this->get_sdk_status_html() in class Gateway
is calling a non-existent method and will fatal; either implement a
get_sdk_status_html() method on the Gateway class (or its parent/trait) that
returns the expected HTML string, or replace the call with the correct existing
method name if misnamed (search for similar helpers), and ensure the new/renamed
method is declared in the same class or an imported trait so
includes/Gateway.php no longer references an undefined symbol.
In `@includes/Services/WordPressHttpReaderApiClient.php`:
- Around line 126-140: cancel_checkout currently treats a false response from
parent::post() as success because ! is_wp_error(false) is true; update
cancel_checkout (the method calling
parent::post("/merchants/{$this->get_merchant_id()}/readers/{$reader_id}/terminate"))
to explicitly check for a boolean false return before treating the call as
successful (e.g. if $response === false || is_wp_error($response) return false;
otherwise return true), ensuring both network failure (false) and WP_Error are
handled as failures.
In `@tools/generate-prefixed-sumup-autoload.php`:
- Line 76: file_put_contents call that writes $autoload to $root .
'/vendor_prefixed/sumup-sdk-autoload.php' can fail silently; check the return
value and fail fast: after calling file_put_contents($root .
'/vendor_prefixed/sumup-sdk-autoload.php', $autoload) verify it returned !==
false (and ideally matched strlen($autoload)), and if it failed throw an
exception or call exit(1) with a clear error via error_log/processLogger so the
script stops rather than leaving a stale/missing autoload file.
In `@tools/prefix-sumup-sdk-namespaces.php`:
- Around line 22-27: Guard file read/replace/write so a single failure won't
corrupt the vendored SDK: after calling file_get_contents($path) check for false
and skip/log the file; perform each preg_replace into temporary vars and verify
the return is not null (and check preg_last_error()) before applying the next
replacement; only call file_put_contents($path, $contents) when all replacements
succeeded and file_put_contents does not return false; use $path, $contents and
the three preg_replace calls as the unique references to locate and protect
these operations.
In `@vendor_prefixed/sumup-sdk/HttpClient/CurlClient.php`:
- Around line 83-87: The code calls curl_init() in CurlClient and immediately
passes its return ($ch) into curl_setopt() without checking for failure; update
the request path in the CurlClient implementation (the method that initializes
$ch) to check if $ch === false and throw the SDK's ConnectionException with a
clear message and diagnostic details (use curl_error() and curl_errno() when
available) before any curl_setopt() calls so failures are handled explicitly
instead of causing downstream warnings/errors.
In `@vendor_prefixed/sumup-sdk/Memberships/Memberships.php`:
- Around line 87-100: The current serialization omits explicit nulls for
resource.parent.id/resource.parent.type because it uses isset/http_build_query;
add explicit presence flags (e.g., public bool $resourceParentIdPresent = false
and public bool $resourceParentTypePresent = false) and update the
request-parameter assembly logic to include the resource.parent.id and
resource.parent.type keys when their corresponding Present flag is true (even if
the value is null), rather than relying on isset; apply the same change for the
other pair referenced at lines 184-195 so callers can represent “explicit null”
parent filters on the wire.
In `@vendor_prefixed/sumup-sdk/Readers/Readers.php`:
- Around line 102-105: The fromArray method in Readers::fromArray currently
ignores the input array and should fail-fast on unexpected keys; update
Readers::fromArray to check if the provided $data is empty (or contains only
allowed keys) and throw a clear exception (e.g., InvalidArgumentException) when
non-empty/unexpected keys are present so callers are not silently ignored;
reference the Readers::fromArray method and the $data parameter when adding the
guard and exception.
In `@vendor_prefixed/sumup-sdk/RequestEncoder.php`:
- Around line 21-23: The early return for arrays in RequestEncoder.php skips
normalization (the if (is_array($value)) { return $value; }) so nested
enums/objects are not processed; change this to route arrays through the
normalizer instead (e.g., return $this->normalize($value) or
self::normalize($value) depending on context) so each array element is
normalized; apply the same change to the other array-returning branch referenced
(lines ~44-51) so both code paths call the normalize method rather than
returning arrays raw.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: wcpos/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d2489d6-c70c-43a9-86e6-36e24cae2867
📒 Files selected for processing (183)
composer.jsondocs/adr/0001-sumup-sdk-hybrid.mdincludes/Gateway.phpincludes/Services/ReaderApiClientFactory.phpincludes/Services/ReaderApiClientInterface.phpincludes/Services/ReaderService.phpincludes/Services/SdkAvailability.phpincludes/Services/SdkReaderApiClient.phpincludes/Services/WordPressHttpReaderApiClient.phpphp-scoper/composer.jsonphp-scoper/scoper.inc.phpsumup-terminal-for-woocommerce.phptests/regression/activation-guard.phptests/regression/prefixed-sdk-autoload-is-conditional.phptests/regression/sdk-compatibility-message.phptests/regression/sdk-reader-normalization.phptools/generate-prefixed-sumup-autoload.phptools/prefix-sumup-sdk-namespaces.phpvendor_prefixed/sumup-sdk-autoload.phpvendor_prefixed/sumup-sdk/ApiVersion.phpvendor_prefixed/sumup-sdk/Checkouts/Checkouts.phpvendor_prefixed/sumup-sdk/Customers/Customers.phpvendor_prefixed/sumup-sdk/Exception/ApiException.phpvendor_prefixed/sumup-sdk/Exception/ArgumentException.phpvendor_prefixed/sumup-sdk/Exception/ConfigurationException.phpvendor_prefixed/sumup-sdk/Exception/ConnectionException.phpvendor_prefixed/sumup-sdk/Exception/ErrorEnvelope.phpvendor_prefixed/sumup-sdk/Exception/SDKException.phpvendor_prefixed/sumup-sdk/Exception/UnexpectedApiException.phpvendor_prefixed/sumup-sdk/ExceptionMessages.phpvendor_prefixed/sumup-sdk/HttpClient/CurlClient.phpvendor_prefixed/sumup-sdk/HttpClient/GuzzleClient.phpvendor_prefixed/sumup-sdk/HttpClient/HttpClientInterface.phpvendor_prefixed/sumup-sdk/HttpClient/RequestHeaders.phpvendor_prefixed/sumup-sdk/HttpClient/RequestOptions.phpvendor_prefixed/sumup-sdk/HttpClient/Response.phpvendor_prefixed/sumup-sdk/Hydrator.phpvendor_prefixed/sumup-sdk/Members/Members.phpvendor_prefixed/sumup-sdk/Memberships/Memberships.phpvendor_prefixed/sumup-sdk/Merchants/Merchants.phpvendor_prefixed/sumup-sdk/Payouts/Payouts.phpvendor_prefixed/sumup-sdk/Readers/Readers.phpvendor_prefixed/sumup-sdk/Receipts/Receipts.phpvendor_prefixed/sumup-sdk/RequestEncoder.phpvendor_prefixed/sumup-sdk/ResponseDecoder.phpvendor_prefixed/sumup-sdk/Roles/Roles.phpvendor_prefixed/sumup-sdk/SdkInfo.phpvendor_prefixed/sumup-sdk/Services/SumUpService.phpvendor_prefixed/sumup-sdk/Shared/Shared.phpvendor_prefixed/sumup-sdk/SumUp.phpvendor_prefixed/sumup-sdk/Transactions/Transactions.phpvendor_prefixed/sumup-sdk/Types/Address.phpvendor_prefixed/sumup-sdk/Types/AddressLegacy.phpvendor_prefixed/sumup-sdk/Types/BadRequest.phpvendor_prefixed/sumup-sdk/Types/BadRequestErrors.phpvendor_prefixed/sumup-sdk/Types/BadRequestErrorsType.phpvendor_prefixed/sumup-sdk/Types/BasePerson.phpvendor_prefixed/sumup-sdk/Types/Branding.phpvendor_prefixed/sumup-sdk/Types/BusinessProfile.phpvendor_prefixed/sumup-sdk/Types/Card.phpvendor_prefixed/sumup-sdk/Types/CardExpiryMonth.phpvendor_prefixed/sumup-sdk/Types/CardResponse.phpvendor_prefixed/sumup-sdk/Types/CardResponseType.phpvendor_prefixed/sumup-sdk/Types/CardType.phpvendor_prefixed/sumup-sdk/Types/Checkout.phpvendor_prefixed/sumup-sdk/Types/CheckoutAccepted.phpvendor_prefixed/sumup-sdk/Types/CheckoutAcceptedNextStep.phpvendor_prefixed/sumup-sdk/Types/CheckoutCreateRequest.phpvendor_prefixed/sumup-sdk/Types/CheckoutCreateRequestCurrency.phpvendor_prefixed/sumup-sdk/Types/CheckoutCreateRequestPurpose.phpvendor_prefixed/sumup-sdk/Types/CheckoutCurrency.phpvendor_prefixed/sumup-sdk/Types/CheckoutStatus.phpvendor_prefixed/sumup-sdk/Types/CheckoutSuccess.phpvendor_prefixed/sumup-sdk/Types/CheckoutSuccessPaymentInstrument.phpvendor_prefixed/sumup-sdk/Types/ClassicMerchantIdentifiers.phpvendor_prefixed/sumup-sdk/Types/Company.phpvendor_prefixed/sumup-sdk/Types/CompanyIdentifier.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutError.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutErrorErrors.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutRequest.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutRequestAade.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutRequestAffiliate.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutRequestCardType.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutRequestTotalAmount.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutResponse.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutResponseData.phpvendor_prefixed/sumup-sdk/Types/CreateReaderCheckoutUnprocessableEntity.phpvendor_prefixed/sumup-sdk/Types/CreateReaderTerminateError.phpvendor_prefixed/sumup-sdk/Types/CreateReaderTerminateErrorErrors.phpvendor_prefixed/sumup-sdk/Types/CreateReaderTerminateUnprocessableEntity.phpvendor_prefixed/sumup-sdk/Types/Customer.phpvendor_prefixed/sumup-sdk/Types/DetailsError.phpvendor_prefixed/sumup-sdk/Types/Device.phpvendor_prefixed/sumup-sdk/Types/ElvCardAccount.phpvendor_prefixed/sumup-sdk/Types/Error.phpvendor_prefixed/sumup-sdk/Types/ErrorExtended.phpvendor_prefixed/sumup-sdk/Types/ErrorForbidden.phpvendor_prefixed/sumup-sdk/Types/Event.phpvendor_prefixed/sumup-sdk/Types/EventStatus.phpvendor_prefixed/sumup-sdk/Types/EventType.phpvendor_prefixed/sumup-sdk/Types/FinancialPayout.phpvendor_prefixed/sumup-sdk/Types/FinancialPayoutStatus.phpvendor_prefixed/sumup-sdk/Types/FinancialPayoutType.phpvendor_prefixed/sumup-sdk/Types/HostedCheckout.phpvendor_prefixed/sumup-sdk/Types/Invite.phpvendor_prefixed/sumup-sdk/Types/Link.phpvendor_prefixed/sumup-sdk/Types/ListPersonsResponseBody.phpvendor_prefixed/sumup-sdk/Types/MandatePayload.phpvendor_prefixed/sumup-sdk/Types/MandatePayloadType.phpvendor_prefixed/sumup-sdk/Types/MandateResponse.phpvendor_prefixed/sumup-sdk/Types/MandateResponseStatus.phpvendor_prefixed/sumup-sdk/Types/Member.phpvendor_prefixed/sumup-sdk/Types/MemberStatus.phpvendor_prefixed/sumup-sdk/Types/Membership.phpvendor_prefixed/sumup-sdk/Types/MembershipResource.phpvendor_prefixed/sumup-sdk/Types/MembershipStatus.phpvendor_prefixed/sumup-sdk/Types/MembershipUser.phpvendor_prefixed/sumup-sdk/Types/MembershipUserClassic.phpvendor_prefixed/sumup-sdk/Types/Merchant.phpvendor_prefixed/sumup-sdk/Types/NotFound.phpvendor_prefixed/sumup-sdk/Types/NotFoundErrors.phpvendor_prefixed/sumup-sdk/Types/Ownership.phpvendor_prefixed/sumup-sdk/Types/PaymentInstrumentResponse.phpvendor_prefixed/sumup-sdk/Types/PaymentInstrumentResponseCard.phpvendor_prefixed/sumup-sdk/Types/PaymentInstrumentResponseCardType.phpvendor_prefixed/sumup-sdk/Types/PaymentInstrumentResponseType.phpvendor_prefixed/sumup-sdk/Types/Person.phpvendor_prefixed/sumup-sdk/Types/PersonalDetails.phpvendor_prefixed/sumup-sdk/Types/PersonalIdentifier.phpvendor_prefixed/sumup-sdk/Types/Problem.phpvendor_prefixed/sumup-sdk/Types/ProcessCheckout.phpvendor_prefixed/sumup-sdk/Types/ProcessCheckoutPaymentType.phpvendor_prefixed/sumup-sdk/Types/Product.phpvendor_prefixed/sumup-sdk/Types/Reader.phpvendor_prefixed/sumup-sdk/Types/ReaderDevice.phpvendor_prefixed/sumup-sdk/Types/ReaderDeviceModel.phpvendor_prefixed/sumup-sdk/Types/ReaderStatus.phpvendor_prefixed/sumup-sdk/Types/Receipt.phpvendor_prefixed/sumup-sdk/Types/ReceiptAcquirerData.phpvendor_prefixed/sumup-sdk/Types/ReceiptCard.phpvendor_prefixed/sumup-sdk/Types/ReceiptEvent.phpvendor_prefixed/sumup-sdk/Types/ReceiptEventStatus.phpvendor_prefixed/sumup-sdk/Types/ReceiptEventType.phpvendor_prefixed/sumup-sdk/Types/ReceiptMerchantData.phpvendor_prefixed/sumup-sdk/Types/ReceiptMerchantDataMerchantProfile.phpvendor_prefixed/sumup-sdk/Types/ReceiptMerchantDataMerchantProfileAddress.phpvendor_prefixed/sumup-sdk/Types/ReceiptReader.phpvendor_prefixed/sumup-sdk/Types/ReceiptTransaction.phpvendor_prefixed/sumup-sdk/Types/ReceiptTransactionProcessAs.phpvendor_prefixed/sumup-sdk/Types/Role.phpvendor_prefixed/sumup-sdk/Types/StatusResponse.phpvendor_prefixed/sumup-sdk/Types/StatusResponseData.phpvendor_prefixed/sumup-sdk/Types/StatusResponseDataConnectionType.phpvendor_prefixed/sumup-sdk/Types/StatusResponseDataState.phpvendor_prefixed/sumup-sdk/Types/StatusResponseDataStatus.phpvendor_prefixed/sumup-sdk/Types/Timestamps.phpvendor_prefixed/sumup-sdk/Types/TransactionBase.phpvendor_prefixed/sumup-sdk/Types/TransactionBaseCurrency.phpvendor_prefixed/sumup-sdk/Types/TransactionBasePaymentType.phpvendor_prefixed/sumup-sdk/Types/TransactionBaseStatus.phpvendor_prefixed/sumup-sdk/Types/TransactionCheckoutInfo.phpvendor_prefixed/sumup-sdk/Types/TransactionCheckoutInfoEntryMode.phpvendor_prefixed/sumup-sdk/Types/TransactionEvent.phpvendor_prefixed/sumup-sdk/Types/TransactionEventEventType.phpvendor_prefixed/sumup-sdk/Types/TransactionEventStatus.phpvendor_prefixed/sumup-sdk/Types/TransactionFull.phpvendor_prefixed/sumup-sdk/Types/TransactionFullLocation.phpvendor_prefixed/sumup-sdk/Types/TransactionFullPayoutType.phpvendor_prefixed/sumup-sdk/Types/TransactionFullProcessAs.phpvendor_prefixed/sumup-sdk/Types/TransactionFullSimplePaymentType.phpvendor_prefixed/sumup-sdk/Types/TransactionFullSimpleStatus.phpvendor_prefixed/sumup-sdk/Types/TransactionFullVerificationMethod.phpvendor_prefixed/sumup-sdk/Types/TransactionHistory.phpvendor_prefixed/sumup-sdk/Types/TransactionHistoryCardType.phpvendor_prefixed/sumup-sdk/Types/TransactionHistoryPayoutType.phpvendor_prefixed/sumup-sdk/Types/TransactionHistoryType.phpvendor_prefixed/sumup-sdk/Types/TransactionMixinHistory.phpvendor_prefixed/sumup-sdk/Types/TransactionMixinHistoryPayoutPlan.phpvendor_prefixed/sumup-sdk/Types/TransactionsHistoryLink.phpvendor_prefixed/sumup-sdk/Types/Unauthorized.phpvendor_prefixed/sumup-sdk/Types/UnauthorizedErrors.phpvendor_prefixed/sumup-sdk/Types/UnauthorizedErrorsType.phpvendor_prefixed/sumup-sdk/Version.php
|
Addressed the current readiness audit feedback in one commit.
Excluded threads: none. |
Contributor
Author
|
Addressed the current readiness audit feedback in c37e9ba.
Excluded threads: none. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #28
Validation
composer run lint(fails on the repository's existing PHPCS baseline, including missing package/doc comments and legacy Gateway escaping/doc issues; no syntax/regression failures)php tests/regression/pairing-request-includes-name.phpphp tests/regression/activation-guard.phpphp tests/regression/prefixed-sdk-autoload-is-conditional.phpphp tests/regression/sdk-compatibility-message.phpphp tests/regression/sdk-reader-normalization.phpphp -l sumup-terminal-for-woocommerce.phpfind includes -name '*.php' -print0 | xargs -0 -n1 php -lSummary by CodeRabbit
New Features
Improvements
Documentation
Tests