Hide newly added layer(s) when in group. Fixes #19174

[chau-intl]

When loading a single layer QGIS will set it as invisible if the user have chosen so in the settings (new_layers_visible=false). When a data set contains more than one layer and the user
chooses "Add layers to a group" the layers (and group) are added as visible no matter what.

This commit is fixing that problem for both GDAL and OGR data sets by setting both the group and its layers as invisible/unchecked.

Fixes #19174

[luipir]

Are you able to add unit tests? I can't find test related with adding layers to groups... btw it should only test if the new layer is added in a group with the correct visible configuration. Should be added in this test file: https://github.com/qgis/QGIS/blob/master/tests/src/app/testqgisapp.cpp

[nyalldawson]

@luipir We're setup for unit testing of QgisApp yet. Unless @elpaso's recent work has changed this?

[chau-intl]

Unless @nyalldawson meant "We're not setup for unit testing of QgisApp yet." I wouldn't know how to invoke the selection of one or more layers from the ...sublayers dialog and making sure to select "Add to group" so that I could check if they actually became visible/invisible as requested in the QGIS settings.

[nyalldawson]

Sorry, I did mean "we're not setup"

[chau-intl]

No worries @nyalldawson, I just want to help this PR along and was wondering how I could unit test this, when it involves UI/user interaction :)

[elpaso]

@nyalldawson my goal was to automate the builds of the testing docker images, and that is done.
But all the pieces we need to execute tests in headless QGIS on Travis are in place, so I guess that it could be done with minimal effort.
I'll make some experiments.

[luipir]

when it involves UI/user interaction

I dont't think you need user interaction... DataItem in the layer list has properties as visible or not or cheked or not. You should only test that the DataItem property is correctly set (after adding the layer to a group) respect different settings configuration.

[chau-intl]

@luipir, the user process I need to unit test is:

1. Drag a data set which contains more than one layer into QGIS.
2. In the dialog that appears select one or more layer.
3. Make sure the "Add layers to group" option is selected.
4. Click OK and then evaluate whether the created group and the layer(s) is visible/invisible.

Is this possible to unit test?

[luipir]

technically yes, it's possible using QTest [1] (you can move mouse and click) but is it really necessary? should be enough directly test only:
void QgisApp::askUserForOGRSublayers( QgsVectorLayer *layer ) ?

[1] http://doc.qt.io/qt-5/qtest.html

[chau-intl]

@luipir, QgisApp::askUserForOGRSublayers(...) contains the following:

if ( !chooseSublayersDialog.exec() )
    return;

Which is where I need to make the choices. How would I gain access to that dialog from outside QgisApp::askUserForOGRSublayers(...)?

[luipir]

if ( !chooseSublayersDialog.exec() ) return;

effectively... you're right. @elpaso is just adding new test functionalities that allow to run test in a real qgis env. Waiting for this new(old) feature

[elpaso]

@nyalldawson looks like my little experiment is working: https://travis-ci.org/qgis/QGIS/jobs/458069882#L7361 , we should discuss if that's a useful addition to the testing framework and how to add these kind of tests to the mix.

[chau-intl]

@elpaso will the new test functionalities allow threaded testing?

I my case here, I need to test functionality within the method 'QgisApp::askUserForOGRSublayers(...)' which creates and displays a dialog where I need to make some selections before the method returns. To do this I guess I need one thread to make the test call to the above method and one thread to interact with the displayed dialog, or am I on the wrong track here?

[nyalldawson]

I wonder if we can defer the test here for a future pr. This is complex stuff and something we don't (yet) have any working examples of that contributors can use as a starting point. @luipir what do you think?

[luipir]

I wonder if we can defer the test here for a future pr.

+1

[chau-intl]

@luipir, @nyalldawson thanks for merging. I hope I can find something more simple to test the next time I make a PR :)

[nyalldawson]

Thanks @chau-intl -- this contribution is already much appreciated!

[luipir]

tnx @chau-intl stay tuned when a real qgis test infrastructure will be working :)
tnx @nyalldawson to fix build

[chau-intl]

@nyalldawson, You added the Needs Backporting tag to this and I guess it means it should be backported to 3.4.x LTR. Is it something that I should initiate or is it done by you (or others with more privileges)?

[elpaso]

@chau-intl you should do a PR against release-3_4 branch, these are roughly the steps:

• git checkout release-3_4
• git checkout -b my-backported-feature
• git cherry-pick xxxxxx (where xxxx is the commit from master that you want to backport)
• resolve any conflicts
• git push origin my-backported-feature (assuming that your QGIS clone remote is "origin")
• make the PR against release-3_4 branch