-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Integrate UNDP API for SDG data #4
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
base: feat/initial-project-structure
Are you sure you want to change the base?
Conversation
This commit introduces an integration with the United Nations Development Programme (UNDP) API to display data related to Sustainable Development Goals (SDGs). - Adds a new backend endpoint `/api/sdg_data` to fetch data for SDG 15 (Life on Land) from the UNDP API. - The backend processes the API response to filter data for Brazil, Indonesia, and the Democratic Republic of Congo. - Updates the frontend to display the fetched SDG data in a new section on the main page. - Modifies the Flask application to serve the frontend static files, enabling the application to function as a single-page app and avoiding same-origin policy issues during development and testing.
Reviewer's GuideThis PR integrates a new UNDP API endpoint into the Flask backend to retrieve and filter SDG 15 data for selected countries, configures the app to serve frontend static files as a single-page app, and updates the frontend to fetch and render the new SDG data section alongside existing World Bank data. Sequence diagram for fetching and displaying UNDP SDG datasequenceDiagram
participant User
participant Frontend
participant Backend
participant UNDP_API
User->>Frontend: Loads main page
Frontend->>Backend: GET /api/sdg_data
Backend->>UNDP_API: GET target-data.json?sdg=15
UNDP_API-->>Backend: SDG 15 data
Backend->>Backend: Filter for BRA, IDN, COD
Backend-->>Frontend: Filtered SDG data (JSON)
Frontend->>Frontend: Render SDG data in new section
Frontend-->>User: Display SDG data for selected countries
Class diagram for new SDG data structure in backendclassDiagram
class SDGData {
+country: string
+country_iso3_code: string
+target_id: string
+description: string
+budget: float
+expense: float
}
class FlaskApp {
+send_static(path)
+home()
+forest_area()
+sdg_data()
}
FlaskApp "1" -- "*" SDGData: returns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Insecure Processing of Data (1)
More info on how to fix Insecure Processing of Data in JavaScript. 👉 Go to the dashboard for detailed results. 📥 Happy? Share your feedback with us. |
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.
Hey there - I've reviewed your changes - here's some feedback:
Blocking issues:
- Detected a 'requests' call without a timeout set. By default, 'requests' calls wait until the connection is closed. This means a 'requests' call without a timeout will hang the program if a response is never received. Consider setting a timeout for all 'requests'. (link)
- User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
undpDataContainer.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
undpDataContainer.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in methods like
innerHTML,outerHTMLordocument.writeis an anti-pattern that can lead to XSS vulnerabilities (link) - User controlled data in a
undpDataContainer.innerHTMLis an anti-pattern that can lead to XSS vulnerabilities (link)
General comments:
- Consider configuring Flask’s static_folder and static_url_path in app initialization instead of a custom send_static route to simplify asset serving.
- Extract the UNDP API endpoint, SDG target ID, and countries_of_interest into configuration or environment variables to make the integration more flexible.
- Implement caching for the /api/sdg_data endpoint to avoid repeated external API calls and improve response times.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider configuring Flask’s static_folder and static_url_path in app initialization instead of a custom send_static route to simplify asset serving.
- Extract the UNDP API endpoint, SDG target ID, and countries_of_interest into configuration or environment variables to make the integration more flexible.
- Implement caching for the /api/sdg_data endpoint to avoid repeated external API calls and improve response times.
## Individual Comments
### Comment 1
<location> `eco_project/backend/app.py:51-53` </location>
<code_context>
+ url = "https://api.open.undp.org/api/target-data.json?sdg=15"
+ countries_of_interest = ["BRA", "IDN", "COD"]
+
+ try:
+ response = requests.get(url)
+ data = response.json()
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** No timeout specified for external API requests.
Add a timeout to the requests.get call to prevent potential hangs if the API is slow or unresponsive.
```suggestion
try:
response = requests.get(url, timeout=10)
data = response.json()
```
</issue_to_address>
### Comment 2
<location> `eco_project/frontend/script.js:81-82` </location>
<code_context>
+ data.forEach(item => {
+ html += `<li><b>${item.country} (Target ${item.target_id}):</b> ${item.description}
+ <ul>
+ <li>Budget: $${item.budget ? item.budget.toFixed(2) : 'N/A'}</li>
+ <li>Expense: $${item.expense ? item.expense.toFixed(2) : 'N/A'}</li>
+ </ul>
+ </li>`;
</code_context>
<issue_to_address>
**issue (bug_risk):** Potential for runtime errors if budget or expense is not a number.
Explicitly convert budget and expense to numbers before using toFixed, or use a formatting method that handles non-numeric values safely.
</issue_to_address>
### Comment 3
<location> `eco_project/backend/app.py:52` </location>
<code_context>
response = requests.get(url)
</code_context>
<issue_to_address>
**security (python.requests.best-practice.use-timeout):** Detected a 'requests' call without a timeout set. By default, 'requests' calls wait until the connection is closed. This means a 'requests' call without a timeout will hang the program if a response is never received. Consider setting a timeout for all 'requests'.
```suggestion
response = requests.get(url, timeout=30)
```
*Source: opengrep*
</issue_to_address>
### Comment 4
<location> `eco_project/frontend/script.js:68` </location>
<code_context>
undpDataContainer.innerHTML = `<p>Error fetching data: ${data.error}</p>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 5
<location> `eco_project/frontend/script.js:68` </location>
<code_context>
undpDataContainer.innerHTML = `<p>Error fetching data: ${data.error}</p>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `undpDataContainer.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 6
<location> `eco_project/frontend/script.js:88` </location>
<code_context>
undpDataContainer.innerHTML = html;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 7
<location> `eco_project/frontend/script.js:88` </location>
<code_context>
undpDataContainer.innerHTML = html;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `undpDataContainer.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 8
<location> `eco_project/frontend/script.js:91` </location>
<code_context>
undpDataContainer.innerHTML = `<p>Error fetching data: ${error.message}</p>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-document-method):** User controlled data in methods like `innerHTML`, `outerHTML` or `document.write` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 9
<location> `eco_project/frontend/script.js:91` </location>
<code_context>
undpDataContainer.innerHTML = `<p>Error fetching data: ${error.message}</p>`;
</code_context>
<issue_to_address>
**security (javascript.browser.security.insecure-innerhtml):** User controlled data in a `undpDataContainer.innerHTML` is an anti-pattern that can lead to XSS vulnerabilities
*Source: opengrep*
</issue_to_address>
### Comment 10
<location> `eco_project/frontend/script.js:59-93` </location>
<code_context>
</code_context>
<issue_to_address>
**issue (code-quality):** Avoid function declarations, favouring function assignment expressions, inside blocks. ([`avoid-function-declarations-in-blocks`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-function-declarations-in-blocks))
<details><summary>Explanation</summary>Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers.
Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you
should use function expressions, which create functions in-scope.
</details>
</issue_to_address>
### Comment 11
<location> `eco_project/backend/app.py:52` </location>
<code_context>
@app.route('/api/sdg_data')
def sdg_data():
# UNDP API URL for SDG 15: Life on Land
url = "https://api.open.undp.org/api/target-data.json?sdg=15"
countries_of_interest = ["BRA", "IDN", "COD"]
try:
response = requests.get(url)
data = response.json()
if data:
formatted_data = []
for target in data:
for recipient in target.get('top_recipients', []):
if recipient.get('iso3') in countries_of_interest:
formatted_data.append({
'country': recipient.get('name'),
'country_iso3_code': recipient.get('iso3'),
'target_id': target.get('target_id'),
'description': target.get('description'),
'budget': recipient.get('total_budget'),
'expense': recipient.get('total_expense')
})
return jsonify(formatted_data)
else:
return jsonify({"error": "No data found for the selected criteria."}), 404
except requests.exceptions.RequestException as e:
return jsonify({"error": str(e)}), 500
</code_context>
<issue_to_address>
**issue (code-quality):** We've found these issues:
- Use named expression to simplify assignment and conditional ([`use-named-expression`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/use-named-expression/))
- Swap if/else branches ([`swap-if-else-branches`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/swap-if-else-branches/))
- Remove unnecessary else after guard condition ([`remove-unnecessary-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-unnecessary-else/))
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| <li>Budget: $${item.budget ? item.budget.toFixed(2) : 'N/A'}</li> | ||
| <li>Expense: $${item.expense ? item.expense.toFixed(2) : 'N/A'}</li> |
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.
issue (bug_risk): Potential for runtime errors if budget or expense is not a number.
Explicitly convert budget and expense to numbers before using toFixed, or use a formatting method that handles non-numeric values safely.
| async function fetchUNDPData() { | ||
| try { | ||
| const response = await fetch('/api/sdg_data'); | ||
| if (!response.ok) { | ||
| throw new Error(`HTTP error! status: ${response.status}`); | ||
| } | ||
| const data = await response.json(); | ||
|
|
||
| if (data.error) { | ||
| undpDataContainer.innerHTML = `<p>Error fetching data: ${data.error}</p>`; | ||
| return; | ||
| } | ||
|
|
||
| if (data.length === 0) { | ||
| undpDataContainer.innerHTML = '<p>No UNDP data available for the selected countries.</p>'; | ||
| return; | ||
| } | ||
|
|
||
| let html = '<ul>'; | ||
| data.forEach(item => { | ||
| html += `<li><b>${item.country} (Target ${item.target_id}):</b> ${item.description} | ||
| <ul> | ||
| <li>Budget: $${item.budget ? item.budget.toFixed(2) : 'N/A'}</li> | ||
| <li>Expense: $${item.expense ? item.expense.toFixed(2) : 'N/A'}</li> | ||
| </ul> | ||
| </li>`; | ||
| }); | ||
| html += '</ul>'; | ||
|
|
||
| undpDataContainer.innerHTML = html; | ||
|
|
||
| } catch (error) { | ||
| undpDataContainer.innerHTML = `<p>Error fetching data: ${error.message}</p>`; | ||
| } | ||
| } |
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.
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks)
This commit introduces an integration with the United Nations Development Programme (UNDP) API to display data related to Sustainable Development Goals (SDGs).
/api/sdg_datato fetch data for SDG 15 (Life on Land) from the UNDP API.Summary by Sourcery
Integrate the UNDP API to fetch SDG 15 data, expose it via a new backend endpoint, and update the frontend to display the filtered data alongside existing content
New Features:
Enhancements: