Report failed file downloads as notification instead of modal dialog

CHANGELOG.md

@@ -19,6 +19,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 
 - We fixed an issue where the AI chat lost its scroll position when switching back to an entry. [#17172](https://github.com/JabRef/jabref/pull/17172)
 - We fixed an issue where pressing "+" in the "File" field with an automatically found file selected opened the "Add file link" dialog instead of linking the selected file. [#16938](https://github.com/JabRef/jabref/pull/16938)
+- We fixed an issue where failed full text downloads opened an error dialog for each entry. [#774](https://github.com/JabRef/jabref-koppor/pull/774)
 - We fixed an issue where case-sensitive search (`=!`, `==!`, `=~!`) in linked files ignored the casing and matched text in any casing. [#13048](https://github.com/JabRef/jabref/issues/13048)
 
 ### Removed

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java

@@ -18,6 +18,7 @@
 import javafx.beans.property.SimpleDoubleProperty;
 
 import org.jabref.gui.DialogService;
+import org.jabref.gui.Notifications;
 import org.jabref.gui.actions.SimpleCommand;
 import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
@@ -222,15 +223,19 @@ private void onSuccess(Path targetDirectory, Path downloadedFile) {
         }
     }
 
+    /// Notifies instead of opening a modal dialog: the download runs in the background, often as one of many
+    /// (bulk full text download, import), and a blocked publisher must not stop the user with a dialog per entry.
     private void onFailure(URLDownload urlDownload, Exception ex) {
         LOGGER.error("Error downloading from URL: {}", urlDownload, ex);
-        if (ex instanceof FetcherException fetcherException) {
-            dialogService.showErrorDialogAndWait(fetcherException);
-        } else {
-            String fetcherExceptionMessage = ex.getLocalizedMessage();
-            String failedTitle = Localization.lang("Failed to download from URL");
-            dialogService.showErrorDialogAndWait(failedTitle, Localization.lang("Please check the URL and try again.\nURL: %0\nDetails: %1", urlDownload.getSource(), fetcherExceptionMessage));
-        }
+        // The response body (often a whole HTML page) stays in the log
+        String reason = Optional.of(ex)
+                                .filter(FetcherException.class::isInstance)
+                                .flatMap(e -> ((FetcherException) e).getHttpResponse())
+                                .map(response -> "HTTP %d %s".formatted(response.statusCode(), response.responseMessage()))
+                                .orElseGet(ex::getLocalizedMessage);
+        dialogService.notify(new Notifications.UndefinedNotification(
+                Localization.lang("Failed to download from URL"),
+                "%s\n%s\n%s".formatted(entry.getCitationKey().orElse(""), FetcherException.getRedactedUrl(urlDownload.getSource().toString()), reason).strip()));
     }
 
     private boolean checkSSLHandshake(URLDownload urlDownload) {

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java

@@ -19,6 +19,7 @@
 import javafx.scene.control.DialogPane;
 
 import org.jabref.gui.DialogService;
+import org.jabref.gui.Notifications;
 import org.jabref.gui.externalfiletype.ExternalFileType;
 import org.jabref.gui.externalfiletype.ExternalFileTypes;
 import org.jabref.gui.externalfiletype.StandardExternalFileType;
@@ -45,6 +46,7 @@
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.ArgumentCaptor;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -57,6 +59,7 @@
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
 
 // Need to run on JavaFX thread since {@link org.jabref.gui.linkedfile.DeleteFileAction.execute} creates a DialogPane
@@ -112,13 +115,17 @@ void tearDown() {
 
     /// Serves the given bytes for every request on a random free port, so download tests do not depend on external sites
     private String serve(String contentType, byte[] body) throws IOException {
+        return serve(200, contentType, body);
+    }
+
+    private String serve(int status, String contentType, byte[] body) throws IOException {
         httpServer = HttpServer.create(new InetSocketAddress(InetAddress.getLoopbackAddress(), 0), 0);
         httpServer.createContext("/", exchange -> {
             exchange.getResponseHeaders().add("Content-Type", contentType);
             if ("HEAD".equals(exchange.getRequestMethod())) {
                 exchange.sendResponseHeaders(200, -1);
             } else {
-                exchange.sendResponseHeaders(200, body.length);
+                exchange.sendResponseHeaders(status, body.length);
                 exchange.getResponseBody().write(body);
             }
             exchange.close();
@@ -234,6 +241,25 @@ void downloadHtmlFileCausesWarningDisplay(Boolean keepHtmlLink, String warningSu
         verify(dialogService, atLeastOnce()).notify(warningText);
     }
 
+    @Test
+    void failedDownloadNotifiesInsteadOfOpeningDialog() throws IOException {
+        when(filePreferences.shouldStoreFilesRelativeToBibFile()).thenReturn(true);
+        when(filePreferences.getFileNamePattern()).thenReturn("[citationkey]");
+        when(filePreferences.getFileDirectoryPattern()).thenReturn("");
+        databaseContext.setDatabasePath(tempFile);
+        String serverUrl = serve(403, "text/html", "<html>Just a moment...</html>".getBytes(StandardCharsets.UTF_8));
+        linkedFile = new LinkedFile(URLUtil.create(serverUrl), "");
+
+        LinkedFileViewModel viewModel = new LinkedFileViewModel(linkedFile, entry, databaseContext, new CurrentThreadTaskExecutor(), dialogService, preferences);
+        viewModel.download(false, new JabRefUndoManager());
+
+        ArgumentCaptor<Notifications.UndefinedNotification> notification = ArgumentCaptor.forClass(Notifications.UndefinedNotification.class);
+        verify(dialogService).notify(notification.capture());
+        verifyNoMoreInteractions(dialogService);
+        assertEquals("Failed to download from URL", notification.getValue().getTitle());
+        assertEquals("asdf\n" + serverUrl + "\nHTTP 403 Forbidden", notification.getValue().getSummary());
+    }
+
     @Test
     void isNotSamePath() {
         linkedFile = new LinkedFile("desc", tempFile, "pdf");

jablib/src/main/resources/l10n/JabRef_en.properties

@@ -3001,7 +3001,6 @@ Access\ denied.\ You\ do\ not\ have\ permission\ to\ access\ this\ resource.\ Pl
 The\ requested\ resource\ could\ not\ be\ found.\ It\ seems\ that\ the\ file\ you\ are\ trying\ to\ download\ is\ not\ available\ or\ has\ been\ moved.\ Please\ verify\ the\ URL\ and\ try\ again.\ If\ you\ believe\ this\ is\ an\ error,\ please\ contact\ the\ administrator\ for\ further\ assistance.=The requested resource could not be found. It seems that the file you are trying to download is not available or has been moved. Please verify the URL and try again. If you believe this is an error, please contact the administrator for further assistance.
 Something\ is\ wrong\ on\ JabRef\ side.\ Please\ check\ the\ URL\ and\ try\ again.=Something is wrong on JabRef side. Please check the URL and try again.
 Error\ downloading\ from\ URL.\ Cause\ is\ likely\ the\ server\ side.\nPlease\ try\ again\ later\ or\ contact\ the\ server\ administrator.=Error downloading from URL. Cause is likely the server side.\nPlease try again later or contact the server administrator.
-Please\ check\ the\ URL\ and\ try\ again.\nURL\:\ %0\nDetails\:\ %1=Please check the URL and try again.\nURL: %0\nDetails: %1
 
 Finished=Finished
 Finished\ writing\ metadata\ for\ library\ %0\ (%1\ succeeded,\ %2\ skipped,\ %3\ errors).=Finished writing metadata for library %0 (%1 succeeded, %2 skipped, %3 errors).