Fixes that renaming a group did not change the group name in the inte…

…rface Fixes #3189.
JabRef · Dec 23, 2018 · fe6277c · fe6277c
1 parent 949d204
commit fe6277c
Show file tree

Hide file tree

Showing 6 changed files with 36 additions and 25 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -71,6 +71,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where the menu on Mac OS was not displayed in the usual Mac-specific way. https://github.com/JabRef/jabref/issues/3146
 - We improved the integrity check for page numbers. [#4113](https://github.com/JabRef/jabref/issues/4113) and [feature request in the forum](http://discourse.jabref.org/t/pages-field-allow-use-of-en-dash/1199)
 - We fixed an issue where the order of fields in customized entry types was not saved correctly. [#4033](http://github.com/JabRef/jabref/issues/4033)
+- We fixed an issue where renaming a group did not change the group name in the interface. [#3189](https://github.com/JabRef/jabref/issues/3189)
 - We fixed an issue where the groups tree of the last database was still shown even after the database was already closed.
 - We fixed an issue where the "Open file dialog" may disappear behind other windows. https://github.com/JabRef/jabref/issues/3410
 - We fixed an issue where the number of entries matched was not updated correctly upon adding or removing an entry. [#3537](https://github.com/JabRef/jabref/issues/3537)

diff --git a/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupNodeViewModel.java
@@ -63,16 +63,16 @@ public GroupNodeViewModel(BibDatabaseContext databaseContext, StateManager state
         this.groupNode = Objects.requireNonNull(groupNode);
         this.localDragBoard = Objects.requireNonNull(localDragBoard);
 
-        LatexToUnicodeFormatter formatter = new LatexToUnicodeFormatter();
-        displayName = formatter.format(groupNode.getName());
+        displayName = new LatexToUnicodeFormatter().format(groupNode.getName());
         isRoot = groupNode.isRoot();
         if (groupNode.getGroup() instanceof AutomaticGroup) {
             AutomaticGroup automaticGroup = (AutomaticGroup) groupNode.getGroup();
 
-            children = automaticGroup.createSubgroups(databaseContext.getDatabase().getEntries()).stream()
-                    .map(this::toViewModel)
-                    .sorted((group1, group2) -> group1.getDisplayName().compareToIgnoreCase(group2.getDisplayName()))
-                    .collect(Collectors.toCollection(FXCollections::observableArrayList));
+            children = automaticGroup.createSubgroups(this.databaseContext.getDatabase().getEntries())
+                                     .stream()
+                                     .map(this::toViewModel)
+                                     .sorted((group1, group2) -> group1.getDisplayName().compareToIgnoreCase(group2.getDisplayName()))
+                                     .collect(Collectors.toCollection(FXCollections::observableArrayList));
         } else {
             children = BindingsHelper.mapBacked(groupNode.getChildren(), this::toViewModel);
         }

diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeView.java b/src/main/java/org/jabref/gui/groups/GroupTreeView.java
@@ -114,15 +114,16 @@ public void initialize() {
 
         // Icon and group name
         mainColumn.setCellValueFactory(cellData -> cellData.getValue().valueProperty());
-        mainColumn.setCellFactory(new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
+        new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
                 .withText(GroupNodeViewModel::getDisplayName)
                 .withIcon(GroupNodeViewModel::getIcon)
-                .withTooltip(GroupNodeViewModel::getDescription));
+                .withTooltip(GroupNodeViewModel::getDescription)
+                .install(mainColumn);
 
         // Number of hits
         PseudoClass anySelected = PseudoClass.getPseudoClass("any-selected");
         PseudoClass allSelected = PseudoClass.getPseudoClass("all-selected");
-        numberColumn.setCellFactory(new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
+        new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
                 .withGraphic(group -> {
                     final StackPane node = new StackPane();
                     node.getStyleClass().setAll("hits");
@@ -138,11 +139,12 @@ public void initialize() {
                     node.getChildren().add(text);
                     node.setMaxWidth(Control.USE_PREF_SIZE);
                     return node;
-                }));
+                })
+                .install(numberColumn);
 
         // Arrow indicating expanded status
         disclosureNodeColumn.setCellValueFactory(cellData -> cellData.getValue().valueProperty());
-        disclosureNodeColumn.setCellFactory(new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
+        new ViewModelTreeTableCellFactory<GroupNodeViewModel, GroupNodeViewModel>()
                 .withGraphic(viewModel -> {
                     final StackPane disclosureNode = new StackPane();
                     disclosureNode.visibleProperty().bind(viewModel.hasChildrenProperty());
@@ -156,7 +158,8 @@ public void initialize() {
                 .withOnMouseClickedEvent(group -> event -> {
                     group.toggleExpansion();
                     event.consume();
-                }));
+                })
+                .install(disclosureNodeColumn);
 
         // Set pseudo-classes to indicate if row is root or sub-item ( > 1 deep)
         PseudoClass rootPseudoClass = PseudoClass.getPseudoClass("root");
@@ -316,6 +319,7 @@ private ContextMenu createContextMenuForGroup(GroupNodeViewModel group) {
         editGroup.setOnAction(event -> {
             menu.hide();
             viewModel.editGroup(group);
+            groupTree.refresh();
         });
 
         MenuItem addSubgroup = new MenuItem(Localization.lang("Add subgroup"));

diff --git a/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java b/src/main/java/org/jabref/gui/groups/GroupTreeViewModel.java
@@ -57,10 +57,13 @@ public GroupTreeViewModel(StateManager stateManager, DialogService dialogService
         EasyBind.subscribe(selectedGroups, this::onSelectedGroupChanged);
 
         // Set-up bindings
-        filterPredicate
-                .bind(Bindings.createObjectBinding(() -> group -> group.isMatchedBy(filterText.get()), filterText));
+        filterPredicate.bind(Bindings.createObjectBinding(() -> group -> group.isMatchedBy(filterText.get()), filterText));
 
         // Init
+        refresh();
+    }
+
+    private void refresh() {
         onActiveDatabaseChanged(stateManager.activeDatabaseProperty().getValue());
     }
 
@@ -166,13 +169,13 @@ public void editGroup(GroupNodeViewModel oldGroup) {
                     Localization.lang("Change of Grouping Method"),
                     Localization.lang("Assign the original group's entries to this group?"));
             //        WarnAssignmentSideEffects.warnAssignmentSideEffects(newGroup, panel.frame());
-            boolean removePreviousAssignents = (oldGroup.getGroupNode().getGroup() instanceof ExplicitGroup)
+            boolean removePreviousAssignments = (oldGroup.getGroupNode().getGroup() instanceof ExplicitGroup)
                     && (group instanceof ExplicitGroup);
 
             oldGroup.getGroupNode().setGroup(
                     group,
                     keepPreviousAssignments,
-                    removePreviousAssignents,
+                    removePreviousAssignments,
                     stateManager.getEntriesInCurrentDatabase());
 
             // TODO: Add undo
@@ -194,6 +197,9 @@ 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/util/ViewModelTreeTableCellFactory.java b/src/main/java/org/jabref/gui/util/ViewModelTreeTableCellFactory.java
@@ -84,4 +84,8 @@ protected void updateItem(T item, boolean empty) {
             }
         };
     }
+
+    public void install(TreeTableColumn<S, T> column) {
+        column.setCellFactory(this);
+    }
 }
diff --git a/src/main/java/org/jabref/model/groups/GroupTreeNode.java b/src/main/java/org/jabref/model/groups/GroupTreeNode.java
@@ -222,23 +222,19 @@ public GroupTreeNode copyNode() {
      * @param entries list of entries to be searched
      * @return number of hits
      */
-    public int calculateNumberOfMatches(List<BibEntry> entries) {
-        int hits = 0;
+    public long calculateNumberOfMatches(List<BibEntry> entries) {
         SearchMatcher matcher = getSearchMatcher();
-        for (BibEntry entry : entries) {
-            if (matcher.isMatch(entry)) {
-                hits++;
-            }
-        }
-        return hits;
+        return entries.stream()
+                      .filter(matcher::isMatch)
+                      .count();
     }
 
     /**
      * Determines the number of entries in the specified database which are matched by this group.
      * @param database database to be searched
      * @return number of hits
      */
-    public int calculateNumberOfMatches(BibDatabase database) {
+    public long calculateNumberOfMatches(BibDatabase database) {
         return calculateNumberOfMatches(database.getEntries());
     }
-Original file line number
+Diff line change
@@ Expand Up / @@ -84,4 +84,8 @@ protected void updateItem(T item, boolean empty) { @@
                 }
             };
         }
+        public void install(TreeTableColumn<S, T> column) {
+            column.setCellFactory(this);
+        }
     }