Functional fixes

.github/workflows/ci-linux.yml

@@ -103,7 +103,7 @@ jobs:
       - name: CS
         run: |
           cd galette-core/galette/plugins/plugin-stripe
-          ../../vendor/bin/phpcs lib/ ./*.php
+          ../../vendor/bin/phpcs lib/ tests/ ./*.php
 
       - name: CS Fixer
         if: matrix.php-is-min

.php-cs-fixer.dist.php

@@ -3,6 +3,7 @@
 $finder = (new PhpCsFixer\Finder())
     ->in([
         __DIR__ . '/lib',
+        __DIR__ . '/tests',
     ])
 ;
 

bin/release

@@ -478,8 +478,7 @@ def add_libs(rel_name, galette_archive):
         # cleaunp files not required in releases
         todrop = [
             'composer.lock',
-            'composer.json',
-            'composer.json.checker'
+            'composer.json'
         ]
         for td in todrop:
             if os.path.exists(os.path.join(build_dir, td)):

lib/GaletteStripe/Controllers/StripeController.php

@@ -462,10 +462,13 @@ public function webhook(Request $request, Response $response): Response
         if (
             isset($post['type'])
             && $post['type'] == 'payment_intent.succeeded'
-            && $post['data']['object']['metadata']['item_id']
+            && !empty($post['data']['object']['metadata']['item_id'])
         ) {
             $sh = new StripeHistory($this->zdb, $this->login, $this->preferences);
-            $sh->add($post);
+            if (!$sh->add($post)) {
+                //Stripe will send it again
+                return $response->withStatus(500, 'Internal error');
+            }
 
             // are we working on a real contribution?
             $real_contrib = false;

lib/GaletteStripe/PluginGaletteStripe.php

@@ -133,4 +133,15 @@ public function isInstalled(): bool
             && $this->zdb->TableExists(STRIPE_PREFIX . StripeHistory::TABLE)
         ;
     }
+
+    /**
+     * Database version of tables installed before versions tracking
+     *
+     * 0.0.x releases had their own prices table, dropped in 1.0.0 when
+     * amounts moved to core contributions types.
+     */
+    public function getLegacyDbVersion(): ?float
+    {
+        return $this->zdb->tableExists(STRIPE_PREFIX . 'types_cotisation_prices') ? 0.0 : null;
+    }
 }

lib/GaletteStripe/Stripe.php

@@ -62,8 +62,9 @@ public function __construct(Db $zdb, Preferences $preferences)
         $this->pubkey = null;
         $this->privkey = null;
         $this->webhook_secret = null;
+        //installation defaults
         $this->country = 'FR';
-        $this->currency = null;
+        $this->currency = 'eur';
         $this->load();
     }
 
@@ -264,7 +265,8 @@ public function checkout(array $metadata, string $amount, string $currency): arr
     {
         try {
             $stripe = new StripeClient($this->getPrivKey());
-            $checkout_amount = $this->isZeroDecimal($currency) ? round((float)$amount) : (float)$amount * 100;
+            //19.99 * 100 is 1998.9999999999998
+            $checkout_amount = $this->isZeroDecimal($currency) ? round((float)$amount) : round((float)$amount * 100);
             $session = $stripe->checkout->sessions->create([
                 'success_url' => $this->preferences->getURL() . '/plugins/stripe/success?session_id={CHECKOUT_SESSION_ID}',
                 'cancel_url' => $this->preferences->getURL() . '/plugins/stripe/cancel',
@@ -606,9 +608,9 @@ public function getAllCurrencies(string $country): array
      * Is currency a zero-decimal?
      * https://docs.stripe.com/currencies#zero-decimal
      *
-     * @param string $currency Currency
+     * @param ?string $currency Currency, null when not configured
      */
-    public function isZeroDecimal(string $currency): bool
+    public function isZeroDecimal(?string $currency): bool
     {
         $zeroDecimalCurrencies = [
             "bif",
@@ -628,7 +630,7 @@ public function isZeroDecimal(string $currency): bool
             "xpf"
         ];
 
-        return in_array($currency, $zeroDecimalCurrencies);
+        return in_array(strtolower((string)$currency), $zeroDecimalCurrencies, true);
     }
 
     /**

lib/GaletteStripe/StripeHistory.php

@@ -75,27 +75,46 @@ public function add(array|string $action, string $argument = '', string $query =
     {
         $stripe = new Stripe($this->zdb, $this->preferences);
         $request = $action;
-        $payment_method = $this->getStripePaymentMethod($request['data']['object']['payment_method']);
-        $charge = $this->getStripeCharge($request['data']['object']['latest_charge']);
+        $intent = $request['data']['object'];
+
+        //payer, method and receipt are details: the payment is stored without them
+        $payment_method = [];
+        $charge = [];
+        try {
+            if (!empty($intent['payment_method'])) {
+                $payment_method = $this->getStripePaymentMethod($intent['payment_method']);
+            }
+            if (!empty($intent['latest_charge'])) {
+                $charge = $this->getStripeCharge($intent['latest_charge']);
+            }
+        } catch (\Throwable $e) {
+            Analog::log(
+                'Unable to get details of Stripe payment ' . $intent['id'] . ' | ' . $e->getMessage(),
+                Analog::WARNING
+            );
+        }
 
         try {
             $values = [
                 'history_date'  => date('Y-m-d H:i:s'),
-                'intent_id'     => $request['data']['object']['id'],
-                'payer_name'    => $payment_method['billing_details']['name'],
-                'member_id'     => $request['data']['object']['metadata']['member_id'] ?? 0,
-                'comments'      => $request['data']['object']['metadata']['item_name'],
-                'amount'        => $stripe->isZeroDecimal($stripe->getCurrency()) ? $request['data']['object']['amount'] : $request['data']['object']['amount'] / 100,
-                'method'        => $payment_method['type'],
+                'intent_id'     => $intent['id'],
+                'payer_name'    => $payment_method['billing_details']['name'] ?? null,
+                'member_id'     => $intent['metadata']['member_id'] ?? 0,
+                'comments'      => $intent['metadata']['item_name'] ?? null,
+                'amount'        => $stripe->isZeroDecimal($stripe->getCurrency()) ? $intent['amount'] : $intent['amount'] / 100,
+                'method'        => $payment_method['type'] ?? $intent['payment_method_types'][0] ?? '',
                 'state'         => self::STATE_NONE,
-                'receipt_url'   => $charge['receipt_url'],
+                'receipt_url'   => $charge['receipt_url'] ?? null,
                 'request'       => Galette::jsonEncode($request)
             ];
 
             $insert = $this->zdb->insert($this->getTableName());
             $insert->values($values);
             $this->zdb->execute($insert);
-            $this->id = (int)$this->zdb->driver->getLastGeneratedValue();
+            //without the sequence name, pgsql gives no value
+            $this->id = (int)$this->zdb->connection->getLastGeneratedValue(
+                $this->zdb->isPostgres() ? $this->zdb->getSequenceName($this->getTableName(), 'id', prefixed: true) : null
+            );
 
             Analog::log(
                 'An entry has been added in stripe history',
@@ -150,12 +169,13 @@ public function getStripeHistory(): array
             foreach ($orig as $o) {
                 try {
                     if (Galette::isSerialized($o['request'])) {
-                        $oa = unserialize($o['request']);
+                        //legacy entries: only plain data is expected
+                        $oa = unserialize($o['request'], ['allowed_classes' => false]);
                     } else {
                         $oa = Galette::jsonDecode($o['request']);
                     }
 
-                    $o['member_fullname'] = $this->getMemberFullName($o['member_id']);
+                    $o['member_fullname'] = $this->getMemberFullName((int)$o['member_id']);
                     $o['raw_request'] = print_r($oa, true);
                     $o['request'] = $oa;
 

phpstan.neon

@@ -4,6 +4,7 @@ parameters:
     level: 6
     paths:
         - lib/
+        - tests/
     scanFiles:
         - _config.inc.php
         - ../../includes/sys_config/paths.inc.php

scripts/pgsql.sql

@@ -15,9 +15,9 @@ CREATE SEQUENCE galette_stripe_history_id_seq
 DROP TABLE IF EXISTS galette_stripe_history;
 CREATE TABLE galette_stripe_history (
   id_stripe integer DEFAULT nextval('galette_stripe_history_id_seq'::text) NOT NULL,
-  history_date date NOT NULL,
+  history_date timestamp NOT NULL,
   intent_id character varying(255),
-  amount real NOT NULL,
+  amount numeric(15,2) NOT NULL,
   comments character varying(255),
   request text,
   state smallint DEFAULT 0 NOT NULL,

scripts/upgrade-to-1.0.0-mysql.sql

@@ -4,41 +4,46 @@
 -- SPDX-License-Identifier: GPL-3.0-or-later
 --
 
-DROP TABLE galette_stripe_types_cotisation_prices;
+-- Prices are the amounts of core contributions types; keep those already set
+UPDATE galette_types_cotisation t
+  INNER JOIN galette_stripe_types_cotisation_prices p ON p.id_type_cotis = t.id_type_cotis
+  SET t.amount = p.amount
+  WHERE t.amount IS NULL AND p.amount IS NOT NULL;
+DROP TABLE IF EXISTS galette_stripe_types_cotisation_prices;
 
 ALTER TABLE galette_stripe_history
   CHANGE COLUMN comment comments varchar(255),
   CHANGE COLUMN metadata request text;
 
 ALTER TABLE galette_stripe_history
-  MODIFY intent_id VARCHAR(255) COLLATE utf8mb4_unicode_520_ci,
-  MODIFY comments VARCHAR(255) COLLATE utf8mb4_unicode_520_ci,
-  MODIFY request TEXT COLLATE utf8mb4_unicode_520_ci,
+  ADD COLUMN payer_name varchar(255),
+  ADD COLUMN member_id int(10) NOT NULL,
+  ADD COLUMN method varchar(20) NOT NULL,
+  ADD COLUMN receipt_url varchar(255),
   CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_520_ci;
-
+-- CONVERT TO promotes text to mediumtext, and keeps an explicit charset on intent_id
 ALTER TABLE galette_stripe_history
-  ADD COLUMN payer_name VARCHAR(255),
-  ADD COLUMN member_id INT(10) NOT NULL,
-  ADD COLUMN method VARCHAR(20) NOT NULL,
-  ADD COLUMN receipt_url VARCHAR(255);
+  MODIFY intent_id varchar(255),
+  MODIFY request text;
 
+-- Previous versions stored the serialized payment metadata only, with the member
+-- as "adherent_id", and card was the only payment method; their states were
+-- 0 (public donation), 2 (done) and 3 (error)
 UPDATE galette_stripe_history
 SET
-  state = CASE
-    WHEN state = 0 THEN 3
+  state = CASE state
+    WHEN 0 THEN 3
+    WHEN 2 THEN 1
+    WHEN 3 THEN 2
     ELSE state
   END,
-  member_id = COALESCE(
-    CAST(
-      JSON_UNQUOTE(JSON_EXTRACT(request, '$.data.object.metadata.member_id'))
-      AS UNSIGNED
-    ),
-    0
-  ),
-  method = JSON_UNQUOTE(JSON_EXTRACT(request, '$.data.object.payment_method_types[0]')),
-  receipt_url = JSON_UNQUOTE(JSON_EXTRACT(request, '$.receipt_url'));
+  member_id = CASE
+    WHEN request REGEXP '"adherent_id";s:[0-9]+:"[0-9]+"'
+      THEN CAST(SUBSTRING_INDEX(SUBSTRING_INDEX(SUBSTRING_INDEX(request, '"adherent_id";s:', -1), '"', 2), '"', -1) AS UNSIGNED)
+    ELSE 0
+  END,
+  method = 'card';
 
 ALTER TABLE galette_stripe_preferences
-  MODIFY nom_pref VARCHAR(100) NOT NULL DEFAULT '',
-  MODIFY val_pref VARCHAR(200) NOT NULL DEFAULT '',
+  ENGINE=InnoDB,
   CONVERT TO CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_520_ci;

scripts/upgrade-to-1.0.0-pgsql.sql

@@ -4,28 +4,41 @@
 -- SPDX-License-Identifier: GPL-3.0-or-later
 --
 
-DROP TABLE galette_stripe_types_cotisation_prices;
+-- Prices are the amounts of core contributions types; keep those already set
+UPDATE galette_types_cotisation t
+  SET amount = p.amount
+  FROM galette_stripe_types_cotisation_prices p
+  WHERE p.id_type_cotis = t.id_type_cotis AND t.amount IS NULL AND p.amount IS NOT NULL;
+DROP TABLE IF EXISTS galette_stripe_types_cotisation_prices;
+
+ALTER TABLE galette_stripe_history RENAME COLUMN metadata TO request;
 
 ALTER TABLE galette_stripe_history
-  RENAME COLUMN metadata TO request,
   ADD COLUMN payer_name character varying(255),
-  ADD COLUMN member_id integer,
-  ADD COLUMN method character varying(20),
+  ADD COLUMN member_id integer DEFAULT 0 NOT NULL,
+  ADD COLUMN method character varying(20) DEFAULT '' NOT NULL,
   ADD COLUMN receipt_url character varying(255);
+ALTER TABLE galette_stripe_history
+  ALTER COLUMN member_id DROP DEFAULT,
+  ALTER COLUMN method DROP DEFAULT;
 
+-- Keep time of payments, and do not store amounts as floating point numbers
+ALTER TABLE galette_stripe_history
+  ALTER COLUMN history_date TYPE timestamp,
+  ALTER COLUMN amount TYPE numeric(15,2);
+
+-- Previous versions stored the serialized payment metadata only, with the member
+-- as "adherent_id", and card was the only payment method; their states were
+-- 0 (public donation), 2 (done) and 3 (error)
 UPDATE galette_stripe_history
 SET
-  state = CASE
-    WHEN state = 0 THEN 3
+  state = CASE state
+    WHEN 0 THEN 3
+    WHEN 2 THEN 1
+    WHEN 3 THEN 2
     ELSE state
   END,
-  member_id = COALESCE(
-    (request #>> '{data,object,metadata,member_id}')::int,
-    0
-  ),
-  method = request #>> '{data,object,payment_method_types,0}',
-  receipt_url = request #>> '{receipt_url}';
+  member_id = COALESCE(substring(request from '"adherent_id";s:[0-9]+:"([0-9]+)"')::integer, 0),
+  method = 'card';
 
-ALTER TABLE galette_stripe_history
-  ALTER COLUMN member_id SET NOT NULL,
-  ALTER COLUMN method SET NOT NULL;
+UPDATE galette_stripe_preferences SET val_pref = UPPER(val_pref) WHERE nom_pref = 'stripe_country';

templates/default/stripe_preferences.html.twig

@@ -133,8 +133,9 @@
 
 {% block javascripts %}
     <script type="text/javascript">
+        //country is only proposed to administrators, once keys are set
         const stripeCountry = document.getElementById('stripe_country');
-        stripeCountry.addEventListener('change', function() {
+        stripeCountry?.addEventListener('change', function() {
             let countryCode = this.value;
             let refreshedCurrencies = {};
             $('#stripe_currency_field .ui.dropdown').addClass('loading disabled');
@@ -151,8 +152,8 @@
                     $('#stripe_currency_field .ui.dropdown').removeClass('loading disabled');
                     $('#stripe_currency_field .ui.dropdown').transition('glow');
                 },
-                error: function(error) {
-                    console.log('Error fetching data:', error);
+                error: function() {
+                    $('#stripe_currency_field .ui.dropdown').removeClass('loading disabled');
                 }
             });
         });