Merge pull request #5688 from JabRef/changemergesdialog

Show merge entries for modified entries

CHANGELOG.md

@@ -40,6 +40,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
 - We made the columns for groups, files and uri in the main table reorderable and merged the clickable icon columns for uri, url, doi and eprint. [#5544](https://github.com/JabRef/jabref/pull/5544)
 - We reduced the number of write actions performed when autosave is enabled [#5679](https://github.com/JabRef/jabref/issues/5679)
 - We made the column sort order in the main table persistent [#5730](https://github.com/JabRef/jabref/pull/5730)
+- When an entry is modified on disk, the change dialog now shows the merge dialog to highlight the changes [#5688](https://github.com/JabRef/jabref/pull/5688)
 
 ### Fixed
 

src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java

@@ -30,17 +30,23 @@ public ChangeDisplayDialog(BibDatabaseContext database, List<DatabaseChangeViewM
         this.getDialogPane().setPrefSize(800, 600);
 
         tree = new ListView<>(FXCollections.observableArrayList(changes));
-        tree.setPrefWidth(190);
+        tree.setPrefWidth(160);
         EasyBind.subscribe(tree.getSelectionModel().selectedItemProperty(), this::selectedChangeChanged);
 
         SplitPane pane = new SplitPane();
-        pane.setDividerPositions(0.25);
-        pane.getItems().addAll(new ScrollPane(tree), infoPanel);
+        pane.setDividerPositions(0.2);
+        ScrollPane scroll = new ScrollPane(tree);
+        scroll.setFitToHeight(true);
+        scroll.setFitToWidth(true);
+        pane.getItems().addAll(scroll, infoPanel);
+        pane.setResizableWithParent(scroll, false);
+
         getDialogPane().setContent(pane);
 
-        infoPanel.setBottom(cb);
+
         Label rootInfo = new Label(Localization.lang("Select the tree nodes to view and accept or reject changes") + '.');
         infoPanel.setCenter(rootInfo);
+        tree.getSelectionModel().select(0);
 
         ButtonType dismissChanges = new ButtonType(Localization.lang("Dismiss changes"), ButtonBar.ButtonData.CANCEL_CLOSE);
         getDialogPane().getButtonTypes().setAll(
@@ -67,7 +73,7 @@ public ChangeDisplayDialog(BibDatabaseContext database, List<DatabaseChangeViewM
         });
 
         EasyBind.subscribe(cb.selectedProperty(), selected -> {
-            if (selected != null && tree.getSelectionModel().getSelectedItem() != null) {
+            if ((selected != null) && (tree.getSelectionModel().getSelectedItem() != null)) {
                 tree.getSelectionModel().getSelectedItem().setAccepted(selected);
             }
         });
@@ -76,7 +82,10 @@ public ChangeDisplayDialog(BibDatabaseContext database, List<DatabaseChangeViewM
     private void selectedChangeChanged(DatabaseChangeViewModel currentChange) {
         if (currentChange != null) {
             infoPanel.setCenter(currentChange.description());
-            cb.setSelected(currentChange.isAccepted());
+            if (!(currentChange instanceof EntryChangeViewModel)) {
+                infoPanel.setBottom(cb);
+                cb.setSelected(currentChange.isAccepted());
+            }
         }
     }
 }

src/main/java/org/jabref/gui/collab/ChangeScanner.java

@@ -82,6 +82,6 @@ private DatabaseChangeViewModel createBibEntryDiff(BibEntryDiff diff) {
             return new EntryDeleteChangeViewModel(diff.getOriginalEntry());
         }
 
-        return new EntryChangeViewModel(diff.getOriginalEntry(), diff.getNewEntry());
+        return new EntryChangeViewModel(diff.getOriginalEntry(), diff.getNewEntry(), database);
     }
 }

src/main/java/org/jabref/gui/collab/DatabaseChangeMonitor.java

@@ -14,6 +14,7 @@
 import org.slf4j.LoggerFactory;
 
 public class DatabaseChangeMonitor implements FileUpdateListener {
+
     private static final Logger LOGGER = LoggerFactory.getLogger(DatabaseChangeMonitor.class);
 
     private final BibDatabaseContext database;
@@ -46,6 +47,7 @@ public void fileUpdated() {
                               listeners.forEach(listener -> listener.databaseChanged(changes));
                           }
                       })
+                      .onFailure(e -> LOGGER.error("Error while watching for changes", e))
                       .executeWith(taskExecutor);
     }
 

src/main/java/org/jabref/gui/collab/DatabaseChangeViewModel.java

@@ -32,8 +32,8 @@ public void setAccepted(boolean a) {
     }
 
     /**
-     * This method returns a JComponent detailing the nature of the change.
-     * @return JComponent
+     * This method returns a {@link Node} detailing the nature and look of the change, e.g. how it is displayed
+     * @return Node the Node with the layout of the change
      */
     public abstract Node description();
 

src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java

@@ -1,24 +1,17 @@
 package org.jabref.gui.collab;
 
-import java.util.ArrayList;
-import java.util.Comparator;
-import java.util.List;
-import java.util.Optional;
-import java.util.Set;
-import java.util.TreeSet;
-
+import javafx.geometry.Insets;
 import javafx.scene.Node;
 import javafx.scene.control.Label;
 import javafx.scene.layout.VBox;
 
+import org.jabref.gui.mergeentries.MergeEntries;
+import org.jabref.gui.mergeentries.MergeEntries.DefaultRadioButtonSelectionMode;
 import org.jabref.gui.undo.NamedCompound;
-import org.jabref.gui.undo.UndoableFieldChange;
+import org.jabref.gui.undo.UndoableInsertEntry;
 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.entry.field.Field;
-import org.jabref.model.strings.StringUtil;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -27,103 +20,44 @@ class EntryChangeViewModel extends DatabaseChangeViewModel {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(EntryChangeViewModel.class);
 
-    private final List<FieldChangeViewModel> fieldChanges = new ArrayList<>();
+    private final BibEntry oldEntry;
+    private final BibEntry newEntry;
+    private MergeEntries mergePanel;
+
+    private final BibDatabaseContext database;
 
-    public EntryChangeViewModel(BibEntry entry, BibEntry newEntry) {
+    public EntryChangeViewModel(BibEntry entry, BibEntry newEntry, BibDatabaseContext database) {
         super();
+
+        this.oldEntry = entry;
+        this.newEntry = newEntry;
+        this.database = database;
+
         name = entry.getCiteKeyOptional()
                     .map(key -> Localization.lang("Modified entry") + ": '" + key + '\'')
                     .orElse(Localization.lang("Modified entry"));
 
-        Set<Field> allFields = new TreeSet<>(Comparator.comparing(Field::getName));
-        allFields.addAll(entry.getFields());
-        allFields.addAll(newEntry.getFields());
-
-        for (Field field : allFields) {
-            Optional<String> value = entry.getField(field);
-            Optional<String> newValue = newEntry.getField(field);
-
-            if (value.isPresent() && newValue.isPresent()) {
-                if (!value.equals(newValue)) {
-                    // Modified externally.
-                    fieldChanges.add(new FieldChangeViewModel(entry, field, value.get(), newValue.get()));
-                }
-            } else {
-                // Added/removed externally.
-                fieldChanges.add(new FieldChangeViewModel(entry, field, value.orElse(null), newValue.orElse(null)));
-            }
-        }
     }
 
     @Override
     public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
-        for (DatabaseChangeViewModel c : fieldChanges) {
-            if (c.isAccepted()) {
-                c.makeChange(database, undoEdit);
-            }
-        }
+        database.getDatabase().removeEntry(oldEntry);
+        database.getDatabase().insertEntry(mergePanel.getMergeEntry());
+        undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), oldEntry));
+        undoEdit.addEdit(new UndoableInsertEntry(database.getDatabase(), mergePanel.getMergeEntry()));
     }
 
     @Override
     public Node description() {
+
+        mergePanel = new MergeEntries(oldEntry, newEntry, Localization.lang("In JabRef"), Localization.lang("On disk"), DefaultRadioButtonSelectionMode.LEFT);
+
         VBox container = new VBox(10);
         Label header = new Label(name);
         header.getStyleClass().add("sectionHeader");
         container.getChildren().add(header);
-
-        for (FieldChangeViewModel change : fieldChanges) {
-            container.getChildren().add(change.description());
-        }
-
+        container.getChildren().add(mergePanel);
+        container.setMargin(mergePanel, new Insets(5, 5, 5, 5));
         return container;
     }
-
-    static class FieldChangeViewModel extends DatabaseChangeViewModel {
-
-        private final BibEntry entry;
-        private final Field field;
-        private final String value;
-        private final String newValue;
-
-        public FieldChangeViewModel(BibEntry entry, Field field, String value, String newValue) {
-            super(field.getName());
-            this.entry = entry;
-            this.field = field;
-            this.value = value;
-            this.newValue = newValue;
-        }
-
-        @Override
-        public void makeChange(BibDatabaseContext database, NamedCompound undoEdit) {
-            Optional<FieldChange> change;
-            if (StringUtil.isBlank(newValue)) {
-                change = entry.clearField(field);
-            } else {
-                change = entry.setField(field, newValue);
-            }
-
-            change.map(UndoableFieldChange::new).ifPresent(undoEdit::addEdit);
-        }
-
-        @Override
-        public Node description() {
-            VBox container = new VBox();
-            container.getChildren().add(new Label(Localization.lang("Modification of field") + " " + field.getDisplayName()));
-
-            if (StringUtil.isNotBlank(newValue)) {
-                container.getChildren().add(new Label(Localization.lang("Value set externally") + ": " + newValue));
-            } else {
-                container.getChildren().add(new Label(Localization.lang("Value cleared externally")));
-            }
-
-            if (StringUtil.isNotBlank(value)) {
-                container.getChildren().add(new Label(Localization.lang("Current value") + ": " + value));
-            } else {
-                container.getChildren().add(new Label(Localization.lang("Current value") + ": " + Localization.lang("empty")));
-            }
-
-            return container;
-        }
-
-    }
 }

src/main/java/org/jabref/gui/collab/EntryDeleteChangeViewModel.java

@@ -21,6 +21,10 @@ class EntryDeleteChangeViewModel extends DatabaseChangeViewModel {
 
     public EntryDeleteChangeViewModel(BibEntry entry) {
         super(Localization.lang("Deleted entry"));
+
+        this.name = entry.getCiteKeyOptional()
+            .map(key -> Localization.lang("Deleted entry") + ": '" + key + '\'')
+            .orElse(Localization.lang("Deleted entry"));
         this.entry = entry;
     }
 

src/main/java/org/jabref/gui/collab/MetaDataChangeViewModel.java

@@ -20,6 +20,7 @@ public MetaDataChangeViewModel(MetaDataDiff metaDataDiff) {
 
     @Override
     public Node description() {
+
         /*
         // TODO: Show detailed description of the changes
         StringBuilder sb = new StringBuilder(

src/main/java/org/jabref/gui/duplicationFinder/DuplicateResolverDialog.java

@@ -63,14 +63,14 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
                 first = new ButtonType(Localization.lang("Keep left"), ButtonData.APPLY);
                 second = new ButtonType(Localization.lang("Keep right"), ButtonData.APPLY);
                 both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
-                me = new MergeEntries(one, two, database.getMode());
+                me = new MergeEntries(one, two);
                 break;
             case INSPECTION:
                 first = new ButtonType(Localization.lang("Remove old entry"), ButtonData.APPLY);
                 second = new ButtonType(Localization.lang("Remove entry from import"), ButtonData.APPLY);
                 both = new ButtonType(Localization.lang("Keep both"), ButtonData.APPLY);
                 me = new MergeEntries(one, two, Localization.lang("Old entry"),
-                        Localization.lang("From import"), database.getMode());
+                        Localization.lang("From import"));
                 break;
             case DUPLICATE_SEARCH_WITH_EXACT:
                 first = new ButtonType(Localization.lang("Keep left"), ButtonData.APPLY);
@@ -79,14 +79,14 @@ private void init(BibEntry one, BibEntry two, DuplicateResolverType type) {
 
                 removeExactVisible = true;
 
-                me = new MergeEntries(one, two, database.getMode());
+                me = new MergeEntries(one, two);
                 break;
             default:
                 first = new ButtonType(Localization.lang("Import and remove old entry"), ButtonData.APPLY);
                 second = new ButtonType(Localization.lang("Do not import entry"), ButtonData.APPLY);
                 both = new ButtonType(Localization.lang("Import and keep old entry"), ButtonData.APPLY);
                 me = new MergeEntries(one, two, Localization.lang("Old entry"),
-                        Localization.lang("From import"), database.getMode());
+                        Localization.lang("From import"));
                 break;
         }
         if (removeExactVisible) {

src/main/java/org/jabref/gui/mergeentries/FetchAndMergeEntry.java

@@ -84,7 +84,7 @@ public void fetchAndMerge(BibEntry entry, List<Field> fields) {
     }
 
     private void showMergeDialog(BibEntry originalEntry, BibEntry fetchedEntry, WebFetcher fetcher) {
-        MergeEntriesDialog dialog = new MergeEntriesDialog(originalEntry, fetchedEntry, panel.getBibDatabaseContext().getMode());
+        MergeEntriesDialog dialog = new MergeEntriesDialog(originalEntry, fetchedEntry);
         dialog.setTitle(Localization.lang("Merge entry with %0 information", fetcher.getName()));
         dialog.setLeftHeaderText(Localization.lang("Original entry"));
         dialog.setRightHeaderText(Localization.lang("Entry from %0", fetcher.getName()));