Default value (#2247)

## Defaults for fields

This change introduces the ability for any field type to OPTIONALLY specify a custom default value. See also https://openshift.io/openshiftio/Openshift_io/plan/detail/35 and openshiftio/openshift.io#3832.

For lists and simple types, the only restriction is that the default value is of the same kind as the list or simple type (e.g. integer, float, string, user, label, ...). For enum fields the custom default value needs to be in the list of allowed values.

### Example usage of the feature

As an example, we've set the default for severity and priority fields to a medium value instead of the first on in the list, which would be a high severity/priority. (See also openshiftio/openshift.io#3832)

## Improvements to tests

### Space template import
When a space template is imported it creates or overrides the work item types. With this change I've introduced a cascading validation that checks 

1. the work item types, 
2. the fields inside each work item type
3. the field type of each field.

This ensures that people don't accidentally specify a float default (e.g. `33.45`) for an integer field.

Before this change, most of the input and output validation happened at runtime when a value was converted into another representation. Now, the validation happens at import time and also when a default value for a field is calculated.

controller/test-files/work_item_type/list/ok_agile.res.payload.golden.json

@@ -416,6 +416,7 @@
             "required": false,
             "type": {
               "baseType": "string",
+              "defaultValue": "P3 - Medium",
               "kind": "enum",
               "values": [
                 "P1 - Critical",
@@ -458,6 +459,7 @@
             "required": false,
             "type": {
               "baseType": "string",
+              "defaultValue": "SEV3 - Medium",
               "kind": "enum",
               "values": [
                 "SEV1 - Urgent",

controller/test-files/work_item_type/list/ok_scrum.res.payload.golden.json

@@ -709,6 +709,7 @@
             "required": false,
             "type": {
               "baseType": "string",
+              "defaultValue": "Medium",
               "kind": "enum",
               "values": [
                 "Critical",

controller/workitemtype.go

@@ -101,16 +101,26 @@ func ConvertWorkItemTypeFromModel(request *http.Request, t *workitem.WorkItemTyp
 	return converted
 }
 
-// converts the field type from modesl to app representation
+// converts the field type from model to app representation
 func ConvertFieldTypeFromModel(t workitem.FieldType) app.FieldType {
 	result := app.FieldType{}
 	result.Kind = string(t.GetKind())
-	switch t2 := t.(type) {
+	switch modelFieldType := t.(type) {
 	case workitem.ListType:
-		result.ComponentType = ptr.String(string(t2.ComponentType.GetKind()))
+		result.ComponentType = ptr.String(string(modelFieldType.ComponentType.GetKind()))
+		if modelFieldType.DefaultValue != nil {
+			result.DefaultValue = &modelFieldType.DefaultValue
+		}
 	case workitem.EnumType:
-		result.BaseType = ptr.String(string(t2.BaseType.GetKind()))
-		result.Values = t2.Values
+		result.BaseType = ptr.String(string(modelFieldType.BaseType.GetKind()))
+		result.Values = modelFieldType.Values
+		if modelFieldType.DefaultValue != nil {
+			result.DefaultValue = &modelFieldType.DefaultValue
+		}
+	case workitem.SimpleType:
+		if modelFieldType.DefaultValue != nil {
+			result.DefaultValue = &modelFieldType.DefaultValue
+		}
 	}
 
 	return result
@@ -130,7 +140,19 @@ func ConvertFieldTypeToModel(t app.FieldType) (workitem.FieldType, error) {
 		if !componentType.IsSimpleType() {
 			return nil, fmt.Errorf("Component type is not list type: %T", componentType)
 		}
-		return workitem.ListType{workitem.SimpleType{*kind}, workitem.SimpleType{*componentType}}, nil
+		listType := workitem.ListType{
+			SimpleType:    workitem.SimpleType{Kind: *kind},
+			ComponentType: workitem.SimpleType{Kind: *componentType},
+		}
+		// convert list default value from app to model
+		if t.DefaultValue != nil {
+			fieldType, err := listType.SetDefaultValue(*t.DefaultValue)
+			if err != nil {
+				return nil, errs.Wrapf(err, "failed to convert default list value: %+v", *t.DefaultValue)
+			}
+			return fieldType, nil
+		}
+		return listType, nil
 	case workitem.KindEnum:
 		bt, err := workitem.ConvertAnyToKind(*t.BaseType)
 		if err != nil {
@@ -139,24 +161,44 @@ func ConvertFieldTypeToModel(t app.FieldType) (workitem.FieldType, error) {
 		if !bt.IsSimpleType() {
 			return nil, fmt.Errorf("baseType type is not list type: %T", bt)
 		}
-		baseType := workitem.SimpleType{*bt}
+		baseType := workitem.SimpleType{Kind: *bt}
 
+		// convert enum values from app to model
 		values := t.Values
 		converted, err := workitem.ConvertList(func(ft workitem.FieldType, element interface{}) (interface{}, error) {
 			return ft.ConvertToModel(element)
 		}, baseType, values)
 		if err != nil {
 			return nil, errs.WithStack(err)
 		}
-		return workitem.EnumType{ // TODO(kwk): handle RewritableValues here?
+
+		enumType := workitem.EnumType{ // TODO(kwk): handle RewritableValues here?
 			SimpleType: workitem.SimpleType{
 				Kind: *kind,
 			},
 			BaseType: baseType,
 			Values:   converted,
-		}, nil
+		}
+		// convert enum default value from app to model
+		if t.DefaultValue != nil {
+			fieldType, err := enumType.SetDefaultValue(*t.DefaultValue)
+			if err != nil {
+				return nil, errs.Wrapf(err, "failed to convert default enum value: %+v", *t.DefaultValue)
+			}
+			return fieldType, nil
+		}
+		return enumType, nil
 	default:
-		return workitem.SimpleType{*kind}, nil
+		simpleType := workitem.SimpleType{Kind: *kind}
+		// convert simple type default value from app to model
+		if t.DefaultValue != nil {
+			fieldType, err := simpleType.SetDefaultValue(*t.DefaultValue)
+			if err != nil {
+				return nil, errs.Wrapf(err, "failed to convert default simple type value: %+v", *t.DefaultValue)
+			}
+			return fieldType, nil
+		}
+		return simpleType, nil
 	}
 }
 

design/workitemtype.go

@@ -12,7 +12,7 @@ var fieldType = a.Type("fieldType", func() {
 	a.Attribute("componentType", d.String, "The kind of type of the individual elements for a list type. Required for list types. Must be a simple type, not  enum or list")
 	a.Attribute("baseType", d.String, "The kind of type of the enumeration values for an enum type. Required for enum types. Must be a simple type, not  enum or list")
 	a.Attribute("values", a.ArrayOf(d.Any), "The possible values for an enum type. The values must be of a type convertible to the base type")
-
+	a.Attribute("defaultValue", d.Any, "Optional default value (if any)")
 	a.Required("kind")
 })
 

spacetemplate/assets/agile.yaml

@@ -123,6 +123,7 @@ work_item_types:
         - SEV2 - High
         - SEV3 - Medium
         - SEV4 - Low
+        default_value: SEV3 - Medium
     "priority":
       label: Priority
       description: The order in which the developer should resolve a defect.
@@ -137,6 +138,7 @@ work_item_types:
         - P2 - High
         - P3 - Medium
         - P4 - Low
+        default_value: P3 - Medium
     "resolution":
       label: Resolution
       description: >

spacetemplate/assets/scrum.yaml

@@ -58,6 +58,7 @@ work_item_types:
           - Minor
           - Optional
           - Trivial
+          default_value: Minor
 
 - id: &impedimentID "ec6918d6-f732-4fc0-a902-6571415aa73c"
   extends: *scrumCommonTypeID
@@ -178,6 +179,7 @@ work_item_types:
         - High
         - Medium
         - Low
+        default_value: Medium
     "found_in":
       label: Found in
       description: >

spacetemplate/importer/import_helper.go

@@ -29,11 +29,15 @@ func (s *ImportHelper) Validate() error {
 		return errs.Wrap(err, "failed to validate space template")
 	}
 
-	// Ensure all artifacts have the correct space template ID set
+	// Ensure all artifacts have the correct space template ID set and are
+	// valid
 	for _, wit := range s.WITs {
 		if wit.SpaceTemplateID != s.Template.ID {
 			return errors.NewBadParameterError("work item types's space template ID", wit.SpaceTemplateID.String()).Expected(s.Template.ID.String())
 		}
+		if err := wit.Validate(); err != nil {
+			return errs.Wrapf(err, `failed to validate work item type "%s" (ID=%s)`, wit.Name, wit.ID)
+		}
 	}
 	for _, wilt := range s.WILTs {
 		if wilt.SpaceTemplateID != s.Template.ID {

spacetemplate/importer/importer_blackbox_test.go

@@ -283,6 +283,7 @@ work_item_types:
       required: yes
       type:
         kind: string
+        default_value: "foobar"
     state:
       label: State
       description: The state of the bug
@@ -295,6 +296,7 @@ work_item_types:
         values:
           - new
           - closed
+        default_value: closed
     priority:
       label: Priority
       description: The priority of the bug
@@ -303,7 +305,8 @@ work_item_types:
         simple_type:
           kind: list
         component_type:
-          kind: integer
+          kind: float
+        default_value: 42.0
 work_item_link_types:
 - id: "` + wiltID.String() + `"
   name: Blocker
@@ -375,7 +378,8 @@ func getValidTestTemplateParsed(t *testing.T, spaceTemplateID, witID, wiltID uui
 						Description: "The title of the bug",
 						Required:    true,
 						Type: workitem.SimpleType{
-							Kind: workitem.KindString,
+							Kind:         workitem.KindString,
+							DefaultValue: "foobar",
 						},
 					},
 					"state": {
@@ -390,6 +394,7 @@ func getValidTestTemplateParsed(t *testing.T, spaceTemplateID, witID, wiltID uui
 								"new",
 								"closed",
 							},
+							DefaultValue: "closed",
 						},
 					},
 					"priority": {
@@ -398,7 +403,8 @@ func getValidTestTemplateParsed(t *testing.T, spaceTemplateID, witID, wiltID uui
 						Required:    false,
 						Type: workitem.ListType{
 							SimpleType:    workitem.SimpleType{Kind: workitem.KindList},
-							ComponentType: workitem.SimpleType{Kind: workitem.KindInteger},
+							ComponentType: workitem.SimpleType{Kind: workitem.KindFloat},
+							DefaultValue:  42.0,
 						},
 					},
 				},

spacetemplate/importer/repository.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/davecgh/go-spew/spew"
 	"github.com/fabric8-services/fabric8-wit/errors"
 	"github.com/fabric8-services/fabric8-wit/id"
 	"github.com/fabric8-services/fabric8-wit/log"
@@ -145,9 +146,10 @@ func (r *GormRepository) createOrUpdateWITs(ctx context.Context, s *ImportHelper
 			loadedWIT.Icon = wit.Icon
 			loadedWIT.CanConstruct = wit.CanConstruct
 
-			//--------------------------------------------------------------------------------
-			// Double check all existing fields are still present in new fields with same type
-			//--------------------------------------------------------------------------------
+			//------------------------------------------------------------------
+			// Double check all fields from the old work item type are still
+			// present in new work item type and still have the same field type.
+			//------------------------------------------------------------------
 			// verify that FieldTypes are same as loadedWIT
 			toBeFoundFields := map[string]workitem.FieldType{}
 			for k, fd := range loadedWIT.Fields {
@@ -156,16 +158,28 @@ func (r *GormRepository) createOrUpdateWITs(ctx context.Context, s *ImportHelper
 			// Remove fields directly defined in WIT
 			for fieldName, fd := range wit.Fields {
 				// verify FieldType with original value
-				if originalType, ok := toBeFoundFields[fieldName]; ok {
-					if equal := fd.Type.Equal(originalType); !equal {
+				if oldFieldType, ok := toBeFoundFields[fieldName]; ok {
+
+					// When comparing the new and old field types we don't want
+					// to compare the default value. That is why we always
+					// overwrite the default value of the old type with the
+					// default value of the new type.
+
+					defVal := fd.Type.GetDefaultValue()
+					oldFieldType, err = oldFieldType.SetDefaultValue(defVal)
+					if err != nil {
+						return errs.Wrapf(err, "failed to overwrite default of old field type with %+v (%[1]T)", defVal)
+					}
+
+					if equal := fd.Type.Equal(oldFieldType); !equal {
 						// Special treatment for EnumType
-						origEnum, ok1 := originalType.(workitem.EnumType)
+						origEnum, ok1 := oldFieldType.(workitem.EnumType)
 						newEnum, ok2 := fd.Type.(workitem.EnumType)
 						if ok1 && ok2 {
 							equal = newEnum.EqualEnclosing(origEnum)
 						}
 						if !equal {
-							return errs.Errorf("type of the field %s changed from %s to %s", fieldName, originalType, fd.Type)
+							return errs.Errorf("type of the field %s changed from %+v to %+v", fieldName, spew.Sdump(oldFieldType), spew.Sdump(fd.Type))
 						}
 					}
 				}