Incorrect value in `MeprSubscription.total` when adjusting price in `nl_NL` WordPress installation, `9,99` is saved as `999`

[remcotolsma]

When the WordPress language / locale is configured to nl_NL the MeprSubscription->total value is calculated / stored incorrect. This is due to the incorrect handling in MeprSubscriptionsCtrl->create_or_update( $sub ) and MeprUtils::format_currency_us_float( $number, $num_decimals = 2 ).

app/lib/MeprUtils.php#L381-L408

  /**
   * Converts number to US format
   *
   * @param  mixed $number
   * @param  mixed $num_decimals
   * @return void
   */
  public static function format_currency_us_float($number, $num_decimals = 2) {
    global $wp_locale;

    if ( ! isset( $wp_locale ) || false === function_exists('number_format_i18n') ) {
      return self::format_float($number, $num_decimals);
    }

    $decimal_point = $wp_locale->number_format['decimal_point'];
    $thousands_sep = $wp_locale->number_format['thousands_sep'];

    // Remove thousand separator
    $number = str_replace ($thousands_sep, '' , $number);

    // Replaces decimal separator
    $index = strrpos( $number, $decimal_point );
    if( $index !== FALSE ){
      $number[ $index ] = '.';
    }

    return (float) $number;
  }

When the WordPress language / locale is configured to nl_NL this function will work with the following values:

// In `nl_NL` this will be a comma character: ','.
$decimal_point = $wp_locale->number_format['decimal_point'];

// In `nl_NL` this will be a full stop (period) character: '.'.
$thousands_sep = $wp_locale->number_format['thousands_sep'];

When a Dutch user enters for example a price of 9,99 this means 9 euros and 99 cents. It should be stored a 9.99 in the database, but it's stored as 999. This is due to the following code:

app/controllers/MeprSubscriptionsCtrl.php#L137-L172

  private function create_or_update($sub) {
    check_admin_referer( 'mepr_create_or_update_subscription', 'mepr_subscriptions_nonce' );

    extract($_POST, EXTR_SKIP);
    $user = new MeprUser();
    $user->load_user_data_by_login($user_login);
    $sub->user_id = $user->ID;
    $sub->subscr_id = wp_unslash($subscr_id);
    $sub->product_id = $product_id;
    $product = new MeprProduct($product_id);
    $sub->price = isset($price) ? MeprUtils::format_currency_us_float( $price ) : MeprUtils::format_currency_us_float( $product->price );
    $sub->period = isset($period) ? (int) $period : (int) $product->period;
    $sub->period_type = isset($period_type) ? (string) $period_type : (string) $product->period_type;
    $sub->limit_cycles = isset($limit_cycles) ? (boolean) $limit_cycles : $product->limit_cycles;
    $sub->limit_cycles_num = isset($limit_cycles_num) ? (int) $limit_cycles_num : (int) $product->limit_cycles_num;
    $sub->limit_cycles_action = isset($limit_cycles_action) ? $limit_cycles_action : $product->limit_cycles_action;
    $sub->limit_cycles_expires_after = isset($limit_cycles_expires_after) ? (int) $limit_cycles_expires_after : (int) $product->limit_cycles_expires_after;
    $sub->limit_cycles_expires_type = isset($limit_cycles_expires_type) ? (string) $limit_cycles_expires_type : (string) $product->limit_cycles_expires_type;
    $sub->tax_amount = MeprUtils::format_currency_us_float( $tax_amount );
    $sub->tax_rate = MeprUtils::format_currency_us_float( $tax_rate );
    $sub->total = MeprUtils::format_currency_us_float( $sub->price + $sub->tax_amount );
    $sub->status = $status;
    $sub->gateway = $gateway;
    $sub->trial = isset($trial) ? (boolean) $trial : false;
    $sub->trial_days = (int) $trial_days;
    $sub->trial_amount = MeprUtils::format_currency_us_float( $trial_amount );
    $sub->trial_tax_amount = (isset($trial_tax_amount) ? (float) $trial_tax_amount : 0.0);
    $sub->trial_total = (isset($trial_total) ? (float) $trial_total : 0.0);
    if(isset($created_at) && (empty($created_at) || is_null($created_at))) {
      $sub->created_at = MeprUtils::ts_to_mysql_date(time());
    }
    else {
      $sub->created_at = MeprUtils::ts_to_mysql_date(strtotime($created_at));
    }
    return $sub->store();
  }

On app/controllers/MeprSubscriptionsCtrl.php#L147 the submitted $_POST['price'] of 9,99 is converted to 9.99 and stored in $sub->price. On app/controllers/MeprSubscriptionsCtrl.php#L157 this value is passed to the MeprUtils::format_currency_us_float( ) function again and then it goes wrong. The $thousands_sep = '.' is replaced by an empty string '' and this will convert 9.99 to 999.

[remcotolsma]

We just notified MemberPress about this bug:

If I am correct, we will receive a response via support@pronamic.nl.

[remcotolsma]

Our customer has just been informed about this matter as well:

Zoals telefonisch besproken zouden we nog even onderzoeken waarom een bedrag met decimalen zoals 9,99 niet gebruikt kan worden. Na heel wat onderzoek zijn we er achter gekomen dat dit komt door een fout in de code van MemberPress. Het probleem doet zich alleen voor in Nederlandse WordPress installatie en heeft te maken met dat we in Nederland een komma gebruiken als decimaalteken en scheiden we duizendtallen met een punt. Als je de taal van je website tijdelijk instelt op Engels en dan het 9,99 invoert dan gaat het wel goed. We hebben inmiddels een technische omschrijving uitgeschreven voor de MemberPress ontwikkelaars en ze op de hoogte gesteld van deze fout. Deze technische informatie is eventueel te raadplegen op de volgende pagina: https://github.com/pronamic/memberpress/issues/1. We zullen proberen je op de hoogte te houden van de ontwikkelingen.

Internal HelpScout ticket: https://secure.helpscout.net/conversation/1864487965/23887/

[remcotolsma]

Just received a response from support@memberpress.com:

Hi there,

Thank you for reaching out and bringing this to our attention. I've filed this as a bug report to our developers, and hopefully, this is something that we'll be able to fix soon.

Best regards,

Tyler support@memberpress.com

Internal HelpScout ticket: https://secure.helpscout.net/conversation/1878863028/23932

[rvdsteege]

Also in Help Scout ticket https://secure.helpscout.net/conversation/1891215060/23976/

[remcotolsma]

I just asked MemberPress for an ETA:

Hello Tyler or colleague,

Could we get a status update on the previous issue? There are now several users who are hindered by this. It's quite a tricky bug, so an ETA is more than welcome.

Remco Tolsma Pronamic

Also somewhat disappointing that we have not heard from them for almost 2 weeks.

[rvdsteege]

Received from support@memberpress.com:

Hi Remco,

It looks like this has been assigned to one of our developers to fix. Unfortunately, I don't have an ETA on when this will be fixed but as soon as I hear it has been I'll be sure to update you on this ticket. I apologize for the wait. Once it's fixed, it'll be available in the next edge build or stable release of MemberPress. If you'd rather not wait until then, I can provide you with the fix our developer implemented.

Best regards,

Tyler support@memberpress.com

[remcotolsma]

I just sent a response to supsupport@memberpress.com:

There are several Dutch MemberPress users who are now having problems with this. Can a non-developer get on with the fix in question? Can't you at least issue an ETA for this problem? If you can share the fix anyway, then we can also share it with the relevant Dutch MemberPress users. However, I don't think the MemberPress users in question will be very happy if they have to manually apply a fix.

The issue has been open for a while and can cause problems in the next recurring collection round.

[rvdsteege]

Received reply from support@memberpress.com:

Hi Remco,

I haven't heard anything back from our developers yet I'm afraid, but let me see if I can find a fix for this so you're not having to wait.

Would it be possible to provide me with temporary admin access to one of the sites that are currently experiencing the issue? If you are unsure how to do this, please follow my instructions here.

You can use the following information to set up the Admin account: USERNAME: Tyler_MemberPress EMAIL: support@memberpress.com

If you have given our team access before there may already be a user with that email address. In which case you'll just need to reset the password.

If you could reply back with the password and URL to your site, that would be helpful. Thanks!

[remcotolsma]

If you'd rather not wait until then, I can provide you with the fix our developer implemented.

3 days later:

I haven't heard anything back from our developers yet I'm afraid, but let me see if I can find a fix for this so you're not having to wait.

😩

Replied back:

Administrator access is unfortunately not possible and in my opinion not necessary. The problem is quite clear and you should be able to reproduce it fairly easily on your own test environment. Let's focus on a quick release that fixes this and not slow things down unnecessarily with admin access request. Thanks in advance.

[remcotolsma]

Received reply from support@memberpress.com:

Hi Remco,

I heard back from one of our developers, and it looks like this change would need to undergo quite a bit of testing before we could merge it into MemberPress so it's not something that we'd be able to fix quickly, but they mentioned that using decimal points (.) instead of commas (,) as the separator will prevent this issue from occurring. I'd recommend informing users in the Netherlands to use this workaround until we're able to figure out a viable solution.

Best regards,

Tyler support@memberpress.com

Replied back:

After your message: "If you'd rather not wait until then, I can provide you with the fix our developer implemented." I assumed the fix was already there. I understand that it must be properly tested, you have already failed to do that with the current implementation. Personally, I think you owe it to your users to find a solution as soon as possible. We are now 20 days further and nothing concrete has happened yet? Then an ETA seems to me the least you can give.

[remcotolsma]

Just informed again at MemberPress:

​We are now 4 weeks further, how are things going? Can you give us an update on this issue and its solution?

[remcotolsma]

Just informed again at MemberPress:

What's the status on this issue? Unfortunately, we have not received a response to our previous message.

[remcotolsma]

Just inquired about the status via Twitter: https://twitter.com/remcotolsma/status/1551929232976224259.

Hello @memberpress, we found a bug on Dutch MemberPress websites (locale `nl_NL`), we and a number of Dutch users are waiting for a solution. However, since May 31, you no longer respond to our messages, what is going on? #WordPress #MemberPress → https://t.co/OYQbT2646L

— Remco Tolsma (@remcotolsma) July 26, 2022

[rvdsteege]

Still no response from MemberPress by email? The tweet doesn't have any replies.

[remcotolsma]

No, just sent a reminder:

We are now almost 3 months further, what is the status of this issue?

https://github.com/pronamic/memberpress/blob/0e9aaacd603e8525343d15986e757dcf894ce527/app/lib/MeprUtils.php#L381-L408

https://github.com/pronamic/memberpress/blob/0e9aaacd603e8525343d15986e757dcf894ce527/app/controllers/MeprSubscriptionsCtrl.php#L137-L172

[remcotolsma]

Hi Remco,

I hope you're doing well today. I got in touch with one of our developers, and they were able to get it fixed so we should have it available in the next release of MemberPress. In the meantime, you should be able to apply the fix by editing the wp-content/plugins/memberpress/app/controllers/MeprSubscriptions.php file, and on line 157 replace this line:

$sub->total = MeprUtils::format_currency_us_float( $sub->price + $sub->tax_amount );

With this:

$sub->total = $sub->price + $sub->tax_amount;

I hope this helps, but if you have any questions or concerns please feel free to ask.

Best regards,

-- Tyler support@memberpress.com

[remcotolsma]

It's unbelievable, but the problem seems to be finally solved.

https://github.com/pronamic/memberpress/blob/349b0950b5fab8a1da0f2633642fcca98bdbce53/app/controllers/MeprSubscriptionsCtrl.php#L157

https://github.com/pronamic/memberpress/blame/349b0950b5fab8a1da0f2633642fcca98bdbce53/app/controllers/MeprSubscriptionsCtrl.php#L157