Skip to content
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

Filter PayPal-supported language codes #154

Merged
merged 2 commits into from
May 20, 2021

Conversation

Dinamiko
Copy link
Contributor

@Dinamiko Dinamiko commented May 3, 2021

Description

When selecting a language in WordPress like German [formal] or Norsk [Bokmål] banners and buttons do not appear on the selected pages and also there will be an error after trying to buy a product on the checkout page.

Steps to test:

  1. Select German [formal] in WordPress general settings.
  2. Enable all banners and buttons in the plugin settings and save it.
  3. Check the product pages.
  4. Buy a product.

@Dinamiko Dinamiko requested a review from strangerkir May 3, 2021 14:30
Copy link
Contributor

@strangerkir strangerkir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe some changes are needed.

@@ -726,7 +726,7 @@ private function url(): string {
$params = array(
'client-id' => $this->client_id,
'currency' => get_woocommerce_currency(),
'locale' => get_user_locale(),
'locale' => $this->valid_locale_code(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest not pass the locale at all, as PayPal detects it automatically, and their advice (the locale parameter description in the table below) is to not use this parameter unless it's really needed.

But if we really need this, then it maybe makes sense to map possible WP locales to the ones accepted by PayPal. For example, when the Bulgarian language is set for the site, then the WP locale is bg_BG, but PayPal accepts en_BG only. A similar problem appears for the Belarus language, and should be for some others I haven't checked. I tested it, and in my case when the locale is not accepted by PayPal, the JS SDK script is not loaded. Instead, I've had HTTP error code 400.

PCP-138-locale-issue

Comment on lines 68 to 77
protected function valid_bcp47_code() {
$locale = str_replace( '_', '-', get_user_locale() );
$parts = explode( '-', $locale );

if ( count( $parts ) < 3 ) {
return $locale;
}

return substr( $locale, 0, strrpos( $locale, '-' ) );
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some doubts about this. I believe this will work in most cases but I expect it fail when get_user_locale() returns locale containing 3 letters with no delimiter (like bel when Belarusian site language is chosen, also ceb for Cebuano, ary, dsb, etc.).

PayPal documentation states they use the ^[a-z]{2}(?:-[A-Z][a-z]{3})?(?:-(?:[A-Z]{2}))?$ regex to validate the language. I tested a few of possible WP locales and it worked for the xx-XX format (like de-DE), also for xx format (like ar), but not for xxx format (like the bel and others I provided above). This function will return three-letters code as is (which is valid BCP 47, if I understand it correctly). But it doesn't meet the regex from PayPal docs provided above.

I have no good suggestion about this issue. Probably, it would make sense to use that regex to validate the result and fallback to default if validation wasn't passed.

@Dinamiko Dinamiko requested a review from strangerkir May 13, 2021 14:43
@Dinamiko
Copy link
Contributor Author

Hi @strangerkir,

I´ve removed locale on smart button, also added validation and fallback for bcp47, when you have time please take a look, thanks.

Copy link
Contributor

@strangerkir strangerkir left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me :)

@Dinamiko Dinamiko merged commit cc1cd30 into trunk May 20, 2021
@Dinamiko Dinamiko deleted the PCP-138-paypal-payments-doesnt-work-with branch May 20, 2021 09:42
@Dinamiko Dinamiko mentioned this pull request Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants