Bug 334633

Summary:	Album metadata sorted in non-deterministic way in some cases
Product:	[Applications] digikam	Reporter:	Michal Sylwester <msylwester>
Component:	Albums-Sort	Assignee:	Digikam Developers <digikam-bugs-null>
Status:	RESOLVED FIXED
Severity:	minor	CC:	caulier.gilles, mohammed.ahmed.anwer
Priority:	NOR
Version:	4.0.0
Target Milestone:	---
Platform:	Compiled Sources
OS:	Linux
Latest Commit:		Version Fixed In:	4.1.0
Sentry Crash Report:

Description Michal Sylwester 2014-05-12 02:52:08 UTC

When sorting by metadata it is likely that several albums will have exactly same sorting key (came category or same date). In such case order of such albums is random and changes on every re-sort.

Reproducible: Always

Steps to Reproduce:
1. Sort albums by metadata (category or date)
2. Make sure there are at least 2 albums with same category/date that are siblings in album tree
3. Expand/collapse a album tree branch to trigger re-sort
Actual Results:  
Albums jump around more or less randomly

Expected Results:  
Albums stay in-place with the branch expanded/collapsed

I think it is necessary to add additional sorting key in case of primary key conflict. Sorting by album name is deterministic as duplicate names are not allowed, not so with metadata.

My suggestion: make "sort by category/date" mean "sort by category/date, then by album name".

Comment 1 caulier.gilles 2014-05-12 14:50:24 UTC

Mohamed,

New entry from Michal, relevant of AlbumFilter implementation

Gilles Caulier

Comment 2 caulier.gilles 2014-05-12 21:15:46 UTC

Git commit 64c44af9f6f2f0f172e12ca8f718b4acedb6dcc2 by Mohamed Anwer.
Committed on 12/05/2014 at 21:00.
Pushed by mohamedanwer into branch 'master'.

Fixing CCBUG:334633

M  +6    -0    libs/models/albumfiltermodel.cpp

http://commits.kde.org/digikam/64c44af9f6f2f0f172e12ca8f718b4acedb6dcc2

diff --git a/libs/models/albumfiltermodel.cpp b/libs/models/albumfiltermodel.cpp
index 9ec3a77..4ff57ce 100644
--- a/libs/models/albumfiltermodel.cpp
+++ b/libs/models/albumfiltermodel.cpp
@@ -372,6 +372,12 @@ bool AlbumFilterModel::lessThan(const QModelIndex& left, const QModelIndex& righ
     QVariant valRight = dataForCurrentSortRole(right);

     AlbumSettings::StringComparisonType strComparisonType = AlbumSettings::instance()->getStringComparisonType();
+    AlbumSettings::AlbumSortOrder role = AlbumSettings::instance()->getAlbumSortOrder();
+
+    if((role == AlbumSettings::ByDate || role == AlbumSettings::ByCategory)&&(valLeft == valRight))
+    {
+            return QSortFilterProxyModel::lessThan(left, right);
+    }

     if((valLeft.type() == QVariant::String) && (valRight.type() == QVariant::String))
     {

Comment 3 caulier.gilles 2014-05-12 21:17:04 UTC

Mohamed, please use "BUG: xxxxxx" macro ALONE to close file automatically, not "Fixing ...."

Gilles Caulier

Comment 4 Mohamed 2014-05-12 21:19:34 UTC

I tried to CC the commit to tell Mr Michal about the commit, and test the implementation then closing the entry.
But it didn't send a CC !

Comment 5 caulier.gilles 2014-05-12 21:22:55 UTC

CC is sent using "BUG: xxxxx" macro alone...

Gilles