0017354: Non-required custom fields with a minimum length fail to validate - MantisBT

ID	Project	Category	View Status	Date Submitted	Last Update
0017354	mantisbt	custom fields	public	2014-05-19 04:20	2016-11-27 00:45

Reporter	jpeschke	Assigned To	cproensa
Priority	normal	Severity	minor	Reproducibility	always
Status	closed	Resolution	no change required
Product Version	1.2.17

Summary	0017354: Non-required custom fields with a minimum length fail to validate
Description	We have just migrated from version 1.2.9 to 1.2.17. There is now a problem with a custom field we have defined for our workflow: The custom field is marked not required for report generation but has a minimum and maximum length (1,10) When submitting a report, the custom field doesn't validate (Message "Invalid value for...")
Steps To Reproduce	1.) Create a custom field (Manage custom fields) 2.) Set min and max length to some value 3.) Disable "Required On Report" 4.) Disable "Display When Reporting Issues" 5.) Report Issue
Additional Information	I have compared source code of the current version to our old (1.2.9) one. There have been changes in the custom_field_api.php that check the minimum length / maximum length and disregarding the "required" state of this field. I think the field lengths should only be checked if the field is required. A quick fix may be to only validate the field if it is required: bug_report.php: Instead of if ( !custom_field_validate( $t_id, gpc_get_custom_field( "customfield$t_id", $t_def['type'], NULL ) ) ) { error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR ); } Maybe something like this: if ( $t_def['require_report'] ) { if ( !custom_field_validate... ... }
Tags	patch
Attached Files	CustomFieldValidation.patch (1,605 bytes) Index: bug_report.php =================================================================== --- bug_report.php (revision 449) +++ bug_report.php (working copy) @@ -126,9 +126,11 @@ trigger_error( ERROR_EMPTY_FIELD, ERROR ); } - if ( !custom_field_validate( $t_id, gpc_get_custom_field( "custom_field_$t_id", $t_def['type'], NULL ) ) ) { - error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); - trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR ); + if ( $t_def['require_report'] ) { + if ( !custom_field_validate( $t_id, gpc_get_custom_field( "custom_field_$t_id", $t_def['type'], NULL ) ) ) { + error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); + trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR ); + } } } @@ -162,10 +164,12 @@ } $t_def = custom_field_get_definition( $t_id ); - if( !custom_field_set_value( $t_id, $t_bug_id, gpc_get_custom_field( "custom_field_$t_id", $t_def['type'], $t_def['default_value'] ), false ) ) { - error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); - trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR ); - } + if ( $t_def['require_report'] ) { + if( !custom_field_set_value( $t_id, $t_bug_id, gpc_get_custom_field( "custom_field_$t_id", $t_def['type'], $t_def['default_value'] ), false ) ) { + error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); + trigger_error( ERROR_CUSTOM_FIELD_INVALID_VALUE, ERROR ); + } + } } $f_master_bug_id = gpc_get_int( 'm_id', 0 ); CustomFieldValidation.patch (1,605 bytes)

jpeschke 2014-05-19 04:42 reporter ~0040570	I submitted a patch that fixes the problem for us. Not sure if this is the "right" way to do it though.

cproensa 2016-08-16 20:49 developer ~0053825	The validation is working as designed. Validation of field content should not be tied to the fact that it is required or not in any step of issue cycle. Otherwise, not validating the field content would lead to having bad data when, as you suggest, the field should later be validated.

cproensa 2016-08-16 20:51 developer ~0053826	Or, if i unnderstood wrong, do you mean that a field that is not showed in report step, thus having an empty content, triggers an error due to the validation rule? If so, that is solved in version 1.3

cproensa 2016-11-20 16:17 developer ~0054554	jpeschke, You did not provide any feedback; I am therefore resolving this issue as "no change required". Feel free to reopen the issue at a later time and provide the requested information.