Sd outside grp - Githubissues

FLO-2DSoftware / qgis-flo-2d-plugin

A plugin for pre-processing/post-processing FLO-2D models

5 stars 7 forks source link

Sd outside grp #1333

Closed rpachaly closed 1 month ago

FLO-2DJJ commented 1 month ago

I tested the pull request with a shapefile layer outside the project group.

When the project is closed, the outside shapefile is moved to new group External Layers:

There is no problem with the dialog Select components from shapefile layer; it keeps the reference to the moved layer.

I suppose that's OK. I'm just mentioning it because in a previous plugin, all the shapefiles where moved to the External Layers, but in a subsequent plugin, they are keep inside Storm Drain.

But, again, I'm just checking that this pull request does what we want.

rpachaly commented 1 month ago

If the layer is inside the Storm Drain User Group, it will be saved there. If the layer is outside the Storm Drain User Group, it will be saved on the External Layers Group.

FLO-2DJJ commented 1 month ago

If the layer is inside the Storm Drain User Group, it will be saved there. If the layer is outside the Storm Drain User Group, it will be saved on the External Layers Group.

Ok. That's fine. I was just checking. I'll give my approval.

rpachaly commented 1 month ago

JJ, I've added a commit to this pull request addressing https://github.com/FLO-2DSoftware/qgis-flo-2d-plugin/issues/1331

It worked on my testing, but you are more familiar with the code. Can you test it?

Thanks!

FLO-2DJJ commented 1 month ago

JJ, I've added a commit to this pull request addressing #1331

It worked on my testing, but you are more familiar with the code. Can you test it?

Thanks!

The pull request was merged. I'll test it in my local repo.

FLO-2DJJ commented 1 month ago

JJ, I've added a commit to this pull request addressing #1331

It worked on my testing, but you are more familiar with the code. Can you test it?

Thanks!

Hi @rpachaly,

I tested the solution to #1331. It looks good. I saw internal code structure and logic, and ran several cases. It is fine.

Maybe, in the future, we could check that the fields selected from the shapefiles are of the same type of the layer fields. For example, if the user selects a string to assign to a number, an error may immediately occur or cascade for a later error, like:

FLO-2DJJ commented 1 month ago

On second thoughts, I'm going to include an issue for my above comment: #1334