DlgTagFetcher new feedback system

src/library/dlgtagfetcher.cpp

@@ -10,6 +10,36 @@
 
 namespace {
 
+// Constant percentages for QProgressBar.
+// There are 3 constant steps while fetching metadata from musicbrainz.
+// 1. -> "Fingerprinting track"
+// 2. -> "Identifying track through Acoustid"
+// 3. -> "Retrieving metadata from MusicBrainz
+// These steps never change and they are called "ConstantTask(s)" on here.
+// After these steps are passed, last step remains.
+// 4. -> "Fetching track data from the Musicbrainz database"
+// These step can be changed according to recording(s) fetched from Acoust ID
+
+// In order to have a better scaling, constant steps set to a percentage and never changes.
+// Last step is set to a constant percentage and it is divided to recording(s) found from Acoust ID.
+
+// Due to various recording(s) can be fetched from Acoust ID
+// The dialog populates with the 0%.
+// After each constant step progress bar value increases by 15%.
+// Before we get recording(s) from AcoustID, the QProgressBar will be 45%.
+// Last step set to 55% and it is going to be divided to recording(s) received from Acoust ID.
+
+constexpr int kPercentOfConstantTask = 15;
+
+constexpr int kPercentLeftForRecordingsFound = 55;
+
+constexpr int kMaximumValueOfQProgressBar = 100;
+
+constexpr int kMinimumValueOfQProgressBar = 0;
+
+// Original Index of the track tag, listed all the time below 'Original Tags'.
+constexpr int kOriginalTrackIndex = -1;
+
 QStringList trackColumnValues(
         const Track& track) {
     const mixxx::TrackMetadata trackMetadata = track.getMetadata();
@@ -62,8 +92,7 @@ DlgTagFetcher::DlgTagFetcher(
         // style which can make it unreadable. Bug #673411
         : QDialog(nullptr),
           m_pTrackModel(pTrackModel),
-          m_tagFetcher(this),
-          m_networkResult(NetworkResult::Ok) {
+          m_tagFetcher(this) {
     init();
 }
 
@@ -80,11 +109,22 @@ void DlgTagFetcher::init() {
     }
     connect(btnApply, &QPushButton::clicked, this, &DlgTagFetcher::apply);
     connect(btnQuit, &QPushButton::clicked, this, &DlgTagFetcher::quit);
+    connect(btnRetry, &QPushButton::clicked, this, &DlgTagFetcher::retry);
     connect(results, &QTreeWidget::currentItemChanged, this, &DlgTagFetcher::resultSelected);
 
     connect(&m_tagFetcher, &TagFetcher::resultAvailable, this, &DlgTagFetcher::fetchTagFinished);
     connect(&m_tagFetcher, &TagFetcher::fetchProgress, this, &DlgTagFetcher::fetchTagProgress);
+    connect(&m_tagFetcher,
+            &TagFetcher::currentRecordingFetchedFromMusicBrainz,
+            this,
+            &DlgTagFetcher::increaseValueOfProgressBar);
+    connect(&m_tagFetcher,
+            &TagFetcher::numberOfRecordingsFoundFromAcoustId,
+            this,
+            &DlgTagFetcher::setPercentOfEachRecordings);
     connect(&m_tagFetcher, &TagFetcher::networkError, this, &DlgTagFetcher::slotNetworkResult);
+
+    loadingProgressBar->setMaximum(kMaximumValueOfQProgressBar);
 }
 
 void DlgTagFetcher::slotNext() {
@@ -117,16 +157,22 @@ void DlgTagFetcher::loadTrackInternal(const TrackPointer& track) {
 
     m_track = track;
     m_data = Data();
-    m_networkResult = NetworkResult::Ok;
+
+    btnRetry->setDisabled(true);
+    btnApply->setDisabled(true);
+    successMessage->setVisible(false);
+    loadingProgressBar->setVisible(true);
+    loadingProgressBar->setValue(kMinimumValueOfQProgressBar);
+    results->clear();
+    addDivider(tr("Original tags"), results);
+    addTrack(trackColumnValues(*m_track), kOriginalTrackIndex, results);
 
     connect(m_track.get(),
             &Track::changed,
             this,
             &DlgTagFetcher::slotTrackChanged);
 
     m_tagFetcher.startFetch(m_track);
-
-    updateStack();
 }
 
 void DlgTagFetcher::loadTrack(const TrackPointer& track) {
@@ -147,7 +193,40 @@ void DlgTagFetcher::loadTrack(const QModelIndex& index) {
 
 void DlgTagFetcher::slotTrackChanged(TrackId trackId) {
     if (m_track && m_track->getId() == trackId) {
-        updateStack();
+        // We clear the results to update the Original Tag.
+        // It is listed with the guessedTrackReleases
+        // Without resetting the Data()
+        // Clearing the results and adding them will only update the original tag.
+        // If the track metadata listed in somewhere else such as QLabel.
+        // We wouldn't need to clear the results, only updating the label would be enough.
+        results->clear();
+        addDivider(tr("Original tags"), results);
+        addTrack(trackColumnValues(*m_track), kOriginalTrackIndex, results);
+
+        addDivider(tr("Suggested tags"), results);
+        {
+            int trackIndex = 0;
+            QSet<QStringList> allColumnValues; // deduplication
+            for (const auto& trackRelease : qAsConst(m_data.m_results)) {
+                const auto columnValues = trackReleaseColumnValues(trackRelease);
+                // Ignore duplicate results
+                if (!allColumnValues.contains(columnValues)) {
+                    allColumnValues.insert(columnValues);
+                    addTrack(columnValues, trackIndex, results);
+                }
+                ++trackIndex;
+            }
+        }
+
+        // Find the item that was selected last time
+        for (int i = 0; i < results->model()->rowCount(); ++i) {
+            const QModelIndex index = results->model()->index(i, 0);
+            const QVariant id = index.data(Qt::UserRole);
+            if (!id.isNull() && id.toInt() == m_data.m_selectedResult) {
+                results->setCurrentIndex(index);
+                break;
+            }
+        }
     }
 }
 
@@ -222,6 +301,13 @@ void DlgTagFetcher::apply() {
             QDateTime::currentDateTimeUtc());
 }
 
+void DlgTagFetcher::retry() {
+    btnRetry->setDisabled(true);
+    btnApply->setDisabled(true);
+    loadingProgressBar->setValue(kMinimumValueOfQProgressBar);
+    m_tagFetcher.startFetch(m_track);
+}
+
 void DlgTagFetcher::quit() {
     m_tagFetcher.cancel();
     accept();
@@ -233,8 +319,20 @@ void DlgTagFetcher::reject() {
 }
 
 void DlgTagFetcher::fetchTagProgress(const QString& text) {
-    QString status = tr("Status: %1");
-    loadingStatus->setText(status.arg(text));
+    QString status = tr("%1");
+    loadingProgressBar->setFormat(status.arg(text));
+    loadingProgressBar->setValue(loadingProgressBar->value() + kPercentOfConstantTask);
+}
+
+void DlgTagFetcher::increaseValueOfProgressBar() {
+    QString status = tr("Fetching track data from the MusicBrainz database");
+    loadingProgressBar->setFormat(status);
+    loadingProgressBar->setValue(loadingProgressBar->value() + m_incrementBarValueBy);
+}
+
+// TODO(fatihemreyildiz): display the task results one by one.
+void DlgTagFetcher::setPercentOfEachRecordings(int totalRecordingsFound) {
+    m_incrementBarValueBy = kPercentLeftForRecordingsFound / totalRecordingsFound;
 }
 
 void DlgTagFetcher::fetchTagFinished(
@@ -243,82 +341,64 @@ void DlgTagFetcher::fetchTagFinished(
     VERIFY_OR_DEBUG_ASSERT(pTrack == m_track) {
         return;
     }
-    m_data.m_pending = false;
     m_data.m_results = guessedTrackReleases;
-    // qDebug() << "number of results = " << guessedTrackReleases.size();
-    updateStack();
-}
-
-void DlgTagFetcher::slotNetworkResult(
-        int httpError, const QString& app, const QString& message, int code) {
-    m_networkResult = httpError == 0 ? NetworkResult::UnknownError : NetworkResult::HttpError;
-    m_data.m_pending = false;
-    QString strError = tr("HTTP Status: %1");
-    QString strCode = tr("Code: %1");
-    httpStatus->setText(strError.arg(httpError) + "\n" + strCode.arg(code) + "\n" + message);
-    QString unknownError = tr("Mixxx can't connect to %1 for an unknown reason.");
-    cantConnectMessage->setText(unknownError.arg(app));
-    QString cantConnect = tr("Mixxx can't connect to %1.");
-    cantConnectHttp->setText(cantConnect.arg(app));
-    updateStack();
-}
-
-void DlgTagFetcher::updateStack() {
-    if (m_data.m_pending) {
-        stack->setCurrentWidget(loading_page);
-        return;
-    } else if (m_networkResult == NetworkResult::HttpError) {
-        stack->setCurrentWidget(networkError_page);
+    if (guessedTrackReleases.size() == 0) {
+        loadingProgressBar->setValue(kMaximumValueOfQProgressBar);
+        QString emptyMessage = tr("Could not find this track in the MusicBrainz database.");
+        loadingProgressBar->setFormat(emptyMessage);
         return;
-    } else if (m_networkResult == NetworkResult::UnknownError) {
-        stack->setCurrentWidget(generalnetworkError_page);
-        return;
-    } else if (m_data.m_results.isEmpty()) {
-        stack->setCurrentWidget(error_page);
-        return;
-    }
-    btnApply->setEnabled(true);
-    stack->setCurrentWidget(results_page);
+    } else {
+        btnApply->setEnabled(true);
+        btnRetry->setEnabled(false);
 
-    results->clear();
+        loadingProgressBar->setValue(kMaximumValueOfQProgressBar);
+        loadingProgressBar->setVisible(false);
+        successMessage->setVisible(true);
 
-    VERIFY_OR_DEBUG_ASSERT(m_track) {
-        return;
-    }
+        VERIFY_OR_DEBUG_ASSERT(m_track) {
+            return;
+        }
 
-    addDivider(tr("Original tags"), results);
-    addTrack(trackColumnValues(*m_track), -1, results);
-
-    addDivider(tr("Suggested tags"), results);
-    {
-        int trackIndex = 0;
-        QSet<QStringList> allColumnValues; // deduplication
-        for (const auto& trackRelease : qAsConst(m_data.m_results)) {
-            const auto columnValues = trackReleaseColumnValues(trackRelease);
-            // Ignore duplicate results
-            if (!allColumnValues.contains(columnValues)) {
-                allColumnValues.insert(columnValues);
-                addTrack(columnValues, trackIndex, results);
+        addDivider(tr("Suggested tags"), results);
+        {
+            int trackIndex = 0;
+            QSet<QStringList> allColumnValues; // deduplication
+            for (const auto& trackRelease : qAsConst(m_data.m_results)) {
+                const auto columnValues = trackReleaseColumnValues(trackRelease);
+                // Ignore duplicate results
+                if (!allColumnValues.contains(columnValues)) {
+                    allColumnValues.insert(columnValues);
+                    addTrack(columnValues, trackIndex, results);
+                }
+                ++trackIndex;
             }
-            ++trackIndex;
         }
-    }
 
-    for (int i = 0; i < results->model()->columnCount(); i++) {
-        results->resizeColumnToContents(i);
-        int sectionSize = (results->columnWidth(i) + 10);
-        results->header()->resizeSection(i, sectionSize);
-    }
-
-    // Find the item that was selected last time
-    for (int i = 0; i < results->model()->rowCount(); ++i) {
-        const QModelIndex index = results->model()->index(i, 0);
-        const QVariant id = index.data(Qt::UserRole);
-        if (!id.isNull() && id.toInt() == m_data.m_selectedResult) {
-            results->setCurrentIndex(index);
-            break;
+        for (int i = 0; i < results->model()->columnCount(); i++) {
+            results->resizeColumnToContents(i);
+            int sectionSize = (results->columnWidth(i) + 10);
+            results->header()->resizeSection(i, sectionSize);
         }
     }
+
+    // qDebug() << "number of results = " << guessedTrackReleases.size();
+}
+
+void DlgTagFetcher::slotNetworkResult(
+        int httpError,
+        const QString& app,
+        const QString& message,
+        int code) {
+    QString cantConnect = tr("Can't connect to %1: %2");
+    loadingProgressBar->setFormat(cantConnect.arg(app, message));
+    qWarning() << "Error while getting track metadata!"
+               << "Service:" << app
+               << "HTTP Status:" << httpError
+               << "Code:" << code
+               << "Server message:" << message;
+    btnRetry->setEnabled(true);
+    loadingProgressBar->setValue(kMaximumValueOfQProgressBar);
+    return;
 }
 
 void DlgTagFetcher::addDivider(const QString& text, QTreeWidget* parent) const {
@@ -338,6 +418,10 @@ void DlgTagFetcher::resultSelected() {
         return;
     }
 
+    if (results->currentItem()->data(0, Qt::UserRole).toInt() == kOriginalTrackIndex) {
+        results->currentItem()->setFlags(Qt::ItemIsEnabled);
+        return;
+    }
     const int resultIndex = results->currentItem()->data(0, Qt::UserRole).toInt();
     m_data.m_selectedResult = resultIndex;
 }

src/library/dlgtagfetcher.h

@@ -38,17 +38,20 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher {
             const QList<mixxx::musicbrainz::TrackRelease>& guessedTrackReleases);
     void resultSelected();
     void fetchTagProgress(const QString&);
+    void setPercentOfEachRecordings(int totalRecordingsFound);
+    void increaseValueOfProgressBar();
     void slotNetworkResult(int httpStatus, const QString& app, const QString& message, int code);
     void slotTrackChanged(TrackId trackId);
     void apply();
+    void retry();
     void quit();
     void reject() override;
     void slotNext();
     void slotPrev();
 
   private:
     void loadTrackInternal(const TrackPointer& track);
-    void updateStack();
+    void resetStack();
     void addDivider(const QString& text, QTreeWidget* parent) const;
 
     const TrackModel* const m_pTrackModel;
@@ -59,22 +62,14 @@ class DlgTagFetcher : public QDialog, public Ui::DlgTagFetcher {
 
     QModelIndex m_currentTrackIndex;
 
+    int m_incrementBarValueBy;
+
     struct Data {
         Data()
-                : m_pending(true),
-                  m_selectedResult(-1) {
+                : m_selectedResult(-1) {
         }
-
-        bool m_pending;
         int m_selectedResult;
         QList<mixxx::musicbrainz::TrackRelease> m_results;
     };
     Data m_data;
-
-    enum class NetworkResult {
-        Ok,
-        HttpError,
-        UnknownError,
-    };
-    NetworkResult m_networkResult;
 };