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

[FEATURE] [needs-docs] Add circular data dependencies #30947

Merged
merged 6 commits into from Aug 26, 2019

Conversation

@troopa81
Copy link
Contributor

troopa81 commented Jul 26, 2019

Description

This PR is the first step of the proposed GRANT Snap Cache Improvements.

If fixes the QGEP issue described here

It add the possibility to define circular data dependencies (layer1 depends on layer2 that depends on layer 1), and by extension on the layer itself (layer1 depends on layer1). This last is useful for instance when you modify one line in a line layer that trigger connected lines modifications on database side. In this case, dataChanged signal needs to be fired in order to refresh the snapping cache (and avoid what is described in QGEP issue).

This PR doesn't propose any optimisations on snap cache indexing. It will be done in a separate PR.

API BREAK : I remove the QgsMapLayer::hasDependencyCycle method because it has no use anymore. Even if the method was protected, it changes the API. Is it an issue? Is it better to keep it and mark it as deprecated? I remove it for now.

cc @mhugo @lbartoletti @m-kuhn @haubourg

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 contain 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
@troopa81 troopa81 added the API Break! label Jul 26, 2019
@mhugo
mhugo approved these changes Jul 26, 2019
Copy link
Contributor

mhugo left a comment

Good for me. It seems to suppress more code than it adds, even better :)

troopa81 added 3 commits Jul 31, 2019
@troopa81 troopa81 removed the API Break! label Jul 31, 2019
@stale

This comment has been minimized.

Copy link

stale bot commented Aug 15, 2019

The QGIS project highly values your contribution and would love to see this work merged! Unfortunately this PR has not had any activity in the last 14 days and is being automatically marked as "stale". If you think this pull request should be merged, please check

  • that all unit tests are passing

  • that all comments by reviewers have been addressed

  • that there is enough information for reviewers, in particular

    • link to any issues which this pull request fixes

    • add a description of workflows which this pull request fixes

    • add screenshots if applicable

  • that you have written unit tests where possible
    In case you should have any uncertainty, please leave a comment and we will be happy to help you proceed with this pull request.
    If there is no further activity on this pull request, it will be closed in a week.

@stale stale bot added the stale label Aug 15, 2019
@troopa81

This comment has been minimized.

Copy link
Contributor Author

troopa81 commented Aug 16, 2019

Please don't stale!

@stale stale bot removed the stale label Aug 16, 2019
@troopa81 troopa81 mentioned this pull request Aug 23, 2019
4 of 6 tasks complete
@haubourg

This comment has been minimized.

Copy link
Contributor

haubourg commented Aug 25, 2019

just tested in production environment and it works nicely!
fixed_snapping_index_out_of_sync_for_transactiongroup_and_dbtriggers

@nyalldawson or @m-kuhn Can someone merge this please?. I think having this in LTR will be a great relief for many application using database side intelligence :)

Of course this slows the digitizing work, but #31374 should help.

@mhugo mhugo merged commit 5b71e21 into qgis:master Aug 26, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@haubourg

This comment has been minimized.

Copy link
Contributor

haubourg commented Aug 26, 2019

Note for documentation team (note for myself), this feature is barely visible in the UI.
It only adds one line in the layer depency dialog.

@Chaz6

This comment has been minimized.

Copy link

Chaz6 commented Nov 8, 2019

Thanks for this improvement! I would really like to read a blog post about using this feature and its applications (i.e. what kind of editing tasks with some simple sql examples).

@troopa81

This comment has been minimized.

Copy link
Contributor Author

troopa81 commented Jan 10, 2020

@Chaz6 Here is a blog post where we talk (among other things) about data depedencies

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.