This is maybe a task rather than a bug, but in the process of getting the import/export feature working (http://drupal.org/node/99446) I found we have a number of things in the current code that don't play well with 5.x ability to allow modules to automatically execute forms. I have created a patch to fix the problems I ran into, but it probably takes some explanation as to why I made the changes I did. Most of the changes are to the fieldgroup module, so fago please be sure I haven't done anything that will create problems in that module.

1) If another module wants to create a field or a group, there is no way for it to provide a specific name for it. Instead, the program goes through the routine of finding a way to turn the label into a field or group name. This is unnecessary work if we already know that we have a valid name, and it prevents external modules from defining fields or groups and knowing for sure that the names they supply will be preserved. I altered the routine to add fields and groups to accept an optional parameter for the name. If there is a value supplied, the program runs it through a validation routine to check that it is not already in use and that it contains no illegal characters, then uses it. If no name is supplied, the current procedure is executed. That means that manual form submissions will work just as they do now, and external modules can either choose to create fields and groups with specific names, or to let CCK create a name automatically.

2) The fieldgroup module was inserting values like arg(3) into forms, which won't work when forms are executed programmatically. I added an 'action' parameter to the group creation that accepts 'add' or 'edit' as a value that can be supplied by a module or taken from the arg for manual form submissions. I got rid of the 'add_group' name for a group since the action parameter will indicate a group is being created and submit a blank value instead to conform with the changes mentioned above so that a name is created by the system.

3) I changed group_name from a parameter to a form value so I can use form_set_error() on it, so the module executing the form has a way to know if there was a problem inserting the value.

4) I needed a way for an external module to get a list of all groups, rather than a list for a specific content type. That list already was being created in fieldname_groups(), but there was no way to retrieve it, so I made the type_name optional so the function will return the whole list if no content type is provided.

5) That same function, fieldname_groups() was setting a static variable with a list of all group info, but there was no way to force it to reset itself after I added new groups. I found it was not resetting when I ran a program that created a new group and then added fields to it, which was generating 'illegal option' errors, so I added a reset parameter to the function that I could use to get that variable to re-create itself.

Anyway, since there are a number of changes, I'd like to get them reviewed before committing anything.

CommentFileSizeAuthor
#5 cck_15.patch12.12 KBkarens
#4 cck_14.patch11.49 KBkarens
#2 cck_13.patch11.05 KBkarens
#1 cck_12.patch10.8 KBkarens
cck_11.patch10.81 KBkarens

Comments

karens’s picture

StatusFileSize
new10.8 KB

Oops. Found a slight change needed to be sure the field_name gets initialized to a blank value for a manual submission.

karens’s picture

StatusFileSize
new11.05 KB

Sorry about the quick changes but there is one more item I would like to add to this patch to help programmatic functions along. I'm adding the name of the widget module as well as the field module to the field form array. This info makes it easier to check that the right modules are installed before trying to execute the field programmatically.

moshe weitzman’s picture

Status: Needs review » Needs work

a brief read of the diff:
- _content_admin_field_add_new() does not use its new field_name variable

i tested fieldgroups pretty thoroughly. I only found 1 bug - edits are not sticking. add/delete and associating fields work fine.

karens’s picture

Status: Needs work » Needs review
StatusFileSize
new11.49 KB

Thanks for the feedback moshe. I think I found the problems and re-rolled the patch.

karens’s picture

StatusFileSize
new12.12 KB

One more change. I found that hidden values need '#default_value' instead of '#value' if we want to be able to override those values later using drupal_execute, so I've added this change to the patch.

moshe weitzman’s picture

Status: Needs review » Reviewed & tested by the community

works as advertised. thanks.

karens’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

fago’s picture

yep, everything ok with fieldgroup. good work, thanks

Anonymous’s picture

Status: Fixed » Closed (fixed)