Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Date module select lists and text fields are required even if they are hidden (their controlling value is not selected).
I believe the problem stems from how a Date field is specified as required:
Results of print_r($form['field_birthdate']):
Array
(
[#tree] => 1
[#weight] => 0
[#validate] => Array
(
[date_widget_validate] => Array
(
)
)
[0] => Array
(
[#type] => date_combo
[#field] => Array
(
[field_name] => field_birth_date
[type] => date
[required] => 1
...snip...
With this there are two issues:
- Conditional Fields looks for '#required', but not 'required'
- Conditional Fields uses element_children() and so does not look at #field property children.
Should the Date module should change to represent required in a normal way? Or should Conditional Fields accommodate the Date module?
Comment | File | Size | Author |
---|---|---|---|
#29 | conditional_fields-reset-default-special-268560-29.patch | 3.85 KB | scottrigby |
#27 | conditional_fields-reset-default-special-268560-27.patch | 3.85 KB | scottrigby |
#17 | conditional-fields-date-7.patch | 8.68 KB | cdale |
#16 | conditional-fields-date-6.patch | 8.51 KB | cdale |
#15 | conditional-fields-date-5.patch | 12.78 KB | cdale |
Comments
Comment #1
threexk CreditAttribution: threexk commentedAfter further research, the Date module does use the #required property for all its form elements. However, it is done by a #process function, which is called after hook_form_alter() where Conditional Fields looks for #required.
At the hook_form_alter() stage when Conditional Fields runs, a date field is sort of a super field that can consist of multiple normal fields; at the #process stage it expands the super field into all its constituent fields.
I found this diagram really helpful for visualizing the sequence: http://drupal.org/node/165104
I am now wondering if Conditional Fields's hook_form_alter() code should instead be executed at the #afterbuild stage, so it deals with the final, processed form (hook_form_alter() could register a #afterbuild callback.) Will need to think about it more...
Comment #2
threexk CreditAttribution: threexk commentedI hereby offer a $5 bounty to anyone who can fix this issue within two months. Payment would be via PayPal.
The money probably is not commensurate with the work, but perhaps it will provide some extra motivation to someone who was already thinking about fixing it.
Comment #3
peterpoe CreditAttribution: peterpoe commentedPostponing all compatibility with non-core cck modules to future releases.
Comment #4
mitchell CreditAttribution: mitchell commentedPlease try the latest releases of both modules and update this issue.
Comment #5
peterpoe CreditAttribution: peterpoe commentedPlease report here any incompatibility with date module.
Please note that work on compatibility issues with non-core cck modules is postponed after the release of Conditional Fields 1.0 (but patches are welcome of course).
Comment #6
GuyPaddock CreditAttribution: GuyPaddock commentedI'm still seeing this behavior with the Date and Location modules. Both field types have nested fields with their own #required attributes, but
conditional_fields_node_editing_form
doesn't seem to recurse.I should note that the date and location fields are being used within field groups, and it is the field group that has the conditional field setting (i.e. when another field has the right value, the field group becomes visible).
Comment #7
akolahi CreditAttribution: akolahi commentedUsing Conditional Fields 6.1-dev (Aug 15 2009) and Date 6-2.3 Initial testing is showing that i am able to set a date field as a conditional field that is properly triggered. I tried saving the field triggered and not triggered and so far is appearing to work properly.
Will test further and will report of any strange behavior.
Comment #8
akolahi CreditAttribution: akolahi commentedIf the Date field is required I am getting an error message saying the date field is required even though the date field is not triggered and thus is unavailable.
If the date field is not required then it looks like it is working properly - at least thus far in my testing...
Comment #9
akolahi CreditAttribution: akolahi commentedI'm willing to make a financial contribution to getting the this issue fixed and the date module compatible with conditional fields. Let me know if your interested.
Comment #10
cdale CreditAttribution: cdale commentedI've done some work on this. I have not extensively test this, so please use with care. I think it should for the most part work however.
Comment #11
cdale CreditAttribution: cdale commentedSmall change
Comment #12
cdale CreditAttribution: cdale commentedMissed the returns in the after build. They now return the form, instead of nothing.
Comment #13
cdale CreditAttribution: cdale commentedLast patch has a typo on both _form_validate() lines which causes the required checking to not work. #conditional_date_fields should be #conditional_controlled_fields.
Updated patch attached.
Comment #14
cdale CreditAttribution: cdale commentedI've done a bit more testing, and I think this is working quite well now with the last patch. It doesn't seem to affect anything else, and appears to make date fields work as expected.
The patch itself could probably be re-factored a little bit to reduce duplicate code and checking of conditions, but I don't quite have the time to do that at the moment.
Comment #15
cdale CreditAttribution: cdale commentedSorry for so many unanswered posts and renditions of this I believe this one is the right patch now. :) The last one would work, this one just cleans up a variable that was made which was never used.
PATCH SUMMARY
After build callback (Was form alter).
Validation callback
Having written that, I'm not 100% sure on the logic behind needing to re-validate the date fields manually. I know as I was putting this patch together there was a reason for it, but now that it's finished, I might need to recheck if that is actually required.
Comment #16
cdale CreditAttribution: cdale commentedOk, so I just checked what I wrote above, and it turns out I was right, the logic is wrong. Here is what I hopefully think will be the last patch. :)
SUMMARY
Comment #17
cdale CreditAttribution: cdale commentedRealized that XOR logic was a bit miffed.
Corrected.
Comment #18
peterpoe CreditAttribution: peterpoe commented@cdale: in 2.x-dev branch I used your suggestion of moving alteration to after_build. Please use the 6.x-2.x-dev branch to test compatibility fixes.
The alteration function has been completely rewritten for greater maintainability and to prepare the code for new features.
To add compatibility with other modules, like your work with Date module, we can't just add the code in the main function, otherwise the module would become in short a messy nightmare, with exceptions for every widget and so on.
We must think about a plugin system so every non-core cck widget has its own include file and can be maintained separately.
Do do this we must identify the things that conditional fields needs to know to guarantee compatibility. My ideas are still not clear on the subject, so contributions are welcome!
Comment #19
cdale CreditAttribution: cdale commentedThe only piece of code in that patch specific to the date module was the default value setting. Normally, the CCK callback checking should work, however the date module appears to do its checking in its #process callback.
The main problems I faced when adding this compatibility with the date module were the following:
I can't think of any other challenges that were faced, but perhaps this gives you a starting point? I might get a chance to look at a plugin system, perhaps using ctools or something similar.
What do you think?
Comment #20
djdevin@cdale: we had the same issue with date fields being required even though they were hidden by a controlled field - your patch worked perfectly. thanks!
patched against conditional fields 6.x-1.1, using date 6.x-2.4
Comment #21
thekevinday CreditAttribution: thekevinday commentedThe date fields do not completely work properly under 2.x.
Specifically, the "required fields" do not work properly when the required field is disabled by a conditional field.
When trying to save, when a date field is required but disabled, then all fields after that field fail to be saved properly and end up with having their values set to NULL or an empty string.
If I have 3 fields:
1) a true/false field (controlling #2)
2) a date field (controlled by #1)
3) a text field
When #1 and #2 are in two different groups:
G1:
- #1
G2:
- #2
G3:
- #3
During the form submit process if #1 is false (and #2 requires #1 to be true) and #3 has "Example Text" in its field, then on save #3 will be empty because #2 had some sort of error for being required but unfilled.
Comment #22
YK85 CreditAttribution: YK85 commentedsubscribing - has anyone been able to further develop compatibility with date module? Thank you!
Comment #23
chalee CreditAttribution: chalee commentedI have this problem too
Comment #24
threexk CreditAttribution: threexk commentedThis seems to be fixed in the current 6.x-2.0-beta2. Anyone disagree? I no longer have a problem with a controlled Date field being required when it is not triggered.
Comment #26
timlie CreditAttribution: timlie commentedWhen I have a date field and the controlling field doesn't trigger this date field, the value which is previously saved for this date field stays in the database...
Comment #27
scottrigbyThis is a rather old issue, but still a problem on D6 sites - normally this would be fixed first upstream (7x) then backported to 6x. But i'm not sure this is even an issue in 7.x-3.x?
In any case here's a patch against 6.x-2.x, which - as @peterpoe requested in #18 - adds a new function that calls a simple plugin system to help keep things tidy.
The code is self-documenting, but in short, it starts by adding a new function which:
Date support is added in an include, which implements WIDGET_MODULE_conditional_fields_reset_default() - that function is basically what @cdale initially wrote, just isolated and easy to now support additional non-core CCK widget modules.
This passed testing on our end, but would be good to get further reviews and feedback :)
Comment #28
scottrigbyComment #29
scottrigbyNew updated patch.
Shouldn't include field widget module plugin files more than once.
Comment #30
peterpoe CreditAttribution: peterpoe as a volunteer commented