From 4c9c821e675dea3befec34162fac8bc4ccaf21ab Mon Sep 17 00:00:00 2001
From: Priyanshu Gupta <priyanshu16095@gmail.com>
Date: Fri, 7 Feb 2025 01:28:24 +0530
Subject: [PATCH] Move generate key option to citation key preference tab
 (#12436)

* Move 'Generate key on import' option from WebSearch tab to CitationKeyTab

* Takes preferences as arguments

* Add correct comment

* Fix javadoc link, shift `importerPreferences` declaration below

---------

Co-authored-by: Subhramit Basu Bhowmick <subhramit.bb@live.in>
---
 CHANGELOG.md                                  |  1 +
 .../gui/externalfiles/ImportHandler.java      |  2 +-
 .../CitationKeyPatternTab.fxml                |  1 +
 .../CitationKeyPatternTab.java                |  4 +++-
 .../CitationKeyPatternTabViewModel.java       | 19 ++++++++++++++++++-
 .../preferences/websearch/WebSearchTab.fxml   |  1 -
 .../preferences/websearch/WebSearchTab.java   |  2 --
 .../websearch/WebSearchTabViewModel.java      |  7 -------
 .../logic/importer/ImporterPreferences.java   |  2 +-
 .../CitationsRelationsTabViewModelTest.java   |  2 +-
 10 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index bccf8db02be..a61f39170fe 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
 
 ### Changed
 
+- We moved the "Generate a new key for imported entries" option from the "Web search" tab to the "Citation key generator" tab in preferences. [#12436](https://github.com/JabRef/jabref/pull/12436)
 - We improved the offline parsing of BibTeX data from PDF-documents. [#12278](https://github.com/JabRef/jabref/issues/12278)
 - The tab bar is now hidden when only one library is open. [#9971](https://github.com/JabRef/jabref/issues/9971)
 
diff --git a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
index 58a4f4eabe8..5555ef03fda 100644
--- a/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
+++ b/src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
@@ -351,7 +351,7 @@ private void addToGroups(List<BibEntry> entries, Collection<GroupTreeNode> group
      * @param entries entries to generate keys for
      */
     private void generateKeys(List<BibEntry> entries) {
-        if (!preferences.getImporterPreferences().isGenerateNewKeyOnImport()) {
+        if (!preferences.getImporterPreferences().shouldGenerateNewKeyOnImport()) {
             return;
         }
         CitationKeyGenerator keyGenerator = new CitationKeyGenerator(
diff --git a/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.fxml b/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.fxml
index 3b2a5bb0103..adc13ae7070 100644
--- a/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.fxml
+++ b/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.fxml
@@ -27,6 +27,7 @@
         </padding>
     </CheckBox>
     <CheckBox fx:id="generateOnSave" text="%Generate keys before saving (for entries without a key)"/>
+    <CheckBox fx:id="generateNewKeyOnImport" text="%Generate a new key for imported entries (overwriting their default)"/>
 
     <Label text="%Letters after duplicate generated keys"/>
     <VBox spacing="10.0">
diff --git a/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.java b/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.java
index 99133e31a1c..dac2489ca0e 100644
--- a/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.java
+++ b/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTab.java
@@ -24,6 +24,7 @@ public class CitationKeyPatternTab extends AbstractPreferenceTabView<CitationKey
     @FXML private CheckBox overwriteAllow;
     @FXML private CheckBox overwriteWarning;
     @FXML private CheckBox generateOnSave;
+    @FXML private CheckBox generateNewKeyOnImport;
     @FXML private RadioButton letterStartA;
     @FXML private RadioButton letterStartB;
     @FXML private RadioButton letterAlwaysAdd;
@@ -45,11 +46,12 @@ public String getTabName() {
     }
 
     public void initialize() {
-        this.viewModel = new CitationKeyPatternTabViewModel(preferences.getCitationKeyPatternPreferences());
+        this.viewModel = new CitationKeyPatternTabViewModel(preferences.getCitationKeyPatternPreferences(), preferences.getImporterPreferences());
 
         overwriteAllow.selectedProperty().bindBidirectional(viewModel.overwriteAllowProperty());
         overwriteWarning.selectedProperty().bindBidirectional(viewModel.overwriteWarningProperty());
         generateOnSave.selectedProperty().bindBidirectional(viewModel.generateOnSaveProperty());
+        generateNewKeyOnImport.selectedProperty().bindBidirectional(viewModel.generateKeyOnImportProperty());
         letterStartA.selectedProperty().bindBidirectional(viewModel.letterStartAProperty());
         letterStartB.selectedProperty().bindBidirectional(viewModel.letterStartBProperty());
         letterAlwaysAdd.selectedProperty().bindBidirectional(viewModel.letterAlwaysAddProperty());
diff --git a/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTabViewModel.java b/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTabViewModel.java
index c653e840ba7..2ccfdf85897 100644
--- a/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTabViewModel.java
+++ b/src/main/java/org/jabref/gui/preferences/citationkeypattern/CitationKeyPatternTabViewModel.java
@@ -15,12 +15,14 @@
 import org.jabref.gui.preferences.PreferenceTabViewModel;
 import org.jabref.logic.citationkeypattern.CitationKeyPatternPreferences;
 import org.jabref.logic.citationkeypattern.GlobalCitationKeyPatterns;
+import org.jabref.logic.importer.ImporterPreferences;
 
 public class CitationKeyPatternTabViewModel implements PreferenceTabViewModel {
 
     private final BooleanProperty overwriteAllowProperty = new SimpleBooleanProperty();
     private final BooleanProperty overwriteWarningProperty = new SimpleBooleanProperty();
     private final BooleanProperty generateOnSaveProperty = new SimpleBooleanProperty();
+    private final BooleanProperty generateKeyOnImportProperty = new SimpleBooleanProperty();
     private final BooleanProperty letterStartAProperty = new SimpleBooleanProperty();
     private final BooleanProperty letterStartBProperty = new SimpleBooleanProperty();
     private final BooleanProperty letterAlwaysAddProperty = new SimpleBooleanProperty();
@@ -36,8 +38,17 @@ public class CitationKeyPatternTabViewModel implements PreferenceTabViewModel {
 
     private final CitationKeyPatternPreferences keyPatternPreferences;
 
-    public CitationKeyPatternTabViewModel(CitationKeyPatternPreferences keyPatternPreferences) {
+    /**
+     * The preference for whether to use the key generator on import is different from how it is configured.
+     * In the UI, there is no better place to put the option than the Citation Key Generator tab.
+     * However, shifting the preference to {@link CitationKeyPatternPreferences} would break the abstraction or hierarchy.
+     * Hence, we keep the preference in {@link ImporterPreferences}, but for the UI, we initialize it here.
+     */
+    private final ImporterPreferences importerPreferences;
+
+    public CitationKeyPatternTabViewModel(CitationKeyPatternPreferences keyPatternPreferences, ImporterPreferences importerPreferences) {
         this.keyPatternPreferences = keyPatternPreferences;
+        this.importerPreferences = importerPreferences;
     }
 
     @Override
@@ -45,6 +56,7 @@ public void setValues() {
         overwriteAllowProperty.setValue(!keyPatternPreferences.shouldAvoidOverwriteCiteKey());
         overwriteWarningProperty.setValue(keyPatternPreferences.shouldWarnBeforeOverwriteCiteKey());
         generateOnSaveProperty.setValue(keyPatternPreferences.shouldGenerateCiteKeysBeforeSaving());
+        generateKeyOnImportProperty.setValue(importerPreferences.shouldGenerateNewKeyOnImport());
 
         if (keyPatternPreferences.getKeySuffix()
                 == CitationKeyPatternPreferences.KeySuffix.ALWAYS) {
@@ -97,6 +109,7 @@ public void storeSettings() {
         keyPatternPreferences.setAvoidOverwriteCiteKey(!overwriteAllowProperty.getValue());
         keyPatternPreferences.setWarnBeforeOverwriteCiteKey(overwriteWarningProperty.getValue());
         keyPatternPreferences.setGenerateCiteKeysBeforeSaving(generateOnSaveProperty.getValue());
+        importerPreferences.setGenerateNewKeyOnImport(generateKeyOnImportProperty.getValue());
         keyPatternPreferences.setKeySuffix(keySuffix);
         keyPatternPreferences.setKeyPatternRegex(keyPatternRegexProperty.getValue());
         keyPatternPreferences.setKeyPatternReplacement(keyPatternReplacementProperty.getValue());
@@ -116,6 +129,10 @@ public BooleanProperty generateOnSaveProperty() {
         return generateOnSaveProperty;
     }
 
+    public BooleanProperty generateKeyOnImportProperty() {
+        return generateKeyOnImportProperty;
+    }
+
     public BooleanProperty letterStartAProperty() {
         return letterStartAProperty;
     }
diff --git a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.fxml b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.fxml
index 870b9633ac5..86fc1471ee9 100644
--- a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.fxml
+++ b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.fxml
@@ -18,7 +18,6 @@
 
     <Label styleClass="sectionHeader" text="%General"/>
     <CheckBox fx:id="enableWebSearch" text="%Enable web search"/>
-    <CheckBox fx:id="generateNewKeyOnImport" text="%Generate a new key for imported entries (overwriting their default)"/>
     <CheckBox fx:id="warnAboutDuplicatesOnImport" text="%Warn about duplicates on import"/>
     <CheckBox fx:id="downloadLinkedOnlineFiles" text="%Download linked online files"/>
     <CheckBox fx:id="keepDownloadUrl" text="%Store url for downloaded file" />
diff --git a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.java b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.java
index b981b4148d1..f842ba1233e 100644
--- a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.java
+++ b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTab.java
@@ -30,7 +30,6 @@
 public class WebSearchTab extends AbstractPreferenceTabView<WebSearchTabViewModel> implements PreferencesTab {
 
     @FXML private CheckBox enableWebSearch;
-    @FXML private CheckBox generateNewKeyOnImport;
     @FXML private CheckBox warnAboutDuplicatesOnImport;
     @FXML private CheckBox downloadLinkedOnlineFiles;
     @FXML private CheckBox keepDownloadUrl;
@@ -73,7 +72,6 @@ public void initialize() {
         this.viewModel = new WebSearchTabViewModel(preferences, dialogService, refAiEnabled);
 
         enableWebSearch.selectedProperty().bindBidirectional(viewModel.enableWebSearchProperty());
-        generateNewKeyOnImport.selectedProperty().bindBidirectional(viewModel.generateKeyOnImportProperty());
         warnAboutDuplicatesOnImport.selectedProperty().bindBidirectional(viewModel.warnAboutDuplicatesOnImportProperty());
         downloadLinkedOnlineFiles.selectedProperty().bindBidirectional(viewModel.shouldDownloadLinkedOnlineFiles());
         keepDownloadUrl.selectedProperty().bindBidirectional(viewModel.shouldKeepDownloadUrl());
diff --git a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java
index 46183d7f62b..ced3fc6a146 100644
--- a/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java
+++ b/src/main/java/org/jabref/gui/preferences/websearch/WebSearchTabViewModel.java
@@ -40,7 +40,6 @@
 
 public class WebSearchTabViewModel implements PreferenceTabViewModel {
     private final BooleanProperty enableWebSearchProperty = new SimpleBooleanProperty();
-    private final BooleanProperty generateKeyOnImportProperty = new SimpleBooleanProperty();
     private final BooleanProperty warnAboutDuplicatesOnImportProperty = new SimpleBooleanProperty();
     private final BooleanProperty shouldDownloadLinkedOnlineFiles = new SimpleBooleanProperty();
     private final BooleanProperty shouldkeepDownloadUrl = new SimpleBooleanProperty();
@@ -126,7 +125,6 @@ private void setupPlainCitationParsers(CliPreferences preferences) {
     @Override
     public void setValues() {
         enableWebSearchProperty.setValue(importerPreferences.areImporterEnabled());
-        generateKeyOnImportProperty.setValue(importerPreferences.isGenerateNewKeyOnImport());
         warnAboutDuplicatesOnImportProperty.setValue(importerPreferences.shouldWarnAboutDuplicatesOnImport());
         shouldDownloadLinkedOnlineFiles.setValue(filePreferences.shouldDownloadLinkedFiles());
         shouldkeepDownloadUrl.setValue(filePreferences.shouldKeepDownloadUrl());
@@ -158,7 +156,6 @@ public void setValues() {
     @Override
     public void storeSettings() {
         importerPreferences.setImporterEnabled(enableWebSearchProperty.getValue());
-        importerPreferences.setGenerateNewKeyOnImport(generateKeyOnImportProperty.getValue());
         importerPreferences.setWarnAboutDuplicatesOnImport(warnAboutDuplicatesOnImportProperty.getValue());
         filePreferences.setDownloadLinkedFiles(shouldDownloadLinkedOnlineFiles.getValue());
         filePreferences.setKeepDownloadUrl(shouldkeepDownloadUrl.getValue());
@@ -184,10 +181,6 @@ public BooleanProperty enableWebSearchProperty() {
         return enableWebSearchProperty;
     }
 
-    public BooleanProperty generateKeyOnImportProperty() {
-        return generateKeyOnImportProperty;
-    }
-
     public ListProperty<PlainCitationParserChoice> plainCitationParsers() {
         return plainCitationParsers;
     }
diff --git a/src/main/java/org/jabref/logic/importer/ImporterPreferences.java b/src/main/java/org/jabref/logic/importer/ImporterPreferences.java
index 64df8d2c9bc..9f7a7923d6a 100644
--- a/src/main/java/org/jabref/logic/importer/ImporterPreferences.java
+++ b/src/main/java/org/jabref/logic/importer/ImporterPreferences.java
@@ -65,7 +65,7 @@ public void setImporterEnabled(boolean importerEnabled) {
         this.importerEnabled.set(importerEnabled);
     }
 
-    public boolean isGenerateNewKeyOnImport() {
+    public boolean shouldGenerateNewKeyOnImport() {
         return generateNewKeyOnImport.get();
     }
 
diff --git a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java
index de416b2ecd6..b3190fc57f4 100644
--- a/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java
+++ b/src/test/java/org/jabref/gui/entryeditor/citationrelationtab/CitationsRelationsTabViewModelTest.java
@@ -63,7 +63,7 @@ void setUp() {
         when(preferences.getImportFormatPreferences()).thenReturn(importFormatPreferences);
 
         ImporterPreferences importerPreferences = mock(ImporterPreferences.class, Answers.RETURNS_DEEP_STUBS);
-        when(importerPreferences.isGenerateNewKeyOnImport()).thenReturn(false);
+        when(importerPreferences.shouldGenerateNewKeyOnImport()).thenReturn(false);
         when(preferences.getImporterPreferences()).thenReturn(importerPreferences);
 
         FieldPreferences fieldPreferences = mock(FieldPreferences.class);