hide Tracks: make removal from playlists and crates optional

src/coreservices.cpp

@@ -268,6 +268,9 @@ void CoreServices::initialize(QApplication* pApp) {
     }
     // Create a connection for the main thread
     m_pDbConnectionPool->createThreadLocalConnection();
+    // TODO Check if we upgrade from version before Mixxx 2.1
+    // If yes, note that and remove hidden tracks from playlists. This can happen
+    // in Library init or in PlaylistTableModel::selectPlaylist()
     if (!initializeDatabase()) {
         exit(-1);
     }

src/library/library.cpp

@@ -102,6 +102,7 @@ Library::Library(
 
     m_pPlaylistFeature = new PlaylistFeature(this, UserSettingsPointer(m_pConfig));
     addFeature(m_pPlaylistFeature);
+    // TODO Remove hidden tracks from playlists here when upgrading from Mixxx < 2.1?
 
     m_pCrateFeature = new CrateFeature(this, m_pConfig);
     addFeature(m_pCrateFeature);

src/library/playlisttablemodel.cpp

@@ -15,11 +15,9 @@ const QString kModelName = "playlist:";
 
 PlaylistTableModel::PlaylistTableModel(QObject* parent,
         TrackCollectionManager* pTrackCollectionManager,
-        const char* settingsNamespace,
-        bool keepHiddenTracks)
+        const char* settingsNamespace)
         : TrackSetTableModel(parent, pTrackCollectionManager, settingsNamespace),
-          m_iPlaylistId(kInvalidPlaylistId),
-          m_keepHiddenTracks(keepHiddenTracks) {
+          m_iPlaylistId(kInvalidPlaylistId) {
     connect(&m_pTrackCollectionManager->internalCollection()->getPlaylistDAO(),
             &PlaylistDAO::tracksAdded,
             this,
@@ -147,14 +145,6 @@ void PlaylistTableModel::selectPlaylist(int playlistId) {
 
     m_iPlaylistId = playlistId;
 
-    if (!m_keepHiddenTracks) {
-        // From Mixxx 2.1 we drop tracks that have been explicitly deleted
-        // in the library (mixxx_deleted = 0) from playlists.
-        // These invisible tracks, consuming a playlist position number were
-        // a source user of confusion in the past.
-        m_pTrackCollectionManager->internalCollection()->getPlaylistDAO().removeHiddenTracks(m_iPlaylistId);
-    }
-
     QString playlistTableName = "playlist_" + QString::number(m_iPlaylistId);
     QSqlQuery query(m_database);
     FieldEscaper escaper(m_database);

src/library/playlisttablemodel.h

@@ -9,8 +9,7 @@ class PlaylistTableModel final : public TrackSetTableModel {
   public:
     PlaylistTableModel(QObject* parent,
             TrackCollectionManager* pTrackCollectionManager,
-            const char* settingsNamespace,
-            bool keepHiddenTracks = false);
+            const char* settingsNamespace);
     ~PlaylistTableModel() final = default;
 
     void selectPlaylist(int playlistId = -1 /* kInvalidPlaylistId */);
@@ -51,6 +50,5 @@ class PlaylistTableModel final : public TrackSetTableModel {
     void initSortColumnMapping() override;
 
     int m_iPlaylistId;
-    bool m_keepHiddenTracks;
     QHash<int, QString> m_searchTexts;
 };

src/library/trackcollection.cpp

@@ -274,7 +274,8 @@ QList<TrackId> TrackCollection::resolveTrackIdsFromLocations(
 bool TrackCollection::hideTracks(const QList<TrackId>& trackIds) {
     DEBUG_ASSERT_QOBJECT_THREAD_AFFINITY(this);
 
-    // Warn if tracks have a playlist membership
+    // Warn if tracks are in a playlist.
+    // Always keep them in History playlists.
     QSet<int> allPlaylistIds;
     for (const auto& trackId: trackIds) {
         QSet<int> playlistIds;
@@ -286,28 +287,28 @@ bool TrackCollection::hideTracks(const QList<TrackId>& trackIds) {
         }
     }
 
+    bool removeFromplaylists = false;
     if (!allPlaylistIds.isEmpty()) {
          QStringList playlistNames;
          playlistNames.reserve(allPlaylistIds.count());
          for (const auto& playlistId: allPlaylistIds) {
              playlistNames.append(m_playlistDao.getPlaylistName(playlistId));
          }
 
-         QString playlistNamesSection =
-                 "\n\n\"" %
+         const QString playlistNamesSection =
+                 "\n\n\"" % // prepend linebreak
                  playlistNames.join("\"\n\"") %
-                 "\"\n\n";
+                 "\"\n\n"; // append linebreak
 
-         if (QMessageBox::question(
-                 nullptr,
+         QMessageBox::StandardButton btn = QMessageBox::question(nullptr,
                  tr("Hiding tracks"),
                  tr("The selected tracks are in the following playlists:"
-                     "%1"
-                     "Hiding them will remove them from these playlists. Continue?")
+                    "%1"
+                    "Do you want to remove them from these playlists. Continue?")
                          .arg(playlistNamesSection),
-                 QMessageBox::Ok | QMessageBox::Cancel) != QMessageBox::Ok) {
-             return false;
-         }
+                 QMessageBox::Yes | QMessageBox::No,
+                 QMessageBox::No);
+         removeFromplaylists = btn == QMessageBox::Yes;
      }
 
     // Transactional
@@ -322,7 +323,9 @@ bool TrackCollection::hideTracks(const QList<TrackId>& trackIds) {
         return false;
     }
 
-    m_playlistDao.removeTracksFromPlaylists(trackIds);
+    if (removeFromplaylists) {
+        m_playlistDao.removeTracksFromPlaylists(trackIds);
+    }
 
     // Post-processing
     // TODO(XXX): Move signals from TrackDAO to TrackCollection

src/library/trackset/baseplaylistfeature.cpp

@@ -42,15 +42,13 @@
         PlaylistTableModel* pModel,
         const QString& rootViewName,
         const QString& iconName,
-        const QString& countsDurationTableName,
-        bool keepHiddenTracks)
+        const QString& countsDurationTableName)
         : BaseTrackSetFeature(pLibrary, pConfig, rootViewName, iconName),
           m_playlistDao(pLibrary->trackCollectionManager()
-                                ->internalCollection()
-                                ->getPlaylistDAO()),
+                          ->internalCollection()
+                          ->getPlaylistDAO()),
           m_pPlaylistTableModel(pModel),
-          m_countsDurationTableName(countsDurationTableName),
-          m_keepHiddenTracks(keepHiddenTracks) {
+          m_countsDurationTableName(countsDurationTableName) {
     pModel->setParent(this);
 
     initActions();
@@ -575,7 +573,7 @@
             std::make_unique<PlaylistTableModel>(this,
                     m_pLibrary->trackCollectionManager(),
                     "mixxx.db.model.playlist_export",
                     m_keepHiddenTracks);
 
     emit saveModelState();
     pPlaylistTableModel->selectPlaylist(playlistId);

src/library/trackset/baseplaylistfeature.h

@@ -26,8 +26,7 @@ class BasePlaylistFeature : public BaseTrackSetFeature {
             PlaylistTableModel* pModel,
             const QString& rootViewName,
             const QString& iconName,
-            const QString& countsDurationTableName,
-            bool keepHiddenTracks = false);
+            const QString& countsDurationTableName);
     ~BasePlaylistFeature() override = default;
 
     TreeItemModel* sidebarModel() const override;
@@ -130,7 +129,4 @@ class BasePlaylistFeature : public BaseTrackSetFeature {
     virtual QString getRootViewHtml() const = 0;
     void markTreeItem(TreeItem* pTreeItem);
     QString fetchPlaylistLabel(int playlistId);
-
-
-    const bool m_keepHiddenTracks;
 };