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

Leading zero values not respected #869

Closed
gyrus opened this issue Feb 28, 2017 · 9 comments
Closed

Leading zero values not respected #869

gyrus opened this issue Feb 28, 2017 · 9 comments
Labels

Comments

@gyrus
Copy link
Contributor

gyrus commented Feb 28, 2017

If a select is populated with, say, months, where leading zeros are added to numeric values (e.g. December is 12 but January is 01) the correct value should be selected after saving.

However, this isn't the case. I think it's these lines in 2.2.4, includes/types/CMB2_Type_Multi_Base.php:77:

	if ( is_numeric( $value ) ) {
		$value = intval( $value );
	}

Is there a reason for this?

@jtsternberg
Copy link
Member

Is there a particular field configuration that causes this for you?

@gyrus
Copy link
Contributor Author

gyrus commented Feb 28, 2017

This is the field definition:

$month_options = array( '00' => '[No month]' );
for ( $m = 1; $m <= 12; $m++ ) {
	$month_options[ str_pad( $m, 2, '0', STR_PAD_LEFT ) ] = date("F", mktime(0, 0, 0, $m, 10));
}
$cmb->add_field( array(
	'name'				=> __( 'Written - month' ),
	'id'				=>  '_pilau_written-month',
	'type'				=> 'select',
	'options'			=> $month_options,
	'on_front'			=> false,
) );

@bradp bradp added the bug label Apr 3, 2017
@timothyjensen
Copy link

timothyjensen commented Aug 14, 2017

I am having a related issue with the code that @gyrus referred to: https://github.com/CMB2/CMB2/blob/trunk/includes/types/CMB2_Type_Multi_Base.php#L81-L83.

My issue is that I'm dynamically populating the select options with Gravity Forms field IDs, so sometimes the value is a float, such as 1.3 (for example, the first-name field in a Gravity Form). However, the code above converts the value from, say 1.3 to 1. CMB2 already does a good job of escaping the data before this point, so perhaps it's safe to remove the intval() function call on line 82. Commenting out those lines fixes the issue but, but it breaks all other option values that are numeric.

@jtsternberg
Copy link
Member

@timothyjensen since you have the optimal testing scenario, can you test by changing those lines to this:

if ( is_numeric( $value ) ) {
	$value = is_float( $value ) ? floatval( $value ) : intval( $value );
}

@timothyjensen
Copy link

@jtsternberg Thanks for addressing this. Unfortunately the code you suggested doesn't quite fix the issue. I should have been more accurate in my previous description; the option value is saved as a string in the database. Because of that is_float( $value ) returns false when retrieving the meta value. Here's a screenshot of what I'm talking about: http://cloud.timj.us/0Q1r152U0u2d.

@jtsternberg
Copy link
Member

Can you please test #1013?

@timothyjensen
Copy link

Works perfectly!

@timothyjensen
Copy link

Also tested #1013 with a leading zero value like @gyrus originally reported, and that issue is now resolved as well.

@jtsternberg
Copy link
Member

I added some leading zero unit tests as well: c2c4d47

Thanks for testing, i'll merge it in now.

jtsternberg added a commit that referenced this issue Aug 16, 2017
…e problems. See #869. Squashed commit of the following:

commit c2c4d47
Author: Justin Sternberg <justin@dsgnwrks.pro>
Date:   Wed Aug 16 09:31:15 2017 -0400

    Test leading zero values as well. For #869

commit 45ade84
Author: Justin Sternberg <justin@dsgnwrks.pro>
Date:   Tue Aug 15 18:43:54 2017 -0400

    Add CMB2_Utils::normalize_if_numeric to address floats as select value problems. See #869

commit 94cf8bb
Author: Justin Sternberg <justin@dsgnwrks.pro>
Date:   Tue Aug 15 17:45:36 2017 -0400

    Fix "PHP Strict Standards: Static function should not be abstract"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants