felt / qgis-plugin

20 stars 5 forks source link

Create layer groups when uploading maps #69

Closed nyalldawson closed 1 month ago

nyalldawson commented 1 month ago

@ChrisLoer

There's something I can't explain happening here! The first group creation and layer assignment works ok, but if I have multiple groups, then some of these groups just disappear when the processing finishes. Here's a screencast showing this occuring. At frist, the group is shown with the "processing" icon, but as soon as the processing finishes the group disappears and the layer is placed in no group (and its name is reset). Is this a backend issue?

https://github.com/felt/qgis-plugin/assets/1829991/05497f6b-44c1-43ec-b54c-e6a7303447b3

github-actions[bot] commented 1 month ago

Plugin ready!

A test version of this PR is available for testing here.

(Built from commit d2bdc8393287f40fa405025f8220307d20cc47b7)

ChrisLoer commented 1 month ago

Yeah, I think this is a backend issue, and I have an idea what it might be, but I'll have to investigate and get back to you.

ChrisLoer commented 1 month ago

Just to confirm, are you only seeing it when you're creating a group with a single item? We have some oddities where under the hood a single "lone layer" is actually a layer within an invisible layer group. I think what's happening is that when the upload completes, the backend is seeing that there's a single layer in the group and it's hiding it. We should be able to make a change to avoid doing that in this case, but I'll need to check in with the guy who implemented the logic.

As a workaround, it might be possible to avoid creating groups with only one layer? (Although I know that's kind of odd logic)

ChrisLoer commented 1 month ago

Just to keep you updated -- yes, this is an issue with single layer groups, so that's a potential workaround. I have a fix on the Felt side but I'm still working on getting the naming logic right in the different cases (e.g. "single layer name provided by API but upload has multiple datasets, and upload got moved into a different group before it finished"). So I'm not sure if I'll have a PR in by tomorrow, may be more like Monday.

nyalldawson commented 1 month ago

@ChrisLoer I just tested this morning, and things are working much better now, thanks! Something you've fixed on the backend has improved things here.

ChrisLoer commented 1 month ago

Yup! From email, it was three things (well really two that were relevant to this use case):

OK, so I ended up finding and fixing three layer-group related bugs (thanks for exposing them!), although you were only running into two of them:

When you move a layer into another group after uploading it, we no longer:
- make the group disappear when upload finishes
- rename the upload to the default name from the pipeline
- (in the case where the upload has multiple datasets, shouldn't happen with QGIS) duplicate all the layers except for the first layer (this was a surprising and gnarly one!)

Upshot is your layer grouping PR should work much better now (I haven't tested myself yet, but it shouldn't need any changes).