Fix move to group always moves first entry (#4457)

* Fix move to group always moves first entry * clarify comment a bit better
JabRef · Nov 9, 2018 · bab9ae8 · bab9ae8
1 parent 144522d
commit bab9ae8
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 8 deletions.
diff --git a/CHANGELOG.md b/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)
 
 
 

diff --git a/src/main/java/org/jabref/gui/groups/GroupAddRemoveDialog.java b/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;