Fix number formatting according to the form field setting

[omarkasem]

Fix formating

Fixes #1872

Also related to GravityEdit: https://github.com/GravityKit/GravityEdit/pull/236

💾 Build file (e005343a9).

[rafaehlers]

@mrcasual can you check here what's going on so I can test?

[mrcasual]

@omarkasem, while this PR works, the View's field number_format option is no longer respected (see the failing test).

When unchecked, all numbers are now displayed with the thousands separator (, as a decimal separator is correct):

When checked, a . decimal separator is forced on long numbers:

Please fix.

[zackkatz]

@omarkasem Please see https://github.com/GravityKit/GravityView/blob/d22d9f05ac62094c74dd063d8829d37356e452ff/includes/helper-functions.php#L223-L262 and see if that helps

[omarkasem]

@mrcasual @zackkatz

	$field->update_configuration( array( 'number_format' => false ) );

	$field->field->numberFormat = '';

	$this->assertEquals( '7982489.239', $renderer->render( $field, $view, $form, $entry, $request ) );

So this part of the test is the main issue and I think it has to change so we can fix the main bug of the PR.

The PR was to fix the formatting so the number field will look like that '7,982,489.23929' always with no number_format or decimals and to achieve that we have to edit this part of the test or we can keep it like it is and expect the correct format to work only when the number_format is set to true in the field (which is what i did in the last commit to bypass the tests fails).

I hope I made my point clear, please let me know what you think

[mrcasual]

@omarkasem, I appreciate your October 11 comment. However, I'm afraid I still have concerns about the number formatting issue that this PR was supposed to address.

When the number field is displayed in the View, the form field's "number format" option isn't being respected. Specifically, if that option is set to 9,999.99, the number should be displayed as either 9999.99 or 9,999.99, depending on the View field's "format number" setting.

Similarly, if the form field's "number format" is set to 9.999,99, the View show display the number as 9999,99 or 9.999,99. Again, the formatting used should match the configuration of the View field's "format number" setting and that of the form field configuration.

Please let us know what it would take to properly implement this functionality. If it helps, we can discuss this further in a call or on Slack.

[rafaehlers]

@omarkasem please don't forget Vlad's remark above ☝️ Thanks 🙏

[zackkatz]

@omarkasem Please review Vlad's comment https://github.com/GravityKit/GravityView/pull/1879#issuecomment-1776063871

[omarkasem]

@mrcasual Sorry for late reply here, this task was always very confusing for me, So before I work on it again i want to make sure of a couple of things.

So the above screenshot is how gravityforms handles the formatting in entries, are we going to follow them? because what you said in the comment is the opposite of that.

Also when can it be shown as 9,999.99? how can i get that through the format number or decimals options in the view? Same thing for 9.999,99

Also if we put 2 in the decimal field in view, should it be 999,999.00 for decimal dot 999.999,00 for decimal comma or something else?

[mrcasual]

@omarkasem, when you configure the Number form field to use the 9.999,99 format, GF will display the number formatted exactly like that. GV (this feature branch), on the other hand, will format the number as 9999.99, which indicates that it does not respect the setting where , is the decimal separator. If I edit the View field and toggle the "Format Number?" setting, the number will be displayed as 9,999.99. Expected output: 9999,99 and 9.999,99.

Form:

Entries:

View:

You can use GravityMigrate to import this data and see for yourself. Make sure that your site language (Settings > General) is set to English (United States). Please let me know if you still have questions.

[omarkasem]

@mrcasual So It should be fixed now, the test test_frontend_field_html_number has an issue as it uses the field id 9 from complete.json and that field has a numberformat of 'currency' so it will fail when it tries to assertequals a regular number when the main field format is currency.

[doekenorg]

@omarkasem for displaying purposes I find it beneficial to stay as close to gravityforms as possible. I think you should be able to use https://github.com/wp-premium/gravityforms/blob/1b322a9405fdfb00a08bcae3bb20262fed12114c/includes/fields/class-gf-field-number.php#L229 on the number field to get the proper formatting. This should remove some of your custom code.

[omarkasem]

@doekenorg Thanks, I actually tried this function before but it only works on fields in views that has 'format_number' checked but the other one works on both, See screenshot for more details

[doekenorg]

@omarkasem Hey, sorry I missed the call to get_value_entry_list; I saw the code on my phone 😄

Still, in this case; this should be enough:

/** @var \GF_Field_Number $field */
$field = $gravityview->field->field;
if ($decimals) {
	$value = number_format( $value, (int) $decimals, '.', '' );
}
echo $field->get_value_entry_list( $value, $gravityview->entry->as_entry(), $gravityview->field->id, [], $form );

The field is already available on the gravityview instance; so you don't need to retrieve it via GFFormsModel.

Since the value is always stored as 9999.12. So the value will always have this format. To adjust the decimals we then use number_format. But the further formatting with the separators is now still handled by gravity forms.

[omarkasem]

@doekenorg No worries at all :) And thanks for the new comments, It looks better now for sure

[mrcasual]

@omarkasem, could you please provide a status update? If this is implemented, please make sure that all tests pass and update the project status to Blocked: Needs Review.

[omarkasem]

@mrcasual It's working but the tests needs to be changed as I mentioned before here

So should I work on the tests change? If so I will need some help setting up docker tests on my localhost as I tried before and i couldn't make it fully work

[mrcasual]

@mrcasual It's working but the tests needs to be changed as I mentioned before here

So should I work on the tests change? If so I will need some help setting up docker tests on my localhost as I tried before and i couldn't make it fully work

@omarkasem, yes, please. You can use our Docker unit tests or run tests locally:

ln -nsf /path/to/gravityforms /tmp
MYSQL_DATABASE=<db> \
MYSQL_USER=<user> \
MYSQL_PASSWORD=<pass> \
MYSQL_HOST=<host> \
WP_TESTS_DIR=<path/to/wp-develop/tests/phpunit> \
WP_SOURCES_DIR=<path/to/wp-develop/src> \
php /path/to/phpunit7 --no-coverage

[doekenorg]

Even better, just set those values in your own phpunit.xml :-D Thats why we removed it from the repo, and kept the .dist. Nobody got time for typing all that. :-D

[mrcasual]

@omarkasem, I can see that the display of numbers with thousand separators has been fixed, but not the default format.

☝️ 9999.99 should be 9999,99.

[omarkasem]

@mrcasual

It should be working with the latest commit if i understood correctly

[mrcasual]

@omarkasem, it does work! Thank you. Could you please write unit (integration) tests for this particular issue? You can add them here or find a better-suited place.

[omarkasem]

@mrcasual I added it on it's own function with all 4 possible tests it can go and i also added a fourth test for decimal dot test so we are cover everything.

[mrcasual]

Hey @omarkasem, there are a couple of issues related to when the number format is changed to Currency on the form field:

1. The thousand separator is forced, regardless of whether the View field is configured to show it or not.
2. The displayed number with a thousand separator loses the currency symbol.
3. Decimal formatting differs when the thousand separator is enabled (e.g., $5.50 vs 5.5). "Existing precision" should match that of the Gravity Forms.

Please address these issues and ensure to add tests as well.

[omarkasem]

@mrcasual Are you sure you are on the correct branch? I tested it multiple times and i only got your results when i'm in the develop branch. Please correct me if i misunderstood

[mrcasual]

@omarkasem, you're right, sorry! Everything works. Thank you.