Compatibilty with date module

threexk - June 9, 2008 - 18:28
Project:Conditional Fields
Version:6.x-1.x-dev
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:needs review
Description

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:

  1. Conditional Fields looks for '#required', but not 'required'
  2. 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?

#1

threexk - June 9, 2008 - 20:08

After 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...

#2

threexk - January 10, 2009 - 17:16

I 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.

#3

peterpoe - January 24, 2009 - 14:45
Status:active» postponed

Postponing all compatibility with non-core cck modules to future releases.

#4

opensanta - May 25, 2009 - 19:04
Version:5.x-1.x-dev» 6.x-1.x-dev
Status:postponed» postponed (maintainer needs more info)

Please try the latest releases of both modules and update this issue.

#5

peterpoe - June 9, 2009 - 16:38
Title:Date module fields required even if hidden» Compatibilty with date module
Category:bug report» feature request
Status:postponed (maintainer needs more info)» postponed

Please 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).

#6

GuyPaddock - June 27, 2009 - 04:38

I'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).

#7

akolahi - September 7, 2009 - 08:01

Using 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.

#8

akolahi - September 7, 2009 - 08:32

If 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...

#9

akolahi - September 7, 2009 - 08:40

I'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.

#10

cdale - November 27, 2009 - 01:00
Status:postponed» needs review

I'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.

AttachmentSize
conditional-fields-date.patch 12.32 KB

#11

cdale - November 27, 2009 - 01:08

Small change

AttachmentSize
conditional-fields-date-2.patch 12.37 KB

#12

cdale - November 27, 2009 - 01:42

Missed the returns in the after build. They now return the form, instead of nothing.

AttachmentSize
conditional-fields-date-3.patch 12.7 KB

#13

cdale - November 28, 2009 - 07:08

Last 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.

AttachmentSize
conditional-fields-date-4.patch 12.72 KB

#14

cdale - November 28, 2009 - 07:11

I'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.

#15

cdale - November 28, 2009 - 07:29

Sorry 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

  • Changed the node editing form alter to be run in an #after_build. This should catch any modules which expand their fields in a #process callback.

After build callback (Was form alter).

  • Adds a copy and a reference of each controlled field in the afterbuild for use in validation callbacks. (See below)
  • If the controlled field type is 'date', then set the field to be validated right away in the afterbuild, and check the validation later ourselves. (This avoids duplicate validation errors being output).

Validation callback

  • If the controlled field is not triggered, use CCK's default value callbacks to get the default values. If it is a date field, then do some extra work to handle the default value, and finally set it not only in form state, but also in #value on the elements so that if there is a validation error, the form is rebuilt correctly. (This was the reason for the by ref copying).
  • If the controlled field type is a date, then validate the field now (This was the reason for the copied value above, along with the by reference). We have to do this, as part of the date validation routines use #value on the elements to check, so we had to update it before validation could go on.
  • Finally, check the required property on date now that it's value has been reset

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.

AttachmentSize
conditional-fields-date-5.patch 12.78 KB

#16

cdale - November 28, 2009 - 07:38

Ok, 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

  • Form alter made into an after build
  • In after build, a reference to each controlled element is placed in $form
  • In validation, if the field is not triggered, CCK Default value gathering is now used
  • If the field is a date type, then we use a custom process to get the default value
  • We set the default value in $form_state AND on the element copy in form. We have to do this as if there is a validation error, then #value on the element is used to rebuild the form, not $form_state
AttachmentSize
conditional-fields-date-6.patch 8.51 KB

#17

cdale - November 28, 2009 - 07:59

Realized that XOR logic was a bit miffed.

Corrected.

AttachmentSize
conditional-fields-date-7.patch 8.68 KB
 
 

Drupal is a registered trademark of Dries Buytaert.