All payment request rejected with invalid locale

Created on 14 February 2023, about 2 years ago
Updated 19 March 2023, about 2 years ago

Problem/Motivation

The commerce.current_locale service is used to get the locale for Mollie. Unfortunately this does not garantes a valid ISO code. This should not be an issue, but Mollie rejects all payment requests when an invalid locale code is supplied. So for example with a drupal installation in German, but the visitor is in the Netherlands, the locale would be de_NL which is invalid.

Proposed resolution

Map the current site local to the available Mollie locales and only send the value when one is available.

Remaining tasks

Please review the patch.

🐛 Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

🇳🇱Netherlands mvdve

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Issue created by @mvdve
  • Status changed to Needs work about 2 years ago
  • 🇳🇱Netherlands ndf Amsterdam

    With the example: drupal installation in German, visitor is in the Netherlands,
    what would be the correct langcode for Mollie?

    Is it a correct assumption that the langcode is used by mollie to display their dialogs in the visitor language?
    If that's the case indeed the drupal interface-language should be used.

    NB we have a test that now fails \Drupal\Tests\commerce_mollie\Functional\MolliePaymentOffsiteFormTest::testPaymentTransactionData
    which assumes the following:

        $language = ConfigurableLanguage::createFromLangcode('fr');
        $language->save();
    // ... doing a transaction ...
        // The commerce locale resolver tacks the current language to the store
        // country.
        $this->assertEquals('fr_US', $last_transaction['locale']);
    

    So this was actually all the time wrong and it should be:
    $this->assertEquals('fr_FR', $last_transaction['locale']);
    Is that correct?

  • 🇳🇱Netherlands mvdve

    Yes that's correct, fr_US is not a valid locale code as French is not an official language of the US.

    The second issue is that Mollie supports a selection of ISO 15897 codes, see locale in the docs and indeed, the locale variable is used to set the payment interface in the correct language.

    Because of the selection of locales on Mollie's end, I mapped the available locales to the drupal interface locale and otherwise Mollie can do it's own thing, not the most generic method but it prevents the request error.

    Updated the patch with the correct locale code within the test.

  • Status changed to Needs review about 2 years ago
  • Status changed to RTBC about 2 years ago
  • 🇳🇱Netherlands ndf Amsterdam

    mvdve, this looks good thanks.
    One more question. What is the reason to use \Drupal::languageManager() instead of \Drupal::service('commerce.current_locale')

    The default locale resolver of commerce seems to do a very similar thing see \Drupal\commerce\Resolver\DefaultLocaleResolver::resolve

      public function resolve() {
        // The getCurrentLanguage() fallback is a workaround for core bug #2684873.
        $language = $this->languageManager->getConfigOverrideLanguage() ?: $this->languageManager->getCurrentLanguage();
        $langcode = $language->getId();
        $langcode_parts = explode('-', $langcode);
        if (count($langcode_parts) > 1 && strlen(end($langcode_parts)) == 2) {
          // The current language already has a country component (e.g. 'pt-br'),
          // it qualifies as a full locale.
          $locale = $langcode;
        }
        elseif ($country = $this->currentCountry->getCountry()) {
          // Assemble the locale using the resolved country. This can result
          // in non-existent combinations such as 'en-RS', it's up to the locale
          // consumers (e.g. the number format repository) to perform fallback.
          $locale = $langcode . '-' . $country;
        }
        else {
          // Worst case scenario, the country is unknown.
          $locale = $langcode;
        }
    
        return new Locale($locale);
      }
    

    Lets add a comment that we intentionally not use that resolver.

    I'll do that on commit.

  • 🇳🇱Netherlands ndf Amsterdam

    Another thought; the supported list on Mollie is:
    Possible values: en_US en_GB nl_NL nl_BE fr_FR fr_BE de_DE de_AT de_CH es_ES ca_ES pt_PT it_IT nb_NO sv_SE fi_FI da_DK is_IS hu_HU pl_PL lv_LV lt_LT

    We now not have en_GB nl_BE fr_BE de_AT (and probably more)

    What if we keep the commerce.current_locale logic for the making the locale code,
    but if it's not in Mollie's supported list we remove it.
    So when the lang-code is not in the valid list, we don't send $transaction_data['locale'].
    Then the following happens: When this parameter is omitted, the browser language will be used instead if supported by the payment method.

  • 🇳🇱Netherlands ndf Amsterdam

    @mvdve what do you think from my comment above?

  • 🇳🇱Netherlands mvdve

    I replaced the commerce.current_locale service for the language_manager service because of the invalid locale codes.

    In my case this was invoked:

    elseif ($country = $this->currentCountry->getCountry()) {
          // Assemble the locale using the resolved country. This can result
          // in non-existent combinations such as 'en-RS', it's up to the locale
          // consumers (e.g. the number format repository) to perform fallback.
          $locale = $langcode . '-' . $country;
        }
    

    Which returns invalid codes as described in the comments. Maybe i am missing something but i think this service should only return valid codes or none at all. Using the commerce.current_locale would result in a lot of false negative results with the mapping to the Mollie locales. For example current country is NL, site locale is DE, this would result in de_NL which is invalid. The language_manager service will return the site locale DE which can be mapped to one of the DE Mollie locales.

    I don't know if Drupal has functionality for country specific language versions, so de_DE and de_AT for example. I was lazy and picked one country of each language to keep the switch block short. I agree that it would be best to map against the complete locale code, but i don't have these in my Drupal sites. Just double character locale codes.

    • ndf committed 0d2e5fc8 on 8.x-1.x
      #3341547 by mvdve, ndf: All payment request rejected with invalid locale
      
  • Status changed to Fixed about 2 years ago
  • 🇳🇱Netherlands ndf Amsterdam

    Thanks mvdve, I just committed the patch as is.

    After the patch we have this updated test:

    @@ -167,7 +167,7 @@ class MolliePaymentOffsiteFormTest extends CommerceBrowserTestBase {
         $this->assertEquals('2', $last_transaction['metadata']['order_id']);
         // The commerce locale resolver tacks the current language to the store
         // country.
    -    $this->assertEquals('fr_US', $last_transaction['locale']);
    +    $this->assertEquals('fr_FR', $last_transaction['locale']);
         $this->assertEquals($this->store->getName() . ' order 2', $last_transaction['description']->__toString());
       }
    

    where indeed fr_FR seems a lot better as default then fr_US.
    This happens when a customer buys a product on the following url: /fr/product/1

  • 🇳🇱Netherlands ndf Amsterdam

    It's not in the 1.10 release of today, but only in the develop branch.
    So that people have time to test this on their multi-language websites.

  • Status changed to Fixed about 2 years ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024