Fatal errors in Date Repeat
c.lam - June 23, 2008 - 03:26
| Project: | Date |
| Version: | 6.x-2.x-dev |
| Component: | Date Repeat API |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | patch (code needs review) |
Description
I am using the date_repeat_rrule field in a form in a module I'm writing. Upon form submission, the following occurs:
Fatal error: Cannot unset string offsets in date_api_ical.inc on line 579
When the 3 unset() are commented out, an if() following after causes this:
Fatal error: Cannot use string offset as an array in date_api_ical.inc on line 592
Timezone is -7 GMT (PDT) Vancouver, Canada. Using all defaults.
This seems to be caused by date_repeat_rrule_validate() assuming that the form values are in a tree, trying to access the non-existent key 'rrule' in its parent field, and then passing this empty variable to date_api_ical_build_rrule().
In my case, changing line 232 of date_repeat_form.inc to $item = $form_values; solves the problem.
#1
I made a patch that should account for either setting of #tree. (so, not just a 1-line change like my original solution)
It also fixes another problem I encountered, where upon form submission,
_date_repeat_rrule_process()runsdate_api_ical_build_rrule($element['#value']), butdate_repeat_merge()doesn't perform the necessary changes to ensure thatdate_api_ical_build_rrule($element['#value'])gets all the values it expects, causing errors. This situation only occurs when the form that is using the date_repeat_rrule field fails its own validation, causing the form to be redisplayed.Please review to make sure I'm on the right track. Thanks!
#2
This patch is the same as above, but with a fix for a typo in repeat exceptions, and a fix to keep EXDATEs together in their own array (right now, each EXDATE entry is just floating around in $form_state['values']).
#3
I applied this patch to the latest release of Date (6.x-2.0-beta4) and it resolved the problem I had with the Bookings API.
Since c.lam maintains Bookings API, I'm assuming that's where he sees the problem as well. Are there any other modules are using date_repeat that I could test the patch with?
#4
The Date module uses it, so we need to be sure it doesn't break that. The Event Repeat module may implement it in the future, but hasn't yet. We definitely need to be sure it will work correctly as an API. My only question is why the 'date' index isn't set to begin with, if the merge isn't creating the right results I want to fix that.
Other than that there are some minor Drupal coding style issues -- we put the 'else' part of a if/else on a separate line and all indents are exactly two spaces. And I like to avoid abbreviated variables like $v -- it makes maintenance harder - make it a complete descriptive name like $value.
I'll do some testing later today.