Merge remote-tracking branch 'upstream/master' into fileDescr

* upstream/master:
  Add tests for removal changes
  Improve telemetry (#3218)
  Real test linking real other entry (#3214)
  Check for explicit group at different location
  Update java-string-similarity from 0.24 -> 1.0.0
  Only remove ExplicitGroups from entries, but not keyword-based groups
  Remove unused import in GroupTreeNode
  Move getEntriesInGroup to GroupTreeNode
  Inline Consumer updateViewModel
  Undo IntelliJ autoformat of FXML annotations
  Remove unused import
  Rewrite selection logic to avoid NPEs
  Clear group selection when a group is removed
  Also remove a group from entries when you remove it

Author: Siedlerchr

CHANGELOG.md

@@ -39,6 +39,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We fixed an issue where the arrow keys in the search bar did not work as expected [#3081](https://github.com/JabRef/jabref/issues/3081)
 - We fixed wrong hotkey being displayed at "automatically file links" in the entry editor
 - We fixed an issue where metadata syncing with local and shared database were unstable. It will also fix syncing groups and sub-groups in database. [#2284](https://github.com/JabRef/jabref/issues/2284)
+- We fixed an issue where the "Remove group and subgroups" operation did not remove group information from entries in the group [#3190](https://github.com/JabRef/jabref/issues/3190)
 - We fixed an issue where it was possible to leave the entry editor with an imbalance of braces. [#3167](https://github.com/JabRef/jabref/issues/3167)
 - Renaming files now truncates the filename to not exceed the limit of 255 chars [#2622](https://github.com/JabRef/jabref/issues/2622)
 - We improved the handling of hyphens in names. [#2775](https://github.com/JabRef/jabref/issues/2775)

build.gradle

@@ -82,7 +82,7 @@ dependencies {
     compile 'org.openoffice:unoil:4.1.2'
 
     compile 'com.github.bkromhout:java-diff-utils:2.1.1'
-    compile 'info.debatty:java-string-similarity:0.24'
+    compile 'info.debatty:java-string-similarity:1.0.0'
 
     antlr3 'org.antlr:antlr:3.5.2'
     compile 'org.antlr:antlr-runtime:3.5.2'

src/main/java/org/jabref/FallbackExceptionHandler.java

@@ -1,24 +1,21 @@
 package org.jabref;
 
-import org.apache.logging.log4j.LogManager;
-import org.apache.logging.log4j.Logger;
-import org.apache.logging.log4j.Marker;
-import org.apache.logging.log4j.MarkerManager;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * Catch and log any unhandled exceptions.
  */
 public class FallbackExceptionHandler implements Thread.UncaughtExceptionHandler {
 
-    private static final Marker UncaughtException_MARKER = MarkerManager.getMarker("UncaughtException");
+    private static final Log LOGGER = LogFactory.getLog(FallbackExceptionHandler.class);
 
     public static void installExceptionHandler() {
         Thread.setDefaultUncaughtExceptionHandler(new FallbackExceptionHandler());
     }
 
     @Override
     public void uncaughtException(Thread thread, Throwable exception) {
-        Logger logger = LogManager.getLogger(FallbackExceptionHandler.class);
-        logger.error(UncaughtException_MARKER, "Uncaught exception Occurred in " + thread, exception);
+        LOGGER.error("Uncaught exception occurred in " + thread, exception);
     }
 }

src/main/java/org/jabref/gui/actions/NewEntryAction.java

@@ -1,10 +1,13 @@
 package org.jabref.gui.actions;
 
 import java.awt.event.ActionEvent;
+import java.util.HashMap;
+import java.util.Map;
 
 import javax.swing.Action;
 import javax.swing.KeyStroke;
 
+import org.jabref.Globals;
 import org.jabref.gui.EntryTypeDialog;
 import org.jabref.gui.IconTheme;
 import org.jabref.gui.JabRefFrame;
@@ -58,6 +61,8 @@ public void actionPerformed(ActionEvent e) {
                 return;
             }
             thisType = tp.getName();
+
+            trackNewEntry(tp);
         }
 
         if (jabRefFrame.getBasePanelCount() > 0) {
@@ -68,4 +73,12 @@ public void actionPerformed(ActionEvent e) {
             LOGGER.info("Action 'New entry' must be disabled when no database is open.");
         }
     }
+
+    private void trackNewEntry(EntryType type) {
+        Map<String, String> properties = new HashMap<>();
+        properties.put("EntryType", type.getName());
+        Map<String, Double> measurements = new HashMap<>();
+
+        Globals.getTelemetryClient().ifPresent(client -> client.trackEvent("NewEntry", properties, measurements));
+    }
 }

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

@@ -2,10 +2,10 @@
 
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 import java.util.function.Consumer;
-import java.util.stream.Collectors;
 
 import javax.inject.Inject;
 
@@ -79,19 +79,12 @@ public void initialize() {
         // Set-up bindings
         Consumer<ObservableList<GroupNodeViewModel>> updateSelectedGroups =
                 (newSelectedGroups) -> newSelectedGroups.forEach(this::selectNode);
-        Consumer<List<TreeItem<GroupNodeViewModel>>> updateViewModel =
-                (newSelectedGroups) -> {
-                    if (newSelectedGroups == null) {
-                        viewModel.selectedGroupsProperty().clear();
-                    } else {
-                        viewModel.selectedGroupsProperty().setAll(newSelectedGroups.stream().map(TreeItem::getValue).collect(Collectors.toList()));
-                    }
-                };
+
         BindingsHelper.bindContentBidirectional(
                 groupTree.getSelectionModel().getSelectedItems(),
                 viewModel.selectedGroupsProperty(),
                 updateSelectedGroups,
-                updateViewModel
+                this::updateSelection
         );
 
         viewModel.filterTextProperty().bind(searchField.textProperty());
@@ -247,6 +240,20 @@ public void initialize() {
         setupClearButtonField(searchField);
     }
 
+    private void updateSelection(List<TreeItem<GroupNodeViewModel>> newSelectedGroups) {
+        if (newSelectedGroups == null) {
+            viewModel.selectedGroupsProperty().clear();
+        } else {
+            List<GroupNodeViewModel> list = new ArrayList<>();
+            for (TreeItem<GroupNodeViewModel> model : newSelectedGroups) {
+                if (model != null && model.getValue() != null) {
+                    list.add(model.getValue());
+                }
+            }
+            viewModel.selectedGroupsProperty().setAll(list);
+        }
+    }
+
     private void selectNode(GroupNodeViewModel value) {
         getTreeItemByValue(value)
                 .ifPresent(treeItem -> groupTree.getSelectionModel().select(treeItem));
@@ -257,7 +264,7 @@ private Optional<TreeItem<GroupNodeViewModel>> getTreeItemByValue(GroupNodeViewM
     }
 
     private Optional<TreeItem<GroupNodeViewModel>> getTreeItemByValue(TreeItem<GroupNodeViewModel> root,
-            GroupNodeViewModel value) {
+                                                                      GroupNodeViewModel value) {
         if (root.getValue().equals(value)) {
             return Optional.of(root);
         }

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

@@ -27,6 +27,7 @@
 import org.jabref.logic.l10n.Localization;
 import org.jabref.model.FieldChange;
 import org.jabref.model.database.BibDatabaseContext;
+import org.jabref.model.entry.BibEntry;
 import org.jabref.model.groups.AbstractGroup;
 import org.jabref.model.groups.ExplicitGroup;
 import org.jabref.model.groups.GroupTreeNode;
@@ -248,13 +249,27 @@ public void removeGroupAndSubgroups(GroupNodeViewModel group) {
             //final UndoableAddOrRemoveGroup undo = new UndoableAddOrRemoveGroup(groupsRoot, node, UndoableAddOrRemoveGroup.REMOVE_NODE_AND_CHILDREN);
             //panel.getUndoManager().addEdit(undo);
 
+            removeGroupsAndSubGroupsFromEntries(group);
+
             group.getGroupNode().removeFromParent();
 
             dialogService.notify(Localization.lang("Removed group \"%0\" and its subgroups.", group.getDisplayName()));
             writeGroupChangesToMetaData();
         }
     }
 
+    void removeGroupsAndSubGroupsFromEntries(GroupNodeViewModel group) {
+        for (GroupNodeViewModel child: group.getChildren()) {
+            removeGroupsAndSubGroupsFromEntries(child);
+        }
+
+        // only remove explicit groups from the entries, keyword groups should not be deleted
+        if (group.getGroupNode().getGroup() instanceof ExplicitGroup) {
+            List<BibEntry> entriesInGroup = group.getGroupNode().getEntriesInGroup(this.currentDatabase.get().getEntries());
+            group.getGroupNode().removeEntriesFromGroup(entriesInGroup);
+        }
+    }
+
     public void addSelectedEntries(GroupNodeViewModel group) {
         // TODO: Warn
         // if (!WarnAssignmentSideEffects.warnAssignmentSideEffects(node.getNode().getGroup(), panel.frame())) {

src/main/java/org/jabref/gui/logging/ApplicationInsightsAppender.java

@@ -39,6 +39,7 @@ public void append(LogEvent rawEvent) {
         Telemetry telemetry;
         if (event.isException()) {
             ExceptionTelemetry exceptionTelemetry = new ExceptionTelemetry(event.getException());
+            exceptionTelemetry.getProperties().put("Message", rawEvent.getMessage().getFormattedMessage());
             exceptionTelemetry.setSeverityLevel(event.getNormalizedSeverityLevel());
             telemetry = exceptionTelemetry;
         } else {

src/main/java/org/jabref/model/database/BibDatabase.java

@@ -613,4 +613,5 @@ public String generateSharedDatabaseID() {
     public DuplicationChecker getDuplicationChecker() {
         return duplicationChecker;
     }
+
 }

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

@@ -181,6 +181,16 @@ public List<GroupTreeNode> getMatchingGroups(List<BibEntry> entries) {
         return groups;
     }
 
+    public List<BibEntry> getEntriesInGroup(List<BibEntry> entries) {
+        List<BibEntry> result = new ArrayList<>();
+        for (BibEntry entry: entries) {
+            if (this.group.contains(entry)) {
+                result.add(entry);
+            }
+        }
+        return result;
+    }
+
     public String getName() {
         return group.getName();
     }

src/test/java/org/jabref/gui/groups/GroupNodeViewModelTest.java

@@ -10,8 +10,10 @@
 import org.jabref.gui.util.TaskExecutor;
 import org.jabref.model.database.BibDatabaseContext;
 import org.jabref.model.entry.BibEntry;
+import org.jabref.model.entry.FieldName;
 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;
@@ -149,6 +151,20 @@ public void draggedOnTopOfGroupAddsBeforeItWhenSourceGroupWasBefore() throws Exc
         assertEquals(Arrays.asList(groupBViewModel, groupAViewModel, groupCViewModel), rootViewModel.getChildren());
     }
 
+    @Test
+    public void entriesAreAddedCorrectly() {;
+        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);
+        model.addEntriesToGroup(databaseContext.getEntries());
+
+        assertEquals(databaseContext.getEntries(), model.getGroupNode().getEntriesInGroup(databaseContext.getEntries()));
+        assertEquals(groupName, entry.getField(FieldName.GROUPS).get());
+    }
+
     private GroupNodeViewModel getViewModelForGroup(AbstractGroup group) {
         return new GroupNodeViewModel(databaseContext, stateManager, taskExecutor, group);
     }