From d92f26d94391d81a91890f10d4d3e849e57fcb7f Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 6 Jun 2024 10:56:17 +0200 Subject: [PATCH 1/3] [forms] Validate value length for value maps Display errors and trim the values. Fix #57634 --- .../editorwidgets/qgsvaluemapconfigdlg.cpp | 74 ++++++++++- src/gui/editorwidgets/qgsvaluemapconfigdlg.h | 8 ++ .../editorwidgets/qgsvaluemapconfigdlgbase.ui | 121 +++++++++++------- 3 files changed, 152 insertions(+), 51 deletions(-) diff --git a/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp b/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp index 879a38f7575a..b16e3ef50c9d 100644 --- a/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp +++ b/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp @@ -33,6 +33,9 @@ QgsValueMapConfigDlg::QgsValueMapConfigDlg( QgsVectorLayer *vl, int fieldIdx, QW { setupUi( this ); + mValueMapErrorsLabel->setVisible( false ); + mValueMapErrorsLabel->setStyleSheet( QStringLiteral( "QLabel { color : red; }" ) ); + tableWidget->insertRow( 0 ); tableWidget->horizontalHeader()->setSectionsClickable( true ); @@ -90,12 +93,13 @@ void QgsValueMapConfigDlg::setConfig( const QVariantMap &config ) } QList valueList = config.value( QStringLiteral( "map" ) ).toList(); + QList> orderedMap; if ( valueList.count() > 0 ) { for ( int i = 0, row = 0; i < valueList.count(); i++, row++ ) { - setRow( row, valueList[i].toMap().constBegin().value().toString(), valueList[i].toMap().constBegin().key() ); + orderedMap.append( qMakePair( valueList[i].toMap().constBegin().key(), valueList[i].toMap().constBegin().value() ) ); } } else @@ -105,11 +109,14 @@ void QgsValueMapConfigDlg::setConfig( const QVariantMap &config ) for ( QVariantMap::ConstIterator mit = values.constBegin(); mit != values.constEnd(); mit++, row++ ) { if ( QgsVariantUtils::isNull( mit.value() ) ) - setRow( row, mit.key(), QString() ); + orderedMap.append( qMakePair( mit.key(), QVariant() ) ); else - setRow( row, mit.value().toString(), mit.key() ); + orderedMap.append( qMakePair( mit.key(), mit.value() ) ); } } + + updateMap( orderedMap, false ); + } void QgsValueMapConfigDlg::vCellChanged( int row, int column ) @@ -120,6 +127,21 @@ void QgsValueMapConfigDlg::vCellChanged( int row, int column ) tableWidget->insertRow( row + 1 ); } //else check type + // check cell value + QTableWidgetItem *item = tableWidget->item( row, 0 ); + if ( item ) + { + const QString validValue = checkValueLength( item->text() ); + if ( validValue.length() != item->text().length() ) + { + const QString errorMessage = tr( "Value '%1' has been trimmed (maximum field length: %2)" ) + .arg( item->text(), QString::number( layer()->fields().field( field() ).length() ) ); + item->setText( validValue ); + mValueMapErrorsLabel->setVisible( true ); + mValueMapErrorsLabel->setText( QStringLiteral( "%1
%2" ).arg( errorMessage, mValueMapErrorsLabel->text() ) ); + } + } + emit changed(); } @@ -163,6 +185,8 @@ void QgsValueMapConfigDlg::updateMap( const QMap &map, bool i void QgsValueMapConfigDlg::updateMap( const QList> &list, bool insertNull ) { tableWidget->clearContents(); + mValueMapErrorsLabel->setVisible( false ); + for ( int i = tableWidget->rowCount() - 1; i > 0; i-- ) { tableWidget->removeRow( i ); @@ -175,16 +199,58 @@ void QgsValueMapConfigDlg::updateMap( const QList> &lis ++row; } + constexpr int maxOverflowErrors { 5 }; + QStringList reportedErrors; + const QgsField mappedField { layer()->fields().field( field() ) }; + for ( const auto &pair : list ) { if ( QgsVariantUtils::isNull( pair.second ) ) setRow( row, pair.first, QString() ); else - setRow( row, pair.first, pair.second.toString() ); + { + const QString value { pair.first }; + // Check value + const QString validValue = checkValueLength( value ); + + if ( validValue.length() != value.length() ) + { + + if ( reportedErrors.length() < maxOverflowErrors ) + { + reportedErrors.push_back( tr( "Value '%1' has been trimmed (maximum field length: %2)" ) + .arg( value, QString::number( mappedField.length() ) ) ); + } + else if ( reportedErrors.length() == maxOverflowErrors ) + { + reportedErrors.push_back( tr( "Only first %1 errors have been reported." ) + .arg( maxOverflowErrors ) ); + } + } + + setRow( row, validValue, pair.second.toString() ); + + // Show errors if any + if ( !reportedErrors.isEmpty() ) + { + mValueMapErrorsLabel->setVisible( true ); + mValueMapErrorsLabel->setText( reportedErrors.join( QStringLiteral( "
" ) ) ); + } + } ++row; } } +QString QgsValueMapConfigDlg::checkValueLength( const QString &value ) +{ + const QgsField mappedField { layer()->fields().field( field() ) }; + if ( mappedField.length() > 0 && value.length() > mappedField.length() ) + { + return value.mid( 0, mappedField.length() ); + } + return value; +} + void QgsValueMapConfigDlg::populateComboBox( QComboBox *comboBox, const QVariantMap &config, bool skipNull ) { const QList valueList = config.value( QStringLiteral( "map" ) ).toList(); diff --git a/src/gui/editorwidgets/qgsvaluemapconfigdlg.h b/src/gui/editorwidgets/qgsvaluemapconfigdlg.h index 947ecca17980..67f9bfe886e2 100644 --- a/src/gui/editorwidgets/qgsvaluemapconfigdlg.h +++ b/src/gui/editorwidgets/qgsvaluemapconfigdlg.h @@ -58,6 +58,14 @@ class GUI_EXPORT QgsValueMapConfigDlg : public QgsEditorConfigWidget, private Ui */ void updateMap( const QList> &list, bool insertNull ); + /** + * Validates a value against the maximum allowed field length and trims it is necessary. + * \param value + * \return the validated field value trimmed if necessary + * \since QGIS 3.38 + */ + QString checkValueLength( const QString &value ); + /** * Updates the displayed table with the values from a CSV file. * \param filePath the absolute file path of the CSV file. diff --git a/src/ui/editorwidgets/qgsvaluemapconfigdlgbase.ui b/src/ui/editorwidgets/qgsvaluemapconfigdlgbase.ui index e2d8c8612d40..caac3d79116b 100644 --- a/src/ui/editorwidgets/qgsvaluemapconfigdlgbase.ui +++ b/src/ui/editorwidgets/qgsvaluemapconfigdlgbase.ui @@ -14,7 +14,7 @@ Form - + Combo box with predefined items. Value is stored in the attribute, description is shown in the combo box. @@ -31,66 +31,93 @@ - - - - Qt::Horizontal - - - - 339 - 20 - + + + + Load Data from CSV File - - - - - - - Value - - - - - Description - - - - + + Qt::Horizontal - 518 + 339 20 - - - - Add "NULL" value - - + + + + + + + Value + + + + + Description + + + + + + + + true + + + + 16777215 + 100 + + + + No errors + + + true + + + + - - - - Remove Selected - - - - - - - Load Data from CSV File - - + + + + + + Add "NULL" value + + + + + + + Remove Selected + + + + + + + Qt::Horizontal + + + + 518 + 20 + + + + + From 9c0cb0f5fe0d25d9ff028fd73e404b86fb45677a Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Thu, 6 Jun 2024 15:53:23 +0200 Subject: [PATCH 2/3] Address PR comments --- .../editorwidgets/qgsvaluemapconfigdlg.cpp | 47 +++++++++++-------- src/gui/editorwidgets/qgsvaluemapconfigdlg.h | 16 +++---- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp b/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp index b16e3ef50c9d..093697eeeae8 100644 --- a/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp +++ b/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp @@ -93,13 +93,13 @@ void QgsValueMapConfigDlg::setConfig( const QVariantMap &config ) } QList valueList = config.value( QStringLiteral( "map" ) ).toList(); - QList> orderedMap; + QList> orderedList; if ( valueList.count() > 0 ) { for ( int i = 0, row = 0; i < valueList.count(); i++, row++ ) { - orderedMap.append( qMakePair( valueList[i].toMap().constBegin().key(), valueList[i].toMap().constBegin().value() ) ); + orderedList.append( qMakePair( valueList[i].toMap().constBegin().key(), valueList[i].toMap().constBegin().value() ) ); } } else @@ -109,13 +109,13 @@ void QgsValueMapConfigDlg::setConfig( const QVariantMap &config ) for ( QVariantMap::ConstIterator mit = values.constBegin(); mit != values.constEnd(); mit++, row++ ) { if ( QgsVariantUtils::isNull( mit.value() ) ) - orderedMap.append( qMakePair( mit.key(), QVariant() ) ); + orderedList.append( qMakePair( mit.key(), QVariant() ) ); else - orderedMap.append( qMakePair( mit.key(), mit.value() ) ); + orderedList.append( qMakePair( mit.key(), mit.value() ) ); } } - updateMap( orderedMap, false ); + updateMap( orderedList, false ); } @@ -127,18 +127,21 @@ void QgsValueMapConfigDlg::vCellChanged( int row, int column ) tableWidget->insertRow( row + 1 ); } //else check type - // check cell value - QTableWidgetItem *item = tableWidget->item( row, 0 ); - if ( item ) + if ( layer()->fields().exists( field() ) ) { - const QString validValue = checkValueLength( item->text() ); - if ( validValue.length() != item->text().length() ) + // check cell value + QTableWidgetItem *item = tableWidget->item( row, 0 ); + if ( item ) { - const QString errorMessage = tr( "Value '%1' has been trimmed (maximum field length: %2)" ) - .arg( item->text(), QString::number( layer()->fields().field( field() ).length() ) ); - item->setText( validValue ); - mValueMapErrorsLabel->setVisible( true ); - mValueMapErrorsLabel->setText( QStringLiteral( "%1
%2" ).arg( errorMessage, mValueMapErrorsLabel->text() ) ); + const QString validValue = checkValueLength( item->text() ); + if ( validValue.length() != item->text().length() ) + { + const QString errorMessage = tr( "Value '%1' has been trimmed (maximum field length: %2)" ) + .arg( item->text(), QString::number( layer()->fields().field( field() ).length() ) ); + item->setText( validValue ); + mValueMapErrorsLabel->setVisible( true ); + mValueMapErrorsLabel->setText( QStringLiteral( "%1
%2" ).arg( errorMessage, mValueMapErrorsLabel->text() ) ); + } } } @@ -201,7 +204,8 @@ void QgsValueMapConfigDlg::updateMap( const QList> &lis constexpr int maxOverflowErrors { 5 }; QStringList reportedErrors; - const QgsField mappedField { layer()->fields().field( field() ) }; + const bool hasField { layer()->fields().exists( field() ) }; + const QgsField mappedField { hasField ? layer()->fields().field( field() ) : QgsField() }; for ( const auto &pair : list ) { @@ -211,7 +215,7 @@ void QgsValueMapConfigDlg::updateMap( const QList> &lis { const QString value { pair.first }; // Check value - const QString validValue = checkValueLength( value ); + const QString validValue = checkValueLength( value ) ; if ( validValue.length() != value.length() ) { @@ -243,10 +247,13 @@ void QgsValueMapConfigDlg::updateMap( const QList> &lis QString QgsValueMapConfigDlg::checkValueLength( const QString &value ) { - const QgsField mappedField { layer()->fields().field( field() ) }; - if ( mappedField.length() > 0 && value.length() > mappedField.length() ) + if ( layer()->fields().exists( field() ) ) { - return value.mid( 0, mappedField.length() ); + const QgsField mappedField { layer()->fields().field( field() ) }; + if ( mappedField.length() > 0 && value.length() > mappedField.length() ) + { + return value.mid( 0, mappedField.length() ); + } } return value; } diff --git a/src/gui/editorwidgets/qgsvaluemapconfigdlg.h b/src/gui/editorwidgets/qgsvaluemapconfigdlg.h index 67f9bfe886e2..2cb7c4289357 100644 --- a/src/gui/editorwidgets/qgsvaluemapconfigdlg.h +++ b/src/gui/editorwidgets/qgsvaluemapconfigdlg.h @@ -58,14 +58,6 @@ class GUI_EXPORT QgsValueMapConfigDlg : public QgsEditorConfigWidget, private Ui */ void updateMap( const QList> &list, bool insertNull ); - /** - * Validates a value against the maximum allowed field length and trims it is necessary. - * \param value - * \return the validated field value trimmed if necessary - * \since QGIS 3.38 - */ - QString checkValueLength( const QString &value ); - /** * Updates the displayed table with the values from a CSV file. * \param filePath the absolute file path of the CSV file. @@ -86,6 +78,14 @@ class GUI_EXPORT QgsValueMapConfigDlg : public QgsEditorConfigWidget, private Ui private: void setRow( int row, const QString &value, const QString &description ); + /** + * Validates a value against the maximum allowed field length and trims it is necessary. + * \param value + * \return the validated field value trimmed if necessary + */ + QString checkValueLength( const QString &value ); + + private slots: void copySelectionToClipboard(); void vCellChanged( int row, int column ); From d4cf6b0f6fd5e9e79920972cdc9b35dacfbf0e5b Mon Sep 17 00:00:00 2001 From: Alessandro Pasotti Date: Fri, 7 Jun 2024 08:13:05 +0200 Subject: [PATCH 3/3] Fix key/value order --- src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp b/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp index 093697eeeae8..5cae47fadafd 100644 --- a/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp +++ b/src/gui/editorwidgets/qgsvaluemapconfigdlg.cpp @@ -99,7 +99,7 @@ void QgsValueMapConfigDlg::setConfig( const QVariantMap &config ) { for ( int i = 0, row = 0; i < valueList.count(); i++, row++ ) { - orderedList.append( qMakePair( valueList[i].toMap().constBegin().key(), valueList[i].toMap().constBegin().value() ) ); + orderedList.append( qMakePair( valueList[i].toMap().constBegin().value().toString(), valueList[i].toMap().constBegin().key() ) ); } } else @@ -111,7 +111,7 @@ void QgsValueMapConfigDlg::setConfig( const QVariantMap &config ) if ( QgsVariantUtils::isNull( mit.value() ) ) orderedList.append( qMakePair( mit.key(), QVariant() ) ); else - orderedList.append( qMakePair( mit.key(), mit.value() ) ); + orderedList.append( qMakePair( mit.value().toString(), mit.key() ) ); } }