stripe / stripe-php

PHP library for the Stripe API.
https://stripe.com
MIT License
3.74k stars 849 forks source link

Add optional appInfo to StripeClient config #1700

Closed helenye-stripe closed 5 months ago

helenye-stripe commented 5 months ago

This adds $appInfo as a valid key in $config when initializing a StripeClient. $appInfo expects the type

array{name: string, version?: string, url?: string, appPartnerId?: string}

It passes this in to APIRequestor as an init parameter with default null, and will be set in headers if not null.

Tests verify that we fall back to global configuration if not passed into StripeClient, by verifying the User-Agent header.

Changelog

admsev commented 5 months ago

Hey @gooroodev, can you review this please?

gooroodev commented 5 months ago

Thanks for bringing me on board, @admsev!

{mistakes: 0, typos: 1, security: 0, performance: 0, code_styling: 3, others: 0}

  1. Summary of Changes:

    • Added _appInfo property to ApiRequestor class.
    • Updated the constructor of ApiRequestor to accept appInfo.
    • Modified _defaultHeaders method to include appInfo.
    • Added app_info to BaseStripeClient configuration.
    • Added getter for app_info in BaseStripeClient.
    • Updated request and requestStream methods in BaseStripeClient to pass appInfo.
    • Added validation for app_info in BaseStripeClient.
    • Added tests for app_info in BaseStripeClientTest.
  2. Issues, Bugs, or Typos:

    • Typo:
      • lib/BaseStripeClient.php:line 43
      • Typo in comment: verion should be version.
        • stripe_version (null|string): a Stripe API version. If set, all requests sent by the client
  3. General Review of Code Quality and Style:

    • The code is generally well-structured and follows good practices.
    • Proper validation and error handling are present.
    • The new feature is well-integrated into the existing codebase.
    • Tests are added to ensure the new functionality works as expected.
  4. Code Styling Improvements:

    • Indentation Consistency:

      • Ensure consistent indentation for better readability.
      • lib/ApiRequestor.php:line 131
        $this->_requestRaw($method, $url, $params, $headers, $usage);
      • lib/ApiRequestor.php:line 153
        $this->_requestRawStreaming($method, $url, $params, $headers, $usage, $readBodyChunkCallable);
    • Comment Formatting:

      • Align comments to improve readability.
      • lib/ApiRequestor.php:line 220
        // no break
    • Function Parameter Alignment:

      • Align parameters for better readability.
      • lib/ApiRequestor.php:line 469
        isset($rheaders['request-id'])
        && \is_string($rheaders['request-id'])
        && '' !== $rheaders['request-id']

Overall, the changes are well-implemented, with minor improvements needed for code styling and a typo correction. The new feature of adding optional appInfo to the StripeClient configuration is well-integrated and tested.

Yours, Gooroo.dev. I'd appreciate your feedback! React or reply.