Merge remote-tracking branch 'upstream/main' into download-should-use…

…-user-directory-first * upstream/main: fix l10n Compile fix Fix parsing of JabRef v5.7 study.yml files (#9124) Fix integrity check for tilde accents in author names (#9097) Rework the Define study parameters dialog (#9123) # Conflicts: # CHANGELOG.md
JabRef · Sep 3, 2022 · 00d2dab · 00d2dab
2 parents caf101a + 069eedc
commit 00d2dab
Show file tree

Hide file tree

Showing 24 changed files with 481 additions and 278 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,13 +24,16 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - We call backup files `.bak` and temporary writing files now `.sav`.
 - JabRef keeps 10 older versions of a `.bib` file in the [user data dir](https://github.com/harawata/appdirs#supported-directories) (instead of a single `.sav` (now: `.bak`) file in the directory of the `.bib` file)
 - We changed the button label from "Return to JabRef" to "Return to library" to better indicate the purpose of the action.
-- We removed "last-search-date" from the SLR feature, because the last-search-date can be deducted from the git logs.
+- We removed "last-search-date" from the SLR feature, because the last-search-date can be deducted from the git logs. [#9116](https://github.com/JabRef/jabref/pull/9116)
+- A user can now add arbitrary data into `study.yml`. JabRef just ignores this data. [#9124](https://github.com/JabRef/jabref/pull/9124)
 - We reworked the External Changes Resolver dialog. [#9021](https://github.com/JabRef/jabref/pull/9021)
 - The fallback directory of the file folder now is the general file directory. In case there was a directory configured for a library and this directory was not found, JabRef placed the PDF next to the .bib file and not into the general file directory.
 - The global default directory for storing PDFs is now the subdirectory "JabRef" in the user's home.
+- We reworked the Define study parameters dialog. [#9123](https://github.com/JabRef/jabref/pull/9123)
 
 ### Fixed
 
+- We fixed an issue where author names with tilde accents (for example ñ) were marked as "Names are not in the standard BibTex format" [#8071](https://github.com/JabRef/jabref/issues/8071)
 - We fixed an issue where the possibility to generate a subdatabase from an aux file was writing empty files when called from the commandline [#9115](https://github.com/JabRef/jabref/issues/9115), [forum#3516](https://discourse.jabref.org/t/export-subdatabase-from-aux-file-on-macos-command-line/3516)
 - We fixed the display of issue, number, eid and pages fields in the entry preview. [#8607](https://github.com/JabRef/jabref/pull/8607), [#8372](https://github.com/JabRef/jabref/issues/8372), [Koppor#514](https://github.com/koppor/jabref/issues/514), [forum#2390](https://discourse.jabref.org/t/unable-to-edit-my-bibtex-file-that-i-used-before-vers-5-1/2390), [forum#3462](https://discourse.jabref.org/t/jabref-5-6-need-help-with-export-from-jabref-to-microsoft-word-entry-preview-of-apa-7-not-rendering-correctly/3462)
 - We fixed the page ranges checker to detect article numbers in the pages field (used at [Check Integrity](https://docs.jabref.org/finding-sorting-and-cleaning-entries/checkintegrity)). [#8607](https://github.com/JabRef/jabref/pull/8607)

diff --git a/src/main/java/org/jabref/gui/slr/ManageStudyDefinition.fxml b/src/main/java/org/jabref/gui/slr/ManageStudyDefinition.fxml
diff --git a/src/main/java/org/jabref/gui/slr/ManageStudyDefinitionView.java b/src/main/java/org/jabref/gui/slr/ManageStudyDefinitionView.java
@@ -13,7 +13,6 @@
 import javafx.scene.Node;
 import javafx.scene.control.Button;
 import javafx.scene.control.ButtonType;
-import javafx.scene.control.ComboBox;
 import javafx.scene.control.Label;
 import javafx.scene.control.TableColumn;
 import javafx.scene.control.TableView;
@@ -22,30 +21,34 @@
 import javafx.scene.control.cell.CheckBoxTableCell;
 import javafx.scene.control.cell.TextFieldTableCell;
 import javafx.scene.input.KeyCode;
+import javafx.scene.input.MouseButton;
 
 import org.jabref.gui.DialogService;
 import org.jabref.gui.icon.IconTheme;
 import org.jabref.gui.theme.ThemeManager;
 import org.jabref.gui.util.BaseDialog;
 import org.jabref.gui.util.DirectoryDialogConfiguration;
 import org.jabref.gui.util.ValueTableCellFactory;
-import org.jabref.gui.util.ViewModelListCellFactory;
+import org.jabref.gui.util.ViewModelTableRowFactory;
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.study.Study;
 import org.jabref.preferences.PreferencesService;
 
 import com.airhacks.afterburner.views.ViewLoader;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /**
  * This class controls the user interface of the study definition management dialog. The UI elements and their layout
  * are defined in the FXML file.
  */
 public class ManageStudyDefinitionView extends BaseDialog<SlrStudyAndDirectory> {
+    private static final Logger LOGGER = LoggerFactory.getLogger(ManageStudyDefinitionView.class);
+
     @FXML private TextField studyTitle;
     @FXML private TextField addAuthor;
     @FXML private TextField addResearchQuestion;
     @FXML private TextField addQuery;
-    @FXML private ComboBox<StudyDatabaseItem> databaseSelector;
     @FXML private TextField studyDirectory;
 
     @FXML private ButtonType saveButtonType;
@@ -66,7 +69,6 @@ public class ManageStudyDefinitionView extends BaseDialog<SlrStudyAndDirectory>
     @FXML private TableView<StudyDatabaseItem> databaseTable;
     @FXML private TableColumn<StudyDatabaseItem, Boolean> databaseEnabledColumn;
     @FXML private TableColumn<StudyDatabaseItem, String> databaseColumn;
-    @FXML private TableColumn<StudyDatabaseItem, String> databaseActionColumn;
 
     @Inject private DialogService dialogService;
     @Inject private PreferencesService prefs;
@@ -80,7 +82,7 @@ public class ManageStudyDefinitionView extends BaseDialog<SlrStudyAndDirectory>
     /**
      * This can be used to either create new study objects or edit existing ones.
      *
-     * @param study          null if a new study is created. Otherwise the study object to edit.
+     * @param study          null if a new study is created. Otherwise, the study object to edit.
      * @param studyDirectory the directory where the study to edit is located (null if a new study is created)
      */
     public ManageStudyDefinitionView(Study study, Path studyDirectory, Path workingDirectory) {
@@ -118,11 +120,14 @@ private void setupSaveButton() {
 
     @FXML
     private void initialize() {
-        viewModel = new ManageStudyDefinitionViewModel(
-                study,
-                workingDirectory,
-                prefs.getImportFormatPreferences(),
-                prefs.getImporterPreferences());
+        if (Objects.isNull(study)) {
+            viewModel = new ManageStudyDefinitionViewModel(
+                    prefs.getImportFormatPreferences(),
+                    prefs.getImporterPreferences());
+        } else {
+            LOGGER.error("Not yet implemented");
+            return;
+        }
 
         // Listen whether any databases are removed from selection -> Add back to the database selector
         studyTitle.textProperty().bindBidirectional(viewModel.titleProperty());
@@ -161,25 +166,24 @@ private void initQueriesTab() {
     }
 
     private void initDatabasesTab() {
-        new ViewModelListCellFactory<StudyDatabaseItem>().withText(StudyDatabaseItem::getName)
-                                                         .install(databaseSelector);
-        databaseSelector.setItems(viewModel.getNonSelectedDatabases());
+        new ViewModelTableRowFactory<StudyDatabaseItem>()
+                .withOnMouseClickedEvent((entry, event) -> {
+                    if (event.getButton() == MouseButton.PRIMARY) {
+                        entry.setEnabled(!entry.isEnabled());
+                    }
+                })
+                .install(databaseTable);
 
-        setupCommonPropertiesForTables(databaseSelector, this::addDatabase, databaseColumn, databaseActionColumn);
+        databaseColumn.setReorderable(false);
+        databaseColumn.setCellFactory(TextFieldTableCell.forTableColumn());
 
         databaseEnabledColumn.setResizable(false);
         databaseEnabledColumn.setReorderable(false);
-        databaseEnabledColumn.setCellValueFactory(param -> param.getValue().enabledProperty());
         databaseEnabledColumn.setCellFactory(CheckBoxTableCell.forTableColumn(databaseEnabledColumn));
+        databaseEnabledColumn.setCellValueFactory(param -> param.getValue().enabledProperty());
 
+        databaseColumn.setEditable(false);
         databaseColumn.setCellValueFactory(param -> param.getValue().nameProperty());
-        databaseActionColumn.setCellValueFactory(param -> param.getValue().nameProperty());
-        new ValueTableCellFactory<org.jabref.gui.slr.StudyDatabaseItem, String>()
-                .withGraphic(item -> IconTheme.JabRefIcons.DELETE_ENTRY.getGraphicNode())
-                .withTooltip(name -> Localization.lang("Remove"))
-                .withOnMouseClickedEvent(item -> evt ->
-                        viewModel.removeDatabase(item))
-                .install(databaseActionColumn);
 
         databaseTable.setItems(viewModel.getDatabases());
     }
@@ -231,14 +235,6 @@ private void addQuery() {
         addQuery.setText("");
     }
 
-    /**
-     * Add selected entry from combobox, push onto database pop from nonselecteddatabase (combobox)
-     */
-    @FXML
-    private void addDatabase() {
-        viewModel.addDatabase(databaseSelector.getSelectionModel().getSelectedItem());
-    }
-
     @FXML
     public void selectStudyDirectory() {
         DirectoryDialogConfiguration directoryDialogConfiguration = new DirectoryDialogConfiguration.Builder()

diff --git a/src/main/java/org/jabref/gui/slr/ManageStudyDefinitionViewModel.java b/src/main/java/org/jabref/gui/slr/ManageStudyDefinitionViewModel.java
@@ -2,9 +2,10 @@
 
 import java.nio.file.InvalidPathException;
 import java.nio.file.Path;
-import java.util.Comparator;
+import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import javafx.beans.property.Property;
@@ -17,6 +18,11 @@
 import org.jabref.logic.importer.ImporterPreferences;
 import org.jabref.logic.importer.SearchBasedFetcher;
 import org.jabref.logic.importer.WebFetchers;
+import org.jabref.logic.importer.fetcher.ACMPortalFetcher;
+import org.jabref.logic.importer.fetcher.CompositeSearchBasedFetcher;
+import org.jabref.logic.importer.fetcher.DBLPFetcher;
+import org.jabref.logic.importer.fetcher.IEEE;
+import org.jabref.logic.importer.fetcher.SpringerFetcher;
 import org.jabref.model.study.Study;
 import org.jabref.model.study.StudyDatabase;
 import org.jabref.model.study.StudyQuery;
@@ -31,45 +37,69 @@
 public class ManageStudyDefinitionViewModel {
     private static final Logger LOGGER = LoggerFactory.getLogger(ManageStudyDefinitionViewModel.class);
 
+    private static final Set<String> DEFAULT_SELECTION = Set.of(
+            ACMPortalFetcher.FETCHER_NAME,
+            IEEE.FETCHER_NAME,
+            SpringerFetcher.FETCHER_NAME,
+            DBLPFetcher.FETCHER_NAME);
+
+
     private final StringProperty title = new SimpleStringProperty();
     private final ObservableList<String> authors = FXCollections.observableArrayList();
     private final ObservableList<String> researchQuestions = FXCollections.observableArrayList();
     private final ObservableList<String> queries = FXCollections.observableArrayList();
     private final ObservableList<StudyDatabaseItem> databases = FXCollections.observableArrayList();
+
     // Hold the complement of databases for the selector
-    private final ObservableList<StudyDatabaseItem> nonSelectedDatabases = FXCollections.observableArrayList();
     private final SimpleStringProperty directory = new SimpleStringProperty();
-    private Study study;
 
+    /**
+     * Constructor for a new study
+     */
+    public ManageStudyDefinitionViewModel(ImportFormatPreferences importFormatPreferences,
+                                          ImporterPreferences importerPreferences) {
+        databases.addAll(WebFetchers.getSearchBasedFetchers(importFormatPreferences, importerPreferences)
+                                    .stream()
+                                    .map(SearchBasedFetcher::getName)
+                                    // The user wants to select specific fetchers
+                                    // The fetcher summarizing ALL fetchers can be emulated by selecting ALL fetchers (which happens rarely when doing an SLR)
+                                    .filter(name -> !name.equals(CompositeSearchBasedFetcher.FETCHER_NAME))
+                                    .map(name -> {
+                                        boolean enabled = DEFAULT_SELECTION.contains(name);
+                                        return new StudyDatabaseItem(name, enabled);
+                                    })
+                                    .toList());
+    }
+
+    /**
+     * Constructor for an existing study
+     *
+     * @param study The study to initialize the UI from
+     * @param studyDirectory The path where the study resides
+     */
     public ManageStudyDefinitionViewModel(Study study,
                                           Path studyDirectory,
                                           ImportFormatPreferences importFormatPreferences,
                                           ImporterPreferences importerPreferences) {
-        if (Objects.isNull(study)) {
-            computeNonSelectedDatabases(importFormatPreferences, importerPreferences);
-            return;
-        }
-        this.study = study;
+        // copy the content of the study object into the UI fields
+        authors.addAll(Objects.requireNonNull(study).getAuthors());
         title.setValue(study.getTitle());
-        authors.addAll(study.getAuthors());
         researchQuestions.addAll(study.getResearchQuestions());
         queries.addAll(study.getQueries().stream().map(StudyQuery::getQuery).toList());
-        databases.addAll(study.getDatabases()
-                              .stream()
-                              .map(studyDatabase -> new StudyDatabaseItem(studyDatabase.getName(), studyDatabase.isEnabled()))
-                              .toList());
-        computeNonSelectedDatabases(importFormatPreferences, importerPreferences);
-        if (!Objects.isNull(studyDirectory)) {
-            this.directory.set(studyDirectory.toString());
-        }
-    }
-
-    private void computeNonSelectedDatabases(ImportFormatPreferences importFormatPreferences, ImporterPreferences importerPreferences) {
-        nonSelectedDatabases.addAll(WebFetchers.getSearchBasedFetchers(importFormatPreferences, importerPreferences)
-                                               .stream()
-                                               .map(SearchBasedFetcher::getName)
-                                               .map(s -> new StudyDatabaseItem(s, true))
-                                               .filter(studyDatabase -> !databases.contains(studyDatabase)).toList());
+        List<StudyDatabase> studyDatabases = study.getDatabases();
+        databases.addAll(WebFetchers.getSearchBasedFetchers(importFormatPreferences, importerPreferences)
+                                    .stream()
+                                    .map(SearchBasedFetcher::getName)
+                                    // The user wants to select specific fetchers
+                                    // The fetcher summarizing ALL fetchers can be emulated by selecting ALL fetchers (which happens rarely when doing an SLR)
+                                    .filter(name -> !name.equals(CompositeSearchBasedFetcher.FETCHER_NAME))
+                                    .map(name -> {
+                                        boolean enabled = studyDatabases.contains(new StudyDatabase(name, true));
+                                        return new StudyDatabaseItem(name, enabled);
+                                    })
+                                    .toList());
+
+        this.directory.set(Objects.requireNonNull(studyDirectory).toString());
     }
 
     public StringProperty getTitle() {
@@ -96,10 +126,6 @@ public ObservableList<StudyDatabaseItem> getDatabases() {
         return databases;
     }
 
-    public ObservableList<StudyDatabaseItem> getNonSelectedDatabases() {
-        return nonSelectedDatabases;
-    }
-
     public void addAuthor(String author) {
         if (author.isBlank()) {
             return;
@@ -121,29 +147,18 @@ public void addQuery(String query) {
         queries.add(query);
     }
 
-    public void addDatabase(StudyDatabaseItem database) {
-        if (Objects.isNull(database)) {
-            return;
-        }
-        nonSelectedDatabases.remove(database);
-        if (!databases.contains(database)) {
-            databases.add(database);
-        }
-    }
-
     public SlrStudyAndDirectory saveStudy() {
-        if (Objects.isNull(study)) {
-            study = new Study();
-        }
-        study.setTitle(title.getValueSafe());
-        study.setAuthors(authors);
-        study.setResearchQuestions(researchQuestions);
-        study.setQueries(queries.stream().map(StudyQuery::new).collect(Collectors.toList()));
-        study.setDatabases(databases.stream().map(studyDatabaseItem -> new StudyDatabase(studyDatabaseItem.getName(), studyDatabaseItem.isEnabled())).collect(Collectors.toList()));
+        Study study = new Study(
+                authors,
+                title.getValueSafe(),
+                researchQuestions,
+                queries.stream().map(StudyQuery::new).collect(Collectors.toList()),
+                databases.stream().map(studyDatabaseItem -> new StudyDatabase(studyDatabaseItem.getName(), studyDatabaseItem.isEnabled())).filter(StudyDatabase::isEnabled).collect(Collectors.toList()));
         Path studyDirectory = null;
         try {
             studyDirectory = Path.of(directory.getValueSafe());
-        } catch (InvalidPathException e) {
+        } catch (
+                InvalidPathException e) {
             LOGGER.error("Invalid path was provided: {}", directory);
         }
         return new SlrStudyAndDirectory(study, studyDirectory);
@@ -153,20 +168,6 @@ public Property<String> titleProperty() {
         return title;
     }
 
-    public void removeDatabase(String database) {
-        // If a database is added from the combo box it should be enabled by default
-        Optional<StudyDatabaseItem> correspondingDatabase = databases.stream().filter(studyDatabaseItem -> studyDatabaseItem.getName().equals(database)).findFirst();
-        if (correspondingDatabase.isEmpty()) {
-            return;
-        }
-        StudyDatabaseItem databaseToRemove = correspondingDatabase.get();
-        databases.remove(databaseToRemove);
-        databaseToRemove.setEnabled(true);
-        nonSelectedDatabases.add(databaseToRemove);
-        // Resort list
-        nonSelectedDatabases.sort(Comparator.comparing(StudyDatabaseItem::getName));
-    }
-
     public void setStudyDirectory(Optional<Path> studyRepositoryRoot) {
         getDirectory().setValue(studyRepositoryRoot.map(Path::toString).orElseGet(() -> getDirectory().getValueSafe()));
     }