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

[bug] Appointments over multiple day shrink to one day #50

Closed
mitras2 opened this issue Jul 15, 2016 · 14 comments · Fixed by #64
Closed

[bug] Appointments over multiple day shrink to one day #50

mitras2 opened this issue Jul 15, 2016 · 14 comments · Fixed by #64
Assignees

Comments

@mitras2
Copy link

mitras2 commented Jul 15, 2016

This is the problem:
I exported an calendar with appointments that span over multiple days. Some of them have date+time for beginning and end. Others are define to be on from day x to y (complete day).
While the appointments with date+time stay spanned over multiple days, the other ones that are defined for the complete days are shrunk down to the first day.

Example:
LAN-Party
from 15.07.2016 10:00
to 17.07.2016 18:00

becomes:
BEGIN:VEVENT
...
SUMMARY:LAN-Party
ORGANIZER:mailto:[email protected]
LOCATION:Somewhere
STATUS:CONFIRMED
DTSTART;TZID=Europe/Berlin:20160114T100000
DTEND:20160717T180000Z
END:VEVENT

4 days of Filming
from 25.07.2016
to 28.07.2016

becomes:
BEGIN:VEVENT
...
SUMMARY:4 days of Filming
ORGANIZER:mailto:[email protected]
LOCATION:Somewhere
STATUS:CONFIRMED
DTSTART;VALUE=DATE:20160725
DTEND;VALUE=DATE:20160726
END:VEVENT

Version is 2.3
on Android 4.4.3

@mitras2
Copy link
Author

mitras2 commented Jul 15, 2016

I think the bug is in ProcessVEvent.java around line 249:
https://github.com/SufficientlySecure/calendar-import-export/blob/master/CalendarImportExport/src/main/java/org/sufficientlysecure/ical/ProcessVEvent.java#L249

The assumption that all-day-events always only span one day is wrong...

@mitras2
Copy link
Author

mitras2 commented Jul 15, 2016

OK - I have to correct my last message:
The error lies in SaveCalendar.java, line 333:
https://github.com/SufficientlySecure/calendar-import-export/blob/master/CalendarImportExport/src/main/java/org/sufficientlysecure/ical/SaveCalendar.java#L333

I forked the git-repo some night-hours ago and build a quick and dirty fix:

            String endTz = Events.EVENT_END_TIMEZONE;
            if (endTz == null) {
                endTz = Events.EVENT_TIMEZONE;
            }
            Date dateEnd = getDateTime(cur, Events.DTEND, endTz, cal);
            if(dateEnd.getTime() % 86400000 == 0){
                dtEnd = new DtEnd(utcDateFromMs(dateEnd.getTime()));
            } else {
                dtEnd = new DtEnd(utcDateFromMs(start.getTime() + DateUtils.DAY_IN_MILLIS));
            }
            l.add(dtEnd);

If I replace line 333 and 334 with that code i have a working solution for me BUT THIS MIGHT BE A BAD SOLUTION FOR OTHER CASES! So don't copy that code as a final bugfix (that's why I won't start a pull request with that code either...)

@mitras2
Copy link
Author

mitras2 commented Jul 15, 2016

After a few crashes and tests i just refined my code a bit. It's quite more to read, but it handles more (and some common) cases:

            String endTz = Events.EVENT_END_TIMEZONE;
            if (endTz == null) {
                endTz = Events.EVENT_TIMEZONE;
            }
            Date dateEnd = getDateTime(cur, Events.DTEND, endTz, cal);
            if(dateEnd == null || dateEnd.getTime() < start.getTime()+DateUtils.DAY_IN_MILLIS){
                dtEnd = new DtEnd(utcDateFromMs(start.getTime() + DateUtils.DAY_IN_MILLIS));
            } else {
                if(dateEnd.getTime() % DateUtils.DAY_IN_MILLIS == 0){
                    dtEnd = new DtEnd(utcDateFromMs(dateEnd.getTime()));
                } else {
                    Long dateEndCorrection = 0L;
                    if(dateEnd.getTime() % DateUtils.DAY_IN_MILLIS >= (DateUtils.DAY_IN_MILLIS/2L)){
                        dateEndCorrection = (DateUtils.DAY_IN_MILLIS - (dateEnd.getTime() % DateUtils.DAY_IN_MILLIS));
                    } else {
                        dateEndCorrection = (-dateEnd.getTime() % DateUtils.DAY_IN_MILLIS);
                    }
                    dtEnd = new DtEnd(utcDateFromMs(dateEnd.getTime() + dateEndCorrection));
                }
            }
            l.add(dtEnd);

PS:
It replaces the same lines - 333 + 334

PPS:
This is still no perfect solution - but it's was better than the one before. Caution: At the moment I completely ignore "DURATION"...

exit("Good night")

@jgriffiths jgriffiths self-assigned this Jul 15, 2016
@jgriffiths
Copy link
Contributor

@mitras2 thanks for the great report. I'll try to look at this this weekend, no doubt some test cases will be needed.

@doak
Copy link

doak commented Nov 10, 2016

@jgriffiths: Did you gain any progress? The nasty bug which prevents this app to be used as an stable importer/exporter still exists :(

@jgriffiths
Copy link
Contributor

Hi @doak unfortunately I have been very busy with work. I will try to take some time to look at recent reports this weekend.

@tasn
Copy link
Contributor

tasn commented Feb 8, 2017

Hey there,
Any updates on this one?
@doak, does your code work, or is it a hack? If it works, maybe consider creating a pull request?

Thanks,
Tom.

tasn added a commit to tasn/calendar-import-export that referenced this issue Feb 16, 2017
The code was creating a datetime object, but all day events should have
a date object for both the start and the end dates.

Fixes SufficientlySecure#50
tasn added a commit to tasn/calendar-import-export that referenced this issue Feb 16, 2017
If an event's start date is a date (and not datetime) we can assume the
end date would be one too, and that means it's an all day event.

The previous code was forcing the event to be a one day event although
this was completely unnecessary.

Fixes SufficientlySecure#50
@tasn tasn mentioned this issue Feb 16, 2017
@mitras2
Copy link
Author

mitras2 commented Mar 11, 2017

@tasn So is this Problem fixed now?

@tasn
Copy link
Contributor

tasn commented Mar 12, 2017

It's fixed in my pull request (#64), though not yet merged.

@doak
Copy link

doak commented Mar 20, 2017 via email

@tasn
Copy link
Contributor

tasn commented Mar 22, 2017

@doak, same here. At least it's available on Google Play for now.

@dschuermann
Copy link
Member

I pinged the F-Droid guys on IRC. App page says "build failed", but logs says "build suceeded" O_o

https://f-droid.org/wiki/page/org.sufficientlysecure.ical

@tasn
Copy link
Contributor

tasn commented Mar 27, 2017

Looks good now according to the wiki.

Oops, didn't mean to send yet.

Edit: thanks. :P

@doak
Copy link

doak commented Mar 27, 2017

Confirmed, just got the F-Droid update: it works.

Thanks all lot!

Regards,
doak

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

Successfully merging a pull request may close this issue.

5 participants