Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bugfix] Set missing expression context in some filters (fixes #17893) #6118

Merged
merged 1 commit into from
Jan 21, 2018

Conversation

dgoedkoop
Copy link
Contributor

@dgoedkoop dgoedkoop commented Jan 20, 2018

Description

In some filters the expression context was not set, so that references to variables would only work in the preview, but not when actually using them. This PR fixes that.

I have attached a sample project with test data: example.zip. The point layer has two cities ("Amsterdam" and "Zaandam") and two villages. A project variable is set to show only cities.

  • The symbology is configured so that only the points for cities are shown. This works correctly already.
  • The line layer "verbindingen" has two value relation fields connected to the points, with corresponding filter. This does not yet work, this PR fixes this in file src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp
  • The layout "A4" has an atlas connected to the points, with corresponding filter. This does not yet work, this PR fixes this in file src/core/composer/qgsatlascomposition.cpp

The file src/core/layout/qgslayoutatlas.cpp seems very similar to the latter of the above, so I changed it as well. However, I cannot find any actual usage of it. What is the purpose of it?

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include fixes #11111 in the commit message next to the description
  • Commits which add new features are tagged with [FEATURE] in the commit message
  • Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and containt sufficient information in the commit message to be documented
  • I have read the QGIS Coding Standards and this PR complies with them
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit

@@ -234,6 +234,9 @@ int QgsLayoutAtlas::updateFeatures()
req.setFilterExpression( mFilterExpression );
}

QgsExpressionContext context( QgsExpressionContextUtils::globalProjectLayerScopes( mCoverageLayer.get() ) );
req.setExpressionContext( context );
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should use the expression context created a couple of lines above

@nyalldawson
Copy link
Collaborator

Composer is about to be removed (this week). Layout atlas is the new future :)

In some filters the expression context was not set, so that references to
variables would only work in the preview, but not when actually using them.
@dgoedkoop
Copy link
Contributor Author

Ok! I have changed it in both places now, to test with the current composer that it works correctly.

@nyalldawson nyalldawson merged commit fcbf4cb into qgis:master Jan 21, 2018
@dgoedkoop dgoedkoop deleted the filterexprcontext branch January 22, 2018 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants