owncloud / tasks

:white_check_mark: Tasks app for ownCloud
GNU Affero General Public License v3.0
183 stars 44 forks source link

Use X- Prefix for non-standard parameters in comments #105

Closed David-Development closed 9 years ago

David-Development commented 9 years ago

I'm the developer of the TaskSync Android App and there are some issues when I'm using your comment functionality in ownCloud. The iCal4j Library is unable to parse those entrys because you haven't specified the X- Prefix for custom attributes.

Below is an example of a valid comment: COMMENT;X-ID=1;X-USERID=admin;X-DATE-TIME=20141123T110631Z:Kommentaaar! and this is what you app generates: COMMENT;ID=1;USERID=admin;DATE-TIME=20141123T110631Z:Kommentaaar!

After this small change the tasks can be synced. Is it possible to use this format in your web-app as well? Otherwise my users won't be able to sync todo's with comments properly.

raimund-schluessler commented 9 years ago

Thank you very much for this hint. I wasn't aware of this. We sure can use the X-prefix. I will create a PR asap.

Do old tasks without the prefix break your app completely, so we have to update old tasks?

Am 23.11.2014 um 13:57 schrieb David-Development notifications@github.com:

I'm the developer of the TaskSync Android App and there are some issues when I'm using your comment functionality in ownCloud. The iCal4j Library is unable to parse those entrys because you haven't specified the X- Prefix for custom attributes.

Below is an example of a valid comment: COMMENT;X-ID=1;X-USERID=admin;X-DATE-TIME=20141123T110631Z:Kommentaaar! and this is what you app generates: COMMENT;ID=1;USERID=admin;DATE-TIME=20141123T110631Z:Kommentaaar!

After this small change the tasks can be synced. Is it possible to use this format in your web-app as well? Otherwise my users won't be able to sync todo's with comments properly.

— Reply to this email directly or view it on GitHub.

David-Development commented 9 years ago

Tasks without the X- Prefix won't be parsed - so they're not visible in the ui. The other tasks without a comment will be synced. So it would be nice if the old tasks are updated - but it's not a "must have" feature. Thank you for fixing it! :)

raimund-schluessler commented 9 years ago

Please have a look at https://github.com/owncloud/tasks/commit/5a8e0a79e796bf34c70d28621a7283bdf1fe330c to check if this fixes the issue with your app.

I have to write an update script to update the old comments to the new format. Currently old comments are not shown correctly in the web app.

And can you maybe also provide a link to the documentation of the X-OC-prefix. Under http://tools.ietf.org/html/rfc5545#section-3.8.8.2 it is only specified for properties but not for their parameters.

David-Development commented 9 years ago

Awesome! Thank you for the fast fix. I'm going to test it tomorrow.

https://tools.ietf.org/html/rfc2445#section-4.8.1.4 ->Format Definition -> (";" xparam)

And the xparam stands for: "xparam =x-name "=" param-value *("," param-value)" see section https://tools.ietf.org/html/rfc2445#section-4.2 (last line)

raimund-schluessler commented 9 years ago

Alright, thank you. I used "X-OC-" as a prefix now (including a vendor extension), but I guess this should be ok too.

I will have a look at the update script, but this may take some days as I don't have to much time at the moment.

David-Development commented 9 years ago

Sorry for the delay. And yes, it's working now! Thanks for the fix :)

raimund-schluessler commented 9 years ago

@David-Development Could you please test #121 if it fixes the issue. Please escpecially test the update script if it updates old comments properly and does not break anything. Would be also good to test it for multiple users and maybe shared calendars too (And don't use it on your production environment yet). Thanks.

raimund-schluessler commented 9 years ago

@David-Development ping :)

David-Development commented 9 years ago

@raimund-schluessler Ah damn sorry for the delay! Thanks for fixing this issue. I'm going to test it within the next two days (sorry I'm currently really busy).

raimund-schluessler commented 9 years ago

No problem, take your time. It will take some time until the next release.

David-Development commented 9 years ago

@raimund-schluessler I tested the update script with a few todo's and it's working fine. Nice work! Tested also with two users and a shared calendar (Both users created comments in the shared cal/todo). Everything is working, awesome!

raimund-schluessler commented 9 years ago

Thanks for testing. I will merge it this evening into master and stable7.