Fixed silent crash and filter input behaviour for numbers#4457
Fixed silent crash and filter input behaviour for numbers#4457gabriel-bolbotina wants to merge 2 commits intodev/filteringfrom
Conversation
Added logging for these scenarios Fixed checked status for filter inputs added guards for other potential silent crashes added support for negative numbers and 0 as inputs for number range filters
Coverage Report for CI Build 24560858057Warning No base build found for commit Coverage: 58.114%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsRequires a base build to compare against. How to fix this → Coverage Stats
💛 - Coveralls |
📦 Build Artifacts Ready
|
Withalion
left a comment
Withalion
left a comment
There was a problem hiding this comment.
In general I would prefer triggering the logging when the filter tries to get applied and fails instead of when the filter is loaded from QgsProject
| if ( !vectorLayer ) | ||
| { | ||
| CoreUtils::log( QStringLiteral( "Feature Filtering" ), | ||
| QStringLiteral( "Layer '%1' is not available. Cannot clear its filters." ).arg( layerId ) ); | ||
| return; | ||
| } |
There was a problem hiding this comment.
this doesn't seem necessary as this function gets the layerID passed from the selected layer by user and not from filter
| const QgsVectorLayer *filterLayer = qobject_cast<QgsVectorLayer *>( QgsProject::instance()->mapLayer( newFieldFilter.layerId ) ); | ||
| if ( !filterLayer ) | ||
| { | ||
| CoreUtils::log( QStringLiteral( "Feature Filtering" ), | ||
| QStringLiteral( "Layer '%1' for filter '%2' is not available in the project. Skipping filter." ) | ||
| .arg( newFieldFilter.layerId, newFieldFilter.filterName ) ); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
I wouldn't do the check here, but when creating the filter expression
There was a problem hiding this comment.
Also, QgsProject is passed to this function
| if ( !layer ) | ||
| { | ||
| CoreUtils::log( QStringLiteral( "Feature Filtering" ), | ||
| QStringLiteral( "Layer '%1' is not available. Cannot check active filter." ).arg( layerId ) ); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
This function again gets passed layerID from layer which needs to exist or the user wouldn't be able to navigate to where this function gets triggered
2 participants
Uh oh!
There was an error while loading. Please reload this page.