Ticket #21825: 21825.7.patch

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

@@ -138 +140 @@
         return getMembers().stream().filter(rmv -> role.equals(rmv.getRole()))
                 .map(IRelationMember::getMember).collect(Collectors.toList());
     }
+
+    /**
+     * Check if this relation is useful
+     * @return {@code true} if this relation is useful
+     * @since xxx
+     */
+    default boolean isUseful() {
+        return !this.isEmpty() && this.hasKeys();
+    }
 }

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

@@ -47 +47 @@
 import javax.swing.JTable;
 import javax.swing.JToolBar;
 import javax.swing.KeyStroke;
+import javax.swing.event.TableModelListener;
 
 import org.openstreetmap.josm.actions.JosmAction;
 import org.openstreetmap.josm.command.ChangeMembersCommand;
@@ -54 +55 @@
 import org.openstreetmap.josm.data.UndoRedoHandler;
 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;
 import org.openstreetmap.josm.data.osm.RelationMember;
@@ -132 +134 @@
     private final SelectionTableModel selectionTableModel;
 
     private final AutoCompletingTextField tfRole;
+    private final RelationEditorActionAccess actionAccess;
 
     /**
      * the menu item in the windows menu. Required to properly hide on dialog close.
@@ -262 +265 @@
             selectedTabPane = sourceTabbedPane.getSelectedComponent();
         });
 
-        IRelationEditorActionAccess actionAccess = new RelationEditorActionAccess();
+        actionAccess = new RelationEditorActionAccess();
 
         refreshAction = new RefreshAction(actionAccess);
         applyAction = new ApplyAction(actionAccess);
         selectAction = new SelectAction(actionAccess);
         duplicateAction = new DuplicateRelationAction(actionAccess);
         deleteAction = new DeleteCurrentRelationAction(actionAccess);
+
+        this.memberTableModel.addTableModelListener(applyAction);
+        this.tagEditorPanel.getModel().addTableModelListener(applyAction);
+
         addPropertyChangeListener(deleteAction);
 
         okAction = new OKAction(actionAccess);
@@ -276 +283 @@
 
         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());
 
@@ -407 +414 @@
      *
      * @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"));
+        }
+    }
+
     /**
      * builds the panel with the tag editor
      * @param tagEditorPanel tag editor panel
@@ -1001 +1035 @@
         @Override
         public void mouseClicked(MouseEvent e) {
             if (e.getButton() == MouseEvent.BUTTON1 && e.getClickCount() == 2) {
-                new EditAction(new RelationEditorActionAccess()).actionPerformed(null);
+                new EditAction(actionAccess).actionPerformed(null);
             }
         }
     }

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

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

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

@@ -8 +8 @@
 import javax.swing.JOptionPane;
 import javax.swing.RootPaneContainer;
 
+import org.openstreetmap.josm.data.osm.IRelation;
 import org.openstreetmap.josm.data.osm.Relation;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane;
 import org.openstreetmap.josm.gui.HelpAwareOptionPane.ButtonSpec;
@@ -47 +48 @@
         if ((!getMemberTableModel().hasSameMembersAs(snapshot) || getTagModel().isDirty())
          && !(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()
                 Config.getPref().put("relation.editor.generic.lastrole", Utils.strip(tfRole.getText()));
@@ -60 +61 @@
         hideEditor();
     }
 
-    protected int confirmClosingByCancel() {
+    protected int confirmClosingByCancel(final IRelation<?> newRelation) {
         ButtonSpec[] options = {
                 new ButtonSpec(
                         tr("Yes, save the changes and close"),
@@ -82 +83 @@
                 )
         };
 
+        // 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(),
                 tr("<html>The relation has been changed.<br><br>Do you want to save your changes?</html>"),

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

@@ -3 +3 @@
 
 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;
 import org.openstreetmap.josm.gui.dialogs.relation.MemberTableModel;
@@ -65 +67 @@
      */
     TagEditorModel getTagModel();
 
+    /**
+     * Get the changed relation
+     * @return The changed relation (note: will not be part of a dataset). This should never be {@code null}.
+     * @since xxx
+     */
+    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.

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

@@ -54 +54 @@
         tagEditorModel.applyToPrimitive(newRelation);
         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));