search

dmfs / opentasks

A task app for Android
Apache License 2.0
943 stars 249 forks source link

Give more information about affected task if validation fails #366

Open dmfs opened 7 years ago

dmfs commented 7 years ago

@korelstar commented on Mon Aug 22 2016

TaskValidatorProcessor throws an IllegalArgumentException if a task does not comply with all constraints. However, sometimes it is not possible to reproduce which task is affected. Therefore, it would be nice, if the exception's message would contain some information in order to identify the affected task, i.e., it's title.

Background and motivation: if there are tasks on a server with DUE < START (yes, that is not allowed but some server do not enforce this) and these tasks should be synchronized using e.g. DAVdroid with OpenTasks. Now, the user gets the error message DUE must not be < DTSTART but is not able to find out which tasks are affected, without complicated and time-consuming debugging. Extending the exception's message with the task title would be very helpful in order to quickly correct the wrong data on the server.

See also this discussion: https://forums.bitfire.at/topic/1178/more-details-on-error-due-must-not-be-dtstart

jkufner commented 7 years ago

The exception is not very practical. Just log the corrupted data, ignore the invalid start date (keep the due date as it is more important) and let the sync continue.

The task with such change should be considered unchanged and not synced back to server until user changes it.

  1. července 2017 22:14:47 CEST, Marten Gajda notifications@github.com napsal:

    @korelstar commented on Mon Aug 22 2016

    TaskValidatorProcessor throws an IllegalArgumentException if a task does not comply with all constraints. However, sometimes it is not possible to reproduce which task is affected. Therefore, it would be nice, if the exception's message would contain some information in order to identify the affected task, i.e., it's title.

    Background and motivation: if there are tasks on a server with DUE < START (yes, that is not allowed but some server do not enforce this) and these tasks should be synchronized using e.g. DAVdroid with OpenTasks. Now, the user gets the error message DUE must not be < DTSTART but is not able to find out which tasks are affected, without complicated and time-consuming debugging. Extending the exception's message with the task title would be very helpful in order to quickly correct the wrong data on the server.

    See also this discussion: https://forums.bitfire.at/topic/1178/more-details-on-error-due-must-not-be-dtstart

    -- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/dmfs/opentasks/issues/366

dmfs commented 7 years ago

@jkufner IMO, this verification should be done by the sync app before it writes to the content provider. As far as it concerns the content provider, a task with DUE < START is just illegal and will be rejected. This issue is about giving helpful feedback about which task is broken.

jkufner commented 7 years ago

Question is whether or not to throw the exception. Slightly malformed data on input are not a big problem. If data can be fixed, they should be fixed. User should be informed about the fix, but the app should continue and deal with the situation in a useful way.

Synchronisation mechanism and content provider simply may not interpret the dates. Purpose of the sync is to sync, not to validate. CalDAV server should validate such things, but it may not validate data too. So we get to a bug in an other tasks app the user uses.

dmfs commented 7 years ago

IMO, the data should be validated whenever it's written into some sort of permanent storage, which could be the server or a database on the client. In this case the provider represents this sort of permanent storage and the sync adapter must ensure all data it stores conforms to the provider contract.

I'm not a friend of error-tolerant software because the result is software with errors. Here is an article about this topic which I fully agree with.

In some cases it is certainly more user-friendly to resolve some issues automatically if possible. But the decision to do so should be done at the highest possible level, which is the sync-adapter (or even the user) in this case. Also, a sync-adapter has more options to deal with incorrect data, like asking for user consent before applying a fix, report the issue back to the server (if the protocol supports it) or even back to the server developers (there is a draft for a server information document which might make this possible in future).