Bug 179076

*** Bug 179158 has been marked as a duplicate of this bug. ***

I've experienced similar behaviour with smart date ranges, and I get the 'unknown' category using "activity by date, threaded/flat" too. See my duplicate bug (#179158).

Also note that bogus emails will use the "unknown" label. e.g: if the date is in the future.

Same issue here, the threads from last week are in an unknown group. This week's are sorted into Yesterday, Monday etc.

Could this issue have the same reason as bug 179019?

From that bug:
30 December 2008 is "Week 1" (of 2009 , but for 2008 too)
28 December 2008 is "Week 52" (of 2008)

(this year have "53 weeks")

so having I file created/modified in the earlier December, does the comparision
(week 1 - week (49-52) ) and fails.

----------

I had an "unknown" group before, but if I clicked on those emails, they were sorted into the correct group.

In kdepim/messagelist/core/model.cpp you have: 

       // within this month
1369         int weekStartOffset = KGlobal::locale()->workingWeekStartDay() - 1;
1370         int todayWeekNumber = mTodayDate.addDays( -weekStartOffset ).weekNumber();
1371         int dateWeekNumber = dDate.addDays( -weekStartOffset ).weekNumber();
1372         if ( dateWeekNumber == todayWeekNumber )
1373         {
1374           // within this week
1375           groupLabel = KGlobal::locale()->calendar()->weekDayName( dDate );
1376         } else {
1377           // previous weeks
1378           int weekDiff = todayWeekNumber - dateWeekNumber;
1379           switch( weekDiff )
1380           {
1381             case 1:
1382               groupLabel = mCachedLastWeekLabel;
1383             break;
1384             case 2:
1385               groupLabel = mCachedTwoWeeksAgoLabel;
1386             break;
1387             case 3:
1388               groupLabel = mCachedThreeWeeksAgoLabel;
1389             break;
1390             case 4:
1391               groupLabel = mCachedFourWeeksAgoLabel;
1392             break;
1393             case 5:
1394               groupLabel = mCachedFiveWeeksAgoLabel;
1395             break;
1396             default:
1397               groupLabel = mCachedUnknownLabel; // should never happen
1398             break;
1399           }
1400         }

The default value is where I would guess the 'Unknown' comes from, because the code assumes that any 2 days in the same month will have the same or consecutive week numbers up to 5 weeks apart, i.e. traditional week numbering starting from 1st Jan.  This is not the case as weekNumber() returns the ISO week number (see http://en.wikipedia.org/wiki/ISO_week_date, yes we need to rename the api).  For example the week of 28 Dec 2009 to 3 Jan 2010 is week 53 of 2009, so Sun 3 Jan 2010 is week 53/2009 and Mon 4 Jan 2010 is week 1/2010, so line 1378 will return 1 - 53 = -52 and so get Unknown.

The correct way to calculate this would be something like:

int weekDiff;
if (todayWeekNumber < dateWeekNumber) {
    weekDiff = todayWeekNumber - dateWeekNumber;
} else {
    weekDiff = KLocale::locale()->Calendar()->weeksInYear(dDate) + todayWeekNumber - dateWeekNumber;
}

This assumes dDate is always <= today, if not the case then I can propose an alternative.

Three other points I notice that may need fixing.  

First, I'm not sure about the whole adjusting for workingWeekStartDay() thing, I think I see why you are doing it but I'm not sure it will work with ISO Week Numbers, I need to think some more.

Secondly, I think lines 1306 - 1320 are OK, other than my point about workingWeekStartDay().

Thirdly, and this definitely needs fixing, is you mix using QDate methods like month() and day() and KCalendarSystem methods like formatDate() and monthName(), which will produced inconsistent results if the user has selected a non-Gregorian calendar system.  You need to always use the KLocale::locale()->Calendar() methods (including weekNumber, year, month, day, addDays, etc) to ensure consistent and localised results.  I'd be happy to help with a patch for this.

John, thanks for your elaborate reply, good to have the KDE calendar expert here :)

> This assumes dDate is always <= today, if not the case then I can propose an
> alternative.

There are some mails with an incorrect Date header, so dDate can be in the future as well.

> Thirdly, and this definitely needs fixing, is you mix using QDate methods 
> like month() and day() and KCalendarSystem methods like formatDate() and
> monthName(), which will produced inconsistent results if the user has 
> selected a non-Gregorian calendar system.

I see. I guess nobody did think about other calendar systems when this code was first written.

> I'd be happy to help with a patch for this.

Great, a patch would be very welcome. Using reviewboard.kde.org is the best here.

OK, I'll work up a patch for all these.  Could you just clarify how you want the week grouping to split:
1) By ISO week number, i.e. always split starting from Monday
2) By Working Week start day, always split starting from KLocale::workingWeekStartDay() 
3) By Week Start day, i.e. always split starting from KLocale::weekStartDay()

*** Bug 221285 has been marked as a duplicate of this bug. ***

See ReviewBoard http://reviewboard.kde.org/r/2521/

*** Bug 221912 has been marked as a duplicate of this bug. ***

SVN commit 1073169 by jlayt:

Fix message aggregation by date.

As reviewed and approved at http://reviewboard.kde.org/r/2521/

This commit fixes SC 4.4.  Fix for trunk will be held until kmail trunk is
unfrozen after merge of akonadi-ports.

CCBUG: 179076


 M  +54 -127   model.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1073169

SVN commit 1075752 by tmcguire:

Fix Message Group Aggregation by Date

Patch by calendar expert John Layt <john at layt dot net>, thank you!

reviewboard.kde.org/r/2521

BUG: 179076


 M  +55 -128   model.cpp  


WebSVN link: http://websvn.kde.org/?view=rev&revision=1075752

*** Bug 222516 has been marked as a duplicate of this bug. ***