I've committed the initial D6 port to HEAD (#222217: Port to 6.x), but I haven't yet tested any of the CCK date field integration. Let's use this issue for any discussion, patches, etc, related to that. Thanks.

CommentFileSizeAuthor
#5 330821_signup_cck_date_fixes.testreport.txt2.36 KBstborchert
#4 330821_signup_cck_date_fixes.4.patch5.42 KBdww

Comments

geodaniel’s picture

geodaniel commented

The start date seems to be picked up fine when substituted in for %time. Tested with a date field that has both start and end times enabled.

scottrigby’s picture

scottrigby
he/him
Primary language English
Location Brooklyn, NY
commented

Only started testing this morning, so it's not exhaustive... but one thing I noticed relates to this issue...

Signup does not seem to work with Datetime, only Date option for date field.

On content-type settings screen, under Workflow settings, the 'Date field to use with signup' select box only has the options <Not specified> and <None>, if I have a 'Datetime' date field enabled for that content-type. If I select the 'Date' option for date field it seems to be ok.

dww’s picture

dww
we/he/they
commented

@scottrigby #2: That makes sense, in date 5.x-2.* there was no "datetime" field type, so the code isn't setup to handle that one...

Thanks for the testing.

dww’s picture

dww
we/he/they
commented
Assigned: Unassigned » dww
Status: Active » Needs review
StatusFileSize
new 330821_signup_cck_date_fixes.4.patch5.42 KB

I was messing with this for a while earlier today, but ran out of time before I could finish testing and post the patch. This fixes a bunch of problems with the CCK date support:

A) Fixed signup_help() for what paths it tries to do the CCK date config checking on, and makes better use of the D6 hook_help API.

B) Fixed some PHP warnings in some of the cases where we're checking the CCK date config.

C) Fixed the links in the warning messages about misconfigured date fields. Tee hee, now see duellj posted a patch for this part: #331673: Broken links in signup-enabled CCK content type warning .

D) We only try to alter the node type form to add the Signup date field selector if we're editing a node type, not when we're first adding a node type (since there aren't going to be any date fields yet to choose from).

E) Fixed some bugs that were introduced with the conversion to the new (annoying) hook_mail() API, where the %time token wasn't being properly handled.

F) Added support for "datetime" fields.

I ran into a few more PHP warnings related to the signup node form, but that belongs in another issue. Any reviews + tests of this patch (and CCK date support in general)

stborchert’s picture

stborchert
he/him
Primary language German
commented
StatusFileSize
new 330821_signup_cck_date_fixes.testreport.txt2.36 KB

Hi.
As promised yesterday I've tested the patch. Attached is my detailed test report (if anyone would like to reproduce :-)).
I couldn't find any errors or misbehaviors (and no php warnings at all) but am thinking the patch should be reviewed by others, too (sure I left out some points).

First I found the broken link in the date field warning message until I noticed I forgot to apply the patch :-}

 Stefan

dww’s picture

dww
we/he/they
commented
Status: Needs review » Fixed

Committed #4 to HEAD, and backported a few of the PHP warning fixes to DRUPAL-5--2. Thanks for testing.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.