| Summary: | Group by date message up sometimes, e.g. showing yesterday - saturday - yesterday - saturday | ||
|---|---|---|---|
| Product: | [Unmaintained] KMail Mobile | Reporter: | Bernhard E. Reiter <bernhard> |
| Component: | general | Assignee: | Ludwig Reiter <ludwig.reiter> |
| Status: | VERIFIED FIXED | ||
| Severity: | normal | CC: | aheinecke, bjoern.ricks, tokoe |
| Priority: | HI | ||
| Version: | unspecified | ||
| Target Milestone: | --- | ||
| Platform: | Maemo 5 | ||
| OS: | Linux | ||
| Latest Commit: | Version Fixed In: | ||
| Sentry Crash Report: | |||
| Attachments: | N900 Screenshot showing messed up group sorting of Kontact Touch Mail | ||
Could also reproduce the issue when searching with N900, got Dezember 2010, Mittwoch, Dezember 2010 Reproducible on Linux/Desktop... will debug it commit b984b66bd8e090d96bcd451a9d19e4af0c5d86ca branch master Author: Tobias Koenig <tokoe@kde.org> Date: Wed Jan 12 13:51:40 2011 +0100 Fix grouping of messages in 'most recent' mode Let ThreadGrouperComparator handle the grouper string, since it knows what are the thread leader and the most recent items in a thread. BUG: 262735 diff --git a/mobile/lib/threadgroupermodel.cpp b/mobile/lib/threadgroupermodel.cpp index 8a00c09..cedb635 100644 --- a/mobile/lib/threadgroupermodel.cpp +++ b/mobile/lib/threadgroupermodel.cpp @@ -60,6 +60,11 @@ ThreadGrouperComparator::~ThreadGrouperComparator() { } +QString ThreadGrouperComparator::grouperString( const Akonadi::Item& ) const +{ + return QString(); +} + Akonadi::Item ThreadGrouperComparator::threadItem( const Akonadi::Item &item ) const { Q_ASSERT( m_grouper ); @@ -186,6 +191,9 @@ QVariant ThreadGrouperModel::data( const QModelIndex &index, int role ) const if ( role == ThreadIdRole ) return d->threadRoot( index ).id(); + else if ( role == GrouperRole ) { + return d->m_comparator->grouperString( index.data( Akonadi::EntityTreeModel::ItemRole ).value<Akonadi::Item>() ); + } return QSortFilterProxyModel::data( index, role ); } @@ -209,6 +217,10 @@ void ThreadGrouperModel::setSourceModel( QAbstractItemModel *sourceModel ) if ( d->m_dynamicModelRepopulation ) connect( sourceModel, SIGNAL( dataChanged( const QModelIndex&, const QModelIndex& ) ), this, SLOT( populateThreadGrouperModel() ) ); + + QHash<int, QByteArray> names = roleNames(); + names.insert( GrouperRole, "grouperString" ); + setRoleNames( names ); } bool ThreadGrouperModel::lessThan( const QModelIndex &left, const QModelIndex &right ) const @@ -228,6 +240,7 @@ void ThreadGrouperModel::setThreadingEnabled( bool enabled ) d->m_threadingEnabled = enabled; invalidate(); + reset(); } bool ThreadGrouperModel::threadingEnabled() const diff --git a/mobile/lib/threadgroupermodel.h b/mobile/lib/threadgroupermodel.h index ba43ea0..0e05171 100644 --- a/mobile/lib/threadgroupermodel.h +++ b/mobile/lib/threadgroupermodel.h @@ -62,6 +62,11 @@ class MOBILEUI_EXPORT ThreadGrouperComparator */ virtual bool lessThan( const Akonadi::Item &left, const Akonadi::Item &right ) const = 0; + /** + * Returns the grouper string for the given @p item. + */ + virtual QString grouperString( const Akonadi::Item &item ) const; + protected: /** * Returns the thread item for @p item. @@ -102,7 +107,8 @@ class MOBILEUI_EXPORT ThreadGrouperModel : public QSortFilterProxyModel public: enum CustomRoles { // FIXME Fix custom role handling in proxies. - ThreadIdRole = Akonadi::EntityTreeModel::UserRole + 30 + ThreadIdRole = Akonadi::EntityTreeModel::UserRole + 30, + GrouperRole }; /** diff --git a/mobile/mail/HeaderView.qml b/mobile/mail/HeaderView.qml index 7cabc62..3e99446 100644 --- a/mobile/mail/HeaderView.qml +++ b/mobile/mail/HeaderView.qml @@ -29,7 +29,6 @@ KPIM.ItemListView { property variant checkModel property string collapsedSections property bool showSections : true - property string groupingRole : "dateGroup" delegate: [ KPIM.ItemListViewDelegate { @@ -37,7 +36,7 @@ KPIM.ItemListView { showCheckBox : _top.showCheckBox checkModel : _top.checkModel navigationModel : _top.navigationModel - height : (_top.collapsedSections.indexOf(model[groupingRole]) >= 0) ? 0 : (itemListView.height / 7) + height : (_top.collapsedSections.indexOf(model.grouperString) >= 0) ? 0 : (itemListView.height / 7) clip: true summaryContent : [ QML.Text { @@ -208,7 +207,7 @@ KPIM.ItemListView { } ] - section.property: showSections ? _top.groupingRole : "" + section.property: showSections ? "grouperString" : "" section.criteria: QML.ViewSection.FullString section.delegate: QML.Item { id: sectionDelegate diff --git a/mobile/mail/kmail-mobile.qml b/mobile/mail/kmail-mobile.qml index 6c77807..66fb2fd 100644 --- a/mobile/mail/kmail-mobile.qml +++ b/mobile/mail/kmail-mobile.qml @@ -296,7 +296,6 @@ KPIM.MainView { navigationModel : _threadSelector showDeleteButton : false // too easy to accidentally hit it, although very useful... opacity : threadContentsViewContainer.opacity == 0 ? 1 : 0 - groupingRole : messageListSettings.groupingRole showSections : messageListSettings.groupingRole != "" } QML.Connections { diff --git a/mobile/mail/mailthreadgroupercomparator.cpp b/mobile/mail/mailthreadgroupercomparator.cpp index 35dec0f..c931509 100644 --- a/mobile/mail/mailthreadgroupercomparator.cpp +++ b/mobile/mail/mailthreadgroupercomparator.cpp @@ -24,6 +24,10 @@ #include <akonadi/kmime/messageflags.h> #include <messagecore/stringutil.h> +#include <klocale.h> +#include <kglobal.h> +#include <kcalendarsystem.h> + MailThreadGrouperComparator::MailThreadGrouperComparator() : mSortingOption( SortByDateTimeMostRecent ), mIsOutboundCollection( false ) @@ -87,8 +91,8 @@ bool MailThreadGrouperComparator::lessThan( const Akonadi::Item &leftItem, const break; case SortByDateTimeMostRecent: { - const KDateTime leftNewest = mostRecentUpdate( leftThreadRootMessage, leftThreadRootItem.id() ); - const KDateTime rightNewest = mostRecentUpdate( rightThreadRootMessage, rightThreadRootItem.id() ); + const KDateTime leftNewest = mostRecentDateTimeInThread( leftThreadRootMessage, leftThreadRootItem.id() ); + const KDateTime rightNewest = mostRecentDateTimeInThread( rightThreadRootMessage, rightThreadRootItem.id() ); if ( leftNewest != rightNewest ) { return leftNewest > rightNewest; @@ -183,6 +187,18 @@ MailThreadGrouperComparator::SortingOption MailThreadGrouperComparator::sortingO return mSortingOption; } +void MailThreadGrouperComparator::setGroupingOption( GroupingOption option ) +{ + mGroupingOption = option; + + invalidate(); +} + +MailThreadGrouperComparator::GroupingOption MailThreadGrouperComparator::groupingOption() const +{ + return mGroupingOption; +} + void MailThreadGrouperComparator::setIsOutboundCollection( bool outbound ) { mIsOutboundCollection = outbound; @@ -190,6 +206,72 @@ void MailThreadGrouperComparator::setIsOutboundCollection( bool outbound ) invalidate(); } +QString MailThreadGrouperComparator::grouperString( const Akonadi::Item &item ) const +{ + KMime::Message::Ptr msg; + + if ( mSortingOption == SortByDateTimeMostRecent ) { + const Akonadi::Item::Id newestItem = mostRecentIdInThread( messageForItem( item ), item.id() ); + msg = messageForItem( Akonadi::Item( newestItem ) ); + } else { + const Akonadi::Item rootItem = threadItem( item ); + msg = messageForItem( rootItem ); + } + + if ( mGroupingOption == GroupByDate ) { + // simplified version taken from libmessagelist + const KDateTime& dt = msg->date()->dateTime(); + const QDate dDate = dt.date(); + const KCalendarSystem *calendar = KGlobal::locale()->calendar(); + int daysAgo = -1; + if ( calendar->isValid( dDate ) && calendar->isValid( QDate::currentDate() ) ) { + daysAgo = dDate.daysTo( QDate::currentDate() ); + } + + if ( daysAgo < 0 || !dt.isValid() ) // In the future or invalid + return i18n( "Unknown" ); + else if( daysAgo == 0 ) // Today + return i18n( "Today" ); + else if ( daysAgo == 1 ) // Yesterday + return i18n( "Yesterday" ); + else if ( daysAgo > 1 && daysAgo < calendar->daysInWeek( QDate::currentDate() ) ) // Within last seven days + return KGlobal::locale()->calendar()->weekDayName( dDate ); + else if( calendar->month( dDate ) == calendar->month( QDate::currentDate() ) && calendar->year( dDate ) == calendar->year( QDate::currentDate() ) ) { // within this month + const int startOfWeekDaysAgo = ( calendar->daysInWeek( QDate::currentDate() ) + calendar->dayOfWeek( QDate::currentDate() ) - + KGlobal::locale()->weekStartDay() ) % calendar->daysInWeek( QDate::currentDate() ); + const int weeksAgo = ( ( daysAgo - startOfWeekDaysAgo ) / calendar->daysInWeek( QDate::currentDate() ) ) + 1; + if ( weeksAgo == 0 ) + return KGlobal::locale()->calendar()->weekDayName( dDate ); + else + return i18np( "One Week Ago", "%1 Weeks Ago", weeksAgo ); + } else if ( calendar->year( dDate ) == calendar->year( QDate::currentDate() ) ) { // within this year + return calendar->monthName( dDate ); + } else { // in previous years + static QHash<int, QString> yearNameHash; + + QString yearName; + if ( yearNameHash.contains( dDate.year() ) ) { + yearName = yearNameHash.value( dDate.year() ); + } else { + yearName = calendar->yearString( dDate ); + yearNameHash.insert( dDate.year(), yearName ); + } + return i18nc( "Message Aggregation Group Header: Month name and Year number", "%1 %2", calendar->monthName( dDate ), yearName ); + } + } else if ( mGroupingOption == GroupBySenderReceiver ) { + QStringList l; + foreach ( const KMime::Types::Mailbox &mbox, msg->from()->mailboxes() ) { + if ( mbox.hasName() ) + l.append( mbox.name() ); + else + l.append( mbox.addrSpec().asPrettyString() ); + } + return l.join( ", " ); + } else { + return QLatin1String( "dummy" ); + } +} + void MailThreadGrouperComparator::resetCaches() { mMessageCache.clear(); @@ -205,18 +287,22 @@ QByteArray MailThreadGrouperComparator::identifierForMessage( const KMime::Messa return identifier; } -KDateTime MailThreadGrouperComparator::mostRecentUpdate( const KMime::Message::Ptr &threadRoot, Akonadi::Item::Id itemId ) const +KDateTime MailThreadGrouperComparator::mostRecentDateTimeInThread( const KMime::Message::Ptr &threadRoot, Akonadi::Item::Id itemId ) const { - const QHash<Akonadi::Item::Id, KDateTime>::const_iterator it = mMostRecentCache.constFind( itemId ); + const QHash<Akonadi::Item::Id, MostRecentEntry>::const_iterator it = mMostRecentCache.constFind( itemId ); if ( it != mMostRecentCache.constEnd() ) - return *it; + return (*it).dateTime; const QSet<QByteArray> messageIds = threadDescendants( identifierForMessage( threadRoot, itemId ) ); KDateTime newest = threadRoot->date()->dateTime(); + Akonadi::Item::Id newestId = itemId; if ( messageIds.isEmpty() ) { - mMostRecentCache.insert( itemId, newest ); + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + mMostRecentCache.insert( itemId, entry ); return newest; } @@ -227,14 +313,59 @@ KDateTime MailThreadGrouperComparator::mostRecentUpdate( const KMime::Message::P const KMime::Message::Ptr message = messageForItem( item ); const KDateTime messageDateTime = message->date()->dateTime(); - if ( messageDateTime > newest ) + if ( messageDateTime > newest ) { newest = messageDateTime; + newestId = item.id(); + } } - mMostRecentCache.insert( itemId, newest ); + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + + mMostRecentCache.insert( itemId, entry ); return newest; } +Akonadi::Item::Id MailThreadGrouperComparator::mostRecentIdInThread( const KMime::Message::Ptr &threadRoot, Akonadi::Item::Id itemId ) const +{ + const QHash<Akonadi::Item::Id, MostRecentEntry>::const_iterator it = mMostRecentCache.constFind( itemId ); + if ( it != mMostRecentCache.constEnd() ) + return (*it).id; + + const QSet<QByteArray> messageIds = threadDescendants( identifierForMessage( threadRoot, itemId ) ); + + KDateTime newest = threadRoot->date()->dateTime(); + Akonadi::Item::Id newestId = itemId; + + if ( messageIds.isEmpty() ) { + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + mMostRecentCache.insert( itemId, entry ); + return itemId; + } + + foreach ( const QByteArray &messageId, messageIds ) { + const Akonadi::Item item = itemForIdentifier( messageId ); + Q_ASSERT( item.isValid() ); + Q_ASSERT( item.hasPayload<KMime::Message::Ptr>() ); + + const KMime::Message::Ptr message = messageForItem( item ); + const KDateTime messageDateTime = message->date()->dateTime(); + if ( messageDateTime > newest ) + newest = messageDateTime; + newestId = item.id(); + } + + MostRecentEntry entry; + entry.id = newestId; + entry.dateTime = newest; + + mMostRecentCache.insert( itemId, entry ); + return itemId; +} + KMime::Message::Ptr MailThreadGrouperComparator::messageForItem( const Akonadi::Item &item ) const { const QHash<Akonadi::Item::Id, KMime::Message::Ptr>::const_iterator it = mMessageCache.constFind( item.id() ); diff --git a/mobile/mail/mailthreadgroupercomparator.h b/mobile/mail/mailthreadgroupercomparator.h index 5c3d696..4fa41e4 100644 --- a/mobile/mail/mailthreadgroupercomparator.h +++ b/mobile/mail/mailthreadgroupercomparator.h @@ -39,6 +39,13 @@ class MailThreadGrouperComparator : public ThreadGrouperComparator SortByActionItem }; + enum GroupingOption + { + GroupByNone, + GroupByDate, + GroupBySenderReceiver + }; + /** * Creates a new mail thread grouper comparator. */ @@ -67,24 +74,38 @@ class MailThreadGrouperComparator : public ThreadGrouperComparator void setSortingOption( SortingOption option ); SortingOption sortingOption() const; + void setGroupingOption( GroupingOption option ); + GroupingOption groupingOption() const; + /** * Sets whether the currently compared items come from an outbound mail collection * (e.g. outbox, sent or drafts). */ void setIsOutboundCollection( bool outbound ); + virtual QString grouperString( const Akonadi::Item &item ) const; + protected: virtual void resetCaches(); private: QByteArray identifierForMessage( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; - KDateTime mostRecentUpdate( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; + KDateTime mostRecentDateTimeInThread( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; + Akonadi::Item::Id mostRecentIdInThread( const KMime::Message::Ptr&, Akonadi::Item::Id ) const; KMime::Message::Ptr messageForItem( const Akonadi::Item &item ) const; SortingOption mSortingOption; + GroupingOption mGroupingOption; bool mIsOutboundCollection; mutable QHash<Akonadi::Item::Id, KMime::Message::Ptr> mMessageCache; - mutable QHash<Akonadi::Item::Id, KDateTime> mMostRecentCache; + + struct MostRecentEntry + { + Akonadi::Item::Id id; + KDateTime dateTime; + }; + + mutable QHash<Akonadi::Item::Id, MostRecentEntry> mMostRecentCache; }; #endif diff --git a/mobile/mail/mainview.cpp b/mobile/mail/mainview.cpp index 35f0d14..cdfae0e 100644 --- a/mobile/mail/mainview.cpp +++ b/mobile/mail/mainview.cpp @@ -1753,6 +1753,18 @@ void MainView::messageListSettingsChanged( const MessageListSettings &settings ) break; } + switch ( settings.groupingOption() ) { + case MessageListSettings::GroupByDate: + m_grouperComparator->setGroupingOption( MailThreadGrouperComparator::GroupByDate ); + break; + case MessageListSettings::GroupBySenderReceiver: + m_grouperComparator->setGroupingOption( MailThreadGrouperComparator::GroupBySenderReceiver ); + break; + case MessageListSettings::GroupByNone: + m_grouperComparator->setGroupingOption( MailThreadGrouperComparator::GroupByNone ); + break; + } + m_threadGrouperModel->setThreadingEnabled( settings.useThreading() ); m_threadGrouperModel->sort( 0, settings.sortingOrder() ); It does not seem to be fixed completely with windows-ce/Kontact-Mobile/testing/2011-01-17-02-23/ I still get groups like: "Mittwoch" "Vor 2 Wochen" "Dezember 2010" Vor 2 Wochen" "Dezember 2010" Reproduced with a freshly configured account, just selected INBOX and stayed in there until it was synced. Maillist view settings: Sorting: "nach neuster nachricht in der diskussion" "aufsteigend" [ ] not checked "Ordner verwendet immer diese Sortierung" Grouping: "nach neuster nachricht in der diskussion" [X] checked "Diskussionen anzeigen" Second time I've started the account, the grouping order was fine, so I _guess_ it only happens when (several) emails come in. A situation which can happen during any sync. Git commit f947dd12a27fe1a79a20416a085592073988bbd0 by Tobias Koenig. Committed on 02/02/11 at 17:54. Pushed by tokoe into branch 'komo3'. Fix sorting/grouping of incoming messages We have to resort the message list whenever new messages are inserted to make it work in 'most-recent-message-in-thread' mode. BUG: 262735 M +9 -0 mobile/lib/threadgroupermodel.cpp M +1 -0 mobile/lib/threadgroupermodel.h M +2 -1 mobile/mail/mailthreadgroupercomparator.cpp http://commits.kde.org/kdepim/f947dd12a27fe1a79a20416a085592073988bbd0 Git commit 5a808eb3b87641f372f0ee4ebb336e38d3fb8521 by Tobias Koenig. Committed on 02/02/11 at 17:54. Pushed by tokoe into branch 'master'. Fix sorting/grouping of incoming messages We have to resort the message list whenever new messages are inserted to make it work in 'most-recent-message-in-thread' mode. BUG: 262735 M +9 -0 mobile/lib/threadgroupermodel.cpp M +1 -0 mobile/lib/threadgroupermodel.h M +2 -1 mobile/mail/mailthreadgroupercomparator.cpp http://commits.kde.org/kdepim/5a808eb3b87641f372f0ee4ebb336e38d3fb8521 Seems to be fixed in 4:4.6~.20110214.1233.gitb901f2e-1maemo1.1219908 on maemo |
Created attachment 55813 [details] N900 Screenshot showing messed up group sorting of Kontact Touch Mail Package: kdepim-mobile, N900 Version: 4:4.6~.20110106.1209.gitfde48d5-1maemo1.1211475 Package: libqt4-experimental-gui Version: 4.7.0~git20101111-0maemo1 Selected my INBOX this morning (Monday) after touchung it the last time on Friday where I left it with four read emails. After the explicit sync from the menu and wachting the emails coming in, I had several groups for the same day. See attached screenshot. (For demonstration purposes I collapsed the groups, this does not change the order.) INBOX: takes sync options from account Account: no offline mode; no intervall check This should stay keyword komo3 at least until we have clarified that this does not happen on the HTC Touch Pro2.