Fixed Grobid Preference Dialog Logic, Removed Checkbox (#12034)

* Modified GROBID dialog logic, allowing users to explicitly save GROBID usage preference. "Do not ask" checkbox replaced with "Save Preference", and GROBID_OPTOUT replaced with GROBID_PREFERENCE to reduce ambiguity. * Removed checkbox entirely, dialog logic updated such that users will only be asked about Grobid permissions once, with the preference being stored for the future. Updated CHANGELOG.md * Updated dialog wording to be 'Send to Grobid' and 'Do not send' to improve clarity. * Refactored grobidPreference boolean to grobidUseAsked to improve clarity. Renamed GrobidPreferenceDialogHelper.java to GrobidUseDialogHelper.java. * Update CHANGELOG.md Co-authored-by: Christoph <[email protected]> * Updated JabRef_en.properties to reflect language keys: "Send to Grobid" and "Do not send". * Modified Grobid dialog logic such that users are not prompted to enable Grobid on first use, if Grobid has already been manually enabled. * Update src/main/java/org/jabref/gui/importer/GrobidUseDialogHelper.java Co-authored-by: Loay Ghreeb <[email protected]> --------- Co-authored-by: Christoph <[email protected]> Co-authored-by: Oliver Kopp <[email protected]> Co-authored-by: Loay Ghreeb <[email protected]>
JabRef · Oct 27, 2024 · ce27eb3 · ce27eb3
1 parent aa31a58
commit ce27eb3
Show file tree

Hide file tree

Showing 9 changed files with 38 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -105,6 +105,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 <kbd>Tab</kbd> 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 the "Do not ask again" checkbox was not working, when asking for permission to use Grobid [koppor#556](https://github.com/koppor/jabref/issues/566).
 - 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)

diff --git a/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java b/src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
@@ -37,7 +37,7 @@
 import org.jabref.gui.entryeditor.fileannotationtab.FulltextSearchResultsTab;
 import org.jabref.gui.externalfiles.ExternalFilesEntryLinker;
 import org.jabref.gui.help.HelpAction;
-import org.jabref.gui.importer.GrobidOptInDialogHelper;
+import org.jabref.gui.importer.GrobidUseDialogHelper;
 import org.jabref.gui.keyboard.KeyBinding;
 import org.jabref.gui.keyboard.KeyBindingRepository;
 import org.jabref.gui.menus.ChangeEntryTypeMenu;
@@ -423,7 +423,7 @@ private void setupToolBar() {
             if (fetcher instanceof PdfMergeMetadataImporter.EntryBasedFetcherWrapper) {
                 // Handle Grobid Opt-In in case of the PdfMergeMetadataImporter
                 fetcherMenuItem.setOnAction(event -> {
-                    GrobidOptInDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences());
+                    GrobidUseDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences());
                     PdfMergeMetadataImporter.EntryBasedFetcherWrapper pdfMergeMetadataImporter =
                             new PdfMergeMetadataImporter.EntryBasedFetcherWrapper(
                                     preferences.getImportFormatPreferences(),

diff --git a/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java b/src/main/java/org/jabref/gui/fieldeditors/LinkedFilesEditor.java
@@ -41,7 +41,7 @@
 import org.jabref.gui.copyfiles.CopySingleFileAction;
 import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.icon.JabRefIconView;
-import org.jabref.gui.importer.GrobidOptInDialogHelper;
+import org.jabref.gui.importer.GrobidUseDialogHelper;
 import org.jabref.gui.keyboard.KeyBinding;
 import org.jabref.gui.linkedfile.DeleteFileAction;
 import org.jabref.gui.linkedfile.LinkedFileEditDialog;
@@ -236,7 +236,7 @@ private Node createFileDisplay(LinkedFileViewModel linkedFile) {
         parsePdfMetadata.setTooltip(new Tooltip(Localization.lang("Parse Metadata from PDF.")));
         parsePdfMetadata.visibleProperty().bind(linkedFile.isOfflinePdfProperty());
         parsePdfMetadata.setOnAction(event -> {
-            GrobidOptInDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences());
+            GrobidUseDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences());
             linkedFile.parsePdfMetadataAndShowMergeDialog();
         });
         parsePdfMetadata.getStyleClass().setAll("icon-button");

diff --git a/...gui/importer/GrobidOptInDialogHelper.java → ...f/gui/importer/GrobidUseDialogHelper.java b/...gui/importer/GrobidOptInDialogHelper.java → ...f/gui/importer/GrobidUseDialogHelper.java
@@ -7,29 +7,31 @@
 /**
  * Metadata extraction from PDFs and plaintext works very well using Grobid, but we do not want to enable it by default
  * due to data privacy concerns.
- * To make users aware of the feature, we ask each time before querying Grobid, giving the option to opt-out.
+ * To make users aware of the feature, we ask before querying for the first time Grobid, saving the users' preference
  */
-public class GrobidOptInDialogHelper {
+public class GrobidUseDialogHelper {
 
     /**
-     * If Grobid is not enabled but the user has not explicitly opted-out of Grobid, we ask for permission to send data
-     * to Grobid using a dialog and giving an opt-out option.
+     * If the user has not explicitly opted-in/out of Grobid, we ask for permission to send data to Grobid by using
+     * a dialog. The users' preference is saved.
      *
      * @param dialogService the DialogService to use
      * @return if the user enabled Grobid, either in the past or after being asked by the dialog.
      */
     public static boolean showAndWaitIfUserIsUndecided(DialogService dialogService, GrobidPreferences preferences) {
+        if (preferences.isGrobidUseAsked()) {
+            return preferences.isGrobidEnabled();
+        }
         if (preferences.isGrobidEnabled()) {
+            preferences.setGrobidUseAsked(true);
             return true;
         }
-        if (preferences.isGrobidOptOut()) {
-            return false;
-        }
-        boolean grobidEnabled = dialogService.showConfirmationDialogWithOptOutAndWait(
+        boolean grobidEnabled = dialogService.showConfirmationDialogAndWait(
                 Localization.lang("Remote services"),
                 Localization.lang("Allow sending PDF files and raw citation strings to a JabRef online service (Grobid) to determine Metadata. This produces better results."),
-                Localization.lang("Do not ask again"),
-                optOut -> preferences.setGrobidOptOut(optOut));
+                Localization.lang("Send to Grobid"),
+                Localization.lang("Do not send"));
+        preferences.setGrobidUseAsked(true);
         preferences.setGrobidEnabled(grobidEnabled);
         return grobidEnabled;
     }

diff --git a/src/main/java/org/jabref/gui/importer/ImportCommand.java b/src/main/java/org/jabref/gui/importer/ImportCommand.java
@@ -144,7 +144,7 @@ private ParserResult doImport(List<Path> files, Importer importFormat) throws IO
                 if (importer.isEmpty()) {
                     // Unknown format
                     UiTaskExecutor.runAndWaitInJavaFXThread(() -> {
-                        if (FileUtil.isPDFFile(filename) && GrobidOptInDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences())) {
+                        if (FileUtil.isPDFFile(filename) && GrobidUseDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences())) {
                             importFormatReader.reset();
                         }
                         dialogService.notify(Localization.lang("Importing file %0 as unknown format", filename.getFileName().toString()));
@@ -155,7 +155,7 @@ private ParserResult doImport(List<Path> files, Importer importFormat) throws IO
                     UiTaskExecutor.runAndWaitInJavaFXThread(() -> {
                         if (((importer.get() instanceof PdfGrobidImporter)
                                 || (importer.get() instanceof PdfMergeMetadataImporter))
-                                && GrobidOptInDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences())) {
+                                && GrobidUseDialogHelper.showAndWaitIfUserIsUndecided(dialogService, preferences.getGrobidPreferences())) {
                             importFormatReader.reset();
                         }
                         dialogService.notify(Localization.lang("Importing in %0 format", importer.get().getName()) + "...");

diff --git a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java
@@ -164,7 +164,7 @@ public void storeSettings() {
         filePreferences.setKeepDownloadUrl(shouldkeepDownloadUrl.getValue());
         importerPreferences.setDefaultPlainCitationParser(defaultPlainCitationParser.getValue());
         grobidPreferences.setGrobidEnabled(grobidEnabledProperty.getValue());
-        grobidPreferences.setGrobidOptOut(grobidPreferences.isGrobidOptOut());
+        grobidPreferences.setGrobidUseAsked(grobidPreferences.isGrobidUseAsked());
         grobidPreferences.setGrobidURL(grobidURLProperty.getValue());
         doiPreferences.setUseCustom(useCustomDOIProperty.get());
         doiPreferences.setDefaultBaseURI(useCustomDOINameProperty.getValue().trim());

diff --git a/src/main/java/org/jabref/logic/importer/util/GrobidPreferences.java b/src/main/java/org/jabref/logic/importer/util/GrobidPreferences.java
@@ -7,14 +7,14 @@
 
 public class GrobidPreferences {
     private final BooleanProperty grobidEnabled;
-    private final BooleanProperty grobidOptOut;
+    private final BooleanProperty grobidUseAsked;
     private final StringProperty grobidURL;
 
     public GrobidPreferences(boolean grobidEnabled,
-                             boolean grobidOptOut,
+                             boolean grobidUseAsked,
                              String grobidURL) {
         this.grobidEnabled = new SimpleBooleanProperty(grobidEnabled);
-        this.grobidOptOut = new SimpleBooleanProperty(grobidOptOut);
+        this.grobidUseAsked = new SimpleBooleanProperty(grobidUseAsked);
         this.grobidURL = new SimpleStringProperty(grobidURL);
     }
 
@@ -30,19 +30,19 @@ public void setGrobidEnabled(boolean grobidEnabled) {
         this.grobidEnabled.set(grobidEnabled);
     }
 
-    // region: optout; models "Do not ask again" option
-    public boolean isGrobidOptOut() {
-        return grobidOptOut.get();
+    // region: GrobidUseAsked;
+    public boolean isGrobidUseAsked() {
+        return grobidUseAsked.get();
     }
 
-    public BooleanProperty grobidOptOutProperty() {
-        return grobidOptOut;
+    public BooleanProperty grobidUseAskedProperty() {
+        return grobidUseAsked;
     }
 
-    public void setGrobidOptOut(boolean grobidOptOut) {
-        this.grobidOptOut.set(grobidOptOut);
+    public void setGrobidUseAsked(boolean grobidUseAsked) {
+        this.grobidUseAsked.set(grobidUseAsked);
     }
-    // endregion: optout
+    // endregion: GrobidUseAsked
 
     public String getGrobidURL() {
         return grobidURL.get();

diff --git a/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java b/src/main/java/org/jabref/logic/preferences/JabRefCliPreferences.java
@@ -222,7 +222,7 @@ public class JabRefCliPreferences implements CliPreferences {
     public static final String IMPORTERS_ENABLED = "importersEnabled";
     public static final String GENERATE_KEY_ON_IMPORT = "generateKeyOnImport";
     public static final String GROBID_ENABLED = "grobidEnabled";
-    public static final String GROBID_OPT_OUT = "grobidOptOut";
+    public static final String GROBID_PREFERENCE = "grobidPreference";
     public static final String GROBID_URL = "grobidURL";
 
     public static final String DEFAULT_CITATION_KEY_PATTERN = "defaultBibtexKeyPattern";
@@ -457,7 +457,7 @@ protected JabRefCliPreferences() {
 
         // region: Grobid
         defaults.put(GROBID_ENABLED, Boolean.FALSE);
-        defaults.put(GROBID_OPT_OUT, Boolean.FALSE);
+        defaults.put(GROBID_PREFERENCE, Boolean.FALSE);
         defaults.put(GROBID_URL, "http://grobid.jabref.org:8070");
         // endregion
 
@@ -2189,11 +2189,11 @@ public GrobidPreferences getGrobidPreferences() {
 
         grobidPreferences = new GrobidPreferences(
                 getBoolean(GROBID_ENABLED),
-                getBoolean(GROBID_OPT_OUT),
+                getBoolean(GROBID_PREFERENCE),
                 get(GROBID_URL));
 
         EasyBind.listen(grobidPreferences.grobidEnabledProperty(), (obs, oldValue, newValue) -> putBoolean(GROBID_ENABLED, newValue));
-        EasyBind.listen(grobidPreferences.grobidOptOutProperty(), (obs, oldValue, newValue) -> putBoolean(GROBID_OPT_OUT, newValue));
+        EasyBind.listen(grobidPreferences.grobidUseAskedProperty(), (obs, oldValue, newValue) -> putBoolean(GROBID_PREFERENCE, newValue));
         EasyBind.listen(grobidPreferences.grobidURLProperty(), (obs, oldValue, newValue) -> put(GROBID_URL, newValue));
 
         return grobidPreferences;

diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties
@@ -2804,3 +2804,6 @@ 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.
+
+Send\ to\ Grobid=Send to Grobid
+Do\ not\ send=Do not send