Make sure that also groups with "required" layers cannot be removed

Fixes an unreported bug where it was possible to select a layer tree group and remove even when it contained required layers. When removing layers/groups, the following checks will now also recursively test layers and not just directly selected layers: - test for layers with unsaved changes - test for layers with active tasks running in background
qgis · Sep 26, 2018 · ea17162 · ea17162
1 parent 6aa00b1
commit ea17162
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 10 deletions.
diff --git a/python/gui/auto_generated/layertree/qgslayertreeview.sip.in b/python/gui/auto_generated/layertree/qgslayertreeview.sip.in
@@ -109,6 +109,15 @@ Returns list of selected nodes filtered to just layer nodes
     QList<QgsMapLayer *> selectedLayers() const;
 %Docstring
 Gets list of selected layers
+%End
+
+    QList<QgsMapLayer *> selectedLayersRecursive() const;
+%Docstring
+Gets list of selected layers, including those that are not directly selected, but their
+ancestor groups is selected. If we have a group with two layers L1, L2 and just the group
+node is selected, this method returns L1 and L2, while selectedLayers() returns an empty list.
+
+.. versionadded:: 3.4
 %End
 
     void addIndicator( QgsLayerTreeNode *node, QgsLayerTreeViewIndicator *indicator );

diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -9564,7 +9564,21 @@ void QgisApp::removeLayer()
     return;
   }
 
-  const QList<QgsMapLayer *> selectedLayers = mLayerTreeView->selectedLayers();
+  // look for layers recursively so we catch also those that are within selected groups
+  const QList<QgsMapLayer *> selectedLayers = mLayerTreeView->selectedLayersRecursive();
+
+  QStringList nonRemovableLayerNames;
+  for ( QgsMapLayer *layer : selectedLayers )
+  {
+    if ( !layer->flags().testFlag( QgsMapLayer::Removable ) )
+      nonRemovableLayerNames << layer->name();
+  }
+  if ( !nonRemovableLayerNames.isEmpty() )
+  {
+    QMessageBox::warning( this, tr( "Required Layers" ),
+                          tr( "The following layers are marked as required by the project:\n\n%1\n\nPlease deselect them (or unmark as required) and retry." ).arg( nonRemovableLayerNames.join( QStringLiteral( "\n" ) ) ) );
+    return;
+  }
 
   for ( QgsMapLayer *layer : selectedLayers )
   {
@@ -9586,15 +9600,6 @@ void QgisApp::removeLayer()
     }
   }
 
-  // extra check for required layers
-  // In theory it should not be needed because the remove action should be disabled
-  // if there are required layers in the selection...
-  for ( QgsMapLayer *layer : selectedLayers )
-  {
-    if ( !layer->flags().testFlag( QgsMapLayer::Removable ) )
-      return;
-  }
-
   if ( !activeTaskDescriptions.isEmpty() )
   {
     QMessageBox::warning( this, tr( "Active Tasks" ),

diff --git a/src/gui/layertree/qgslayertreeview.cpp b/src/gui/layertree/qgslayertreeview.cpp
@@ -356,6 +356,31 @@ QList<QgsMapLayer *> QgsLayerTreeView::selectedLayers() const
   return list;
 }
 
+static void _collectMapLayers( const QList<QgsLayerTreeNode *> &nodes, QSet<QgsMapLayer *> &layersSet )
+{
+  for ( QgsLayerTreeNode *node : nodes )
+  {
+    if ( QgsLayerTree::isLayer( node ) )
+    {
+      QgsLayerTreeLayer *nodeLayer = QgsLayerTree::toLayer( node );
+      if ( nodeLayer->layer() )
+        layersSet << nodeLayer->layer();
+    }
+    else if ( QgsLayerTree::isGroup( node ) )
+    {
+      _collectMapLayers( QgsLayerTree::toGroup( node )->children(), layersSet );
+    }
+  }
+}
+
+QList<QgsMapLayer *> QgsLayerTreeView::selectedLayersRecursive() const
+{
+  QSet<QgsMapLayer *> layersSet;
+  const QList<QgsLayerTreeNode *> nodes = layerTreeModel()->indexes2nodes( selectionModel()->selectedIndexes(), false );
+  _collectMapLayers( nodes, layersSet );
+  return layersSet.toList();
+}
+
 void QgsLayerTreeView::addIndicator( QgsLayerTreeNode *node, QgsLayerTreeViewIndicator *indicator )
 {
   if ( !mIndicators[node].contains( indicator ) )

diff --git a/src/gui/layertree/qgslayertreeview.h b/src/gui/layertree/qgslayertreeview.h
@@ -107,6 +107,14 @@ class GUI_EXPORT QgsLayerTreeView : public QTreeView
     //! Gets list of selected layers
     QList<QgsMapLayer *> selectedLayers() const;
 
+    /**
+     * Gets list of selected layers, including those that are not directly selected, but their
+     * ancestor groups is selected. If we have a group with two layers L1, L2 and just the group
+     * node is selected, this method returns L1 and L2, while selectedLayers() returns an empty list.
+     * \since QGIS 3.4
+     */
+    QList<QgsMapLayer *> selectedLayersRecursive() const;
+
     /**
      * Adds an indicator to the given layer tree node. Indicators are icons shown next to layer/group names
      * in the layer tree view. They can be used to show extra information with tree nodes and they allow