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

fix pasting features into vector layer from clipboard (fix #21154) #9065

Merged
merged 1 commit into from Feb 7, 2019
Merged

fix pasting features into vector layer from clipboard (fix #21154) #9065

merged 1 commit into from Feb 7, 2019

Conversation

alexbruy
Copy link
Contributor

@alexbruy alexbruy commented Feb 1, 2019

Description

There are two layers (in my tests this is 1 shapefile and 1 geopackage). They have different tables, but the shapefile table is just a subset of the GPKG one meaning that the shape has columns with the same name and type of the GPKG. Copy/pasting from the SHP to GPKG results in only a few attributes to be copied.

Looks like this happens because QgsVectorLayerUtils::matchAttributesToFields() updates feature attributes while fields stay untouched. Later untouched fields are used to create attribute map for pasted feature leading to the data loss.

Not sure that this is proper fix.

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

@gioman
Copy link
Contributor

gioman commented Feb 1, 2019

@alexbruy this is possibly an unreported issue (possibly even a regression), anyway I confirm the bug and will test the patch asap.

@gioman
Copy link
Contributor

gioman commented Feb 2, 2019

This will fix the now reported issue https://issues.qgis.org/issues/21154

@alexbruy alexbruy changed the title fix pasting features into vector layer from clipboard fix pasting features into vector layer from clipboard (fix #21154) Feb 3, 2019
@m-kuhn
Copy link
Member

m-kuhn commented Feb 3, 2019

@alexbruy thanks for tackling this, I think I've run into this too in the past.
Sidenote: the fix #xyz comment will need to go to a commit message (git commit --am && git push -f)

@gioman
Copy link
Contributor

gioman commented Feb 5, 2019

@alexbruy thanks for tackling this, I think I've run into this too in the past.

Hi all, can anyone review this? To me seems to work ok.

@m-kuhn
Copy link
Member

m-kuhn commented Feb 6, 2019

Looking at this now...

@gioman
Copy link
Contributor

gioman commented Feb 6, 2019

Looking at this now...

@m-kuhn thanks!

@gioman
Copy link
Contributor

gioman commented Feb 7, 2019

Is there any problem holding this from merging?

@luipir
Copy link
Contributor

luipir commented Feb 7, 2019

Is there any problem holding this from merging?

IMHO it's up to @alexbruy to merge. review has been already done

@m-kuhn
Copy link
Member

m-kuhn commented Feb 7, 2019

As said, I'll have a look at it soon, I'm working on this area currently. I'd be happy if you could hold back till the end of the week.

@gioman
Copy link
Contributor

gioman commented Feb 7, 2019

ah ok, sorry, I just wanted to have this merged sooner than later to allow a wider testing on master (I know several users affected by this bug).

@m-kuhn m-kuhn merged commit 98ec323 into qgis:master Feb 7, 2019
@m-kuhn
Copy link
Member

m-kuhn commented Feb 7, 2019

ok, let's merge it for testing, I can also proceed with the updated codebase ;)

@gioman
Copy link
Contributor

gioman commented Feb 18, 2019

Hi there,
I just noticed this https://issues.qgis.org/issues/21304 and as it affects only master I was guessing if this could possibly be related. I guess I can compile 3.4 with this patch and check... I'll let you know.

@m-kuhn
Copy link
Member

m-kuhn commented Feb 18, 2019

That's the issue which I was investigating and already existed in master before this patch. Are you sure this works in 3.4?

@gioman
Copy link
Contributor

gioman commented Feb 18, 2019

That's the issue which I was investigating and already existed in master before this patch

ok, so is unrelated

Are you sure this works in 3.4?

yes, just tested (3.4.4 from osgeo4w) and linux.

@luipir
Copy link
Contributor

luipir commented Feb 18, 2019

Hi there,
I just noticed this https://issues.qgis.org/issues/21304 and as it affects only master I was guessing if this could possibly be related. I guess I can compile 3.4 with this patch and check... I'll let you know.

I do not think is related... the differente between feature.fields() and feature.attributes() is that the first is static the second not, but has been already parsed from clipboard

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants