-
Notifications
You must be signed in to change notification settings - Fork 17
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
recreate thumbnail on setting new default style #440
recreate thumbnail on setting new default style #440
Conversation
Codecov Report
@@ Coverage Diff @@
## 2.8.x-qgis_server #440 +/- ##
=====================================================
- Coverage 41.13% 41.12% -0.01%
=====================================================
Files 403 403
Lines 28725 28734 +9
Branches 3650 3651 +1
=====================================================
+ Hits 11815 11816 +1
- Misses 16185 16193 +8
Partials 725 725
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank's @boney-bun .
I just add some more comment on this.
@@ -810,6 +811,21 @@ def default_qml_style(request, layername, style_name=None): | |||
qgis_layer.default_style = style | |||
qgis_layer.save() | |||
|
|||
# Recreate thumbnail |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you refactor thumbnail default creation to inside of create_qgis_server_thumbnail
, so this view function can just call one function?
Also please include in unittest of setting default_qml_style that the thumbnail is indeed recreated.
b6dc37d
to
6d90155
Compare
Codecov Report
@@ Coverage Diff @@
## 2.8.x-qgis_server #440 +/- ##
=====================================================
- Coverage 41.13% 41.12% -0.01%
=====================================================
Files 403 403
Lines 28725 28741 +16
Branches 3650 3652 +2
=====================================================
+ Hits 11815 11820 +5
- Misses 16185 16195 +10
- Partials 725 726 +1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@boney-bun please fix this first
@@ -516,7 +516,33 @@ def test_add_delete_style(self): | |||
default_style_url = reverse( | |||
'qgis_server:default-qml', | |||
kwargs={ | |||
'layername': layer.name}) | |||
'layername': layer.name, | |||
'style_name': 'new_style'}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This url should return default style, so you should not specify the style name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also response = self.client.get(default_style_url)
and the rest at the bottom should be directly below this line (because that was this test all about, checking that the new default style changed).
Basically your new test should be before layer.delete()
|
||
# Check thumbnail is created | ||
self.assertTrue(os.path.exists(thumbnail_path)) | ||
self.assertEqual(what(thumbnail_path), 'png') | ||
|
||
response = self.client.get(default_style_url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this to up there where it belongs.
thanks for the feedback @lucernae |
fix #426