Bug 113356

*** Bug 119927 has been marked as a duplicate of this bug. ***

*** Bug 133224 has been marked as a duplicate of this bug. ***

*** Bug 115425 has been marked as a duplicate of this bug. ***

SVN commit 598090 by aoliveira:

* Save dynamic playlist items by their "path" (eg: "Smart Playlists/Collection/Favorite Tracks/By Pearl Jam"), instead of using 
only the name.
If we don't allow two playlists with the same name on the same folder, all our problems with playlists being misindentified will be 
fixed.
I bumped the version for dynamicbrowser_save.xml, and I still didn't write the migration code.

* While I was at it, adding and removing items to dynamic playlist is now recursive on Playlist Browser, for consistency with the 
Playlist Selection dialog.


BUG: 135993
CCBUG: 113356


 M  +4 -0      amarok.h  
 M  +4 -4      dynamicmode.cpp  
 M  +2 -2      dynamicmode.h  
 M  +1 -1      playlist.cpp  
 M  +104 -52   playlistbrowser.cpp  
 M  +9 -4      playlistbrowser.h  
 M  +20 -11    playlistbrowseritem.cpp  
 M  +2 -0      playlistbrowseritem.h  
 M  +17 -2     playlistselection.cpp  
 M  +2 -0      playlistselection.h  


--- trunk/extragear/multimedia/amarok/src/amarok.h #598089:598090
@@ -22,6 +22,8 @@
 class QPixmap;
 class QWidget;
 class DynamicMode;
+class QListView;
+class QListViewItem;
 namespace KIO { class Job; }
 
 namespace Amarok
@@ -197,6 +199,8 @@
 
     const DynamicMode *dynamicMode(); //defined in playlist.cpp
 
+    QListViewItem* findItemByPath( QListView *view, QString path ); //defined in playlistbrowser.cpp
+
     /**
      * Maps the icon name to a system icon or custom Amarok icon, depending on the settings.
      */
--- trunk/extragear/multimedia/amarok/src/dynamicmode.cpp #598089:598090
@@ -68,16 +68,16 @@
 void  DynamicMode::setAppendType( int type ) { m_appendType = type; }
 void  DynamicMode::setTitle( const QString& title ) { m_title = title; }
 
-void DynamicMode::setDynamicItems(const QPtrList<QListViewItem>& newList)
+void DynamicMode::setDynamicItems(const QPtrList<PlaylistBrowserEntry>& newList)
 {
     QStringList strListEntries;
-    QListViewItem* entry;
-    QPtrListIterator<QListViewItem> it( newList );
+    PlaylistBrowserEntry* entry;
+    QPtrListIterator<PlaylistBrowserEntry> it( newList );
 
     while( (entry = it.current()) != 0 )
     {
         ++it;
-        strListEntries << entry->text(0);
+        strListEntries << entry->name();
     }
 
     setItems(strListEntries);
--- trunk/extragear/multimedia/amarok/src/dynamicmode.h #598089:598090
@@ -37,7 +37,7 @@
 class QString;
 class QStringList;
 template<class T> class QPtrList;
-class QListViewItem;
+class PlaylistBrowserEntry;
 
 class DynamicMode
 {
@@ -48,7 +48,7 @@
 
         void edit();
         void deleting();
-        void setDynamicItems(const QPtrList<QListViewItem>& newList);
+        void setDynamicItems(const QPtrList<PlaylistBrowserEntry>& newList);
 
     public: //accessors
         QString title() const;
--- trunk/extragear/multimedia/amarok/src/playlist.cpp #598089:598090
@@ -646,7 +646,7 @@
     PlaylistBrowser *pb = PlaylistBrowser::instance();
     QListViewItem *item = 0;
 
-    QPtrList<QListViewItem> dynamicEntries = pb->dynamicEntries();
+    QPtrList<PlaylistBrowserEntry> dynamicEntries = pb->dynamicEntries();
 
     //FIXME: What if the randomiser grabs the same playlist again and again?  Lets remove the playlist from the list.
     for( uint y=0; y < dynamicEntries.count(); y++ )
--- trunk/extragear/multimedia/amarok/src/playlistbrowser.cpp #598089:598090
@@ -60,13 +60,42 @@
 #include <cstdio>              //rename() in renamePlaylist()
 
 
+
+namespace Amarok {
+    QListViewItem*
+    findItemByPath( QListView *view, QString name )
+    {
+        debug() << "Searching " << name << endl;
+        QStringList path = QStringList::split( '/' , name );
+
+        QListViewItem *prox = view->firstChild();
+        QListViewItem *item = 0;
+
+        foreach( path ) {
+            item = prox;
+            QString text( *it );
+
+            for ( ; item; item = item->nextSibling() ) {
+                if ( text == item->text(0) ) {
+                    break;
+                }
+            }
+
+            if ( !item )
+                return 0;
+            prox = item->firstChild();
+        }
+        return item;
+    }
+}
+
+
 inline QString
 fileExtension( const QString &fileName )
 {
     return Amarok::extension( fileName );
 }
 
-
 PlaylistBrowser *PlaylistBrowser::s_instance = 0;
 
 
@@ -202,24 +231,8 @@
         loadLastfmStreams( subscriber );
     }
 
-    if( Amarok::dynamicMode() )
-    {
-        QStringList playlists = Amarok::dynamicMode()->items();
+    markDynamicEntries();
 
-        for( uint i=0; i < playlists.count(); i++ )
-        {
-            QListViewItem *item = m_listview->findItem( playlists[i], 0, Qt::ExactMatch );
-            if( item )
-            {
-                m_dynamicEntries.append( item );
-                if( item->rtti() == PlaylistEntry::RTTI )
-                     static_cast<PlaylistEntry*>( item )->setDynamic( true );
-                if( item->rtti() == SmartPlaylist::RTTI )
-                     static_cast<SmartPlaylist*>( item )->setDynamic( true );
-            }
-        }
-    }
-
     // ListView item state restoration:
     // First we check if the number of items in the listview is the same as it was on last
     // application exit. If true, we iterate over all items and restore their open/closed state.
@@ -296,6 +309,29 @@
 }
 
 
+void
+PlaylistBrowser::markDynamicEntries()
+{
+    if( Amarok::dynamicMode() )
+    {
+        QStringList playlists = Amarok::dynamicMode()->items();
+
+        for( uint i=0; i < playlists.count(); i++ )
+        {
+            PlaylistBrowserEntry *item = dynamic_cast<PlaylistBrowserEntry*>( Amarok::findItemByPath( m_listview, playlists[i]  ) );
+
+            if( item )
+            {
+                m_dynamicEntries.append( item );
+                if( item->rtti() == PlaylistEntry::RTTI )
+                     static_cast<PlaylistEntry*>( item )->setDynamic( true );
+                if( item->rtti() == SmartPlaylist::RTTI )
+                     static_cast<SmartPlaylist*>( item )->setDynamic( true );
+            }
+        }
+    }
+}
+
 /**
  *************************************************************************
  *  STREAMS
@@ -883,11 +919,19 @@
     }
     else {
         e = d.namedItem( "category" ).toElement();
-        if ( e.attribute("formatversion") =="1.1" ) {
+        QString version = e.attribute("formatversion");
+        if ( version == "1.2" ) {
             PlaylistCategory* p = new PlaylistCategory( m_listview, after , e );
             p->setText( 0, i18n("Dynamic Playlists") );
             return p;
         }
+        else if ( version == "1.1" ) {
+            // In 1.1, playlists would be refered only by its name.
+            // TODO: We can *try* to convert by using findItem
+            PlaylistCategory* p = new PlaylistCategory( m_listview, after , QDomElement() );
+            p->setText( 0, i18n("Dynamic Playlists") );
+            return p;
+        }
         else { // Old unversioned format
             PlaylistCategory* p = new PlaylistCategory( m_listview, after, i18n("Dynamic Playlists") );
             QListViewItem *last = 0;
@@ -917,7 +961,7 @@
     QDomElement dynamicB = m_dynamicCategory->xml();
     dynamicB.setAttribute( "product", "Amarok" );
     dynamicB.setAttribute( "version", APP_VERSION );
-    dynamicB.setAttribute( "formatversion", "1.1" );
+    dynamicB.setAttribute( "formatversion", "1.2" );
     QDomNode dynamicsNode = doc.importNode( dynamicB, true );
     doc.appendChild( dynamicsNode );
 
@@ -944,20 +988,7 @@
     m_dynamicEntries.clear();  // Don't use remove(), since we do i++, which would cause skip overs!!!
 
     // Mark appropriate items as used
-    if( Amarok::dynamicMode() && Amarok::dynamicMode()->appendType()== DynamicMode::CUSTOM )
-    {
-        QStringList playlists = Amarok::dynamicMode()->items();
-        for( uint i=0; i < playlists.count(); i++ )
-        {
-            QListViewItem *it = findItem( playlists[i], 0 );
-
-            if( it )
-            {
-                m_dynamicEntries.append( it );
-                static_cast<PlaylistBrowserEntry*>(it)->setDynamic( true );
-            }
-        }
-    }
+    markDynamicEntries();
 }
 
 /**
@@ -1935,38 +1966,59 @@
     }
 }
 
-void PlaylistBrowser::addToDynamic()
+
+void PlaylistBrowser::addToDynamic( QListViewItem *item )
 {
+    PlaylistBrowserEntry *entry = dynamic_cast<PlaylistBrowserEntry*>( item );
+
+    if( m_dynamicEntries.find( entry ) < 0 ) // check that it is not there
+        m_dynamicEntries.append( entry );
+
+    entry->setDynamic( true );
+    // add all children as well
+    QListViewItem *it = item->firstChild();
+    while ( it ) {
+        addToDynamic( it );
+        it = it->nextSibling();
+    }
+}
+
+
+void PlaylistBrowser::addSelectedToDynamic()
+{
     QListViewItemIterator it( m_listview, QListViewItemIterator::Selected );
 
     for( ; it.current(); ++it )
-    {
-        if( m_dynamicEntries.find( *it ) < 0 ) // check that it is not there
-        {
-            m_dynamicEntries.append( *it );
-            PlaylistBrowserEntry *entry = dynamic_cast<PlaylistBrowserEntry*>( *it );
-            entry->setDynamic( true );
-        }
-    }
+        addToDynamic( *it );
 
     DynamicMode *m = Playlist::instance()->modifyDynamicMode();
     m->setDynamicItems(m_dynamicEntries);
     Playlist::instance()->finishedModifying( m );
 }
 
-void PlaylistBrowser::subFromDynamic()
+
+void PlaylistBrowser::subFromDynamic( QListViewItem *item )
 {
+    PlaylistBrowserEntry *entry = dynamic_cast<PlaylistBrowserEntry*>( item );
+
+    if( m_dynamicEntries.find( entry ) >= 0 ) // check if it is already present
+        m_dynamicEntries.remove( entry );
+
+    entry->setDynamic( false );
+    // remove all children as well
+    QListViewItem *it = item->firstChild();
+    while ( it ) {
+        subFromDynamic( it );
+        it = it->nextSibling();
+    }
+}
+
+void PlaylistBrowser::subSelectedFromDynamic()
+{
     QListViewItemIterator it( m_listview, QListViewItemIterator::Selected );
 
     for( ; it.current(); ++it )
-    {
-        if( m_dynamicEntries.find( *it ) >= 0 ) // check if it is already present
-        {
-            m_dynamicEntries.remove( *it );
-            PlaylistBrowserEntry *entry = dynamic_cast<PlaylistBrowserEntry*>( *it );
-            entry->setDynamic( false );
-        }
-    }
+        subFromDynamic( *it );
 
     DynamicMode *m = Playlist::instance()->modifyDynamicMode();
     m->setDynamicItems( m_dynamicEntries );
--- trunk/extragear/multimedia/amarok/src/playlistbrowser.h #598089:598090
@@ -90,7 +90,7 @@
         QListViewItem *findItemInTree( const QString &searchstring, int c ) const;
         PodcastEpisode *findPodcastEpisode( const KURL &episode, const KURL &feed ) const;
 
-        QPtrList<QListViewItem> dynamicEntries() const { return m_dynamicEntries; }
+        QPtrList<PlaylistBrowserEntry> dynamicEntries() const { return m_dynamicEntries; }
         DynamicMode *findDynamicModeByTitle( const QString &title );
         QListViewItem *podcastCategory() const { return m_podcastCategory; }
 
@@ -116,7 +116,8 @@
 
     private slots:
         void abortPodcastQueue();
-        void addToDynamic();
+        void addSelectedToDynamic();
+        void addToDynamic( QListViewItem *item );
         void addSelectedToPlaylist( int options = -1 );
         void collectionScanDone();
         void currentItemChanged( QListViewItem * );
@@ -127,7 +128,8 @@
         void renameSelectedItem();
         void invokeItem( QListViewItem*, const QPoint &, int column );
         void slotDoubleClicked( QListViewItem *item );
-        void subFromDynamic();
+        void subSelectedFromDynamic();
+        void subFromDynamic( QListViewItem *item );
 
         void slotAddMenu( int id );
         void showContextMenu( QListViewItem*, const QPoint&, int );
@@ -172,6 +174,9 @@
         void savePodcastFolderStates( PlaylistCategory *folder );
         PodcastChannel *findPodcastChannel( const KURL &feed, QListViewItem *parent=0 ) const;
 
+        void markDynamicEntries();
+        PlaylistBrowserEntry* findByName( QString name );
+
         PlaylistCategory* loadPlaylists();
         void savePlaylists();
         void savePlaylist( PlaylistEntry * );
@@ -215,7 +220,7 @@
         QValueList<int>      m_dynamicSizeSave;
 
         QDict<PodcastSettings>   m_podcastSettings;
-        QPtrList<QListViewItem>  m_dynamicEntries;
+        QPtrList<PlaylistBrowserEntry>  m_dynamicEntries;
 
         QTimer                  *m_podcastTimer;
         int                      m_podcastTimerInterval;        //in ms
--- trunk/extragear/multimedia/amarok/src/playlistbrowseritem.cpp #598089:598090
@@ -159,6 +159,18 @@
     setRenameEnabled( 0, false );
 }
 
+QString
+PlaylistBrowserEntry::name() const
+{
+    QString fullName = text(0);
+    QListViewItem *p = parent();
+    while ( p ) {
+        fullName.prepend( p->text(0) + '/' );
+        p = p->parent();
+    }
+    return fullName;
+}
+
 /////////////////////////////////////////////////////////////////////////////
 ///    CLASS PlaylistCategory
 ////////////////////////////////////////////////////////////////////////////
@@ -891,10 +903,10 @@
             PlaylistBrowser::instance()->addSelectedToPlaylist();
             break;
         case DYNADD:
-            PlaylistBrowser::instance()->addToDynamic();
+            PlaylistBrowser::instance()->addSelectedToDynamic();
             break;
         case DYNSUB:
-            PlaylistBrowser::instance()->subFromDynamic();
+            PlaylistBrowser::instance()->subSelectedFromDynamic();
             break;
         case RENAME:
             PlaylistBrowser::instance()->renameSelectedItem();
@@ -3389,13 +3401,10 @@
 
 void SmartPlaylist::setDynamic( bool enable )
 {
-    if( enable != m_dynamic )
-    {
-        enable ?
-            setPixmap( 0, SmallIcon( "favorites" ) ) :
-            setPixmap( 0, SmallIcon( Amarok::icon( "playlist" ) ) );
-        m_dynamic = enable;
-    }
+    enable ?
+        setPixmap( 0, SmallIcon( "favorites" ) ) :
+        setPixmap( 0, SmallIcon( Amarok::icon( "playlist" ) ) );
+    m_dynamic = enable;
 }
 
 
@@ -3450,10 +3459,10 @@
             Playlist::instance()->insertMediaSql( query(), Playlist::Append );
             break;
         case DYNADD:
-            PlaylistBrowser::instance()->addToDynamic();
+            PlaylistBrowser::instance()->addSelectedToDynamic();
             break;
         case DYNSUB:
-            PlaylistBrowser::instance()->subFromDynamic();
+            PlaylistBrowser::instance()->subSelectedFromDynamic();
             break;
         case EDIT:
             PlaylistBrowser::instance()->editSmartPlaylist( this );
--- trunk/extragear/multimedia/amarok/src/playlistbrowseritem.h #598089:598090
@@ -64,6 +64,8 @@
         virtual void updateInfo();
         virtual void setDynamic( bool ) {};
 
+        virtual QString name() const;
+
     public slots:
         virtual void slotDoubleClicked();
         virtual void slotRenameItem();
--- trunk/extragear/multimedia/amarok/src/playlistselection.cpp #598089:598090
@@ -114,7 +114,8 @@
             QStringList items = mode->items();
             foreach( items )
             {
-                QCheckListItem* current = static_cast<QCheckListItem*>( nd->selectPlaylist->findItem((*it),0) );
+                QCheckListItem* current = dynamic_cast<QCheckListItem*>(
+                                            Amarok::findItemByPath(nd->selectPlaylist, (*it)) );
                 if( current )
                     current->setOn(true);
             }
@@ -161,7 +162,8 @@
 
         while( it.current() )
         {
-            list.append( it.current()->text(0) );
+            SelectionListItem *item = static_cast<SelectionListItem*>(it.current());
+            list.append( item->name() );
             ++it;
         }
         saveMe->setItems( list );
@@ -204,4 +206,17 @@
         it = it->nextSibling();
     }
 }
+
+QString
+SelectionListItem::name() const
+{
+    QString fullName = text(0);
+    QListViewItem *p = parent();
+    while ( p ) {
+        fullName.prepend( p->text(0) + "/" );
+        p = p->parent();
+    }
+    return fullName;
+}
+
 #include "playlistselection.moc"
--- trunk/extragear/multimedia/amarok/src/playlistselection.h #598089:598090
@@ -47,6 +47,8 @@
         SelectionListItem( QListViewItem  * parent, const QString& text, QListViewItem* browserEquivalent );
         SelectionListItem( QCheckListItem * parent, const QString& text, QListViewItem* browserEquivalent );
 
+        virtual QString name() const;
+
     protected:
         virtual void stateChange( bool );
 

SVN commit 616494 by aoliveira:

Don't allow 2 smart playlists with the same name on the same folder.
It seems this solves the dynamic playlist getting wrong smart playlists problem.
BUG: 113356


 M  +18 -0     playlistbrowser.cpp  


--- trunk/extragear/multimedia/amarok/src/playlistbrowser.cpp #616493:616494
@@ -645,8 +645,26 @@
 
     if( !parent ) parent = static_cast<QListViewItem*>(m_smartCategory);
 
+
     SmartPlaylistEditor dialog( i18n("Untitled"), this );
     if( dialog.exec() == QDialog::Accepted ) {
+
+        PlaylistCategory *category = dynamic_cast<PlaylistCategory*>(parent);
+        for( QListViewItem *item = category->firstChild(); item; item = item->nextSibling() ) {
+            SmartPlaylist *sp = dynamic_cast<SmartPlaylist*>(item);
+            if ( sp && sp->title() == dialog.name() ) {
+                if( KMessageBox::warningContinueCancel(
+                    PlaylistWindow::self(),
+                    i18n( "A Smart Playlist named \"%1\" already exists. Do you want to overwrite it?" ).arg( dialog.name() ),
+                    i18n( "Overwrite Playlist?" ), i18n( "Overwrite" ) ) == KMessageBox::Continue )
+                {
+                    delete item;
+                    break;
+                }
+                else
+                    return;
+            }
+        }
         new SmartPlaylist( parent, 0, dialog.result() );
         parent->sortChildItems( 0, true );
         parent->setOpen( true );