Restore original subsetString() when QgsVectorLayerProperties dialog is canceled (fix #13620) #2480
Conversation
|
Wouldn't it be better to set it only on apply/ok instead of restoring on cancel? |
|
I agree that's what one would expect. But there are some points to consider here:
|
|
Valid points. The first one could easily be solved by adding a new method on The second one is actually a problem with the fact that the field calculator is not really a layer property. So there is probably no "right" solution. The best would probably be to disable the field calculator if the query string has been changed and not applied or in raise a warning and propose to apply before opening the field calculator. The third one (joins) could IMO be changed to the same workflow (copy fields, modify the copy, warn on certain functions as long as copy and original differ). However, if you don't have the time to work on this, the current PR already improves the situation considerably. |
|
OK, then I would leave this PR to be merged as it is and the workflow you suggested to be implemented later with a new PR. |
| @@ -531,6 +532,7 @@ void QgsVectorLayerProperties::apply() | |||
| layer->setSubsetString( txtSubsetSQL->toPlainText() ); | |||
| mMetadataFilled = false; | |||
| } | |||
| mOriginalSubsetSQL = txtSubsetSQL->toPlainText(); | |||
There was a problem hiding this comment.
I think it would be better to set it to layer->subsetString() instead of this, it's possible that the providers trim whitespaces or similar magic.
m-kuhn
added
the
Requires Changes!
|
Merged as 5399855 |
This PR saves the original
subsetString()of the layer and restores it when the user cancels the properties dialog. This is necessary because QgsQueryBuilder::accept() stores the new subset string directly in the layer thereby making it persistent even if the user cancels the QgsVectorLayerProperties dialog.