From c6d62bc939f737127a0523cb16195ec253002aa5 Mon Sep 17 00:00:00 2001 From: zirunye Date: Mon, 16 Sep 2024 20:27:30 +0800 Subject: [PATCH 1/5] minor refactor to JabRefDialogService 1. replace multiple if-else to switch case 2. remove unused generics `` 3. replace `.collect(Collectors.toList())` to `.toList()` 4. remove unnecessary braces --- .../org/jabref/gui/JabRefDialogService.java | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java index 9d907840e43..1f2e38b8506 100644 --- a/src/main/java/org/jabref/gui/JabRefDialogService.java +++ b/src/main/java/org/jabref/gui/JabRefDialogService.java @@ -10,7 +10,6 @@ import java.util.List; import java.util.Optional; import java.util.function.Consumer; -import java.util.stream.Collectors; import javafx.concurrent.Task; import javafx.geometry.Pos; @@ -227,14 +226,15 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { Optional httpResponse = fetcherException.getHttpResponse(); if (httpResponse.isPresent()) { int statusCode = httpResponse.get().statusCode(); - if (statusCode == 401) { - this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance.") + "\n\n" + localizedMessage); - } else if (statusCode == 403) { - this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action.") + "\n\n" + localizedMessage); - } else if (statusCode == 404) { - this.showInformationDialogAndWait(failedTitle, Localization.lang("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.") + "\n\n" + localizedMessage); - } else { - this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); + switch (statusCode) { + case 401 -> + this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance.") + "\n\n" + localizedMessage); + case 403 -> + this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action.") + "\n\n" + localizedMessage); + case 404 -> + this.showInformationDialogAndWait(failedTitle, Localization.lang("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.") + "\n\n" + localizedMessage); + default -> + this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); } } else if (fetcherException instanceof FetcherClientException) { this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); @@ -388,7 +388,7 @@ public void showProgressDialogAndWait(String title, String content, Task } @Override - public Optional showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) { + public Optional showBackgroundProgressDialogAndWait(String title, String content, StateManager stateManager) { TaskProgressView> taskProgressView = new TaskProgressView<>(); EasyBind.bindContent(taskProgressView.getTasks(), stateManager.getRunningBackgroundTasks()); taskProgressView.setRetainTasks(false); @@ -424,25 +424,23 @@ public void notify(String message) { // The event log is not that user friendly (different purpose). LOGGER.info(message); - UiTaskExecutor.runInJavaFXThread(() -> { - Notifications.create() - .text(message) - .position(Pos.BOTTOM_CENTER) - .hideAfter(TOAST_MESSAGE_DISPLAY_TIME) - .owner(mainWindow) - .threshold(5, - Notifications.create() - .title(Localization.lang("Last notification")) - .text( - "(" + Localization.lang("Check the event log to see all notifications") + ")" - + "\n\n" + message) - .onAction(e -> { - ErrorConsoleAction ec = new ErrorConsoleAction(); - ec.execute(); - })) - .hideCloseButton() - .show(); - }); + UiTaskExecutor.runInJavaFXThread(() -> Notifications.create() + .text(message) + .position(Pos.BOTTOM_CENTER) + .hideAfter(TOAST_MESSAGE_DISPLAY_TIME) + .owner(mainWindow) + .threshold(5, + Notifications.create() + .title(Localization.lang("Last notification")) + .text( + "(" + Localization.lang("Check the event log to see all notifications") + ")" + + "\n\n" + message) + .onAction(e -> { + ErrorConsoleAction ec = new ErrorConsoleAction(); + ec.execute(); + })) + .hideCloseButton() + .show()); } @Override @@ -472,7 +470,7 @@ public Optional showDirectorySelectionDialog(DirectoryDialogConfiguration public List showFileOpenDialogAndGetMultipleFiles(FileDialogConfiguration fileDialogConfiguration) { FileChooser chooser = getConfiguredFileChooser(fileDialogConfiguration); List files = chooser.showOpenMultipleDialog(mainWindow); - return files != null ? files.stream().map(File::toPath).collect(Collectors.toList()) : Collections.emptyList(); + return files != null ? files.stream().map(File::toPath).toList() : Collections.emptyList(); } private DirectoryChooser getConfiguredDirectoryChooser(DirectoryDialogConfiguration directoryDialogConfiguration) { From fd3b7f088c86c1a849fba4b410d68b3249166a9b Mon Sep 17 00:00:00 2001 From: zirunye Date: Tue, 17 Sep 2024 09:48:19 +0800 Subject: [PATCH 2/5] Update JabRefDialogService 1. remove `type` parameter in createDialogWithOptOut, since it always be `AlertType.CONFIRMATION` 2. remove `Math.min` in `shortenDialogMessage`, since it after a if compare, it means the dialogMessage.length() is always not less than JabRefDialogService.DIALOG_SIZE_LIMIT 3. refactor the switch case more. 4. fix the indent issue. --- .../org/jabref/gui/JabRefDialogService.java | 73 ++++++++++--------- 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java index 1f2e38b8506..48de534710f 100644 --- a/src/main/java/org/jabref/gui/JabRefDialogService.java +++ b/src/main/java/org/jabref/gui/JabRefDialogService.java @@ -6,7 +6,6 @@ import java.nio.file.FileSystems; import java.nio.file.Path; import java.util.Collection; -import java.util.Collections; import java.util.List; import java.util.Optional; import java.util.function.Consumer; @@ -98,9 +97,9 @@ private FXDialog createDialog(AlertType type, String title, String content) { return alert; } - private FXDialog createDialogWithOptOut(AlertType type, String title, String content, + private FXDialog createDialogWithOptOut(String title, String content, String optOutMessage, Consumer optOutAction) { - FXDialog alert = new FXDialog(type, title, true); + FXDialog alert = new FXDialog(AlertType.CONFIRMATION, title, true); // Need to force the alert to layout in order to grab the graphic as we are replacing the dialog pane with a custom pane alert.getDialogPane().applyCss(); Node graphic = alert.getDialogPane().getGraphic(); @@ -134,7 +133,7 @@ public static String shortenDialogMessage(String dialogMessage) { if (dialogMessage.length() < JabRefDialogService.DIALOG_SIZE_LIMIT) { return dialogMessage.trim(); } - return (dialogMessage.substring(0, Math.min(dialogMessage.length(), JabRefDialogService.DIALOG_SIZE_LIMIT)) + "...").trim(); + return (dialogMessage.substring(0, JabRefDialogService.DIALOG_SIZE_LIMIT) + "...").trim(); } private ChoiceDialog createChoiceDialog(String title, String content, String okButtonLabel, T defaultChoice, Collection choices) { @@ -225,17 +224,7 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { String localizedMessage = fetcherException.getLocalizedMessage(); Optional httpResponse = fetcherException.getHttpResponse(); if (httpResponse.isPresent()) { - int statusCode = httpResponse.get().statusCode(); - switch (statusCode) { - case 401 -> - this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance.") + "\n\n" + localizedMessage); - case 403 -> - this.showInformationDialogAndWait(failedTitle, Localization.lang("Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action.") + "\n\n" + localizedMessage); - case 404 -> - this.showInformationDialogAndWait(failedTitle, Localization.lang("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.") + "\n\n" + localizedMessage); - default -> - this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); - } + this.showInformationDialogAndWait(failedTitle, getContentByCode(httpResponse.get().statusCode())); } else if (fetcherException instanceof FetcherClientException) { this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); } else if (fetcherException instanceof FetcherServerException) { @@ -246,6 +235,19 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { } } + private String getContentByCode(int statusCode) { + return switch (statusCode) { + case 401 -> + "Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance."; + case 403 -> + "Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action."; + case 404 -> + "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."; + default -> + "Something is wrong on JabRef side. Please check the URL and try again."; + }; + } + @Override public void showErrorDialogAndWait(String title, String content, Throwable exception) { ExceptionDialog exceptionDialog = new ExceptionDialog(exception); @@ -288,7 +290,7 @@ public boolean showConfirmationDialogAndWait(String title, String content, @Override public boolean showConfirmationDialogWithOptOutAndWait(String title, String content, String optOutMessage, Consumer optOutAction) { - FXDialog alert = createDialogWithOptOut(AlertType.CONFIRMATION, title, content, optOutMessage, optOutAction); + FXDialog alert = createDialogWithOptOut(title, content, optOutMessage, optOutAction); alert.getButtonTypes().setAll(ButtonType.YES, ButtonType.NO); return alert.showAndWait().filter(buttonType -> buttonType == ButtonType.YES).isPresent(); } @@ -297,7 +299,7 @@ public boolean showConfirmationDialogWithOptOutAndWait(String title, String cont public boolean showConfirmationDialogWithOptOutAndWait(String title, String content, String okButtonLabel, String cancelButtonLabel, String optOutMessage, Consumer optOutAction) { - FXDialog alert = createDialogWithOptOut(AlertType.CONFIRMATION, title, content, optOutMessage, optOutAction); + FXDialog alert = createDialogWithOptOut(title, content, optOutMessage, optOutAction); ButtonType okButtonType = new ButtonType(okButtonLabel, ButtonBar.ButtonData.YES); ButtonType cancelButtonType = new ButtonType(cancelButtonLabel, ButtonBar.ButtonData.NO); alert.getButtonTypes().setAll(okButtonType, cancelButtonType); @@ -424,23 +426,24 @@ public void notify(String message) { // The event log is not that user friendly (different purpose). LOGGER.info(message); - UiTaskExecutor.runInJavaFXThread(() -> Notifications.create() - .text(message) - .position(Pos.BOTTOM_CENTER) - .hideAfter(TOAST_MESSAGE_DISPLAY_TIME) - .owner(mainWindow) - .threshold(5, - Notifications.create() - .title(Localization.lang("Last notification")) - .text( - "(" + Localization.lang("Check the event log to see all notifications") + ")" - + "\n\n" + message) - .onAction(e -> { - ErrorConsoleAction ec = new ErrorConsoleAction(); - ec.execute(); - })) - .hideCloseButton() - .show()); + UiTaskExecutor.runInJavaFXThread(() -> + Notifications.create() + .text(message) + .position(Pos.BOTTOM_CENTER) + .hideAfter(TOAST_MESSAGE_DISPLAY_TIME) + .owner(mainWindow) + .threshold(5, + Notifications.create() + .title(Localization.lang("Last notification")) + .text( + "(" + Localization.lang("Check the event log to see all notifications") + ")" + + "\n\n" + message) + .onAction(e -> { + ErrorConsoleAction ec = new ErrorConsoleAction(); + ec.execute(); + })) + .hideCloseButton() + .show()); } @Override @@ -470,7 +473,7 @@ public Optional showDirectorySelectionDialog(DirectoryDialogConfiguration public List showFileOpenDialogAndGetMultipleFiles(FileDialogConfiguration fileDialogConfiguration) { FileChooser chooser = getConfiguredFileChooser(fileDialogConfiguration); List files = chooser.showOpenMultipleDialog(mainWindow); - return files != null ? files.stream().map(File::toPath).toList() : Collections.emptyList(); + return files != null ? files.stream().map(File::toPath).toList() : List.of(); } private DirectoryChooser getConfiguredDirectoryChooser(DirectoryDialogConfiguration directoryDialogConfiguration) { From 53d046c138062ee0432439af8cb13101bc4f8a20 Mon Sep 17 00:00:00 2001 From: zirunye Date: Tue, 17 Sep 2024 16:17:38 +0800 Subject: [PATCH 3/5] fix missing Localization.lang fix missing Localization.lang --- src/main/java/org/jabref/gui/JabRefDialogService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java index 48de534710f..ebac3022d29 100644 --- a/src/main/java/org/jabref/gui/JabRefDialogService.java +++ b/src/main/java/org/jabref/gui/JabRefDialogService.java @@ -224,7 +224,7 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { String localizedMessage = fetcherException.getLocalizedMessage(); Optional httpResponse = fetcherException.getHttpResponse(); if (httpResponse.isPresent()) { - this.showInformationDialogAndWait(failedTitle, getContentByCode(httpResponse.get().statusCode())); + this.showInformationDialogAndWait(failedTitle, Localization.lang(getContentByCode(httpResponse.get().statusCode()))); } else if (fetcherException instanceof FetcherClientException) { this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); } else if (fetcherException instanceof FetcherServerException) { From 15ec6de01fbc6119026f58358b7a54ac52dbe9de Mon Sep 17 00:00:00 2001 From: zirunye Date: Tue, 17 Sep 2024 17:24:17 +0800 Subject: [PATCH 4/5] fix missing string add missing newline character and localizedMessage --- src/main/java/org/jabref/gui/JabRefDialogService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java index ebac3022d29..60f3e104e0a 100644 --- a/src/main/java/org/jabref/gui/JabRefDialogService.java +++ b/src/main/java/org/jabref/gui/JabRefDialogService.java @@ -224,7 +224,7 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { String localizedMessage = fetcherException.getLocalizedMessage(); Optional httpResponse = fetcherException.getHttpResponse(); if (httpResponse.isPresent()) { - this.showInformationDialogAndWait(failedTitle, Localization.lang(getContentByCode(httpResponse.get().statusCode()))); + this.showInformationDialogAndWait(failedTitle, Localization.lang(getContentByCode(httpResponse.get().statusCode())) + "\n\n" + localizedMessage); } else if (fetcherException instanceof FetcherClientException) { this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); } else if (fetcherException instanceof FetcherServerException) { From a9fcc67414c1942d5a7e2bdc769c4882ee2a41be Mon Sep 17 00:00:00 2001 From: zirunye Date: Wed, 18 Sep 2024 07:11:52 +0800 Subject: [PATCH 5/5] fix Localization.lang issue need to use plain string not a method result or a var to pass the unit test. --- .../org/jabref/gui/JabRefDialogService.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/jabref/gui/JabRefDialogService.java b/src/main/java/org/jabref/gui/JabRefDialogService.java index 60f3e104e0a..6d0ad1ceeb8 100644 --- a/src/main/java/org/jabref/gui/JabRefDialogService.java +++ b/src/main/java/org/jabref/gui/JabRefDialogService.java @@ -224,7 +224,7 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { String localizedMessage = fetcherException.getLocalizedMessage(); Optional httpResponse = fetcherException.getHttpResponse(); if (httpResponse.isPresent()) { - this.showInformationDialogAndWait(failedTitle, Localization.lang(getContentByCode(httpResponse.get().statusCode())) + "\n\n" + localizedMessage); + this.showInformationDialogAndWait(failedTitle, getContentByCode(httpResponse.get().statusCode()) + "\n\n" + localizedMessage); } else if (fetcherException instanceof FetcherClientException) { this.showErrorDialogAndWait(failedTitle, Localization.lang("Something is wrong on JabRef side. Please check the URL and try again.") + "\n\n" + localizedMessage); } else if (fetcherException instanceof FetcherServerException) { @@ -235,19 +235,6 @@ public void showErrorDialogAndWait(FetcherException fetcherException) { } } - private String getContentByCode(int statusCode) { - return switch (statusCode) { - case 401 -> - "Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance."; - case 403 -> - "Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action."; - case 404 -> - "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."; - default -> - "Something is wrong on JabRef side. Please check the URL and try again."; - }; - } - @Override public void showErrorDialogAndWait(String title, String content, Throwable exception) { ExceptionDialog exceptionDialog = new ExceptionDialog(exception); @@ -521,4 +508,17 @@ public void showCustomWindow(BaseWindow window) { window.applyStylesheets(mainWindow.getScene().getStylesheets()); window.show(); } + + private String getContentByCode(int statusCode) { + return switch (statusCode) { + case 401 -> + Localization.lang("Access denied. You are not authorized to access this resource. Please check your credentials and try again. If you believe you should have access, please contact the administrator for assistance."); + case 403 -> + Localization.lang("Access denied. You do not have permission to access this resource. Please contact the administrator for assistance or try a different action."); + case 404 -> + Localization.lang("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."); + default -> + Localization.lang("Something is wrong on JabRef side. Please check the URL and try again."); + }; + } }