add backend support for cck date fields

sounds great. i hope someone has time to write a patch for it. ;)

+1 for this feature

A different approach. If you look at how the event module works it allows you to specify content types that are events. It generally handles the adding of the date fields which is what it normally uses to produce the calendar. One could take the same approach for reminders, you could add another flag to say which content type works as reminders and add fields as needed to the approriate content forms.

it is possible that 'scheduled actions' module could be cajoled into servicing this reminder need.

Has anyone done anything on this as of recent? I am considering the same thing. There's now also the date module. What seems like the best point of entry, getting signup to send reminders or another module like scheduled actions?

cheers,
Ian

tracking

+1 (and a Mars bar) for anyone who writes a patch for CCK integration ;)

any mind reviewing part 1? http://drupal.org/node/146248

FYI: see killes's efforts for this over at http://drupal.org/node/154580

I just committed http://drupal.org/node/154580, so full date.module support will now be rather trivial to add. Someone just has to copy signup_event_5.x-1.inc to signup_date.inc, and re-write it accordingly. Should be quite obvious from looking at the existing file to see what needs to be done.

Any volunteers want to assign this to themselves and take a stab at a first draft?

Thanks,
-Derek

p.s. I marked http://drupal.org/node/146248 duplicate with this, since there's no longer anything to gain by keeping reminders and auto-close in separate issues.

This may not be so easy after all. I haven't actually tried this out, but just looking at the code, it seems to make an assumption that you know what the field names and date types are just by knowing that the date module is enabled. Your sql() calls have no parameters, so there's no way to know what field is being manipulated. It works for the event module because that module has fixed names for its fields and tables, but to write sql for date module fields you need to know the type of field (iso or timestamp) and you also need to know the field name so you can figure out the database column names and what table the field is stored in.

And for even more complication, there will always only be one set of event module dates on a node, but there could be several different date module dates on the same node.

I don't have time to dig into this right now, but if anyone else does, I think the key is figuring out a way to let those functions know something about what field is being evaluated so they can do the right thing.

Maybe signup_date should form_alter the field widget form so that there is a checkbox - allow signups. that would clearly identify what field holds the date for a given node, and what that field's settings are.

Thinking about this some more...

Maybe what is needed is when you set up a content type for the signup module, you can select the specific date field that should be used for the signup for that content type. Store that content type/field_name relationship somewhere -- in a variable or in a table.

Then we just need the signup module to provide a way to alter the sql by content type instead of using the same sql for all content types, and the field info can be retrieved using content_database_info($field) to write the correct sql for that content type.

Karen and I proposed the same thing at the same time, with a slightly different UI. Either UI works. It sounds like some change to signup.module will be required.

I've always thought (e.g. http://drupal.org/node/146248#comment-559264) that when enabling a content type for signups that had date fields, you had to select which field was the start time via an admin setting. I had assumed it'd be something like a drop-down of all date fields on that content type for the global content admin page (admin/content/types/[type]), since that's the page signup admins are already used to visiting to decide if they want to signup-enable any given content type. However, if folks thought it was better to have it as something you can specify when editing the field itself inside CCK, please make a convincing argument, I'm open to other approaches. However, this assumption is why the signup_*_sql() hooks don't take any arguments -- I thought the implementations would just look at some variables to know what to do, instead. The whole point is to hide the implementation details of each particular date-related backend from the main signup code as much as possible.

However, now that I think about it more closely, it gets complicated for the "signup-enable a single node of this type" use-case. :( You can't just select the "enable for signups" radio (or whatever it's called). If it's got date fields, you'd also have to select *which* date field to use as the start time. Not sure what to do about that. I see a few options:

A) Punt. Say that particular combination of uses isn't exactly supported, and that if you signup-enable a single node that has date fields, signup.module won't think of it as having a start time, and from signup's perspective, it's an untimed event. If you want timed events with date fields, use the global content type admin page.

B) Let the admin select which date field is the start time for a given node type, even if that node type isn't signup-enabled by default. So, they could still globally indicate which field to use, even if signups are only enabled on a per-node basis. This would probably be a little counter-intuative and confusing, but it'd be somewhat consistent in its own way. ;) Worst case that someone signup-enables a single node, but there's no global setting pointing to the right date field, we fall back to option (A) and consider this an untimed event.

C) If signups are globally disabled on a given node type, and the admin is trying to signup enable a single node of that type, they get to select which date field is the start time for that particular node. This seems like the most complicated for the underlying code and DB schema, and would probably be too confusing in the UI as well.

Thoughts?
-Derek

Maybe something like this patch (completely untested). It provides a way to pass the content type to the *_sql() hooks, which I think is necessary for anything to work at all.

With the name of the content type and the name of the specific date field to track in the content type, you could create a signup_date.inc file to create the right sql for that field and content type.

I think a variation on A) is best. Namely, let the site admin specify the date field for a given content type even if that type has signup disabled by default. then when a given node gets enabled, it automatically picks up the full signup functionality.

@moshe #17 -- that's exactly what option (B) was. ;)

@KarenS #16: Wow, thanks. That's quite a patch. Sadly, it seems there's some stray whitespace in signup* which is making your patch bigger than it needs to be. ;) That aside, I suppose you're right that we need something like this -- without the content types getting passed around and dealt with, we'd be screwed. I'll have to take a closer look when I have more time, but that seems like a reasonable start. One obvious thing would be to cache the results in signup_types(), since that LEFT JOIN is going to be expensive. :(

But, anyway, thanks!

Has anyone had a chance to review Karen's patch? The many groups on groups.drupal.org would appreciate your efforts.

rerolled to keep up with HEAD. still haven't had a chance to look deeper yet. anyone manage to find any funding for this yet from somewhere?

Bumping prio, too. I'd like to get this in before the 5.x-2.4 release.

subscribing

I'd like to test this patch, but I'd like to know what version of date this patch is meant for. Is it the stable 5.17 or the 5.2 (Head) release? The date module page says that there are significant api changes in the 5.2, so if it works for both I'd be pretty surprised.

It's for the stable version, not HEAD.

And it's not a finished patch. It tries to change the way that the Signup module handles things so you can pass the content type and field name to the .inc file. What is still needed is to create the .inc file for the Date module that can create the right results from the content type and field name.

BTW, 5.x-2.x-dev is from HEAD, (which you can tell from looking at the 5.x-2.x-dev release node) and that's where all this pluggable date back-end stuff is happening. ;)

Is there an update on this? Has anyone done further work on making the signup module work with date.module version 5.2?

+1
subscribing
really useful to have it fixed for date module

If KarenS or dww want to get this patch working in a stable release soon, I may be able to sponsor this.

I've got a client project I'm doing that I would use this on with a budget at my discretion.

Contact me at preston at ptone d_t com with details of what would be needed.

-Preston

subscribing

subscribing

Here's my attempt at implementing this feature, based off of the patch KarenS provided above. It has been tested with cck date fields, but not so much with the Event module (not much has changed in regards to the Event/signup integration, so it still should work).

Here's a list of improvements that still can/need to be done:

• Support for datestamp cck field types (currently only supports date cck field types)
• Ability to order by date when viewing signup list in admin (not sure if this is possible, since there's the possibility of different date fields for each node)
• Better theming of date ranges
• Better documentation

Let me know what everyone thinks, and if anyone runs into any bugs. I'm working on getting this active on a client site, so if I run into any bugs, I'll be sure to update the patch.

Thanks,
Jon Duell

Changing status to patch review

Sorry, forgot to run my patch through coder to meet Drupal coding standards. Attached is a cleaned up version.

subscribing

subscribing

subscribing

This patch is incompatible with date v2. Simply replacing all instances of date_show_date with date_format_date in signup_date.inc worked for me.

subscribing

I am getting an error when I edit a node. I have this patch enabled.

Fatal error: Call to undefined function date_iso2unix() in /homepages/21/d245084341/htdocs/chssite/sites/all/modules/signup/signup_date.inc on line 95

On line 95, I think date_iso2unix needs to be replaced with strtotime().

@will_in_wi: Are you using date v2? Currently, this patch only works with date v1, though I'll be posting an updated patch soon.

I am using date v2. It seems to work with the modifications from comments 39 and 41.

Attached is an updated patch with support for date v2, including the changes listed by will_in_wi and a few other fixes (thanks will_in_wi, didn't realize at first you made both comments 39 and 41).

I applied this (signup_date_2.x.patch - comment #44) patch for Signup module with latest Date module (rc 2 from July, 10th)
but I got a Fatal Error after I submit signup:

Fatal error: Call to undefined function signup_format_date() in /home/users/somvprahe.sk/web/www/sites/all/modules/signup/signup.module on line 1432

Igorik
http://www.somvprahe.sk

Works for me. What version of signup did you patch?

hi!
It is Signup 5.x-2.4 2008-Feb-08 (the only one 2x there is)

thanks
Igorik
http://www.somvprahe.sk

Hmm, if the code can't find signup_format_date, then the signup_date_5.x-2.inc code isn't loading for you. Can you verify that the file exists?

Hi!

I am not sutre if I had that file like you wrote - signup_date_5.x-2.inc, propably no.
I applied patch again (on new signup module)
but I got this error:

Fatal error: Cannot access empty property in /home/users/somvprahe.sk/web/www/sites/all/modules/signup/signup_date_5.x-2.inc on line 113

Igorik

hmm, it looks like the node object wasn't passed to the signup_format_date function. What were you doing when that error occurred?

I tried to submit signup.

Hi, the problem is that I have no signup_date_5.x-2.inc file in signup module. (directory)
Where can I find it?
I use module - Signup 5.x-2.4 2008-Feb-08 61.52 KB

thanks
Igorik

I looked into latest dev version of signup module - signup 5.x-2.x-dev , Last updated: June 7, 2008 - 12:08
but there is no such file too.

Igorik

The file is in the latest patch I uploaded (Comment #44)

Hi

It is possible that I am doing some mistake, but that patch you wrote I apply (a few times, for new clean downloaded module),
files in signup module are patched, but the file signup_date_5.x-2.inc is not created.

other patches I applied on various modules run correctly, so I have no idea what can be problem.

thanks
Igorik

(subscribe)

[Sorry, I was out of town for a few weeks and just now catching up on things...]

It's not obvious from the trail of comments here what needs work about #44. @duellj: Is anything actually broken that you know of, or is all of this just user-error from people trying to test your patch? ;) Can you please comment (and if appropriate, set this back to "needs review"). If something needs work, can you summarize what it is and state your intentions (if any) to fix it yourself? I don't want to spend scarce time reviewing something that's known to be broken, nor do I want to duplicate effort fixing it if you're going to fix it yourself. However, if everything is peachy in your eyes and you want me to give this another look in its current form, please let me know. Thanks!

@dww

I just tested patching HEAD of signup with the patch at #44, and everything went through fine. Not sure what the problem that igorik is having with signup_date_5.x-2.inc not being created; it looks like maybe he's not patching it against HEAD?
It'd be great if you could take a look at this in its current form. Let me know any bugs or hangups you find.

Thanks,
Jon

I used #44 and patched signup-5.x-2.x-dev and got:

Fatal error: Call to undefined function _signup_event_completed() in /var/www/sites/all/modules/signup/signup.module on line 640

This happened when submitting a signup-enabled node.

What backend (if any) are you using (either Event or CCK Date)? It looks like the include file isn't loading for you. Can you verify that signup_date_5.x-2.inc was created during the patching process?

@duellj: The backend is Date and CCK. I patched it again with #44 and sure enough the signup_date_5.x-2.inc was created. I probably had the patch in a folder other than the signup folder and now have a signup_date_5.x-2.inc somewhere.

Signing up at a node now causes a different error.

When reloading or viewing the node again, the signup button says "Cancel signup" so the member is indeed signed up.

ok, found the bug you encountered. When you don't select the cck date field for signup to use (Located in the signup settings in the Content Type admin section for your signup node type), then signup_date fails because it doesn't know which cck field to operate on.

Attached is a new patch that suppresses the errors if the cck date field isn't define for that node type. This probably isn't the most desired final solution. What shoud be? I see a couple solutions when cck Date is enabled but a cck Date field hasn't been defined for that node type:

• Show a warning message for node types that are signups and have at least one CCK date field asking the admin to define a signup CCK date field
• Automatically select the first cck Date field for the node type, maybe show a message saying that is happening
• If a node type hasn't defined a cck Date field, then regress to signup_event_none (not sure if this is possible, and might be a separate issue)

Any other ideas?

re #62: Good question. I'm definitely not in favor of automatically selecting the "first" date field -- the admin should have to intervene. Seems like you need a hybrid solution that's something like the following:

- use hook_requirements() to ensure that all node types with a CCK date field which are signup-enabled have a date field selected, and if not, generate an error for the site status report about it.

- treat signup-enabled node types that don't have a date field selected as untimed events for the purposes of signup.module.

I haven't played with the new code yet, but perhaps the UI to signup-enable a node type could also generate a warning if there are 1 or more CCK date fields and you signup-enable without selecting the date field you want to use for the signup-related start time. Basically, it'd be an early warning before you even get to the site status report error from hook_requirements().

Does that all make sense?

#62 is also broken if you use Date 5.x-1.x and run cron. There's an SQL error in the confirmation email query:

user warning: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FROM do_test_signup s INNER JOIN do_test_node n ON n.nid = s.nid LEFT JOIN do_te' at line 1 query: _signup_cron_send_reminders SELECT n.title, n.nid, s.reminder_email, s.forwarding_email, n.type, FROM do_test_signup s INNER JOIN do_test_node n ON n.nid = s.nid LEFT JOIN do_test_content_ ON do_test_content_.nid = n.nid WHERE (s.send_reminder = 1 AND ('2008-07-30 19:09:35' + INTERVAL s.reminder_days_before DAY ) > ) in ...

Tracing this through the code, it's another problem where you assume every content type has a date. A mostly empty $content_type_field gets passed into signup_reminder_sql(), which results in a NULL being stuffed into the 'fields' member of the array it returns, which results in the extra comma at the end of the FROM clause in the SQL, which is an SQL syntax error. This makes it impossible to have signups on untimed events, once you enable the date.module. This is functionality I've tried hard to keep working up till now, and I don't want to break it for this.

So, we need even more changes to this patch:

IFF a node type *has* a CCK date field, and signup.module doesn't know which one to use, that should generate a requirement error on the status report, etc.

However, if a node type doesn't have a CCK date field at all, we need to properly ignore that, and treat it basically like signup_event_none.inc would.

FYI, see also #289326: Add event backend API for knowing if a node/node type is untimed...

Here's the patch that generates a requirement error on the status report page if a content type is signup enabled but no date field is selected. It also fixes the problem with #64 with cron and Date.

The other difficulties that arise when a CCK date field and signup doesn't know which one to use should be addressed by #289326: Add event backend API for knowing if a node/node type is untimed, correct?

* I am having trouble getting this to work. I am using Drupal 5.9, CCK 5.x-1.7, and Date 5.x-2.0-rc4. I signup enabled the page content type, then added a start_date field, and then did a "Save content type". However, on the status report I continue to get this message:
"Signup: page empty
Your signup enabled content type (page) has one or more Date fields, but no Date field is set to be used for signup date integration. Please select the appropriate Event Date field on the Content Type edit form"
And when I set the start_date field in a page to a minute in the future, the signup does not auto-close when cron runs after that time. Maybe I have set something up wrong?

* Suggestion: On the admin/content/types/page form, the label "Event date field:" should be "Signup date field".
* It seems possible that someone might signup enable a node, and have a date field, but still not want to use that date field as the Signup date field. Ex: A class is doing group projects on historical events. Weird edge case, but possible, so the drop down should have a option.

Fixed the issue with cron (started using date_sql api function to account for iso and database differences). Also added in the appropriate functions patched in #289326: Add event backend API for knowing if a node/node type is untimed.

@starbow:

Integrated your two suggestions listed in #67. As far as the first issue, that's part of the reason why I wanted to automatically select the first CCK date field (see #62). The reason why it's a bit confusing is because it's not really intuitive in the workflow. Here's what currently needs happens when a signup enabled content type is created:

-User creates a new content type
-the content type is set to be signup enabled
-user saves content type
-user adds CCK date field
-user then needs to go back to the main content type edit form, select the appropriate CCK date field, and save it again

It seems like most people will miss the last step, which is why we put the requirements message in there. But it still seems like it's not enough. Any suggestions?

thinking-out-loud...

Can we form_alter the 'add field' workflow and inject a checkbox or something on date fields about "use this field as the signup start time"? Might be too much of a can of worms.

At the very least, we should be able to add a drupal_set_message() whenever a date field is saved on a node type where signups are allowed or enabled that don't have a signup date field, with a link to the page and text something like "[content_type_name] allows users to signup but you have not selected a signup date field. Choosing a signup date field is required for [link to help text]certain features[/link] to work. You can configure this on the [link-to-admin/content/types/[foo]] page."

Oh, and for the record (starbow and I discussed this in person, but I wanted to leave it here for all to see)...

- I think it's important that we don't assume the "first" date field is the signup date field. The admin should consciously decide what field will be used for autoclose and reminder emails, since that's sort of a big deal.

- We also need to support node types that have dates that don't want to use signup date-driven stuff.

- I think it's reasonable to have warnings to nag admins that have signup-enabled node types which don't have a signup date field selected. We should remind people in the common case they need to do something else, even if it means we're nagging the people in the edge case who know what they're doing and are choosing the weird thing. We could consider a checkbox to hide the warning, if we wanted to be kind to the edge case folks, but I don't think that's essential.

I agree that the date field should not be automatically selected, I was just trying to think of ways to make the signup process easier for admins (or at least more intuitive). I'll look more into either altering the "add date field" form or setting a message after a date field is created (maybe both?).

Right, and I appreciate everyone's eye for usability with this stuff. This module is already way more complicated that it probably should be, and it's only getting worse. ;)

p.s. @duellj ever use IRC? If so, what's your nick? I'm dww in that world, too, and although I haven't been in #drupal for a while, I'm there now if you want to discuss in real time...

I'm fairly new to IRC, but on the advice of add1sun I'm starting to spend more time there. My nick is duellj as well.

#68 and #69 - adding the signup selection to the CCK field edit form is fine -- that's how the fieldgroup module does its group selection element. And that seems like it would be the most intuitive method of doing this. The logic for that element can test if there's already another datefield selected on that content type and if so add some text like 'The xxx field is currently selected as the signup field for this content type, do you want to choose this field instead?'.

That would take care of a lot of the problems.

And that form could default to 'yes' for the first date field it encounters if no other date field is already selected, which makes it semi-automatic but still controllable.

[edit] It should only do this when the field is first created. After that we wouldn't want to override a conscious decision not to use a signup.

One more thought -- you could display informational messages on the date field edit form about the status of the signup. Displaying them there might be as useful or even more useful that using hook_requirements since you'd be right where you need to be to do anything about it:

'You have enabled this content type for signup but have not selected a date field',

'You have more than one date field in this content type and have chosen field xxx for signup.'

'Signup is not enabled for this content type. Click here to enable it.'

Cool, thanks KarenS -- those are all great ideas.

I happen to know duellj will be offline until Tuesday, so if anyone else is inspired to incorporate any of this into the patch before then, it won't be duplication of effort. ;) Just reply here and assign it to yourself so others know it's in progress. Thanks.

Has anyone tackled integrating signup messages into cck date edit forms? If not, I've got some time to throw this together.

I integrate it, it works fine.
date and time of event is properly shown in mail (confirmation mail about signup)

What doesn't work is sending notify mail x days before event start, no that mail is send.

Igorik
http://www.somvprahe.sk

@igorik

Do you mean you integrated what was discussed in #74,#75, and #76 by KarenS? If so, could you post a patch?

Here's a patch that adds support for selecting the signup event date field from within the cck edit form (as discussed in #74, #75, and #76).

[edit] It should only do this when the field is first created. After that we wouldn't want to override a conscious decision not to use a signup.

Anybody know how to check if a cck form is on Add mode vs. Edit mode? This patch currently doesn't support only selecting when the field is first created.

I also fixed the problem with the mail notification and autoclosing.

Whoops, forgot to include the new files in the patch. Don't use the #81 patch, use this one instead.

Anybody know how to check if a cck form is on Add mode vs. Edit mode?

Hmm... in the form itself we do it by checking isset(), but you're not using a regular field here, it's a variable, and there's no way to know whether the variable was deliberately unset or was never set. So maybe that's not possible.

Maybe it's reasonable to assume that if signup is turned on for a content type they want *some* date field selected. If so, this patch would work OK. And we're on the edit form here so they can turn it off if they want.

Oops, another thing -- there's no test in the form_alter that we're on a date field, we want to test that $field['type'] = 'date' or 'datestamp' and just exit if it's not one of them.

I didn't see a message about signup being turned off and how to turn it on, do we want that to show up on any date field that does not have signup? [Edit] The message is there, it wasn't in the place I was looking for it.

@KarenS: We want to be able to support having a CCK node type that has the following properties:
- Signup-enabled
- Contains 1 or more date fields
- None of those date fields are the signup start time (for auto-closing and reminder emails)

For the purpose of signup, this would be an untimed event, it just happens to have date field(s). An example use case for this (seemingly crazy) configuration would be something like this: you've got a site where students can claim topics for a given class project. Each topic is some historical event with a date field. You signup-enable "class topic" nodes, and set the signup limit to 1. But, the date field has nothing to do with when signups should be closed or reminders emails. If it automatically said "you've got a date field, that must be the signup start time", all the topics would be closed on the first cron run after they were created.

Make sense?

Fixed #84.

@dww: So as it is now, this could be done by explicitly unchecking the Signup box on each of the date fields added to the CCK node type. The only problem is that they would have to uncheck the Signup box for the date field every time they edit it, since it is checked by default if no date fields have been selected. So the conflict is having someone add a date field to a signup-enabled CCK node and accidentally not selecting it for use with Signup; or having someone add a date field and not wanting it to be used with Signup and accidentally not unchecking it.

I suppose we could add an option in the edit form for the Content Type that sets the nodes as "Untimed", which would then dismiss all of the "warning" messages and not select any date fields automatically.

Subscribing.

Very interested in this feature and a port to d6

I haven't seen any activity on this lately. Is the patch in #86 good to use, do I patch it against the 5.x-2.x-dev version or the 5.x-2.4 version?

Thanks,
Shane

I tried to apply the patch in #86 to both the dev and 2.4 versions and both returned the following:

Do I need to apply other patches first, or can I just go straight to patch #86?

Thanks,
Shane

patch #86 was for dev a few months ago. It looks like dev is in active development, but if there's enough need I can reroll this for the most current version of dev.

I would like to see it rerolled... What are we waiting on to actually get this added to the dev? Seems to me that we've been patching this patch for quite a while and this has been a request for a long time. Lets get it added to dev and then work against patching it for the changes. To me, it does what it should be doing...

- Shane

p.s. Great work on this module and patch for everyone that has helped!

@shane_jordan: "Lets get it added to dev and then work against patching it for the changes."

Because I'm about to release 5.x-2.5, I'm not sure I'm going to include this patch in that release yet, so I can't just commit this in an unfinished state. See #293005: Create a signup 5.x-2.5 release for more.

@duellj: If you wanted to re-roll, that'd be great, just be aware that there are a few more patches that are going to land before I can turn my attention here. So, if you want to avoid needing to reroll twice, you could just wait a few days. Either way -- I'm just letting you know. It's possible none of the remaining patches will conflict, but I can't be sure.

@all: There are still some unresolved UI issues from #83-#86. So, setting this to CNW for both the reroll and the final UI efforts before this can go in.