Several weeks ago I contacted thehunmonkgroup to inquire whether any work was being done on improving the UI or if I should take a crack at it. Here is my work to date.

This patch is a first stab at bringing the repeat interface into the node creation and edit form. This will allow users to set up repeat patterns at the point of node creation and node edit. Generally speaking, this interface is meant to be the gracefully degraded version, where javascript is not an option. The only javascript that is being used are drupal’s collapsible field sets. I tried to make this patch as simple as possible given the scope of the changes: any additional changes listed below were attempts to deal with inconsistencies given the new interface, or misleading default options that I found were causing everyone (i.e. testers) confusion.

Here's a quick list of changes:

  • The repeat form is presented in a "Repeat" field set.
  • Inside that field set are the end controls field set, advanced field set and exceptions field set.
  • I've also created some additional options in the settings page that will allow site admins to determine whether each of the advanced options can be turned on or off.
  • The default option for editing repeating events has been changed to "Apply this to all future events" instead of "this occurrence only." In testing, the original default value of "this" was causing some confusion with the repeat form elements also on the page.

Thanks to my fellow tractiv.com developers for testing the new code. As of this patch, all of the functionality from the previous cvs version is available. However, there is one bug that I'm still trying to track down that happens when you change the time for all events in a repeat sequence. I believe this may have something to do with the fact that updating a node now calls the code to save a repeat pattern, as well as the code to update all nodes tied to that pattern. I have left "TODO:" comments to help track this down. This code should be considered beta until this bug is fixed.

Additional "TODO:" comments are included for areas that should probably be double-checked.

Possible next steps:

  1. General community review and revision
  2. Fix the bug described above
  3. Review validation handling to make sure it is properly using the new 4.7 forms API methods
  4. Review any additional points of usuabilty
  5. Commit to cvs
  6. Consider implementing drupal javascript controls to make the interface more contextual

The rewriting of the workflow required a lot of code to be moved around, so this patch is rather large.

CommentFileSizeAuthor
interface01.patch61.71 KBseanbfuller

Comments

hunmonk’s picture

hunmonk commented

sean,

thanks for all your hard work. i won't have time this week to check out the patch, but i'll do my best to dig into it next week.

However, there is one bug that I'm still trying to track down that happens when you change the time for all events in a repeat sequence.

the timestamp bug upon node edit is one i've dealt with several times before--most likely it's due to the fact that the timestamps in the calendar map table are GMT, which can lead to the wrong day being pulled if you don't perform some calculations beforehand. you might want to check that out. can you include detailed steps on how to reproduce it in this thread? then i can have a look as well...

this patch may take some time to get in, for several reasons:

  1. it's large
  2. there may be some coding style differences that will need to be resolved
  3. this module is particularly sensitive to bugs, and requires extensive testing to ensure that any change hasn't created problems
seanbfuller’s picture

seanbfuller commented

The bug seems to be happening whenever you update the start time of a repeating event, so it should be pretty easy to see when it is happening. On the other hand, I'm not sure if it is happening 100% of the time, so if it looks like it is working fine, try to edit it again.

Like I hinted at above, I think part of this bug has to do with the change in workflow.

The current cvs version works like this:

  1. User creates an event
  2. User saves a repeat event for that event, spawning additional events
  3. User possibly updates the event information, calling _eventrepeat_update_nodes
  4. User possibly updates the repeat schedule, calling _eventrepeat_form_submit

Bringing the repeat form into the node form makes the workflow look something more like this:

  1. User creates an event, saves the repeat data, and spawns the additional nodes, all in one action.
  2. User goes back and changes the event information and repeat information.

For this second action, I am calling _eventrepeat_form_submit (renamed to _eventrepeat_save_repeat), followed by a call to _eventrepeat_update_nodes. Both need to update the event timestamps: the first to possibly remap nodes and the second to possibly update the time of day on any siblings to this node. One possibility is that this one-two step is double adjusting the times. Hope that helps.

Sorry for the size of the patch and any differences in coding styles. I tried to keep everything simple, but redoing the workflow without loosing functionality required more changes that I initially thought were required. My apologies in advance for anything that was not necessary.

seanbfuller’s picture

seanbfuller commented

The bug mentioned above has been moved to here: http://drupal.org/node/69920

seanbfuller’s picture

seanbfuller commented
Status: Needs review » Fixed

Patch has been committed to head.

Anonymous’s picture

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