kartoza / geonode

GeoNode is an open source platform that facilitates the creation, sharing, and collaborative use of geospatial data.
http://geonode.org/
GNU General Public License v3.0
8 stars 17 forks source link

fix difference between layers symbology and map's thumbnails #445

Closed boney-bun closed 6 years ago

boney-bun commented 6 years ago

fix #406

boney-bun commented 6 years ago

the PR in action: geonode440

it turns out the request to otf project includes removing a qml file. that's why the qgis-server can't pick up the style as the qml file is deleted in qgis_layer folder.

the question is: in what use case do we need to remove qml when uploading a layer?

codecov[bot] commented 6 years ago

Codecov Report

Merging #445 into 2.8.x-qgis_server will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@                Coverage Diff                 @@
##           2.8.x-qgis_server     #445   +/-   ##
==================================================
  Coverage              41.13%   41.13%           
==================================================
  Files                    403      403           
  Lines                  28725    28725           
  Branches                3650     3650           
==================================================
  Hits                   11815    11815           
  Misses                 16185    16185           
  Partials                 725      725
Impacted Files Coverage Δ
geonode/qgis_server/helpers.py 66.66% <ø> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7e42dce...c7f5b34. Read the comment docs.

gubuntu commented 6 years ago

that was a simple fix! LGTM

Good that you're getting to know the innards of OTF plugin and QGIS server backend

lucernae commented 6 years ago

the question is: in what use case do we need to remove qml when uploading a layer?

When reuploading a layer you need to delete existing qml file, because it is possible that somehow you reupload a layer without qml file, so the previous qml file needs to be deleted. I still don't understand though, the scenario was supposed to be always deleting previous QML files when you upload a layer. Why does turning it to False here make it work? @boney-bun can you try upload reupload with different style? If the QML is not removed, it will always uses previous style even if you upload a new one. Just to check.

boney-bun commented 6 years ago

thanks @gubuntu and @NyakudyaA for the review and hints. i'll merge and bring this to testing.

boney-bun commented 6 years ago

@lucernae i observe everytime a user upload a layer, geonode will copy the layer and add some suffix to the layer name into uploaded folder. so, it is unlikely that the same layer will refer to the same qml file. if the qml is missing when uploading a layer, the chance is qgis-server will generate random style for the layer's thumbnail as in the first image below: screen shot 2018-07-16 at 1 56 33 pm

from the image above, the first layer was uploaded without qml file, while the second one included the qml file.

in the case of generating a thumbnail for a map, the files of the corresponding layer will be copied into qgis_layer folder (including the qml file). the bug is because otf-project will delete this qml file, then make a thumbnail request to qgis-server. that's the reason why qgis-server generate random symbol.

i'll reopen this PR if we find another use case.

lucernae commented 6 years ago

@lucernae i observe everytime a user upload a layer, geonode will copy the layer and add some suffix to the layer name into uploaded folder.

GeoNode will put a suffix because you uploaded a file with the same name. This is not a reupload, rather a different layer. You need to check it if you reupload a layer (using layer replace button). The QML needs to be deleted because it needs a fresh qml file from what you upload, and also delete existing one it if you didn't upload a QML file in your reupload scenario.

in the case of generating a thumbnail for a map, the files of the corresponding layer will be copied into qgis_layer folder (including the qml file).

Map thumbnail doesn't work this way. IIRC, QGIS Server create another qgs project file to remember which layer needs to be rendered. It will not have qml file associated with the qgs project because QGIS needs to remember the style in the qgs project file. One counter argument would be because it is possible for a layer to have multiple styles, these styles were not saved in the QML (because there would be multiple QML if that's the case), these styles were saved in qgs project, so we won't need the qml file. It was saved on qgs project for layer, but currently it's not saved on qgs project for map. You have to modify current otf-project to save default style taken from layer's qgs project.

gubuntu commented 6 years ago

@boney-bun does this fix #325 as well?

gubuntu commented 6 years ago

@boney-bun can you explain what you finally implemented in this PR and add a screencast from the testing server if you believe it's fixed.