add missing CRS to project load errors (#2813)

* add missing CRS to project load errors * fix std::bad_function in tests
MerginMaps · Oct 5, 2023 · cb4af91 · cb4af91
1 parent 31608a1
commit cb4af91
Show file tree

Hide file tree

Showing 9 changed files with 128 additions and 69 deletions.
diff --git a/app/activeproject.cpp b/app/activeproject.cpp
@@ -194,30 +194,15 @@ bool ActiveProject::forceLoad( const QString &filePath, bool force )
     emit positionTrackingSupportedChanged();
   }
 
-  bool foundInvalidLayer = false;
-  QStringList invalidLayers;
-  QMap<QString, QgsMapLayer *> projectLayers = mQgsProject->mapLayers();
-
-  for ( QgsMapLayer *layer : projectLayers )
-  {
-    if ( !layer->isValid() )
-    {
-      invalidLayers.append( layer->name() );
-      foundInvalidLayer = true;
-      emit reportIssue( layer->name(), layer->publicSource() );
-    }
-  }
+  bool foundErrorsInLoadedProject = validateProject();
 
   flagFile.remove();
   if ( !force )
   {
     emit loadingFinished();
 
-    if ( foundInvalidLayer )
+    if ( foundErrorsInLoadedProject )
     {
-      QString message = QStringLiteral( "WARNING: The following layers are invalid: %1" ).arg( invalidLayers.join( ", " ) );
-      CoreUtils::log( "project loading", message );
-
       QFile file( logFilePath );
       if ( file.open( QIODevice::ReadOnly ) )
       {
@@ -263,6 +248,47 @@ bool ActiveProject::forceLoad( const QString &filePath, bool force )
   return res;
 }
 
+bool ActiveProject::validateProject()
+{
+  Q_ASSERT( mQgsProject );
+
+  bool errorsFound = false;
+
+  // A. Per project validations
+  // A.1. Project CRS
+  if ( !mQgsProject->crs().isValid() )
+  {
+    errorsFound = true;
+    CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid canvas CRS" ) );
+    emit reportIssue( tr( "General" ), tr( "Project has invalid CRS assigned. Map and tools have undefined behaviour!" ) );
+  }
+
+  // B. Per-Layer validations
+  QMap<QString, QgsMapLayer *> projectLayers = mQgsProject->mapLayers();
+  for ( QgsMapLayer *layer : projectLayers )
+  {
+    // B.1. Layer Validity
+    if ( !layer->isValid() )
+    {
+      errorsFound = true;
+      CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid layer %1" ).arg( layer->name() ) );
+      emit reportIssue( tr( "Layer" ) + ": " + layer->name(), tr( "Unable to load source " ) + ": " + layer->publicSource() );
+    }
+    else
+    {
+      // B.2. Layer CRS
+      if ( layer->isSpatial() && !layer->crs().isValid() )
+      {
+        errorsFound = true;
+        CoreUtils::log( QStringLiteral( "Project load" ), QStringLiteral( "Invalid layer CRS %1" ).arg( layer->name() ) );
+        emit reportIssue( tr( "Layer" ) + ": " + layer->name(), tr( "Layer has invalid CRS assigned. Recording tools have undefined behaviour." ) );
+      }
+    }
+  }
+
+  return errorsFound;
+}
+
 bool ActiveProject::reloadProject( QString projectDir )
 {
   if ( mQgsProject->homePath() == projectDir )

diff --git a/app/activeproject.h b/app/activeproject.h
@@ -128,7 +128,7 @@ class ActiveProject: public QObject
     void loadingFinished();
 
     void projectReadingFailed( QString error );
-    void reportIssue( QString layerName, QString message );
+    void reportIssue( QString title, QString message );
     void loadingErrorFound();
     void qgisLogChanged();
 
@@ -155,6 +155,13 @@ class ActiveProject: public QObject
 
   private:
 
+    /**
+     * Build up warning list from loaded project
+     * Emits reportIssue for each issue found
+     * Returns true if there were errors found
+     */
+    bool validateProject();
+
     //! Tries to match current visible layers with some theme and if it fails, invalidates current map theme
     void updateMapTheme();
 

diff --git a/app/qml/ProjectIssuesPanel.qml b/app/qml/ProjectIssuesPanel.qml
@@ -1,3 +1,4 @@
+
 /***************************************************************************
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
@@ -21,17 +22,16 @@ Item {
   property real rowHeight: InputStyle.rowHeight
   property var projectIssuesModel: ListModel {}
   property string projectLoadingLog: ""
-  property string headerText: qsTr( "The following layers failed loading" ) + ":"
 
-  function reportIssue( layerName, message ) {
-    projectIssuesModel.append( { name: layerName, message: message } );
+  function reportIssue(title, message) {
+    projectIssuesModel.append( { title: title, message: message } )
   }
 
   function clear() {
-    projectIssuesModel.clear();
+    projectIssuesModel.clear()
   }
 
-  Keys.onReleased: function( event ) {
+  Keys.onReleased: function (event) {
     if (event.key === Qt.Key_Back || event.key === Qt.Key_Escape) {
       event.accepted = true
 
@@ -80,22 +80,11 @@ Item {
         contentWidth: availableWidth // to only scroll vertically
         spacing: InputStyle.panelSpacing
 
-        background: Rectangle {
-          anchors.fill: parent
-          color: InputStyle.panelBackgroundLight
-        }
-
         Column {
           id: settingListContent
           anchors.fill: parent
           spacing: 1
 
-          PanelItem {
-            color: InputStyle.panelBackgroundLight
-            text: headerText
-            bold: true
-          }
-
           PanelItem {
             id: invalidLayersList
             height: 0
@@ -108,37 +97,32 @@ Item {
               spacing: 3
               delegate: PanelItem {
                 anchors.margins: 5
-                width:  ListView.view.width
+                width: ListView.view.width
                 height: row.height
                 color: InputStyle.clrPanelMain
-                Row {
+                Column {
                   id: row
-                   width: parent.width
-                   anchors.left: parent.left
-                   anchors.top: parent.top
-                    Text {
-                      id: nameTextItem
-                      padding: 5
-                      font.pixelSize: InputStyle.fontPixelSizeBig
-                      text:  qsTr( name )
-                      wrapMode: Text.Wrap
-                    }
-
-                    Rectangle {
-                        id: seperator
-                        width: 3
-                        height: parent.height
-                        color: "gray"
-                    }
-
-                    Text {
-                      id: messageTextItem
-                      width: parent.width - nameTextItem.width - seperator.width
-                      padding: 5
-                      font.pixelSize: InputStyle.fontPixelSizeBig
-                      text:  qsTr( message )
-                      wrapMode: Text.Wrap
-                    }
+                  width: parent.width
+                  anchors.left: parent.left
+                  anchors.top: parent.top
+                  Text {
+                    id: nameTextItem
+                    width: parent.width
+                    padding: 5
+                    font.pixelSize: InputStyle.fontPixelSizeBig
+                    text: title
+                    color: InputStyle.fontColor
+                    wrapMode: Text.Wrap
+                  }
+
+                  Text {
+                    id: messageTextItem
+                    width: parent.width
+                    padding: 10
+                    font.pixelSize: InputStyle.fontPixelSizeNormal
+                    text: message
+                    wrapMode: Text.Wrap
+                  }
                 }
                 onHeightChanged: invalidLayersList.height += height;
               }
@@ -150,12 +134,18 @@ Item {
             }
           }
 
-
           // Debug/Logging
           PanelItem {
-            color: InputStyle.panelBackgroundLight
-            text: qsTr("QGIS log")
-            bold: true
+            height: qgisLogTextHeader.height
+            width: parent.width
+            Text {
+              id: qgisLogTextHeader
+              width: parent.width
+              padding: 5
+              text: qsTr("QGIS log")
+              font.pixelSize: InputStyle.fontPixelSizeBig
+              color: InputStyle.fontColor
+            }
           }
 
           PanelItem {
@@ -164,7 +154,7 @@ Item {
             Text {
               id: qgisLogTextItem
               width: parent.width
-              padding: 5
+              padding: 10
               text: projectLoadingLog
               wrapMode: Text.Wrap
             }

diff --git a/app/qml/main.qml b/app/qml/main.qml
@@ -806,8 +806,8 @@ ApplicationWindow {
         failedToLoadProjectBanner.pushNotificationMessage( qsTr( "There were issues loading the project." ) )
       }
 
-      function onReportIssue( layerName, message ) {
-        projectIssuesPanel.reportIssue( layerName, message )
+      function onReportIssue( title, message ) {
+        projectIssuesPanel.reportIssue( title, message )
       }
 
       function onProjectReloaded( project ) {

diff --git a/app/test/testactiveproject.cpp b/app/test/testactiveproject.cpp
@@ -30,6 +30,30 @@ void TestActiveProject::cleanup()
 {
 }
 
+void TestActiveProject::testProjectValidations()
+{
+  QString projectDir = TestUtils::testDataDir() + "/project-with-missing-layer-and-invalid-crs";
+  QString projectFilename = "bad_layer.qgz";
+
+  AppSettings as;
+  ActiveLayer al;
+  LayersModel lm;
+  LayersProxyModel lpm( &lm, LayerModelTypes::ActiveLayerSelection );
+  ActiveProject activeProject( as, al, lpm, mApi->localProjectsManager() );
+
+  QSignalSpy spyReportIssues( &activeProject, &ActiveProject::reportIssue );
+  QSignalSpy spyErrorsFound( &activeProject, &ActiveProject::loadingErrorFound );
+
+  mApi->localProjectsManager().addLocalProject( projectDir, projectFilename );
+  QVERIFY( activeProject.load( projectDir + "/" + projectFilename ) );
+
+  QCOMPARE( spyErrorsFound.count(), 1 );
+  QCOMPARE( spyReportIssues.count(), 3 ); // invalid project CRS, invalid layer CRS, missing layer Survey
+
+  const QString id = mApi->localProjectsManager().projectId( projectDir + "/" + projectFilename );
+  mApi->localProjectsManager().removeLocalProject( id );
+}
+
 void TestActiveProject::testProjectLoadFailure()
 {
   QString projectname = QStringLiteral( "testProjectLoadFailure" );
@@ -50,6 +74,9 @@ void TestActiveProject::testProjectLoadFailure()
   QVERIFY( !activeProject.load( projectdir + "/" + projectfilename ) );
   QVERIFY( !activeProject.localProject().isValid() );
   QVERIFY( spy.count() );
+
+  const QString id = mApi->localProjectsManager().projectId( projectdir + "/" + projectdir );
+  mApi->localProjectsManager().removeLocalProject( id );
 }
 
 void TestActiveProject::testPositionTrackingFlag()
@@ -76,6 +103,9 @@ void TestActiveProject::testPositionTrackingFlag()
   QCOMPARE( spy.count(), 1 );
   QCOMPARE( activeProject.positionTrackingSupported(), false );
 
+  QString id = mApi->localProjectsManager().projectId( projectDir + "/" + projectName );
+  mApi->localProjectsManager().removeLocalProject( id );
+
   // project "tracking" - tracking enabled
   projectDir = TestUtils::testDataDir() + "/tracking/";
   projectName = "tracking-project.qgz";
@@ -86,4 +116,7 @@ void TestActiveProject::testPositionTrackingFlag()
 
   QCOMPARE( spy.count(), 2 );
   QCOMPARE( activeProject.positionTrackingSupported(), true );
+
+  id = mApi->localProjectsManager().projectId( projectDir + "/" + projectName );
+  mApi->localProjectsManager().removeLocalProject( id );
 }
diff --git a/app/test/testactiveproject.h b/app/test/testactiveproject.h
@@ -24,6 +24,7 @@ class TestActiveProject : public QObject
     void init();
     void cleanup();
 
+    void testProjectValidations();
     void testProjectLoadFailure();
     void testPositionTrackingFlag();
 

diff --git a/test/test_data/project-with-missing-layer-and-invalid-crs/Survey_points_no_crs.gpkg b/test/test_data/project-with-missing-layer-and-invalid-crs/Survey_points_no_crs.gpkg
diff --git a/test/test_data/project-with-missing-layer-and-invalid-crs/bad_layer.qgz b/test/test_data/project-with-missing-layer-and-invalid-crs/bad_layer.qgz
diff --git a/test/test_data/project-with-missing-layer-and-invalid-crs/data.csv b/test/test_data/project-with-missing-layer-and-invalid-crs/data.csv
@@ -0,0 +1,2 @@
+test1,test1val
+test2,test2val