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

Honour queryable field in layer info when bulding WMS getFeatureInfo request #1161

Merged
merged 2 commits into from
Oct 20, 2016

Conversation

manisandro
Copy link
Contributor

In QWC2, the entire project is published as one WMS with individual layers in
the QGIS project handled as sublayers of this WMS layer. We keep track of the
individual sublayers by extending the WMS layer params object with the fields:

sublayers: Array
opacities: Array
queryable: Array

This commit makes MapInfoUtils::buildIdentifyWMSRequest honour the queryable
array to exclude sublayers from the query which are flagged as non-identifyable.

@simboss simboss added the ready label Oct 17, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.02%) to 80.55% when pulling 476dbfe on sourcepole:wms_info_queryable into ffbaac9 on geosolutions-it:master.

@@ -152,12 +152,16 @@ const MapInfoUtils = {
const center = {x: lngCorrected, y: props.point.latlng.lat};
let centerProjected = CoordinatesUtils.reproject(center, 'EPSG:4326', props.map.projection);
let bounds = MapInfoUtils.getProjectedBBox(centerProjected, resolution, rotation, size, null);
let queryLayers = layer.name;
if (layer.queryable) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the queryable attribute of layers something custom to your application? I get it's an array of layers to be queried instead of the original layer. I think queryable is a misleading name. Usually queryable is a flag to distinguish layers that can be queries.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is custom in the sense that the upstream there is not support for WMS sublayer handling AFAICS. Say you have a WMS layer published as

somelayer
   |--> sublayer1
   |--> sublayer2
   etc

If you pass somelayer as layer parameter in the WMS request, it means the main layer including all possible sublayers. You can however select only certain sublayers by passing instead a comma-separated list of sublayer names, i.e. sublayer1,sublayer2. QGIS server (and possibly other WMS servers) can publish in the capabilities whether individual sublayers are "queryable" aka identifyable.

As for the name "queryable", I have no problems renaming it to "identifyable" or something else.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please rename to something like queryLayers and I think it's good. If it's not too complex, please also add a unit test

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I just noticed when grepping for queryable: there already are traces of a queryable field in the layer object [1], but I don't see other traces of it. Should I rename it also?

[1] https://github.com/geosolutions-it/MapStore2/blob/master/web/client/utils/MapInfoUtils.js#L223

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the flag I was referring too. It's coming from WMS capabilities to tell you if a layer is queryable or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh ok.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 80.572% when pulling 9297c8d on sourcepole:wms_info_queryable into 3fad9fd on geosolutions-it:master.

Copy link
Contributor

@mbarto mbarto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a unit test also for this one?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 80.706% when pulling 00f9317 on sourcepole:wms_info_queryable into 3fad9fd on geosolutions-it:master.

@mbarto mbarto merged commit 2b5b7e0 into geosolutions-it:master Oct 20, 2016
@manisandro manisandro deleted the wms_info_queryable branch October 20, 2016 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants