Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show merge entries for modified entries #5688

Merged
merged 34 commits into from
Dec 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
7eab878
[WIP] Show merge entries for modified entries
Siedlerchr Nov 30, 2019
1965b74
refactor and insert entry
Siedlerchr Nov 30, 2019
ceb9a6a
remove obsolete l10n key
Siedlerchr Nov 30, 2019
abd0d5d
Merge remote-tracking branch 'upstream/master' into changemergesdialog
Siedlerchr Dec 5, 2019
ab2beef
Merge remote-tracking branch 'upstream/master' into changemergesdialog
Siedlerchr Dec 7, 2019
743554a
fix spacing
Siedlerchr Dec 8, 2019
21b4de3
Merge remote-tracking branch 'upstream/master' into changemergesdialog
Siedlerchr Dec 14, 2019
dd3defe
fix javadoc
Siedlerchr Dec 14, 2019
ab48a98
Merge remote-tracking branch 'upstream/master' into changemergesdialog
Siedlerchr Dec 20, 2019
6884034
pass second entry to merge dialog
Siedlerchr Dec 20, 2019
475fb6b
add Exception logging to Databsae Change Monitor
Siedlerchr Dec 20, 2019
014edb1
fix l10n
Siedlerchr Dec 20, 2019
916498f
add changelog
Siedlerchr Dec 20, 2019
340a43f
remove old entry, move merge Panel init to description
Siedlerchr Dec 20, 2019
c909db9
fix error
Siedlerchr Dec 20, 2019
2750183
beautify controls
Siedlerchr Dec 20, 2019
22b0939
improve layout of merge dialog
Siedlerchr Dec 21, 2019
c825212
Merge remote-tracking branch 'upstream/master' into changemergesdialog
Siedlerchr Dec 23, 2019
7b0ee12
remove checkbox when merge entries
Siedlerchr Dec 24, 2019
392f51c
make koppor happy
Siedlerchr Dec 26, 2019
001e3f0
fix tests
Siedlerchr Dec 26, 2019
e830f52
remove obsolete parameter
Siedlerchr Dec 26, 2019
798d497
Fix parameter name
koppor Dec 26, 2019
0b62ff6
Remove obsolete newline
koppor Dec 26, 2019
f266af3
Fix localization
koppor Dec 26, 2019
241502c
Change arrows
koppor Dec 26, 2019
1432148
Fix checkstyle
koppor Dec 26, 2019
53a0b11
Center selection column
koppor Dec 26, 2019
55e2f23
Remove obsolete language keys
koppor Dec 26, 2019
09fbfca
Remove l10n from arrows
Siedlerchr Dec 26, 2019
f033134
Update arrows (#5787)
koppor Dec 26, 2019
756d4bc
preselect left
Siedlerchr Dec 29, 2019
fa6fb54
Merge remote-tracking branch 'upstream/master' into changemergesdialog
Siedlerchr Dec 29, 2019
4c0e055
fix checkstyle
Siedlerchr Dec 29, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
21 changes: 15 additions & 6 deletions src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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);
}
});
Expand All @@ -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());
}
}
}
}
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/collab/ChangeScanner.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
114 changes: 24 additions & 90 deletions src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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());
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
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));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not seem to work, as there appears to be no space on the left side (or 5 is too small)

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;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public MetaDataChangeViewModel(MetaDataDiff metaDataDiff) {

@Override
public Node description() {

/*
// TODO: Show detailed description of the changes
StringBuilder sb = new StringBuilder(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
Loading