From 890c36eb616cf5f1d115efa8b5de61b8b737937a Mon Sep 17 00:00:00 2001 From: Victor Boctor Date: Sun, 9 Jan 2011 00:00:44 -0800 Subject: [PATCH] Issue #12619: Custom fields are reported as required after upgrade from 1.1.0 to 1.2.3. Issue #11684: Incorrect error "A necessary field "MyField" was empty. Please recheck your inputs." when submitting new issue. There is a lot of details about this issue in issue #11684. In 1.2.1 a breaking change was introduced which blocks users from submitting / updating issues for projects with certain custom field configurations. The aim of this fix is to unblock these users. We can do follow up fixes to improve on this for 1.2.x or to come up with the alternative strategy for 1.3.x. The goal is to come up with an approach that doesn't break existing users. The worst thing is that users upgrade and then find out that they can continue to use the product. If there is a path for upgrade to avoid these issues, then we should automate it as part of the upgrade script. I haven't marked the issue as completely fixed until dhx checks it out, since he was working on the issue. I've used my fix for #12619 + one more fix for the update code path. When I compared my fix to the attached patches, I note that the patches have more changes to handle things like special cases for checkboxes and the case where user doesn't have read-write. We may incorporate these after we do the necessary validations. Thanks for Sergiodf for contributing the patches. --- bug_report.php | 9 +++------ bug_update.php | 6 +----- core/custom_field_api.php | 13 ++++++++++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/bug_report.php b/bug_report.php index 0bc43e6ef4..129559a263 100644 --- a/bug_report.php +++ b/bug_report.php @@ -105,14 +105,11 @@ # Produce an error if the field is required but wasn't posted if ( !gpc_isset_custom_field( $t_id, $t_def['type'] ) && - ( $t_def['require_report'] || - $t_def['type'] == CUSTOM_FIELD_TYPE_ENUM || - $t_def['type'] == CUSTOM_FIELD_TYPE_LIST || - $t_def['type'] == CUSTOM_FIELD_TYPE_MULTILIST || - $t_def['type'] == CUSTOM_FIELD_TYPE_RADIO ) ) { + ( $t_def['require_report'] ) ) { error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); 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 ); @@ -135,7 +132,7 @@ # Handle custom field submission foreach( $t_related_custom_field_ids as $t_id ) { - # Do not set custom field value if user has no write access. + # Do not set custom field value if user has no write access if( !custom_field_has_write_access( $t_id, $t_bug_id ) ) { continue; } diff --git a/bug_update.php b/bug_update.php index 6b7617db34..3ca886aa22 100644 --- a/bug_update.php +++ b/bug_update.php @@ -139,11 +139,7 @@ # Produce an error if the field is required but wasn't posted if ( !gpc_isset_custom_field( $t_id, $t_def['type'] ) && - ( $t_def['require_' . $t_custom_status_label] || - $t_def['type'] == CUSTOM_FIELD_TYPE_ENUM || - $t_def['type'] == CUSTOM_FIELD_TYPE_LIST || - $t_def['type'] == CUSTOM_FIELD_TYPE_MULTILIST || - $t_def['type'] == CUSTOM_FIELD_TYPE_RADIO ) ) { + ( $t_def['require_' . $t_custom_status_label] ) ) { error_parameters( lang_get_defaulted( custom_field_get_field( $t_id, 'name' ) ) ); trigger_error( ERROR_EMPTY_FIELD, ERROR ); } diff --git a/core/custom_field_api.php b/core/custom_field_api.php index d2f3c61702..d69a1e74fe 100644 --- a/core/custom_field_api.php +++ b/core/custom_field_api.php @@ -1169,21 +1169,28 @@ function custom_field_validate( $p_field_id, $p_value ) { $t_valid &= ( $p_value == null ) || ( ( $p_value !== false ) && ( $p_value > 0 ) ); break; case CUSTOM_FIELD_TYPE_CHECKBOX: + case CUSTOM_FIELD_TYPE_MULTILIST: # Checkbox fields can hold a null value (when no checkboxes are ticked) - if( $p_value === '' ) { + if ( $p_value === '' ) { break; } + # If checkbox field value is not null then we need to validate it... (note: no "break" statement here!) - case CUSTOM_FIELD_TYPE_MULTILIST: $t_values = explode( '|', $p_value ); $t_possible_values = custom_field_prepare_possible_values( $row['possible_values'] ); $t_possible_values = explode( '|', $t_possible_values ); $t_invalid_values = array_diff( $t_values, $t_possible_values ); $t_valid &= ( count( $t_invalid_values ) == 0 ); break; - case CUSTOM_FIELD_TYPE_ENUM: case CUSTOM_FIELD_TYPE_LIST: + case CUSTOM_FIELD_TYPE_ENUM: case CUSTOM_FIELD_TYPE_RADIO: + # List fields can hold be empty (when no checkboxes are ticked) + if ( $p_value === '' ) { + break; + } + + # If list field value is not empty then we need to validate it... (note: no "break" statement here!) $t_possible_values = custom_field_prepare_possible_values( $row['possible_values'] ); $t_values_arr = explode( '|', $t_possible_values ); $t_valid &= in_array( $p_value, $t_values_arr );