Fix issue no warning message for moving attached files (JabRef#11987)

* implement showing warning message in the cleanupworker * Edited changelog * Add missing Localization * Edited casting * Implemented the suggestion, moved the showFailure() to the GUI package * Implemented suggestions to use collectors, removed unused comments and methods
koppor · Oct 25, 2024 · 982aeb1 · 982aeb1
1 parent 2523377
commit 982aeb1
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 7 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -100,6 +100,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We fixed an issue where recently opened files were not displayed in the main menu properly. [#9042](https://github.com/JabRef/jabref/issues/9042)
 - We fixed an issue where the DOI lookup would show an error when a DOI was found for an entry. [#11850](https://github.com/JabRef/jabref/issues/11850)
 - We fixed an issue where <kbd>Tab</kbd> cannot be used to jump to next field in some single-line fields. [#11785](https://github.com/JabRef/jabref/issues/11785)
+- We fixed an issue where we display warning message for moving attached open files. [#10121](https://github.com/JabRef/jabref/issues/10121)
 - We fixed an issue where it was not possible to select selecting content of other user's comments.[#11106](https://github.com/JabRef/jabref/issues/11106)
 - We fixed an issue where web search preferences "Custom API key" table modifications not discarded. [#11925](https://github.com/JabRef/jabref/issues/11925)
 - We fixed an issue where trying to open a library from a failed mounted directory on Mac would cause an error. [#10548](https://github.com/JabRef/jabref/issues/10548)

diff --git a/src/main/java/org/jabref/gui/cleanup/CleanupAction.java b/src/main/java/org/jabref/gui/cleanup/CleanupAction.java
@@ -1,18 +1,23 @@
 package org.jabref.gui.cleanup;
 
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 
 import javax.swing.undo.UndoManager;
 
+import javafx.application.Platform;
+
 import org.jabref.gui.DialogService;
 import org.jabref.gui.LibraryTab;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.ActionHelper;
 import org.jabref.gui.actions.SimpleCommand;
 import org.jabref.gui.undo.NamedCompound;
 import org.jabref.gui.undo.UndoableFieldChange;
+import org.jabref.logic.JabRefException;
 import org.jabref.logic.cleanup.CleanupPreferences;
 import org.jabref.logic.cleanup.CleanupWorker;
 import org.jabref.logic.l10n.Localization;
@@ -31,6 +36,7 @@ public class CleanupAction extends SimpleCommand {
     private final StateManager stateManager;
     private final TaskExecutor taskExecutor;
     private final UndoManager undoManager;
+    private final List<JabRefException> failures;
 
     private boolean isCanceled;
     private int modifiedEntriesCount;
@@ -47,6 +53,7 @@ public CleanupAction(Supplier<LibraryTab> tabSupplier,
         this.stateManager = stateManager;
         this.taskExecutor = taskExecutor;
         this.undoManager = undoManager;
+        this.failures = new ArrayList<>();
 
         this.executable.bind(ActionHelper.needsEntriesSelected(stateManager));
     }
@@ -105,14 +112,17 @@ private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences pr
         CleanupWorker cleaner = new CleanupWorker(
                 databaseContext,
                 preferences.getFilePreferences(),
-                preferences.getTimestampPreferences());
+                preferences.getTimestampPreferences()
+        );
 
         List<FieldChange> changes = cleaner.cleanup(preset, entry);
 
         // Register undo action
         for (FieldChange change : changes) {
             ce.addEdit(new UndoableFieldChange(change));
         }
+
+        failures.addAll(cleaner.getFailures());
     }
 
     private void showResults() {
@@ -135,17 +145,36 @@ private void showResults() {
     }
 
     private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) {
+        this.failures.clear();
+        NamedCompound ce = new NamedCompound(Localization.lang("Cleanup entries"));
+
         for (BibEntry entry : stateManager.getSelectedEntries()) {
             // undo granularity is on entry level
-            NamedCompound ce = new NamedCompound(Localization.lang("Cleanup entry"));
-
             doCleanup(databaseContext, cleanupPreferences, entry, ce);
 
-            ce.end();
             if (ce.hasEdits()) {
                 modifiedEntriesCount++;
-                undoManager.addEdit(ce);
             }
         }
+
+        ce.end();
+
+        if (ce.hasEdits()) {
+            undoManager.addEdit(ce);
+        }
+
+        if (!failures.isEmpty()) {
+            showFailures(failures);
+        }
+    }
+
+    private void showFailures(List<JabRefException> failures) {
+        String message = failures.stream()
+                                 .map(exception -> "- " + exception.getLocalizedMessage())
+                                 .collect(Collectors.joining("\n"));
+
+        Platform.runLater(() ->
+                dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), message)
+        );
     }
 }
diff --git a/src/main/java/org/jabref/gui/cleanup/CleanupSingleAction.java b/src/main/java/org/jabref/gui/cleanup/CleanupSingleAction.java
@@ -5,12 +5,15 @@
 
 import javax.swing.undo.UndoManager;
 
+import javafx.application.Platform;
+
 import org.jabref.gui.DialogService;
 import org.jabref.gui.StateManager;
 import org.jabref.gui.actions.ActionHelper;
 import org.jabref.gui.actions.SimpleCommand;
 import org.jabref.gui.undo.NamedCompound;
 import org.jabref.gui.undo.UndoableFieldChange;
+import org.jabref.logic.JabRefException;
 import org.jabref.logic.cleanup.CleanupPreferences;
 import org.jabref.logic.cleanup.CleanupWorker;
 import org.jabref.logic.l10n.Localization;
@@ -81,14 +84,19 @@ private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences pr
         CleanupWorker cleaner = new CleanupWorker(
                 databaseContext,
                 preferences.getFilePreferences(),
-                preferences.getTimestampPreferences());
+                preferences.getTimestampPreferences()
+        );
 
         List<FieldChange> changes = cleaner.cleanup(preset, entry);
 
         // Register undo action
         for (FieldChange change : changes) {
             ce.addEdit(new UndoableFieldChange(change));
         }
+
+        if (!cleaner.getFailures().isEmpty()) {
+            this.showFailures(cleaner.getFailures());
+        }
     }
 
     private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences cleanupPreferences) {
@@ -102,4 +110,14 @@ private void cleanup(BibDatabaseContext databaseContext, CleanupPreferences clea
                 undoManager.addEdit(ce);
             }
     }
+
+    private void showFailures(List<JabRefException> failures) {
+        StringBuilder sb = new StringBuilder();
+        for (JabRefException exception : failures) {
+            sb.append("- ").append(exception.getLocalizedMessage()).append("\n");
+        }
+        Platform.runLater(() ->
+                dialogService.showErrorDialogAndWait(Localization.lang("File Move Errors"), sb.toString())
+        );
+    }
 }
diff --git a/src/main/java/org/jabref/logic/cleanup/CleanupWorker.java b/src/main/java/org/jabref/logic/cleanup/CleanupWorker.java
@@ -5,6 +5,7 @@
 import java.util.Objects;
 
 import org.jabref.logic.FilePreferences;
+import org.jabref.logic.JabRefException;
 import org.jabref.logic.preferences.TimestampPreferences;
 import org.jabref.model.FieldChange;
 import org.jabref.model.database.BibDatabaseContext;
@@ -15,22 +16,26 @@ public class CleanupWorker {
     private final BibDatabaseContext databaseContext;
     private final FilePreferences filePreferences;
     private final TimestampPreferences timestampPreferences;
+    private final List<JabRefException> failures;
 
     public CleanupWorker(BibDatabaseContext databaseContext, FilePreferences filePreferences, TimestampPreferences timestampPreferences) {
         this.databaseContext = databaseContext;
         this.filePreferences = filePreferences;
         this.timestampPreferences = timestampPreferences;
+        this.failures = new ArrayList<>();
     }
 
     public List<FieldChange> cleanup(CleanupPreferences preset, BibEntry entry) {
         Objects.requireNonNull(preset);
         Objects.requireNonNull(entry);
 
         List<CleanupJob> jobs = determineCleanupActions(preset);
-
         List<FieldChange> changes = new ArrayList<>();
         for (CleanupJob job : jobs) {
             changes.addAll(job.cleanup(entry));
+            if (job instanceof MoveFilesCleanup cleanup) {
+                failures.addAll(cleanup.getIoExceptions());
+            }
         }
 
         return changes;
@@ -86,4 +91,8 @@ private CleanupJob toJob(CleanupPreferences.CleanupStep action) {
                     throw new UnsupportedOperationException(action.name());
         };
     }
+
+    public List<JabRefException> getFailures() {
+        return failures;
+    }
 }
diff --git a/src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java b/src/main/java/org/jabref/logic/cleanup/MoveFilesCleanup.java
@@ -1,13 +1,16 @@
 package org.jabref.logic.cleanup;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
 
 import org.jabref.logic.FilePreferences;
+import org.jabref.logic.JabRefException;
 import org.jabref.logic.externalfiles.LinkedFileHandler;
+import org.jabref.logic.l10n.Localization;
 import org.jabref.model.FieldChange;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
@@ -26,10 +29,12 @@ public class MoveFilesCleanup implements CleanupJob {
 
     private final BibDatabaseContext databaseContext;
     private final FilePreferences filePreferences;
+    private final List<JabRefException> ioExceptions;
 
     public MoveFilesCleanup(BibDatabaseContext databaseContext, FilePreferences filePreferences) {
         this.databaseContext = Objects.requireNonNull(databaseContext);
         this.filePreferences = Objects.requireNonNull(filePreferences);
+        this.ioExceptions = new ArrayList<>();
     }
 
     @Override
@@ -43,6 +48,7 @@ public List<FieldChange> cleanup(BibEntry entry) {
                 changed = fileHandler.moveToDefaultDirectory() || changed;
             } catch (IOException exception) {
                 LOGGER.error("Error while moving file {}", file.getLink(), exception);
+                ioExceptions.add(new JabRefException(Localization.lang("Could not move file %0. Please close this file and retry.", file.getLink()), exception));
             }
         }
 
@@ -53,4 +59,8 @@ public List<FieldChange> cleanup(BibEntry entry) {
 
         return Collections.emptyList();
     }
+
+    public List<JabRefException> getIoExceptions() {
+        return ioExceptions;
+    }
 }
diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -2800,3 +2800,6 @@ Store\ url\ for\ downloaded\ file=Store url for downloaded file
 Compare\ with\ existing\ entry=Compare with existing entry
 Library\ Entry=Library Entry
 Citation\ Entry=Citation Entry
+
+File\ Move\ Errors=File Move Errors
+Could\ not\ move\ file\ %0.\ Please\ close\ this\ file\ and\ retry.=Could not move file %0. Please close this file and retry.