When auto selecting the default identifier field for a layer,

prefer something like "admin_name" over "type_name". By penalising results with "type", "class", "cat" in their names we are less likely to accidentally select a category field as the friendly identifier when a better one exists. Also add tests for this logic.
qgis · Feb 6, 2021 · d0882d0 · d0882d0
1 parent 590b7f4
commit d0882d0
Show file tree

Hide file tree

Showing 5 changed files with 172 additions and 37 deletions.
diff --git a/python/core/auto_generated/vector/qgsvectorlayerutils.sip.in b/python/core/auto_generated/vector/qgsvectorlayerutils.sip.in
@@ -321,6 +321,19 @@ Optionally, ``sinkFlags`` can be specified to further refine the compatibility l
          Details about cascading effects will be written to ``context``.
 
 .. versionadded:: 3.14
+%End
+
+    static QString guessFriendlyIdentifierField( const QgsFields &fields );
+%Docstring
+Given a set of ``fields``, attempts to pick the "most useful" field
+for user-friendly identification of features.
+
+For instance, if a field called "name" is present, this will be returned.
+
+Assumes that the user has organized the data with the more "interesting" field
+names first. As such, "name" would be selected before "oldname", "othername", etc.
+
+.. versionadded:: 3.18
 %End
 
 };

diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp
@@ -101,6 +101,7 @@
 #include "qgsexpressioncontextutils.h"
 #include "qgsruntimeprofiler.h"
 #include "qgsfeaturerenderergenerator.h"
+#include "qgsvectorlayerutils.h"
 
 #include "diagram/qgsdiagram.h"
 
@@ -3602,46 +3603,14 @@ QString QgsVectorLayer::displayExpression() const
   }
   else
   {
-    QString idxName;
-
-    // Check the fields and keep the first one that matches.
-    // We assume that the user has organized the data with the
-    // more "interesting" field names first. As such, name should
-    // be selected before oldname, othername, etc.
-    // This candidates list is a prioritized list of candidates ranked by "interestingness"!
-    // See discussion at https://github.com/qgis/QGIS/pull/30245 - this list must NOT be translated,
-    // but adding hardcoded localized variants of the strings is encouraged.
-    static QStringList sCandidates{ QStringLiteral( "name" ),
-                                    QStringLiteral( "title" ),
-                                    QStringLiteral( "heibt" ),
-                                    QStringLiteral( "desc" ),
-                                    QStringLiteral( "nom" ),
-                                    QStringLiteral( "street" ),
-                                    QStringLiteral( "road" ),
-                                    QStringLiteral( "id" )};
-    for ( const QString &candidate : sCandidates )
+    const QString candidateName = QgsVectorLayerUtils::guessFriendlyIdentifierField( mFields );
+    if ( !candidateName.isEmpty() )
     {
-      for ( const QgsField &field : qgis::as_const( mFields ) )
-      {
-        QString fldName = field.name();
-        if ( fldName.indexOf( candidate, 0, Qt::CaseInsensitive ) > -1 )
-        {
-          idxName = fldName;
-          break;
-        }
-      }
-
-      if ( !idxName.isEmpty() )
-        break;
-    }
-
-    if ( !idxName.isNull() )
-    {
-      return QgsExpression::quotedColumnRef( idxName );
+      return QgsExpression::quotedColumnRef( candidateName );
     }
     else
     {
-      return QgsExpression::quotedColumnRef( mFields.at( 0 ).name() );
+      return QString();
     }
   }
 }

diff --git a/src/core/vector/qgsvectorlayerutils.cpp b/src/core/vector/qgsvectorlayerutils.cpp
@@ -1077,5 +1077,93 @@ bool QgsVectorLayerUtils::impactsCascadeFeatures( const QgsVectorLayer *layer, c
     }
   }
 
-  return context.layers().count();
+  return !context.layers().isEmpty();
+}
+
+QString QgsVectorLayerUtils::guessFriendlyIdentifierField( const QgsFields &fields )
+{
+  if ( fields.isEmpty() )
+    return QString();
+
+  // Check the fields and keep the first one that matches.
+  // We assume that the user has organized the data with the
+  // more "interesting" field names first. As such, name should
+  // be selected before oldname, othername, etc.
+  // This candidates list is a prioritized list of candidates ranked by "interestingness"!
+  // See discussion at https://github.com/qgis/QGIS/pull/30245 - this list must NOT be translated,
+  // but adding hardcoded localized variants of the strings is encouraged.
+  static QStringList sCandidates{ QStringLiteral( "name" ),
+                                  QStringLiteral( "title" ),
+                                  QStringLiteral( "heibt" ),
+                                  QStringLiteral( "desc" ),
+                                  QStringLiteral( "nom" ),
+                                  QStringLiteral( "street" ),
+                                  QStringLiteral( "road" ) };
+
+  // anti-names
+  // this list of strings indicates parts of field names which make the name "less interesting".
+  // For instance, we'd normally like to default to a field called "name" or "id", but if instead we
+  // find one called "typename" or "typeid", then that's most likely a classification of the feature and not the
+  // best choice to default to
+  static QStringList sAntiCandidates{ QStringLiteral( "type" ),
+                                      QStringLiteral( "class" ),
+                                      QStringLiteral( "cat" )
+                                    };
+
+  QString bestCandidateName;
+  QString bestCandidateNameWithAntiCandidate;
+
+  for ( const QString &candidate : sCandidates )
+  {
+    for ( const QgsField &field : fields )
+    {
+      const QString fldName = field.name();
+      if ( fldName.contains( candidate, Qt::CaseInsensitive ) )
+      {
+        bool isAntiCandidate = false;
+        for ( const QString &antiCandidate : sAntiCandidates )
+        {
+          if ( fldName.contains( antiCandidate, Qt::CaseInsensitive ) )
+          {
+            isAntiCandidate = true;
+            break;
+          }
+        }
+
+        if ( isAntiCandidate )
+        {
+          if ( bestCandidateNameWithAntiCandidate.isEmpty() )
+          {
+            bestCandidateNameWithAntiCandidate = fldName;
+          }
+        }
+        else
+        {
+          bestCandidateName = fldName;
+          break;
+        }
+      }
+    }
+
+    if ( !bestCandidateName.isEmpty() )
+      break;
+  }
+
+  const QString candidateName = bestCandidateName.isEmpty() ? bestCandidateNameWithAntiCandidate : bestCandidateName;
+  if ( !candidateName.isEmpty() )
+  {
+    return candidateName;
+  }
+  else
+  {
+    // no good matches found by name, so scan through and look for the first string field
+    for ( const QgsField &field : fields )
+    {
+      if ( field.type() == QVariant::String )
+        return field.name();
+    }
+
+    // no string fields found - just return first field
+    return fields.at( 0 ).name();
+  }
 }
diff --git a/src/core/vector/qgsvectorlayerutils.h b/src/core/vector/qgsvectorlayerutils.h
@@ -349,6 +349,19 @@ class CORE_EXPORT QgsVectorLayerUtils
      */
     static bool impactsCascadeFeatures( const QgsVectorLayer *layer, const QgsFeatureIds &fids, const QgsProject *project, QgsDuplicateFeatureContext &context SIP_OUT, QgsVectorLayerUtils::CascadedFeatureFlags flags = QgsVectorLayerUtils::CascadedFeatureFlags() );
 
+    /**
+     * Given a set of \a fields, attempts to pick the "most useful" field
+     * for user-friendly identification of features.
+     *
+     * For instance, if a field called "name" is present, this will be returned.
+     *
+     * Assumes that the user has organized the data with the more "interesting" field
+     * names first. As such, "name" would be selected before "oldname", "othername", etc.
+     *
+     * \since QGIS 3.18
+     */
+    static QString guessFriendlyIdentifierField( const QgsFields &fields );
+
 };
 
 

diff --git a/tests/src/python/test_qgsvectorlayerutils.py b/tests/src/python/test_qgsvectorlayerutils.py
@@ -690,6 +690,58 @@ def test_unique_pk_when_subset(self):
         vl.addFeatures(features)
         self.assertTrue(vl.commitChanges())
 
+    def testGuessFriendlyIdentifierField(self):
+        """
+        Test guessing a user friendly identifier field
+        """
+        fields = QgsFields()
+        self.assertFalse(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields))
+
+        fields.append(QgsField('id', QVariant.Int))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'id')
+
+        fields.append(QgsField('name', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'name')
+
+        fields.append(QgsField('title', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'name')
+
+        # regardless of actual field order, we prefer "name" over "title"
+        fields = QgsFields()
+        fields.append(QgsField('title', QVariant.String))
+        fields.append(QgsField('name', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'name')
+
+        # test with an "anti candidate", which is a substring which makes a field containing "name" less preferred...
+        fields = QgsFields()
+        fields.append(QgsField('id', QVariant.Int))
+        fields.append(QgsField('typename', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'typename')
+        fields.append(QgsField('title', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'title')
+
+        fields = QgsFields()
+        fields.append(QgsField('id', QVariant.Int))
+        fields.append(QgsField('classname', QVariant.String))
+        fields.append(QgsField('x', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'classname')
+        fields.append(QgsField('desc', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'desc')
+
+        fields = QgsFields()
+        fields.append(QgsField('id', QVariant.Int))
+        fields.append(QgsField('areatypename', QVariant.String))
+        fields.append(QgsField('areaadminname', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'areaadminname')
+
+        # if no good matches by name found, the first string field should be used
+        fields = QgsFields()
+        fields.append(QgsField('id', QVariant.Int))
+        fields.append(QgsField('date', QVariant.Date))
+        fields.append(QgsField('station', QVariant.String))
+        fields.append(QgsField('org', QVariant.String))
+        self.assertEqual(QgsVectorLayerUtils.guessFriendlyIdentifierField(fields), 'station')
+
 
 if __name__ == '__main__':
     unittest.main()