diff options
author | Dries Buytaert <dries@buytaert.net> | 2009-09-26 15:57:39 +0000 |
---|---|---|
committer | Dries Buytaert <dries@buytaert.net> | 2009-09-26 15:57:39 +0000 |
commit | db09a6178ba423fe2ce85317afaca5c58a5b6887 (patch) | |
tree | 7a23bc57bfb65197a9ac1416d8c989b506e5e05d /modules | |
parent | dba2ebb118a25ff6ed9bcc6a59cc42c20d55ad66 (diff) | |
download | brdo-db09a6178ba423fe2ce85317afaca5c58a5b6887.tar.gz brdo-db09a6178ba423fe2ce85317afaca5c58a5b6887.tar.bz2 |
- Patch #367013 by bjaspan, KarenS | yched, Dries: add support for field updates.
Diffstat (limited to 'modules')
-rw-r--r-- | modules/field/field.api.php | 52 | ||||
-rw-r--r-- | modules/field/field.attach.inc | 15 | ||||
-rw-r--r-- | modules/field/field.crud.inc | 83 | ||||
-rw-r--r-- | modules/field/field.test | 107 | ||||
-rw-r--r-- | modules/field/modules/field_sql_storage/field_sql_storage.module | 60 | ||||
-rw-r--r-- | modules/field/modules/field_sql_storage/field_sql_storage.test | 63 | ||||
-rw-r--r-- | modules/field/modules/list/list.module | 8 | ||||
-rw-r--r-- | modules/field/modules/number/number.module | 4 | ||||
-rw-r--r-- | modules/field/modules/text/text.module | 7 | ||||
-rw-r--r-- | modules/field_ui/field_ui.admin.inc | 61 | ||||
-rw-r--r-- | modules/field_ui/field_ui.api.php | 15 | ||||
-rw-r--r-- | modules/field_ui/field_ui.module | 27 | ||||
-rw-r--r-- | modules/file/file.field.inc | 3 | ||||
-rw-r--r-- | modules/simpletest/tests/field_test.module | 13 | ||||
-rw-r--r-- | modules/taxonomy/taxonomy.module | 3 |
15 files changed, 446 insertions, 75 deletions
diff --git a/modules/field/field.api.php b/modules/field/field.api.php index 5ed330f32..6fd2cef32 100644 --- a/modules/field/field.api.php +++ b/modules/field/field.api.php @@ -665,7 +665,7 @@ function hook_field_widget_error($element, $error) { * The name of the theme hook invoked when displaying the values is derived * from formatter type names, using the pattern field_formatter_FORMATTER_NAME. * field.module takes care of exposing the corresponding theme functions - * through hook_theme(). Specifically, field.module defines the theme + * through hook_theme(). Specifically, field.module defines the theme * hook: * * @code @@ -1274,6 +1274,56 @@ function hook_field_create_instance($instance) { } /** + * Forbid a field update from occurring. + * + * Any module may forbid any update for any reason. For example, the + * field's storage module might forbid an update if it would change + * the storage schema while data for the field exists. A field type + * module might forbid an update if it would change existing data's + * semantics, or if there are external dependencies on field settings + * that cannot be updated. + * + * @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 updated. + * + * This hook is invoked just after field is updated. + * + * @param $field + * The field as it is post-update. + * @param $prior_field + * The field as it was pre-update. + * @param $has_data + * Whether any data already exists for this field. + */ +function hook_field_update_field($field, $prior_field, $has_data) { +} + +/** * Act on a field being deleted. * * This hook is invoked just after field is deleted. diff --git a/modules/field/field.attach.inc b/modules/field/field.attach.inc index 5f2229376..7571ee4fc 100644 --- a/modules/field/field.attach.inc +++ b/modules/field/field.attach.inc @@ -153,7 +153,7 @@ define('FIELD_STORAGE_INSERT', 'insert'); * An associative array of additional options, with the following keys: * - 'field_name': The name of the field whose operation should be * invoked. By default, the operation is invoked on all the fields - * in the object's bundle. NOTE: This option is not compatible with + * in the object's bundle. NOTE: This option is not compatible with * the 'deleted' option; the 'field_id' option should be used * instead. * - 'field_id': The id of the field whose operation should be @@ -1012,6 +1012,19 @@ function field_attach_query_revisions($field_id, $conditions, $count, &$cursor = } /** + * 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 diff --git a/modules/field/field.crud.inc b/modules/field/field.crud.inc index 7f316c4db..210dad136 100644 --- a/modules/field/field.crud.inc +++ b/modules/field/field.crud.inc @@ -312,6 +312,89 @@ function field_create_field($field) { return $field; } +/* + * Update a field. + * + * Any module may forbid any update for any reason. For example, the + * field's storage module might forbid an update if it would change + * the storage schema while data for the field exists. A field type + * module might forbid an update if it would change existing data's + * semantics, or if there are external dependencies on field settings + * that cannot be updated. + * + * @param $field + * A field structure. $field['field_name'] must provided; it + * identifies the field that will be updated to match this + * structure. Any other properties of the field that are not + * specified in $field will be left unchanged, so it is not + * necessary to pass in a fully populated $field structure. + * @return + * Throws a FieldException if the update cannot be performed. + * @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 non-existent 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. diff --git a/modules/field/field.test b/modules/field/field.test index b530885ad..2523c8b72 100644 --- a/modules/field/field.test +++ b/modules/field/field.test @@ -1429,13 +1429,14 @@ class FieldCrudTestCase extends FieldTestCase { 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 @@ -1726,6 +1727,108 @@ class FieldCrudTestCase extends FieldTestCase { $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. + $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); + + // Update it to a deciaml 5.3 field. + $field['settings']['scale'] = 3; + field_update_field($field); + + // Save values with 2, 3, and 4 decimal places. + $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']); + + // Verify that the updated 5.3 field rounds to 3 decimal places. + 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 { diff --git a/modules/field/modules/field_sql_storage/field_sql_storage.module b/modules/field/modules/field_sql_storage/field_sql_storage.module index ff096dfe6..97d372f3b 100644 --- a/modules/field/modules/field_sql_storage/field_sql_storage.module +++ b/modules/field/modules/field_sql_storage/field_sql_storage.module @@ -12,7 +12,7 @@ function field_sql_storage_help($path, $arg) { switch ($path) { case 'admin/help#field_sql_storage': - $output = '<p>' . t('The Field SQL Storage module stores Field API data in the database. It is the default field storage module, but other field storage modules may be available in the contributions repository.') . '</p>'; + $output = '<p>' . t('The Field SQL Storage module stores Field API data in the database. It is the default field storage module, but other field storage modules may be available in the contributions repository.') . '</p>'; return $output; } } @@ -204,6 +204,64 @@ function field_sql_storage_field_storage_create_field($field) { } /** + * 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) { diff --git a/modules/field/modules/field_sql_storage/field_sql_storage.test b/modules/field/modules/field_sql_storage/field_sql_storage.test index cb4aee045..5f8a30a42 100644 --- a/modules/field/modules/field_sql_storage/field_sql_storage.test +++ b/modules/field/modules/field_sql_storage/field_sql_storage.test @@ -22,7 +22,7 @@ class FieldSqlStorageTestCase extends DrupalWebTestCase { } function setUp() { - parent::setUp('field_sql_storage', 'field', 'field_test'); + parent::setUp('field_sql_storage', 'field', 'field_test', 'text'); $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 DrupalWebTestCase { ->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; + } } diff --git a/modules/field/modules/list/list.module b/modules/field/modules/list/list.module index 9f29b2161..119cc9fea 100644 --- a/modules/field/modules/list/list.module +++ b/modules/field/modules/list/list.module @@ -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( diff --git a/modules/field/modules/number/number.module b/modules/field/modules/number/number.module index 19fd5091f..594b0f1a5 100644 --- a/modules/field/modules/number/number.module +++ b/modules/field/modules/number/number.module @@ -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($field, $instance) { '#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($field, $instance) { '#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', diff --git a/modules/field/modules/text/text.module b/modules/field/modules/text/text.module index 978a0d33a..cc0245dee 100644 --- a/modules/field/modules/text/text.module +++ b/modules/field/modules/text/text.module @@ -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, $instance) { '#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; @@ -318,7 +321,7 @@ function theme_field_formatter_text_trimmed($element) { /** * Theme function for 'summary or trimmed' field formatter for - * text_with_summary fields. This formatter returns the summary + * text_with_summary fields. This formatter returns the summary * element of the field or, if the summary is empty, the trimmed * version of the full element of the field. */ diff --git a/modules/field_ui/field_ui.admin.inc b/modules/field_ui/field_ui.admin.inc index 9335ecc8a..ae5ce6368 100644 --- a/modules/field_ui/field_ui.admin.inc +++ b/modules/field_ui/field_ui.admin.inc @@ -781,14 +781,6 @@ function field_ui_existing_field_options($bundle) { } /** - * 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($form, &$form_state, $obj_type, $bundle, $ // 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'] = '<div class=error>' . t('There is data for this field in the database. The field settings can no longer be changed.' . '</div>') . $form['field']['#description']; } @@ -841,27 +833,19 @@ function field_ui_field_settings_form($form, &$form_state, $obj_type, $bundle, $ '#description' => t("Translatable fields can have a different value for each available language. An example of a translatable field is an article's <em>body</em>. Language neutral fields will retain the same value across all translations. An example of a language neutral field is a user profile's <em>first name</em>."), ); - // 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_submit($form, &$form_state) { // 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, &$form_state, $obj_type, $bundle, $inst $description .= $info['description'] . '</p>'; $form['#prefix'] = '<div class="description">' . $description . '</div>'; - $description = '<p>' . t('These settings apply to the %field field everywhere it is used.', array('%field' => $instance['label'])) . '</p>'; + $has_data = field_attach_field_has_data($field); + if ($has_data) { + $description = '<p>' . 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'])) . '</p>'; + } + else { + $description = '<p>' . t('These settings apply to the %field field everywhere it is used.', array('%field' => $instance['label'])) . '</p>'; + } // Create a form structure for the field values. $form['field'] = array( @@ -1151,10 +1143,11 @@ function field_ui_field_edit_form($form, &$form_state, $obj_type, $bundle, $inst '#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($form, &$form_state) { // 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'])) { diff --git a/modules/field_ui/field_ui.api.php b/modules/field_ui/field_ui.api.php index 9a938fae9..9442d94eb 100644 --- a/modules/field_ui/field_ui.api.php +++ b/modules/field_ui/field_ui.api.php @@ -14,14 +14,27 @@ /** * 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', diff --git a/modules/field_ui/field_ui.module b/modules/field_ui/field_ui.module index 0f29be6cb..c5139b496 100644 --- a/modules/field_ui/field_ui.module +++ b/modules/field_ui/field_ui.module @@ -242,33 +242,6 @@ function field_ui_field_ui_build_modes_tabs() { } /** - * 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) { diff --git a/modules/file/file.field.inc b/modules/file/file.field.inc index 3552229b2..1e6ad8d4b 100644 --- a/modules/file/file.field.inc +++ b/modules/file/file.field.inc @@ -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, $instance) { '#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( diff --git a/modules/simpletest/tests/field_test.module b/modules/simpletest/tests/field_test.module index a83d8dff3..1f21e5dc4 100644 --- a/modules/simpletest/tests/field_test.module +++ b/modules/simpletest/tests/field_test.module @@ -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) { @@ -634,7 +645,7 @@ function field_test_entity_info_translatable($obj_type = NULL, $translatable = N * * This function is a simple in-memory key-value store with the * distinction that it stores all values for a given key instead of - * just the most recently set value. field_test module hooks call + * just the most recently set value. field_test module hooks call * this function to record their arguments, keyed by hook name. The * unit tests later call this function to verify that the correct * hooks were called and were passed the correct arguments. diff --git a/modules/taxonomy/taxonomy.module b/modules/taxonomy/taxonomy.module index ee18ede4c..b8b6c2e6e 100644 --- a/modules/taxonomy/taxonomy.module +++ b/modules/taxonomy/taxonomy.module @@ -2137,7 +2137,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(); @@ -2156,6 +2156,7 @@ function taxonomy_field_settings_form($field, $instance) { '#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', |