diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fecc8aace0..b62ac13d111 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,8 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve - We fixed an issue where pdfs were re-indexed on each startup. [#9166](https://github.com/JabRef/jabref/pull/9166) - We fixed an issue where Capitalize didn't capitalize words after hyphen characters. [#9157](https://github.com/JabRef/jabref/issues/9157) - We fixed an issue with the message that is displayed when fetcher returns an empty list of entries for given query. [#9195](https://github.com/JabRef/jabref/issues/9195) +- We fixed an issue where hitting enter on the search field within the preferences dialog closed the dialog. [koppor#630](https://github.com/koppor/jabref/issues/630) +- We fixed a typo within a connection error message. [koppor#625](https://github.com/koppor/jabref/issues/625) ### Removed diff --git a/src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java b/src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java index a8e007dda62..38cb28f216a 100644 --- a/src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java +++ b/src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java @@ -83,7 +83,7 @@ private void parseUsingGrobid() { .onRunning(() -> dialogService.notify(Localization.lang("Your text is being parsed..."))) .onFailure((e) -> { if (e instanceof FetcherException) { - String msg = Localization.lang("There are connection issues with a JabRef server. Detailed information: %0.", + String msg = Localization.lang("There are connection issues with a JabRef server. Detailed information: %0", e.getMessage()); dialogService.notify(msg); } else { diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java index 5b0b3db25c5..8cf74178629 100644 --- a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java +++ b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java @@ -30,8 +30,13 @@ import org.jabref.model.database.BibDatabaseContext; import org.jabref.model.entry.BibEntry; import org.jabref.model.groups.AbstractGroup; +import org.jabref.model.groups.AutomaticKeywordGroup; +import org.jabref.model.groups.AutomaticPersonsGroup; import org.jabref.model.groups.ExplicitGroup; import org.jabref.model.groups.GroupTreeNode; +import org.jabref.model.groups.RegexKeywordGroup; +import org.jabref.model.groups.SearchGroup; +import org.jabref.model.groups.TexGroup; import org.jabref.model.groups.WordKeywordGroup; import org.jabref.model.metadata.MetaData; import org.jabref.preferences.PreferencesService; @@ -145,7 +150,6 @@ private void onActiveDatabaseChanged(Optional newDatabase) { } else { rootGroup.setValue(null); } - currentDatabase = newDatabase; } @@ -153,7 +157,6 @@ private void onActiveDatabaseChanged(Optional newDatabase) { * Opens "New Group Dialog" and adds the resulting group as subgroup to the specified group while maintaining the * alphabetical order */ - public void addNewSubgroup(GroupNodeViewModel parent, GroupDialogHeader groupDialogHeader) { currentDatabase.ifPresent(database -> { Optional newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView( @@ -185,20 +188,121 @@ private void writeGroupChangesToMetaData() { currentDatabase.ifPresent(database -> database.getMetaData().setGroups(rootGroup.get().getGroupNode())); } + private boolean isGroupTypeEqual(AbstractGroup oldGroup, AbstractGroup newGroup) { + return oldGroup.getClass().equals(newGroup.getClass()); + } + + /** + * Check if it is necessary to show a group modified, reassign entry dialog
+ * Group name change is handled separately + * + * @param oldGroup Original Group + * @param newGroup Edited group + * @return true if just trivial modifications (e.g. color or description) or the relevant group properties are equal, false otherwise + */ + boolean onlyMinorChanges(AbstractGroup oldGroup, AbstractGroup newGroup) { + // we need to use getclass here because we have different subclass inheritance e.g. ExplicitGroup is a subclass of WordKeyWordGroup + if (oldGroup.getClass() == WordKeywordGroup.class) { + WordKeywordGroup oldWordKeywordGroup = (WordKeywordGroup) oldGroup; + WordKeywordGroup newWordKeywordGroup = (WordKeywordGroup) newGroup; + + return Objects.equals(oldWordKeywordGroup.getSearchField().getName(), newWordKeywordGroup.getSearchField().getName()) + && Objects.equals(oldWordKeywordGroup.getSearchExpression(), newWordKeywordGroup.getSearchExpression()) + && Objects.equals(oldWordKeywordGroup.isCaseSensitive(), newWordKeywordGroup.isCaseSensitive()); + } else if (oldGroup.getClass() == RegexKeywordGroup.class) { + RegexKeywordGroup oldRegexKeywordGroup = (RegexKeywordGroup) oldGroup; + RegexKeywordGroup newRegexKeywordGroup = (RegexKeywordGroup) newGroup; + + return Objects.equals(oldRegexKeywordGroup.getSearchField().getName(), newRegexKeywordGroup.getSearchField().getName()) + && Objects.equals(oldRegexKeywordGroup.getSearchExpression(), newRegexKeywordGroup.getSearchExpression()) + && Objects.equals(oldRegexKeywordGroup.isCaseSensitive(), newRegexKeywordGroup.isCaseSensitive()); + } else if ((oldGroup.getClass() == SearchGroup.class)) { + SearchGroup oldSearchGroup = (SearchGroup) oldGroup; + SearchGroup newSearchGroup = (SearchGroup) newGroup; + + return Objects.equals(oldSearchGroup.getSearchExpression(), newSearchGroup.getSearchExpression()) + && Objects.equals(oldSearchGroup.getSearchFlags(), newSearchGroup.getSearchFlags()); + } else if (oldGroup.getClass() == AutomaticKeywordGroup.class) { + AutomaticKeywordGroup oldAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup; + AutomaticKeywordGroup newAutomaticKeywordGroup = (AutomaticKeywordGroup) oldGroup; + + return Objects.equals(oldAutomaticKeywordGroup.getKeywordDelimiter(), newAutomaticKeywordGroup.getKeywordDelimiter()) + && Objects.equals(oldAutomaticKeywordGroup.getKeywordHierarchicalDelimiter(), newAutomaticKeywordGroup.getKeywordHierarchicalDelimiter()) + && Objects.equals(oldAutomaticKeywordGroup.getField().getName(), newAutomaticKeywordGroup.getField().getName()); + } else if (oldGroup.getClass() == AutomaticPersonsGroup.class) { + AutomaticPersonsGroup oldAutomaticPersonsGroup = (AutomaticPersonsGroup) oldGroup; + AutomaticPersonsGroup newAutomaticPersonsGroup = (AutomaticPersonsGroup) newGroup; + + return Objects.equals(oldAutomaticPersonsGroup.getField().getName(), newAutomaticPersonsGroup.getField().getName()); + } else if (oldGroup.getClass() == TexGroup.class) { + TexGroup oldTexGroup = (TexGroup) oldGroup; + TexGroup newTexGroup = (TexGroup) newGroup; + return Objects.equals(oldTexGroup.getFilePath().toString(), newTexGroup.getFilePath().toString()); + } + return true; + } + /** * Opens "Edit Group Dialog" and changes the given group to the edited one. */ public void editGroup(GroupNodeViewModel oldGroup) { currentDatabase.ifPresent(database -> { Optional newGroup = dialogService.showCustomDialogAndWait(new GroupDialogView( - dialogService, - database, - preferences, - oldGroup.getGroupNode().getGroup(), - GroupDialogHeader.SUBGROUP)); - + dialogService, + database, + preferences, + oldGroup.getGroupNode().getGroup(), + GroupDialogHeader.SUBGROUP)); newGroup.ifPresent(group -> { - // TODO: Keep assignments + + AbstractGroup oldGroupDef = oldGroup.getGroupNode().getGroup(); + String oldGroupName = oldGroupDef.getName(); + + boolean groupTypeEqual = isGroupTypeEqual(oldGroupDef, group); + boolean onlyMinorModifications = groupTypeEqual && onlyMinorChanges(oldGroupDef, group); + + // dialog already warns us about this if the new group is named like another existing group + // We need to check if only the name changed as this is relevant for the entry's group field + if (groupTypeEqual && !group.getName().equals(oldGroupName) && onlyMinorModifications) { + int groupsWithSameName = 0; + Optional databaseRootGroup = currentDatabase.get().getMetaData().getGroups(); + if (databaseRootGroup.isPresent()) { + // we need to check the old name for duplicates. If the new group name occurs more than once, it won't matter + groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(oldGroupName)).size(); + } + boolean removePreviousAssignments = true; + // We found more than 2 groups, so we cannot simply remove old assignment + if (groupsWithSameName >= 2) { + removePreviousAssignments = false; + } + + oldGroup.getGroupNode().setGroup( + group, + true, + removePreviousAssignments, + database.getEntries()); + + dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName())); + writeGroupChangesToMetaData(); + // This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything + refresh(); + return; + } + + if (groupTypeEqual && onlyMinorChanges(oldGroup.getGroupNode().getGroup(), group)) { + oldGroup.getGroupNode().setGroup( + group, + true, + true, + database.getEntries()); + + writeGroupChangesToMetaData(); + refresh(); + return; + } + + // Major modifications + String content = Localization.lang("Assign the original group's entries to this group?"); ButtonType keepAssignments = new ButtonType(Localization.lang("Assign"), ButtonBar.ButtonData.YES); ButtonType removeAssignments = new ButtonType(Localization.lang("Do not assign"), ButtonBar.ButtonData.NO); @@ -206,17 +310,16 @@ public void editGroup(GroupNodeViewModel oldGroup) { if (newGroup.get().getClass() == WordKeywordGroup.class) { content = content + "\n\n" + - Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)"); + Localization.lang("(Note: If original entries lack keywords to qualify for the new group configuration, confirming here will add them)"); } Optional previousAssignments = dialogService.showCustomButtonDialogAndWait(Alert.AlertType.WARNING, - Localization.lang("Change of Grouping Method"), - content, - keepAssignments, - removeAssignments, - cancel); - // WarnAssignmentSideEffects.warnAssignmentSideEffects(newGroup, panel.frame()); + Localization.lang("Change of Grouping Method"), + content, + keepAssignments, + removeAssignments, + cancel); boolean removePreviousAssignments = (oldGroup.getGroupNode().getGroup() instanceof ExplicitGroup) - && (group instanceof ExplicitGroup); + && (group instanceof ExplicitGroup); int groupsWithSameName = 0; Optional databaseRootGroup = currentDatabase.get().getMetaData().getGroups(); @@ -224,22 +327,24 @@ public void editGroup(GroupNodeViewModel oldGroup) { String name = oldGroup.getGroupNode().getGroup().getName(); groupsWithSameName = databaseRootGroup.get().findChildrenSatisfying(g -> g.getName().equals(name)).size(); } + // okay we found more than 2 groups with the same name + // If we only found one we can still do it if (groupsWithSameName >= 2) { removePreviousAssignments = false; } if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.YES)) { oldGroup.getGroupNode().setGroup( - group, - true, - removePreviousAssignments, - database.getEntries()); + group, + true, + removePreviousAssignments, + database.getEntries()); } else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.NO)) { oldGroup.getGroupNode().setGroup( - group, - false, - removePreviousAssignments, - database.getEntries()); + group, + false, + removePreviousAssignments, + database.getEntries()); } else if (previousAssignments.isPresent() && (previousAssignments.get().getButtonData() == ButtonBar.ButtonData.CANCEL_CLOSE)) { return; } @@ -269,7 +374,6 @@ public void editGroup(GroupNodeViewModel oldGroup) { dialogService.notify(Localization.lang("Modified group \"%0\".", group.getName())); writeGroupChangesToMetaData(); - // This is ugly but we have no proper update mechanism in place to propagate the changes, so redraw everything refresh(); }); diff --git a/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java b/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java index a4bb0baab0c..b366d81c3db 100644 --- a/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java +++ b/src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java @@ -6,6 +6,7 @@ import javafx.scene.control.ButtonType; import javafx.scene.control.ListView; import javafx.scene.control.ScrollPane; +import javafx.scene.input.KeyCode; import org.jabref.gui.DialogService; import org.jabref.gui.JabRefFrame; @@ -51,6 +52,13 @@ public PreferencesDialogView(JabRefFrame frame) { ControlHelper.setAction(saveButton, getDialogPane(), event -> savePreferencesAndCloseDialog()); + // Stop the default button from firing when the user hits enter within the search box + searchBox.setOnKeyPressed(event -> { + if (event.getCode() == KeyCode.ENTER) { + event.consume(); + } + }); + themeManager.updateFontStyle(getDialogPane().getScene()); } diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java index 6cb8b15d722..efe4d233fee 100644 --- a/src/main/java/org/jabref/model/groups/GroupTreeNode.java +++ b/src/main/java/org/jabref/model/groups/GroupTreeNode.java @@ -72,17 +72,17 @@ public List setGroup(AbstractGroup newGroup, boolean shouldKeepPrev group = Objects.requireNonNull(newGroup); List changes = new ArrayList<>(); - boolean shouldRemove = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger); - boolean shouldAdd = shouldKeepPreviousAssignments && (newGroup instanceof GroupEntryChanger); - if (shouldAdd || shouldRemove) { + boolean shouldRemoveFromOldGroup = shouldRemovePreviousAssignments && (oldGroup instanceof GroupEntryChanger); + boolean shouldAddToNewGroup = shouldKeepPreviousAssignments && (newGroup instanceof GroupEntryChanger); + if (shouldAddToNewGroup || shouldRemoveFromOldGroup) { List entriesMatchedByOldGroup = entriesInDatabase.stream().filter(oldGroup::isMatch) .collect(Collectors.toList()); - if (shouldRemove) { + if (shouldRemoveFromOldGroup) { GroupEntryChanger entryChanger = (GroupEntryChanger) oldGroup; changes.addAll(entryChanger.remove(entriesMatchedByOldGroup)); } - if (shouldAdd) { + if (shouldAddToNewGroup) { GroupEntryChanger entryChanger = (GroupEntryChanger) newGroup; changes.addAll(entryChanger.add(entriesMatchedByOldGroup)); } diff --git a/src/main/resources/l10n/JabRef_en.properties b/src/main/resources/l10n/JabRef_en.properties index 91c4d19ad3e..bb1ae785eeb 100644 --- a/src/main/resources/l10n/JabRef_en.properties +++ b/src/main/resources/l10n/JabRef_en.properties @@ -1689,7 +1689,7 @@ User=User Connect=Connect Connection\ error=Connection error Connection\ to\ %0\ server\ established.=Connection to %0 server established. -There\ are\ connection\ issues\ with\ a\ JabRef\ server.\ Detailed\ information\:\ %0.=There are connection issues with a JabRef server. Detailed information: %0. +There\ are\ connection\ issues\ with\ a\ JabRef\ server.\ Detailed\ information\:\ %0=There are connection issues with a JabRef server. Detailed information: %0 Required\ field\ "%0"\ is\ empty.=Required field "%0" is empty. %0\ driver\ not\ available.=%0 driver not available. The\ connection\ to\ the\ server\ has\ been\ terminated.=The connection to the server has been terminated. diff --git a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java index c96d5262396..96ba2684e5d 100644 --- a/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java +++ b/src/test/java/org/jabref/gui/groups/GroupTreeViewModelTest.java @@ -12,6 +12,7 @@ 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.AllEntriesGroup; import org.jabref.model.groups.ExplicitGroup; import org.jabref.model.groups.GroupHierarchyType; @@ -20,17 +21,22 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import org.mockito.Answers; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; class GroupTreeViewModelTest { + private StateManager stateManager; private GroupTreeViewModel groupTree; private BibDatabaseContext databaseContext; private TaskExecutor taskExecutor; private PreferencesService preferencesService; + private DialogService dialogService; @BeforeEach void setUp() { @@ -39,12 +45,13 @@ void setUp() { stateManager.activeDatabaseProperty().setValue(Optional.of(databaseContext)); taskExecutor = new CurrentThreadTaskExecutor(); preferencesService = mock(PreferencesService.class); + dialogService = mock(DialogService.class, Answers.RETURNS_DEEP_STUBS); + when(preferencesService.getGroupsPreferences()).thenReturn(new GroupsPreferences( - GroupViewMode.UNION, - true, - true, - new SimpleObjectProperty<>(',') - )); + GroupViewMode.UNION, + true, + true, + new SimpleObjectProperty<>(','))); groupTree = new GroupTreeViewModel(stateManager, mock(DialogService.class), preferencesService, taskExecutor, new CustomLocalDragboard()); } @@ -85,4 +92,51 @@ void keywordGroupsAreNotRemovedFromEntriesOnDelete() { assertEquals(groupName, entry.getField(StandardField.KEYWORDS).get()); } + + @Test + void shouldNotShowDialogWhenGroupNameChanges() { + AbstractGroup oldGroup = new ExplicitGroup("group", GroupHierarchyType.INDEPENDENT, ','); + AbstractGroup newGroup = new ExplicitGroup("newGroupName", GroupHierarchyType.INDEPENDENT, ','); + BibEntry entry = new BibEntry(); + databaseContext.getDatabase().insertEntry(entry); + + GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); + assertTrue(model.onlyMinorChanges(oldGroup, newGroup)); + } + + @Test + void shouldNotShowDialogWhenGroupsAreEqual() { + AbstractGroup oldGroup = new WordKeywordGroup("group", GroupHierarchyType.INCLUDING, StandardField.KEYWORDS, "keywordTest", true, ',', true); + AbstractGroup newGroup = new WordKeywordGroup("group", GroupHierarchyType.INCLUDING, StandardField.KEYWORDS, "keywordTest", true, ',', true); + + BibEntry entry = new BibEntry(); + databaseContext.getDatabase().insertEntry(entry); + + GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); + assertTrue(model.onlyMinorChanges(oldGroup, newGroup)); + } + + @Test + void shouldShowDialogWhenKeywordDiffers() { + AbstractGroup oldGroup = new WordKeywordGroup("group", GroupHierarchyType.INCLUDING, StandardField.KEYWORDS, "keywordTest", true, ',', true); + AbstractGroup newGroup = new WordKeywordGroup("group", GroupHierarchyType.INCLUDING, StandardField.KEYWORDS, "keywordChanged", true, ',', true); + + BibEntry entry = new BibEntry(); + databaseContext.getDatabase().insertEntry(entry); + + GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); + assertFalse(model.onlyMinorChanges(oldGroup, newGroup)); + } + + @Test + void shouldShowDialogWhenCaseSensitivyDiffers() { + AbstractGroup oldGroup = new WordKeywordGroup("group", GroupHierarchyType.INCLUDING, StandardField.KEYWORDS, "keywordTest", false, ',', true); + AbstractGroup newGroup = new WordKeywordGroup("group", GroupHierarchyType.INCLUDING, StandardField.KEYWORDS, "keywordChanged", true, ',', true); + + BibEntry entry = new BibEntry(); + databaseContext.getDatabase().insertEntry(entry); + + GroupTreeViewModel model = new GroupTreeViewModel(stateManager, dialogService, preferencesService, taskExecutor, new CustomLocalDragboard()); + assertFalse(model.onlyMinorChanges(oldGroup, newGroup)); + } }