Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added an option in preferences/groups to change the default the hierarchical context #9344

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve

### Added

- We added an option in preferences/groups to change the default the hierarchical context when creating a new group [9141](https://github.com/JabRef/jabref/issues/9141)
- We added a fetcher for [Biodiversity Heritage Library](https://www.biodiversitylibrary.org/). [8539](https://github.com/JabRef/jabref/issues/8539)
- We added support for multiple messages in the snackbar. [#7340](https://github.com/JabRef/jabref/issues/7340)
- We added an extra option in the 'Find Unlinked Files' dialog view to ignore unnecessary files like Thumbs.db, DS_Store, etc. [koppor#373](https://github.com/koppor/jabref/issues/373)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ public void setValues() {
// creating new group -> defaults!
colorProperty.setValue(IconTheme.getDefaultGroupColor());
typeExplicitProperty.setValue(true);
groupHierarchySelectedProperty.setValue(GroupHierarchyType.INDEPENDENT);
groupHierarchySelectedProperty.setValue(preferencesService.getGroupsPreferences().getDefaultGroupMode());
} else {
nameProperty.setValue(editedGroup.getName());
colorProperty.setValue(editedGroup.getColor().orElse(IconTheme.getDefaultGroupColor()));
Expand Down
19 changes: 18 additions & 1 deletion src/main/java/org/jabref/gui/groups/GroupsPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import javafx.beans.property.ObjectProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleObjectProperty;
import org.jabref.model.groups.GroupHierarchyType;

public class GroupsPreferences {

Expand All @@ -12,15 +13,19 @@ public class GroupsPreferences {
private final BooleanProperty shouldDisplayGroupCount;
private final ObjectProperty<Character> keywordSeparator;

private final ObjectProperty<GroupHierarchyType> defaultGroupMode;

public GroupsPreferences(GroupViewMode groupViewMode,
boolean shouldAutoAssignGroup,
boolean shouldDisplayGroupCount,
ObjectProperty<Character> keywordSeparator) {
ObjectProperty<Character> keywordSeparator,
GroupHierarchyType defaultGroupMode) {

this.groupViewMode = new SimpleObjectProperty<>(groupViewMode);
this.shouldAutoAssignGroup = new SimpleBooleanProperty(shouldAutoAssignGroup);
this.shouldDisplayGroupCount = new SimpleBooleanProperty(shouldDisplayGroupCount);
this.keywordSeparator = keywordSeparator;
this.defaultGroupMode = new SimpleObjectProperty<>(defaultGroupMode);
}

public GroupViewMode getGroupViewMode() {
Expand Down Expand Up @@ -70,4 +75,16 @@ public ObjectProperty<Character> keywordSeparatorProperty() {
public void setKeywordSeparator(Character keywordSeparator) {
this.keywordSeparator.set(keywordSeparator);
}

public GroupHierarchyType getDefaultGroupMode() {
return defaultGroupMode.get();
}

public ObjectProperty<GroupHierarchyType> defaultGroupCreateProperty() {
return defaultGroupMode;
}

public void setDefaultGroupCreate(GroupHierarchyType defaultGroup){
this.defaultGroupMode.set(defaultGroup);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
<?import javafx.scene.control.ToggleGroup?>
<?import javafx.scene.layout.HBox?>
<?import javafx.scene.layout.VBox?>
<?import javafx.scene.control.ComboBox?>
<?import javafx.scene.layout.GridPane?>
<fx:root spacing="10.0" type="VBox"
xmlns="http://javafx.com/javafx" xmlns:fx="http://javafx.com/fxml"
fx:controller="org.jabref.gui.preferences.groups.GroupsTab">
Expand All @@ -23,6 +25,11 @@
<CheckBox fx:id="autoAssignGroup" text="%Automatically assign new entry to selected groups"/>
<CheckBox fx:id="displayGroupCount" text="%Display count of items in group"/>

<GridPane hgap="10.0" vgap="4.0">
<Label text="Default Group Creation" GridPane.columnIndex="1" GridPane.rowIndex="1"/>
<ComboBox fx:id="defaultSelectionCombo"
prefWidth="200.0" GridPane.columnIndex="2" GridPane.rowIndex="1" />
</GridPane>
<HBox spacing="4" alignment="CENTER_LEFT">
<Label text="%Keyword separator"/>
<TextField fx:id="keywordSeparator" minWidth="30.0" maxWidth="30.0"/>
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/org/jabref/gui/preferences/groups/GroupsTab.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,17 @@

import javafx.fxml.FXML;
import javafx.scene.control.CheckBox;
import javafx.scene.control.ComboBox;
import javafx.scene.control.RadioButton;
import javafx.scene.control.TextField;

import org.jabref.gui.preferences.AbstractPreferenceTabView;
import org.jabref.gui.preferences.PreferencesTab;
import org.jabref.gui.util.ViewModelListCellFactory;
import org.jabref.logic.l10n.Localization;

import com.airhacks.afterburner.views.ViewLoader;
import org.jabref.model.groups.GroupHierarchyType;

public class GroupsTab extends AbstractPreferenceTabView<GroupsTabViewModel> implements PreferencesTab {

Expand All @@ -19,6 +22,8 @@ public class GroupsTab extends AbstractPreferenceTabView<GroupsTabViewModel> imp
@FXML private CheckBox displayGroupCount;
@FXML private TextField keywordSeparator;

@FXML private ComboBox<GroupHierarchyType> defaultSelectionCombo;

public GroupsTab() {
ViewLoader.view(this)
.root(this)
Expand All @@ -33,6 +38,14 @@ public String getTabName() {
public void initialize() {
this.viewModel = new GroupsTabViewModel(dialogService, preferencesService.getGroupsPreferences());

new ViewModelListCellFactory<GroupHierarchyType>()
.withText(GroupHierarchyType::getFormattedName)
.install(defaultSelectionCombo);

defaultSelectionCombo.itemsProperty().bind(viewModel.defaultGroupsListProperty());
defaultSelectionCombo.valueProperty().bindBidirectional(viewModel.selectedDefaultGroup());


groupViewModeIntersection.selectedProperty().bindBidirectional(viewModel.groupViewModeIntersectionProperty());
groupViewModeUnion.selectedProperty().bindBidirectional(viewModel.groupViewModeUnionProperty());
autoAssignGroup.selectedProperty().bindBidirectional(viewModel.autoAssignGroupProperty());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
package org.jabref.gui.preferences.groups;

import javafx.beans.property.BooleanProperty;
import javafx.beans.property.SimpleBooleanProperty;
import javafx.beans.property.SimpleStringProperty;
import javafx.beans.property.StringProperty;
import javafx.beans.property.*;

import javafx.collections.FXCollections;
import org.jabref.gui.DialogService;
import org.jabref.gui.groups.GroupViewMode;
import org.jabref.gui.groups.GroupsPreferences;
import org.jabref.gui.preferences.PreferenceTabViewModel;
import org.jabref.model.groups.GroupHierarchyType;

public class GroupsTabViewModel implements PreferenceTabViewModel {

Expand All @@ -17,6 +16,9 @@ public class GroupsTabViewModel implements PreferenceTabViewModel {
private final BooleanProperty autoAssignGroupProperty = new SimpleBooleanProperty();
private final BooleanProperty displayGroupCountProperty = new SimpleBooleanProperty();
private final StringProperty keywordSeparatorProperty = new SimpleStringProperty("");
private final ListProperty<GroupHierarchyType> groupDefaultListProperty = new SimpleListProperty<>();

private final ObjectProperty<GroupHierarchyType> selectedGroupDefaultProperty = new SimpleObjectProperty<>();

private final DialogService dialogService;
private final GroupsPreferences groupsPreferences;
Expand All @@ -38,6 +40,10 @@ public void setValues() {
groupViewModeUnionProperty.setValue(true);
}
}
groupDefaultListProperty.setValue(FXCollections.observableArrayList(GroupHierarchyType.values()));
selectedGroupDefaultProperty.setValue(groupsPreferences.getDefaultGroupMode());


autoAssignGroupProperty.setValue(groupsPreferences.shouldAutoAssignGroup());
displayGroupCountProperty.setValue(groupsPreferences.shouldDisplayGroupCount());
keywordSeparatorProperty.setValue(groupsPreferences.getKeywordSeparator().toString());
Expand All @@ -49,6 +55,7 @@ public void storeSettings() {
groupsPreferences.setAutoAssignGroup(autoAssignGroupProperty.getValue());
groupsPreferences.setDisplayGroupCount(displayGroupCountProperty.getValue());
groupsPreferences.keywordSeparatorProperty().setValue(keywordSeparatorProperty.getValue().charAt(0));
groupsPreferences.setDefaultGroupCreate(selectedGroupDefaultProperty.getValue());
}

public BooleanProperty groupViewModeIntersectionProperty() {
Expand All @@ -70,4 +77,12 @@ public BooleanProperty displayGroupCount() {
public StringProperty keywordSeparatorProperty() {
return keywordSeparatorProperty;
}

public ListProperty<GroupHierarchyType> defaultGroupsListProperty() {
return this.groupDefaultListProperty;
}

public Property<GroupHierarchyType> selectedDefaultGroup() {
return this.selectedGroupDefaultProperty;
}
}
11 changes: 11 additions & 0 deletions src/main/java/org/jabref/model/groups/GroupHierarchyType.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,4 +29,15 @@ public static GroupHierarchyType getByNumberOrDefault(int type) {
return INDEPENDENT;
}
}

public String getFormattedName() {
if (this == INDEPENDENT) {
return "Independent";
} else if (this == REFINING) {
return "Intersection";
}
else {
return "Union";
}
}
Comment on lines +33 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be implemented as an constructor argument for the enum e.g. INDEPENDENT("Independent") and be localized. Open/closed design principle.

}
10 changes: 8 additions & 2 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.types.EntryType;
import org.jabref.model.entry.types.EntryTypeFactory;
import org.jabref.model.groups.GroupHierarchyType;
import org.jabref.model.metadata.SaveOrderConfig;
import org.jabref.model.search.rules.SearchRules;
import org.jabref.model.strings.StringUtil;
Expand Down Expand Up @@ -376,6 +377,8 @@ public class JabRefPreferences implements PreferencesService {
// GroupViewMode
private static final String GROUP_INTERSECT_UNION_VIEW_MODE = "groupIntersectUnionViewModes";

private static final String GROUP_CREATE_DEFAULT = "groupCreateDefault";

// Dialog states
private static final String PREFS_EXPORT_PATH = "prefsExportPath";
private static final String DOWNLOAD_LINKED_FILES = "downloadLinkedFiles";
Expand Down Expand Up @@ -602,7 +605,9 @@ private JabRefPreferences() {
defaults.put(AUTO_ASSIGN_GROUP, Boolean.TRUE);
defaults.put(DISPLAY_GROUP_COUNT, Boolean.TRUE);
defaults.put(GROUP_INTERSECT_UNION_VIEW_MODE, GroupViewMode.INTERSECTION.name());

defaults.put(KEYWORD_SEPARATOR, ", ");
defaults.put(GROUP_CREATE_DEFAULT, GroupHierarchyType.INDEPENDENT.name());
defaults.put(DEFAULT_ENCODING, StandardCharsets.UTF_8.name());
defaults.put(DEFAULT_OWNER, System.getProperty("user.name"));
defaults.put(MEMORY_STICK_MODE, Boolean.FALSE);
Expand Down Expand Up @@ -1395,12 +1400,13 @@ public GroupsPreferences getGroupsPreferences() {
GroupViewMode.valueOf(get(GROUP_INTERSECT_UNION_VIEW_MODE)),
getBoolean(AUTO_ASSIGN_GROUP),
getBoolean(DISPLAY_GROUP_COUNT),
getInternalPreferences().keywordSeparatorProperty()
);
getInternalPreferences().keywordSeparatorProperty(),
GroupHierarchyType.valueOf(get(GROUP_CREATE_DEFAULT)));

EasyBind.listen(groupsPreferences.groupViewModeProperty(), (obs, oldValue, newValue) -> put(GROUP_INTERSECT_UNION_VIEW_MODE, newValue.name()));
EasyBind.listen(groupsPreferences.autoAssignGroupProperty(), (obs, oldValue, newValue) -> putBoolean(AUTO_ASSIGN_GROUP, newValue));
EasyBind.listen(groupsPreferences.displayGroupCountProperty(), (obs, oldValue, newValue) -> putBoolean(DISPLAY_GROUP_COUNT, newValue));
EasyBind.listen(groupsPreferences.defaultGroupCreateProperty(), (obs, oldValue, newValue) -> put(GROUP_CREATE_DEFAULT, newValue.name()));
// KeywordSeparator is handled by JabRefPreferences::getInternalPreferences

return groupsPreferences;
Expand Down
75 changes: 75 additions & 0 deletions src/test/java/org/jabref/gui/groups/GroupCreateTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package org.jabref.gui.groups;

import java.util.Arrays;

import javafx.beans.property.SimpleObjectProperty;
import javafx.collections.FXCollections;
import javafx.collections.ObservableList;

import org.jabref.gui.StateManager;
import org.jabref.gui.util.CurrentThreadTaskExecutor;
import org.jabref.gui.util.CustomLocalDragboard;
import org.jabref.gui.util.DroppingMouseLocation;
import org.jabref.gui.util.TaskExecutor;
import org.jabref.model.database.BibDatabaseContext;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.groups.AbstractGroup;
import org.jabref.model.groups.AutomaticKeywordGroup;
import org.jabref.model.groups.ExplicitGroup;
import org.jabref.model.groups.GroupHierarchyType;
import org.jabref.model.groups.GroupTreeNode;
import org.jabref.model.groups.WordKeywordGroup;
import org.jabref.preferences.PreferencesService;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

class GroupCreateTest {

private StateManager stateManager;
private BibDatabaseContext databaseContext;
private GroupNodeViewModel viewModel;
private TaskExecutor taskExecutor;
private PreferencesService preferencesService;

@BeforeEach
void setUp() {
stateManager = mock(StateManager.class);
when(stateManager.getSelectedEntries()).thenReturn(FXCollections.emptyObservableList());
databaseContext = new BibDatabaseContext();
taskExecutor = new CurrentThreadTaskExecutor();
preferencesService = mock(PreferencesService.class);
when(preferencesService.getGroupsPreferences()).thenReturn(new GroupsPreferences(
GroupViewMode.UNION,
true,
true,
new SimpleObjectProperty<>(','),
GroupHierarchyType.REFINING));

viewModel = getViewModelForGroup(
new WordKeywordGroup("Test group", GroupHierarchyType.REFINING, StandardField.TITLE, "search", true, ',', false));
}

@Test
void groupContextCorrect() {
String groupName = "group";
ExplicitGroup group = new ExplicitGroup(groupName, GroupHierarchyType.INDEPENDENT, ',');
BibEntry entry = new BibEntry();
databaseContext.getDatabase().insertEntry(entry);
GroupNodeViewModel model = new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group, new CustomLocalDragboard(), preferencesService);
model.addEntriesToGroup(databaseContext.getEntries());
// make sure group default hierarchy preference does not override new group
assertEquals(model.getGroupNode().getGroup().getHierarchicalContext(),GroupHierarchyType.INDEPENDENT);
}
Comment on lines +59 to +69
Copy link
Member

@calixtus calixtus Nov 8, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really don't get what you want to test here?
The constructor of GroupNodeViewModel?
This test seems superfluous to me, especially it does not seem to be related to any code change in the PR. The thing that should be tested is if someone clicks in the UI on "add new group" the default GroupHierarchyType is selected.



private GroupNodeViewModel getViewModelForGroup(AbstractGroup group) {
return new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group, new CustomLocalDragboard(), preferencesService);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ void setUp() {
GroupViewMode.UNION,
true,
true,
new SimpleObjectProperty<>(',')
));
new SimpleObjectProperty<>(','),
GroupHierarchyType.INDEPENDENT));

viewModel = getViewModelForGroup(
new WordKeywordGroup("Test group", GroupHierarchyType.INDEPENDENT, StandardField.TITLE, "search", true, ',', false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ void setUp() {
GroupViewMode.UNION,
true,
true,
new SimpleObjectProperty<>(',')));
new SimpleObjectProperty<>(','), GroupHierarchyType.getByNumberOrDefault(0)));
groupTree = new GroupTreeViewModel(stateManager, mock(DialogService.class), preferencesService, taskExecutor, new CustomLocalDragboard());
}

Expand Down