View Issue Details

WikiRelated ChangesetsJump to Notes
IDProjectCategoryView StatusDate SubmittedLast Update
0012619mantisbtcustom fieldspublic2010-12-18 17:362013-10-11 23:23
Reportervboctor Assigned Tovboctor  
PriorityhighSeverityminorReproducibilityhave not tried
Status closedResolutionduplicate 
Product Version1.2.3 
Target Version1.2.5 
Summary0012619: Custom fields are reported as required after upgrade from 1.1.0 to 1.2.3
Description

For a project that had associated non-required custom fields, once the MantisBT instance is upgraded to 1.2.3, the users started getting errors that a required custom field was not provided. The reason for that is the change of definition of required custom fields between the two versions:

1.1.0 - a required custom field is one that is marked as required.
1.2.3 - a required custom field is one that is marked as required OR is of type LIST or is of type MULTISELECT LIST or is of type RADIO or is of type ENUM.

The problem in this case is that the project had custom field of type LIST. The custom field was configured to not be visible on the report page.

We could fix this issue by reverting back to the 1.1.0 definition, but we can also attempt to go with the following definition:

  1. A custom field is required if it is required or of type ENUM / RADIO
  2. List / MultiSelect List are allowed to be empty.

Ideas are welcome. The goal is to find a setup that makes sense going forward, but also doesn't break backward compatibility.

Tagspatch
Attached Files
12619.diff (2,483 bytes)    
From 4311399909a89c72c9eb1cc3d759d29b7d2c9f46 Mon Sep 17 00:00:00 2001
From: Victor Boctor <vboctor@gmail.com>
Date: Sat, 18 Dec 2010 14:41:11 -0800
Subject: [PATCH] Fix #12619: Custom fields are reported as required after upgrade from 1.1.0 to 1.2.3

---
 bug_report.php            |    2 --
 core/custom_field_api.php |   18 ++++++++++++------
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/bug_report.php b/bug_report.php
index 32393ce..47743bc 100644
--- a/bug_report.php
+++ b/bug_report.php
@@ -107,8 +107,6 @@
 		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 ) ) {
 			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 3b0baa1..778fbc0 100644
--- a/core/custom_field_api.php
+++ b/core/custom_field_api.php
@@ -1175,14 +1175,20 @@ function custom_field_validate( $p_field_id, $p_value ) {
 			}
 			# 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 );
+			if ( $p_value !== '' ) {
+				$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:
+			if ( $p_value === '' ) {
+				break;
+			}
+			# If list field value is not empty then we need to validate it... (note: no "break" statement here!)
+		case CUSTOM_FIELD_TYPE_ENUM:
 		case CUSTOM_FIELD_TYPE_RADIO:
 			$t_possible_values = custom_field_prepare_possible_values( $row['possible_values'] );
 			$t_values_arr = explode( '|', $t_possible_values );
-- 
1.7.3.3
12619.diff (2,483 bytes)   

Relationships

duplicate of 0011684 closedvboctor Incorrect error "A necessary field "MyField" was empty. Please recheck your inputs." when submitting new issue 

Activities

vboctor

vboctor

2010-12-18 17:44

manager   ~0027641

Attached is a proposed fix. I'm also ok with completely going for the 1.1.0 approach where empty values are valid unless required is set to true. In fact the 1.1.0 might be the safest option.
ldsandon

ldsandon

2010-12-19 09:26

reporter   ~0027646

IMHO 1.1.0 is the better option. As database fields, if a custom field is not marked as "required" (aka NOT NULL) it should allow for NULL values. Especially since custom fields may not available to reporters, and may not have simple default values. I believe it is better to leave those fields as "not assigned" (telling that they need to be set by someone who has the responsibility) instead of setting an unproper default value (how someone can tell if it was properly set or not?).
djcarr

djcarr

2010-12-19 17:27

reporter   ~0027647

Last edited: 2010-12-19 17:28

Yes please do rollback, at least for the rest of the 1.2.x releases as this regression in 1.2.1 affected a lot of people who installed 1.2.0 and then continued upgrading to the minor releases.

I understand the desire for data integrity that motivated this change. Would you instead consider introducing it in 1.3.0, with more consideration of custom fields that become visible & populated later in the workflow.

Related Changesets

MantisBT: master-1.2.x 890c36eb

2011-01-09 03:00

vboctor


Details Diff		 Issue 0012619: Custom fields are reported as required after upgrade from 1.1.0 to 1.2.3.
Issue 0011684: 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 0011684. 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 0012619 + 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.		 Affected Issues
0011684, 0012619
mod - bug_report.php Diff File
mod - bug_update.php Diff File
mod - core/custom_field_api.php Diff File

MantisBT: master-1.2.x 7ab6eb38

2011-01-09 03:23

vboctor


Details Diff		 Follow up fix for:

Issue 0012619: Custom fields are reported as required after upgrade from 1.1.0 to 1.2.3.
Issue 0011684: Incorrect error "A necessary field "MyField" was empty. Please recheck your inputs." when submitting new issue.		 Affected Issues
0011684, 0012619
mod - core/custom_field_api.php Diff File

MantisBT: master 3d969053

2013-06-19 21:31

dregad


Details Diff		 Fix 0016020: allow empty required custom fields

This is a port of the following 1.2.x commits:
- 890c36eb616cf5f1d115efa8b5de61b8b737937a
- 7ab6eb383a29a73ad65ade7de504d45477c87fb3

See related issues 0012619, 0011684 for details		 Affected Issues
0011684, 0012619, 0016020
mod - bug_report.php Diff File
mod - core/custom_field_api.php Diff File