[Kde-pim] KCal::Calendar sortEvents

[Ron Goodheart]

On Thursday 02 October 2008 05:48:49 Allen Winter wrote:
> On Wednesday 01 October 2008 00:22:25 ron wrote:
> > Hi all,
> > I was working on the issues identified in Bug 170894: 	"month printout is
> > hard to read"...
> > One of the problems is that the calendar event sorting 'sortEvents()' is
> > not returning the event in date & time order -  only date.  While it's
> > easy enough to fix this in the printing it will certainly pop up in
> > several places and is better to take care of in one place.
> >
> > My thought was to add another eventSortField enum entry called
> > EventSortStartDateTime.
> >
> > Question: is this the right thing to do?
> >
> > Using EventSortStartDate compares two KDateTime objects but the resulting
> > sorted list is only in date & summary order (the first sort done).
>
> I think the right thing to do is fix EventSortStartDate to take the date &
> time into consideration. My bad.  I wrote this stuff.
>
> _______________________________________________
> KDE PIM mailing list kde-pim at kde.org
> https://mail.kde.org/mailman/listinfo/kde-pim
> KDE PIM home page at http://pim.kde.org/

Well, digging a little deeper...
I think the sort code is actually okay, I think KDateTime is at fault. 
It comes down to a slight difference in how KDateTime and QDateTime handle 
date equality if one of the dates is a 'date only' type of date.

QDateTime considers the 'date only' to be less than a date that has a time.
KDateTime does not because it only compares the dates (unless the time zones 
are different then it changes the time if the first date is 'date only').  

The operator less-than code in KDateTime didn't look right so I ran a small 
test with the following results:

dateOnly = "2008-10-02T00:00:00-07:00"
datetime =  "2008-10-02T00:01:00-07:00"
dateOnly isDateOnly? =  true
datetime isDateOnly? =  false
dateOnly <  datetime =  false
dateOnly <= datetime =  true   <---
dateOnly >  datetime =  false
dateOnly >= datetime =  true   <---
dateOnly == datetime =  false
datetime <  dateOnly =  false
datetime <= dateOnly =  true  <---
datetime >  dateOnly =  false
datetime >= dateOnly =  true  <---
datetime == dateOnly =  false

This is clearly incorrect!

With the attached patch (with old code commented) I get:
dateOnly = "2008-10-03T00:00:00-07:00"
datetime =  "2008-10-03T00:01:00-07:00"
dateOnly isDateOnly? =  true
datetime isDateOnly? =  false
dateOnly <  datetime =  true    <---
dateOnly <= datetime =  true  <---
dateOnly >  datetime =  false
dateOnly >= datetime =  false
dateOnly == datetime =  false
datetime <  dateOnly =  false
datetime <= dateOnly =  false
datetime >  dateOnly =  true    <---
datetime >= dateOnly =  true   <---
datetime == dateOnly =  false

I don't understand why the code makes a 'date only' end-of-day unless there 
was some kind of assumption of start-date and end-date? It does have an 
interesting side effect, but clearly not desirable. 

Are there automated tests for KDateTime that I should be checking?

Thanks,
I appreciate the assistance.

Regards,
Ron 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kdatetime.diff
Type: text/x-diff
Size: 1762 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20081003/d618b4b1/attachment.diff>
-------------- next part --------------
  KDateTime dts = KDateTime::currentLocalDateTime();
  KDateTime dateOnly( dts.date() );
  KDateTime datetime( dts );
  datetime.setTime(QTime(0,1,0,0));

  kDebug() << "dateOnly ="  << dateOnly.toString();
  kDebug() << "datetime = " << datetime.toString();
  kDebug() << "dateOnly isDateOnly? = "  << (dateOnly.isDateOnly() ? "true" : "false");
  kDebug() << "datetime isDateOnly? = "  << (datetime.isDateOnly() ? "true" : "false");
  
  kDebug() << "dateOnly <  datetime = " << ((dateOnly <  datetime) ? "true" : "false");
  kDebug() << "dateOnly <= datetime = " << ((dateOnly <= datetime) ? "true" : "false");
  kDebug() << "dateOnly >  datetime = " << ((dateOnly >  datetime) ? "true" : "false");
  kDebug() << "dateOnly >= datetime = " << ((dateOnly >= datetime) ? "true" : "false");
  kDebug() << "dateOnly == datetime = " << ((dateOnly == datetime) ? "true" : "false");
  kDebug() << "datetime <  dateOnly = " << ((datetime <  dateOnly) ? "true" : "false");
  kDebug() << "datetime <= dateOnly = " << ((datetime <= dateOnly) ? "true" : "false");
  kDebug() << "datetime >  dateOnly = " << ((datetime >  dateOnly) ? "true" : "false");
  kDebug() << "datetime >= dateOnly = " << ((datetime >= dateOnly) ? "true" : "false");
  kDebug() << "datetime == dateOnly = " << ((datetime == dateOnly) ? "true" : "false");
-------------- next part --------------
_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/