fix-for-issue-4489 (#4538)

* Removed DownloadExternalFile call, replaced with LinkedFileViewModel fuctionality * Added some notes * Added addLinkedFileFromURL method to handle linking and display dialogs * Fixed issues per codacity review * Fixed issues per gradlew check, added new Localization.lang entry, removed unused Localization.lang entry * Added extends SimpleCommand to FindFullTextAction * Added entry to changelog
JabRef · Dec 17, 2018 · 3c87491 · 3c87491
1 parent 49c3ef3
commit 3c87491
Show file tree

Hide file tree

Showing 4 changed files with 51 additions and 44 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -89,6 +89,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where the ArXiv Fetcher did not support HTTP URLs [koppor#328](https://github.com/koppor/jabref/issues/328)
 - We fixed an issue where only one PDF file could be imported [#4422](https://github.com/JabRef/jabref/issues/4422)
 - We fixed an issue where "Move to group" would always move the first entry in the library and not the selected [#4414](https://github.com/JabRef/jabref/issues/4414)
+- We fixed an issue where an older dialog appears when downloading full texts from the quality menu. [#4489](https://github.com/JabRef/jabref/issues/4489)
 
 
 

diff --git a/src/main/java/org/jabref/gui/BasePanel.java b/src/main/java/org/jabref/gui/BasePanel.java
@@ -429,7 +429,7 @@ private void setupActions() {
         actions.put(Actions.REMOVE_FROM_GROUP, new GroupAddRemoveDialog(this, false, false));
         actions.put(Actions.MOVE_TO_GROUP, new GroupAddRemoveDialog(this, true, true));
 
-        actions.put(Actions.DOWNLOAD_FULL_TEXT, new FindFullTextAction(frame.getDialogService(), this));
+        actions.put(Actions.DOWNLOAD_FULL_TEXT, new FindFullTextAction(this)::execute);
     }
 
     /**

diff --git a/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java b/src/main/java/org/jabref/gui/externalfiles/FindFullTextAction.java
@@ -1,6 +1,5 @@
 package org.jabref.gui.externalfiles;
 
-import java.io.IOException;
 import java.net.URL;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -9,28 +8,25 @@
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
 
-import javax.swing.SwingUtilities;
-
 import org.jabref.Globals;
 import org.jabref.gui.BasePanel;
 import org.jabref.gui.DialogService;
-import org.jabref.gui.actions.BaseAction;
-import org.jabref.gui.undo.UndoableFieldChange;
+import org.jabref.gui.actions.SimpleCommand;
+import org.jabref.gui.fieldeditors.LinkedFileViewModel;
 import org.jabref.gui.util.BackgroundTask;
-import org.jabref.gui.util.DefaultTaskExecutor;
 import org.jabref.logic.importer.FulltextFetchers;
 import org.jabref.logic.l10n.Localization;
-import org.jabref.model.FieldChange;
 import org.jabref.model.entry.BibEntry;
-import org.jabref.model.entry.FieldName;
+import org.jabref.model.entry.LinkedFile;
+import org.jabref.preferences.JabRefPreferences;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 /**
  * Try to download fulltext PDF for selected entry(ies) by following URL or DOI link.
  */
-public class FindFullTextAction implements BaseAction {
+public class FindFullTextAction extends SimpleCommand {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(FindFullTextAction.class);
     // The minimum number of selected entries to ask the user for confirmation
@@ -39,16 +35,16 @@ public class FindFullTextAction implements BaseAction {
     private final BasePanel basePanel;
     private final DialogService dialogService;
 
-    public FindFullTextAction(DialogService dialogService, BasePanel basePanel) {
+    public FindFullTextAction(BasePanel basePanel) {
         this.basePanel = basePanel;
-        this.dialogService = dialogService;
+        this.dialogService = basePanel.frame().getDialogService();
     }
 
     @Override
-    public void action() {
+    public void execute() {
         BackgroundTask.wrap(this::findFullTexts)
-                      .onSuccess(downloads -> SwingUtilities.invokeLater(() -> downloadFullTexts(downloads)))
-                      .executeWith(Globals.TASK_EXECUTOR);
+                .onSuccess(this::downloadFullTexts)
+                .executeWith(Globals.TASK_EXECUTOR);
     }
 
     private Map<Optional<URL>, BibEntry> findFullTexts() {
@@ -101,40 +97,50 @@ private void downloadFullTexts(Map<Optional<URL>, BibEntry> downloads) {
 
                     return;
                 }
-                DownloadExternalFile fileDownload = new DownloadExternalFile(dialogService,
-                        basePanel.getBibDatabaseContext(), entry);
-                try {
-                    fileDownload.download(result.get(), "application/pdf", file -> {
-                        DefaultTaskExecutor.runInJavaFXThread(() -> {
-                            Optional<FieldChange> fieldChange = entry.addFile(file);
-                            if (fieldChange.isPresent()) {
-                                UndoableFieldChange edit = new UndoableFieldChange(entry, FieldName.FILE,
-                                        entry.getField(FieldName.FILE).orElse(null), fieldChange.get().getNewValue());
-                                basePanel.getUndoManager().addEdit(edit);
-                                basePanel.markBaseChanged();
-                            }
-                        });
-
-                    });
-                } catch (IOException e) {
-                    LOGGER.warn("Problem downloading file", e);
-                    basePanel.output(Localization.lang("Full text document download failed for entry %0",
-                            entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
-                }
-                basePanel.output(Localization.lang("Finished downloading full text document for entry %0.",
-                        entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
-            } else {
-                String title = Localization.lang("No full text document found");
-                String message = Localization.lang("No full text document found for entry %0.",
-                        entry.getCiteKeyOptional().orElse(Localization.lang("undefined")));
 
-                basePanel.output(message);
-                DefaultTaskExecutor.runInJavaFXThread(() -> dialogService.showErrorDialogAndWait(title, message));
+                //Download full text
+                addLinkedFileFromURL(result.get(), entry);
+
+           } else {
+                dialogService.notify(Localization.lang("No full text document found for entry %0.",
+                        entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
             }
             finishedTasks.add(result);
         }
         for (Optional<URL> result : finishedTasks) {
             downloads.remove(result);
         }
     }
+
+    /**
+     * This method attaches a linked file from a URL (if not already linked) to an entry using the key and value pair
+     * from the findFullTexts map
+     * @param url the url "key"
+     * @param entry the entry "value"
+     */
+    private void addLinkedFileFromURL(URL url, BibEntry entry) {
+
+        LinkedFile newLinkedFile = new LinkedFile(url, "");
+
+        if (!entry.getFiles().contains(newLinkedFile)) {
+
+            LinkedFileViewModel onlineFile = new LinkedFileViewModel(
+                    newLinkedFile,
+                    entry,
+                    basePanel.getBibDatabaseContext(),
+                    Globals.TASK_EXECUTOR,
+                    dialogService,
+                    JabRefPreferences.getInstance());
+
+            onlineFile.download();
+
+            entry.addFile(onlineFile.getFile());
+
+            dialogService.notify(Localization.lang("Finished downloading full text document for entry %0.",
+                    entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
+        } else {
+            dialogService.notify(Localization.lang("Full text document for entry %0 already linked.",
+                    entry.getCiteKeyOptional().orElse(Localization.lang("undefined"))));
+        }
+    }
 }
diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -1991,6 +1991,7 @@ However,\ a\ new\ database\ was\ created\ alongside\ the\ pre-3.6\ one.=However,
 Opens\ a\ link\ where\ the\ current\ development\ version\ can\ be\ downloaded=Opens a link where the current development version can be downloaded
 See\ what\ has\ been\ changed\ in\ the\ JabRef\ versions=See what has been changed in the JabRef versions
 Referenced\ BibTeX\ key\ does\ not\ exist=Referenced BibTeX key does not exist
+Full\ text\ document\ for\ entry\ %0\ already\ linked.=Full text document for entry %0 already linked.
 Finished\ downloading\ full\ text\ document\ for\ entry\ %0.=Finished downloading full text document for entry %0.
 Look\ up\ full\ text\ documents=Look up full text documents
 You\ are\ about\ to\ look\ up\ full\ text\ documents\ for\ %0\ entries.=You are about to look up full text documents for %0 entries.
@@ -2132,7 +2133,6 @@ Any\ file=Any file
 
 No\ linked\ files\ found\ for\ export.=No linked files found for export.
 
-Full\ text\ document\ download\ failed\ for\ entry\ %0=Full text document download failed for entry %0
 No\ full\ text\ document\ found\ for\ entry\ %0.=No full text document found for entry %0.
 
 Delete\ Entry=Delete Entry