Always return a non-empty result from ajax calls, if the callbacks have to do
authorJan Schneider <jan@horde.org>
Wed, 17 Feb 2010 17:56:15 +0000 (18:56 +0100)
committerJan Schneider <jan@horde.org>
Wed, 17 Feb 2010 18:06:48 +0000 (19:06 +0100)
some cleanup.
Make task ajax calls more robust.

kronolith/js/kronolith.js
kronolith/lib/Ajax/Application.php

index c94a94e..62f902b 100644 (file)
@@ -15,7 +15,7 @@ var frames = { horde_main: true },
 /* Kronolith object. */
 KronolithCore = {
     // Vars used and defaulting to null/false:
-    //   DMenu, Growler, inAjaxCallback, is_logout, onDoActionComplete,
+    //   DMenu, Growler, inAjaxCallback, is_logout,
     //   daySizes, viewLoading, freeBusy
 
     view: '',
@@ -89,7 +89,6 @@ KronolithCore = {
         }
 
         var r = request.responseJSON;
-
         if (!r.msgs) {
             r.msgs = [];
         }
@@ -108,11 +107,6 @@ KronolithCore = {
         this.server_error = 0;
 
         this.showNotifications(r.msgs);
-
-        if (this.onDoActionComplete) {
-            this.onDoActionComplete(r);
-        }
-
         this.inAjaxCallback = false;
     },
 
@@ -362,6 +356,7 @@ KronolithCore = {
                                   Object.isUndefined(r.response.events)) {
                                   return;
                               }
+                              delete this.eventsLoading['search'];
                               $H(r.response.events).each(function(calendars) {
                                   $H(calendars.value).each(function(events) {
                                       this.createAgendaDay(events.key);
@@ -1059,10 +1054,6 @@ KronolithCore = {
             $('kronolithLoading').hide();
         }
 
-        if (typeof r.response.sig == 'undefined') {
-            return;
-        }
-
         var start = this.parseDate(r.response.sig.substr(0, 8)),
             end = this.parseDate(r.response.sig.substr(8, 8)),
             dates = [start, end];
@@ -1074,6 +1065,7 @@ KronolithCore = {
             r.response.sig != this.eventsLoading[r.response.cal]) {
             return;
         }
+        delete this.eventsLoading[r.response.cal];
 
         this._insertEvents(dates, this.view, r.response.cal);
     },
@@ -1564,20 +1556,17 @@ KronolithCore = {
         }
 
         tasktypes.each(function(type) {
-            if (Object.isUndefined(this.tcache.get(type))) {
-                this._storeTasksCache($H(), type, null, true);
-            }
             tasklists.each(function(list) {
-                if (Object.isUndefined(this.tcache.get(type).get(list))) {
+                if (Object.isUndefined(this.tcache.get(type)) ||
+                    Object.isUndefined(this.tcache.get(type).get(list))) {
                     loading = true;
-                    this.startLoading('tasks:' + type + list, tasktype);
-                    this._storeTasksCache($H(), type, list, true);
+                    this.loading++;
+                    $('kronolithLoading').show();
                     this.doAction('ListTasks',
                                   { type: type,
-                                    'sig' : tasktype,
                                     list: list },
                                   function(r) {
-                                      this._loadTasksCallback(r, tasktype, true);
+                                      this._loadTasksCallback(r, true);
                                   }.bind(this));
                 }
             }, this);
@@ -1594,14 +1583,12 @@ KronolithCore = {
      * Callback method for inserting tasks in the current view.
      *
      * @param object r             The ajax response object.
-     * @param string tasktype      The (UI) task type for that the response was
-     *                             targeted.
      * @param boolean createCache  Whether to create a cache list entry for the
      *                             response, if none exists yet. Useful for
      *                             adding individual tasks to the cache without
      *                             assuming to have all tasks of the list.
      */
-    _loadTasksCallback: function(r, tasktype, createCache)
+    _loadTasksCallback: function(r, createCache)
     {
         // Hide spinner.
         this.loading--;
@@ -1610,13 +1597,22 @@ KronolithCore = {
         }
 
         this._storeTasksCache(r.response.tasks || {}, r.response.type, r.response.list, createCache);
+        if (Object.isUndefined(r.response.tasks)) {
+            return;
+        }
 
-        // Check if this is the still the result of the most current request.
+        // Check if result is still valid for the current view.
+        // There could be a rare race condition where two responses for the
+        // same task(s) arrive in the wrong order. Checking this too, like we
+        // do for events seems not worth it.
+        var tasktypes = this._getTaskStorage(this.tasktype),
+            tasklist = Kronolith.conf.calendars.tasklists['tasks/' + r.response.list];
         if (this.view != 'tasks' ||
-            this.eventsLoading['tasks:' + r.response.type + r.response.list] != r.response.sig) {
+            !tasklist || !tasklist.show ||
+            !tasktypes.include(r.response.type)) {
             return;
         }
-        this._insertTasks(tasktype, r.response.list);
+        this._insertTasks(this.tasktype, r.response.list);
     },
 
     /**
@@ -1971,9 +1967,10 @@ KronolithCore = {
             return;
         }
 
-        var tasklist = $F('kronolithTaskList'),
+        var tasklist = $F('kronolithTaskOldList'),
             taskid = $F('kronolithTaskId');
-        this.startLoading('tasks:' + ($F('kronolithTaskCompleted') ? 'complete' : 'incomplete') + tasklist, this.tasktype);
+        this.loading++;
+        $('kronolithLoading').show();
         this.doAction('SaveTask',
                       $H($('kronolithTaskForm').serialize({ hash: true }))
                           .merge({ sig: this.tasktype }),
@@ -1981,7 +1978,7 @@ KronolithCore = {
                           if (r.response.tasks && taskid) {
                               this._removeTask(taskid, tasklist);
                           }
-                          this._loadTasksCallback(r, this.tasktype, false);
+                          this._loadTasksCallback(r, false);
                           this._closeRedBox();
                           window.history.back();
                       }.bind(this));
@@ -2003,6 +2000,8 @@ KronolithCore = {
                 if (r.response.chunk) {
                     RedBox.showHtml(r.response.chunk);
                     this._editCalendar(calendar);
+                } else {
+                    this._closeRedBox();
                 }
             }.bind(this));
         }
@@ -3008,7 +3007,7 @@ KronolithCore = {
                 return;
             }
 
-            calClass = elt.retrieve('calendarclass');
+            var calClass = elt.retrieve('calendarclass');
             if (calClass) {
                 var calendar = elt.retrieve('calendar');
                 Kronolith.conf.calendars[calClass][calendar].show = !Kronolith.conf.calendars[calClass][calendar].show;
@@ -3028,32 +3027,15 @@ KronolithCore = {
                 elt.toggleClassName('kronolithCalOff');
                 switch (calClass) {
                 case 'tasklists':
-                    var tasklist = calendar.substr(6), toggle = true;
+                    var tasklist = calendar.substr(6);
                     if (this.view == 'tasks') {
-                        var tasktypes;
-                        switch (this.tasktype) {
-                        case 'all':
-                        case 'future':
-                            tasktypes = [ 'complete', 'incomplete' ];
-                            break;
-                        case 'complete':
-                        case 'incomplete':
-                            tasktypes = [ this.tasktype ];
-                            break;
+                        if (elt.hasClassName('kronolithCalOff')) {
+                            $('kronolithViewTasksBody').select('tr').findAll(function(el) {
+                                return el.retrieve('tasklist') == tasklist;
+                            }).invoke('remove');
+                        } else {
+                            this._loadTasks(this.tasktype, [ tasklist ]);
                         }
-                        tasktypes.each(function(tasktype) {
-                            if (Object.isUndefined(this.tcache.get(tasktype)) ||
-                                Object.isUndefined(this.tcache.get(tasktype).get(tasklist))) {
-                                toggle = false;
-                            }
-                        }, this);
-                    }
-                    if (toggle) {
-                        $('kronolithViewTasksBody').select('tr').findAll(function(el) {
-                            return el.retrieve('tasklist') == tasklist;
-                        }).invoke('toggle');
-                    } else {
-                        this._loadTasks(this.tasktype, [ tasklist ]);
                     }
                     // Fall through.
                 case 'remote':
index dfba1c5..99ace7c 100644 (file)
@@ -39,9 +39,13 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
         if (!($kronolith_driver = $this->_getDriver($this->_vars->cal))) {
             return $result;
         }
-        $events = $kronolith_driver->listEvents($start, $end, true, false, true);
-        if (count($events)) {
-            $result->events = $events;
+        try {
+            $events = $kronolith_driver->listEvents($start, $end, true, false, true);
+            if (count($events)) {
+                $result->events = $events;
+            }
+        } catch (Exception $e) {
+            $GLOBALS['notification']->push($e, 'horde.error');
         }
         return $result;
     }
@@ -51,20 +55,22 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function GetEvent()
     {
+        $result = new stdClass;
+
         if (!($kronolith_driver = $this->_getDriver($this->_vars->cal)) ||
             !isset($this->_vars->id)) {
-            return false;
+            return $result;
         }
 
-        $event = $kronolith_driver->getEvent($this->_vars->id, $this->_vars->date);
-        if (!$event) {
+        try {
+            $event = $kronolith_driver->getEvent($this->_vars->id, $this->_vars->date);
+            $result->event = $event->toJson(null, true, $GLOBALS['prefs']->getValue('twentyFour') ? 'H:i' : 'h:i A');
+        } catch (Horde_Exception_NotFound $e) {
             $GLOBALS['notification']->push(_("The requested event was not found."), 'horde.error');
-            return false;
+        } catch (Exception $e) {
+            $GLOBALS['notification']->push($e, 'horde.error');
         }
 
-        $result = new stdClass;
-        $result->event = $event->toJson(null, true, $GLOBALS['prefs']->getValue('twentyFour') ? 'H:i' : 'h:i A');
-
         return $result;
     }
 
@@ -73,17 +79,19 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function SaveEvent()
     {
-        if (!($kronolith_driver = $this->_getDriver($vars->targetcalendar))) {
-            return false;
+        $result = $this->_signedResponse($this->_vars->cal);
+
+        if (!($kronolith_driver = $this->_getDriver($this->_vars->targetcalendar))) {
+            return $result;
         }
 
         $event = $kronolith_driver->getEvent($this->_vars->event);
         if (!$event) {
             $GLOBALS['notification']->push(_("The requested event was not found."), 'horde.error');
-            return false;
+            return $result;
         } elseif (!$event->hasPermission(Horde_Perms::EDIT)) {
             $notification->push(_("You do not have permission to edit this event."), 'horde.warning');
-            return false;
+            return $result;
         }
 
         $event->readForm();
@@ -105,7 +113,7 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             return $this->_saveEvent($event);
         } catch (Horde_Exception $e) {
             $GLOBALS['notification']->push($e);
-            return false;
+            return $this->_signedResponse($this->_vars->cal);
         }
     }
 
@@ -114,18 +122,25 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function UpdateEvent()
     {
-        if (!($kronolith_driver = $this->_getDriver($vars->cal)) ||
-            !isset($vars->id)) {
-            return false;
+        $result = $this->_signedResponse($this->_vars->cal);
+
+        if (!($kronolith_driver = $this->_getDriver($this->_vars->cal)) ||
+            !isset($this->_vars->id)) {
+            return $result;
         }
 
-        $event = $kronolith_driver->getEvent($vars->id);
+        try {
+            $event = $kronolith_driver->getEvent($this->_vars->id);
+        } catch (Exception $e) {
+            $GLOBALS['notification']->push($e, 'horde.error');
+            return $result;
+        }
         if (!$event) {
             $GLOBALS['notification']->push(_("The requested event was not found."), 'horde.error');
-            return false;
+            return $result;
         } elseif (!$event->hasPermission(Horde_Perms::EDIT)) {
             $GLOBALS['notification']->push(_("You do not have permission to edit this event."), 'horde.warning');
-            return false;
+            return $result;
         }
 
         $attributes = Horde_Serialize::unserialize($this->_vars->att, Horde_Serialize::JSON);
@@ -165,6 +180,7 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             }
         }
 
+        // @todo: What about iTip notifications?
         return $this->_saveEvent($event);
     }
 
@@ -173,29 +189,31 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function DeleteEvent()
     {
+        $result = new stdClass;
+
         if (!($kronolith_driver = $this->_getDriver($this->_vars->cal)) ||
             !isset($this->_vars->id)) {
-            return false;
-        }
-
-        $event = $kronolith_driver->getEvent($this->_vars->id);
-        if (!$event) {
-            $GLOBALS['notification']->push(_("The requested event was not found."), 'horde.error');
-            return false;
-        } elseif (!$event->hasPermission(Horde_Perms::DELETE)) {
-            $GLOBALS['notification']->push(_("You do not have permission to delete this event."), 'horde.warning');
-            return false;
+            return $result;
         }
 
-        $deleted = $kronolith_driver->deleteEvent($event->id);
+        try {
+            $event = $kronolith_driver->getEvent($this->_vars->id);
+            if (!$event->hasPermission(Horde_Perms::DELETE)) {
+                $GLOBALS['notification']->push(_("You do not have permission to delete this event."), 'horde.warning');
+                return $result;
+            }
 
-        if ($this->_vars->sendupdates) {
-            Kronolith::sendITipNotifications($event, $GLOBALS['notification'], Kronolith::ITIP_CANCEL);
+            $deleted = $kronolith_driver->deleteEvent($event->id);
+            if ($this->_vars->sendupdates) {
+                Kronolith::sendITipNotifications($event, $GLOBALS['notification'], Kronolith::ITIP_CANCEL);
+            }
+            $result->deleted = true;
+        } catch (Horde_Exception_NotFound $e) {
+            $GLOBALS['notification']->push(_("The requested event was not found."), 'horde.error');
+        } catch (Exception $e) {
+            $GLOBALS['notification']->push($e, 'horde.error');
         }
 
-        $result = new stdClass;
-        $result->deleted = true;
-
         return $result;
     }
 
@@ -250,7 +268,6 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
         $result = new stdClass;
         $result->list = $this->_vars->list;
         $result->type = $this->_vars->type;
-        $result->sig = $this->_vars->sig;
 
         try {
             $tasks = $GLOBALS['registry']->tasks->listTasks(null, null, null, $this->_vars->list, $this->_vars->type == 'incomplete' ? 'future_incomplete' : $this->_vars->type, true);
@@ -275,20 +292,18 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             return false;
         }
 
+        $result = new stdClass;
         try {
             $task = $GLOBALS['registry']->tasks->getTask($this->_vars->list, $this->_vars->id);
-            if (!$task) {
+            if ($task) {
+                $result->task = $task->toJson(true, $GLOBALS['prefs']->getValue('twentyFour') ? 'H:i' : 'h:i A');
+            } else {
                 $GLOBALS['notification']->push(_("The requested task was not found."), 'horde.error');
-                return false;
             }
         } catch (Exception $e) {
             $GLOBALS['notification']->push($e, 'horde.error');
-            return false;
         }
 
-        $result = new stdClass;
-        $result->task = $task->toJson(true, $GLOBALS['prefs']->getValue('twentyFour') ? 'H:i' : 'h:i A');
-
         return $result;
     }
 
@@ -336,31 +351,22 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             ? $task['alarm']['value'] * $task['alarm']['unit']
             : 0;
 
+        $result = new stdClass;
         try {
-            $result = ($id && $list)
+            $ids = ($id && $list)
                 ? $GLOBALS['registry']->tasks->updateTask($list, $id, $task)
                 : $GLOBALS['registry']->tasks->addTask($task);
-        } catch (Exception $e) {
-            $GLOBALS['notification']->push($e, 'horde.error');
-            return false;
-        }
-
-        if (!$id) {
-            $id = $result[0];
-        }
-        try {
+            if (!$id) {
+                $id = $ids[0];
+            }
             $task = $GLOBALS['registry']->tasks->getTask($task['tasklist'], $id);
+            $result->tasks = array($id => $task->toJson(false, $GLOBALS['prefs']->getValue('twentyFour') ? 'H:i' : 'h:i A'));
+            $result->type = $task->completed ? 'complete' : 'incomplete';
+            $result->list = $task->tasklist;
         } catch (Exception $e) {
             $GLOBALS['notification']->push($e, 'horde.error');
-            return false;
         }
 
-        $result = new stdClass;
-        $result->type = $task->completed ? 'complete' : 'incomplete';
-        $result->list = $task->tasklist;
-        $result->sig = $vars->sig;
-        $result->tasks = array($id => $task->toJson(false, $GLOBALS['prefs']->getValue('twentyFour') ? 'H:i' : 'h:i A'));
-
         return $result;
     }
 
@@ -369,22 +375,21 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function DeleteTask()
     {
+        $result = new stdClass;
+
         if (!$GLOBALS['registry']->hasMethod('tasks/deleteTask') ||
             !isset($this->_vars->id) ||
             !isset($this->_vars->list)) {
-            return false;
+            return $result;
         }
 
         try {
             $GLOBALS['registry']->tasks->deleteTask($this->_vars->list, $this->_vars->id);
+            $result->deleted = true;
         } catch (Exception $e) {
             $GLOBALS['notification']->push($e, 'horde.error');
-            return false;
         }
 
-        $result = new stdClass;
-        $result->deleted = true;
-
         return $result;
     }
 
@@ -393,20 +398,19 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function ToggleCompletion()
     {
+        $result = new stdClass;
+
         if (!$GLOBALS['registry']->hasMethod('tasks/toggleCompletion')) {
-            return false;
+            return $result;
         }
 
         try {
             $GLOBALS['registry']->tasks->toggleCompletion($this->_vars->id, $this->_vars->list);
+            $result->toggled = true;
         } catch (Exception $e) {
             $GLOBALS['notification']->push($e, 'horde.error');
-            return false;
         }
 
-        $result = new stdClass;
-        $result->toggled = true;
-
         return $result;
     }
 
@@ -430,14 +434,12 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
      */
     public function GetFreeBusy()
     {
+        $result = new stdClass;
         try {
-            $fb = Kronolith_FreeBusy::get($this->_vars->email, true);
+            $result->fb = Kronolith_FreeBusy::get($this->_vars->email, true);
         } catch (Exception $e) {
             $GLOBALS['notification']->push($e->getMessage(), 'horde.warning');
-            return false;
         }
-        $result = new stdClass;
-        $result->fb = $fb;
         return $result;
     }
 
@@ -470,13 +472,13 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             if (!$calendar_id) {
                 if (!Horde_Auth::getAuth() ||
                     $GLOBALS['prefs']->isLocked('default_share')) {
-                    return false;
+                    return $result;
                 }
                 try {
                     $calendar = Kronolith::addShare($info);
                 } catch (Exception $e) {
                     $GLOBALS['notification']->push($e, 'horde.error');
-                    return false;
+                    return $result;
                 }
                 $GLOBALS['notification']->push(sprintf(_("The calendar \"%s\" has been created."), $info['name']), 'horde.success');
                 $result->calendar = $calendar->getName();
@@ -488,14 +490,14 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
                 $calendar = $GLOBALS['kronolith_shares']->getShare($calendar_id);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push($e, 'horde.error');
-                return false;
+                return $result;
             }
             $original_name = $calendar->get('name');
             try {
                 Kronolith::updateShare($calendar, $info);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push($e, 'horde.error');
-                return false;
+                return $result;
 
             }
             if ($calendar->get('name') != $original_name) {
@@ -515,13 +517,13 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             if (!$calendar_id) {
                 if (!Horde_Auth::getAuth() ||
                     $GLOBALS['prefs']->isLocked('default_share')) {
-                    return false;
+                    return $result;
                 }
                 try {
                     $tasklist = $GLOBALS['registry']->tasks->addTasklist($calendar['name'], $calendar['description'], $calendar['color']);
                 } catch (Exception $e) {
                     $GLOBALS['notification']->push($e, 'horde.error');
-                    return false;
+                    return $result;
                 }
                 $GLOBALS['notification']->push(sprintf(_("The task list \"%s\" has been created."), $calendar['name']), 'horde.success');
                 $result->calendar = $tasklist;
@@ -533,13 +535,13 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             $tasklists = $GLOBALS['registry']->tasks->listTasklists(true, Horde_Perms::EDIT);
             if (!isset($tasklists[$calendar_id])) {
                 $GLOBALS['notification']->push(_("You are not allowed to change this task list."), 'horde.error');
-                return false;
+                return $result;
             }
             try {
                 $GLOBALS['registry']->tasks->updateTasklist($calendar_id, $calendar);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push($e, 'horde.error');
-                return false;
+                return $result;
             }
             if ($tasklists[$calendar_id]->get('name') != $calendar['name']) {
                 $GLOBALS['notification']->push(sprintf(_("The task list \"%s\" has been renamed to \"%s\"."), $tasklists[$calendar_id]->get('name'), $calendar['name']), 'horde.success');
@@ -557,7 +559,7 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
                 Kronolith::subscribeRemoteCalendar($calendar);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push($e, 'horde.error');
-                return false;
+                return $result;
             }
             if ($calendar_id) {
                 $GLOBALS['notification']->push(sprintf(_("The calendar \"%s\" has been saved."), $calendar['name']), 'horde.success');
@@ -579,6 +581,7 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
     public function DeleteCalendar()
     {
         $calendar_id = $this->_vars->calendar;
+        $result = new stdClass;
 
         switch ($this->_vars->type) {
         case 'internal':
@@ -586,13 +589,13 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
                 $calendar = $GLOBALS['kronolith_shares']->getShare($calendar_id);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push($e, 'horde.error');
-                return false;
+                return $result;
             }
             try {
                 Kronolith::deleteShare($calendar);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push(sprintf(_("Unable to delete \"%s\": %s"), $calendar->get('name'), $e->getMessage()), 'horde.error');
-                return false;
+                return $result;
             }
             $GLOBALS['notification']->push(sprintf(_("The calendar \"%s\" has been deleted."), $calendar->get('name')), 'horde.success');
             break;
@@ -602,13 +605,13 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
             $tasklists = $GLOBALS['registry']->tasks->listTasklists(true);
             if (!isset($tasklists[$calendar_id])) {
                 $GLOBALS['notification']->push(_("You are not allowed to delete this task list."), 'horde.error');
-                return false;
+                return $result;
             }
             try {
                 $GLOBALS['registry']->tasks->deleteTasklist($calendar_id);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push(sprintf(_("Unable to delete \"%s\": %s"), $tasklists[$calendar_id]->get('name'), $e->getMessage()), 'horde.error');
-                return false;
+                return $result;
             }
             $GLOBALS['notification']->push(sprintf(_("The task list \"%s\" has been deleted."), $tasklists[$calendar_id]->get('name')), 'horde.success');
             break;
@@ -618,13 +621,12 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
                 $deleted = Kronolith::unsubscribeRemoteCalendar($calendar_id);
             } catch (Exception $e) {
                 $GLOBALS['notification']->push($e, 'horde.error');
-                return false;
+                return $result;
             }
             $GLOBALS['notification']->push(sprintf(_("You have been unsubscribed from \"%s\" (%s)."), $deleted['name'], $deleted['url']), 'horde.success');
             break;
         }
 
-        $result = new stdClass;
         $result->deleted = true;
 
         return $result;
@@ -643,27 +645,27 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
         if (!empty($GLOBALS['conf']['http']['proxy']['proxy_host'])) {
             $params['proxy'] = $GLOBALS['conf']['http']['proxy'];
         }
-        $driver = Kronolith_Driver::factory('Ical', $params);
-        $driver->open($this->_vars->url);
+
+        $result = new stdClass;
         try {
+            $driver = Kronolith_Driver::factory('Ical', $params);
+            $driver->open($this->_vars->url);
             $ical = $driver->getRemoteCalendar(false);
-        } catch (Kronolith_Exception $e) {
+            $result->success = true;
+            $name = $ical->getAttribute('X-WR-CALNAME');
+            if (!($name instanceof PEAR_Error)) {
+                $result->name = $name;
+            }
+            $desc = $ical->getAttribute('X-WR-CALDESC');
+            if (!($desc instanceof PEAR_Error)) {
+                $result->desc = $desc;
+            }
+        } catch (Exception $e) {
             if ($e->getCode() == 401) {
-                $result = new stdClass;
                 $result->auth = true;
-                return $result;
+            } else {
+                $GLOBALS['notification']->push($e, 'horde.error');
             }
-            throw $e;
-        }
-        $result = new stdClass;
-        $result->success = true;
-        $name = $ical->getAttribute('X-WR-CALNAME');
-        if (!($name instanceof PEAR_Error)) {
-            $result->name = $name;
-        }
-        $desc = $ical->getAttribute('X-WR-CALDESC');
-        if (!($desc instanceof PEAR_Error)) {
-            $result->desc = $desc;
         }
 
         return $result;
@@ -690,7 +692,11 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
 
 
     /**
-     * TODO
+     * Returns the driver object for a calendar.
+     *
+     * @param string $cal  A calendar string in the format "type|name".
+     *
+     * @return Kronolith_Driver|boolean  A driver instance or false on failure.
      */
     protected function _getDriver($cal)
     {
@@ -714,28 +720,43 @@ class Kronolith_Ajax_Application extends Horde_Ajax_Application_Base
     }
 
     /**
-     * TODO
+     * Saves an event and returns a signed result object including the saved
+     * event.
+     *
+     * @param Kronlith_Event $event  An event object.
+     *
+     * @return object  The result object.
      */
     protected function _saveEvent($event)
     {
+        $result = $this->_signedResponse($event->calendarType . '|' . $event->calendar);
         try {
-            $result = $event->save();
+            $event->save();
+            Kronolith::addEvents($events, $event, $start, $end, true, true);
+            if (count($events)) {
+                $result->events = $events;
+            }
         } catch (Exception $e) {
             $GLOBALS['notification']->push($e, 'horde.error');
-            return true;
         }
-        $start = new Horde_Date(Horde_Util::getFormData('view_start'));
-        $end   = new Horde_Date(Horde_Util::getFormData('view_end'));
-        $end->hour = 23;
-        $end->min = $end->sec = 59;
-        Kronolith::addEvents($events, $event, $start, $end, true, true);
+        return $result;
+    }
+
+    /**
+     * Creates a result object with the signature of the current request.
+     *
+     * @param string $signature  A signature.
+     *
+     * @return object  The result object.
+     */
+    protected function _signedResponse($signature)
+    {
         $result = new stdClass;
-        $result->cal = $event->calendarType . '|' . $event->calendar;
-        $result->view = Horde_Util::getFormData('view');
+        $result->cal = $signature;
+        $result->view = $this->_vars->view;
+        $start = new Horde_Date($this->_vars->view_start);
+        $end   = new Horde_Date($this->_vars->view_end);
         $result->sig = $start->dateString() . $end->dateString();
-        if (count($events)) {
-            $result->events = $events;
-        }
         return $result;
     }