Disable remove action instead of message box + unit test

qgis · Apr 12, 2018 · 7e022f0 · 7e022f0
1 parent 42517f8
commit 7e022f0
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 8 deletions.
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -9320,16 +9320,14 @@ 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...
   const QSet<QgsMapLayer *> requiredLayers = QgsProject::instance()->requiredLayers();
   for ( QgsMapLayer *layer : selectedLayers )
   {
     if ( requiredLayers.contains( layer ) )
-    {
-      QMessageBox::warning( this, tr( "Required layers" ),
-                            tr( "The layer '%1' is marked as a required and therefore it cannot be removed from the project.\n\n"
-                                "If you really need to remove the layer, unmark it as required in the Project Properties window > Data Sources tab." ).arg( layer->name() ) );
       return;
-    }
   }
 
   if ( !activeTaskDescriptions.isEmpty() )
@@ -11587,7 +11585,7 @@ void QgisApp::selectionChanged( QgsMapLayer *layer )
 
 void QgisApp::legendLayerSelectionChanged()
 {
-  QList<QgsLayerTreeLayer *> selectedLayers = mLayerTreeView ? mLayerTreeView->selectedLayerNodes() : QList<QgsLayerTreeLayer *>();
+  const QList<QgsLayerTreeLayer *> selectedLayers = mLayerTreeView ? mLayerTreeView->selectedLayerNodes() : QList<QgsLayerTreeLayer *>();
 
   mActionDuplicateLayer->setEnabled( !selectedLayers.isEmpty() );
   mActionSetLayerScaleVisibility->setEnabled( !selectedLayers.isEmpty() );
@@ -11613,6 +11611,19 @@ void QgisApp::legendLayerSelectionChanged()
       mLegendExpressionFilterButton->setChecked( exprEnabled );
     }
   }
+
+  // remove action - check for required layers
+  bool removeEnabled = true;
+  const QSet<QgsMapLayer *> requiredLayers = QgsProject::instance()->requiredLayers();
+  for ( QgsLayerTreeLayer *nodeLayer : selectedLayers )
+  {
+    if ( requiredLayers.contains( nodeLayer->layer() ) )
+    {
+      removeEnabled = false;
+      break;
+    }
+  }
+  mActionRemoveLayer->setEnabled( removeEnabled );
 }
 
 void QgisApp::layerEditStateChanged()

diff --git a/src/app/qgsapplayertreeviewmenuprovider.cpp b/src/app/qgsapplayertreeviewmenuprovider.cpp
@@ -87,7 +87,8 @@ QMenu *QgsAppLayerTreeViewMenuProvider::createContextMenu()
 
       menu->addSeparator();
       menu->addAction( actions->actionAddGroup( menu ) );
-      menu->addAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionRemoveLayer.svg" ) ), tr( "&Remove Group…" ), QgisApp::instance(), SLOT( removeLayer() ) );
+      QAction *removeAction = menu->addAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionRemoveLayer.svg" ) ), tr( "&Remove Group…" ), QgisApp::instance(), SLOT( removeLayer() ) );
+      removeAction->setEnabled( removeActionEnabled() );
       menu->addSeparator();
 
       menu->addAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionSetCRS.png" ) ),
@@ -171,7 +172,8 @@ QMenu *QgsAppLayerTreeViewMenuProvider::createContextMenu()
 
       // duplicate layer
       QAction *duplicateLayersAction = menu->addAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionDuplicateLayer.svg" ) ), tr( "&Duplicate Layer" ), QgisApp::instance(), SLOT( duplicateLayers() ) );
-      menu->addAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionRemoveLayer.svg" ) ), tr( "&Remove Layer…" ), QgisApp::instance(), SLOT( removeLayer() ) );
+      QAction *removeAction = menu->addAction( QgsApplication::getThemeIcon( QStringLiteral( "/mActionRemoveLayer.svg" ) ), tr( "&Remove Layer…" ), QgisApp::instance(), SLOT( removeLayer() ) );
+      removeAction->setEnabled( removeActionEnabled() );
 
       menu->addSeparator();
 
@@ -708,3 +710,15 @@ void QgsAppLayerTreeViewMenuProvider::setSymbolLegendNodeColor( const QColor &co
     layer->emitStyleChanged();
   }
 }
+
+bool QgsAppLayerTreeViewMenuProvider::removeActionEnabled()
+{
+  const QList<QgsLayerTreeLayer *> selectedLayers = mView->selectedLayerNodes();
+  const QSet<QgsMapLayer *> requiredLayers = QgsProject::instance()->requiredLayers();
+  for ( QgsLayerTreeLayer *nodeLayer : selectedLayers )
+  {
+    if ( requiredLayers.contains( nodeLayer->layer() ) )
+      return false;
+  }
+  return true;
+}
diff --git a/src/app/qgsapplayertreeviewmenuprovider.h b/src/app/qgsapplayertreeviewmenuprovider.h
@@ -67,6 +67,9 @@ class QgsAppLayerTreeViewMenuProvider : public QObject, public QgsLayerTreeViewM
     void setVectorSymbolColor( const QColor &color );
     void editSymbolLegendNodeSymbol();
     void setSymbolLegendNodeColor( const QColor &color );
+
+  private:
+    bool removeActionEnabled();
 };
 
 #endif // QGSAPPLAYERTREEVIEWMENUPROVIDER_H
diff --git a/tests/src/core/testqgsproject.cpp b/tests/src/core/testqgsproject.cpp
@@ -40,6 +40,7 @@ class TestQgsProject : public QObject
     void testPathResolverSvg();
     void testProjectUnits();
     void variablesChanged();
+    void testRequiredLayers();
 };
 
 void TestQgsProject::init()
@@ -347,6 +348,40 @@ void TestQgsProject::variablesChanged()
   delete prj;
 }
 
+void TestQgsProject::testRequiredLayers()
+{
+  QString dataDir( TEST_DATA_DIR ); //defined in CmakeLists.txt
+  QString layerPath = dataDir + "/points.shp";
+  QgsVectorLayer *layer1 = new QgsVectorLayer( layerPath, QStringLiteral( "points 1" ), QStringLiteral( "ogr" ) );
+  QgsVectorLayer *layer2 = new QgsVectorLayer( layerPath, QStringLiteral( "points 2" ), QStringLiteral( "ogr" ) );
+
+  QgsProject prj;
+  prj.addMapLayer( layer1 );
+  prj.addMapLayer( layer2 );
+
+  QSet<QgsMapLayer *> reqLayers;
+  reqLayers << layer2;
+  prj.setRequiredLayers( reqLayers );
+
+  QSet<QgsMapLayer *> reqLayersReturned = prj.requiredLayers();
+  QCOMPARE( reqLayersReturned.count(), 1 );
+  QCOMPARE( *reqLayersReturned.constBegin(), layer2 );
+
+  QTemporaryFile f;
+  QVERIFY( f.open() );
+  f.close();
+  prj.setFileName( f.fileName() );
+  prj.write();
+
+  // test reading required layers back
+  QgsProject prj2;
+  prj2.setFileName( f.fileName() );
+  QVERIFY( prj2.read() );
+  QSet<QgsMapLayer *> reqLayersReturned2 = prj2.requiredLayers();
+  QCOMPARE( reqLayersReturned2.count(), 1 );
+  QCOMPARE( ( *reqLayersReturned.constBegin() )->name(), QString( "points 2" ) );
+}
+
 
 QGSTEST_MAIN( TestQgsProject )
 #include "testqgsproject.moc"