From 87fd93a1cfafdfe213347bf615754181cb808493 Mon Sep 17 00:00:00 2001 From: Jonas Kvinge Date: Wed, 20 Feb 2019 21:27:53 +0100 Subject: [PATCH] Show error when editing tags fails, update DB immediately when successful --- src/collection/collectionview.cpp | 6 ++++ src/collection/collectionview.h | 5 ++- src/core/mainwindow.cpp | 13 ++++--- src/core/mainwindow.h | 2 +- src/core/tagreaderclient.cpp | 4 ++- src/core/tagreaderclient.h | 2 -- src/dialogs/edittagdialog.cpp | 56 +++++++++++++++++++++---------- src/dialogs/edittagdialog.h | 11 ++++-- 8 files changed, 67 insertions(+), 32 deletions(-) diff --git a/src/collection/collectionview.cpp b/src/collection/collectionview.cpp index 8fb492d1c..c17baa0b3 100644 --- a/src/collection/collectionview.cpp +++ b/src/collection/collectionview.cpp @@ -664,18 +664,24 @@ void CollectionView::Organise() { else { QMessageBox::warning(this, tr("Error"), tr("None of the selected songs were suitable for copying to a device")); } + } void CollectionView::EditTracks() { if (!edit_tag_dialog_) { edit_tag_dialog_.reset(new EditTagDialog(app_, this)); + connect(edit_tag_dialog_.get(), SIGNAL(Error(QString)), SLOT(EditTagError(QString))); } edit_tag_dialog_->SetSongs(GetSelectedSongs()); edit_tag_dialog_->show(); } +void CollectionView::EditTagError(const QString &message) { + emit Error(message); +} + void CollectionView::CopyToDevice() { #ifndef Q_OS_WIN if (!organise_dialog_) diff --git a/src/collection/collectionview.h b/src/collection/collectionview.h index e71907296..adfa3dc8b 100644 --- a/src/collection/collectionview.h +++ b/src/collection/collectionview.h @@ -98,12 +98,15 @@ class CollectionView : public AutoExpandingTreeView { void SaveFocus(); void RestoreFocus(); -signals: + void EditTagError(const QString &message); + + signals: void ShowConfigDialog(); void TotalSongCountUpdated_(); void TotalArtistCountUpdated_(); void TotalAlbumCountUpdated_(); + void Error(const QString &message); protected: // QWidget diff --git a/src/core/mainwindow.cpp b/src/core/mainwindow.cpp index 28c9b8000..4a83d9c0c 100644 --- a/src/core/mainwindow.cpp +++ b/src/core/mainwindow.cpp @@ -499,6 +499,7 @@ MainWindow::MainWindow(Application *app, SystemTrayIcon *tray_icon, OSD *osd, co // Collection connections connect(collection_view_->view(), SIGNAL(AddToPlaylistSignal(QMimeData*)), SLOT(AddToPlaylist(QMimeData*))); connect(collection_view_->view(), SIGNAL(ShowConfigDialog()), SLOT(ShowCollectionConfig())); + connect(collection_view_->view(), SIGNAL(Error(QString)), SLOT(ShowErrorDialog(QString))); connect(app_->collection_model(), SIGNAL(TotalSongCountUpdated(int)), collection_view_->view(), SLOT(TotalSongCountUpdated(int))); connect(app_->collection_model(), SIGNAL(TotalArtistCountUpdated(int)), collection_view_->view(), SLOT(TotalArtistCountUpdated(int))); connect(app_->collection_model(), SIGNAL(TotalAlbumCountUpdated(int)), collection_view_->view(), SLOT(TotalAlbumCountUpdated(int))); @@ -1581,7 +1582,6 @@ void MainWindow::EditTracks() { } } - //EnsureEditTagDialogCreated(); edit_tag_dialog_->SetSongs(songs, items); edit_tag_dialog_->show(); @@ -1625,7 +1625,7 @@ void MainWindow::RenumberTracks() { if (song.IsEditable()) { song.set_track(track); - TagReaderReply *reply =TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + TagReaderReply *reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)),reply, QPersistentModelIndex(source_index)); } @@ -1634,7 +1634,7 @@ void MainWindow::RenumberTracks() { } -void MainWindow::SongSaveComplete(TagReaderReply *reply,const QPersistentModelIndex &index) { +void MainWindow::SongSaveComplete(TagReaderReply *reply, const QPersistentModelIndex &index) { if (reply->is_successful() && index.isValid()) { app_->playlist_manager()->current()->ReloadItems(QList()<< index.row()); } @@ -1644,7 +1644,7 @@ void MainWindow::SongSaveComplete(TagReaderReply *reply,const QPersistentModelIn void MainWindow::SelectionSetValue() { Playlist::Column column = (Playlist::Column)playlist_menu_index_.column(); - QVariant column_value =app_->playlist_manager()->current()->data(playlist_menu_index_); + QVariant column_value = app_->playlist_manager()->current()->data(playlist_menu_index_); QModelIndexList indexes =ui_->playlist->view()->selectionModel()->selection().indexes(); @@ -1656,9 +1656,9 @@ void MainWindow::SelectionSetValue() { Song song = app_->playlist_manager()->current()->item_at(row)->Metadata(); if (Playlist::set_column_value(song, column, column_value)) { - TagReaderReply *reply =TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); + TagReaderReply *reply = TagReaderClient::Instance()->SaveFile(song.url().toLocalFile(), song); - NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)),reply, QPersistentModelIndex(source_index)); + NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QPersistentModelIndex)), reply, QPersistentModelIndex(source_index)); } } @@ -2136,7 +2136,6 @@ void MainWindow::ShowTranscodeDialog() { #endif void MainWindow::ShowErrorDialog(const QString &message) { - error_dialog_->ShowMessage(message); } diff --git a/src/core/mainwindow.h b/src/core/mainwindow.h index 1bb8d1b91..640a3018e 100644 --- a/src/core/mainwindow.h +++ b/src/core/mainwindow.h @@ -298,8 +298,8 @@ signals: Application *app_; SystemTrayIcon *tray_icon_; OSD *osd_; - Lazy edit_tag_dialog_; Lazy about_dialog_; + Lazy edit_tag_dialog_; AlbumCoverChoiceController *album_cover_choice_controller_; GlobalShortcuts *global_shortcuts_; diff --git a/src/core/tagreaderclient.cpp b/src/core/tagreaderclient.cpp index c10f56d20..b38a3da70 100644 --- a/src/core/tagreaderclient.cpp +++ b/src/core/tagreaderclient.cpp @@ -73,7 +73,9 @@ TagReaderReply *TagReaderClient::SaveFile(const QString &filename, const Song &m req->set_filename(DataCommaSizeFromQString(filename)); metadata.ToProtobuf(req->mutable_metadata()); - return worker_pool_->SendMessageWithReply(&message); + ReplyType *reply = worker_pool_->SendMessageWithReply(&message); + + return reply; } diff --git a/src/core/tagreaderclient.h b/src/core/tagreaderclient.h index 235b8f1ee..d5d89aef6 100644 --- a/src/core/tagreaderclient.h +++ b/src/core/tagreaderclient.h @@ -67,8 +67,6 @@ class TagReaderClient : public QObject { // TODO: Make this not a singleton static TagReaderClient *Instance() { return sInstance; } - public slots: - private slots: void WorkerFailedToStart(); diff --git a/src/dialogs/edittagdialog.cpp b/src/dialogs/edittagdialog.cpp index 82a2cf473..caf524380 100644 --- a/src/dialogs/edittagdialog.cpp +++ b/src/dialogs/edittagdialog.cpp @@ -101,7 +101,8 @@ EditTagDialog::EditTagDialog(Application *app, QWidget *parent) #endif cover_art_id_(0), cover_art_is_set_(false), - results_dialog_(new TrackSelectionDialog(this)) + results_dialog_(new TrackSelectionDialog(this)), + pending_(0) { cover_options_.default_output_image_ = AlbumCoverLoader::ScaleAndPad(cover_options_, QImage(":/pictures/cdcase.png")); @@ -128,8 +129,8 @@ EditTagDialog::EditTagDialog(Application *app, QWidget *parent) ui_->fetch_tag->setEnabled(false); #endif - // An editable field is one that has a label as a buddy. The label is - // important because it gets turned bold when the field is changed. + // An editable field is one that has a label as a buddy. + // The label is important because it gets turned bold when the field is changed. for (QLabel *label : findChildren()) { QWidget *widget = label->buddy(); if (widget) { @@ -171,7 +172,6 @@ EditTagDialog::EditTagDialog(Application *app, QWidget *parent) connect(ui_->song_list->selectionModel(), SIGNAL(selectionChanged(QItemSelection, QItemSelection)), SLOT(SelectionChanged())); connect(ui_->button_box, SIGNAL(clicked(QAbstractButton*)), SLOT(ButtonClicked(QAbstractButton*))); - //connect(ui_->rating, SIGNAL(RatingChanged(float)), SLOT(SongRated(float))); connect(ui_->playcount_reset, SIGNAL(clicked()), SLOT(ResetPlayCounts())); #if defined(HAVE_GSTREAMER) && defined(HAVE_CHROMAPRINT) connect(ui_->fetch_tag, SIGNAL(clicked()), SLOT(FetchTag())); @@ -285,6 +285,7 @@ void EditTagDialog::SetSongs(const SongList &s, const PlaylistItemList &items) { // Reload tags in the background QFuture> future = QtConcurrent::run(this, &EditTagDialog::LoadData, s); NewClosure(future, this, SLOT(SetSongsFinished(QFuture>)), future); + } void EditTagDialog::SetSongsFinished(QFuture> future) { @@ -316,6 +317,7 @@ void EditTagDialog::SetSongsFinished(QFuture> future) { // Hide the list if there's only one song in it SetSongListVisibility(data_.count() != 1); + } void EditTagDialog::SetSongListVisibility(bool visible) { @@ -382,7 +384,6 @@ bool EditTagDialog::IsValueModified(const QModelIndexList &sel, const QString &i void EditTagDialog::InitFieldValue(const FieldData &field, const QModelIndexList &sel) { const bool varies = DoesValueVary(sel, field.id_); -// const bool modified = IsValueModified(sel, field.id_); if (ExtendedEditor *editor = dynamic_cast(field.editor_)) { editor->clear(); @@ -733,10 +734,14 @@ void EditTagDialog::SaveData(const QList &data) { const Data &ref = data[i]; if (ref.current_.IsMetadataEqual(ref.original_)) continue; - if (!TagReaderClient::Instance()->SaveFileBlocking(ref.current_.url().toLocalFile(), ref.current_)) { - emit Error(tr("An error occurred writing metadata to '%1'").arg(ref.current_.url().toLocalFile())); - } + pending_++; + TagReaderReply *reply = TagReaderClient::Instance()->SaveFile(ref.current_.url().toLocalFile(), ref.current_); + NewClosure(reply, SIGNAL(Finished(bool)), this, SLOT(SongSaveComplete(TagReaderReply*, QString, Song)), reply, ref.current_.url().toLocalFile(), ref.current_); + } + + if (pending_ <= 0) AcceptFinished(); + } void EditTagDialog::accept() { @@ -744,16 +749,13 @@ void EditTagDialog::accept() { // Show the loading indicator if (!SetLoading(tr("Saving tracks") + "...")) return; - // Save tags in the background - QFuture future = QtConcurrent::run(this, &EditTagDialog::SaveData, data_); - NewClosure(future, this, SLOT(AcceptFinished())); + SaveData(data_); + } void EditTagDialog::AcceptFinished() { - if (!SetLoading(QString())) return; QDialog::accept(); - } bool EditTagDialog::eventFilter(QObject *o, QEvent *e) { @@ -865,10 +867,9 @@ void EditTagDialog::FetchTagSongChosen(const Song &original_song, const Song &ne const QString filename = original_song.url().toLocalFile(); // Find the song with this filename - auto data_it = - std::find_if(data_.begin(), data_.end(), [&filename](const Data &d) { - return d.original_.url().toLocalFile() == filename; - }); + auto data_it = std::find_if(data_.begin(), data_.end(), [&filename](const Data &d) { + return d.original_.url().toLocalFile() == filename; + }); if (data_it == data_.end()) { qLog(Warning) << "Could not find song to filename: " << filename; return; @@ -887,5 +888,26 @@ void EditTagDialog::FetchTagSongChosen(const Song &original_song, const Song &ne const QModelIndexList sel = ui_->song_list->selectionModel()->selectedIndexes(); UpdateUI(sel); } + } #endif + +void EditTagDialog::SongSaveComplete(TagReaderReply *reply, const QString filename, const Song song) { + + reply->deleteLater(); + + pending_--; + + if (!reply->message().save_file_response().success()) { + QString message = QString("An error occurred writing metadata to '%1'").arg(filename); + emit Error(message); + } + else if (song.directory_id() != -1) { + SongList songs; + songs << song; + app_->collection_backend()->AddOrUpdateSongs(songs); + } + + if (pending_ <= 0) AcceptFinished(); + +} diff --git a/src/dialogs/edittagdialog.h b/src/dialogs/edittagdialog.h index 81f924801..cd4ab6568 100644 --- a/src/dialogs/edittagdialog.h +++ b/src/dialogs/edittagdialog.h @@ -42,6 +42,7 @@ #include #include "core/song.h" +#include "core/tagreaderclient.h" #include "playlist/playlistitem.h" #include "covermanager/albumcoverloaderoptions.h" @@ -73,10 +74,10 @@ class EditTagDialog : public QDialog { void accept(); -signals: + signals: void Error(const QString &message); -protected: + protected: bool eventFilter(QObject *o, QEvent *e); void showEvent(QShowEvent*); void hideEvent(QHideEvent*); @@ -125,6 +126,8 @@ protected: void PreviousSong(); void NextSong(); + void SongSaveComplete(TagReaderReply *reply, const QString filename, const Song song); + private: struct FieldData { FieldData(QLabel *label = nullptr, QWidget *editor = nullptr, const QString &id = QString()) @@ -158,7 +161,7 @@ protected: QList LoadData(const SongList &songs) const; void SaveData(const QList &data); -private: + private: Ui_EditTagDialog *ui_; Application *app_; @@ -189,6 +192,8 @@ private: QPushButton *next_button_; TrackSelectionDialog *results_dialog_; + + int pending_; }; #endif // EDITTAGDIALOG_H