BaseTrackTableModel: Fix column header mapping

src/library/analysis/dlganalysis.cpp

@@ -137,9 +137,8 @@ void DlgAnalysis::analyze() {
 
         QModelIndexList selectedIndexes = m_pAnalysisLibraryTableView->selectionModel()->selectedRows();
         foreach(QModelIndex selectedIndex, selectedIndexes) {
-            TrackId trackId(selectedIndex.sibling(
-                selectedIndex.row(),
-                m_pAnalysisLibraryTableModel->fieldIndex(LIBRARYTABLE_ID)).data());
+            TrackId trackId(m_pAnalysisLibraryTableModel->getFieldVariant(
+                    selectedIndex, ColumnCache::COLUMN_LIBRARYTABLE_ID));
             if (trackId.isValid()) {
                 tracks.append(trackId);
             }

src/library/banshee/bansheeplaylistmodel.cpp

@@ -237,7 +237,8 @@ void BansheePlaylistModel::selectPlaylist(int playlistId) {
 
     setTable(m_tempTableName, idColumn, std::move(tableColumns), trackSource);
     setSearch("");
-    setDefaultSort(fieldIndex(PLAYLISTTRACKSTABLE_POSITION), Qt::AscendingOrder);
+    setDefaultSort(fieldIndex(ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION),
+            Qt::AscendingOrder);
     setSort(defaultSortColumn(), defaultSortOrder());
 }
 
@@ -264,16 +265,6 @@ TrackId BansheePlaylistModel::doGetTrackId(const TrackPointer& pTrack) const {
     return TrackId();
 }
 
-QVariant BansheePlaylistModel::getFieldVariant(const QModelIndex& index,
-        const QString& fieldName) const {
-    return index.sibling(index.row(), fieldIndex(fieldName)).data();
-}
-
-QString BansheePlaylistModel::getFieldString(const QModelIndex& index,
-        const QString& fieldName) const {
-    return getFieldVariant(index, fieldName).toString();
-}
-
 TrackPointer BansheePlaylistModel::getTrack(const QModelIndex& index) const {
     QString location = getTrackLocation(index);
 

src/library/banshee/bansheeplaylistmodel.h

@@ -25,9 +25,6 @@ class BansheePlaylistModel final : public BaseSqlTableModel {
 
   private:
     TrackId doGetTrackId(const TrackPointer& pTrack) const final;
-
-    QString getFieldString(const QModelIndex& index, const QString& fieldName) const;
-    QVariant getFieldVariant(const QModelIndex& index, const QString& fieldName) const;
     void dropTempTable();
 
     BansheeDbConnection* m_pConnection;

src/library/baseexternalplaylistmodel.cpp

@@ -32,7 +32,8 @@ BaseExternalPlaylistModel::~BaseExternalPlaylistModel() {
 }
 
 TrackPointer BaseExternalPlaylistModel::getTrack(const QModelIndex& index) const {
-    QString nativeLocation = index.sibling(index.row(), fieldIndex("location")).data().toString();
+    QString nativeLocation = getFieldString(
+            index, ColumnCache::COLUMN_TRACKLOCATIONSTABLE_LOCATION);
     QString location = QDir::fromNativeSeparators(nativeLocation);
 
     if (location.isEmpty()) {
@@ -49,23 +50,22 @@ TrackPointer BaseExternalPlaylistModel::getTrack(const QModelIndex& index) const
     // saved with the metadata from iTunes. If it was already in the library
     // then we do not touch it so that we do not over-write the user's metadata.
     if (pTrack && !track_already_in_library) {
-        QString artist = index.sibling(index.row(), fieldIndex("artist")).data().toString();
+        QString artist = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_ARTIST);
         pTrack->setArtist(artist);
 
-        QString title = index.sibling(index.row(), fieldIndex("title")).data().toString();
+        QString title = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_TITLE);
         pTrack->setTitle(title);
 
-        QString album = index.sibling(index.row(), fieldIndex("album")).data().toString();
+        QString album = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_ALBUM);
         pTrack->setAlbum(album);
 
-        QString year = index.sibling(index.row(), fieldIndex("year")).data().toString();
+        QString year = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_YEAR);
         pTrack->setYear(year);
 
-        QString genre = index.sibling(index.row(), fieldIndex("genre")).data().toString();
+        QString genre = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_GENRE);
         updateTrackGenre(pTrack.get(), genre);
 
-        float bpm = index.sibling(
-                index.row(), fieldIndex("bpm")).data().toString().toFloat();
+        float bpm = getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_BPM).toFloat();
         pTrack->trySetBpm(bpm);
     }
     return pTrack;
@@ -135,7 +135,7 @@ void BaseExternalPlaylistModel::setPlaylistById(int playlistId) {
                             playlistIdNumber);
     // The ordering of columns is relevant (see below)!
     auto playlistViewColumns = QStringList{
-            QStringLiteral("track_id"),
+            PLAYLISTTRACKSTABLE_TRACKID,
             PLAYLISTTRACKSTABLE_POSITION,
             QStringLiteral("'' AS ") + LIBRARYTABLE_PREVIEW};
     const auto queryString =
@@ -170,7 +170,8 @@ TrackId BaseExternalPlaylistModel::doGetTrackId(const TrackPointer& pTrack) cons
         // The external table has foreign Track IDs, so we need to compare
         // by location
         for (int row = 0; row < rowCount(); ++row) {
-            QString nativeLocation = index(row, fieldIndex("location")).data().toString();
+            QString nativeLocation = getFieldString(index(row, 0),
+                    ColumnCache::COLUMN_TRACKLOCATIONSTABLE_LOCATION);
             QString location = QDir::fromNativeSeparators(nativeLocation);
             if (location == pTrack->getLocation()) {
                 return TrackId(index(row, 0).data());

src/library/baseexternaltrackmodel.cpp

@@ -35,21 +35,21 @@ BaseExternalTrackModel::BaseExternalTrackModel(QObject* parent,
 
     columns[1] = LIBRARYTABLE_PREVIEW;
     setTable(viewTable, columns[0], columns, trackSource);
-    setDefaultSort(fieldIndex("artist"), Qt::AscendingOrder);
+    setDefaultSort(fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_ARTIST), Qt::AscendingOrder);
 }
 
 BaseExternalTrackModel::~BaseExternalTrackModel() {
 }
 
 TrackPointer BaseExternalTrackModel::getTrack(const QModelIndex& index) const {
-    QString artist = index.sibling(index.row(), fieldIndex("artist")).data().toString();
-    QString title = index.sibling(index.row(), fieldIndex("title")).data().toString();
-    QString album = index.sibling(index.row(), fieldIndex("album")).data().toString();
-    QString year = index.sibling(index.row(), fieldIndex("year")).data().toString();
-    QString genre = index.sibling(index.row(), fieldIndex("genre")).data().toString();
-    float bpm = index.sibling(index.row(), fieldIndex("bpm")).data().toString().toFloat();
-
-    QString nativeLocation = index.sibling(index.row(), fieldIndex("location")).data().toString();
+    QString artist = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_ARTIST);
+    QString title = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_TITLE);
+    QString album = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_ALBUM);
+    QString year = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_YEAR);
+    QString genre = getFieldString(index, ColumnCache::COLUMN_LIBRARYTABLE_GENRE);
+    float bpm = getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_BPM).toFloat();
+    QString nativeLocation = getFieldString(
+            index, ColumnCache::COLUMN_TRACKLOCATIONSTABLE_LOCATION);
     QString location = QDir::fromNativeSeparators(nativeLocation);
 
     if (location.isEmpty()) {
@@ -95,7 +95,8 @@ TrackId BaseExternalTrackModel::doGetTrackId(const TrackPointer& pTrack) const {
         // The external table has foreign Track IDs, so we need to compare
         // by location
         for (int row = 0; row < rowCount(); ++row) {
-            QString nativeLocation = index(row, fieldIndex("location")).data().toString();
+            QString nativeLocation = getFieldString(index(row, 0),
+                    ColumnCache::COLUMN_TRACKLOCATIONSTABLE_LOCATION);
             QString location = QDir::fromNativeSeparators(nativeLocation);
             if (location == pTrack->getLocation()) {
                 return TrackId(index(row, 0).data());
@@ -106,7 +107,7 @@ TrackId BaseExternalTrackModel::doGetTrackId(const TrackPointer& pTrack) const {
 }
 
 bool BaseExternalTrackModel::isColumnInternal(int column) {
-    return column == fieldIndex(LIBRARYTABLE_ID) ||
+    return column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_ID) ||
             (PlayerManager::numPreviewDecks() == 0 &&
                     column == fieldIndex(ColumnCache::COLUMN_LIBRARYTABLE_PREVIEW));
 }

src/library/basesqltablemodel.cpp

@@ -51,17 +51,6 @@ BaseSqlTableModel::BaseSqlTableModel(
 BaseSqlTableModel::~BaseSqlTableModel() {
 }
 
-void BaseSqlTableModel::initHeaderProperties() {
-    BaseTrackTableModel::initHeaderProperties();
-    // Add playlist columns
-    setHeaderProperties(ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_POSITION,
-            tr("#"),
-            30);
-    setHeaderProperties(ColumnCache::COLUMN_PLAYLISTTRACKSTABLE_DATETIMEADDED,
-            tr("Timestamp"),
-            80);
-}
-
 void BaseSqlTableModel::initSortColumnMapping() {
     // Add a bijective mapping between the SortColumnIds and column indices
     for (int i = 0; i < static_cast<int>(TrackModel::SortColumnId::IdMax); ++i) {
@@ -633,13 +622,18 @@ int BaseSqlTableModel::fieldIndex(const QString& fieldName) const {
         // column or a source column.
         int sourceTableIndex = m_trackSource->fieldIndex(fieldName);
         if (sourceTableIndex > -1) {
-            // Subtract one from the fieldIndex() result to account for the id column
+            // Subtract one from the fieldIndex() because the id column is in both
             return m_tableColumns.size() + sourceTableIndex - 1;
         }
     }
     return tableIndex;
 }
 
+int BaseSqlTableModel::endFieldIndex() const {
+    // Subtract one to remove the id column which is in both
+    return m_tableColumns.size() + (m_trackSource ? m_trackSource->endFieldIndex() - 1 : 0);
+}
+
 QString BaseSqlTableModel::modelKey(bool noSearch) const {
     if (noSearch) {
         return kModelName + m_tableName;
@@ -782,7 +776,7 @@ TrackPointer BaseSqlTableModel::getTrack(const QModelIndex& index) const {
 
 TrackId BaseSqlTableModel::getTrackId(const QModelIndex& index) const {
     if (index.isValid()) {
-        return TrackId(index.sibling(index.row(), fieldIndex(m_idColumn)).data());
+        return TrackId(getFieldVariant(index, m_idColumn));
     } else {
         return TrackId();
     }
@@ -792,11 +786,8 @@ QString BaseSqlTableModel::getTrackLocation(const QModelIndex& index) const {
     if (!index.isValid()) {
         return QString();
     }
-    QString nativeLocation =
-            index.sibling(index.row(),
-                         fieldIndex(ColumnCache::COLUMN_TRACKLOCATIONSTABLE_LOCATION))
-                    .data()
-                    .toString();
+    QString nativeLocation = getFieldString(
+            index, ColumnCache::COLUMN_TRACKLOCATIONSTABLE_LOCATION);
     return QDir::fromNativeSeparators(nativeLocation);
 }
 
@@ -812,40 +803,17 @@ QUrl BaseSqlTableModel::getTrackUrl(const QModelIndex& index) const {
 CoverInfo BaseSqlTableModel::getCoverInfo(const QModelIndex& index) const {
     CoverInfo coverInfo;
     coverInfo.setImageDigest(
-            index.sibling(index.row(),
-                         fieldIndex(ColumnCache::
-                                         COLUMN_LIBRARYTABLE_COVERART_DIGEST))
-                    .data()
-                    .toByteArray(),
-            index.sibling(index.row(),
-                         fieldIndex(ColumnCache::
-                                         COLUMN_LIBRARYTABLE_COVERART_HASH))
-                    .data()
-                    .toUInt());
+            getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_COVERART_DIGEST).toByteArray(),
+            getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_COVERART_HASH).toUInt());
     coverInfo.color = mixxx::RgbColor::fromQVariant(
-            index.sibling(index.row(),
-                         fieldIndex(ColumnCache::
-                                         COLUMN_LIBRARYTABLE_COVERART_COLOR))
-                    .data());
+            getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_COVERART_COLOR));
     if (coverInfo.hasCacheKey()) {
         coverInfo.type = static_cast<CoverInfo::Type>(
-                index.sibling(index.row(),
-                             fieldIndex(ColumnCache::
-                                             COLUMN_LIBRARYTABLE_COVERART_TYPE))
-                        .data()
-                        .toInt());
+                getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_COVERART_TYPE).toInt());
         coverInfo.source = static_cast<CoverInfo::Source>(
-                index.sibling(index.row(),
-                             fieldIndex(ColumnCache::
-                                             COLUMN_LIBRARYTABLE_COVERART_SOURCE))
-                        .data()
-                        .toInt());
-        coverInfo.coverLocation =
-                index.sibling(index.row(),
-                             fieldIndex(ColumnCache::
-                                             COLUMN_LIBRARYTABLE_COVERART_LOCATION))
-                        .data()
-                        .toString();
+                getFieldVariant(index, ColumnCache::COLUMN_LIBRARYTABLE_COVERART_SOURCE).toInt());
+        coverInfo.coverLocation = getFieldString(
+                index, ColumnCache::COLUMN_LIBRARYTABLE_COVERART_LOCATION);
         coverInfo.trackLocation = getTrackLocation(index);
     }
     return coverInfo;

src/library/basesqltablemodel.h

@@ -75,6 +75,7 @@ class BaseSqlTableModel : public BaseTrackTableModel {
     ///////////////////////////////////////////////////////////////////////////
     int fieldIndex(
             ColumnCache::Column column) const final;
+    int endFieldIndex() const final;
 
     QString modelKey(bool noSearch) const override;
 
@@ -95,7 +96,7 @@ class BaseSqlTableModel : public BaseTrackTableModel {
             QString trackIdColumn,
             QStringList tableColumns,
             QSharedPointer<BaseTrackCache> trackSource);
-    void initHeaderProperties() override;
+
     virtual void initSortColumnMapping();
 
     TrackCollectionManager* const m_pTrackCollectionManager;

src/library/basetrackcache.cpp

@@ -51,6 +51,10 @@ int BaseTrackCache::fieldIndex(const QString& columnName) const {
     return m_columnCache.fieldIndex(columnName);
 }
 
+int BaseTrackCache::endFieldIndex() const {
+    return m_columnCache.endFieldIndex();
+}
+
 QString BaseTrackCache::columnNameForFieldIndex(int index) const {
     return m_columnCache.columnNameForFieldIndex(index);
 }

src/library/basetrackcache.h

@@ -64,6 +64,7 @@ class BaseTrackCache : public QObject {
     QString columnNameForFieldIndex(int index) const;
     QString columnSortForFieldIndex(int index) const;
     int fieldIndex(ColumnCache::Column column) const;
+    int endFieldIndex() const;
     virtual void filterAndSort(const QSet<TrackId>& trackIds,
                                const QString& query,
                                const QString& extraFilter,