Comments

czigor created an issue. See original summary.

czigor’s picture

Status: Active » Needs review
StatusFileSize
new1.02 KB

Discussed this with Ryan and Matt and that's what we came up with, although the appName does not strictly comply with the recommendation.

czigor’s picture

I can see the sent data on my stripe dashboard by clicking "Payments", then open a charge (or payment intent if using the patch in #3039032-18: 3D Secure 2) and click on a Log entry.

Status: Needs review » Needs work

The last submitted patch, 2: commerce_stripe-3058098-app_info-2.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

czigor’s picture

Status: Needs work » Needs review

The coding standard issues have nothing to do with the patch.

mglaman’s picture

Assigned: Unassigned » mglaman

Reviewing today

mglaman’s picture

Assigned: mglaman » Unassigned
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.73 KB
+++ b/src/Plugin/Commerce/PaymentGateway/Stripe.php
@@ -86,6 +86,11 @@ class Stripe extends OnsitePaymentGatewayBase implements StripeInterface {
+    $version = system_get_info('module', 'commerce_stripe')['version'];
+    if (empty($version)) {
+      $version = '8.x-1.0-dev';
+    }
+    \Stripe\Stripe::setAppInfo('Centarro Commerce for Drupal', $version, 'https://www.drupal.org/project/commerce_stripe/');

This is added inbetween proxy setting determination, let's move it up.

Also, I think it's worth checking $info['version'] versus accessing it directly to prevent notices.

I've added a test as well.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 3058098-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mglaman’s picture

Status: Needs work » Reviewed & tested by the community
StatusFileSize
new2.73 KB
+++ b/tests/src/Kernel/AppInfoTest.php
@@ -0,0 +1,53 @@
+namespace Drupal\Tests\commerce_stripe\Unit;

s/Unit/Kernel.

I forgot I first tried this as a Unit test

  • mglaman committed 47d7aee on 8.x-1.x authored by czigor
    Issue #3058098 by mglaman, czigor: Let Stripe know about the app name...
mglaman’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.