From 982aeb1bbed01473c9510469374589a6529ac732 Mon Sep 17 00:00:00 2001 From: juliusalberto <57690582+juliusalberto@users.noreply.github.com> Date: Sat, 26 Oct 2024 00:20:26 +1100 Subject: [PATCH] Fix issue no warning message for moving attached files (#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 --- CHANGELOG.md | 1 + .../org/jabref/gui/cleanup/CleanupAction.java | 39 ++++++++++++++++--- .../gui/cleanup/CleanupSingleAction.java | 20 +++++++++- .../jabref/logic/cleanup/CleanupWorker.java | 11 +++++- .../logic/cleanup/MoveFilesCleanup.java | 10 +++++ src/main/resources/l10n/JabRef_en.properties | 3 ++ 6 files changed, 77 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e957d5cc76a..dc911099c31 100644 --- 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 Tab 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 index 9fdac23cd88..b09d82e6f5f 100644 --- a/src/main/java/org/jabref/gui/cleanup/CleanupAction.java +++ b/src/main/java/org/jabref/gui/cleanup/CleanupAction.java @@ -1,11 +1,15 @@ 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; @@ -13,6 +17,7 @@ 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 failures; private boolean isCanceled; private int modifiedEntriesCount; @@ -47,6 +53,7 @@ public CleanupAction(Supplier tabSupplier, this.stateManager = stateManager; this.taskExecutor = taskExecutor; this.undoManager = undoManager; + this.failures = new ArrayList<>(); this.executable.bind(ActionHelper.needsEntriesSelected(stateManager)); } @@ -105,7 +112,8 @@ private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences pr CleanupWorker cleaner = new CleanupWorker( databaseContext, preferences.getFilePreferences(), - preferences.getTimestampPreferences()); + preferences.getTimestampPreferences() + ); List changes = cleaner.cleanup(preset, entry); @@ -113,6 +121,8 @@ private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences pr 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 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 index 4688ca6c67a..6f9ffc08892 100644 --- 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,7 +84,8 @@ private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences pr CleanupWorker cleaner = new CleanupWorker( databaseContext, preferences.getFilePreferences(), - preferences.getTimestampPreferences()); + preferences.getTimestampPreferences() + ); List changes = cleaner.cleanup(preset, entry); @@ -89,6 +93,10 @@ private void doCleanup(BibDatabaseContext databaseContext, CleanupPreferences pr 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 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 index e9b01ad7081..8f15e8a4d5a 100644 --- 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,11 +16,13 @@ public class CleanupWorker { private final BibDatabaseContext databaseContext; private final FilePreferences filePreferences; private final TimestampPreferences timestampPreferences; + private final List failures; public CleanupWorker(BibDatabaseContext databaseContext, FilePreferences filePreferences, TimestampPreferences timestampPreferences) { this.databaseContext = databaseContext; this.filePreferences = filePreferences; this.timestampPreferences = timestampPreferences; + this.failures = new ArrayList<>(); } public List cleanup(CleanupPreferences preset, BibEntry entry) { @@ -27,10 +30,12 @@ public List cleanup(CleanupPreferences preset, BibEntry entry) { Objects.requireNonNull(entry); List jobs = determineCleanupActions(preset); - List 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 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 index c55fb8a090d..3f7419239c4 100644 --- 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 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 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 cleanup(BibEntry entry) { return Collections.emptyList(); } + + public List getIoExceptions() { + return ioExceptions; + } } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index aa3b4174b58..7505818e8fe 100644 --- 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.