Add parameter for HTML request #242

Gigaszi · 2022-01-25T13:56:10Z

Description

Add a parameter to the API endpoints to allow the request of a HTML snippet.

Corresponding issue

Closes #235

New or changed dependencies

jinja2

Checklist

Gigaszi · 2022-02-01T23:49:15Z

the CSS still needs to be integrated. Also the representation of the report results needs to be adjusted: the map as on the website does not work without javascript (maybe just a big traffic light like the small ones?)

website/website/assets/js/map-en.js

matthiasschaub

Looks good so far. I have not finished looking at every change yet. But here are some minor comments for the parts I already looked at.

Analogous to remove_svg_from_properties there should be a test for remove_html_from_properties.

tests/unittests/test_api.py
    def test_remove_svg_from_properties(self):

~~Update this branch to the latest commit in main.~~
Add jinja2 as dependency to pyproject.toml and poetry.lock using poetry add jinja2

workers/ohsome_quality_analyst/api/api.py

workers/ohsome_quality_analyst/base/indicator.py

matthiasschaub · 2022-02-02T09:14:28Z

the CSS still needs to be integrated. Also the representation of the report results needs to be adjusted: the map as on the website does not work without javascript (maybe just a big traffic light like the small ones?)

Lets have a discussion about the CSS and reports once I finished the first review.

Regarding the map on the website: It is not planned to include the map as part of the OQT API response. The response should only contain the description an plot for the results.

workers/ohsome_quality_analyst/base/indicator.py

workers/ohsome_quality_analyst/base/report.py

website/website/assets/js/map-en.js

Gigaszi · 2022-03-04T10:47:55Z

This is how the html request looks for now. It's not finished yet, but maybe you already have some feedback. I'm not quite sure if I should resize the map to fit the size of the indicators or leave it larger so you can see more

Gigaszi · 2022-03-07T17:39:53Z

I found this tool (https://github.com/rossant/smopy) to integrate OSM maps well with Python. The zoom is also adapted directly to the bounding box here. As far as I can see, there are no dependencies that we don't already have. So I would insert the map with it, if there is nothing against it from your side.

matthiasschaub · 2022-03-08T07:08:56Z

I found this tool (https://github.com/rossant/smopy) to integrate OSM maps well with Python. The zoom is also adapted directly to the bounding box here. As far as I can see, there are no dependencies that we don't already have. So I would insert the map with it, if there is nothing against it from your side.

I am not sure this is a good idea. The packages has not been updated since 2 years, is not available through pypi and has no tests :D. On the other hand the package is quite small and consists only 400 lines of code.
If this is working good for our case this could be a possibility. I would say try it out and see how good it works for us.

Gigaszi · 2022-03-14T09:05:49Z

matthiasschaub

Just a few commets while skiming the changes.
I have not yet tested it or looked at the HTML construction.

workers/ohsome_quality_analyst/api/api.py

workers/tests/unittests/test_api.py

workers/ohsome_quality_analyst/base/indicator.py

workers/ohsome_quality_analyst/base/report.py

workers/tests/integrationtests/test_api_indicator.py

workers/tests/integrationtests/test_api_report.py

The HTTP interactions introduced in #242 are containing binary data (OSM Map tiles as PNG). JSON can not handle those. `vcrpy` does recommend YAML in this case. Recreate all VCR cassettes.

joker234

A first review, still to review:

templates
tests
test website

joker234 · 2022-04-04T17:15:51Z

CHANGELOG.md

@@ -17,10 +17,12 @@
 - Fix concurrent execution of CLI function `create_all_indicators` using async and semaphores ([#254])
 - Support choosing a single indicator and/or single layer for CLI command `create_all_indicators` ([#254])
 - Indicators based on GHS-POP use raster file stored on disk instead of raster in the database ([#276])
+- Add new parameter `includeHtml` to the API endpoints to allow the request of a HTML snippet of the report result ([#242])


It's not only for reports, but indicators as well, isn't it?

website/website/assets/js/map-en.js

workers/ohsome_quality_analyst/api/api.py

workers/ohsome_quality_analyst/base/indicator.py

workers/ohsome_quality_analyst/base/report.py

workers/tests/integrationtests/test_api_report_geojson_io.py

Use HTML templates to create HTML snippets containing the results of an Indicator or Report.

('/indicator' and '/report'). If this parameter is set to True the response will contain an HTML snippet with the Indicator or Report results. The default is not HTML snippet will be included in the response.

Report and embed the HTML snippet of the API response in the website.

@matthiasschaub

@matthiasschaub replaced this review

section

Gigaszi closed this Jan 25, 2022

Gigaszi reopened this Jan 25, 2022

Gigaszi marked this pull request as draft January 25, 2022 13:58

matthiasschaub reviewed Feb 2, 2022

View reviewed changes

website/website/assets/js/map-en.js Outdated Show resolved Hide resolved

matthiasschaub reviewed Feb 2, 2022

View reviewed changes

matthiasschaub reviewed Feb 7, 2022

View reviewed changes

workers/ohsome_quality_analyst/base/indicator.py Outdated Show resolved Hide resolved

matthiasschaub reviewed Feb 7, 2022

View reviewed changes

workers/ohsome_quality_analyst/base/report.py Outdated Show resolved Hide resolved

matthiasschaub reviewed Feb 7, 2022

View reviewed changes

website/website/assets/js/map-en.js Outdated Show resolved Hide resolved

matthiasschaub added this to the Release 0.9.0 milestone Mar 14, 2022

matthiasschaub added enhancement New feature or request waiting An issue or PR which is waiting for an upstream bugfix, further information or is somehow blocked labels Mar 21, 2022

matthiasschaub requested changes Mar 24, 2022

View reviewed changes

joker234 removed the waiting An issue or PR which is waiting for an upstream bugfix, further information or is somehow blocked label Mar 24, 2022

matthiasschaub mentioned this pull request Mar 28, 2022

Change VCR serializer to YAML #278

Merged

2 tasks

Gigaszi requested a review from joker234 March 31, 2022 15:09

Gigaszi marked this pull request as ready for review April 1, 2022 17:03

Gigaszi changed the title ~~Add parameter for HTML request 🚧~~ Add parameter for HTML request Apr 1, 2022

joker234 previously requested changes Apr 4, 2022

View reviewed changes

joker234 force-pushed the html_request branch from 7ba2a7a to 4f009da Compare April 5, 2022 17:51

matthiasschaub force-pushed the html_request branch from 4b51a41 to 985a303 Compare April 6, 2022 17:05

matthiasschaub reviewed Apr 6, 2022

View reviewed changes

workers/tests/integrationtests/test_api_report_geojson_io.py Outdated Show resolved Hide resolved

matthiasschaub reviewed Apr 6, 2022

View reviewed changes

workers/tests/integrationtests/test_api_report_geojson_io.py Outdated Show resolved Hide resolved

matthiasschaub force-pushed the html_request branch from 0d12506 to 936d081 Compare April 6, 2022 17:50

matthiasschaub mentioned this pull request Apr 8, 2022

Loader animation missing on the website #284

Closed

matthiasschaub added 5 commits April 12, 2022 07:25

Add Jinja2 as dependency

021c84b

Create HTML snippet with Indicator/Report results

1df9fa3

Use HTML templates to create HTML snippets containing the results of an Indicator or Report.

Add 'includeHtml' parameter to API endpoints

bca417c

('/indicator' and '/report'). If this parameter is set to True the response will contain an HTML snippet with the Indicator or Report results. The default is not HTML snippet will be included in the response.

Use new API parameter 'includeHtml' to request a

c1c4afb

Report and embed the HTML snippet of the API response in the website.

Add changes to the CHANGELOG

16dfb45

matthiasschaub force-pushed the html_request branch from c36c88c to 16dfb45 Compare April 12, 2022 05:46

matthiasschaub approved these changes Apr 12, 2022

View reviewed changes

matthiasschaub merged commit 49d50a6 into main Apr 12, 2022

matthiasschaub deleted the html_request branch April 12, 2022 06:29

matthiasschaub added a commit that referenced this pull request Apr 12, 2022

Move change description of #242 to New Feature

3cf34b2

section

matthiasschaub added a commit that referenced this pull request Apr 12, 2022

Move change description of #242 to New Feature

a27abc9

section

matthiasschaub added a commit that referenced this pull request Apr 12, 2022

Move change description of #242 to New Feature

467e3ae

section

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add parameter for HTML request #242

Add parameter for HTML request #242

Gigaszi commented Jan 25, 2022 •

edited by matthiasschaub

Gigaszi commented Feb 1, 2022

matthiasschaub left a comment •

edited

matthiasschaub commented Feb 2, 2022

Gigaszi commented Mar 4, 2022 •

edited

Gigaszi commented Mar 7, 2022

matthiasschaub commented Mar 8, 2022 •

edited

Gigaszi commented Mar 14, 2022

matthiasschaub left a comment

joker234 left a comment •

edited by matthiasschaub

joker234 Apr 4, 2022

Add parameter for HTML request #242

Add parameter for HTML request #242

Conversation

Gigaszi commented Jan 25, 2022 • edited by matthiasschaub

Description

Corresponding issue

New or changed dependencies

Checklist

Gigaszi commented Feb 1, 2022

matthiasschaub left a comment • edited

Choose a reason for hiding this comment

matthiasschaub commented Feb 2, 2022

Gigaszi commented Mar 4, 2022 • edited

Gigaszi commented Mar 7, 2022

matthiasschaub commented Mar 8, 2022 • edited

Gigaszi commented Mar 14, 2022

matthiasschaub left a comment

Choose a reason for hiding this comment

joker234 left a comment • edited by matthiasschaub

Choose a reason for hiding this comment

joker234 Apr 4, 2022

Choose a reason for hiding this comment

Gigaszi commented Jan 25, 2022 •

edited by matthiasschaub

matthiasschaub left a comment •

edited

Gigaszi commented Mar 4, 2022 •

edited

matthiasschaub commented Mar 8, 2022 •

edited

joker234 left a comment •

edited by matthiasschaub