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

Refresh virtual fields editing button state based on field selection #5809

Merged
merged 1 commit into from
Dec 12, 2017

Conversation

strk
Copy link
Contributor

@strk strk commented Dec 5, 2017

Allows removing virtual fields defined on read-only PostgreSQL tables.
Closes qgis#17593
@strk strk requested a review from m-kuhn December 5, 2017 17:30
@strk strk self-assigned this Dec 5, 2017
@strk strk added Bugfix Requires Tests! Waiting on the submitter to add unit tests before eligible for merging labels Dec 5, 2017
@strk strk added this to the QGIS 3 milestone Dec 5, 2017
@strk
Copy link
Contributor Author

strk commented Dec 5, 2017

@m-kuhn is there any support or example of testing signals as needed for this case ?
(I'm mentioning you because @haubourg said you're the leader of "Forms and Fields Redesign" task, which seems to be at the origin of this regression)

@m-kuhn
Copy link
Member

m-kuhn commented Dec 5, 2017

QSignalSpy would be the tool to use I guess

@nyalldawson nyalldawson changed the title Include a few sentences describing the overall goals for this PR (pull request). If applicable also add screenshots. Refresh virtual fields editing button state based on field selection Dec 5, 2017
@m-kuhn
Copy link
Member

m-kuhn commented Dec 6, 2017

Forwarding review request to @signedav

@signedav
Copy link
Contributor

signedav commented Dec 6, 2017

Looks good. I reproduced and tested it with success.

@strk strk removed the For Review label Dec 12, 2017
@strk strk merged commit f5cd856 into qgis:master Dec 12, 2017
@strk strk deleted the bugfix/17593-delete-virtual-fields branch December 12, 2017 14:59
@strk
Copy link
Contributor Author

strk commented Dec 12, 2017

I gave up on automating a test for this, too much involved

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Requires Tests! Waiting on the submitter to add unit tests before eligible for merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants