Bug 233200 - Deletion dialog after tracks move (with organize files feature) wrongly shows destination files to be removed, while removes source files (which is a wanted behavior)
Summary: Deletion dialog after tracks move (with organize files feature) wrongly shows...
Status: RESOLVED FIXED
Alias: None
Product: amarok
Classification: Applications
Component: Collections/Local (show other bugs)
Version: 2.3-GIT
Platform: openSUSE Linux
: NOR normal
Target Milestone: ---
Assignee: Amarok Developers
URL:
Keywords:
: 231227 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-04-03 21:41 UTC by Kuba Serafinowski
Modified: 2012-08-19 16:54 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 2.3.1


Attachments
Screen shot of a deletion dialog (66.82 KB, image/png)
2010-04-03 21:43 UTC, Kuba Serafinowski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Kuba Serafinowski 2010-04-03 21:41:06 UTC
Version:           2.3-GIT (using KDE 4.4.2)
OS:                Linux
Installed from:    openSUSE RPMs

Title says it all, wanted behavior is that we actually see tracks which are going to be removed, and not which are in our destination directory.
Comment 1 Kuba Serafinowski 2010-04-03 21:43:02 UTC
Created attachment 42465 [details]
Screen shot of a deletion dialog

Attaching screen shot for destined pattern of "%artist/%year - %album/%track %title"
Comment 2 Casey Link 2010-04-04 00:57:00 UTC
commit 1c25235adbf645e6eeb099abf9af49112ac58320
Author: Casey Link <unnamedrambler@gmail.com>
Date:   Sat Apr 3 18:47:37 2010 -0400

    Architectural changes to prevent the second delete dialog from popping
    up when performing an organize operation
    BUG: 233200

diff --git a/ChangeLog b/ChangeLog
index 9a25c11..8631f5a 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -54,6 +54,8 @@ VERSION 2.3.1-Beta 1
        some MySQL versions. (BR 225052)
 
   BUGFIXES:
+     * Fixed the double delete confirmation dialog when organizing tracks.
+      (BR 233200)
      * Fixed broken rendering of ratings in Current Track applet on startup.
      * Fixed command type names shown in the Bookmark Manager not being translated.
        (BR 226829)
diff --git a/src/core-impl/collections/sqlcollection/SqlCollectionLocation.cpp b/src/core-impl/collections/sqlcollection/SqlCollectionLocation.cpp
index 1ac258f..66a5da2 100644
--- a/src/core-impl/collections/sqlcollection/SqlCollectionLocation.cpp
+++ b/src/core-impl/collections/sqlcollection/SqlCollectionLocation.cpp
@@ -122,16 +122,20 @@ SqlCollectionLocation::remove( const Meta::TrackPtr &track )
     KSharedPtr<Meta::SqlTrack> sqlTrack = KSharedPtr<Meta::SqlTrack>::dynamicCast( track );
     if( sqlTrack && sqlTrack->inCollection() && sqlTrack->collection()->collectionId() == m_collection->collectionId() )
     {
-        debug() << "much much";
         bool removed;
-        KUrl originalUrl = m_originalUrls[track];
+        KUrl src = track->playableUrl();
+        if( destination() == source() ) // is organize operation?
+        {
+            SqlCollectionLocation* destinationloc = dynamic_cast<SqlCollectionLocation*>( destination() );
+            src = destinationloc->m_originalUrls[track];
+        }
         // we are going to delete it from the database only if is no longer on disk
-        removed = !QFile::exists( originalUrl.path() );
-
+        removed = !QFile::exists( src.path() );
         if( removed )
         {
-            int deviceId = m_collection->mountPointManager()->getIdForUrl( originalUrl );
-            QString rpath = m_collection->mountPointManager()->getRelativePath( deviceId, originalUrl.url() );
+            debug() << "file not on disk, remove from dbase";
+            int deviceId = m_collection->mountPointManager()->getIdForUrl( src );
+            QString rpath = m_collection->mountPointManager()->getRelativePath( deviceId, src.url() );
             QString query = QString( "SELECT id FROM urls WHERE deviceid = %1 AND rpath = '%2';" )
                                 .arg( QString::number( deviceId ), m_collection->sqlStorage()->escape( rpath ) );
             QStringList res = m_collection->sqlStorage()->query( query );
@@ -146,7 +150,7 @@ SqlCollectionLocation::remove( const Meta::TrackPtr &track )
                 m_collection->sqlStorage()->query( query );
             }
 
-            QFileInfo file( m_originalUrls[track].path() );
+            QFileInfo file( src.path() );
             QDir dir = file.dir();
             const QStringList collectionFolders = m_collection->mountPointManager()->collectionFolders();
             while( !collectionFolders.contains( dir.absolutePath() ) && !dir.isRoot() && dir.count() == 0 )
@@ -162,7 +166,7 @@ SqlCollectionLocation::remove( const Meta::TrackPtr &track )
     }
     else
     {
-        debug() << "Remove Failed: track exists on disk." << m_originalUrls[track].path();
+        debug() << "Remove Failed";
         return false;
     }
 }
@@ -271,8 +275,6 @@ SqlCollectionLocation::slotJobFinished( KJob *job )
         source()->transferError(m_jobs.value( job ), KIO::buildErrorString( job->error(), job->errorString() ) );
         m_destinations.remove( m_jobs.value( job ) );
     }
-    //we  assume that KIO works correctly...
-    source()->transferSuccessful( m_jobs.value( job ) );
 
     m_jobs.remove( job );
     job->deleteLater();
@@ -323,7 +325,12 @@ void SqlCollectionLocation::slotTransferJobFinished( KJob* job )
         if( !QFileInfo( m_destinations[ track ] ).exists() )
             m_destinations.remove( track );
         m_originalUrls[track] = track->playableUrl();
+        debug() << "inserting original url" << m_originalUrls[track];
+        // report successful transfer with the original url
+        source()->transferSuccessful( track );
+
     }
+    debug () << "m_originalUrls" << m_originalUrls;
     insertTracks( m_destinations );
     insertStatistics( m_destinations );
     m_collection->scanManager()->setBlockScan( false );
@@ -566,7 +573,13 @@ bool SqlCollectionLocation::startNextRemoveJob()
     while ( !m_removetracks.isEmpty() )
     {
         Meta::TrackPtr track = m_removetracks.takeFirst();
-        KUrl src = m_originalUrls[track];
+        KUrl src = track->playableUrl();
+
+        if( destination() == source() ) // is organize operation?
+        {
+            SqlCollectionLocation* destinationloc = dynamic_cast<SqlCollectionLocation*>( destination() );
+            src = destinationloc->m_originalUrls[track];
+        }
 
         if( src == track->playableUrl() ) // src == dst
             break;
diff --git a/src/core/collections/CollectionLocation.cpp b/src/core/collections/CollectionLocation.cpp
index ccc778a..7b14645 100644
--- a/src/core/collections/CollectionLocation.cpp
+++ b/src/core/collections/CollectionLocation.cpp
@@ -35,6 +35,7 @@ CollectionLocation::CollectionLocation()
     , m_parentCollection( 0 )
     , m_removeSources( false )
     , m_isRemoveAction( false )
+    , m_noRemoveConfirmation( false )
 {
     //nothing to do
 }
@@ -296,13 +297,17 @@ CollectionLocation::showRemoveDialog( const Meta::TrackList &tracks )
 {
     DEBUG_BLOCK
 
-    Collections::CollectionLocationDelegate *delegate = Amarok::Components::collectionLocationDelegate();
+    if( !m_noRemoveConfirmation )
+    {
+        Collections::CollectionLocationDelegate *delegate = Amarok::Components::collectionLocationDelegate();
 
-    const bool del = delegate->reallyDelete( this, tracks );
+        const bool del = delegate->reallyDelete( this, tracks );
 
-    if( !del )
-        slotFinishRemove();
-    else
+        if( !del )
+            slotFinishRemove();
+        else
+            slotShowRemoveDialogDone();
+    } else
         slotShowRemoveDialogDone();
 }
 
@@ -368,13 +373,21 @@ CollectionLocation::slotFinishCopy()
 {
     DEBUG_BLOCK
     if( m_removeSources )
+    {
         removeSourceTracks( m_tracksSuccessfullyTransferred );
-    m_sourceTracks.clear();
-    m_tracksSuccessfullyTransferred.clear();
-    if( m_destination )
-        m_destination->deleteLater();
-    m_destination = 0;
-    this->deleteLater();
+        m_sourceTracks.clear();
+        m_tracksSuccessfullyTransferred.clear();
+    }
+    else
+    {
+        m_sourceTracks.clear();
+        m_tracksSuccessfullyTransferred.clear();
+
+        if( m_destination )
+            m_destination->deleteLater();
+        m_destination = 0;
+        this->deleteLater();
+    }
 }
 
 void
@@ -518,6 +531,7 @@ CollectionLocation::removeSourceTracks( const Meta::TrackList &tracks )
     toRemove.subtract( errored );
 
     // start the remove workflow
+    m_noRemoveConfirmation = true;
     prepareRemove( toRemove.toList() );
 }
 
diff --git a/src/core/collections/CollectionLocation.h b/src/core/collections/CollectionLocation.h
index 2401e30..8a243c9 100644
--- a/src/core/collections/CollectionLocation.h
+++ b/src/core/collections/CollectionLocation.h
@@ -333,6 +333,7 @@ class AMAROK_EXPORT CollectionLocation : public QObject
         void setRemoveSources( bool removeSources ) { m_removeSources = removeSources; }
         bool m_removeSources;
         bool m_isRemoveAction;
+        bool m_noRemoveConfirmation;
         //used by the source collection to store the tracks that were successfully
         //copied by the destination and can be removed as part of a move
         Meta::TrackList m_tracksSuccessfullyTransferred;
Comment 3 Casey Link 2010-05-29 22:16:31 UTC
*** Bug 231227 has been marked as a duplicate of this bug. ***