Can't delete inactive fields through the API
| Project: | Content Construction Kit (CCK) |
| Version: | 6.x-2.x-dev |
| Component: | content.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Calling content_field_instance_delete($fieldname, $typename) does not delete the field if the entry in content_node_field_instance has widget_active != 1. Is this intentional?
Looking only in content.crud.inc, I see that widget_active is set to 1 to in content_instance_default_values() if the specified widget type exists and its module is active. In my case (a small CCK CRUD API sample program), I got the widget name wrong the first time I created the field so it got created with widget_active = 0 and now I cannot delete it. This situation could also happen if a module is forcibly disabled without going through the proper API for disabling modules (e.g. by having its files deleted from the filesystem).
It seems to me that the API function content_field_instance_delete() should delete the field if at all possible, and "the widget module is not available" does not seem like it makes deleting the field impossible. But perhaps I'm wrong about that?
#1
I think the logic is : if the module isn't enabled, then it can't take action on field deletion (assuming some field types might want to, e.g. "delete images" for an imagefield). Though I don't think any current module actually does.
Karen should probably know better, she was the one who fought with the 'active' problem.
#2
This was a result of a long conversation on IRC with yched and merlinofchaos about how to handle inactive fields, especially if people try to upgrade CCK when only some of their field modules have been updated to D6 or if they don't have all of them in the CCK folder at the time they re-enable CCK (which seems a likely possibility for some less widely used fields because of the long release cycle). The conclusion was that the safest thing to do with them is nothing at all, just treat them as though they don't exist, since all kinds of unpredictable things could happen if we add those fields into the $fields array as though they were usable values. And content_crud relies on the $fields array for its information when performing various field operations.
There should be no way you'd get this kind of accidental setting if you're using the UI, only if you're using the API, so I think it makes sense that if you break something you will have to clean it up yourself.
So I'm marking it by design, unless you see a reason why you think this is going to be a problem.
#3
And there was another reason for treating them like they don't exist -- we need information about what module created them, and we don't have that information unless the module is enabled. We're storing the module name in the database now as soon as we have it, but it's not there in the D5 version so it won't get set until they enable the module in D6.
#4
The issue where this is all discussed is http://drupal.org/node/198508.
#5
I agree with this logic, but I think that it should only be enforced in the UI rather than the API.
Inactive fields don't do any harm besides cluttering the admin pages with messages, but it should be possible to remove them. Here's my use case:
- D5 to D6 upgrade.
- D5 had an obsolete field module whose data I want to keep.
- I write a custom update function to migrate the data.
- I'd like to get rid of the old fields since both their functionality and their content have been migrated.
Here's a patch that lets content_field_instance_delete() delete inactive fields. The patch only affects the API function, the UI still doesn't let users do anything with inactive fields.
#6
Re: "I write a custom update function to migrate the data."
And here you could have done this:
<?php// This updates the inactive flag of fields created with the old module.
content_notify('enable', 'oldmodule');
// This deletes all fields of the old module from the content tables.
content_notify('uninstall', 'oldmodule');
?>
And you do not need to hack into content_field_instance_delete().
#7
Just a note for anyone coming across this. In the cck update, instances where the module is missing will have an empty string for widget_module in the content_field_instance table - so you'll need to go in and fix those manually, or otherwise repair that table, prior to doing Markus' trick. Otherwise this appears to work very nicely.
#8
Re-opening this since it doesn't quite work. I'm trying to upgrade some Drupal 5 asset module CCK fields to Drupal 6 filefield/imagefield. Since asset.module isn't installed in Drupal 6 (doesn't have any kind of stable D6 release) - this stuff doesn't happen:
$module_fields = module_invoke($module, 'field_info');if ($module_fields) {
foreach ($module_fields as $name => $field_info) {
watchdog('content', 'Updating field type %type with module %module.', array('%type' => $name, '%module' => $module));
db_query("UPDATE {". content_field_tablename() ."} SET module = '%s', active = %d WHERE type = '%s'", $module, 1, $name);
}
}
$module_widgets = module_invoke($module, 'widget_info');
if ($module_widgets) {
foreach ($module_widgets as $name => $widget_info) {
watchdog('content', 'Updating widget type %type with module %module.', array('%type' => $name, '%module' => $module));
db_query("UPDATE {". content_instance_tablename() ."} SET widget_module = '%s', widget_active = %d WHERE widget_type = '%s'", $module, 1, $name);
So the fields never get set back to active when running content_notify('enable'). The only option then is to update that information manually, create 'fake' hook implementations so those results get returned, or allow for deletion of disabled fields.
#9
So we need a method to flag fields and widgets from non existing modules as enabled, I guess. But how do we know which ones?
I think you could simply update the field/instance tables manually, and then do content_notify('uninstall', $module).
#10
As in example, it could probably be done with something similar to:
<?php$modules_info = array(
'assetfield' => array(
'fields' => array('assetfield'),
'widgets' => array('assetfield'),
),
);
foreach ($modules_info as $module => $module_info) {
$affected_total = 0;
foreach ($module_info['fields'] as $field_type) {
db_query("UPDATE {". content_field_tablename() ."} SET module = '%s', active = %d WHERE type = '%s'", $module, 1, $field_type);
$affected = db_affected_rows();
if ($affected > 0) {
watchdog('content', 'Updated field type %type with module %module.', array('%type' => $field_type, '%module' => $module));
$affected_total += $affected;
}
}
foreach ($module_info['widgets'] as $widget_type) {
db_query("UPDATE {". content_instance_tablename() ."} SET widget_module = '%s', widget_active = %d WHERE widget_type = '%s'", $module, 1, $widget_type);
$affected = db_affected_rows();
if ($affected > 0) {
watchdog('content', 'Updated widget type %type with module %module.', array('%type' => $widget_type, '%module' => $module));
$affected_total += $affected;
}
}
if ($affected_total > 0) {
watchdog('content', 'Removing fields and widgets related to module %module.', array('%module' => $module));
content_notify('uninstall', $module);
}
}
?>
#11
That's what I'm doing right now and as far as I can see it's working well - bit more testing/tweaking to do.
I'm just half-thinking about a generic D5 asset -> D6 filefield update module if I ever need to do this on more than one site (which seems possible) - and on this particular site, doing a similar thing with cck_address. This is currently the only thing google shows up when searching for how to do this, so if this issue just ends documenting all the things you need to do to get it working, that'd work for me too.
#12
So, how about posting #10 (or the stuff you have done) as a book page in the CCK handbook for developers?
#13
That was my use case too, a D5 asset => D6 filefield migration. The target filefield doesn't exist in the Drupal 5 version (it's defined using the feature module), but content_notify('uninstall', 'asset_cck') only works in Drupal 5.
Updating the fields' module info and active status is indeed a solution, but it's a lof of code just to delete a field and its values.
#14
I would prefer not adding something like this as an additional API. It may depend on what you actually need to do exactly. So, I think the best we can do is document this in the CCK handbook for developers as an starting to point to resolve particular migration needs.
@catch: do you think the code I posted in #10 is good enough for adding to the CCK handbook, or do you have any suggestion based on what you had to do in your case?
#15
Markus - I think that looks fine - will try to review against the code I was using tomorrow and see how it compares. Not sure where in the CCK handbook would be best to put this.
#16
Here it is for now:
- HOW TO: Remove inactive fields.
Feel free to edit and fix and/or add more examples or notes to help understand how it works and why.
#17
Thanks Markus!
#18
Automatically closed -- issue fixed for 2 weeks with no activity.