Summary: | 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) | ||
---|---|---|---|
Product: | [Applications] amarok | Reporter: | Kuba Serafinowski <kuba.serafinowski> |
Component: | Collections/Local | Assignee: | Amarok Developers <amarok-bugs-dist> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | matej, ralf-engels, unnamedrambler, yyyy12 |
Priority: | NOR | ||
Version: | 2.3-GIT | ||
Target Milestone: | --- | ||
Platform: | openSUSE | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | 2.3.1 | |
Attachments: | Screen shot of a deletion dialog |
Description
Kuba Serafinowski
2010-04-03 21:41:06 UTC
Created attachment 42465 [details]
Screen shot of a deletion dialog
Attaching screen shot for destined pattern of "%artist/%year - %album/%track %title"
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; *** Bug 231227 has been marked as a duplicate of this bug. *** |