Show error when editing tags fails, update DB immediately when successful

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_)

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

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<int>()<< 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);
 }
 

src/core/mainwindow.h

@@ -298,8 +298,8 @@ signals:
   Application *app_;
   SystemTrayIcon  *tray_icon_;
   OSD *osd_;
-  Lazy<EditTagDialog> edit_tag_dialog_;
   Lazy<About> about_dialog_;
+  Lazy<EditTagDialog> edit_tag_dialog_;
   AlbumCoverChoiceController *album_cover_choice_controller_;
 
   GlobalShortcuts *global_shortcuts_;

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;
 
 }
 

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();
 

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
+  // An editable field is one that has a label as a buddy.
-  // important because it gets turned bold when the field is changed.
+  // The label is important because it gets turned bold when the field is changed.
   for (QLabel *label : findChildren<QLabel*>()) {
     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<QList<Data>> future = QtConcurrent::run(this, &EditTagDialog::LoadData, s);
   NewClosure(future, this, SLOT(SetSongsFinished(QFuture<QList<EditTagDialog::Data>>)), future);
+
 }
 
 void EditTagDialog::SetSongsFinished(QFuture<QList<Data>> future) {
@@ -316,6 +317,7 @@ void EditTagDialog::SetSongsFinished(QFuture<QList<Data>> 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<ExtendedEditor*>(field.editor_)) {
     editor->clear();
@@ -733,10 +734,14 @@ void EditTagDialog::SaveData(const QList<Data> &data) {
     const Data &ref = data[i];
     if (ref.current_.IsMetadataEqual(ref.original_)) continue;
 
-    if (!TagReaderClient::Instance()->SaveFileBlocking(ref.current_.url().toLocalFile(), ref.current_)) {
+    pending_++;
-      emit Error(tr("An error occurred writing metadata to '%1'").arg(ref.current_.url().toLocalFile()));
+    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
+  SaveData(data_);
-  QFuture<void> future = QtConcurrent::run(this, &EditTagDialog::SaveData, data_);
+
-  NewClosure(future, this, SLOT(AcceptFinished()));
 }
 
 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 =
+  auto data_it = std::find_if(data_.begin(), data_.end(), [&filename](const Data &d) {
-      std::find_if(data_.begin(), data_.end(), [&filename](const Data &d) {
+    return d.original_.url().toLocalFile() == filename;
-        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();
+
+}

src/dialogs/edittagdialog.h

@@ -42,6 +42,7 @@
 #include <QtEvents>
 
 #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<Data> LoadData(const SongList &songs) const;
   void SaveData(const QList<Data> &data);
 
-private:
+ private:
   Ui_EditTagDialog *ui_;
 
   Application *app_;
@@ -189,6 +192,8 @@ private:
   QPushButton *next_button_;
 
   TrackSelectionDialog *results_dialog_;
+
+  int pending_;
 };
 
 #endif  // EDITTAGDIALOG_H