Rework the Define study parameters dialog

CHANGELOG.md

@@ -26,11 +26,11 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
 - 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 reworked the External Changes Resolver dialog. [#9021](https://github.com/JabRef/jabref/pull/9021)
-
+- We reworked the Define study parameters dialog. [#9123](https://github.com/JabRef/jabref/pull/9123)
 
 ### Fixed
 
-- We fixed an issue where the possibilty 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 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)
 - The [HtmlToLaTeXFormatter](https://docs.jabref.org/finding-sorting-and-cleaning-entries/saveactions#html-to-latex) keeps single `<` characters.

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()

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()));
     }

src/main/java/org/jabref/gui/slr/StudyDatabaseItem.java

@@ -1,16 +1,23 @@
 package org.jabref.gui.slr;
 
+import java.util.Objects;
+
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.SimpleBooleanProperty;
 import javafx.beans.property.SimpleStringProperty;
 import javafx.beans.property.StringProperty;
 
+import org.jabref.model.study.StudyDatabase;
+
+/**
+ * View representation of {@link StudyDatabase}
+ */
 public class StudyDatabaseItem {
     private final StringProperty name;
     private final BooleanProperty enabled;
 
     public StudyDatabaseItem(String name, boolean enabled) {
-        this.name = new SimpleStringProperty(name);
+        this.name = new SimpleStringProperty(Objects.requireNonNull(name));
         this.enabled = new SimpleBooleanProperty(enabled);
     }
 
@@ -38,6 +45,14 @@ public BooleanProperty enabledProperty() {
         return enabled;
     }
 
+    @Override
+    public String toString() {
+        return "StudyDatabaseItem{" +
+                "name=" + name.get() +
+                ", enabled=" + enabled.get() +
+                '}';
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) {
@@ -46,19 +61,12 @@ public boolean equals(Object o) {
         if (o == null || getClass() != o.getClass()) {
             return false;
         }
-
         StudyDatabaseItem that = (StudyDatabaseItem) o;
-
-        if (isEnabled() != that.isEnabled()) {
-            return false;
-        }
-        return getName() != null ? getName().equals(that.getName()) : that.getName() == null;
+        return Object.equals(getName(),that.getName()) && Object.equals(isEnabled(), that.isEnabled());
     }
 
     @Override
     public int hashCode() {
-        int result = getName() != null ? getName().hashCode() : 0;
-        result = 31 * result + (isEnabled() ? 1 : 0);
-        return result;
+        return Objects.hash(getName(), isEnabled());
     }
 }