Fix for delete entries should ask user (JabRef#10591)

* Implemented the feature that deleting files which linked to selected entries when user select deletion, and keeping files unchanged when user select cut * The following features are implemented: 1.Initializes a pop-up dialog box to confirm whether the user wants to delete attached files from selected entry. 2.Keep track of user preference: if the user prefers always delete attached files, delete the files without displaying the dialog box. 3. Add preference options in File>Preference>Linked Files>Attached files so that users can manage preferences * update CHANGELOG.md * Add language keys to english language file * restore files in src/main/resources/csl-locales and src/main/resources/csl-styles * Removed unnecessary comments and finxed some requested changes. Added new features: 1. When deleting attached files, the name of files to be deleted will be displayed. 2. Solved the access error caused by repeated deletion of files when one file is attached to multiple entries. * Add language keys to english language file * Modify language keys to english language file * made deleteFileFromDisk method static * update comment of method deleteFileFromDisk * fixed coding styles * restored unexpected code changes * fix logic * try null * todo * Unify dialogs that confirmation deleting files * Get around LinkedFile in LIbraryTab, Encapsulate LinkedFile into LinkedFileViewModel * fix style * restore files * Unified the different dialogs when deleting entries, removerd unnecessary dialogs * fix csl-styles * try to fix csl-styles * try to fix csl-styles again * try to fix csl-styles again 2 * try to fix csl-styles again 3 * Update prompts in en.properties * New features - Add to Trash - Group file-related language strings together * Fix architecture tests * Introduce list of files to delete * Streamline 1 vs. many files * Fix openRewrite * Discard changes to src/test/resources/org/jabref/logic/search/test-library-with-attached-files.bib * Adapt true/false logic according to expectations * Add "Trash" to CHANGELOG.md * Fix localization * Fix JabRef_en.properties * Add some debug statements * Fix preferences * Separate log entries by empty line * More refined dialog --------- Co-authored-by: Siedlerchr <[email protected]> Co-authored-by: Oliver Kopp <[email protected]> Co-authored-by: Carl Christian Snethlage <[email protected]>
koppor · Feb 26, 2024 · bec55f8 · bec55f8
1 parent ecc02e2
commit bec55f8
Show file tree

Hide file tree

Showing 20 changed files with 377 additions and 160 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -16,6 +16,8 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 - We added a fetcher for [ISIDORE](https://isidore.science/), simply paste in the link into the text field or the last 6 digits in the link that identify that paper. [#10423](https://github.com/JabRef/jabref/issues/10423)
 - When importing entries form the "Citation relations" tab, the field [cites](https://docs.jabref.org/advanced/entryeditor/entrylinks) is now filled according to the relationship between the entries. [#10572](https://github.com/JabRef/jabref/pull/10752)
 - We added a new group icon column to the main table showing the icons of the entry's groups. [#10801](https://github.com/JabRef/jabref/pull/10801)
+- When deleting an entry, the files linked to the entry are now optionally deleted as well. [#10509](https://github.com/JabRef/jabref/issues/10509)
+- We added support to move the file to the system trash (instead of deleting it). [#10591](https://github.com/JabRef/jabref/pull/10591)
 - We added ability to jump to an entry in the command line using `-j CITATIONKEY`. [koppor#540](https://github.com/koppor/jabref/issues/540)
 - We added a new boolean to the style files for Openoffice/Libreoffice integration to switch between ZERO_WIDTH_SPACE (default) and no space. [#10843](https://github.com/JabRef/jabref/pull/10843)
 - When pasting HTML into the abstract or a comment field, the hypertext is automatically converted to Markdown. [#10558](https://github.com/JabRef/jabref/issues/10558)

diff --git a/src/main/java/org/jabref/gui/LibraryTab.java b/src/main/java/org/jabref/gui/LibraryTab.java
@@ -8,6 +8,7 @@
 import java.util.Objects;
 import java.util.Optional;
 import java.util.Random;
+import java.util.stream.Collectors;
 
 import javax.swing.undo.UndoManager;
 
@@ -42,7 +43,9 @@
 import org.jabref.gui.dialogs.AutosaveUiManager;
 import org.jabref.gui.entryeditor.EntryEditor;
 import org.jabref.gui.exporter.SaveDatabaseAction;
+import org.jabref.gui.fieldeditors.LinkedFileViewModel;
 import org.jabref.gui.importer.actions.OpenDatabaseAction;
+import org.jabref.gui.linkedfile.DeleteFileAction;
 import org.jabref.gui.maintable.BibEntryTableViewModel;
 import org.jabref.gui.maintable.MainTable;
 import org.jabref.gui.maintable.MainTableDataModel;
@@ -430,7 +433,7 @@ public SuggestionProviders getSuggestionProviders() {
     }
 
     /**
-     * Removes the selected entries from the database
+     * Removes the selected entries and files linked to selected entries from the database
      *
      * @param mode If DELETE_ENTRY the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut"
      */
@@ -439,7 +442,7 @@ public void delete(StandardActions mode) {
     }
 
     /**
-     * Removes the selected entries from the database
+     * Removes the selected entries and files linked to selected entries from the database
      *
      * @param mode If DELETE_ENTRY the user will get asked if he really wants to delete the entries, and it will be localized as "deleted". If true the action will be localized as "cut"
      */
@@ -451,16 +454,31 @@ private void delete(StandardActions mode, List<BibEntry> entries) {
             return;
         }
 
+        // Delete selected entries
         getUndoManager().addEdit(new UndoableRemoveEntries(bibDatabaseContext.getDatabase(), entries, mode == StandardActions.CUT));
         bibDatabaseContext.getDatabase().removeEntries(entries);
+
+        if (mode != StandardActions.CUT) {
+            List<LinkedFile> linkedFileList = entries.stream()
+                                                     .flatMap(entry -> entry.getFiles().stream())
+                                                     .distinct()
+                                                     .toList();
+
+            if (!linkedFileList.isEmpty()) {
+                List<LinkedFileViewModel> viewModels = linkedFileList.stream()
+                                                                     .map(linkedFile -> linkedFile.toModel(null, bibDatabaseContext, null, null, preferencesService))
+                                                                     .collect(Collectors.toList());
+
+                new DeleteFileAction(dialogService, preferencesService.getFilePreferences(), bibDatabaseContext, viewModels).execute();
+            }
+        }
+
         ensureNotShowingBottomPanel(entries);
 
         this.changedProperty.setValue(true);
         switch (mode) {
-            case StandardActions.CUT ->
-                    dialogService.notify(Localization.lang("Cut %0 entry(ies)", entries.size()));
-            case StandardActions.DELETE_ENTRY ->
-                    dialogService.notify(Localization.lang("Deleted %0 entry(ies)", entries.size()));
+            case StandardActions.CUT -> dialogService.notify(Localization.lang("Cut %0 entry(ies)", entries.size()));
+            case StandardActions.DELETE_ENTRY -> dialogService.notify(Localization.lang("Deleted %0 entry(ies)", entries.size()));
         }
 
         // prevent the main table from loosing focus
@@ -678,6 +696,14 @@ public BibDatabase getDatabase() {
         return bibDatabaseContext.getDatabase();
     }
 
+    /**
+     * Initializes a pop-up dialog box to confirm whether the user wants to delete the selected entry
+     * Keep track of user preference:
+     * if the user prefers not to ask before deleting, delete the selected entry without displaying the dialog box
+     *
+     * @param numberOfEntries number of entries user is selecting
+     * @return true if user confirm to delete entry
+     */
     private boolean showDeleteConfirmationDialog(int numberOfEntries) {
         if (preferencesService.getWorkspacePreferences().shouldConfirmDelete()) {
             String title = Localization.lang("Delete entry");
@@ -691,7 +717,8 @@ private boolean showDeleteConfirmationDialog(int numberOfEntries) {
                 cancelButton = Localization.lang("Keep entries");
             }
 
-            return dialogService.showConfirmationDialogWithOptOutAndWait(title,
+            return dialogService.showConfirmationDialogWithOptOutAndWait(
+                    title,
                     message,
                     okButton,
                     cancelButton,

diff --git a/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java b/src/main/java/org/jabref/gui/desktop/JabRefDesktop.java
@@ -1,5 +1,7 @@
 package org.jabref.gui.desktop;
 
+import java.awt.Desktop;
+import java.io.File;
 import java.io.IOException;
 import java.net.URI;
 import java.nio.file.Files;
@@ -52,7 +54,7 @@ private JabRefDesktop() {
 
     /**
      * Open a http/pdf/ps viewer for the given link string.
-     *
+     * <p>
      * Opening a PDF file at the file field is done at {@link org.jabref.gui.fieldeditors.LinkedFileViewModel#open}
      */
     public static void openExternalViewer(BibDatabaseContext databaseContext,
@@ -239,7 +241,6 @@ public static void openFolderAndSelectFile(Path fileLink,
      * If no command is specified in {@link Globals}, the default system console will be executed.
      *
      * @param file Location the console should be opened at.
-     *
      */
     public static void openConsole(Path file, PreferencesService preferencesService, DialogService dialogService) throws IOException {
         if (file == null) {
@@ -314,4 +315,26 @@ public static void openBrowserShowPopup(String url, DialogService dialogService,
             dialogService.showErrorDialogAndWait(couldNotOpenBrowser, couldNotOpenBrowser + "\n" + openManually + "\n" + copiedToClipboard);
         }
     }
+
+    /**
+     * Moves the given file to the trash.
+     *
+     * @throws UnsupportedOperationException if the current platform does not support the {@link Desktop.Action#MOVE_TO_TRASH} action
+     * @see Desktop#moveToTrash(File)
+     */
+    public static void moveToTrash(Path path) {
+        NATIVE_DESKTOP.moveToTrash(path);
+    }
+
+    public static boolean moveToTrashSupported() {
+        return NATIVE_DESKTOP.moveToTrashSupported();
+    }
+
+    public static Path getApplicationDirectory() {
+        return NATIVE_DESKTOP.getApplicationDirectory();
+    }
+
+    public static Path getFulltextIndexBaseDirectory() {
+        return NATIVE_DESKTOP.getFulltextIndexBaseDirectory();
+    }
 }
diff --git a/src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java b/src/main/java/org/jabref/gui/desktop/os/NativeDesktop.java
@@ -1,5 +1,6 @@
 package org.jabref.gui.desktop.os;
 
+import java.awt.Desktop;
 import java.io.File;
 import java.io.IOException;
 import java.net.InetAddress;
@@ -8,6 +9,7 @@
 import java.nio.file.Path;
 
 import org.jabref.Launcher;
+import org.jabref.architecture.AllowedToUseAwt;
 import org.jabref.gui.DialogService;
 import org.jabref.logic.util.BuildInfo;
 import org.jabref.logic.util.OS;
@@ -19,15 +21,16 @@
 import org.slf4j.LoggerFactory;
 
 /**
+ * This class is not meant to be used directly. Use {@link org.jabref.gui.desktop.JabRefDesktop} instead.
+ * <p>
  * This class contains bundles OS specific implementations for file directories and file/application open handling methods.
  * In case the default does not work, subclasses provide the correct behavior.
- *
- * <p>
+ * * <p>
  * We cannot use a static logger instance here in this class as the Logger first needs to be configured in the {@link Launcher#addLogToDisk}
  * The configuration of tinylog will become immutable as soon as the first log entry is issued.
  * https://tinylog.org/v2/configuration/
- * </p>
  */
+@AllowedToUseAwt("Because of moveToTrash() is not available elsewhere.")
 public abstract class NativeDesktop {
 
     public abstract void openFile(String filePath, String fileType, FilePreferences filePreferences) throws IOException;
@@ -125,4 +128,18 @@ public String getHostName() {
         }
         return hostName;
     }
+
+    /**
+     * Moves the given file to the trash.
+     *
+     * @throws UnsupportedOperationException if the current platform does not support the {@link Desktop.Action#MOVE_TO_TRASH} action
+     * @see Desktop#moveToTrash(File)
+     */
+    public void moveToTrash(Path path) {
+        Desktop.getDesktop().moveToTrash(path.toFile());
+    }
+
+    public boolean moveToTrashSupported() {
+        return Desktop.getDesktop().isSupported(Desktop.Action.MOVE_TO_TRASH);
+    }
 }
diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFileViewModel.java
@@ -19,9 +19,6 @@
 import javafx.beans.property.SimpleDoubleProperty;
 import javafx.beans.property.StringProperty;
 import javafx.scene.Node;
-import javafx.scene.control.Alert.AlertType;
-import javafx.scene.control.ButtonBar.ButtonData;
-import javafx.scene.control.ButtonType;
 
 import org.jabref.gui.AbstractViewModel;
 import org.jabref.gui.DialogService;
@@ -30,6 +27,7 @@
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.icon.JabRefIcon;
+import org.jabref.gui.linkedfile.DeleteFileAction;
 import org.jabref.gui.linkedfile.DownloadLinkedFileAction;
 import org.jabref.gui.linkedfile.LinkedFileEditDialogView;
 import org.jabref.gui.mergeentries.MultiMergeEntriesView;
@@ -86,7 +84,6 @@ public LinkedFileViewModel(LinkedFile linkedFile,
                                TaskExecutor taskExecutor,
                                DialogService dialogService,
                                PreferencesService preferencesService) {
-
         this.linkedFile = linkedFile;
         this.preferencesService = preferencesService;
         this.linkedFileHandler = new LinkedFileHandler(linkedFile, entry, databaseContext, preferencesService.getFilePreferences());
@@ -365,42 +362,16 @@ public void moveToDefaultDirectoryAndRename() {
     }
 
     /**
-     * Asks the user for confirmation that he really wants to the delete the file from disk (or just remove the link).
+     * Asks the user for confirmation that he really wants to the delete the file from disk (or just remove the link)
+     * and then proceeds accordingly.
      *
-     * @return true if the linked file should be removed afterwards from the entry (i.e because it was deleted
-     * successfully, does not exist in the first place or the user choose to remove it)
+     * @return true if the linked file has been removed afterward from the entry (i.e., because it was deleted
+     * successfully, does not exist in the first place, or the user choose to remove it)
      */
     public boolean delete() {
-        Optional<Path> file = linkedFile.findIn(databaseContext, preferencesService.getFilePreferences());
-
-        if (file.isEmpty()) {
-            LOGGER.warn("Could not find file {}", linkedFile.getLink());
-            return true;
-        }
-
-        ButtonType removeFromEntry = new ButtonType(Localization.lang("Remove from entry"), ButtonData.YES);
-        ButtonType deleteFromEntry = new ButtonType(Localization.lang("Delete from disk"));
-        Optional<ButtonType> buttonType = dialogService.showCustomButtonDialogAndWait(AlertType.INFORMATION,
-                Localization.lang("Delete '%0'", file.get().getFileName().toString()),
-                Localization.lang("Delete '%0' permanently from disk, or just remove the file from the entry? Pressing Delete will delete the file permanently from disk.", file.get().toString()),
-                removeFromEntry, deleteFromEntry, ButtonType.CANCEL);
-
-        if (buttonType.isPresent()) {
-            if (buttonType.get().equals(removeFromEntry)) {
-                return true;
-            }
-
-            if (buttonType.get().equals(deleteFromEntry)) {
-                try {
-                    Files.delete(file.get());
-                    return true;
-                } catch (IOException ex) {
-                    dialogService.showErrorDialogAndWait(Localization.lang("Cannot delete file"), Localization.lang("File permission error"));
-                    LOGGER.warn("File permission error while deleting: {}", linkedFile, ex);
-                }
-            }
-        }
-        return false;
+        DeleteFileAction deleteFileAction = new DeleteFileAction(dialogService, preferencesService.getFilePreferences(), databaseContext, null, List.of(this));
+        deleteFileAction.execute();
+        return deleteFileAction.isSuccess();
     }
 
     public void edit() {

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
@@ -254,8 +254,7 @@ private void setUpKeyBindings() {
             if (keyBinding.isPresent()) {
                 switch (keyBinding.get()) {
                     case DELETE_ENTRY:
-                        new DeleteFileAction(dialogService, preferencesService, databaseContext,
-                                viewModel, listView).execute();
+                        deleteAttachedFilesWithConfirmation();
                         event.consume();
                         break;
                     default:
@@ -265,6 +264,11 @@ private void setUpKeyBindings() {
         });
     }
 
+    private void deleteAttachedFilesWithConfirmation() {
+        new DeleteFileAction(dialogService, preferencesService.getFilePreferences(), databaseContext,
+                viewModel, listView.getSelectionModel().getSelectedItems()).execute();
+    }
+
     public LinkedFilesEditorViewModel getViewModel() {
         return viewModel;
     }