It'd be nice if our event/date backend API had functions that told you if a given node or node type had date information or were untimed.

This would be useful for the following:
#219678: Omit 'Date/Time: [Untimed]:' in email notification to event owner
#213239: hide event specific features when module not available
and probably
#86462: add backend support for cck date fields too...

Calling this critical since it has implications for #86462...

CommentFileSizeAuthor
#6 289326_signup_isdated.patch2.54 KBdww
#4 signup-isdated.patch2.96 KBstarbow

Comments

duellj’s picture

duellj commented

Great idea. How should I integrate this into the cck date patch? Should I just integrate it over on #86462: add backend support for cck date fields and reference it here? Or should I separate it out to a separate patch (that affects all event backend includes) and post it here? Whatever is easiest for you.

dww’s picture

dww
we/he/they
commented

Since #86462 needs rerolling for a few reasons, I think my preference here would be a separate patch for this issue that adds the 2 functions to the date-backend API and implements them correctly for the existing backend include files. I'll commit that first, and then we can reroll #86462 to address its other problems, and add the correct implementations of these functions for nodes/node types with CCK date fields. In parallel, I can make use of the new functions to fix #219678 and #213239 (which shouldn't conflict with the CCK backend work). How's that sound?

duellj’s picture

duellj commented

Sounds great. This should fix some of the small problems that arise with #86482 that we've been talking about. When you post the patch I'll add the needed functions to the cck date patch, or if you already know the function names and return values you are going to use, I can do that sooner.

starbow’s picture

starbow commented
StatusFileSize
new signup-isdated.patch2.96 KB

How about this?

dww’s picture

dww
we/he/they
commented
Status: Active » Needs work

Great start. Needs work because:

A) The per-node function needs to be in the backends, not just calling the per-type function, since a CCK node type could have a date field that isn't required. In that case, we might think the node type in general has date information, but a specific node might not.

B) We need a little more thought on what the semantics of the per-type test should be for CCK nodes. Perhaps this part can be punted back to #86462: add backend support for cck date fields since of the issues I listed originally, that's the only one where we're going to need the per-type test, as opposed to the per-node test. The semantics we need will really only be clear once we're using it in that CCK patch, and the CCK backends are the only ones where there's anything funky to do -- in the event or nothing cases, the proper return values for both tests are obvious.

dww’s picture

dww
we/he/they
commented
Status: Needs work » Needs review
StatusFileSize
new 289326_signup_isdated.patch2.54 KB

This fixes (A) and a few code-style spacing problems. I haven't really thought more about (B) yet, but I think we can punt on that. This should be sufficient to unstick all the other issues originally referenced, although the patch is currently untested.

dww’s picture

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

After a review from starbow, Committed to HEAD.

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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