3189 fix group renaming

CHANGELOG.md

@@ -80,7 +80,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where the list of XMP Exclusion fields in the preferences was not be saved [#4072](https://github.com/JabRef/jabref/issues/4072)
 - We fixed an issue where the ArXiv Fetcher did not support HTTP URLs [koppor#328](https://github.com/koppor/jabref/issues/328)
 - We fixed an issue where only one PDF file could be imported [#4422](https://github.com/JabRef/jabref/issues/4422)
-
+- We fixed an issue where "Move to group" would always move the first entry in the library and not the selected [#4414](https://github.com/JabRef/jabref/issues/4414)
 
 
 

build.gradle

@@ -174,8 +174,8 @@ dependencies {
     testCompile 'org.reflections:reflections:0.9.11'
     testCompile 'org.xmlunit:xmlunit-core:2.6.2'
     testCompile 'org.xmlunit:xmlunit-matchers:2.6.2'
-    testCompile 'com.tngtech.archunit:archunit-junit5-api:0.9.1'
-    testRuntime 'com.tngtech.archunit:archunit-junit5-engine:0.9.1'
+    testCompile 'com.tngtech.archunit:archunit-junit5-api:0.9.2'
+    testRuntime 'com.tngtech.archunit:archunit-junit5-engine:0.9.2'
     testCompile "org.testfx:testfx-core:4.0.+"
     testCompile "org.testfx:testfx-junit5:4.0.+"
 

src/main/java/org/jabref/gui/groups/GroupAddRemoveDialog.java

@@ -4,6 +4,7 @@
 import java.awt.Color;
 import java.awt.Component;
 import java.awt.event.ActionEvent;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 
@@ -57,9 +58,9 @@ public void action() throws Exception {
         selection = panel.getSelectedEntries();
 
         final JDialog diag = new JDialog((JFrame) null,
-                (add ? (move ? Localization.lang("Move to group") : Localization.lang("Add to group")) : Localization
-                        .lang("Remove from group")),
-                true);
+                                         (add ? (move ? Localization.lang("Move to group") : Localization.lang("Add to group")) : Localization
+                                                                                                                                              .lang("Remove from group")),
+                                         true);
         JButton ok = new JButton(Localization.lang("OK"));
         JButton cancel = new JButton(Localization.lang("Cancel"));
         tree = new JTree(new GroupTreeNodeViewModel(groups.get()));
@@ -166,7 +167,9 @@ private boolean doAddOrRemove() {
             GroupTreeNodeViewModel node = (GroupTreeNodeViewModel) path.getLastPathComponent();
             if (checkGroupEnable(node)) {
 
-                List<BibEntry> entries = Globals.stateManager.getSelectedEntries();
+                //we need to copy the contents of the observable list here, because when removeFromEntries is called,
+                //probably the focus changes to the first entry in the all entries group and thus getSelectedEntries() no longer contains our entry we want to move
+                List<BibEntry> entries = new ArrayList<>(Globals.stateManager.getSelectedEntries());
 
                 if (move) {
                     recuriveRemoveFromNode((GroupTreeNodeViewModel) tree.getModel().getRoot(), entries);
@@ -175,7 +178,7 @@ private boolean doAddOrRemove() {
                 if (add) {
                     node.addEntriesToGroup(entries);
                 } else {
-                    node.removeEntriesFromGroup(Globals.stateManager.getSelectedEntries());
+                    node.removeEntriesFromGroup(entries);
                 }
 
                 return true;
@@ -187,7 +190,7 @@ private boolean doAddOrRemove() {
 
     private void recuriveRemoveFromNode(GroupTreeNodeViewModel node, List<BibEntry> entries) {
         node.removeEntriesFromGroup(entries);
-        for (GroupTreeNodeViewModel child: node.getChildren()) {
+        for (GroupTreeNodeViewModel child : node.getChildren()) {
             recuriveRemoveFromNode(child, entries);
         }
     }
@@ -207,7 +210,7 @@ class AddRemoveGroupTreeCellRenderer extends GroupTreeCellRenderer {
 
         @Override
         public Component getTreeCellRendererComponent(JTree tree, Object value, boolean selected, boolean expanded,
-                boolean leaf, int row, boolean hasFocus) {
+                                                      boolean leaf, int row, boolean hasFocus) {
             Component c = super.getTreeCellRendererComponent(tree, value, selected, expanded, leaf, row, hasFocus);
 
             GroupTreeNodeViewModel node = (GroupTreeNodeViewModel) value;

src/main/java/org/jabref/model/groups/AbstractGroup.java

@@ -4,6 +4,8 @@
 import java.util.Objects;
 import java.util.Optional;
 
+import javafx.beans.property.SimpleStringProperty;
+import javafx.beans.property.StringProperty;
 import javafx.scene.paint.Color;
 
 import org.jabref.model.entry.BibEntry;
@@ -18,7 +20,7 @@ public abstract class AbstractGroup implements SearchMatcher {
     /**
      * The group's name.
      */
-    protected final String name;
+    protected final StringProperty name = new SimpleStringProperty();
     /**
      * The hierarchical context of the group.
      */
@@ -29,14 +31,14 @@ public abstract class AbstractGroup implements SearchMatcher {
     protected Optional<String> iconName = Optional.empty();
 
     protected AbstractGroup(String name, GroupHierarchyType context) {
-        this.name = name;
+        this.name.setValue(name);
         this.context = Objects.requireNonNull(context);
     }
 
     @Override
     public String toString() {
         return "AbstractGroup{" +
-                "name='" + name + '\'' +
+                "name='" + name.getValue() + '\'' +
                 ", context=" + context +
                 ", color=" + color +
                 ", isExpanded=" + isExpanded +
@@ -54,13 +56,13 @@ public boolean equals(Object other) {
             return false;
         }
         AbstractGroup that = (AbstractGroup) other;
-        return Objects.equals(this.name, that.name) && Objects.equals(this.description, that.description)
+        return Objects.equals(this.name.getValue(), that.name.getValue()) && Objects.equals(this.description, that.description)
                 && Objects.equals(this.context, that.context);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(name, description, context);
+        return Objects.hash(name.getValue(), description, context);
     }
 
     public Optional<Color> getColor() {
@@ -122,6 +124,10 @@ public GroupHierarchyType getHierarchicalContext() {
      * Returns this group's name, e.g. for display in a list/tree.
      */
     public final String getName() {
+        return name.getValue();
+    }
+
+    public StringProperty nameProperty() {
         return name;
     }
 

src/main/java/org/jabref/model/groups/AutomaticKeywordGroup.java

@@ -37,7 +37,7 @@ public String getField() {
 
     @Override
     public AbstractGroup deepCopy() {
-        return new AutomaticKeywordGroup(this.name, this.context, field, this.keywordDelimiter, keywordHierarchicalDelimiter);
+        return new AutomaticKeywordGroup(this.name.getValue(), this.context, field, this.keywordDelimiter, keywordHierarchicalDelimiter);
     }
 
     @Override

src/main/java/org/jabref/model/groups/AutomaticPersonsGroup.java

@@ -38,7 +38,7 @@ public int hashCode() {
 
     @Override
     public AbstractGroup deepCopy() {
-        return new AutomaticPersonsGroup(this.name, this.context, this.field);
+        return new AutomaticPersonsGroup(this.name.getValue(), this.context, this.field);
     }
 
     @Override

src/main/java/org/jabref/model/groups/ExplicitGroup.java

@@ -64,7 +64,7 @@ public List<String> getLegacyEntryKeys() {
 
     @Override
     public int hashCode() {
-        return Objects.hash(name, context, legacyEntryKeys, iconName, color, description, isExpanded);
+        return Objects.hash(name.getValue(), context, legacyEntryKeys, iconName, color, description, isExpanded);
     }
 
     @Override

src/main/java/org/jabref/model/groups/TexGroup.java

@@ -49,7 +49,7 @@ public boolean isDynamic() {
     @Override
     public AbstractGroup deepCopy() {
         try {
-            return new TexGroup(name, context, filePath, auxParser, fileMonitor);
+            return new TexGroup(name.getValue(), context, filePath, auxParser, fileMonitor);
         } catch (IOException ex) {
             // This should never happen because we were able to monitor the file just fine until now
             LOGGER.error("Problem creating copy of group", ex);