Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that START < DUE if both set #351

Closed
korelstar opened this issue Aug 22, 2016 · 16 comments
Closed

Ensure that START < DUE if both set #351

korelstar opened this issue Aug 22, 2016 · 16 comments

Comments

@korelstar
Copy link
Contributor

RFC 5545 says about the DUE field:

For cases where this property
is specified in a "VTODO" calendar component that also specifies a
"DTSTART" property, [...] the value of this property
MUST be later in time than the value of the "DTSTART" property.

Currently, it is possible to create invalid tasks, because the tasks app doesn't validate this. Third-party applications using calendar data from ownCloud could produce errors due to such invalid tasks (e.g. DAVdroid, a calDAV synchronization app for android, does). This should be prevented by at least showing a warning if the user sets DUE < START. Better: enforce a valid task by correcting the dates.

@raimund-schluessler
Copy link
Contributor

We should prevent setting an earlier DUE field completely and adjust the DUE property if the DTSTART value is changed.

@korelstar
Copy link
Contributor Author

Yes, that would be nice. But to what?

  1. to DTSTART+1 (since it "MUST be later") or to
  2. to DUE - DTSTART_OLD + DTSTART_NEW?

For 1.: what is +1? 1 second? that doesn't seem to be good. 1 day? Is that the user intension?
For 2.: this fixes the duration of the task. So how to change the duration? By changing DUE? Here we have the same problem and we have to enforce the constraint, too. So this is an not-so-easy UI-design question.

@raimund-schluessler
Copy link
Contributor

The calendar app uses option 2 exactly as you described it. It keeps the duration constant, if you change DTSTART, however it seems like it does not check the constraint yet: owncloud/calendar#245

I would go with option 2 and keep the duration of the task fixed, but modify it a bit: only adjust the date in case the constraint is broken. So setting an invalid DUE date shifts DTSTART to an earlier time, keeping duration as it is and setting an invalid DTSTART shifts DUE to a later time keeping duration constant. Otherwise do nothing to the other property. Does this sound reasonable?

@korelstar
Copy link
Contributor Author

I like your proposed approach.It solves the problem elegantly and I think it does not confuse the users.

@korelstar
Copy link
Contributor Author

I'm currently working on an implementation of this. However, I've found a special case, which I would like to discuss here.

Let's take a task with DTSTART=10.11.2016 10:00 and DUE=10.11.2016 16:00. Now, we activate all day on this. Currently, the task then has DTSTART=10.11.2016 and DUE=10.11.2016, which means that DTSTART=DUE. However, as stated above, the RFC tells us the constraint DUE>DTSTART.

What to do? I could set DTSTART -= 1day or DUE += 1day in such a case. Both variants are not totally sufficient. However, I would chose the first one, since I would not change DUE which is quite an important date.

What do you think? @raimund-schluessler

@raimund-schluessler
Copy link
Contributor

I would rather change the beginning, although I wonder if we should enforce this constraint also for all-day dates. Thunderbird does not even enforce this for date-time dates. Maybe @jancborchardt has an opinion too?

@korelstar
Copy link
Contributor Author

Okay, I checked this again, and it seems that all implementations I'm using do allow DUE=START -- although this violates the standard. Therefore, I'm fine with allowing this for the tasks app, too.

@raimund-schluessler
Copy link
Contributor

So, we set START <= DUE instead of START < DUE?

@korelstar
Copy link
Contributor Author

Yes, I think that would be okay. Any doubts?

@korelstar
Copy link
Contributor Author

You can test it already: https://github.com/nextcloud/tasks/tree/start-due-constraint
Will make a pull request later.

@raimund-schluessler
Copy link
Contributor

Yes, I think that would be okay. Any doubts?

No, I am fine with that.

@jancborchardt
Copy link
Member

@raimund-schluessler @korelstar let’s also maybe continue discussion in the new repo – we can reopen the issue there or just do it on the pull request. Otherwise it’s confusing. :)

@rfc2822
Copy link

rfc2822 commented Nov 14, 2016

@korelstar Please note that DAVdroid will now ignore DTSTART when it's not "later in time" than DUE (when DTSTART == DUE), too to avoid compatibility problems, and those occur when the standard is violated. For instance, a CalDAV server might refuse a task with DTSTART == DUE, and of course DAVdroid would be blamed in this case.

I would strongly suggest to validate DTSTART > DUE, because RFC 5545 says:

For cases where this property is specified in a "VTODO" calendar component that also specifies a
"DTSTART" property, the value type of this property MUST be the same as the "DTSTART" property, and the value of this property MUST be later in time than the value of the "DTSTART" property.

There's a discussion about this topic at http://www.ietf.org/mail-archive/web/calsify/current/msg02217.html, which is linked from an erratum with status "held for document update": erratum 4626, which suggests adding this paragraph to the "New Restrictions" section:

The value of the "DUE" property MUST be strictly later in time than the value of the "DTSTART" property (as opposed to only "equal or later in time").

I really don't have an opinion on what makes sense in this case and what doesn't (it does't matter for me, personally), but to minimize compatibility problems, I'd stick with the standard.

@korelstar
Copy link
Contributor Author

@rfc2822 Thanks for your hints! This changes my opinion about this issue. Although I personally would like to see that START=DUE is allowed, I think that compatibility is more important.

@raimund-schluessler I will change the PR nextcloud/tasks#11 and enforce START<DUE instead of START<=DUE -- I think that's "alternativlos" 😉

@jancborchardt I really dislike to disrupt the discussion by splitting it in two parts. But if everybody is fine with enforcing START<DUE , we can discuss the details in nextcloud/tasks#11

@raimund-schluessler
Copy link
Contributor

@rfc2822 Thanks for the hints.

Although I personally would like to see that START=DUE is allowed, I think that compatibility is more important.

@korelstar I agree with that. It would be nonsense to implement it in a way, that the compatibility is broken. Although I too would like START=DUE I guess this won't change anymore (the discussion linked by @rfc2822 is seven years old). However, thanks for chaning the PR 😄

@korelstar
Copy link
Contributor Author

@rfc2822 Do you have already any experiences from DAVdroid in practice since disallowing START=DUE? I just remembered a note by @dmfs, who has a special case for Apple user in his popular app OpenTasks: dmfs/opentasks#249 (comment)

[...] Apple clients [...] set START==DUE if a due date is set

Hence, the question is, if your changes in DAVdroid brake Apple compatibility. I don't have any Apple devices, so I can't examine this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants