So there's no way to return back fields after module enabled again, also this blocks #731724: Convert comment settings into a field to make them work with CMI and non-node entities which was converted to annotations
steps to reproduce
1) enable telephone module
2) add telephone field to article
3) disable and then enable telephone module
4) visit admin/structure/types/manage/article/fields - no telephone field!
Also admin/reports/fields
Notice: Undefined index: field_tel in Drupal\field_ui\FieldListController->buildRow() (line 115 of core/modules/field_ui/lib/Drupal/field_ui/FieldListController.php).
Warning: Invalid argument supplied for foreach() in Drupal\field_ui\FieldListController->buildRow() (line 115 of core/modules/field_ui/lib/Drupal/field_ui/FieldListController.php).
foreach ($modules as $module => $module_info) {
// Collect field types and storage backends exposed by the module.
$field_types = (array) $module_handler->invoke($module, 'field_info');
$storage_types = (array) $module_handler->invoke($module, 'field_storage_info');
if ($field_types || $storage_types) {
foreach ($fields as $uuid => &$field) {
// Associate field types.
if (isset($field_types[$field['type']]) && ($field['module'] !== $module || !$field['active'])) {
$field['module'] = $module;
$field['active'] = TRUE;
$changed[$uuid] = $field;
}
| Comment | File | Size | Author |
|---|---|---|---|
| #12 | field-sync-status-2031707.fail_.patch | 3.83 KB | larowlan |
| #12 | field-sync-status-2031707.pass_.patch | 6.47 KB | larowlan |
| #8 | interdiff.txt | 626 bytes | andypost |
| #8 | field-sync-2031707-8.patch | 4.19 KB | andypost |
| #7 | interdiff.txt | 2.17 KB | andypost |
Comments
Comment #1
andyposttaggin
Comment #2
yched commentedOops, indeed. I missed that use of hook_field_info() :-(.
This check needs to switch to using getDefinitions() and the"field type" plugin manager somehow. Although I guess if the current code is not using field_info_field_types() in HEAD, it's probably because things are a bit more complex :-/
field_sync_status(), what's not to love about you...
Comment #3
andypostInitial attempt
Comment #4
andypost@yched what is a proper place for tests?
Patch works for me
Comment #5
andypostAlso there's no way to prevent disabling of the module which fields in use
Comment #6
swentel commentedI also hit a bug while trying to remove field_read_x(s) - see the interdiff in https://drupal.org/node/2018319#comment-7540395 - it's probably not entirely related to this, but yeah, that function is annoying :)
Comment #7
andypostHere's test and new patch
Comment #8
andypostAnd better text check
Comment #9
yched commentedHard for me to provide a proper review right now, but patch looks reasonable.
Telephone module tests seem a weird place to put those tests though. It's generic field api behavior, should be tested in field module using test_field field type.
Comment #10
andypost@yched to properly test this we need module that not hidden
Comment #11
yched commentedOk, but then still put the test in field.module, not telephone.module, it's not code from telephone module that is being tested.
A comment explaining why it uses telephone instead of test_field would be welcome too :-)
Comment #12
larowlanSo this moves the test out of telephone module into field module.
First a fail, then a pass.
Comment #13
andypostGreat! I think this RTBC
Comment #14
penyaskitoRTBC +1, let's unblock #731724: Convert comment settings into a field to make them work with CMI and non-node entities
Comment #15
penyaskitoThis issue was RTBC and passing tests on July 1, the beginning of API freeze.
Comment #16
catchCommitted/pushed to 8.x, thanks!