Bug 334633 - Album metadata sorted in non-deterministic way in some cases
Summary: Album metadata sorted in non-deterministic way in some cases
Status: RESOLVED FIXED
Alias: None
Product: digikam
Classification: Applications
Component: Albums-Sort (show other bugs)
Version: 4.0.0
Platform: Compiled Sources Linux
: NOR minor
Target Milestone: ---
Assignee: Digikam Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-12 02:52 UTC by Michal Sylwester
Modified: 2017-08-18 11:25 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.1.0
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
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