? MISC-FIELD-CHANGES-REAPPLY.patch ? Makefile ? d6-50-nodes.sql.gz ? d7-50-nodes-new.sql.gz ? d7-50-nodes.sql.gz ? head.kpf ? patches ? modules/field/modules/options/options.test ? scripts/OLD-generate-autoload.pl ? scripts/generate-autoload.pl ? sites/all/cck ? sites/all/modules/color ? sites/all/modules/combofield ? sites/all/modules/devel ? sites/all/modules/pbs ? sites/all/modules/taint ? sites/default/files ? sites/default/private ? sites/default/settings.php Index: modules/field/field.api.php =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.api.php,v retrieving revision 1.34 diff -u -F^[fc] -r1.34 field.api.php --- modules/field/field.api.php 10 Sep 2009 06:31:38 -0000 1.34 +++ modules/field/field.api.php 20 Sep 2009 20:42:54 -0000 @@ -1274,6 +1274,34 @@ function hook_field_create_instance($ins } /** + * Forbid a field update from occuring. + * + * @param $field + * The field as it will be post-update. + * @param $prior_field + * The field as it is pre-update. + * @param $has_data + * Whether any data already exists for this field. + * @return + * Throws a FieldException to prevent the update from occuring. + */ +function hook_field_update_field_forbid($field, $prior_field, $has_data) { + // A 'list' field stores integer keys mapped to display values. If + // the new field will have fewer values, and any data exists for the + // abandonded keys, the field will have no way to display them. So, + // forbid such an update. + if ($has_data && count($field['settings']['allowed_values']) < count($prior_field['settings']['allowed_values'])) { + // Identify the keys that will be lost. + $lost_keys = array_diff(array_keys($field['settings']['allowed_values']), array_keys($prior_field['settings']['allowed_values'])); + // If any data exist for those keys, forbid the update. + $count = field_attach_query($prior_field['id'], array('value', $lost_keys, 'IN'), 1); + if ($count > 0) { + throw new FieldException("Cannot update a list field not to include keys with existing data"); + } + } +} + +/** * Act on a field being deleted. * * This hook is invoked just after field is deleted. Index: modules/field/field.attach.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.attach.inc,v retrieving revision 1.46 diff -u -F^[fc] -r1.46 field.attach.inc --- modules/field/field.attach.inc 10 Sep 2009 22:31:58 -0000 1.46 +++ modules/field/field.attach.inc 20 Sep 2009 20:42:54 -0000 @@ -1012,6 +1012,19 @@ function field_attach_query_revisions($f } /** + * Determine whether a field has any data. + * + * @param $field + * A field structure. + * @return + * TRUE if the field has data for any object; FALSE otherwise. + */ +function field_attach_field_has_data($field) { + $results = field_attach_query($field['id'], array(), 1); + return !empty($results); +} + +/** * Generate and return a structured content array tree suitable for * drupal_render() for all of the fields on an object. The format of * each field's rendered content depends on the display formatter and Index: modules/field/field.crud.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.crud.inc,v retrieving revision 1.31 diff -u -F^[fc] -r1.31 field.crud.inc --- modules/field/field.crud.inc 11 Sep 2009 03:42:34 -0000 1.31 +++ modules/field/field.crud.inc 20 Sep 2009 20:42:54 -0000 @@ -302,6 +302,87 @@ function field_create_field($field) { return $field; } +/* + * Update a field. + * + * Any update which changes the field's schema is forbidden if any + * data for the field already exists. Also, any module may forbid an + * update even if the update does not change the field's schema; this + * may be uesful if the update would change existing data's semantics + * without changing the schema, or if there are external dependencies + * on field settings that cannot be updated. + * + * @param $field + * A complete field structure. The field $field['field_name'] must + * exist and will be updated to this structure. Any properties of + * $field that are missing will be left unchanged. + * @throw + * FieldException + * @see field_create_field() + */ +function field_update_field($field) { + // Check that the specified field exists. + $prior_field = field_read_field($field['field_name']); + if (empty($prior_field)) { + throw new FieldException("Attempt to update a nonexistent field."); + } + + // Use the prior field values for anything not specifically set by the new + // field to be sure that all values are set. + $field += $prior_field; + $field['settings'] += $prior_field['settings']; + + // Field type cannot be changed. + if ($field['type'] != $prior_field['type']) { + throw new FieldException("Cannot change an existing field's type."); + } + + // Collect the new storage information, since what is in + // $prior_field may no longer be right. + $schema = (array) module_invoke($field['module'], 'field_schema', $field); + $schema += array('columns' => array(), 'indexes' => array()); + // 'columns' are hardcoded in the field type. + $field['columns'] = $schema['columns']; + // 'indexes' can be both hardcoded in the field type, and specified in the + // incoming $field definition. + $field += array( + 'indexes' => array(), + ); + $field['indexes'] += $schema['indexes']; + + $has_data = field_attach_field_has_data($field); + + // See if any module forbids the update by throwing an exception. + foreach (module_implements('field_update_forbid') as $module) { + $function = $module . '_field_update_forbid'; + $function($field, $prior_field, $has_data); + } + + // Tell the storage engine to update the field. Do this before + // saving the new definition since it still might fail. + module_invoke(variable_get('field_storage_module', 'field_sql_storage'), 'field_storage_update_field', $field, $prior_field, $has_data); + + // Save the new field definition. TODO: refactor with + // field_create_field. + + // The serialized 'data' column contains everything from $field that does not + // have its own column and is not automatically populated when the field is + // read. + $data = $field; + unset($data['columns'], $data['field_name'], $data['type'], $data['locked'], $data['module'], $data['cardinality'], $data['active'], $data['deleted']); + $field['data'] = $data; + + // Store the field and create the id. + $primary_key = array('id'); + drupal_write_record('field_config', $field, $primary_key); + + // Clear caches + field_cache_clear(TRUE); + + // Invoke external hooks after the cache is cleared for API consistency. + module_invoke_all('field_update_field', $field, $prior_field, $has_data); +} + /** * Read a single field record directly from the database. Generally, * you should use the field_info_field() instead. Index: modules/field/field.test =================================================================== RCS file: /cvs/drupal/drupal/modules/field/field.test,v retrieving revision 1.50 diff -u -F^[fc] -r1.50 field.test --- modules/field/field.test 18 Sep 2009 00:04:22 -0000 1.50 +++ modules/field/field.test 20 Sep 2009 20:42:54 -0000 @@ -1429,13 +1429,14 @@ class FieldCrudTestCase extends FieldTes public static function getInfo() { return array( 'name' => 'Field CRUD tests', - 'description' => 'Create / read /update fields.', + 'description' => 'Test field create, read, update, and delete.', 'group' => 'Field', ); } function setUp() { - parent::setUp('field_test'); + // field_update_field() tests use number.module + parent::setUp('field_test', 'number'); } // TODO : test creation with @@ -1697,6 +1698,103 @@ class FieldCrudTestCase extends FieldTes $this->assertEqual($entity->{$field['field_name']}[$langcode][$delta]['value'], $values[$delta]['value'], "Data in previously deleted field saves and loads correctly"); } } + + function testUpdateNonExistentField() { + $test_field = array('field_name' => 'does_not_exist', 'type' => 'number_decimal'); + try { + field_update_field($test_field); + $this->fail(t('Cannot update a field that does not exist.')); + } + catch (FieldException $e) { + $this->pass(t('Cannot update a field that does not exist.')); + } + } + + function testUpdateFieldType() { + $field = array('field_name' => 'field_type', 'type' => 'number_decimal'); + $field = field_create_field($field); + + $test_field = array('field_name' => 'field_type', 'type' => 'number_integer'); + try { + field_update_field($test_field); + $this->fail(t('Cannot update a field to a different type.')); + } + catch (FieldException $e) { + $this->pass(t('Cannot update a field to a different type.')); + } + } + + /** + * Test updating a field. + */ + function testUpdateField() { + // Create a decimal 5.2 field, update it to 5.3, and verify it + // rounds to 3 decimal places. + $field = array('field_name' => 'decimal53', 'type' => 'number_decimal', 'cardinality' => 3, 'settings' => array('precision' => 5, 'scale' => 2)); + $field = field_create_field($field); + $instance = array('field_name' => 'decimal53', 'bundle' => FIELD_TEST_BUNDLE); + $instance = field_create_instance($instance); + $field['settings']['scale'] = 3; + field_update_field($field); + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $entity->decimal53[FIELD_LANGUAGE_NONE][0]['value'] = '1.23'; + $entity->decimal53[FIELD_LANGUAGE_NONE][1]['value'] = '1.235'; + $entity->decimal53[FIELD_LANGUAGE_NONE][2]['value'] = '1.2355'; + field_attach_insert('test_entity', $entity); + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + field_attach_load('test_entity', array(0 => $entity)); + $this->assertEqual($entity->decimal53[FIELD_LANGUAGE_NONE][0]['value'], '1.23', t('2 decimal places are left alone')); + $this->assertEqual($entity->decimal53[FIELD_LANGUAGE_NONE][1]['value'], '1.235', t('3 decimal places are left alone')); + $this->assertEqual($entity->decimal53[FIELD_LANGUAGE_NONE][2]['value'], '1.236', t('4 decimal places are rounded to 3')); + } + + /** + * Test updating a field with data. + */ + function testUpdateFieldSchemaWithData() { + // Create a decimal 5.2 field and add some data. + $field = array('field_name' => 'decimal52', 'type' => 'number_decimal', 'settings' => array('precision' => 5, 'scale' => 2)); + $field = field_create_field($field); + $instance = array('field_name' => 'decimal52', 'bundle' => FIELD_TEST_BUNDLE); + $instance = field_create_instance($instance); + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $entity->decimal52[FIELD_LANGUAGE_NONE][0]['value'] = '1.235'; + field_attach_insert('test_entity', $entity); + + // Attempt to update the field in a way that would work without data. + $field['settings']['scale'] = 3; + try { + field_update_field($field); + $this->fail(t('Cannot update field schema with data.')); + } + catch (FieldException $e) { + $this->pass(t('Cannot update field schema with data.')); + } + } + + /** + * Test field type modules forbidding an update. + */ + function testUpdateFieldForbid() { + $field = array('field_name' => 'forbidden', 'type' => 'test_field', 'settings' => array('changeable' => 0, 'unchangeable' => 0)); + $field = field_create_field($field); + $field['settings']['changeable']++; + try { + field_update_field($field); + $this->pass(t("A changeable setting can be updated.")); + } + catch (FieldException $e) { + $this->fail(t("An unchangeable setting cannot be updated.")); + } + $field['settings']['unchangeable']++; + try { + field_update_field($field); + $this->fail(t("An unchangeable setting can be updated.")); + } + catch (FieldException $e) { + $this->pass(t("An unchangeable setting cannot be updated.")); + } + } } class FieldInstanceCrudTestCase extends FieldTestCase { Index: modules/field/modules/field_sql_storage/field_sql_storage.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/field_sql_storage/field_sql_storage.module,v retrieving revision 1.22 diff -u -F^[fc] -r1.22 field_sql_storage.module --- modules/field/modules/field_sql_storage/field_sql_storage.module 10 Sep 2009 22:31:58 -0000 1.22 +++ modules/field/modules/field_sql_storage/field_sql_storage.module 20 Sep 2009 20:42:54 -0000 @@ -204,6 +204,64 @@ function field_sql_storage_field_storage } /** + * Implement hook_field_update_field_forbid(). + * + * Forbid any field update that changes column definitions if there is + * any data. + */ +function field_sql_storage_field_update_forbid($field, $prior_field, $has_data) { + if ($has_data && $field['columns'] != $prior_field['columns']) { + throw new FieldException("field_sql_storage cannot change the schema for an existing field with data."); + } +} + +/** + * Implement hook_field_storage_update_field(). + */ +function field_sql_storage_field_storage_update_field($field, $prior_field, $has_data) { + $ret = array(); + + if (! $has_data) { + // There is no data. Re-create the tables completely. + $prior_schema = _field_sql_storage_schema($prior_field); + foreach ($prior_schema as $name => $table) { + db_drop_table($ret, $name, $table); + } + $schema = _field_sql_storage_schema($field); + foreach ($schema as $name => $table) { + db_create_table($ret, $name, $table); + } + } + else { + // There is data, so there are no column changes. Drop all the + // prior indexes and create all the new ones, except for all the + // priors that exist unchanged. + $table = _field_sql_storage_tablename($prior_field); + $revision_table = _field_sql_storage_revision_tablename($prior_field); + foreach ($prior_field['indexes'] as $name => $columns) { + if (!isset($field['indexes'][$name]) || $columns != $field['indexes'][$name]) { + $real_name = _field_sql_storage_indexname($field['field_name'], $name); + db_drop_index($ret, $table, $real_name); + db_drop_index($ret, $revision_table, $real_name); + } + } + $table = _field_sql_storage_tablename($field); + $revision_table = _field_sql_storage_revision_tablename($field); + foreach ($field['indexes'] as $name => $columns) { + if (!isset($prior_field['indexes'][$name]) || $columns != $prior_field['indexes'][$name]) { + $real_name = _field_sql_storage_indexname($field['field_name'], $name); + $real_columns = array(); + foreach ($columns as $column_name) { + $real_columns[] = _field_sql_storage_columnname($field['field_name'], $column_name); + } + db_add_index($ret, $table, $real_name, $real_columns); + db_add_index($ret, $revision_table, $real_name, $real_columns); + } + } + } +} + +/** * Implement hook_field_storage_delete_field(). */ function field_sql_storage_field_storage_delete_field($field_name) { Index: modules/field/modules/field_sql_storage/field_sql_storage.test =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/field_sql_storage/field_sql_storage.test,v retrieving revision 1.7 diff -u -F^[fc] -r1.7 field_sql_storage.test --- modules/field/modules/field_sql_storage/field_sql_storage.test 22 Aug 2009 00:58:53 -0000 1.7 +++ modules/field/modules/field_sql_storage/field_sql_storage.test 20 Sep 2009 20:42:54 -0000 @@ -22,7 +22,7 @@ class FieldSqlStorageTestCase extends Dr } function setUp() { - parent::setUp('field_sql_storage', 'field', 'field_test'); + parent::setUp('field_sql_storage', 'field', 'field_test', 'number'); $this->field_name = drupal_strtolower($this->randomName() . '_field_name'); $this->field = array('field_name' => $this->field_name, 'type' => 'test_field', 'cardinality' => 4); $this->field = field_create_field($this->field); @@ -294,4 +294,65 @@ class FieldSqlStorageTestCase extends Dr ->fetchField(); $this->assertEqual($count, 1, 'NULL field translation is wiped.'); } + + + /** + * Test adding and removing indexes while data is present. + */ + function testFieldUpdateIndexesWithData() { + // We do not have a db-agnostic inspection system in core yet, so + // for now we can only test this on mysql. + if (Database::getConnection()->databaseType() == 'mysql') { + // Create a decimal field. + $field_name = 'testfield'; + $field = array('field_name' => $field_name, 'type' => 'text'); + $field = field_create_field($field); + $instance = array('field_name' => $field_name, 'bundle' => FIELD_TEST_BUNDLE); + $instance = field_create_instance($instance); + $tables = array(_field_sql_storage_tablename($field), _field_sql_storage_revision_tablename($field)); + + // Verify the indexes we will create do not exist yet. + foreach ($tables as $table) { + $indexes = $this->getIndexes($table); + $this->assertTrue(empty($indexes['value']), t("No index named value exists in $table")); + $this->assertTrue(empty($indexes['value_format']), t("No index named value_format exists in $table")); + } + + // Add data so the table cannot be dropped. + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + $entity->{$field_name}[FIELD_LANGUAGE_NONE][0]['value'] = 'field data'; + field_attach_insert('test_entity', $entity); + + // Add an index + $field = array('field_name' => $field_name, 'indexes' => array('value' => array('value'))); + field_update_field($field); + foreach ($tables as $table) { + $indexes = $this->getIndexes($table); + $this->assertTrue($indexes["{$field_name}_value"] == array(1 => "{$field_name}_value"), t("Index on value created in $table")); + } + + // Add a different index, removing the existing custom one. + $field = array('field_name' => $field_name, 'indexes' => array('value_format' => array('value', 'format'))); + field_update_field($field); + foreach ($tables as $table) { + $indexes = $this->getIndexes($table); + $this->assertTrue($indexes["{$field_name}_value_format"] == array(1 => "{$field_name}_value", 2 => "{$field_name}_format"), t("Index on value_format created in $table")); + $this->assertTrue(empty($indexes["{$field_name}_value"]), t("Index on value removed in $table")); + } + + // Verify that the tables were not dropped. + $entity = field_test_create_stub_entity(0, 0, $instance['bundle']); + field_attach_load('test_entity', array(0 => $entity)); + $this->assertEqual($entity->{$field_name}[FIELD_LANGUAGE_NONE][0]['value'], 'field data', t("Index changes performed without dropping the tables")); + } + } + + function getIndexes($table) { + $indexes = array(); + $result = db_query("SHOW INDEXES FROM {" . $table . "}"); + foreach ($result as $row) { + $indexes[$row->key_name][$row->seq_in_index] = $row->column_name; + } + return $indexes; + } } Index: modules/field/modules/list/list.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/list/list.module,v retrieving revision 1.14 diff -u -F^[fc] -r1.14 list.module --- modules/field/modules/list/list.module 9 Sep 2009 21:21:54 -0000 1.14 +++ modules/field/modules/list/list.module 20 Sep 2009 20:42:54 -0000 @@ -85,8 +85,14 @@ function list_field_schema($field) { /** * Implement hook_field_settings_form(). + * + * TODO: If $has_data, add a form validate function to verify that the + * new allowed values do not exclude any keys for which data already + * exists in the databae (use field_attach_query()) to find out. + * Implement the validate function via hook_field_update_forbid() so + * list.module does not depend on form submission. */ -function list_field_settings_form($field, $instance) { +function list_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form['allowed_values'] = array( Index: modules/field/modules/number/number.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/number/number.module,v retrieving revision 1.18 diff -u -F^[fc] -r1.18 number.module --- modules/field/modules/number/number.module 11 Sep 2009 00:06:53 -0000 1.18 +++ modules/field/modules/number/number.module 20 Sep 2009 20:42:54 -0000 @@ -96,7 +96,7 @@ function number_field_schema($field) { /** * Implement hook_field_settings_form(). */ -function number_field_settings_form($field, $instance) { +function number_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form = array(); @@ -107,6 +107,7 @@ function number_field_settings_form($fie '#options' => drupal_map_assoc(range(10, 32)), '#default_value' => $settings['precision'], '#description' => t('The total number of digits to store in the database, including those to the right of the decimal.'), + '#disabled' => $has_data, ); $form['scale'] = array( '#type' => 'select', @@ -114,6 +115,7 @@ function number_field_settings_form($fie '#options' => drupal_map_assoc(range(0, 10)), '#default_value' => $settings['scale'], '#description' => t('The number of digits to the right of the decimal.'), + '#disabled' => $has_data, ); $form['decimal'] = array( '#type' => 'select', Index: modules/field/modules/text/text.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field/modules/text/text.module,v retrieving revision 1.28 diff -u -F^[fc] -r1.28 text.module --- modules/field/modules/text/text.module 11 Sep 2009 13:30:49 -0000 1.28 +++ modules/field/modules/text/text.module 20 Sep 2009 20:42:54 -0000 @@ -116,7 +116,7 @@ function text_field_schema($field) { /** * Implement hook_field_settings_form(). */ -function text_field_settings_form($field, $instance) { +function text_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form['max_length'] = array( @@ -126,6 +126,9 @@ function text_field_settings_form($field '#required' => FALSE, '#description' => t('The maximum length of the field in characters. Leave blank for an unlimited size.'), '#element_validate' => array('_element_validate_integer_positive'), + // TODO: If $has_data, add a validate handler that only allows + // max_length to increase. + '#disabled' => $has_data, ); return $form; Index: modules/field_ui/field_ui.admin.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/field_ui/field_ui.admin.inc,v retrieving revision 1.15 diff -u -F^[fc] -r1.15 field_ui.admin.inc --- modules/field_ui/field_ui.admin.inc 18 Sep 2009 00:12:46 -0000 1.15 +++ modules/field_ui/field_ui.admin.inc 20 Sep 2009 20:42:54 -0000 @@ -781,14 +781,6 @@ function field_ui_existing_field_options } /** - * Helper function to determine if a field has data in the database. - */ -function field_ui_field_has_data($field) { - $results = field_attach_query($field['id'], array(), 1); - return !empty($results); -} - -/** * Menu callback; presents the field settings edit page. */ function field_ui_field_settings_form($form, &$form_state, $obj_type, $bundle, $instance) { @@ -821,7 +813,7 @@ function field_ui_field_settings_form($f // See if data already exists for this field. // If so, prevent changes to the field settings. - $has_data = field_ui_field_has_data($field); + $has_data = field_attach_field_has_data($field); if ($has_data) { $form['field']['#description'] = '
' . t('There is data for this field in the database. The field settings can no longer be changed.' . '
') . $form['field']['#description']; } @@ -841,27 +833,19 @@ function field_ui_field_settings_form($f '#description' => t("Translatable fields can have a different value for each available language. An example of a translatable field is an article's body. Language neutral fields will retain the same value across all translations. An example of a language neutral field is a user profile's first name."), ); - // Add settings provided by the field module. + // Add settings provided by the field module. The field module is + // responsible for not returning settings that cannot be changed if + // the field already has data. $form['field']['settings'] = array(); - $additions = module_invoke($field_type['module'], 'field_settings_form', $field, $instance); + $additions = module_invoke($field_type['module'], 'field_settings_form', $field, $instance, $has_data); if (is_array($additions)) { $form['field']['settings'] = $additions; - // @todo Filter this so only the settings that cannot be changed are shown - // in this form. For now, treating all settings as changeable, which means - // they show up here and in edit form. } if (empty($form['field']['settings'])) { $form['field']['settings'] = array( '#markup' => t('%field has no field settings.', array('%field' => $instance['label'])), ); } - else { - foreach ($form['field']['settings'] as $key => $setting) { - if (substr($key, 0, 1) != '#') { - $form['field']['settings'][$key]['#disabled'] = $has_data; - } - } - } $form['#bundle'] = $bundle; $form['submit'] = array('#type' => 'submit', '#value' => t('Save field settings')); @@ -878,20 +862,22 @@ function field_ui_field_settings_form_su // Merge incoming form values into the existing field. $field = field_info_field($field_values['field_name']); - // Do not allow changes to fields with data. - if (field_ui_field_has_data($field)) { - return; - } - $bundle = $form['#bundle']; $instance = field_info_instance($field['field_name'], $bundle); // Update the field. $field = array_merge($field, $field_values); - field_ui_update_field($field); - drupal_set_message(t('Updated field %label field settings.', array('%label' => $instance['label']))); - $form_state['redirect'] = field_ui_next_destination($bundle); + try { + field_update_field($field); + drupal_set_message(t('Updated field %label field settings.', array('%label' => $instance['label']))); + $form_state['redirect'] = field_ui_next_destination($bundle); + } + catch (FieldException $e) { + drupal_set_message(t('Attempt to update field %label failed: %message.', array('%label' => $instance['label'], '%message' => $e->getMessage())), 'error'); + // TODO: Where do we go from here? + $form_state['redirect'] = field_ui_next_destination($bundle); + } } /** @@ -1128,7 +1114,13 @@ function field_ui_field_edit_form($form, $description .= $info['description'] . '

'; $form['#prefix'] = '
' . $description . '
'; - $description = '

' . t('These settings apply to the %field field everywhere it is used.', array('%field' => $instance['label'])) . '

'; + $has_data = field_attach_field_has_data($field); + if ($has_data) { + $description = '

' . t('These settings apply to the %field field everywhere it is used. Because the field already has data, some settings can no longer be changed.', array('%field' => $instance['label'])) . '

'; + } + else { + $description = '

' . t('These settings apply to the %field field everywhere it is used.', array('%field' => $instance['label'])) . '

'; + } // Create a form structure for the field values. $form['field'] = array( @@ -1151,10 +1143,11 @@ function field_ui_field_edit_form($form, '#description' => $description, ); - // Add additional field settings from the field module. - $additions = module_invoke($field['module'], 'field_settings_form', $field, $instance); + // Add additional field settings from the field module. The field module is + // responsible for not returning settings that cannot be changed if + // the field already has data. + $additions = module_invoke($field['module'], 'field_settings_form', $field, $instance, $has_data); if (is_array($additions)) { - // @todo Filter additional settings by whether they can be changed. $form['field']['settings'] = $additions; } @@ -1289,7 +1282,7 @@ function field_ui_field_edit_form_submit // Update any field settings that have changed. $field = field_info_field($instance_values['field_name']); $field = array_merge($field, $field_values); - field_ui_update_field($field); + field_update_field($field); // Move the default value from the sample widget to the default value field. if (isset($instance_values['default_value_widget'])) { Index: modules/field_ui/field_ui.api.php =================================================================== RCS file: /cvs/drupal/drupal/modules/field_ui/field_ui.api.php,v retrieving revision 1.2 diff -u -F^[fc] -r1.2 field_ui.api.php --- modules/field_ui/field_ui.api.php 18 Sep 2009 00:12:46 -0000 1.2 +++ modules/field_ui/field_ui.api.php 20 Sep 2009 20:42:54 -0000 @@ -14,14 +14,29 @@ /** * Field settings form. * + * The field type module is responsible for not returning form + * elements that cannot be changed. It may not always be possible to + * tell in advance, but modules should attempt to inform the user what + * is going/likely to work. + * + * TODO: Only the field type module knows which settings will affect + * the field's schema, but only the field storage module knows what + * schema changes are permitted once a field already has + * data. Probably we need an easy way for a field type module to ask + * whether an update to a new schema will be allowed without having to + * build up a fake $prior_field structure for + * hook_field_update_forbid(). + * * @param $field * The field structure being configured. * @param $instance * The instance structure being configured. + * @param $has_data + * Whether the field already has data. * @return * The form definition for the field settings. */ -function hook_field_settings_form($field, $instance) { +function hook_field_settings_form($field, $instance, $has_data) { $settings = $field['settings']; $form['max_length'] = array( '#type' => 'textfield', Index: modules/field_ui/field_ui.module =================================================================== RCS file: /cvs/drupal/drupal/modules/field_ui/field_ui.module,v retrieving revision 1.7 diff -u -F^[fc] -r1.7 field_ui.module --- modules/field_ui/field_ui.module 29 Aug 2009 04:42:56 -0000 1.7 +++ modules/field_ui/field_ui.module 20 Sep 2009 20:42:54 -0000 @@ -242,33 +242,6 @@ function field_ui_field_ui_build_modes_t } /** - * Updates a field. - * - * Field API does not allow field updates, so we create a method here to - * update a field if no data is created yet. - * - * @see field_create_field() - */ -function field_ui_update_field($field) { - $field_types = field_info_field_types(); - $module = $field_types[$field['type']]['module']; - - // If needed, remove the 'bundles' element added by field_info_field. - unset($field['bundles']); - - $defaults = field_info_field_settings($field['type']); - $field['settings'] = array_merge($defaults, (array) $field['settings']); - $data = $field; - unset($data['id'], $data['columns'], $data['field_name'], $data['type'], $data['locked'], $data['module'], $data['cardinality'], $data['active'], $data['deleted']); - $field['data'] = $data; - - drupal_write_record('field_config', $field, array('field_name')); - - // Clear caches. - field_cache_clear(TRUE); -} - -/** * Implement hook_field_attach_create_bundle(). */ function field_ui_field_attach_create_bundle($bundle) { Index: modules/file/file.field.inc =================================================================== RCS file: /cvs/drupal/drupal/modules/file/file.field.inc,v retrieving revision 1.3 diff -u -F^[fc] -r1.3 file.field.inc --- modules/file/file.field.inc 10 Sep 2009 22:31:58 -0000 1.3 +++ modules/file/file.field.inc 20 Sep 2009 20:42:54 -0000 @@ -67,7 +67,7 @@ function file_field_schema($field) { /** * Implement hook_field_settings_form(). */ -function file_field_settings_form($field, $instance) { +function file_field_settings_form($field, $instance, $has_data) { $defaults = field_info_field_settings($field['type']); $settings = array_merge($defaults, $field['settings']); @@ -98,6 +98,7 @@ function file_field_settings_form($field '#options' => $scheme_options, '#default_value' => $settings['uri_scheme'], '#description' => t('Select where the final files should be stored. Private file storage has significantly more overhead than public files, but allows restricted access to files within this field.'), + '#disabled' => $has_data, ); $form['default_file'] = array( Index: modules/simpletest/tests/field_test.module =================================================================== RCS file: /cvs/drupal/drupal/modules/simpletest/tests/field_test.module,v retrieving revision 1.23 diff -u -F^[fc] -r1.23 field_test.module --- modules/simpletest/tests/field_test.module 18 Sep 2009 00:12:47 -0000 1.23 +++ modules/simpletest/tests/field_test.module 20 Sep 2009 20:42:54 -0000 @@ -349,6 +349,8 @@ function field_test_field_info() { 'label' => t('Test Field'), 'settings' => array( 'test_field_setting' => 'dummy test string', + 'changeable' => 'a changeable field setting', + 'unchangeable' => 'an unchangeable field setting', ), 'instance_settings' => array( 'test_instance_setting' => 'dummy test string', @@ -361,6 +363,15 @@ function field_test_field_info() { } /** + * Implement hook_field_update_forbid(). + */ +function field_test_field_update_forbid($field, $prior_field, $has_data) { + if ($field['type'] == 'test_field' && $field['settings']['unchangeable'] != $prior_field['settings']['unchangeable']) { + throw new FieldException("field_test 'unchangeable' setting cannot be changed'"); + } +} + +/** * Implement hook_field_schema(). */ function field_test_field_schema($field) { Index: modules/taxonomy/taxonomy.module =================================================================== RCS file: /cvs/drupal/drupal/modules/taxonomy/taxonomy.module,v retrieving revision 1.511 diff -u -F^[fc] -r1.511 taxonomy.module --- modules/taxonomy/taxonomy.module 19 Sep 2009 11:07:36 -0000 1.511 +++ modules/taxonomy/taxonomy.module 20 Sep 2009 20:42:54 -0000 @@ -2138,7 +2138,7 @@ function taxonomy_element_info() { /** * Implement hook_field_settings_form(). */ -function taxonomy_field_settings_form($field, $instance) { +function taxonomy_field_settings_form($field, $instance, $has_data) { // Get proper values for 'allowed_values_function', which is a core setting. $vocabularies = taxonomy_get_vocabularies(); $options = array(); @@ -2157,6 +2157,7 @@ function taxonomy_field_settings_form($f '#options' => $options, '#required' => TRUE, '#description' => t('The vocabulary which supplies the options for this field.'), + '#disabled' => $has_data, ); $form['allowed_values'][$delta]['parent'] = array( '#type' => 'value',