[Owncloud] Merge request: Editing multiple calendars

Thomas Olsen thomas at tanghus.net
Fri Dec 16 16:45:21 UTC 2011


Hi Bart

Thanks for reviewing.

On Thursday 15 December 2011 14:07 Bart Visscher wrote:
> Some remarks:
> - Use OC_JSON::checkLoggedIn() in new code instead of OC_USER::isLoggedIn &
> die for example in apps/contacts/ajax/contacts.php

Check

> - Comment in apps/contacts/ajax/activation.php: yes there is a
> OC_JSON::error() function - 

It was mostly a reminder for myself that I forgot :-P Will do some more error 
checking in ajax calls later on.

> Use OC_Contacts_App::getAddressbook($id) instead of
> OC_Contacts_Addressbook::find($id), it contains access checks.
> for example in apps/contacts/ajax/activation.php, 

Check

>  - also for OC_Contacts_VCard::find($contact)

In OC_Contacts_App::getContactObject what does the self::getAddressbook( 
$card['addressbookid'] ) call do?

> - space is not used in apps/contacts/js/interface.js

Too much copy/paste ;-)

> - don't add addressbook to function names in OC_Contacts_Addressbook, this
> is redundant and a 'bug' in the calendar app

Was trying to get the code more uniform, but I get what you mean.

> - in apps/contacts/templates/index.php first parameter of OC_Helper::linkTo
> is a app name, so 'contacts'

Cool. I still need to read 95%+ of the code...

> Other than this, looking good.

Thanks. Pushed to https://gitorious.org/owncloud/tanghus-
owncloud/trees/tanghus_remote_backup. Any idea why I couldn't do a merge 
request from there? I'm afraid I don't know how to do it from command line 
yet, other than push directly to git://gitorious.org/owncloud/owncloud.git.

> Bart
-- 
Med venlig hilsen / Best Regards

Thomas Tanghus Olsen



More information about the Owncloud mailing list