Issue #12619: Custom fields are reported as required after upgrade fr…

…om 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

@@ -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;
 		}

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 );
 		}

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 );