This commit introduces a major UI/UX redesign of the application usin…#5
Open
This commit introduces a major UI/UX redesign of the application usin…#5
Conversation
…g Bootstrap. It also makes the application production-ready by adding a `requirements.txt` file, a `Procfile` for Gunicorn, and configuring the app to use environment variables. The main changes are: - Replaced the old stylesheet with Bootstrap 5. - Updated all major templates to use Bootstrap components for a modern and responsive UI. - Added a `requirements.txt` file with all the necessary dependencies. - Added a `Procfile` to run the application with Gunicorn. - Updated the configuration to use `python-dotenv` to load environment variables. - Fixed several bugs related to missing dependencies and application startup hangs.
Reviewer's GuideThis PR overhauls the application’s UI/UX by integrating Bootstrap 5 and refactoring all major templates into responsive layouts, configures production deployment with Gunicorn and dotenv support, and strengthens reliability through robust external API error handling. Flow diagram for improved external API error handlingflowchart TD
A[User requests WHO/UNESCO data] --> B[App sends API request]
B -->|Success| C[Parse and display data]
B -->|Error/Timeout| D[Show error message to user]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Vulnerable Libraries (3)
More info on how to fix Vulnerable Libraries in Python. 👉 Go to the dashboard for detailed results. 📥 Happy? Share your feedback with us. |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @GYFX35 - I've reviewed your changes and found some issues that need to be addressed.
Blocking issues:
- Running with debug=True in production is a security risk. (link)
General comments:
- Pin dependency versions in requirements.txt to ensure reproducible builds and avoid unexpected upgrades.
- Remove debug=True from run.py in production mode and control debug configuration via environment variables.
- Add SRI integrity and crossorigin attributes to your CDN Bootstrap and jQuery script tags to improve security.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Pin dependency versions in requirements.txt to ensure reproducible builds and avoid unexpected upgrades.
- Remove debug=True from run.py in production mode and control debug configuration via environment variables.
- Add SRI integrity and crossorigin attributes to your CDN Bootstrap and jQuery script tags to improve security.
## Individual Comments
### Comment 1
<location> `app/routes.py:208` </location>
<code_context>
- url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
- response = requests.get(url)
- data[name] = response.json()['dataSets'][0]['series']
+ try:
+ for code, name in indicators.items():
+ url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
+ response = requests.get(url, timeout=10)
</code_context>
<issue_to_address>
Consider more granular error handling for UNESCO API requests.
Wrapping each API request in its own try/except block would allow the function to return available data even if some requests fail.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
try:
for code, name in indicators.items():
url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
response = requests.get(url, timeout=10)
response.raise_for_status()
data[name] = response.json()['dataSets'][0]['series']
except requests.exceptions.RequestException as e:
flash(f"Could not retrieve data from UNESCO API: {e}")
data = {}
=======
for code, name in indicators.items():
url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json"
try:
response = requests.get(url, timeout=10)
response.raise_for_status()
data[name] = response.json()['dataSets'][0]['series']
except requests.exceptions.RequestException as e:
flash(f"Could not retrieve data for '{name}' from UNESCO API: {e}", "warning")
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `templates/base.html:77` </location>
<code_context>
+ {% with messages = get_flashed_messages(with_categories=true) %}
+ {% if messages %}
+ {% for category, message in messages %}
+ <div class="alert alert-{{ category }}">{{ message }}</div>
+ {% endfor %}
+ {% endif %}
</code_context>
<issue_to_address>
Flashed message categories may not map directly to Bootstrap alert classes.
If your flash categories differ from Bootstrap's expected values, map them accordingly to ensure correct alert styling.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
{% with messages = get_flashed_messages(with_categories=true) %}
{% if messages %}
{% for category, message in messages %}
<div class="alert alert-{{ category }}">{{ message }}</div>
{% endfor %}
{% endif %}
=======
{% set bootstrap_alert_map = {
'error': 'danger',
'message': 'info',
'info': 'info',
'success': 'success',
'warning': 'warning',
'danger': 'danger'
} %}
{% with messages = get_flashed_messages(with_categories=true) %}
{% if messages %}
{% for category, message in messages %}
<div class="alert alert-{{ bootstrap_alert_map.get(category, 'info') }}">{{ message }}</div>
{% endfor %}
{% endif %}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `run.py:4` </location>
<code_context>
if __name__ == '__main__':
- app.run(debug=True)
+ app.run(host='0.0.0.0', port=8080, debug=True)
</code_context>
<issue_to_address>
Running with debug=True in production is a security risk.
Use environment variables or configuration files to manage the debug setting and ensure it is disabled in production.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+208
to
+216
| try: | ||
| for code, name in indicators.items(): | ||
| url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json" | ||
| response = requests.get(url, timeout=10) | ||
| response.raise_for_status() | ||
| data[name] = response.json()['dataSets'][0]['series'] | ||
| except requests.exceptions.RequestException as e: | ||
| flash(f"Could not retrieve data from UNESCO API: {e}") | ||
| data = {} |
There was a problem hiding this comment.
suggestion: Consider more granular error handling for UNESCO API requests.
Wrapping each API request in its own try/except block would allow the function to return available data even if some requests fail.
Suggested change
| try: | |
| for code, name in indicators.items(): | |
| url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json" | |
| response = requests.get(url, timeout=10) | |
| response.raise_for_status() | |
| data[name] = response.json()['dataSets'][0]['series'] | |
| except requests.exceptions.RequestException as e: | |
| flash(f"Could not retrieve data from UNESCO API: {e}") | |
| data = {} | |
| for code, name in indicators.items(): | |
| url = f"http://data.uis.unesco.org/api/v1/data/indicator/{code}?format=json" | |
| try: | |
| response = requests.get(url, timeout=10) | |
| response.raise_for_status() | |
| data[name] = response.json()['dataSets'][0]['series'] | |
| except requests.exceptions.RequestException as e: | |
| flash(f"Could not retrieve data for '{name}' from UNESCO API: {e}", "warning") |
Comment on lines
+74
to
+79
| {% with messages = get_flashed_messages(with_categories=true) %} | ||
| {% if messages %} | ||
| {% for category, message in messages %} | ||
| <div class="alert alert-{{ category }}">{{ message }}</div> | ||
| {% endfor %} | ||
| {% endif %} |
There was a problem hiding this comment.
suggestion: Flashed message categories may not map directly to Bootstrap alert classes.
If your flash categories differ from Bootstrap's expected values, map them accordingly to ensure correct alert styling.
Suggested change
| {% with messages = get_flashed_messages(with_categories=true) %} | |
| {% if messages %} | |
| {% for category, message in messages %} | |
| <div class="alert alert-{{ category }}">{{ message }}</div> | |
| {% endfor %} | |
| {% endif %} | |
| {% set bootstrap_alert_map = { | |
| 'error': 'danger', | |
| 'message': 'info', | |
| 'info': 'info', | |
| 'success': 'success', | |
| 'warning': 'warning', | |
| 'danger': 'danger' | |
| } %} | |
| {% with messages = get_flashed_messages(with_categories=true) %} | |
| {% if messages %} | |
| {% for category, message in messages %} | |
| <div class="alert alert-{{ bootstrap_alert_map.get(category, 'info') }}">{{ message }}</div> | |
| {% endfor %} | |
| {% endif %} |
|
|
||
| if __name__ == '__main__': | ||
| app.run(debug=True) | ||
| app.run(host='0.0.0.0', port=8080, debug=True) |
There was a problem hiding this comment.
🚨 issue (security): Running with debug=True in production is a security risk.
Use environment variables or configuration files to manage the debug setting and ensure it is disabled in production.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…g Bootstrap. It also makes the application production-ready by adding a
requirements.txtfile, aProcfilefor Gunicorn, and configuring the app to use environment variables.The main changes are:
requirements.txtfile with all the necessary dependencies.Procfileto run the application with Gunicorn.python-dotenvto load environment variables.Summary by Sourcery
Revamp the application’s UI with Bootstrap 5, make it production-ready by adding dependency and deployment configurations, and enhance API routes with error handling and timeouts.
New Features:
Bug Fixes: