In a simple site we are using usernode for the user location and a few CCK fields (we wanted something a bit more lightweight than nodeprofile).
We made one field required, and then started having problems with new users not being able to edit their profile. Having had a poke around the problem was obviously that usernode was doing drupal_execute on the node form, which was failing silently because of the required field. This caused quite a few associated data integrity problems that got a little ugly to fix.
Here is a quick patch that unsets any #required fields in form_alter - but only when called from usernode_save_node(). This makes things work as expected, and makes it possible for usernodes to be used as I described. It uses array_walk_recursive(), which is PHP5 only, because the alternative recursive function (which I wrote, but then threw away) seemed a bit ugly and cumbersome for such a simple task. Either we could make this a PHP5 module, write it as a regular recursive function, use a compatibility function (there is one in pear compat lib), or make this a 'php5 feature' using function_exists.
The only case where this could still fail is if a custom nodeapi validation function did not accept the default blank input (i.e. it is really required, but does not rely on the #required flag). I don't know of modules like this, but it is possible.
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 171013_0.patch | 2.12 KB | owen barton |
| #1 | 171013.patch | 2.01 KB | owen barton |
| nonrequired.patch | 1.99 KB | owen barton |
Comments
Comment #1
owen barton commentedI had some issues when not checking for #required = TRUE, so this includes that check.
Comment #2
owen barton commentedThis patch includes some more specific checking of the possibilities for #required being TRUE (== TRUE was causing problems, not quite sure why).
I also removed the validate hook for the node, because location module is going to it's settings to determine if a field is required - rather than using #required.
All this could potentially be a little risky if you have code that depends on this validation, however all the validation is enabled as soon as the user edits the node themselves.
Comment #3
Anonymous (not verified) commentedI cannot reproduce this problem.
I have an usernode with additional required CCK fields.
When a user is created the usernode is created as well. The required fields are there, just empty.
Could you post a use case how this problems shows up.
Thanks, Joep
Comment #4
fago@grugnog: hm, that sounds like a hack that might introduce other problems...
so first of, let's make sure which versions are affected - I've never used usernode with required fields so I haven't tested this. So JoepH & grugnog2 what versions of usernode are you using?
Comment #5
fagoI think that in a old version of usernode, required fields worked. However it's not clear to me why, and why it stopped working.. This probably needs more investigation.
Comment #6
owen barton commentedI actually realized that 'marked' required fields (FAPI ones) actually work (as far as I can tell), in which case quite a bit of this patch is cruft. The part that is still needed was the code I added later that removes the form validation callback.
The use case for me was location module enabled on the usernode content type with a field set as required. The validation for location makes creates an error if these values are empty, and this prevents the node from saving. While this would be easy to fix in locations I think usernode should have some immunity to these kind of checks - because I can see various potential fields or nodeapi forms that throw an error if the default form value is submitted right back, even if the field itself is not actually required in the FAPI sense.
Comment #7
fagook, fine if required fields are working, so I mark this issue as fixed.
To the other problem of extra validation done by modules: I can't see what we can do here. If they don't want defaults to be saved, what should we do else? As the usernode is created automatically, there can only be default values.. so this a won't fix, or by design..
Comment #8
(not verified) commented