-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Version 2.0.0 #31
base: main
Are you sure you want to change the base?
Version 2.0.0 #31
Conversation
…doing multiple json_decode
Implementation of truelayer/client SDK
…k-info Settings sidebar permalink info
|
||
update_post_meta( $order_id, '_truelayer_payment_id', $truelayer_payment_id ); | ||
update_post_meta( $order_id, '_truelayer_payment_token', $truelayer_payment_token ); | ||
$build_test_url = Truelayer_Helper_Hosted_Payment_Page_URL::build_hosted_payment_page_url( $order ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove this helper now and use
$payment->hostedPaymentsPage()->returnUri('...')->toUrl();
* | ||
* @return ResponseInterface | ||
*/ | ||
public function sendRequest( RequestInterface $request ): ResponseInterface { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably needs to throw an error here https://www.php-fig.org/psr/psr-18/#error-handling:
A Client MUST throw an instance of Psr\Http\Client\ClientExceptionInterface if and only if it is unable to send the HTTP request at all or if the HTTP response could not be parsed into a PSR-7 response object.
// Create the PSR-7 response. | ||
$response = new Response( $response_code, $response_headers, $response_body, $http_version, $response_reason ); | ||
|
||
$this->logRequest( $uri, $args, $response_body, $response_code ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets remove the response body from this log as it may end up logging PII data.
if ( $request->getBody()->getSize() > 0 ) { | ||
$body = json_decode( (string) $request->getBody(), true ); | ||
if ( ! empty( $body ) ) { | ||
$args['body'] = apply_filters( 'truelayer_request_args', (string) wp_json_encode( $body ) ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets also remove the body from here, unless you want to manually remove any sensitive data from it, such as all the user information.
|
||
/** | ||
* Heler class for the request signing. | ||
* Helper class for the request signing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_tl_signature()
shouldn't be needed anymore and you can drop the direct dependency on the signing package too 👍
|
||
/** | ||
* Heler class for the callback verifying. | ||
* Helper class for the callback verifying. | ||
*/ | ||
class Truelayer_Helper_Verifying { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also won't need to manually verify webhooks anymore, so this class can be removed. You can use the php client to handle webhooks and signatures will be verified under the hood.
Changelog
Note
This update bumps the minimum supported PHP version from 7.4 to 8.1, making it possible that this cant be updated to for some customers still on older version of PHP. Since WordPress itself still supports 7.4.
This update adds in scoping for all the dependencies to put them under our own namespace. This is sadly required to prevent any issues with other plugins using the same composer dependencies as the plugin, but with different versions. This could cause fatal errors if we don't add the scoping.
With that there is a needed change to the deployment script, and the
.distignore
files, to no longer include the vendor folder in the release, but rather use the autoloader from the scoped dependencies instead.We have this setup in other plugins that we develop, but this is by far the one with the most dependencies, and the most customization in the
scoper.custom.php
file. I have added exclusions/exposure for all things I ran into issues with while testing, but with the amount of files there is a potential for something to have been missed.One suggestion before pushing an update to WordPress would be to change the stable tag in the readme.txt file to the previous version again (
1.4.2
).And change the version number in the plugin header to
2.0.0-beta.1
to create a beta release on WordPress. Allowing us to download the zip file generated by the CI script and WordPress to check that everything has built correctly and that everything works as expected. And then make new release where we set the stable tag as well as the plugin header version to 2.0.0 again.