From 31fa8147b7eb7a22b6b4375a7822feb89cf7b8ee Mon Sep 17 00:00:00 2001 From: wayangalihpratama Date: Wed, 25 Mar 2026 09:58:47 +0800 Subject: [PATCH 01/11] [#664] feat(feedback): implement secure feedback download with ISCO access enforcement --- .gitignore | 3 +- backend/db/crud_feedback.py | 73 +++++- backend/routes/feedback.py | 70 +++++- backend/routes/isco_type.py | 23 ++ backend/tests/test_111_feedback_download.py | 210 ++++++++++++++++++ backend/util/sheets.py | 26 +++ frontend/src/App.js | 6 + frontend/src/pages/admin/Admin.jsx | 11 +- .../download-feedback/DownloadFeedback.jsx | 152 +++++++++++++ .../src/pages/download-feedback/style.scss | 35 +++ frontend/src/pages/index.js | 1 + 11 files changed, 593 insertions(+), 17 deletions(-) create mode 100644 backend/tests/test_111_feedback_download.py create mode 100644 frontend/src/pages/download-feedback/DownloadFeedback.jsx create mode 100644 frontend/src/pages/download-feedback/style.scss diff --git a/.gitignore b/.gitignore index fecaaa57..6db63fa1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,5 @@ /notify.team.sh .vscode/ .DS_Store -.env \ No newline at end of file +.env +agent_docs/ \ No newline at end of file diff --git a/backend/db/crud_feedback.py b/backend/db/crud_feedback.py index b786d6c6..bc5c120e 100644 --- a/backend/db/crud_feedback.py +++ b/backend/db/crud_feedback.py @@ -1,18 +1,25 @@ from datetime import datetime -from typing import List +from typing import List, Optional +from sqlalchemy import extract from sqlalchemy.orm import Session from models.feedback import Feedback from models.feedback import FeedbackDict, FeedbackPayload +from models.user import User +from models.organisation import Organisation +from models.organisation_isco import OrganisationIsco -def add_feedback(session: Session, user: int, - payload: FeedbackPayload) -> FeedbackDict: - feedback = Feedback(id=None, - user=user, - title=payload["title"], - category=payload["category"], - content=payload["content"], - created=datetime.now()) +def add_feedback( + session: Session, user: int, payload: FeedbackPayload +) -> FeedbackDict: + feedback = Feedback( + id=None, + user=user, + title=payload["title"], + category=payload["category"], + content=payload["content"], + created=datetime.now(), + ) session.add(feedback) session.commit() session.flush() @@ -24,7 +31,51 @@ def get_feedback(session: Session) -> List[FeedbackDict]: return session.query(Feedback).all() +def get_feedback_for_export( + session: Session, + isco_type_ids: Optional[List[int]] = None, + monitoring_round: Optional[int] = None, +): + query = ( + session.query( + Feedback.created, + Feedback.title, + Feedback.category, + Feedback.content, + User.name.label("user_name"), + Organisation.name.label("organisation_name"), + ) + .join(User, Feedback.user == User.id) + .join(Organisation, User.organisation == Organisation.id) + ) + + if isco_type_ids: + query = query.join( + OrganisationIsco, Organisation.id == OrganisationIsco.organisation + ).filter(OrganisationIsco.isco_type.in_(isco_type_ids)) + + if monitoring_round: + query = query.filter( + extract("year", Feedback.created) == monitoring_round + ) + + # Execute query + results = query.order_by(Feedback.created.desc()).all() + data = [] + for r in results: + data.append({ + "monitoring_round": r.created.year if r.created else None, + "user_name": r.user_name, + "organisation_name": r.organisation_name, + "title": r.title, + "category": r.category, + "content": r.content + }) + return data + + def delete_feedback_by_ids(session: Session, ids: List[int]): - session.query(Feedback).filter( - Feedback.id.in_(ids)).delete(synchronize_session='fetch') + session.query(Feedback).filter(Feedback.id.in_(ids)).delete( + synchronize_session="fetch" + ) session.commit() diff --git a/backend/routes/feedback.py b/backend/routes/feedback.py index b5bfb4d1..79bbc53a 100644 --- a/backend/routes/feedback.py +++ b/backend/routes/feedback.py @@ -1,14 +1,19 @@ from fastapi import Depends, Request, APIRouter, HTTPException, BackgroundTasks from fastapi.security import HTTPBearer from fastapi.security import HTTPBasicCredentials as credentials -from typing import List +from typing import List, Optional from sqlalchemy.orm import Session import db.crud_feedback as crud from db.connection import get_session from db import crud_organisation from models.feedback import FeedbackDict, FeedbackPayload, FeedbackCategory -from middleware import verify_editor, find_secretariat_admins +from middleware import verify_editor, find_secretariat_admins, verify_admin from util.mailer import Email, MailTypeEnum +from util.sheets import generate_feedback_export +from fastapi.responses import StreamingResponse +from models.user import UserRole +from models.organisation_isco import OrganisationIsco +import os security = HTTPBearer() feedback_route = APIRouter() @@ -82,3 +87,64 @@ def get_data(req: Request, credentials: credentials = Depends(security)): feedbacks = crud.get_feedback(session=session) return [f.serialize for f in feedbacks] + + +@feedback_route.get("/feedback/download", + summary="download feedback", + name="feedback:download", + tags=["Feedback"]) +def download(req: Request, + isco_type_id: Optional[int] = None, + monitoring_round: Optional[int] = None, + session: Session = Depends(get_session), + credentials: credentials = Depends(security)): + user = verify_admin(session=session, + authenticated=req.state.authenticated) + + isco_type_ids = None + if user.role == UserRole.member_admin: + # Get ISCOs for the user's organisation + user_org_iscos = session.query(OrganisationIsco).filter( + OrganisationIsco.organisation == user.organisation + ).all() + allowed_isco_ids = [o.isco_type for o in user_org_iscos] + + if isco_type_id: + if isco_type_id not in allowed_isco_ids: + raise HTTPException( + status_code=403, + detail="You don't have access to this ISCO's feedback" + ) + isco_type_ids = [isco_type_id] + else: + isco_type_ids = allowed_isco_ids + else: + # Secretariat admin can see all + if isco_type_id: + isco_type_ids = [isco_type_id] + + results = crud.get_feedback_for_export( + session=session, + isco_type_ids=isco_type_ids, + monitoring_round=monitoring_round) + + if not results: + raise HTTPException( + status_code=404, detail="No feedback data available") + + filename = "feedback_export" + file_path = generate_feedback_export(filename, results) + + def iterfile(): + with open(file_path, mode="rb") as file_like: + yield from file_like + if os.path.exists(file_path): + os.remove(file_path) + + return StreamingResponse( + iterfile(), + media_type="application/vnd.openxmlformats-officedocument" + ".spreadsheetml.sheet", + headers={ + "Content-Disposition": f"attachment; filename={filename}.xlsx" + }) diff --git a/backend/routes/isco_type.py b/backend/routes/isco_type.py index 9bda9d87..f9c98776 100644 --- a/backend/routes/isco_type.py +++ b/backend/routes/isco_type.py @@ -37,6 +37,29 @@ def get(req: Request, session: Session = Depends(get_session)): return [mt.serialize for mt in isco_type] +@isco_type_route.get("/isco_type/mine", + response_model=List[IscoTypeDict], + summary="get all isco types for current user", + name="isco_type:get_mine", + tags=["Isco Type"]) +def get_mine(req: Request, + session: Session = Depends(get_session), + credentials: credentials = Depends(security)): + from models.user import UserRole + from models.organisation_isco import OrganisationIsco + from middleware import verify_admin + user = verify_admin(session=session, + authenticated=req.state.authenticated) + if user.role == UserRole.secretariat_admin: + isco_type = crud.get_isco_type(session=session) + return [mt.serialize for mt in isco_type] + # Filter by user organisation + res = session.query(OrganisationIsco).filter( + OrganisationIsco.organisation == user.organisation).all() + # Return as IscoTypeDict + return [{"id": r.isco_type, "name": r.isco.name} for r in res] + + @isco_type_route.get("/isco_type/{id:path}", response_model=IscoTypeBase, summary="get member type by id", diff --git a/backend/tests/test_111_feedback_download.py b/backend/tests/test_111_feedback_download.py new file mode 100644 index 00000000..487369d3 --- /dev/null +++ b/backend/tests/test_111_feedback_download.py @@ -0,0 +1,210 @@ +import pytest +from fastapi import FastAPI +from httpx import AsyncClient +from sqlalchemy.orm import Session +from datetime import datetime +from models.feedback import Feedback +from models.user import User, UserRole +from models.organisation import Organisation +from models.organisation_isco import OrganisationIsco +from models.isco_type import IscoType +from db.crud_feedback import get_feedback_for_export +from .test_000_main import Acc + +pytestmark = pytest.mark.asyncio + + +class TestFeedbackDownload: + @pytest.mark.asyncio + async def test_get_feedback_for_export( + self, app: FastAPI, session: Session, client: AsyncClient + ) -> None: + # 1. Setup Data + # Add ISCO type first + isco = session.query(IscoType).filter(IscoType.id == 1).first() + if not isco: + isco = IscoType(id=1, name="Test ISCO") + session.add(isco) + session.commit() + + # Explicitly create organisation to avoid NoneType errors + org = Organisation(name="Test Org", code="TO", active=True) + session.add(org) + session.commit() + session.refresh(org) + + # Add ISCO type for this organisation + org_isco = OrganisationIsco(id=None, organisation=org.id, isco_type=1) + session.add(org_isco) + session.commit() + + user = User( + email="test@example.com", + password="password", + name="Test User", + phone_number="123456", + role=UserRole.member_user, + organisation=org.id, + invitation=None, + approved=True, + ) + user.email_verified = datetime.now() + session.add(user) + session.commit() + session.refresh(user) + + # Add feedback for this user + monitoring_round = datetime.now().year + f1 = Feedback( + id=None, + user=user.id, + title="Feedback 1", + category="questionnaire", + content="Content 1", + created=datetime.now(), + ) + session.add(f1) + session.commit() + + # 2. Test extraction without filters + results = get_feedback_for_export(session) + assert len(results) >= 1 + + # 3. Test extraction with monitoring round filter + results = get_feedback_for_export( + session, monitoring_round=monitoring_round + ) + assert len(results) >= 1 + + # 4. Test extraction with ISCO filter + results = get_feedback_for_export(session, isco_type_ids=[1]) + assert len(results) >= 1 + + # 5. Test extraction with non-existent monitoring round + results = get_feedback_for_export(session, monitoring_round=1999) + assert len(results) == 0 + + @pytest.mark.asyncio + async def test_download_feedback_endpoint( + self, app: FastAPI, session: Session, client: AsyncClient + ) -> None: + # 1. Setup Admin User + admin_email = "admin@example.com" + # Ensure org exists for admin (we can reuse or create new) + org = Organisation(name="Admin Org", code="AO", active=True) + session.add(org) + session.commit() + session.refresh(org) + + admin = User( + email=admin_email, + password="password", + name="Admin User", + phone_number="123456", + role=UserRole.secretariat_admin, + organisation=org.id, + invitation=None, + approved=True, + ) + admin.email_verified = datetime.now() + session.add(admin) + session.commit() + + # 2. Use Acc to get token + account = Acc(email=admin_email, token=None) + + # 3. Call endpoint + response = await client.get( + app.url_path_for("feedback:download"), + headers={"Authorization": f"Bearer {account.token}"}, + ) + + assert response.status_code == 200 + assert response.headers["content-type"] == ( + "application/vnd.openxmlformats-officedocument" + ".spreadsheetml.sheet" + ) + assert ( + "attachment; filename=feedback_export.xlsx" + in response.headers["content-disposition"] + ) + + @pytest.mark.asyncio + async def test_member_admin_access( + self, app: FastAPI, session: Session, client: AsyncClient + ) -> None: + # Setup ISCOs + isco1 = session.query(IscoType).filter(IscoType.id == 2).first() + if not isco1: + isco1 = IscoType(id=2, name="ISCO 1") + session.add(isco1) + isco2 = session.query(IscoType).filter(IscoType.id == 3).first() + if not isco2: + isco2 = IscoType(id=3, name="ISCO 2") + session.add(isco2) + session.commit() + + # Setup member admin with ISCO 1 + org = Organisation(name="Member Org", code="MO", active=True) + session.add(org) + session.commit() + session.refresh(org) + + org_isco = OrganisationIsco(id=None, organisation=org.id, isco_type=2) + session.add(org_isco) + session.commit() + + member_email = "member@example.com" + member_admin = User( + email=member_email, + password="password", + name="Member Admin", + phone_number="123456", + role=UserRole.member_admin, + organisation=org.id, + invitation=None, + approved=True, + ) + member_admin.email_verified = datetime.now() + session.add(member_admin) + session.commit() + + account = Acc(email=member_email, token=None) + + # Add feedback for this isco + f1 = Feedback( + id=None, + user=member_admin.id, + title="Member Feedback", + category="questionnaire", + content="Content from ISCO 1", + created=datetime.now(), + ) + session.add(f1) + session.commit() + + # Test /isco_type/mine + response = await client.get( + app.url_path_for("isco_type:get_mine"), + headers={"Authorization": f"Bearer {account.token}"}, + ) + assert response.status_code == 200 + data = response.json() + assert len(data) == 1 + assert data[0]["id"] == 2 + + # Test /feedback/download with allowed ISCO + response = await client.get( + app.url_path_for("feedback:download"), + params={"isco_type_id": 2}, + headers={"Authorization": f"Bearer {account.token}"}, + ) + assert response.status_code == 200 + + # Test /feedback/download with forbidden ISCO + response = await client.get( + app.url_path_for("feedback:download"), + params={"isco_type_id": 3}, + headers={"Authorization": f"Bearer {account.token}"}, + ) + assert response.status_code == 403 diff --git a/backend/util/sheets.py b/backend/util/sheets.py index bb033b3c..f6f1d8d1 100644 --- a/backend/util/sheets.py +++ b/backend/util/sheets.py @@ -252,3 +252,29 @@ def generate_summary( session.close() writer.save() return tmp_file + + +def generate_feedback_export(filename: str, results: list): + tmp_file = f"./tmp/{filename}.xlsx" + df = pd.DataFrame(results) + # Rename columns to be more user friendly + column_mapping = { + "monitoring_round": "Monitoring Round", + "user_name": "User Name", + "organisation_name": "Organisation", + "title": "Title", + "category": "Category", + "content": "Feedback" + } + df = df.rename(columns=column_mapping) + # Reorder columns to match request + cols = [ + "Monitoring Round", "User Name", "Organisation", + "Title", "Category", "Feedback" + ] + df = df[cols] + + writer = pd.ExcelWriter(tmp_file, engine="xlsxwriter") + df.to_excel(writer, index=False, sheet_name="Feedback") + writer.save() + return tmp_file diff --git a/frontend/src/App.js b/frontend/src/App.js index f837afa9..3a78cc97 100644 --- a/frontend/src/App.js +++ b/frontend/src/App.js @@ -26,6 +26,7 @@ import { DataCleaning, DownloadReport, ManageRoadmap, + DownloadFeedback, } from "./pages"; import { Alert } from "antd"; import { useCookies } from "react-cookie"; @@ -260,6 +261,11 @@ const App = () => { path="/manage-roadmap" element={} /> + } + /> } /> { diff --git a/frontend/src/pages/download-feedback/DownloadFeedback.jsx b/frontend/src/pages/download-feedback/DownloadFeedback.jsx new file mode 100644 index 00000000..878767a3 --- /dev/null +++ b/frontend/src/pages/download-feedback/DownloadFeedback.jsx @@ -0,0 +1,152 @@ +import React, { useState } from "react"; +import "./style.scss"; +import { Row, Col, Typography, Select, Card, Space, Button } from "antd"; +import { api, store } from "../../lib"; +import { useNotification } from "../../util"; +import { MonitoringRoundSelector } from "../../components"; +import { globalSelectProps } from "../../lib/util"; + +const { Title } = Typography; + +const handleSelectFilter = (input, option) => + option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0; + +const DownloadFeedback = () => { + const { user } = store.useState((s) => s); + const { isco_type } = store.useState((s) => s.optionValues); + const { notify } = useNotification(); + + const [iscoSelected, setIscoSelected] = useState(null); + const [selectedMonitoringRound, setSelectedMonitoringRound] = useState(null); + const [downloading, setDownloading] = useState(false); + const [allowedIsco, setAllowedIsco] = React.useState([]); + + React.useEffect(() => { + if (user?.role === "member_admin") { + api.get("/isco_type/mine").then((res) => { + setAllowedIsco( + res.data.map((x) => ({ + label: x.name, + value: x.id, + })) + ); + }); + } + }, [user]); + + const iscoOptions = + user?.role === "member_admin" + ? allowedIsco + : isco_type.length + ? isco_type + .filter((x) => x.id !== 1 || x.name.toLowerCase() !== "all") + .map((x) => ({ + label: x.name, + value: x.id, + })) + : []; + + const handleDownloadFeedback = () => { + if (!selectedMonitoringRound) { + notify({ + type: "error", + message: "Monitoring Round is mandatory", + }); + return; + } + setDownloading(true); + let params = `monitoring_round=${selectedMonitoringRound}`; + if (iscoSelected) { + params += `&isco_type_id=${iscoSelected}`; + } + api + .get(`/feedback/download?${params}`, { responseType: "blob" }) + .then((res) => { + const url = window.URL.createObjectURL(new Blob([res.data])); + const link = document.createElement("a"); + link.href = url; + link.setAttribute("download", "feedback_export.xlsx"); + document.body.appendChild(link); + link.click(); + link.parentNode.removeChild(link); + }) + .catch((e) => { + const { status } = e.response || {}; + notify({ + type: "error", + message: + status === 404 + ? "No feedback data available." + : "Something went wrong.", + }); + console.error(e); + }) + .finally(() => { + setDownloading(false); + }); + }; + + return ( +
+ + + + + + Download Feedback + + + + + + + + + + setIscoSelected(val)} - style={{ width: "15rem" }} - {...globalSelectProps} - /> - - - - - - - - + + + +