Changeset 18413 in josm

Timestamp:

2022-03-23T17:07:59+01:00 (2 years ago)

Author:

taylor.smock

Message:

fix #21825: delete non-useful relations

This offers to delete relations when users have either removed all
members or all tags.

Location:

trunk/src/org/openstreetmap/josm

Files:

• data/osm/IRelation.java (modified) (1 diff)
• gui/dialogs/relation/GenericRelationEditor.java (modified) (8 diffs)
• gui/dialogs/relation/actions/ApplyAction.java (modified) (1 diff)
• gui/dialogs/relation/actions/CancelAction.java (modified) (4 diffs)
• gui/dialogs/relation/actions/IRelationEditorActionAccess.java (modified) (2 diffs)
• gui/dialogs/relation/actions/SavingAction.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/data/osm/IRelation.java

@@ -139 +139 @@
                 .map(IRelationMember::getMember).collect(Collectors.toList());
     }
+
+    /**
+     * Check if this relation is useful
+     * @return {@code true} if this relation is useful
+     * @since 18413
+     */
+    default boolean isUseful() {
+        return !this.isEmpty() && this.hasKeys();
+    }
 }

trunk/src/org/openstreetmap/josm/gui/dialogs/relation/GenericRelationEditor.java

@@ -48 +48 @@
 import javax.swing.JToolBar;
 import javax.swing.KeyStroke;
+import javax.swing.event.TableModelListener;
 
 import org.openstreetmap.josm.actions.JosmAction;
@@ -55 +56 @@
 import org.openstreetmap.josm.data.UndoRedoHandler.CommandQueueListener;
 import org.openstreetmap.josm.data.osm.DefaultNameFormatter;
+import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
 import org.openstreetmap.josm.data.osm.Relation;
@@ -133 +135 @@
 
     private final AutoCompletingTextField tfRole;
+    private final RelationEditorActionAccess actionAccess;
 
     /**
@@ -263 +266 @@
         });
 
-        IRelationEditorActionAccess actionAccess = new RelationEditorActionAccess();
+        actionAccess = new RelationEditorActionAccess();
 
         refreshAction = new RefreshAction(actionAccess);
@@ -270 +273 @@
         duplicateAction = new DuplicateRelationAction(actionAccess);
         deleteAction = new DeleteCurrentRelationAction(actionAccess);
+
+        this.memberTableModel.addTableModelListener(applyAction);
+        this.tagEditorPanel.getModel().addTableModelListener(applyAction);
+
         addPropertyChangeListener(deleteAction);
 
@@ -277 +284 @@
         getContentPane().add(buildToolBar(refreshAction, applyAction, selectAction, duplicateAction, deleteAction), BorderLayout.NORTH);
         getContentPane().add(tabbedPane, BorderLayout.CENTER);
-        getContentPane().add(buildOkCancelButtonPanel(okAction, cancelAction), BorderLayout.SOUTH);
+        getContentPane().add(buildOkCancelButtonPanel(okAction, deleteAction, cancelAction), BorderLayout.SOUTH);
 
         setSize(findMaxDialogSize());
@@ -408 +415 @@
      * @return the panel with the OK and the Cancel button
      */
-    protected static JPanel buildOkCancelButtonPanel(OKAction okAction, CancelAction cancelAction) {
-        JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER));
-        pnl.add(new JButton(okAction));
+    protected final JPanel buildOkCancelButtonPanel(OKAction okAction, DeleteCurrentRelationAction deleteAction,
+            CancelAction cancelAction) {
+        final JPanel pnl = new JPanel(new FlowLayout(FlowLayout.CENTER));
+        final JButton okButton = new JButton(okAction);
+        final JButton deleteButton = new JButton(deleteAction);
+        okButton.setPreferredSize(deleteButton.getPreferredSize());
+        pnl.add(okButton);
+        pnl.add(deleteButton);
         pnl.add(new JButton(cancelAction));
         pnl.add(new JButton(new ContextSensitiveHelpAction(ht("/Dialog/RelationEditor"))));
+        // Keep users from saving invalid relations -- a relation MUST have at least a tag with the key "type"
+        // AND must contain at least one other OSM object.
+        final TableModelListener listener = l -> updateOkPanel(this.actionAccess.getChangedRelation(), okButton, deleteButton);
+        listener.tableChanged(null);
+        this.memberTableModel.addTableModelListener(listener);
+        this.tagEditorPanel.getModel().addTableModelListener(listener);
         return pnl;
+    }
+
+    /**
+     * Update the OK panel area
+     * @param newRelation What the new relation would "look" like if it were to be saved now
+     * @param okButton The OK button
+     * @param deleteButton The delete button
+     */
+    private void updateOkPanel(IRelation<?> newRelation, JButton okButton, JButton deleteButton) {
+        okButton.setVisible(newRelation.isUseful() || this.getRelationSnapshot() == null);
+        deleteButton.setVisible(!newRelation.isUseful() && this.getRelationSnapshot() != null);
+        if (this.getRelationSnapshot() == null && !newRelation.isUseful()) {
+            okButton.setText(tr("Delete"));
+        } else {
+            okButton.setText(tr("OK"));
+        }
     }
 
@@ -1002 +1036 @@
         public void mouseClicked(MouseEvent e) {
             if (e.getButton() == MouseEvent.BUTTON1 && e.getClickCount() == 2) {
-                new EditAction(new RelationEditorActionAccess()).actionPerformed(null);
+                new EditAction(actionAccess).actionPerformed(null);
             }
         }

trunk/src/org/openstreetmap/josm/gui/dialogs/relation/actions/ApplyAction.java

@@ -36 +36 @@
     @Override
     public void updateEnabledState() {
-        setEnabled(isEditorDirty());
+        setEnabled(this.editorAccess.getChangedRelation().isUseful() && isEditorDirty());
     }
 }

trunk/src/org/openstreetmap/josm/gui/dialogs/relation/actions/CancelAction.java

@@ -9 +9 @@
 import javax.swing.RootPaneContainer;
 
+import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
@@ -48 +49 @@
          && !(snapshot == null && getTagModel().getTags().isEmpty())) {
             //give the user a chance to save the changes
-            int ret = confirmClosingByCancel();
+            int ret = confirmClosingByCancel(this.editorAccess.getChangedRelation());
             if (ret == 0) { //Yes, save the changes
                 //copied from OKAction.run()
@@ -61 +62 @@
     }
 
-    protected int confirmClosingByCancel() {
+    protected int confirmClosingByCancel(final IRelation<?> newRelation) {
         ButtonSpec[] options = {
                 new ButtonSpec(
@@ -83 +84 @@
         };
 
+        // Keep users from saving invalid relations -- a relation MUST have at least a tag with the key "type"
+        // AND must contain at least one other OSM object.
+        options[0].setEnabled(newRelation.isUseful());
+
         return HelpAwareOptionPane.showOptionDialog(
                 MainApplication.getMainFrame(),

trunk/src/org/openstreetmap/josm/gui/dialogs/relation/actions/IRelationEditorActionAccess.java

@@ -4 +4 @@
 import javax.swing.Action;
 
+import org.openstreetmap.josm.data.osm.IRelation;
+import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.gui.dialogs.relation.IRelationEditor;
 import org.openstreetmap.josm.gui.dialogs.relation.MemberTable;
@@ -67 +69 @@
 
     /**
+     * Get the changed relation
+     * @return The changed relation (note: will not be part of a dataset). This should never be {@code null}.
+     * @since 18413
+     */
+    default IRelation<?> getChangedRelation() {
+        final Relation newRelation;
+        if (getEditor().getRelation() != null) {
+            newRelation = new Relation(getEditor().getRelation());
+        } else {
+            newRelation = new Relation();
+        }
+        getTagModel().applyToPrimitive(newRelation);
+        getMemberTableModel().applyToRelation(newRelation);
+        return newRelation;
+    }
+
+    /**
      * Get the text field that is used to edit the role.
      * @return The role text field.

trunk/src/org/openstreetmap/josm/gui/dialogs/relation/actions/SavingAction.java

@@ -55 +55 @@
         getMemberTableModel().applyToRelation(newRelation);
         // If the user wanted to create a new relation, but hasn't added any members or
-        // tags, don't add an empty relation
-        if (newRelation.isEmpty() && !newRelation.hasKeys())
+        // tags (specifically the "type" tag), don't add the relation
+        if (!newRelation.isUseful())
             return;
         UndoRedoHandler.getInstance().add(new AddCommand(getLayer().getDataSet(), newRelation));