From f53cf29ca06b7e4db204f44ab386a9ae09214da6 Mon Sep 17 00:00:00 2001 From: Florian Kargl Date: Tue, 8 Feb 2022 20:53:35 +0100 Subject: [PATCH] Automatically refresh relation editor when relation is changed outside of editor Relation in editor is automatically refreshed when no changes have been made in the editor yet. Display a warning notification about required conflict resolution otherwise. --- .../data/conflict/ConflictCollection.java | 8 +++--- .../relation/GenericRelationEditor.java | 26 ++++++++++++++++--- .../gui/dialogs/relation/IRelationEditor.java | 24 ++++++++++++++++- .../gui/dialogs/relation/RelationEditor.java | 12 ++++++++- .../relation/actions/RefreshAction.java | 12 +-------- .../relation/actions/SavingAction.java | 4 +-- .../relation/GenericRelationEditorTest.java | 10 +++++++ 7 files changed, 72 insertions(+), 24 deletions(-) diff --git a/src/org/openstreetmap/josm/data/conflict/ConflictCollection.java b/src/org/openstreetmap/josm/data/conflict/ConflictCollection.java index 54550058b7f..a242d5d6dd4 100644 --- a/src/org/openstreetmap/josm/data/conflict/ConflictCollection.java +++ b/src/org/openstreetmap/josm/data/conflict/ConflictCollection.java @@ -316,8 +316,8 @@ public final Collection> getNodeConflicts() { } /** - * Returns the list of conflicts involving nodes. - * @return The list of conflicts involving nodes. + * Returns the list of conflicts involving ways. + * @return The list of conflicts involving ways. * @since 6555 */ public final Collection> getWayConflicts() { @@ -325,8 +325,8 @@ public final Collection> getWayConflicts() { } /** - * Returns the list of conflicts involving nodes. - * @return The list of conflicts involving nodes. + * Returns the list of conflicts involving relations. + * @return The list of conflicts involving relations. * @since 6555 */ public final Collection> getRelationConflicts() { diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java index 743e05f4e08..b611e64b592 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java @@ -62,6 +62,7 @@ import org.openstreetmap.josm.gui.ConditionalOptionPaneUtil; import org.openstreetmap.josm.gui.MainApplication; import org.openstreetmap.josm.gui.MainMenu; +import org.openstreetmap.josm.gui.Notification; import org.openstreetmap.josm.gui.ScrollViewport; import org.openstreetmap.josm.gui.datatransfer.ClipboardUtils; import org.openstreetmap.josm.gui.dialogs.relation.actions.AbstractRelationEditorAction; @@ -1048,10 +1049,27 @@ public AutoCompletingTextField getTextFieldRole() { @Override public void commandChanged(int queueSize, int redoSize) { Relation r = getRelation(); - if (r != null && r.getDataSet() == null) { - // see #19915 - setRelation(null); - applyAction.updateEnabledState(); + if (r != null) { + if (r.getDataSet() == null) { + // see #19915 + setRelation(null); + applyAction.updateEnabledState(); + } else if (isDirtyRelation()) { + if (!isDirtyEditor()) { + reloadDataFromRelation(); + } else { + new Notification(tr("Relation modified outside of relation editor with pending changes. Conflict resolution required.")) + .setIcon(JOptionPane.WARNING_MESSAGE).show(); + } + } } } + + @Override + public boolean isDirtyEditor() { + Relation snapshot = getRelationSnapshot(); + Relation relation = getRelation(); + return (snapshot != null && !memberTableModel.hasSameMembersAs(snapshot)) || + tagEditorPanel.getModel().isDirty() || relation == null || relation.getDataSet() == null; + } } diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/IRelationEditor.java b/src/org/openstreetmap/josm/gui/dialogs/relation/IRelationEditor.java index 049504b3718..3765ef45acb 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/IRelationEditor.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/IRelationEditor.java @@ -39,7 +39,29 @@ public interface IRelationEditor { * * @return true if the currently edited relation has been changed elsewhere. */ - boolean isDirtyRelation(); + default boolean isDirtyRelation() { + return isDirtyRelation(false); + } + + /** + * Replies true if the currently edited relation has been changed elsewhere. + * + * In this case a relation editor can't apply updates to the relation directly. Rather, + * it has to create a conflict. + * + * @param ignoreUninterestingTags whether to ignore uninteresting tag changes + * @return true if the currently edited relation has been changed elsewhere. + */ + boolean isDirtyRelation(boolean ignoreUninterestingTags); + + /** + * Replies true if the relation has been changed in the editor (but not yet applied). + * + * Reloading data from the relation would cause the pending changes to be lost. + * + * @return true if the currently edited relation has been changed in the editor. + */ + boolean isDirtyEditor(); /** * Reloads data from relation. diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/RelationEditor.java b/src/org/openstreetmap/josm/gui/dialogs/relation/RelationEditor.java index 39dc13fb6f2..e429f4bda0b 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/RelationEditor.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/RelationEditor.java @@ -147,7 +147,17 @@ protected final void setRelationSnapshot(Relation snapshot) { @Override public final boolean isDirtyRelation() { - return !relation.hasEqualSemanticAttributes(relationSnapshot); + return isDirtyRelation(false); + } + + @Override + public final boolean isDirtyRelation(boolean ignoreUninterestingTags) { + if (relation != null && relation.getDataSet() == null && + relationSnapshot != null && relationSnapshot.getDataSet() == null) { + return false; + } + + return !relation.hasEqualSemanticAttributes(relationSnapshot, ignoreUninterestingTags); } /* ----------------------------------------------------------------------- */ diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/RefreshAction.java b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/RefreshAction.java index 744bbcdf435..f02be9ea5c8 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/RefreshAction.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/RefreshAction.java @@ -67,17 +67,7 @@ public void actionPerformed(ActionEvent e) { @Override public void updateEnabledState() { - Relation snapshot = getEditor().getRelationSnapshot(); - Relation relation = getEditor().getRelation(); - if (relation != null && relation.getDataSet() == null) - relation = null; // see #19915 - if (relation != null && snapshot != null && snapshot.getDataSet() == null) { - // relation was changed outside of the editor - // either it was modified or deleted or changed by an undo - setEnabled(!snapshot.hasEqualSemanticAttributes(relation, false /* don't ignore uninteresting keys */)); - return; - } - setEnabled(false); + setEnabled(getEditor().isDirtyRelation()); } protected int confirmDiscardDirtyData() { diff --git a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java index 26813f23c1f..79e9b967976 100644 --- a/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java +++ b/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java @@ -192,8 +192,6 @@ protected void hideEditor() { } protected boolean isEditorDirty() { - Relation snapshot = editorAccess.getEditor().getRelationSnapshot(); - return (snapshot != null && !getMemberTableModel().hasSameMembersAs(snapshot)) || getTagModel().isDirty() - || getEditor().getRelation() == null || getEditor().getRelation().getDataSet() == null; + return editorAccess.getEditor().isDirtyEditor(); } } diff --git a/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java b/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java index ac2a9d4628a..199d8f2dc48 100644 --- a/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java +++ b/test/unit/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditorTest.java @@ -58,6 +58,16 @@ public boolean isDirtyRelation() { return false; } + @Override + public boolean isDirtyRelation(boolean ignoreUninterestingTags) { + return false; + } + + @Override + public boolean isDirtyEditor() { + return false; + } + @Override public Relation getRelationSnapshot() { return r;