Smf2.1: Allow decimals in Paid Subscriptions?

Created on 30 Dec 2016  ·  26Comments  ·  Source: SimpleMachines/SMF2.1

I was wondering why it isn't allowed to include decimals for the cost of Paid Subscriptions and if we should allow it

Paid Subs

Most helpful comment

@Sesquipedalian , yep no problem. Will have to give me a couple of weeks though, I have a bit on, but should be able to get round to it. 👍

All 26 comments

I see no reason why we shouldn't allow it. The only thing we'd need to do is make sure we work around the fact that some countries use commas instead of periods for the "decimal point". We should be able to use our existing number format functionality to handle that though.

We should be able to use our existing number format functionality to handle that though.

We'd need to make sure that we take apart the value, analyze it, and then reassemble it into the exact format that PayPal requires. Otherwise, a French user trying to pay "$1,99" might end up being charged $199 instead of $1.99.

@Sesquipedalian Is it too difficult to code? (as for a person with a basic PHP level, heh)

I'd say the coding part isn't the issue per say. It's the testing and ensuring it works under multiple versions of php.

I know there was a regression bug in php 5.3 to do with number formatting as it impacted some logic for where I worked at the time.

Is it too difficult to code?

@d3vcho: The main thing is making sure that we know for sure what number format the values are arriving in. In theory we should be able to do that, since we track and control the format that they are presented in to begin with. If we know the format, then the rest will be pretty straightforward. Reformatting the incoming value to PayPal's required format will be trivial once it has been parsed.

My comment wasn't meant to suggest that it would be very difficult to do this, but only to remind whoever takes this on that they will need to pay careful attention to formatting issues, especially regarding decimal notations. As @tinoest said, it's the testing that will be the harder part (or at least, the more laborious part). Especially because the paid subscriptions feature deals with people's money, we want to be _very_ sure that this code works 100% flawlessly no matter what gets thrown at it.

I think I've come up with a solution to all of the number validation, though we still need to test this thoroughly. In 2.0, there's a "default number format" setting (which was moved to $txt['number_format'] in 2.1). Based on the possible options for that setting in 2.0, we could end up with about 10 different number formats (assuming we limit numbers to at most 6 digits with an optional 2 additional digits for the decimal). Playing around with preg_match (and with help from regexr.com), I've come up with this possible regex:

'/^\d{1,6}$|^\d{1,3}((\.(\d{2}|(\d{3}(,\d{2})?)))|(,(\d{2}|(\d{3}(\.\d{2})?)))|(\s{1}\d{1,3}(,\d{2})?))$/

That gives us the following options:
1-6 digits
1-3 digits and one of the following:

  • period followed by either 2 digits or (3 digits optionally followed by a comma and 2 digits)
  • comma followed by either 2 digits or (3 digits optionally followed by a period and 2 digits)
  • space followed by 3 digits optionally followed by a comma and 2 digits

Note that regex isn't my strong point so there may be a simpler way to do this, but the above matched everything "valid" that I threw at it. The only issue is that we'd have to assume "1.000" is "1000" and not "1.00", but hopefully between restrictions from browsers with proper HTML5 support and users being careful that shouldn't be an issue.

This might be another area where setlocale() may help. Using the LC_Monetary category based on $txt['lang_locale']. That's what it's there for...

I think there are minor problems now They do not last very long In my opinion

Just a quick note (dont know if its useful or useless)

Currencies like Japanese yen, or Hungarian forint do not support decimals (at least not by paypal) and will raise an error. Im guessing its one of the things that must be handled by SMF.

Source: https://developer.paypal.com/docs/integration/direct/rest/currency-codes/

My opnions about this feature request:

  • ther is no dev how invest time in this
  • when a user like this feature he could go and ask a mod maker
  • for smf i see no benefit (when i'm the project manager i would forbid sutch feature in opensource software which is free)

so i would request to close this issue.

Why would you forbid to allow decimals in paid subs? What if someones wants to create a subscription of 4,50€/month?

Plus there is currency conversion. If something costs 10EU, that will be around 11.50USD (just an example not accurate). So to allow people to pay in multiple currencies without requiring them to pay for conversion themselves decimals are a must.

i would forbid to use smf for paid content,
so we make work for free someone else get paid for this and
we are responsible for this api.

loose - loose - win

so when you like the idea develop this or drop this feature request.

We can't forbid SMF for anything. We just provide the software with X features, then anyone uses it as they want.

In fact SM gets some money from Paid Subs with the Charter Membership, and like us, there could be a lot of projects who just want some donations to keep their project running, that's what Paid Subscriptions are for.

Well that payments could be a way to run community as non profit. They also could be a means of "donation" where donators are rewarded with some kind of rank on forums or something like that. Also people that earn using SMF could donate to SMF itself (rarely but it still happens).

Why is that important to the core of smf that this support exists?
Why can't mod provide this feature or a better version of the existing one?

Because no functional mod for this currently exists, and I don't think we should rip out a core feature like this for no apparent reason. We may or may not bother to add decimal support before releasing 2.1, but there's no need to remove the Paid Subscriptions feature.

I was thinking about this. I am of the opinion that this should never have been introduced as a core feature. It shouts 'mod' to me. It could have even been an 'official' mod.

That being said, it is a core feature and removing it isn't a viable option. However, if we have a feature like this (which we do), it should be implemented properly. Not accepting decimals isn't in my view 'working properly', it's a bug/design-flaw.

If the plan is for an RC2, let's target that. If the plan is RC1 goes well, so we go gold, then let's get this in RC1. It will just be delayed for a short while.

Either way, a decision needs to be made.

Would you be willing to put together a PR to implement this, @equinoxmatt? As I said in a previous comment, it shouldn't be difficult to code. It just requires very thorough testing.

@Sesquipedalian , yep no problem. Will have to give me a couple of weeks though, I have a bit on, but should be able to get round to it. 👍

Excellent! :)

Really struggling for time at the moment, can anyone pick this up?

Does anyone object to using two fields for "dollars" and "cents" in their respective currencies? Seems to me that'd be easier than trying to parse a text field for commas and/or periods and guessing what fraction or whole number the user intended.

And we could disable or hide the decimal field when the currency is to one of Hungarian forint, Japanese yen or New Taiwan dollar.

I dont think anyone would object as long as it works. But parsing it would be single simple function ~10-20 lines max...I dont see how or why it would be easier with two fields but if it is just go ahead with it, there is no difference as long as it works.

I did some digging and it turns out that the backend already supports decimal values.

$cost = array('fixed' => sprintf('%01.2f', strtr($_POST['cost'], ',', '.')));

Seems that when the UI was updated to HTML 5, someone forgot to add the step attribute to the input, so it only allows whole number submissions.

PR coming to remedy the UI shortly.

4808 fixes this.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jdarwood007 picture jdarwood007  ·  3Comments

albertlast picture albertlast  ·  11Comments

jdarwood007 picture jdarwood007  ·  5Comments

NegativeIQ picture NegativeIQ  ·  11Comments

lurkalot picture lurkalot  ·  8Comments