From e3a8d15ebbb9789e5664a2f890e3b150546f2007 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 00:03:48 +0100 Subject: [PATCH 01/37] refactor: enhance task status handling in dashboard logic and update AdminDashboard component to include backlog and review tasks --- .../api/controllers/dashboard_controller.py | 32 ++++++++---- frontend/src/pages/AdminDashboard.jsx | 51 +++++++++++-------- .../src/tests/pages/AdminDashboard.test.jsx | 36 +++++++++++++ 3 files changed, 88 insertions(+), 31 deletions(-) diff --git a/backend/src/api/controllers/dashboard_controller.py b/backend/src/api/controllers/dashboard_controller.py index 3f24b01..9e334af 100644 --- a/backend/src/api/controllers/dashboard_controller.py +++ b/backend/src/api/controllers/dashboard_controller.py @@ -11,6 +11,8 @@ logging.basicConfig(level=logging.INFO) logger = logging.getLogger(__name__) +COMPLETED_TASK_STATUSES = {'done', 'completed'} + def _safe_query_all(model): try: @@ -22,6 +24,14 @@ def _safe_query_all(model): def _count(items, predicate): return sum(1 for item in items if predicate(item)) + +def _is_completed_status(status): + return status in COMPLETED_TASK_STATUSES + + +def _is_completed_task(task): + return _is_completed_status(getattr(task, 'status', None)) + def get_user_tasks(user_id): """Helper function to get all tasks for a user""" try: @@ -38,7 +48,7 @@ def get_tasks_due_soon(user_id): return Task.query.filter_by(assigned_to=user_id)\ .filter(Task.deadline.isnot(None))\ .filter(Task.deadline.between(today, week_later))\ - .filter(Task.status != 'done').all() + .filter(~Task.status.in_(COMPLETED_TASK_STATUSES)).all() except Exception as e: logger.error(f"Error fetching tasks due soon: {str(e)}") return [] @@ -61,8 +71,11 @@ def get_recent_completed_tasks(user_id, timeframe='month'): days = 30 time_ago = today - timedelta(days=days) - return Task.query.filter_by(assigned_to=user_id, status='done')\ - .filter(Task.updated_at >= time_ago).all() + return Task.query.filter_by(assigned_to=user_id)\ + .filter( + Task.status.in_(COMPLETED_TASK_STATUSES), + Task.updated_at >= time_ago, + ).all() except Exception as e: logger.error(f"Error fetching completed tasks: {str(e)}") return [] @@ -83,7 +96,7 @@ def get_project_tasks_due_soon(project_id): return Task.query.filter_by(project_id=project_id)\ .filter(Task.deadline.isnot(None))\ .filter(Task.deadline.between(today, week_later))\ - .filter(Task.status != 'done').all() + .filter(~Task.status.in_(COMPLETED_TASK_STATUSES)).all() except Exception as e: logger.error(f"Error fetching project tasks due soon: {str(e)}") return [] @@ -134,8 +147,8 @@ def get_user_dashboard(): }, 'tasks': { 'assigned_count': len(assigned_tasks), - 'pending_count': len([t for t in assigned_tasks if t.status != 'done']), - 'completed_count': len([t for t in assigned_tasks if t.status == 'done']), + 'pending_count': len([t for t in assigned_tasks if not _is_completed_task(t)]), + 'completed_count': len([t for t in assigned_tasks if _is_completed_task(t)]), 'due_soon': [{ 'id': task.id, 'title': task.title, @@ -165,7 +178,7 @@ def get_user_dashboard(): dashboard_data['team'] = { 'total_tasks': len(team_tasks), - 'completed_tasks': len([t for t in team_tasks if t.status == 'done']), + 'completed_tasks': len([t for t in team_tasks if _is_completed_task(t)]), 'in_progress_tasks': len([t for t in team_tasks if t.status == 'in_progress']), 'pending_tasks': len([t for t in team_tasks if t.status == 'todo']) } @@ -278,10 +291,11 @@ def get_admin_dashboard(): # Calculate task statistics task_stats = { 'total': len(all_tasks), + 'backlog': len([t for t in all_tasks if t.status == 'backlog']), 'todo': len([t for t in all_tasks if t.status == 'todo']), 'in_progress': len([t for t in all_tasks if t.status == 'in_progress']), 'review': len([t for t in all_tasks if t.status == 'review']), - 'done': len([t for t in all_tasks if t.status == 'done']) + 'done': len([t for t in all_tasks if _is_completed_task(t)]) } # Format response data @@ -333,7 +347,7 @@ def get_project_dashboard(project_id): 'todo': len([t for t in tasks if t.status == 'todo']), 'in_progress': len([t for t in tasks if t.status == 'in_progress']), 'review': len([t for t in tasks if t.status == 'review']), - 'done': len([t for t in tasks if t.status == 'done']) + 'done': len([t for t in tasks if _is_completed_task(t)]) } # Calculate completion percentage diff --git a/frontend/src/pages/AdminDashboard.jsx b/frontend/src/pages/AdminDashboard.jsx index 79d556f..aea0b17 100644 --- a/frontend/src/pages/AdminDashboard.jsx +++ b/frontend/src/pages/AdminDashboard.jsx @@ -319,29 +319,36 @@ const AdminDashboard = () => {
- {[ - { label: 'To Do', value: dashboardData?.tasks?.todo || 0, color: 'bg-slate-500' }, - { label: 'In Progress', value: dashboardData?.tasks?.active || 0, color: 'bg-sky-500' }, - { label: 'In Review', value: dashboardData?.tasks?.inReview || 0, color: 'bg-amber-500' }, - { label: 'Done', value: dashboardData?.tasks?.done || 0, color: 'bg-emerald-500' }, - ].map(({ label, value, color }) => { - const total = (dashboardData?.tasks?.todo || 0) - + (dashboardData?.tasks?.active || 0) - + (dashboardData?.tasks?.inReview || 0) - + (dashboardData?.tasks?.done || 0); - const pct = total > 0 ? Math.round((value / total) * 100) : 0; - return ( -
-
- {label} - {value} ({pct}%) -
-
-
+ {(() => { + const backlog = dashboardData?.tasks?.backlog || 0; + const todo = dashboardData?.tasks?.todo || 0; + const inProgress = dashboardData?.tasks?.in_progress ?? dashboardData?.tasks?.active ?? 0; + const inReview = dashboardData?.tasks?.review ?? dashboardData?.tasks?.inReview ?? 0; + const done = dashboardData?.tasks?.done || 0; + const total = backlog + todo + inProgress + inReview + done; + + return [ + { label: 'Backlog', value: backlog, color: 'bg-orange-500' }, + { label: 'To Do', value: todo, color: 'bg-slate-500' }, + { label: 'In Progress', value: inProgress, color: 'bg-sky-500' }, + { label: 'In Review', value: inReview, color: 'bg-amber-500' }, + { label: 'Done', value: done, color: 'bg-emerald-500' }, + ].map(({ label, value, color }) => { + const pct = total > 0 ? Math.round((value / total) * 100) : 0; + + return ( +
+
+ {label} + {value} ({pct}%) +
+
+
+
-
- ); - })} + ); + }); + })()}
diff --git a/frontend/src/tests/pages/AdminDashboard.test.jsx b/frontend/src/tests/pages/AdminDashboard.test.jsx index 6810eb1..e2b4f53 100644 --- a/frontend/src/tests/pages/AdminDashboard.test.jsx +++ b/frontend/src/tests/pages/AdminDashboard.test.jsx @@ -10,6 +10,9 @@ jest.mock('../../services/utils/api', () => ({ dashboardService: { getAdminDashboardStats: jest.fn(), }, + projectService: { + getAllProjects: jest.fn(), + }, })); jest.mock('../../context/AuthContext', () => ({ @@ -68,6 +71,8 @@ describe('AdminDashboard page', () => { dashboardService.getAdminDashboardStats.mockReset(); dashboardService.getAdminDashboardStats.mockResolvedValue(adminStatsPayload); + require('../../services/utils/api').projectService.getAllProjects.mockReset(); + require('../../services/utils/api').projectService.getAllProjects.mockResolvedValue([]); }); afterEach(() => { @@ -87,6 +92,37 @@ describe('AdminDashboard page', () => { expect(screen.getByRole('link', { name: /DevSync Core/i })).toHaveAttribute('href', '/projects/1'); }); + test('renders review tasks in the task breakdown', async () => { + dashboardService.getAdminDashboardStats.mockResolvedValueOnce({ + projects: { total: 5 }, + tasks: { + total: 10, + backlog: 2, + todo: 2, + in_progress: 3, + review: 1, + done: 2, + }, + users: { total: 8 }, + recentProjects: [ + { + id: 99, + name: 'Recent Project', + status: 'active', + created_at: '2099-01-01T00:00:00.000Z', + task_count: 4, + }, + ], + }); + + renderAdminDashboard(); + + expect(await screen.findByText('In Review')).toBeInTheDocument(); + expect(screen.getByText('Backlog')).toBeInTheDocument(); + expect(screen.getAllByText('1', { selector: '.text-slate-400' }).length).toBeGreaterThan(0); + expect(screen.getAllByText('2', { selector: '.text-slate-400' }).length).toBeGreaterThan(0); + }); + test('refetches dashboard data when time range changes and when refresh is clicked', async () => { renderAdminDashboard(); From 28085be36cd4d8e76af7bb4efda0252bf46bb946 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 00:04:01 +0100 Subject: [PATCH 02/37] refactor: update task status handling in dashboard tests to include 'completed' and 'backlog' statuses --- .../test_auth_socket_dashboard_integration.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/backend/tests/integration/test_auth_socket_dashboard_integration.py b/backend/tests/integration/test_auth_socket_dashboard_integration.py index 616e91a..d2e1364 100644 --- a/backend/tests/integration/test_auth_socket_dashboard_integration.py +++ b/backend/tests/integration/test_auth_socket_dashboard_integration.py @@ -286,6 +286,7 @@ def test_dashboard_client_route_returns_computed_task_stats(client, app, monkeyp assigned_tasks = [ SimpleNamespace(status='todo'), SimpleNamespace(status='done'), + SimpleNamespace(status='completed'), ] class StubUser: @@ -301,9 +302,9 @@ class StubUser: assert response.status_code == 200 payload = response.get_json() - assert payload['tasks']['total'] == 2 + assert payload['tasks']['total'] == 3 assert payload['tasks']['todo'] == 1 - assert payload['tasks']['done'] == 1 + assert payload['tasks']['done'] == 2 assert payload['tasks_due_soon'][0]['id'] == 9 assert payload['projects'][0]['name'] == 'Project A' @@ -316,10 +317,12 @@ def test_dashboard_admin_route_returns_user_and_task_totals(client, app, monkeyp SimpleNamespace(role='team_lead'), ] tasks = [ + SimpleNamespace(status='backlog'), SimpleNamespace(status='todo'), SimpleNamespace(status='in_progress'), SimpleNamespace(status='review'), SimpleNamespace(status='done'), + SimpleNamespace(status='completed'), ] class StubUser: @@ -348,7 +351,9 @@ class StubProject: assert payload['users']['admin'] == 1 assert payload['users']['developer'] == 1 assert payload['users']['team_lead'] == 1 - assert payload['tasks']['total'] == 4 + assert payload['tasks']['total'] == 6 + assert payload['tasks']['backlog'] == 1 + assert payload['tasks']['done'] == 2 assert payload['projects']['total'] == 4 From aad3939c3d55a35a1efad43e5cca59af9aa8e529 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 00:04:22 +0100 Subject: [PATCH 03/37] refactor: update report and task services to include backlog metrics and enhance API tests --- frontend/src/components/ReportTable.jsx | 2 +- frontend/src/services/utils/api.js | 74 ++++++++++++++++++++++++- frontend/src/tests/services/api.test.js | 2 + 3 files changed, 75 insertions(+), 3 deletions(-) diff --git a/frontend/src/components/ReportTable.jsx b/frontend/src/components/ReportTable.jsx index 36d6b32..a1da999 100644 --- a/frontend/src/components/ReportTable.jsx +++ b/frontend/src/components/ReportTable.jsx @@ -70,7 +70,7 @@ const ReportTable = ({ data = [], type }) => { case 'Actions': return ( View diff --git a/frontend/src/services/utils/api.js b/frontend/src/services/utils/api.js index a7a289f..b878f06 100644 --- a/frontend/src/services/utils/api.js +++ b/frontend/src/services/utils/api.js @@ -402,6 +402,7 @@ const normalizeTaskStatus = (status) => { }; const normalizeAdminDashboardTasks = (tasks = {}) => { + const backlog = toSafeMetricValue(tasks.backlog, 0); const todo = toSafeMetricValue(tasks.todo, 0); const inProgress = toSafeMetricValue(tasks.in_progress, 0); const review = toSafeMetricValue(tasks.review, 0); @@ -409,9 +410,10 @@ const normalizeAdminDashboardTasks = (tasks = {}) => { return { ...tasks, - total: toSafeMetricValue(tasks.total, todo + inProgress + review + done), + total: toSafeMetricValue(tasks.total, backlog + todo + inProgress + review + done), active: todo + inProgress + review, completed: done, + backlog, todo, in_progress: inProgress, review, @@ -834,6 +836,73 @@ const notificationService = { } }; + +// Report service +const reportService = { + saveReport: async (reportType, dateRange, summary, details) => { + try { + const response = await fetchWithAuth('reports', { + method: 'POST', + body: JSON.stringify({ + report_type: reportType, + date_range: dateRange, + summary, + details + }) + }); + return response; + } catch (error) { + console.error('Failed to save report:', error); + return { error: error.message }; + } + }, + + getSavedReports: async (filter = {}) => { + try { + let endpoint = 'reports'; + const params = new URLSearchParams(); + + if (filter.type) params.append('type', filter.type); + if (filter.dateRange) params.append('dateRange', filter.dateRange); + if (filter.page) params.append('page', filter.page); + if (filter.per_page) params.append('per_page', filter.per_page); + + const queryString = params.toString(); + if (queryString) { + endpoint = `${endpoint}?${queryString}`; + } + + const response = await fetchWithAuth(endpoint); + return response; + } catch (error) { + console.error('Failed to fetch saved reports:', error); + return { error: error.message, reports: [] }; + } + }, + + getReportById: async (reportId) => { + try { + const response = await fetchWithAuth(`reports/${reportId}`); + return response; + } catch (error) { + console.error(`Failed to fetch report ${reportId}:`, error); + return { error: error.message }; + } + }, + + deleteReport: async (reportId) => { + try { + const response = await fetchWithAuth(`reports/${reportId}`, { + method: 'DELETE' + }); + return response; + } catch (error) { + console.error(`Failed to delete report ${reportId}:`, error); + return { error: error.message }; + } + } +}; + export { fetchWithAuth, taskService, @@ -841,5 +910,6 @@ export { githubService, userService, dashboardService, - notificationService + notificationService, + reportService }; diff --git a/frontend/src/tests/services/api.test.js b/frontend/src/tests/services/api.test.js index 9532de8..d002022 100644 --- a/frontend/src/tests/services/api.test.js +++ b/frontend/src/tests/services/api.test.js @@ -483,6 +483,7 @@ describe('api utilities', () => { projects: { total: 3 }, tasks: { total: 9, + backlog: 1, todo: 2, in_progress: 3, review: 1, @@ -498,6 +499,7 @@ describe('api utilities', () => { expect(stats.tasks.active).toBe(6); expect(stats.tasks.completed).toBe(3); expect(stats.tasks.total).toBe(9); + expect(stats.tasks.backlog).toBe(1); }); test('dashboardService.getBasicDashboardStats returns fallback on error', async () => { From 173b257868271b811d2378f8237f3017b59acc45 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 00:04:36 +0100 Subject: [PATCH 04/37] refactor: enhance Team Lead permissions to include task updates and deletions; update RBAC documentation and adjust TaskList header --- backend/src/auth/rbac.py | 2 ++ docs/backend/rbac.md | 3 ++- frontend/src/pages/TaskList.jsx | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/backend/src/auth/rbac.py b/backend/src/auth/rbac.py index 18be141..179e712 100644 --- a/backend/src/auth/rbac.py +++ b/backend/src/auth/rbac.py @@ -30,6 +30,8 @@ class Role(Enum): *DEVELOPER_PERMISSIONS, 'can_create_tasks', 'can_assign_tasks', + 'can_update_any_task', + 'can_delete_tasks', 'can_view_team_metrics', 'can_view_team_reports', 'can_generate_reports', diff --git a/docs/backend/rbac.md b/docs/backend/rbac.md index ad6e959..b52c9e1 100644 --- a/docs/backend/rbac.md +++ b/docs/backend/rbac.md @@ -27,6 +27,8 @@ All Developer permissions, plus: - Create new tasks - Assign tasks to team members +- Update any task +- Delete any task - View team statistics and metrics - Generate reports - View all team member profiles @@ -35,7 +37,6 @@ All Developer permissions, plus: All Team Lead permissions, plus: -- Delete tasks - Manage users (create, update, delete) - Modify system settings - View audit logs diff --git a/frontend/src/pages/TaskList.jsx b/frontend/src/pages/TaskList.jsx index 33c9fc4..e0de0f6 100644 --- a/frontend/src/pages/TaskList.jsx +++ b/frontend/src/pages/TaskList.jsx @@ -140,7 +140,7 @@ const TaskList = () => {
-

Your Tasks

+

Tasks

+ ) : ( +

Only the assignee can link GitHub items.

)}
diff --git a/frontend/src/pages/TaskList.jsx b/frontend/src/pages/TaskList.jsx index e0de0f6..429dbec 100644 --- a/frontend/src/pages/TaskList.jsx +++ b/frontend/src/pages/TaskList.jsx @@ -5,6 +5,8 @@ import { useAuth } from '../context/AuthContext'; import LoadingSpinner from '../components/LoadingSpinner'; const TaskList = () => { + const navigate = useNavigate(); + const { currentUser } = useAuth(); const [tasks, setTasks] = useState([]); const [loading, setLoading] = useState(true); const [updating, setUpdating] = useState(false); @@ -12,11 +14,10 @@ const TaskList = () => { const [filters, setFilters] = useState({ status: 'all', priority: 'all', - search: '' + search: '', + scope: 'all' }); - const navigate = useNavigate(); - const { currentUser } = useAuth(); const canCreateTasks = currentUser?.role === 'admin' || currentUser?.role === 'team_lead'; // Fetch tasks when component mounts @@ -124,6 +125,11 @@ const TaskList = () => { return false; } + // Filter by scope (My Tasks vs All Tasks) + if (filters.scope === 'my' && task.assigned_to !== currentUser?.id) { + return false; + } + return true; }); @@ -136,9 +142,9 @@ const TaskList = () => { } return ( -
-
-
+
+
+

Tasks

@@ -235,6 +241,32 @@ const TaskList = () => { />
+ + {/* Scope Toggle for Developers */} +
+
+ + +
+
{/* Task List */} {filteredTasks.length === 0 ? ( @@ -346,7 +378,7 @@ const TaskList = () => { e.stopPropagation(); handleUpdateStatus(task.id, e.target.value); }} - disabled={updating} + disabled={updating || (currentUser.role === 'developer' && task.assigned_to !== currentUser.id)} className="text-sm border-slate-700/60 rounded-md bg-slate-950/60 text-slate-100 focus:outline-none focus:ring-rose-400/60 focus:border-rose-400/60 mr-2" > diff --git a/frontend/src/tests/pages/TaskDetailsUser.test.jsx b/frontend/src/tests/pages/TaskDetailsUser.test.jsx index 3f562d9..0f086f6 100644 --- a/frontend/src/tests/pages/TaskDetailsUser.test.jsx +++ b/frontend/src/tests/pages/TaskDetailsUser.test.jsx @@ -56,6 +56,7 @@ describe('TaskDetailsUser page', () => { progress: 40, priority: 'high', assignee_name: 'Dev User', + assigned_to: 5, created_at: '2099-01-01T00:00:00.000Z', deadline: '2099-02-01T00:00:00.000Z', github_links: [], @@ -357,6 +358,7 @@ describe('TaskDetailsUser page', () => { test('Connect GitHub account link shown when no repos available', async () => { taskService.getTaskComments.mockResolvedValue([]); githubService.getUserRepos.mockResolvedValue([]); + useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer', name: 'Dev User', email: 'dev@example.com' } }); render(); expect(await screen.findByText(/Connect GitHub Account/i)).toBeInTheDocument(); @@ -407,8 +409,8 @@ describe('TaskDetailsUser page', () => { expect(await screen.findByText('Updated notifications task')).toBeInTheDocument(); }); - test('hides edit task button for non-admin and non-team-lead users', async () => { - useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer', name: 'Dev User', email: 'dev@example.com' } }); + test('hides edit task button for non-admin and non-team-lead users who are not assigned', async () => { + useAuth.mockReturnValue({ currentUser: { id: 999, role: 'developer', name: 'Other User', email: 'other@example.com' } }); render(); diff --git a/frontend/src/tests/pages/TaskList.test.jsx b/frontend/src/tests/pages/TaskList.test.jsx index 809597f..08a184d 100644 --- a/frontend/src/tests/pages/TaskList.test.jsx +++ b/frontend/src/tests/pages/TaskList.test.jsx @@ -49,6 +49,7 @@ describe('TaskList page', () => { status: 'todo', priority: 'high', progress: 10, + assigned_to: 5, deadline: '2099-02-01T00:00:00.000Z', }, { @@ -58,6 +59,7 @@ describe('TaskList page', () => { status: 'in_progress', priority: 'medium', progress: 60, + assigned_to: 5, deadline: '2099-02-03T00:00:00.000Z', }, ]); @@ -131,6 +133,7 @@ describe('TaskList page', () => { status: 'in_progress', priority: 'high', progress: 20, + assigned_to: 5, deadline: '2000-01-01T00:00:00.000Z', // past deadline }, { @@ -140,6 +143,7 @@ describe('TaskList page', () => { status: 'completed', priority: 'low', progress: 100, + assigned_to: 5, deadline: '2000-01-01T00:00:00.000Z', // past but completed — no overdue badge }, ]); @@ -159,6 +163,7 @@ describe('TaskList page', () => { status: 'todo', priority: 'medium', progress: 0, + assigned_to: 5, deadline: null, }, ]); @@ -173,10 +178,10 @@ describe('TaskList page', () => { test('renders all priority badge variants and progress colour thresholds', async () => { useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer' } }); taskService.getAllTasks.mockResolvedValue([ - { id: 1, title: 'High P', status: 'todo', priority: 'high', progress: 100, deadline: null }, - { id: 2, title: 'Med P', status: 'in_progress', priority: 'medium', progress: 60, deadline: null }, - { id: 3, title: 'Low P', status: 'review', priority: 'low', progress: 25, deadline: null }, - { id: 4, title: 'Unknown P',status: 'backlog', priority: 'other', progress: 0, deadline: null }, + { id: 1, title: 'High P', status: 'todo', priority: 'high', progress: 100, assigned_to: 5, deadline: null }, + { id: 2, title: 'Med P', status: 'in_progress', priority: 'medium', progress: 60, assigned_to: 5, deadline: null }, + { id: 3, title: 'Low P', status: 'review', priority: 'low', progress: 25, assigned_to: 5, deadline: null }, + { id: 4, title: 'Unknown P',status: 'backlog', priority: 'other', progress: 0, assigned_to: 5, deadline: null }, ]); render(); @@ -196,11 +201,11 @@ describe('TaskList page', () => { test('renders all status badge variants including unknown', async () => { useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer' } }); taskService.getAllTasks.mockResolvedValue([ - { id: 1, title: 'T1', status: 'todo', priority: 'low', progress: 0, deadline: null }, - { id: 2, title: 'T2', status: 'backlog', priority: 'low', progress: 0, deadline: null }, - { id: 3, title: 'T3', status: 'review', priority: 'low', progress: 0, deadline: null }, - { id: 4, title: 'T4', status: 'completed', priority: 'low', progress: 0, deadline: null }, - { id: 5, title: 'T5', status: 'new_status', priority: 'low', progress: 0, deadline: null }, + { id: 1, title: 'T1', status: 'todo', priority: 'low', progress: 0, assigned_to: 5, deadline: null }, + { id: 2, title: 'T2', status: 'backlog', priority: 'low', progress: 0, assigned_to: 5, deadline: null }, + { id: 3, title: 'T3', status: 'review', priority: 'low', progress: 0, assigned_to: 5, deadline: null }, + { id: 4, title: 'T4', status: 'completed', priority: 'low', progress: 0, assigned_to: 5, deadline: null }, + { id: 5, title: 'T5', status: 'new_status', priority: 'low', progress: 0, assigned_to: 5, deadline: null }, ]); render(); @@ -215,7 +220,7 @@ describe('TaskList page', () => { test('clear-filters button is shown when a filter is active, hidden when none', async () => { useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer' } }); taskService.getAllTasks.mockResolvedValue([ - { id: 1, title: 'Alpha', status: 'todo', priority: 'high', progress: 0, deadline: null }, + { id: 1, title: 'Alpha', status: 'todo', priority: 'high', progress: 0, assigned_to: 5, deadline: null }, ]); render(); @@ -234,7 +239,7 @@ describe('TaskList page', () => { test('updating task status fails and shows update error', async () => { useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer' } }); taskService.getAllTasks.mockResolvedValue([ - { id: 1, title: 'Failing Task', status: 'todo', priority: 'medium', progress: 0, deadline: null }, + { id: 1, title: 'Failing Task', status: 'todo', priority: 'medium', progress: 0, assigned_to: 5, deadline: null }, ]); taskService.updateTask.mockRejectedValue(new Error('update failed')); @@ -265,7 +270,7 @@ describe('TaskList page', () => { test('clicking a task row navigates to task detail', async () => { useAuth.mockReturnValue({ currentUser: { id: 5, role: 'developer' } }); taskService.getAllTasks.mockResolvedValue([ - { id: 7, title: 'Clickable Task', status: 'todo', priority: 'low', progress: 0, deadline: null }, + { id: 7, title: 'Clickable Task', status: 'todo', priority: 'low', progress: 0, assigned_to: 5, deadline: null }, ]); render(); From 1a11507b0ec9e97b10ba004240e2b36f6a6fa26f Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:39:15 +0100 Subject: [PATCH 10/37] feat: implement audit log management with API routes and frontend integration --- .../src/api/controllers/audit_controller.py | 68 ++++++++ backend/src/api/routes/audit_routes.py | 23 +++ .../integration/test_audit_logs_routes.py | 105 ++++++++++++ frontend/src/pages/AdminAuditLogs.jsx | 161 ++++++++++++++++++ 4 files changed, 357 insertions(+) create mode 100644 backend/src/api/controllers/audit_controller.py create mode 100644 backend/src/api/routes/audit_routes.py create mode 100644 backend/tests/integration/test_audit_logs_routes.py create mode 100644 frontend/src/pages/AdminAuditLogs.jsx diff --git a/backend/src/api/controllers/audit_controller.py b/backend/src/api/controllers/audit_controller.py new file mode 100644 index 0000000..8c996ea --- /dev/null +++ b/backend/src/api/controllers/audit_controller.py @@ -0,0 +1,68 @@ +"""Audit Log Controller""" + +from flask import request, jsonify +from ...db.models import AuditLog + +def get_audit_logs(): + """Get paginated and filtered audit logs""" + action = request.args.get('action') + actor_id = request.args.get('actor') + from_date = request.args.get('from') + to_date = request.args.get('to') + + page = request.args.get('page', 1, type=int) + per_page = request.args.get('per_page', 50, type=int) + + query = AuditLog.query + + if action: + query = query.filter(AuditLog.action.ilike(f'%{action}%')) + if actor_id: + query = query.filter_by(actor_user_id=actor_id) + if from_date: + query = query.filter(AuditLog.created_at >= from_date) + if to_date: + query = query.filter(AuditLog.created_at <= to_date) + + pagination = query.order_by(AuditLog.created_at.desc()).paginate( + page=page, per_page=per_page, error_out=False + ) + + logs_data = [{ + 'id': log.id, + 'actor_user_id': log.actor_user_id, + 'actor_role': log.actor_role, + 'action': log.action, + 'resource_type': log.resource_type, + 'resource_id': log.resource_id, + 'ip': log.ip, + 'user_agent': log.user_agent, + 'metadata': log.metadata_info, + 'created_at': log.created_at.isoformat() if log.created_at else None + } for log in pagination.items] + + return jsonify({ + 'logs': logs_data, + 'total': pagination.total, + 'pages': pagination.pages, + 'current_page': page + }) + +def get_audit_log_by_id(log_id): + """Get a specific audit log""" + log = AuditLog.query.get_or_404(log_id) + + return jsonify({ + 'log': { + 'id': log.id, + 'actor_user_id': log.actor_user_id, + 'actor_role': log.actor_role, + 'action': log.action, + 'resource_type': log.resource_type, + 'resource_id': log.resource_id, + 'ip': log.ip, + 'user_agent': log.user_agent, + 'metadata': log.metadata_info, + 'created_at': log.created_at.isoformat() if log.created_at else None + } + }) diff --git a/backend/src/api/routes/audit_routes.py b/backend/src/api/routes/audit_routes.py new file mode 100644 index 0000000..f892134 --- /dev/null +++ b/backend/src/api/routes/audit_routes.py @@ -0,0 +1,23 @@ +"""Audit Log API routes""" + +from flask import request +from flask_jwt_extended import jwt_required +from ..controllers.audit_controller import get_audit_logs, get_audit_log_by_id +from ..middlewares import admin_required + +def register_routes(bp): + """Register all audit routes with the provided Blueprint""" + + @bp.route('/admin/audit-logs', methods=['GET']) + @jwt_required() + @admin_required() + def get_audit_logs_route(): + """Route to get all audit logs (Admin only)""" + return get_audit_logs() + + @bp.route('/admin/audit-logs/', methods=['GET']) + @jwt_required() + @admin_required() + def get_audit_log_route(log_id): + """Route to get a single audit log (Admin only)""" + return get_audit_log_by_id(log_id) diff --git a/backend/tests/integration/test_audit_logs_routes.py b/backend/tests/integration/test_audit_logs_routes.py new file mode 100644 index 0000000..994ccb6 --- /dev/null +++ b/backend/tests/integration/test_audit_logs_routes.py @@ -0,0 +1,105 @@ +"""Tests for audit log route access control, filters, and pagination.""" +import os +import sys + +import pytest +from unittest.mock import MagicMock +from flask_jwt_extended import create_access_token + +sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '../..'))) + +from src.app import create_app +from src.api.routes import audit_routes + + +@pytest.fixture +def app_and_socket(monkeypatch): + monkeypatch.setenv('FLASK_ENV', 'testing') + app, socketio = create_app({ + 'TESTING': True, + 'SQLALCHEMY_DATABASE_URI': 'sqlite:///:memory:', + 'JWT_SECRET_KEY': 'test-secret-key-audit-logs-32chars', + 'JWT_COOKIE_SECURE': False, + 'JWT_COOKIE_SAMESITE': 'Lax', + }) + return app, socketio + + +@pytest.fixture +def app(app_and_socket): + app, _ = app_and_socket + return app + + +@pytest.fixture +def client(app): + return app.test_client() + + +def auth_headers(app, role, user_id=1): + with app.app_context(): + token = create_access_token( + identity={'user_id': user_id}, + additional_claims={'role': role} + ) + return {'Authorization': f'Bearer {token}'} + + +def test_audit_logs_requires_auth(client): + """Unauthenticated requests must be rejected.""" + resp = client.get('/api/v1/admin/audit-logs') + assert resp.status_code == 401 + + +def test_audit_logs_requires_admin(client, app): + """Developer and Team Lead must be denied.""" + for role in ('developer', 'team_lead'): + resp = client.get( + '/api/v1/admin/audit-logs', + headers=auth_headers(app, role) + ) + assert resp.status_code == 403, f'{role} should be denied' + + +def test_audit_logs_admin_allowed(client, app, monkeypatch): + """Admin should reach the handler.""" + handler = MagicMock(return_value=({ + 'logs': [], + 'total': 0, + 'pages': 0, + 'current_page': 1 + }, 200)) + monkeypatch.setattr(audit_routes, 'get_audit_logs', handler) + + resp = client.get( + '/api/v1/admin/audit-logs', + headers=auth_headers(app, 'admin') + ) + assert resp.status_code == 200 + data = resp.get_json() + assert 'logs' in data + handler.assert_called_once() + + +def test_audit_log_detail_requires_admin(client, app): + """GET /admin/audit-logs/ must require admin.""" + resp = client.get( + '/api/v1/admin/audit-logs/1', + headers=auth_headers(app, 'developer') + ) + assert resp.status_code == 403 + + +def test_audit_log_detail_admin_allowed(client, app, monkeypatch): + """Admin should be able to fetch a single log.""" + handler = MagicMock(return_value=({ + 'log': {'id': 1, 'action': 'user_login'} + }, 200)) + monkeypatch.setattr(audit_routes, 'get_audit_log_by_id', handler) + + resp = client.get( + '/api/v1/admin/audit-logs/1', + headers=auth_headers(app, 'admin') + ) + assert resp.status_code == 200 + handler.assert_called_once_with(1) diff --git a/frontend/src/pages/AdminAuditLogs.jsx b/frontend/src/pages/AdminAuditLogs.jsx new file mode 100644 index 0000000..f91810c --- /dev/null +++ b/frontend/src/pages/AdminAuditLogs.jsx @@ -0,0 +1,161 @@ +import { useState, useEffect, useCallback } from 'react'; +import { auditLogService } from '../services/utils/api'; + +const AdminAuditLogs = () => { + const [logs, setLogs] = useState([]); + const [total, setTotal] = useState(0); + const [pages, setPages] = useState(0); + const [page, setPage] = useState(1); + const [loading, setLoading] = useState(true); + const [actionFilter, setActionFilter] = useState(''); + const [actorFilter, setActorFilter] = useState(''); + const [selectedLog, setSelectedLog] = useState(null); + + const perPage = 25; + + const fetchLogs = useCallback(async () => { + setLoading(true); + try { + const filters = { page, per_page: perPage }; + if (actionFilter) filters.action = actionFilter; + if (actorFilter) filters.actor = actorFilter; + const data = await auditLogService.getLogs(filters); + setLogs(data.logs || []); + setTotal(data.total || 0); + setPages(data.pages || 0); + } catch (err) { + console.error('Failed to fetch audit logs', err); + } finally { + setLoading(false); + } + }, [page, actionFilter, actorFilter]); + + useEffect(() => { fetchLogs(); }, [fetchLogs]); + + const handleViewDetail = async (logId) => { + const detail = await auditLogService.getLogById(logId); + setSelectedLog(detail); + }; + + const actionBadge = (action) => { + if (action?.includes('login')) return 'bg-blue-500/20 text-blue-300'; + if (action?.includes('register')) return 'bg-emerald-500/20 text-emerald-300'; + if (action?.includes('delete')) return 'bg-rose-500/20 text-rose-300'; + if (action?.includes('role')) return 'bg-amber-500/20 text-amber-300'; + return 'bg-slate-500/20 text-slate-300'; + }; + + return ( +
+
+

Audit Logs

+ + {/* Filters */} +
+ { setActionFilter(e.target.value); setPage(1); }} + className="flex-1 rounded-lg border border-slate-700/60 bg-slate-900/80 py-2 px-3 text-slate-100 placeholder:text-slate-500 focus:outline-none focus:ring-2 focus:ring-rose-400/60" /> + { setActorFilter(e.target.value); setPage(1); }} + className="w-40 rounded-lg border border-slate-700/60 bg-slate-900/80 py-2 px-3 text-slate-100 placeholder:text-slate-500 focus:outline-none focus:ring-2 focus:ring-rose-400/60" /> +
+ + {/* Table */} + {loading ? ( +
Loading audit logs...
+ ) : ( + <> +
+ + + + + + + + + + + + {logs.map((log) => ( + + + + + + + + ))} + {logs.length === 0 && ( + + )} + +
TimeActionActorResourceDetails
+ {log.created_at ? new Date(log.created_at).toLocaleString() : '—'} + + + {log.action} + + + {log.actor_user_id ? `User ${log.actor_user_id}` : 'System'} + {log.actor_role && ({log.actor_role})} + + {log.resource_type || '—'}{log.resource_id ? ` #${log.resource_id}` : ''} + + +
No audit logs found.
+
+ + {/* Pagination */} +
+ Total: {total} entries +
+ + Page {page} of {pages || 1} + +
+
+ + )} + + {/* Detail Drawer */} + {selectedLog && ( +
+
+

Audit Log Detail

+
+ {[['ID', selectedLog.id], ['Action', selectedLog.action], ['Actor', selectedLog.actor_user_id], + ['Role', selectedLog.actor_role], ['Resource', `${selectedLog.resource_type || ''} ${selectedLog.resource_id || ''}`], + ['IP', selectedLog.ip], ['User Agent', selectedLog.user_agent], + ['Timestamp', selectedLog.created_at ? new Date(selectedLog.created_at).toLocaleString() : '—'] + ].map(([label, val]) => ( +
+
{label}
+
{val || '—'}
+
+ ))} + {selectedLog.metadata && ( +
+
Metadata
+ 
{JSON.stringify(selectedLog.metadata, null, 2)}
+
+ )} +
+
+ +
+
+
+ )} +
+
+ ); +}; + +export default AdminAuditLogs; From c51f99c634123d805c12d43a52c17b568de995af Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:39:35 +0100 Subject: [PATCH 11/37] refactor: enhance GitHub integration routes with role-based access control and update frontend styling --- backend/src/api/routes/github_routes.py | 4 ++++ backend/tests/unit/routes/test_github_routes.py | 5 ++++- frontend/src/pages/GitHubIntegration.jsx | 8 ++++---- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/backend/src/api/routes/github_routes.py b/backend/src/api/routes/github_routes.py index 258b8d0..174a337 100644 --- a/backend/src/api/routes/github_routes.py +++ b/backend/src/api/routes/github_routes.py @@ -20,6 +20,8 @@ delete_task_github_link, ) from ..middlewares.validation_middleware import validate_json +from ..middlewares import role_required +from ...auth.rbac import Role, require_permission from ...db.models import db, User, GitHubToken from ...services.github_client import GitHubClient @@ -133,6 +135,8 @@ def repositories_list(): @bp.route('/github/repositories', methods=['POST']) @jwt_required() + @role_required([Role.ADMIN]) + @require_permission('can_link_github_repos') @validate_json() def add_repository(): """Route to add a GitHub repository for tracking""" diff --git a/backend/tests/unit/routes/test_github_routes.py b/backend/tests/unit/routes/test_github_routes.py index ad6c5f1..f865ddd 100644 --- a/backend/tests/unit/routes/test_github_routes.py +++ b/backend/tests/unit/routes/test_github_routes.py @@ -21,7 +21,10 @@ def app(monkeypatch): app.config['JWT_SECRET_KEY'] = 'jwt-secret-key' monkeypatch.setattr(github_routes, 'jwt_required', passthrough_decorator) - monkeypatch.setattr(github_routes, 'validate_json', passthrough_decorator) + monkeypatch.setattr(github_routes, 'validate_json', passthrough_decorator, raising=False) + monkeypatch.setattr(github_routes, 'admin_required', passthrough_decorator, raising=False) + monkeypatch.setattr(github_routes, 'role_required', passthrough_decorator, raising=False) + monkeypatch.setattr(github_routes, 'require_permission', passthrough_decorator, raising=False) bp = Blueprint('api', __name__, url_prefix='/api/v1') github_routes.register_routes(bp) diff --git a/frontend/src/pages/GitHubIntegration.jsx b/frontend/src/pages/GitHubIntegration.jsx index 4478940..aa625a9 100644 --- a/frontend/src/pages/GitHubIntegration.jsx +++ b/frontend/src/pages/GitHubIntegration.jsx @@ -331,10 +331,10 @@ const GitHubIntegration = () => { } return ( -
-
-
-

GitHub Integration

+
+
+
+

GitHub Integration

{connectionStatus.rateLimitError ? renderRateLimitError() : connectionStatus.error && (
From 0526b8465568af834c65d7cc1c0c031db5a36481 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:44:15 +0100 Subject: [PATCH 12/37] refactor: enhance authentication validation and role management; add Forbidden page and RBAC utilities --- backend/src/api/validators/auth_validator.py | 26 +-- backend/src/auth/auth.py | 189 +++++------------- backend/src/auth/rbac.py | 65 +++++- .../test_role_access_integration.py | 30 ++- frontend/src/pages/Forbidden.jsx | 30 +++ frontend/src/utils/rbac.js | 49 +++++ 6 files changed, 218 insertions(+), 171 deletions(-) create mode 100644 frontend/src/pages/Forbidden.jsx create mode 100644 frontend/src/utils/rbac.js diff --git a/backend/src/api/validators/auth_validator.py b/backend/src/api/validators/auth_validator.py index 4a2c37c..d719751 100644 --- a/backend/src/api/validators/auth_validator.py +++ b/backend/src/api/validators/auth_validator.py @@ -9,29 +9,29 @@ def validate_login_data(data): if not all(k in data for k in ['email', 'password']): return jsonify({'message': 'Email and password required'}), 400 - # Validate email format - email_pattern = r'^[\w\.-]+@[\w\.-]+\.\w+$' + # Relaxed email pattern to support plus signs, subdomains, etc. + email_pattern = r'^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$' if not re.match(email_pattern, data['email']): return jsonify({'message': 'Invalid email format'}), 400 return None def validate_registration_data(data): - """Validate user registration data""" - if not all(k in data for k in ['name', 'email', 'password', 'role']): + """Validate user registration data. + + Note: the ``role`` field is **not** required. The backend always forces + new registrations to the ``developer`` role for security. + """ + if not all(k in data for k in ['name', 'email', 'password']): return jsonify({'message': 'Missing required fields'}), 400 - - # Validate email format - email_pattern = r'^[\w\.-]+@[\w\.-]+\.\w+$' + + # Relaxed email pattern to support plus signs, subdomains, etc. + email_pattern = r'^[a-zA-Z0-9_.+-]+@[a-zA-Z0-9-]+\.[a-zA-Z0-9-.]+$' if not re.match(email_pattern, data['email']): return jsonify({'message': 'Invalid email format'}), 400 - + # Validate password length if len(data['password']) < 8: return jsonify({'message': 'Password must be at least 8 characters long'}), 400 - - valid_roles = [role.value for role in Role] - if data['role'] not in valid_roles: - return jsonify({'message': f'Role must be one of: {", ".join(valid_roles)}'}), 400 - + return None diff --git a/backend/src/auth/auth.py b/backend/src/auth/auth.py index 87606fb..a93c968 100644 --- a/backend/src/auth/auth.py +++ b/backend/src/auth/auth.py @@ -4,55 +4,67 @@ from flask_jwt_extended import ( jwt_required, get_jwt_identity, get_jwt, set_access_cookies, set_refresh_cookies, unset_jwt_cookies, - create_access_token # Added missing import + create_access_token ) from sqlalchemy.exc import IntegrityError from ..db.models import db, User # Fix import path from .helpers import hash_password, verify_password, generate_tokens from .rbac import Role +from ..services import audit_service, settings_service -auth_bp = Blueprint('auth', __name__) +def register_user(): + """Function to register a new user. -def _validate_role(role): - valid_roles = [item.value for item in Role] - if role not in valid_roles: - return jsonify({'message': f'Role must be one of: {", ".join(valid_roles)}'}), 400 - return None - -@auth_bp.route('/register', methods=['POST']) -def register(): + Security: the role field from the request body is **ignored**. + All new accounts are created with the 'developer' role. + Admins can promote users after registration via PUT /admin/users//role. + """ data = request.get_json() - - # Validate required fields - if not all(k in data for k in ['name', 'email', 'password', 'role']): + + # Validate required fields (role is no longer required from the client) + if not all(k in data for k in ['name', 'email', 'password']): return jsonify({'message': 'Missing required fields'}), 400 - role_validation = _validate_role(data['role']) - if role_validation: - return role_validation - # Check if email already exists existing_user = User.query.filter_by(email=data['email']).first() if existing_user: return jsonify({'message': 'Email already registered'}), 409 - - # Create new user + + # If this is the very first user, automatically make them an admin + user_count = User.query.count() + if user_count == 0: + forced_role = Role.ADMIN.value + print(f"First user registration detected! Automatically granting admin role to {data['email']}") + else: + # Otherwise get default role from settings + forced_role = settings_service.get_default_role() + + print(f"Registering user: {data['email']} with role: {forced_role} (ignoring any client-supplied role)") + try: new_user = User( name=data['name'], email=data['email'], password=hash_password(data['password']), - role=data['role'] + role=forced_role ) - + db.session.add(new_user) db.session.commit() - + + # Record audit log + audit_service.record( + action='user_registered', + actor={'user_id': new_user.id, 'role': new_user.role}, + resource_type='user', + resource_id=new_user.id + ) + # Generate tokens for the new user tokens = generate_tokens(new_user.id, {'role': new_user.role}) - + # Create response with tokens resp = jsonify({ 'message': 'User registered successfully', @@ -60,21 +72,22 @@ def register(): 'id': new_user.id, 'name': new_user.name, 'email': new_user.email, - 'role': new_user.role + 'role': new_user.role, + 'token': tokens['access_token'] } }) - + # Set cookies set_access_cookies(resp, tokens['access_token']) set_refresh_cookies(resp, tokens['refresh_token']) - + return resp, 201 - - except IntegrityError: + + except Exception as e: db.session.rollback() - return jsonify({'message': 'An error occurred while registering the user'}), 500 + print(f"Registration error: {str(e)}") + return jsonify({'message': f'An error occurred while registering the user: {str(e)}'}), 500 -@auth_bp.route('/login', methods=['POST']) def login(): """Function to authenticate a user and create a session""" data = request.get_json() @@ -106,6 +119,14 @@ def login(): github_connected = github_token is not None github_username = user.github_username + # Record audit log + audit_service.record( + action='user_login', + actor={'user_id': user.id, 'role': user.role}, + resource_type='user', + resource_id=user.id + ) + # Create response resp = jsonify({ 'message': 'Login successful', @@ -126,114 +147,6 @@ def login(): return resp -@auth_bp.route('/refresh', methods=['POST']) -@jwt_required(refresh=True) -def refresh(): - """Endpoint for refreshing an access token""" - current_user = get_jwt_identity() - - # Create new access token - access_token = create_access_token(identity=current_user) - - # Create response - resp = jsonify({ - 'message': 'Token refreshed successfully', - 'token': access_token - }) - - # Set new access cookie - set_access_cookies(resp, access_token) - - return resp - -@auth_bp.route('/logout', methods=['POST']) -def logout(): - """Endpoint for logging out a user""" - resp = jsonify({'message': 'Logout successful'}) - - # Remove JWT cookies - unset_jwt_cookies(resp) - - return resp - -@auth_bp.route('/me', methods=['GET']) -@jwt_required() -def me(): - """Get current user information""" - current_user = get_jwt_identity() - - user = User.query.get(current_user['user_id']) - if not user: - return jsonify({'message': 'User not found'}), 404 - - return jsonify({ - 'user': { - 'id': user.id, - 'name': user.name, - 'email': user.email, - 'role': user.role - } - }) - -def register_user(): - """Function to register a new user""" - data = request.get_json() - - # Validate required fields - if not all(k in data for k in ['name', 'email', 'password', 'role']): - return jsonify({'message': 'Missing required fields'}), 400 - - role_validation = _validate_role(data['role']) - if role_validation: - return role_validation - - # Check if email already exists - existing_user = User.query.filter_by(email=data['email']).first() - if existing_user: - return jsonify({'message': 'Email already registered'}), 409 - - # Create new user - try: - # Print debug information - print(f"Attempting to register user: {data['email']} with role: {data['role']}") - - new_user = User( - name=data['name'], - email=data['email'], - password=hash_password(data['password']), - role=data['role'] - ) - - db.session.add(new_user) - db.session.commit() - - # Generate tokens for the new user - tokens = generate_tokens(new_user.id, {'role': new_user.role}) - - # Create response with tokens - resp = jsonify({ - 'message': 'User registered successfully', - 'user': { - 'id': new_user.id, - 'name': new_user.name, - 'email': new_user.email, - 'role': new_user.role, - 'token': tokens['access_token'] # Include token in response for frontend - } - }) - - # Set cookies - set_access_cookies(resp, tokens['access_token']) - set_refresh_cookies(resp, tokens['refresh_token']) - - return resp, 201 - - except Exception as e: - # Enhanced error handling to catch all exceptions - db.session.rollback() - print(f"Registration error: {str(e)}") - return jsonify({'message': f'An error occurred while registering the user: {str(e)}'}), 500 - def refresh_token(): """Function to refresh an access token""" current_user = get_jwt_identity() diff --git a/backend/src/auth/rbac.py b/backend/src/auth/rbac.py index 179e712..8a97b92 100644 --- a/backend/src/auth/rbac.py +++ b/backend/src/auth/rbac.py @@ -2,7 +2,7 @@ from enum import Enum from functools import wraps -from flask_jwt_extended import get_jwt +from flask_jwt_extended import get_jwt, get_jwt_identity from flask import jsonify class Role(Enum): @@ -15,15 +15,12 @@ class Role(Enum): DEVELOPER_PERMISSIONS = [ 'can_view_tasks', 'can_update_assigned_tasks', - 'can_comment', 'can_comment_on_tasks', 'can_view_notifications', 'can_manage_personal_notifications', 'can_view_own_profile', 'can_update_own_profile', 'can_link_github_account', - 'can_view_github_repos', - 'can_link_github_commits', ] TEAM_LEAD_PERMISSIONS = [ @@ -31,21 +28,17 @@ class Role(Enum): 'can_create_tasks', 'can_assign_tasks', 'can_update_any_task', - 'can_delete_tasks', - 'can_view_team_metrics', - 'can_view_team_reports', + 'can_view_all_users', + 'can_view_system_stats', 'can_generate_reports', - 'can_view_team_profiles', ] ADMIN_PERMISSIONS = [ *TEAM_LEAD_PERMISSIONS, - 'can_update_any_task', - 'can_delete_tasks', 'can_manage_users', + 'can_manage_projects', 'can_manage_system_settings', 'can_view_audit_logs', - 'can_access_all_data', 'can_link_github_repos', ] @@ -57,6 +50,33 @@ class Role(Enum): } +# --------------------------------------------------------------------------- +# Role hierarchy – higher value = more privileged +# --------------------------------------------------------------------------- +ROLE_HIERARCHY = { + Role.DEVELOPER: 0, + Role.TEAM_LEAD: 1, + Role.ADMIN: 2, +} + + +def _role_level(role_value): + """Return the numeric hierarchy level for a role string value.""" + for role_enum, level in ROLE_HIERARCHY.items(): + if role_enum.value == role_value: + return level + return -1 + + +def has_permission(role_value, permission): + """Check whether *role_value* (a string such as 'admin') grants *permission*.""" + return permission in ROLE_PERMISSIONS.get(role_value, []) + + +# --------------------------------------------------------------------------- +# Decorators +# --------------------------------------------------------------------------- + def require_role(role): """Decorator to require a specific role""" def decorator(fn): @@ -88,3 +108,26 @@ def wrapper(*args, **kwargs): return fn(*args, **kwargs) return wrapper return decorator + + +def role_at_least(min_role): + """Decorator that requires the caller's role to be *min_role* or higher. + + Example:: + + @role_at_least(Role.TEAM_LEAD) + def some_view(): ... + """ + min_role_enum = min_role if isinstance(min_role, Role) else Role(min_role) + min_level = ROLE_HIERARCHY[min_role_enum] + + def decorator(fn): + @wraps(fn) + def wrapper(*args, **kwargs): + claims = get_jwt() + user_role = claims.get('role') + if not user_role or _role_level(user_role) < min_level: + return jsonify({'message': 'Insufficient permissions'}), 403 + return fn(*args, **kwargs) + return wrapper + return decorator diff --git a/backend/tests/integration/test_role_access_integration.py b/backend/tests/integration/test_role_access_integration.py index 6931cf2..d3ab64a 100644 --- a/backend/tests/integration/test_role_access_integration.py +++ b/backend/tests/integration/test_role_access_integration.py @@ -55,7 +55,7 @@ def auth_headers(app, role, user_id=1): return {'Authorization': f'Bearer {token}'} -def test_users_route_requires_admin_role(client, app, monkeypatch): +def test_users_route_allows_developers(client, app, monkeypatch): handler = MagicMock(return_value=({'users': []}, 200)) monkeypatch.setattr(users_routes, 'get_all_users', handler) @@ -63,15 +63,22 @@ def test_users_route_requires_admin_role(client, app, monkeypatch): assert unauthorized_response.status_code == 401 assert handler.call_count == 0 - forbidden_response = client.get('/api/v1/users', headers=auth_headers(app, 'developer')) - assert forbidden_response.status_code == 403 - assert forbidden_response.get_json()['message'] == 'Insufficient permissions' - assert handler.call_count == 0 - + # Developer should be allowed + dev_response = client.get('/api/v1/users', headers=auth_headers(app, 'developer')) + assert dev_response.status_code == 200 + assert dev_response.get_json() == {'users': []} + assert handler.call_count == 1 + + # Team Lead should be allowed + team_lead_response = client.get('/api/v1/users', headers=auth_headers(app, 'team_lead')) + assert team_lead_response.status_code == 200 + assert team_lead_response.get_json() == {'users': []} + + # Admin should also be allowed (hierarchy) allowed_response = client.get('/api/v1/users', headers=auth_headers(app, 'admin')) assert allowed_response.status_code == 200 assert allowed_response.get_json() == {'users': []} - handler.assert_called_once_with() + assert handler.call_count == 3 def test_admin_stats_route_requires_admin_role(client, app, monkeypatch): @@ -84,13 +91,18 @@ def test_admin_stats_route_requires_admin_role(client, app, monkeypatch): forbidden_response = client.get('/api/v1/admin/stats', headers=auth_headers(app, 'developer')) assert forbidden_response.status_code == 403 - assert forbidden_response.get_json()['message'] == 'Admin access required' + assert forbidden_response.get_json()['message'] == 'Insufficient permissions' assert handler.call_count == 0 + # Team Lead should be allowed + team_lead_response = client.get('/api/v1/admin/stats', headers=auth_headers(app, 'team_lead')) + assert team_lead_response.status_code == 200 + assert team_lead_response.get_json()['users']['total'] == 5 + allowed_response = client.get('/api/v1/admin/stats', headers=auth_headers(app, 'admin')) assert allowed_response.status_code == 200 assert allowed_response.get_json()['users']['total'] == 5 - handler.assert_called_once_with() + assert handler.call_count == 2 def test_member_dashboard_route_requires_member_role(client, app, monkeypatch): diff --git a/frontend/src/pages/Forbidden.jsx b/frontend/src/pages/Forbidden.jsx new file mode 100644 index 0000000..91f5162 --- /dev/null +++ b/frontend/src/pages/Forbidden.jsx @@ -0,0 +1,30 @@ +import React from 'react'; +import { useNavigate } from 'react-router-dom'; + +const Forbidden = () => { + const navigate = useNavigate(); + + return ( +
+
+
+ + + +

Access Denied

+

+ You don't have the necessary permissions to access this page or perform this action. +

+ +
+
+
+ ); +}; + +export default Forbidden; diff --git a/frontend/src/utils/rbac.js b/frontend/src/utils/rbac.js new file mode 100644 index 0000000..d7e3d29 --- /dev/null +++ b/frontend/src/utils/rbac.js @@ -0,0 +1,49 @@ +// RBAC Constants and Helpers + +export const ROLES = { + DEVELOPER: 'developer', + TEAM_LEAD: 'team_lead', + ADMIN: 'admin' +}; + +export const ROLE_HIERARCHY = { + [ROLES.DEVELOPER]: 0, + [ROLES.TEAM_LEAD]: 1, + [ROLES.ADMIN]: 2 +}; + +// Available permissions matching backend +export const PERMISSIONS = { + CAN_MANAGE_PROJECTS: 'can_manage_projects', + CAN_ASSIGN_TASKS: 'can_assign_tasks', + CAN_UPDATE_ANY_TASK: 'can_update_any_task', + CAN_VIEW_ALL_USERS: 'can_view_all_users', + CAN_MANAGE_USERS: 'can_manage_users', + CAN_MANAGE_SYSTEM_SETTINGS: 'can_manage_system_settings', + CAN_VIEW_SYSTEM_STATS: 'can_view_system_stats', + CAN_LINK_GITHUB_ACCOUNT: 'can_link_github_account', + CAN_LINK_GITHUB_REPOS: 'can_link_github_repos', + CAN_COMMENT_ON_TASKS: 'can_comment_on_tasks', + CAN_MANAGE_PERSONAL_NOTIFICATIONS: 'can_manage_personal_notifications' +}; + +// We don't redefine the map here, the backend controls the actual map and we fetch it. +// These helpers just act on the user's role/permissions list. + +export const hasRole = (userRole, targetRole) => { + return userRole === targetRole; +}; + +export const hasAnyRole = (userRole, targetRoles) => { + return targetRoles.includes(userRole); +}; + +export const roleAtLeast = (userRole, minRole) => { + const userLevel = ROLE_HIERARCHY[userRole] ?? -1; + const minLevel = ROLE_HIERARCHY[minRole] ?? 0; + return userLevel >= minLevel; +}; + +export const hasPermission = (userPermissions, targetPermission) => { + return Array.isArray(userPermissions) && userPermissions.includes(targetPermission); +}; From 7f3207cae09a9f5764897e30707a5ddaaf722c03 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:44:35 +0100 Subject: [PATCH 13/37] refactor: remove role selection from Register component and update related tests --- frontend/src/pages/Register.jsx | 20 +------------------- frontend/src/tests/pages/Register.test.jsx | 4 ---- 2 files changed, 1 insertion(+), 23 deletions(-) diff --git a/frontend/src/pages/Register.jsx b/frontend/src/pages/Register.jsx index 048dc40..d6c2f09 100644 --- a/frontend/src/pages/Register.jsx +++ b/frontend/src/pages/Register.jsx @@ -19,7 +19,6 @@ const Register = () => { email: "", password: "", confirmPassword: "", - role: "developer", }); const [formError, setFormError] = useState(""); const [isSubmitting, setIsSubmitting] = useState(false); @@ -96,7 +95,6 @@ const Register = () => { email: "", password: "", confirmPassword: "", - role: "developer", }); // Redirect to login page after success @@ -112,7 +110,7 @@ const Register = () => { }; return ( -
+

DevSync

@@ -194,22 +192,6 @@ const Register = () => { />
-
- - -
-
+
+ Users + Settings + Audit Logs +
+
+ GitHub + + ) : isTeamLead ? ( + <> + Dashboard + Tasks + Projects + Create Task + Developer Progress + Reports + GitHub ) : ( <> - - Dashboard - - - Tasks - - {canCreateTasks && ( - - Create Task - - )} - - GitHub - + Dashboard + My Tasks + GitHub )} - - {/* Connection Status Indicator */} -
- - - {isConnected ? 'Connected' : 'Disconnected'} - -
- + + +
{/* Notification Bell */}
{showNotifications && ( -
-
- Notifications -
- -
- +
+
+

Notifications

+
- - {notifications.length > 0 && ( -
- -
- )} + setShowNotifications(false)} + onMarkRead={markAsRead} + onMarkAllRead={markAllAsRead} + />
)}
- - {/* User Menu */} -
- {currentUser.name || currentUser.email} + +
+
+
{currentUser.name}
+
{currentUser.role?.replace('_', ' ')}
+
- + {/* Mobile Menu Button */}
- + {/* Mobile Menu */} {showMobileMenu && ( -
- {isAdmin ? ( - <> - - Dashboard - - - Create Task - - - Developer Progress - - - Reports - - - GitHub - - - ) : ( - <> - - Dashboard - - - Tasks - - {canCreateTasks && ( - - Create Task - - )} - - GitHub - - - )} - +
+
+ {isAdmin ? ( + <> + Dashboard + Create Task + Developer Progress + Reports + Users + Settings + Audit Logs + GitHub + + ) : isTeamLead ? ( + <> + Dashboard + Tasks + {canCreateTasks && ( + Create Task + )} + Reports + GitHub + + ) : ( + <> + Dashboard + Tasks + GitHub + + )} +
+
+
+
+ {currentUser.name?.charAt(0)} +
+
{currentUser.name}
+
+ +
)} - + ); }; diff --git a/frontend/src/pages/AdminDashboard.jsx b/frontend/src/pages/AdminDashboard.jsx index aea0b17..80e7c80 100644 --- a/frontend/src/pages/AdminDashboard.jsx +++ b/frontend/src/pages/AdminDashboard.jsx @@ -1,48 +1,50 @@ import React, { useState, useEffect, useCallback } from 'react'; import { Link } from 'react-router-dom'; -import { dashboardService, projectService } from '../services/utils/api'; +import { dashboardService, projectService, userService } from '../services/utils/api'; import LoadingSpinner from '../components/LoadingSpinner'; +import { useAuth } from '../context/AuthContext'; // ─── Stat Card ──────────────────────────────────────────────────────────────── const StatCard = ({ title, value, change, icon, color, currentTimeRange }) => { - const iconClass = { - primary: 'bg-rose-500/10 text-rose-300 border border-rose-400/20', - secondary: 'bg-sky-500/10 text-sky-300 border border-sky-400/20', - success: 'bg-emerald-500/10 text-emerald-300 border border-emerald-400/20', - warning: 'bg-amber-500/10 text-amber-300 border border-amber-400/20', - error: 'bg-rose-500/10 text-rose-300 border border-rose-400/20', - }[color] ?? 'bg-slate-800 text-slate-300 border border-slate-700'; + const colorClasses = { + primary: 'bg-sky-500/15 text-sky-300 border border-sky-400/20', + secondary: 'bg-rose-500/15 text-rose-300 border border-rose-400/20', + success: 'bg-emerald-500/15 text-emerald-300 border border-emerald-400/20', + warning: 'bg-amber-500/15 text-amber-300 border border-amber-400/20', + error: 'bg-rose-500/15 text-rose-300 border border-rose-400/20', + purple: 'bg-purple-500/15 text-purple-300 border border-purple-400/20', + }; return ( -
-
-
+
+
+
{icon}
-
-

{title}

-

{value}

+
+

{title}

+

{value}

{change !== undefined && ( -

+

{change > 0 ? ( - - - + + + - {change}% increase + {change}% ) : change < 0 ? ( - - - + + + - {Math.abs(change)}% decrease + {Math.abs(change)}% ) : ( No change )} - since last {currentTimeRange} -

+ vs last {currentTimeRange} +
)}
@@ -73,7 +75,7 @@ const StatusBadge = ({ status }) => { // ─── Panel wrapper ───────────────────────────────────────────────────────────── const Panel = ({ children, className = '' }) => ( -
+
{children}
); @@ -91,7 +93,9 @@ const PanelHeader = ({ title, linkTo, linkLabel }) => ( // ─── Admin Dashboard ─────────────────────────────────────────────────────────── const AdminDashboard = () => { + const { currentUser } = useAuth(); const [dashboardData, setDashboardData] = useState(null); + const [teamUsers, setTeamUsers] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [timeRange, setTimeRange] = useState('week'); @@ -101,6 +105,15 @@ const AdminDashboard = () => { setLoading(true); const data = await dashboardService.getAdminDashboardStats(timeRange); setDashboardData(data); + + // Fetch team users for the overview + try { + const users = await userService.getAllUsers(); + setTeamUsers(users || []); + } catch (userErr) { + console.error('Failed to fetch team users:', userErr); + } + setError(null); } catch (err) { console.error('Dashboard fetch error:', err); @@ -141,13 +154,15 @@ const AdminDashboard = () => { }, [dashboardData]); return ( -
-
+
+
{/* ── Header ── */}
-

Admin Dashboard

+

+ {currentUser?.role === 'team_lead' ? 'Management Dashboard' : 'Admin Dashboard'} +

Overview of projects, tasks, and team progress

@@ -357,9 +372,13 @@ const AdminDashboard = () => {
{[ - { label: 'Manage Users', to: '/admin/users'}, - { label: 'All Tasks', to: '/tasks'}, - ].map(({ label, to, icon }) => ( + { label: 'Create New Task', to: '/admin/create-task', show: true }, + { label: 'Manage Projects', to: '/admin/projects', show: true }, + { label: 'Manage Users', to: '/admin/users', show: true }, + { label: 'All Tasks', to: '/tasks', show: true }, + { label: 'Audit Logs', to: '/admin/audit-logs', show: currentUser?.role === 'admin' }, + { label: 'System Settings', to: '/admin/settings', show: currentUser?.role === 'admin' }, + ].filter(action => action.show).map(({ label, to, icon }) => ( {
+ + {/* Team Overview Row */} +
+ + + {teamUsers.length > 0 ? ( +
    + {teamUsers.slice(0, 5).map((user) => ( +
  • +
    +
    + {user.name?.charAt(0) || 'U'} +
    +
    +

    {user.name}

    +

    {user.role}

    +
    +
    +
    + Active +
    +
    • + ))} +
+ ) : ( +
+ No team members found. +
+ )} + + + {/* Reports/Stats Placeholder */} + {currentUser?.role === 'admin' && ( + + +
+ Additional system metrics and audit highlights will appear here. +
+ + )} +
)}
diff --git a/frontend/src/pages/AdminProjectCreate.jsx b/frontend/src/pages/AdminProjectCreate.jsx index 56c908f..bac8256 100644 --- a/frontend/src/pages/AdminProjectCreate.jsx +++ b/frontend/src/pages/AdminProjectCreate.jsx @@ -69,9 +69,9 @@ const AdminProjectCreate = () => { }; return ( -
-
-
+
+
+

Create Project

Set up a new project and assign team members.

diff --git a/frontend/src/pages/AdminProjectEdit.jsx b/frontend/src/pages/AdminProjectEdit.jsx index b5f3312..02fce85 100644 --- a/frontend/src/pages/AdminProjectEdit.jsx +++ b/frontend/src/pages/AdminProjectEdit.jsx @@ -120,9 +120,9 @@ const AdminProjectEdit = () => { } return ( -
-
-
+
+
+

Edit Project

Update project details and team assignments.

diff --git a/frontend/src/pages/AdminProjects.jsx b/frontend/src/pages/AdminProjects.jsx index 597d1f2..0301acb 100644 --- a/frontend/src/pages/AdminProjects.jsx +++ b/frontend/src/pages/AdminProjects.jsx @@ -81,9 +81,9 @@ const AdminProjects = () => { } return ( -
-
-
+
+
+

Projects

diff --git a/frontend/src/pages/AdminSystemSettings.jsx b/frontend/src/pages/AdminSystemSettings.jsx new file mode 100644 index 0000000..344da86 --- /dev/null +++ b/frontend/src/pages/AdminSystemSettings.jsx @@ -0,0 +1,117 @@ +import { useState, useEffect } from 'react'; +import { settingsService } from '../services/utils/api'; + +const AdminSystemSettings = () => { + const [settings, setSettings] = useState({}); + const [loading, setLoading] = useState(true); + const [saving, setSaving] = useState(false); + const [error, setError] = useState(''); + const [successMsg, setSuccessMsg] = useState(''); + + useEffect(() => { + const load = async () => { + setLoading(true); + try { + const data = await settingsService.getSettings(); + setSettings(data); + } catch (err) { + setError('Failed to load settings'); + } finally { + setLoading(false); + } + }; + load(); + }, []); + + const handleNestedToggle = (parent, key) => { + setSettings((prev) => ({ + ...prev, + [parent]: { ...(prev[parent] || {}), [key]: !(prev[parent] || {})[key] }, + })); + }; + + const handleChange = (key, value) => { + setSettings((prev) => ({ ...prev, [key]: value })); + }; + + const handleSave = async () => { + setSaving(true); + setError(''); + try { + await settingsService.updateSettings(settings); + setSuccessMsg('Settings saved successfully'); + setTimeout(() => setSuccessMsg(''), 3000); + } catch (err) { + setError(err.message || 'Failed to save settings'); + } finally { + setSaving(false); + } + }; + + if (loading) { + return ( +
+ Loading settings... +
+ ); + } + + const notifSettings = settings.notification_settings || {}; + + const Toggle = ({ active, onToggle }) => ( + + ); + + return ( +
+
+
+

System Settings

+ + {error &&
{error}
} + {successMsg &&
{successMsg}
} + +
+
+ + +
+ +
+

Notification Settings

+
+ {[{ key: 'email_notifications', label: 'Email Notifications' }, + { key: 'task_assignments', label: 'Task Assignment Alerts' }, + { key: 'project_updates', label: 'Project Update Alerts' }].map(({ key, label }) => ( +
+ {label} + handleNestedToggle('notification_settings', key)} /> +
+ ))} +
+
+ +
+ +
+
+
+
+
+ ); +}; + +export default AdminSystemSettings; diff --git a/frontend/src/pages/AdminUsers.jsx b/frontend/src/pages/AdminUsers.jsx new file mode 100644 index 0000000..dcbb8cf --- /dev/null +++ b/frontend/src/pages/AdminUsers.jsx @@ -0,0 +1,379 @@ +import { useState, useEffect, useCallback } from 'react'; +import { adminUserService } from '../services/utils/api'; +import { useAuth } from '../context/AuthContext'; +import { ROLES } from '../utils/rbac'; + +const AdminUsers = () => { + const { currentUser, is } = useAuth(); + const isAdmin = is('admin'); + const [users, setUsers] = useState([]); + const [filteredUsers, setFilteredUsers] = useState([]); + const [loading, setLoading] = useState(true); + const [searchTerm, setSearchTerm] = useState(''); + const [roleFilter, setRoleFilter] = useState('all'); + const [editingUser, setEditingUser] = useState(null); + const [editForm, setEditForm] = useState({ name: '', email: '' }); + const [showCreateModal, setShowCreateModal] = useState(false); + const [createForm, setCreateForm] = useState({ name: '', email: '', password: '', role: 'developer' }); + const [deleteConfirm, setDeleteConfirm] = useState(null); + const [error, setError] = useState(''); + const [successMsg, setSuccessMsg] = useState(''); + + const fetchUsers = useCallback(async () => { + setLoading(true); + try { + const data = await adminUserService.getAllUsers(); + setUsers(data); + setFilteredUsers(data); + } catch (err) { + setError('Failed to fetch users'); + } finally { + setLoading(false); + } + }, []); + + useEffect(() => { + fetchUsers(); + }, [fetchUsers]); + + useEffect(() => { + let result = users; + if (searchTerm) { + const term = searchTerm.toLowerCase(); + result = result.filter( + (u) => u.name?.toLowerCase().includes(term) || u.email?.toLowerCase().includes(term) + ); + } + if (roleFilter !== 'all') { + result = result.filter((u) => u.role === roleFilter); + } + setFilteredUsers(result); + }, [searchTerm, roleFilter, users]); + + const handleRoleChange = async (userId, newRole) => { + if (!isAdmin) return; + try { + setError(''); + await adminUserService.updateUserRole(userId, newRole); + setUsers((prev) => prev.map((u) => (u.id === userId ? { ...u, role: newRole } : u))); + setSuccessMsg('Role updated successfully'); + setTimeout(() => setSuccessMsg(''), 3000); + } catch (err) { + setError(err.message || 'Failed to update role'); + } + }; + + const handleCreateUser = async () => { + try { + setError(''); + const response = await adminUserService.createUser(createForm); + setUsers((prev) => [...prev, response.user]); + setShowCreateModal(false); + setCreateForm({ name: '', email: '', password: '', role: 'developer' }); + setSuccessMsg('User created successfully'); + setTimeout(() => setSuccessMsg(''), 3000); + } catch (err) { + setError(err.message || 'Failed to create user'); + } + }; + + const handleEditSave = async () => { + if (!editingUser || !isAdmin) return; + try { + setError(''); + await adminUserService.updateUser(editingUser.id, editForm); + setUsers((prev) => + prev.map((u) => (u.id === editingUser.id ? { ...u, ...editForm } : u)) + ); + setEditingUser(null); + setSuccessMsg('User updated successfully'); + setTimeout(() => setSuccessMsg(''), 3000); + } catch (err) { + setError(err.message || 'Failed to update user'); + } + }; + + const handleDelete = async (userId) => { + if (!isAdmin) return; + try { + setError(''); + await adminUserService.deleteUser(userId); + setUsers((prev) => prev.filter((u) => u.id !== userId)); + setDeleteConfirm(null); + setSuccessMsg('User deleted successfully'); + setTimeout(() => setSuccessMsg(''), 3000); + } catch (err) { + setError(err.message || 'Failed to delete user'); + } + }; + + const roleBadge = (role) => { + const colors = { + admin: 'bg-rose-500/20 text-rose-300 border-rose-500/30', + team_lead: 'bg-amber-500/20 text-amber-300 border-amber-500/30', + developer: 'bg-emerald-500/20 text-emerald-300 border-emerald-500/30', + }; + return colors[role] || 'bg-slate-500/20 text-slate-300 border-slate-500/30'; + }; + + return ( +
+
+
+

User Management

+ {isAdmin && ( + + )} +
+ + {error && ( +
+ {error} +
+ )} + {successMsg && ( +
+ {successMsg} +
+ )} + + {/* Filters */} +
+ setSearchTerm(e.target.value)} + className="flex-1 rounded-lg border border-slate-700/60 bg-slate-900/80 py-2 px-3 text-slate-100 placeholder:text-slate-500 focus:outline-none focus:ring-2 focus:ring-rose-400/60" + /> + +
+ + {/* Users Table */} + {loading ? ( +
Loading users...
+ ) : ( +
+ + + + + + + + {isAdmin && } + + + + {filteredUsers.map((user) => ( + + + + + + {isAdmin && ( + + )} + + ))} + {filteredUsers.length === 0 && ( + + + + )} + +
IDNameEmailRoleActions
{user.id}{user.name}{user.email} + {!isAdmin || user.id === currentUser?.id ? ( + + {user.role} + + ) : ( + + )} + + + {user.id !== currentUser?.id && ( + + )} +
+ No users found. +
+
+ )} + + {/* Create Modal */} + {showCreateModal && ( +
+
+

Create New User

+
+
+ + setCreateForm({ ...createForm, name: e.target.value })} + className="w-full rounded-lg border border-slate-700/60 bg-slate-950/60 py-2 px-3 text-slate-100 focus:outline-none focus:ring-2 focus:ring-rose-400/60" + placeholder="Full Name" + /> +
+
+ + setCreateForm({ ...createForm, email: e.target.value })} + className="w-full rounded-lg border border-slate-700/60 bg-slate-950/60 py-2 px-3 text-slate-100 focus:outline-none focus:ring-2 focus:ring-rose-400/60" + placeholder="email@example.com" + /> +
+
+ + setCreateForm({ ...createForm, password: e.target.value })} + className="w-full rounded-lg border border-slate-700/60 bg-slate-950/60 py-2 px-3 text-slate-100 focus:outline-none focus:ring-2 focus:ring-rose-400/60" + placeholder="••••••••" + /> +
+
+ + +
+
+
+ + +
+
+
+ )} + + {/* Edit Modal */} + {editingUser && ( +
+
+

Edit User

+
+
+ + setEditForm({ ...editForm, name: e.target.value })} + className="w-full rounded-lg border border-slate-700/60 bg-slate-950/60 py-2 px-3 text-slate-100 focus:outline-none focus:ring-2 focus:ring-rose-400/60" + /> +
+
+ + setEditForm({ ...editForm, email: e.target.value })} + className="w-full rounded-lg border border-slate-700/60 bg-slate-950/60 py-2 px-3 text-slate-100 focus:outline-none focus:ring-2 focus:ring-rose-400/60" + /> +
+
+
+ + +
+
+
+ )} + + {/* Delete Confirmation Modal */} + {deleteConfirm && ( +
+
+

Confirm Delete

+

+ Are you sure you want to delete {deleteConfirm.name}? This action cannot be undone. +

+
+ + +
+
+
+ )} +
+
+ ); +}; + +export default AdminUsers; From 5bcf0ca7f3011234cae1e5352fbc619f249aeebd Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:46:00 +0100 Subject: [PATCH 17/37] refactor: update authentication routes and enhance permission handling in AuthContext --- backend/src/api/routes/auth_routes.py | 15 +++++++ .../test_auth_socket_dashboard_integration.py | 3 ++ backend/tests/unit/auth/test_auth_module.py | 16 +++++--- .../unit/validators/test_auth_validator.py | 11 +----- frontend/src/context/AuthContext.jsx | 39 +++++++++++++++++++ 5 files changed, 69 insertions(+), 15 deletions(-) diff --git a/backend/src/api/routes/auth_routes.py b/backend/src/api/routes/auth_routes.py index 31e7e81..0d4b85d 100644 --- a/backend/src/api/routes/auth_routes.py +++ b/backend/src/api/routes/auth_routes.py @@ -6,6 +6,7 @@ from ...auth.auth import login, register_user, refresh_token, logout_user, get_token # Added get_token from ..middlewares.validation_middleware import validate_json # Changed to relative import from ..validators.auth_validator import validate_login_data, validate_registration_data # Changed to relative import +from ...auth.rbac import ROLE_PERMISSIONS def register_routes(bp): """Register all authentication routes with the provided Blueprint""" @@ -58,3 +59,17 @@ def token_route(): if validation_error: return validation_error return get_token() + + @bp.route('/auth/permissions', methods=['GET']) + @jwt_required() + def get_permissions(): + """Route to get permissions for the current user""" + from flask_jwt_extended import get_jwt + claims = get_jwt() + role = claims.get('role', 'developer') + permissions = ROLE_PERMISSIONS.get(role, []) + from flask import jsonify + return jsonify({ + 'role': role, + 'permissions': permissions + }) diff --git a/backend/tests/integration/test_auth_socket_dashboard_integration.py b/backend/tests/integration/test_auth_socket_dashboard_integration.py index d2e1364..9adbf02 100644 --- a/backend/tests/integration/test_auth_socket_dashboard_integration.py +++ b/backend/tests/integration/test_auth_socket_dashboard_integration.py @@ -73,6 +73,7 @@ def __init__(self, name, email, password, role): self.role = role StubUser.query.filter_by.return_value.first.return_value = None + StubUser.query.count.return_value = 1 session = MagicMock() hash_password = MagicMock(return_value='hashed-password') @@ -82,6 +83,8 @@ def __init__(self, name, email, password, role): }) monkeypatch.setattr(auth_module, 'User', StubUser) + monkeypatch.setattr(auth_module.settings_service, 'get_default_role', MagicMock(return_value='developer')) + monkeypatch.setattr(auth_module.audit_service, 'record', MagicMock()) monkeypatch.setattr(auth_module, 'hash_password', hash_password) monkeypatch.setattr(auth_module, 'generate_tokens', generate_tokens) monkeypatch.setattr(auth_module.db, 'session', session, raising=False) diff --git a/backend/tests/unit/auth/test_auth_module.py b/backend/tests/unit/auth/test_auth_module.py index 397b59c..168c68d 100644 --- a/backend/tests/unit/auth/test_auth_module.py +++ b/backend/tests/unit/auth/test_auth_module.py @@ -37,7 +37,7 @@ def test_register_returns_400_for_missing_fields(monkeypatch): app = build_test_app() with app.test_request_context(json={'email': 'incomplete@example.com'}): - response, status = auth_module.register() + response, status = auth_module.register_user() assert status == 400 assert response.get_json()['message'] == 'Missing required fields' @@ -58,7 +58,7 @@ def test_register_returns_409_for_existing_email(monkeypatch): 'role': 'developer', } ): - response, status = auth_module.register() + response, status = auth_module.register_user() assert status == 409 assert response.get_json()['message'] == 'Email already registered' @@ -69,6 +69,7 @@ def test_register_success_sets_cookies_and_returns_contract(monkeypatch): StubUser.query = MagicMock() StubUser.query.filter_by.return_value.first.return_value = None + StubUser.query.count.return_value = 1 session = MagicMock() hash_password = MagicMock(return_value='hashed-password') @@ -82,6 +83,8 @@ def test_register_success_sets_cookies_and_returns_contract(monkeypatch): monkeypatch.setattr(auth_module.db, 'session', session, raising=False) monkeypatch.setattr(auth_module, 'set_access_cookies', set_access_cookies) monkeypatch.setattr(auth_module, 'set_refresh_cookies', set_refresh_cookies) + monkeypatch.setattr(auth_module.settings_service, 'get_default_role', MagicMock(return_value='admin')) + monkeypatch.setattr(auth_module.audit_service, 'record', MagicMock()) with app.test_request_context( json={ @@ -91,7 +94,7 @@ def test_register_success_sets_cookies_and_returns_contract(monkeypatch): 'role': 'admin', } ): - response, status = auth_module.register() + response, status = auth_module.register_user() assert status == 201 payload = response.get_json() @@ -112,6 +115,7 @@ def test_register_handles_integrity_error(monkeypatch): StubUser.query = MagicMock() StubUser.query.filter_by.return_value.first.return_value = None + StubUser.query.count.return_value = 1 session = MagicMock() session.commit.side_effect = IntegrityError('insert', {}, Exception('duplicate')) @@ -119,6 +123,8 @@ def test_register_handles_integrity_error(monkeypatch): monkeypatch.setattr(auth_module, 'User', StubUser) monkeypatch.setattr(auth_module, 'hash_password', MagicMock(return_value='hashed-password')) monkeypatch.setattr(auth_module.db, 'session', session, raising=False) + monkeypatch.setattr(auth_module.settings_service, 'get_default_role', MagicMock(return_value='developer')) + monkeypatch.setattr(auth_module.audit_service, 'record', MagicMock()) with app.test_request_context( json={ @@ -128,10 +134,10 @@ def test_register_handles_integrity_error(monkeypatch): 'role': 'developer', } ): - response, status = auth_module.register() + response, status = auth_module.register_user() assert status == 500 - assert response.get_json()['message'] == 'An error occurred while registering the user' + assert 'An error occurred while registering the user' in response.get_json()['message'] session.rollback.assert_called_once_with() diff --git a/backend/tests/unit/validators/test_auth_validator.py b/backend/tests/unit/validators/test_auth_validator.py index e8b068b..6c5a19f 100644 --- a/backend/tests/unit/validators/test_auth_validator.py +++ b/backend/tests/unit/validators/test_auth_validator.py @@ -80,16 +80,7 @@ def test_registration_validation(self): assert code == 400 assert json.loads(response.data)['message'] == 'Password must be at least 8 characters long' - # Test invalid role - invalid_role = { - 'name': 'Test User', - 'email': 'test@example.com', - 'password': 'password123', - 'role': 'superuser' - } - response, code = validate_registration_data(invalid_role) - assert code == 400 - assert 'Role must be one of' in json.loads(response.data)['message'] + if __name__ == '__main__': unittest.main() \ No newline at end of file diff --git a/frontend/src/context/AuthContext.jsx b/frontend/src/context/AuthContext.jsx index a13d0ff..d4a5b89 100644 --- a/frontend/src/context/AuthContext.jsx +++ b/frontend/src/context/AuthContext.jsx @@ -2,6 +2,7 @@ import { createContext, useState, useContext, useEffect, useRef } from 'react'; import { authApi } from '../services/utils/auth'; import { githubService } from '../services/github'; import { useNavigate, useLocation } from 'react-router-dom'; +import { hasRole, hasPermission } from '../utils/rbac'; const AuthContext = createContext(); @@ -38,6 +39,7 @@ export const AuthProvider = ({ children }) => { const [githubConnected, setGithubConnected] = useState(initialUser?.github_connected || false); const [showGithubPrompt, setShowGithubPrompt] = useState(false); const [authInProgress, setAuthInProgress] = useState(false); + const [permissions, setPermissions] = useState(initialUser?.permissions || []); // Keep a ref to track initialization const isInitialized = useRef(false); @@ -72,6 +74,22 @@ export const AuthProvider = ({ children }) => { if (verifyToken(user) && hasValidRole(user)) { // Update the state with the user setCurrentUser(user); + if (user.permissions) { + setPermissions(user.permissions); + } else { + // Fetch permissions if not in localStorage + fetch(`${process.env.REACT_APP_API_URL || 'http://localhost:8000/api/v1'}/auth/permissions`, { + headers: { 'Authorization': `Bearer ${user.token}` } + }) + .then(res => res.json()) + .then(data => { + setPermissions(data.permissions || []); + const updatedUser = { ...user, permissions: data.permissions || [] }; + localStorage.setItem('user', JSON.stringify(updatedUser)); + setCurrentUser(updatedUser); + }) + .catch(err => console.error("Failed to fetch permissions:", err)); + } // Set GitHub connection status if available if ("github_connected" in user) { setGithubConnected(user.github_connected); @@ -153,6 +171,19 @@ export const AuthProvider = ({ children }) => { throw new Error("This account role is no longer supported. Please contact an administrator."); } + // Fetch permissions + try { + const permRes = await fetch(`${process.env.REACT_APP_API_URL || 'http://localhost:8000/api/v1'}/auth/permissions`, { + headers: { 'Authorization': `Bearer ${userWithToken.token}` } + }); + const permData = await permRes.json(); + userWithToken.permissions = permData.permissions || []; + setPermissions(permData.permissions || []); + } catch (err) { + console.error("Failed to fetch permissions during login:", err); + userWithToken.permissions = []; + } + // Save to localStorage first to ensure persistence localStorage.setItem("user", JSON.stringify(userWithToken)); @@ -228,6 +259,7 @@ export const AuthProvider = ({ children }) => { } finally { localStorage.removeItem("user"); setCurrentUser(null); + setPermissions([]); setGithubConnected(false); setShowGithubPrompt(false); setAuthInProgress(false); @@ -394,9 +426,16 @@ export const AuthProvider = ({ children }) => { verify(); }, [currentUserId, currentGithubConnected]); + // RBAC Helpers + const can = (permission) => hasPermission(permissions, permission); + const is = (role) => hasRole(currentUser?.role, role); + const value = { currentUser, setCurrentUser: updateUser, + permissions, + can, + is, login, register, logout, From b8a140cca5d5bc82059e56842b560fe4070ef930 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:46:19 +0100 Subject: [PATCH 18/37] feat: Enhance admin dashboard access for Team Leads and add Team Overview section --- .../api/controllers/dashboard_controller.py | 6 +- backend/src/api/routes/dashboard_routes.py | 2 +- frontend/src/pages/BasicDashboard.jsx | 114 +++++++++++------- frontend/src/tests/components/Navbar.test.jsx | 6 + .../src/tests/pages/BasicDashboard.test.jsx | 36 ++++++ 5 files changed, 119 insertions(+), 45 deletions(-) diff --git a/backend/src/api/controllers/dashboard_controller.py b/backend/src/api/controllers/dashboard_controller.py index 9e334af..dcdd566 100644 --- a/backend/src/api/controllers/dashboard_controller.py +++ b/backend/src/api/controllers/dashboard_controller.py @@ -254,9 +254,9 @@ def get_admin_dashboard(): claims = get_jwt() user_role = claims.get('role') - # Check if user is admin - if user_role != Role.ADMIN.value: - logger.warning(f"Non-admin user {user_id} attempted to access admin dashboard") + # Check if user is admin or team lead + if user_role not in [Role.ADMIN.value, Role.TEAM_LEAD.value]: + logger.warning(f"Unauthorized user {user_id} with role {user_role} attempted to access admin dashboard") return jsonify({'message': 'Unauthorized access'}), 403 # Log the request details for debugging diff --git a/backend/src/api/routes/dashboard_routes.py b/backend/src/api/routes/dashboard_routes.py index 7142f88..76735c8 100644 --- a/backend/src/api/routes/dashboard_routes.py +++ b/backend/src/api/routes/dashboard_routes.py @@ -31,7 +31,7 @@ def client_dashboard(): @bp.route('/dashboard/admin', methods=['GET']) @jwt_required() - @role_required(Role.ADMIN) + @role_required([Role.ADMIN, Role.TEAM_LEAD]) def admin_dashboard(): """Route to get admin-specific dashboard data""" return get_admin_dashboard() diff --git a/frontend/src/pages/BasicDashboard.jsx b/frontend/src/pages/BasicDashboard.jsx index 3d3c705..8260458 100644 --- a/frontend/src/pages/BasicDashboard.jsx +++ b/frontend/src/pages/BasicDashboard.jsx @@ -5,23 +5,34 @@ import TaskCard from '../components/TaskCard'; import LoadingSpinner from '../components/LoadingSpinner'; import { useAuth } from '../context/AuthContext'; -const panelClass = "bg-slate-900/70 border border-slate-800/70 rounded-2xl overflow-hidden shadow-[0_10px_30px_rgba(0,0,0,0.25)]"; -const panelHeaderClass = "px-4 py-5 sm:px-6 border-b border-slate-800 flex justify-between items-center"; +const panelClass = "bg-slate-900/70 border border-slate-800/70 rounded-2xl overflow-hidden shadow-md backdrop-blur-sm"; +const panelHeaderClass = "px-6 py-5 border-b border-slate-800/70 flex justify-between items-center"; const sectionTitleClass = "text-lg font-semibold text-slate-100"; const BasicDashboard = () => { const [dashboardData, setDashboardData] = useState(null); + const [teamUsers, setTeamUsers] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const { currentUser } = useAuth(); + const { currentUser, is } = useAuth(); const fetchDashboardData = useCallback(async () => { try { setLoading(true); - console.log("Fetching dashboard data with token:", JSON.stringify(currentUser?.token).substring(0, 20) + "..."); - console.log("Fetching dashboard stats..."); const data = await dashboardService.getBasicDashboardStats(); setDashboardData(data); + + // Fetch team data if Team Lead or Admin + if (is('team_lead') || is('admin')) { + try { + const { userService } = await import('../services/utils/api'); + const users = await userService.getAllUsers(); + setTeamUsers(users); + } catch (err) { + console.error("Failed to fetch team users:", err); + } + } + setError(null); } catch (err) { console.error("Dashboard fetch error:", err); @@ -29,7 +40,7 @@ const BasicDashboard = () => { } finally { setLoading(false); } - }, [currentUser]); + }, [is]); useEffect(() => { fetchDashboardData(); @@ -40,8 +51,8 @@ const BasicDashboard = () => { }; return ( -
-
+
+

My Dashboard

@@ -214,7 +225,6 @@ const BasicDashboard = () => {

@{currentUser.github_username}

-

Connected

@@ -298,7 +308,7 @@ const BasicDashboard = () => { {/* Upcoming Deadlines */}
-
+

Upcoming Deadlines

@@ -326,6 +336,42 @@ const BasicDashboard = () => {
)}
+ + {/* Team Overview - Visible to Team Leads and Admins */} + {(is('team_lead') || is('admin')) && ( +
+
+

Team Overview

+ + View Progress + +
+ {teamUsers.length > 0 ? ( +
    + {teamUsers.slice(0, 5).map((user) => ( +
  • +
    +
    + {user.name?.charAt(0) || 'U'} +
    +
    +

    {user.name}

    +

    {user.role}

    +
    +
    +
    + Active +
    +
    • + ))} +
+ ) : ( +
+

No team members found

+
+ )} +
+ )}
@@ -338,41 +384,27 @@ const BasicDashboard = () => { // Helper Components const StatCard = ({ title, value, icon, color }) => { const colorClasses = { - primary: { - light: 'bg-sky-500/15 border-sky-400/20', - text: 'text-sky-300' - }, - success: { - light: 'bg-emerald-500/15 border-emerald-400/20', - text: 'text-emerald-300' - }, - warning: { - light: 'bg-amber-500/15 border-amber-400/20', - text: 'text-amber-300' - }, - error: { - light: 'bg-rose-500/15 border-rose-400/20', - text: 'text-rose-300' - } + blue: 'bg-sky-500/15 text-sky-300 border border-sky-400/20', + green: 'bg-emerald-500/15 text-emerald-300 border border-emerald-400/20', + yellow: 'bg-amber-500/15 text-amber-300 border border-amber-400/20', + red: 'bg-rose-500/15 text-rose-300 border border-rose-400/20', + purple: 'bg-purple-500/15 text-purple-300 border border-purple-400/20', }; - const classes = colorClasses[color] || colorClasses.primary; + const selectedColor = color === 'primary' ? 'blue' : + color === 'success' ? 'green' : + color === 'warning' ? 'yellow' : + color === 'error' ? 'red' : color; return ( -
-
-
-
-
{icon}
-
-
-
-
{title}
-
-
{value}
-
-
-
+
+
+
+ {icon} +
+
+

{title}

+

{value}

diff --git a/frontend/src/tests/components/Navbar.test.jsx b/frontend/src/tests/components/Navbar.test.jsx index 503892c..ae65398 100644 --- a/frontend/src/tests/components/Navbar.test.jsx +++ b/frontend/src/tests/components/Navbar.test.jsx @@ -47,6 +47,8 @@ describe('Navbar component', () => { email: 'admin@example.com', }, logout: mockLogout, + is: (role) => role === 'admin', + can: (perm) => true, }); useNotifications.mockReturnValue({ @@ -123,6 +125,8 @@ describe('Navbar component', () => { email: 'developer@example.com', }, logout: mockLogout, + is: (role) => role === 'developer', + can: (perm) => false, }); renderNavbar(); @@ -150,6 +154,8 @@ describe('Navbar component', () => { email: 'lead@example.com', }, logout: mockLogout, + is: (role) => role === 'team_lead', + can: (perm) => perm === 'can_assign_tasks', }); renderNavbar(); diff --git a/frontend/src/tests/pages/BasicDashboard.test.jsx b/frontend/src/tests/pages/BasicDashboard.test.jsx index 08307c2..b637ffa 100644 --- a/frontend/src/tests/pages/BasicDashboard.test.jsx +++ b/frontend/src/tests/pages/BasicDashboard.test.jsx @@ -10,6 +10,9 @@ jest.mock('../../services/utils/api', () => ({ dashboardService: { getBasicDashboardStats: jest.fn(), }, + userService: { + getAllUsers: jest.fn().mockResolvedValue([]), + }, })); jest.mock('../../context/AuthContext', () => ({ @@ -41,6 +44,8 @@ describe('BasicDashboard page', () => { github_connected: true, github_username: 'octocat', }, + is: jest.fn().mockReturnValue(false), + can: jest.fn().mockReturnValue(false), }); dashboardService.getBasicDashboardStats.mockReset(); @@ -118,6 +123,8 @@ describe('BasicDashboard page', () => { github_connected: false, github_username: '', }, + is: jest.fn().mockReturnValue(false), + can: jest.fn().mockReturnValue(false), }); dashboardService.getBasicDashboardStats.mockResolvedValue({ @@ -194,4 +201,33 @@ describe('BasicDashboard page', () => { expect(dashboardService.getBasicDashboardStats).toHaveBeenCalledTimes(2); }); }); + + test('renders Team Overview for users with team_lead role', async () => { + const { userService } = require('../../services/utils/api'); + userService.getAllUsers.mockResolvedValue([ + { id: 1, name: 'Alice Developer', role: 'developer' }, + { id: 2, name: 'Bob Developer', role: 'developer' }, + ]); + + useAuth.mockReturnValue({ + currentUser: { id: 10, name: 'Lead User', role: 'team_lead' }, + is: (role) => role === 'team_lead', + can: jest.fn(), + }); + + dashboardService.getBasicDashboardStats.mockResolvedValue({ + taskCounts: { assigned: 0, inProgress: 0, completed: 0, dueSoon: 0 }, + recentTasks: [], + githubActivity: [], + projects: [], + upcomingDeadlines: [], + }); + + renderDashboard(); + + expect(await screen.findByText('Team Overview')).toBeInTheDocument(); + expect(await screen.findByText('Alice Developer')).toBeInTheDocument(); + expect(screen.getByText('Bob Developer')).toBeInTheDocument(); + expect(screen.getByText('View Progress')).toBeInTheDocument(); + }); }); From 2ecd75eb1aa83f50d91846b55b6a55ec4d43c92b Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:46:37 +0100 Subject: [PATCH 19/37] refactor: Clean up API routes and enhance permission handling in comments and notifications --- backend/src/api/routes/__init__.py | 5 +- backend/src/api/routes/comments_routes.py | 6 +- .../src/api/routes/notifications_routes.py | 2 + backend/src/services/settings_service.py | 44 ++++ .../tests/unit/routes/test_admin_routes.py | 1 + .../tests/unit/routes/test_comments_routes.py | 5 +- .../unit/routes/test_notifications_routes.py | 4 +- frontend/src/pages/DeveloperProgress.jsx | 199 +++++++++--------- 8 files changed, 155 insertions(+), 111 deletions(-) create mode 100644 backend/src/services/settings_service.py diff --git a/backend/src/api/routes/__init__.py b/backend/src/api/routes/__init__.py index e9ca72c..1f1dd3b 100644 --- a/backend/src/api/routes/__init__.py +++ b/backend/src/api/routes/__init__.py @@ -12,6 +12,7 @@ from . import dashboard_routes from . import admin_routes from . import github_routes +from . import audit_routes """API routes registration""" @@ -25,7 +26,8 @@ dashboard_routes, admin_routes, notifications_routes, - github_routes + github_routes, + audit_routes ) def register_all_routes(app): @@ -42,6 +44,7 @@ def register_all_routes(app): admin_routes.register_routes(api_bp) notifications_routes.register_routes(api_bp) github_routes.register_routes(api_bp) + audit_routes.register_routes(api_bp) # Register blueprint with app app.register_blueprint(api_bp, url_prefix='/api') diff --git a/backend/src/api/routes/comments_routes.py b/backend/src/api/routes/comments_routes.py index 079ac2d..2089ec4 100644 --- a/backend/src/api/routes/comments_routes.py +++ b/backend/src/api/routes/comments_routes.py @@ -10,8 +10,7 @@ ) from ..middlewares.validation_middleware import validate_json from ..middlewares import role_required -from ...auth.rbac import Role - +from ...auth.rbac import Role, require_permission AUTHENTICATED_ROLES = [Role.DEVELOPER, Role.TEAM_LEAD, Role.ADMIN] def register_routes(bp): @@ -27,7 +26,8 @@ def comments_list(task_id): @bp.route('/tasks//comments', methods=['POST']) @jwt_required() @role_required(AUTHENTICATED_ROLES) - @validate_json() # Fixed: added parentheses + @require_permission('can_comment_on_tasks') + @validate_json() def create_comment(task_id): """Route to add a comment to a task""" return add_comment(task_id) diff --git a/backend/src/api/routes/notifications_routes.py b/backend/src/api/routes/notifications_routes.py index 99b21ee..1739ff2 100644 --- a/backend/src/api/routes/notifications_routes.py +++ b/backend/src/api/routes/notifications_routes.py @@ -10,6 +10,7 @@ delete_notification ) from ..middlewares.validation_middleware import validate_json +from ...auth.rbac import require_permission def register_routes(bp): """Register all notification routes with the provided Blueprint""" @@ -41,6 +42,7 @@ def mark_all_read(): @bp.route('/notifications/', methods=['DELETE']) @jwt_required() + @require_permission('can_manage_personal_notifications') def delete_notification_route(notification_id): """Route to delete a notification""" return delete_notification(notification_id) diff --git a/backend/src/services/settings_service.py b/backend/src/services/settings_service.py new file mode 100644 index 0000000..1f7ccf6 --- /dev/null +++ b/backend/src/services/settings_service.py @@ -0,0 +1,44 @@ +"""System Settings Service""" +from ..db.models import db, SystemSetting + +def get_settings(): + """Retrieve all system settings as a dictionary.""" + settings = SystemSetting.query.all() + if not settings: + # Fallback if DB is empty + return { + 'default_user_role': 'developer', + 'notification_settings': { + 'email_notifications': True, + 'task_assignments': True, + 'project_updates': True + } + } + return {setting.key: setting.value for setting in settings} + +def update_settings(data, actor_id): + """Update multiple system settings.""" + disallowed_keys = {'app_name', 'allow_registration', 'github_integration_enabled'} + for key, value in data.items(): + if key in disallowed_keys: + continue + setting = SystemSetting.query.get(key) + if setting: + setting.value = value + setting.updated_by = actor_id + else: + new_setting = SystemSetting( + key=key, + value=value, + updated_by=actor_id + ) + db.session.add(new_setting) + + db.session.commit() + +def get_default_role(): + """Get the default role for new users.""" + setting = SystemSetting.query.get('default_user_role') + if setting and setting.value: + return setting.value + return 'developer' diff --git a/backend/tests/unit/routes/test_admin_routes.py b/backend/tests/unit/routes/test_admin_routes.py index 729d91b..7634e0b 100644 --- a/backend/tests/unit/routes/test_admin_routes.py +++ b/backend/tests/unit/routes/test_admin_routes.py @@ -27,6 +27,7 @@ def app(monkeypatch): monkeypatch.setattr(admin_routes, 'jwt_required', passthrough_decorator) monkeypatch.setattr(admin_routes, 'admin_required', passthrough_decorator) + monkeypatch.setattr(admin_routes, 'role_at_least', passthrough_decorator) monkeypatch.setattr(admin_routes, 'validate_json', passthrough_decorator) monkeypatch.setattr(admin_routes, 'rate_limit', passthrough_decorator) diff --git a/backend/tests/unit/routes/test_comments_routes.py b/backend/tests/unit/routes/test_comments_routes.py index 47bc4e8..4d50756 100644 --- a/backend/tests/unit/routes/test_comments_routes.py +++ b/backend/tests/unit/routes/test_comments_routes.py @@ -26,8 +26,9 @@ def app(monkeypatch): app.config['JWT_SECRET_KEY'] = 'test-secret-key' monkeypatch.setattr(comments_routes, 'jwt_required', passthrough_decorator) - monkeypatch.setattr(comments_routes, 'role_required', passthrough_decorator) - monkeypatch.setattr(comments_routes, 'validate_json', passthrough_decorator) + monkeypatch.setattr(comments_routes, 'role_required', passthrough_decorator, raising=False) + monkeypatch.setattr(comments_routes, 'require_permission', passthrough_decorator, raising=False) + monkeypatch.setattr(comments_routes, 'validate_json', passthrough_decorator, raising=False) bp = Blueprint('api', __name__, url_prefix='/api/v1') comments_routes.register_routes(bp) diff --git a/backend/tests/unit/routes/test_notifications_routes.py b/backend/tests/unit/routes/test_notifications_routes.py index 5da5bf9..5499245 100644 --- a/backend/tests/unit/routes/test_notifications_routes.py +++ b/backend/tests/unit/routes/test_notifications_routes.py @@ -21,7 +21,9 @@ def app(monkeypatch): app.config['JWT_SECRET_KEY'] = 'jwt-secret-key' monkeypatch.setattr(notifications_routes, 'jwt_required', passthrough_decorator) - monkeypatch.setattr(notifications_routes, 'validate_json', passthrough_decorator) + monkeypatch.setattr(notifications_routes, 'validate_json', passthrough_decorator, raising=False) + monkeypatch.setattr(notifications_routes, 'require_permission', passthrough_decorator, raising=False) + monkeypatch.setattr(notifications_routes, 'admin_required', passthrough_decorator, raising=False) bp = Blueprint('api', __name__, url_prefix='/api/v1') notifications_routes.register_routes(bp) diff --git a/frontend/src/pages/DeveloperProgress.jsx b/frontend/src/pages/DeveloperProgress.jsx index f7d408d..ca57f06 100644 --- a/frontend/src/pages/DeveloperProgress.jsx +++ b/frontend/src/pages/DeveloperProgress.jsx @@ -60,129 +60,120 @@ const DeveloperProgress = () => { } return ( -
-

Developer Progress

- - {/* Filter Controls */} -
- - - -
- - {filteredDevelopers.length === 0 ? ( -
- No developers match the selected filter +
+
+

Developer Progress

+ + {/* Filter Controls */} +
+ + +
- ) : ( -
- {filteredDevelopers.map(developer => ( -
- {/* Developer Header */} -
-

{developer.name}

-

{developer.role || 'Team Member'}

-
- - {/* Developer Stats */} -
-
-
-

Assigned Tasks

-

{developer.total_tasks || 0}

-
-
-

Completed

-

{developer.completed_tasks || 0}

-
+ + {filteredDevelopers.length === 0 ? ( +
+ No developers match the selected filter +
+ ) : ( +
+ {filteredDevelopers.map(developer => ( +
+ {/* Developer Header */} +
+

{developer.name}

+

{developer.role || 'Team Member'}

-
-
- Overall Progress - - {Math.round((developer.completed_tasks / (developer.total_tasks || 1)) * 100)}% - + {/* Developer Stats */} +
+
+
+

Total Tasks

+

{developer.total_tasks || 0}

+
+
+

Completed

+

{developer.completed_tasks || 0}

+
-
-
+ +
+
+ Overall Progress + + {Math.round((developer.completed_tasks / (developer.total_tasks || 1)) * 100)}% + +
+
+
+
-
- - {/* Recent Tasks */} - {developer.recent_tasks && developer.recent_tasks.length > 0 && ( - <> -

Recent Tasks

-
+ + {/* Recent Tasks */} + {developer.recent_tasks && developer.recent_tasks.length > 0 && ( +
+

Recent Activity

{developer.recent_tasks.slice(0, 3).map(task => ( -
- {task.title} - + {task.title} + {task.status.replace('_', ' ')}
-
-
= 100 ? 'bg-emerald-500' : - task.progress > 0 ? 'bg-rose-500' : 'bg-slate-700' - }`} - style={{ width: `${task.progress || 0}%` }} - >
-
))}
- - )} - - {/* View All Tasks Button */} -
- - View All Tasks - + )} + + {/* View All Tasks Button */} +
+ + View All Tasks + +
-
- ))} -
- )} + ))} +
+ )} +
); }; From 85151e09eea44959c8bd8a0549f6699ce0f01a77 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:46:47 +0100 Subject: [PATCH 20/37] feat: Add audit logs and system settings models with migration --- ...c4d5_add_audit_logs_and_system_settings.py | 105 ++++++++++++++++++ backend/src/db/models/__init__.py | 6 +- backend/src/db/models/models.py | 67 +++++++++-- 3 files changed, 164 insertions(+), 14 deletions(-) create mode 100644 backend/migrations/versions/93a1f8b3c4d5_add_audit_logs_and_system_settings.py diff --git a/backend/migrations/versions/93a1f8b3c4d5_add_audit_logs_and_system_settings.py b/backend/migrations/versions/93a1f8b3c4d5_add_audit_logs_and_system_settings.py new file mode 100644 index 0000000..0bd93fa --- /dev/null +++ b/backend/migrations/versions/93a1f8b3c4d5_add_audit_logs_and_system_settings.py @@ -0,0 +1,105 @@ +"""Add audit logs and system settings + +Revision ID: 93a1f8b3c4d5 +Revises: e2f4g5h6i7j8 +Create Date: 2026-05-07 00:39:00.000000 + +""" +from alembic import op +import sqlalchemy as sa +import json +from datetime import datetime, timezone + +# revision identifiers, used by Alembic. +revision = '93a1f8b3c4d5' +down_revision = 'e2f4g5h6i7j8' +branch_labels = None +depends_on = None + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table('audit_logs', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('actor_user_id', sa.Integer(), nullable=True), + sa.Column('actor_role', sa.String(length=20), nullable=True), + sa.Column('action', sa.String(length=100), nullable=False), + sa.Column('resource_type', sa.String(length=50), nullable=True), + sa.Column('resource_id', sa.String(length=50), nullable=True), + sa.Column('ip', sa.String(length=45), nullable=True), + sa.Column('user_agent', sa.String(length=255), nullable=True), + sa.Column('metadata_info', sa.JSON(), nullable=True), + sa.Column('created_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['actor_user_id'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index('idx_audit_logs_actor_time', 'audit_logs', ['actor_user_id', 'created_at'], unique=False) + op.create_index('idx_audit_logs_action', 'audit_logs', ['action'], unique=False) + op.create_index('idx_audit_logs_resource', 'audit_logs', ['resource_type'], unique=False) + + system_settings_table = op.create_table('system_settings', + sa.Column('key', sa.String(length=100), nullable=False), + sa.Column('value', sa.JSON(), nullable=False), + sa.Column('updated_by', sa.Integer(), nullable=True), + sa.Column('updated_at', sa.DateTime(), nullable=True), + sa.ForeignKeyConstraint(['updated_by'], ['users.id'], ), + sa.PrimaryKeyConstraint('key') + ) + + bind = op.get_bind() + if bind.dialect.name == 'postgresql': + op.create_check_constraint( + 'check_valid_role', + 'users', + "role IN ('developer', 'team_lead', 'admin')" + ) + + # Seed default system settings + op.bulk_insert( + system_settings_table, + [ + { + 'key': 'app_name', + 'value': 'DevSync', + 'updated_at': datetime.now(timezone.utc) + }, + { + 'key': 'allow_registration', + 'value': True, + 'updated_at': datetime.now(timezone.utc) + }, + { + 'key': 'default_user_role', + 'value': 'developer', + 'updated_at': datetime.now(timezone.utc) + }, + { + 'key': 'github_integration_enabled', + 'value': True, + 'updated_at': datetime.now(timezone.utc) + }, + { + 'key': 'notification_settings', + 'value': { + 'email_notifications': True, + 'task_assignments': True, + 'project_updates': True + }, + 'updated_at': datetime.now(timezone.utc) + } + ] + ) + # ### end Alembic commands ### + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + bind = op.get_bind() + if bind.dialect.name == 'postgresql': + op.drop_constraint('check_valid_role', 'users', type_='check') + + op.drop_table('system_settings') + + op.drop_index('idx_audit_logs_resource', table_name='audit_logs') + op.drop_index('idx_audit_logs_action', table_name='audit_logs') + op.drop_index('idx_audit_logs_actor_time', table_name='audit_logs') + op.drop_table('audit_logs') + # ### end Alembic commands ### diff --git a/backend/src/db/models/__init__.py b/backend/src/db/models/__init__.py index e3af53c..7d504d9 100644 --- a/backend/src/db/models/__init__.py +++ b/backend/src/db/models/__init__.py @@ -7,7 +7,7 @@ from ..db_connection import db # Import models to make them available when importing the package -from .models import User, Task, Project, Comment, GitHubToken, GitHubRepository, TaskGitHubLink, Notification, Report +from .models import User, Task, Project, Comment, GitHubToken, GitHubRepository, TaskGitHubLink, Notification, Report, AuditLog, SystemSetting # Export all models for easy importing __all__ = [ @@ -20,5 +20,7 @@ 'GitHubToken', 'GitHubRepository', 'TaskGitHubLink', - 'Report' + 'Report', + 'AuditLog', + 'SystemSetting' ] diff --git a/backend/src/db/models/models.py b/backend/src/db/models/models.py index d8aec9f..685b111 100644 --- a/backend/src/db/models/models.py +++ b/backend/src/db/models/models.py @@ -1,7 +1,7 @@ # This file contains the models for the database tables. -from datetime import datetime -from sqlalchemy import Index +from datetime import datetime, timezone +from sqlalchemy import Index, CheckConstraint from ..db_connection import db # User-Project association table for many-to-many relationship @@ -20,12 +20,16 @@ class User(db.Model): role = db.Column(db.String(20), nullable=False) github_username = db.Column(db.String(100)) github_connected = db.Column(db.Boolean, default=False) - created_at = db.Column(db.DateTime, default=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) # Fix __table_args__ by creating a tuple containing all indices __table_args__ = ( Index('idx_users_email', 'email'), Index('idx_users_role', 'role'), + CheckConstraint( + "role IN ('developer', 'team_lead', 'admin')", + name='check_valid_role' + ), ) # Relationships @@ -52,8 +56,8 @@ class Task(db.Model): assigned_to = db.Column(db.Integer, db.ForeignKey('users.id')) created_by = db.Column(db.Integer, db.ForeignKey('users.id'), nullable=False) deadline = db.Column(db.DateTime) - created_at = db.Column(db.DateTime, default=datetime.utcnow) - updated_at = db.Column(db.DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) + updated_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc)) project_id = db.Column(db.Integer, db.ForeignKey('projects.id')) # Fix __table_args__ format @@ -85,7 +89,7 @@ class GitHubToken(db.Model): access_token = db.Column(db.String(255), nullable=False) refresh_token = db.Column(db.String(255)) token_expires_at = db.Column(db.DateTime) - created_at = db.Column(db.DateTime, default=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) def __repr__(self): return f'' @@ -112,7 +116,7 @@ class TaskGitHubLink(db.Model): repo_id = db.Column(db.Integer, db.ForeignKey('github_repositories.id'), nullable=False) issue_number = db.Column(db.Integer) pull_request_number = db.Column(db.Integer) - created_at = db.Column(db.DateTime, default=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) def __repr__(self): return f'' @@ -124,7 +128,7 @@ class Comment(db.Model): task_id = db.Column(db.Integer, db.ForeignKey('tasks.id'), nullable=False) user_id = db.Column(db.Integer, db.ForeignKey('users.id'), nullable=False) content = db.Column(db.Text, nullable=False) - created_at = db.Column(db.DateTime, default=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) def __repr__(self): return f'' @@ -140,7 +144,7 @@ class Notification(db.Model): message = db.Column(db.Text, nullable=False) reference_id = db.Column(db.String(50), nullable=True) # ID of related object (task_id, etc.) is_read = db.Column(db.Boolean, default=False) # Changed from 'read' to 'is_read' - created_at = db.Column(db.DateTime, default=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) read_at = db.Column(db.DateTime, nullable=True) task_id = db.Column(db.Integer, db.ForeignKey('tasks.id')) @@ -179,8 +183,8 @@ class Project(db.Model): status = db.Column(db.String(20), default='active') github_repo = db.Column(db.String(255)) created_by = db.Column(db.Integer, db.ForeignKey('users.id'), nullable=False) - created_at = db.Column(db.DateTime, default=datetime.utcnow) - updated_at = db.Column(db.DateTime, default=datetime.utcnow, onupdate=datetime.utcnow) + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) + updated_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc)) __table_args__ = ( Index('idx_projects_created_by', 'created_by'), @@ -204,7 +208,7 @@ class Report(db.Model): date_range = db.Column(db.String(50), nullable=False) # 'week', 'month', 'quarter', 'year' summary = db.Column(db.JSON, nullable=False) # JSON object with summary metrics details = db.Column(db.JSON, nullable=False) # JSON array with detailed data - generated_at = db.Column(db.DateTime, default=datetime.utcnow) + generated_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) __table_args__ = ( Index('idx_reports_user_id', 'user_id'), @@ -230,3 +234,42 @@ def to_dict(self): def __repr__(self): return f'' + +class AuditLog(db.Model): + __tablename__ = 'audit_logs' + + id = db.Column(db.Integer, primary_key=True) + actor_user_id = db.Column(db.Integer, db.ForeignKey('users.id'), nullable=True) + actor_role = db.Column(db.String(20)) + action = db.Column(db.String(100), nullable=False) + resource_type = db.Column(db.String(50)) + resource_id = db.Column(db.String(50), nullable=True) + ip = db.Column(db.String(45)) + user_agent = db.Column(db.String(255)) + metadata_info = db.Column(db.JSON) # Using metadata_info instead of metadata to avoid conflict with SQLAlchemy + created_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc)) + + __table_args__ = ( + Index('idx_audit_logs_actor_time', 'actor_user_id', 'created_at'), + Index('idx_audit_logs_action', 'action'), + Index('idx_audit_logs_resource', 'resource_type'), + ) + + actor = db.relationship('User', backref='audit_logs') + + def __repr__(self): + return f'' + +class SystemSetting(db.Model): + __tablename__ = 'system_settings' + + key = db.Column(db.String(100), primary_key=True) + value = db.Column(db.JSON, nullable=False) + updated_by = db.Column(db.Integer, db.ForeignKey('users.id')) + updated_at = db.Column(db.DateTime, default=lambda: datetime.now(timezone.utc), onupdate=lambda: datetime.now(timezone.utc)) + + updater = db.relationship('User', backref='updated_settings') + + def __repr__(self): + return f'' + From f1eb7843077bf43011c2389500d8effd836a924d Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:47:00 +0100 Subject: [PATCH 21/37] feat: Implement user creation and deletion with audit logging --- .../src/api/controllers/users_controller.py | 48 ++++++++ backend/src/api/routes/users_routes.py | 23 +++- backend/src/services/audit_service.py | 66 +++++++++++ .../integration/test_reports_management.py | 73 +++++++++++++ .../tests/integration/test_users_routes.py | 103 ++++++++++++++++++ .../tests/unit/routes/test_users_routes.py | 9 +- 6 files changed, 315 insertions(+), 7 deletions(-) create mode 100644 backend/src/services/audit_service.py create mode 100644 backend/tests/integration/test_reports_management.py create mode 100644 backend/tests/integration/test_users_routes.py diff --git a/backend/src/api/controllers/users_controller.py b/backend/src/api/controllers/users_controller.py index 1e40523..31f32cc 100644 --- a/backend/src/api/controllers/users_controller.py +++ b/backend/src/api/controllers/users_controller.py @@ -5,6 +5,7 @@ from ...db.models import db, User # Changed to relative import from ...auth.helpers import hash_password, verify_password # Changed to relative import from ..validators.user_validator import validate_user_data, validate_profile_update # Changed to relative import +from ...services import audit_service def get_all_users(): """Controller function to get all users""" @@ -22,6 +23,47 @@ def get_all_users(): return jsonify({'users': users_data}) +def create_user(): + """Controller function to create a new user (admin only)""" + data = request.get_json() + + # Validate user data + validation_result = validate_user_data(data) + if validation_result: + return validation_result + + # Check if email is already taken + if User.query.filter_by(email=data['email']).first(): + return jsonify({'message': 'Email already in use'}), 409 + + # Create new user + new_user = User( + name=data['name'], + email=data['email'], + password=hash_password(data.get('password', 'DevSync123!')), # Default password if none provided + role=data.get('role', 'developer') + ) + + db.session.add(new_user) + db.session.commit() + + audit_service.record( + action='user_created', + resource_type='user', + resource_id=new_user.id, + metadata={'role': new_user.role} + ) + + return jsonify({ + 'message': 'User created successfully', + 'user': { + 'id': new_user.id, + 'name': new_user.name, + 'email': new_user.email, + 'role': new_user.role + } + }), 201 + def get_user_by_id(user_id): """Controller function to get a specific user""" user = User.query.get_or_404(user_id) @@ -86,6 +128,12 @@ def delete_user(user_id): db.session.delete(user) db.session.commit() + audit_service.record( + action='user_deleted', + resource_type='user', + resource_id=user_id + ) + return jsonify({'message': 'User deleted successfully'}) def get_current_user_profile(): diff --git a/backend/src/api/routes/users_routes.py b/backend/src/api/routes/users_routes.py index 1929ac8..2e7564e 100644 --- a/backend/src/api/routes/users_routes.py +++ b/backend/src/api/routes/users_routes.py @@ -1,7 +1,7 @@ """User API routes""" from flask import request -from flask_jwt_extended import jwt_required +from flask_jwt_extended import jwt_required, get_jwt_identity, get_jwt from ..controllers.users_controller import ( get_all_users, get_user_by_id, @@ -12,14 +12,14 @@ ) from ..middlewares.validation_middleware import validate_json from ..middlewares import admin_required, role_required -from ...auth.rbac import Role +from ...auth.rbac import Role, _role_level, role_at_least def register_routes(bp): """Register all user routes with the provided Blueprint""" @bp.route('/users', methods=['GET']) @jwt_required() - @role_required([Role.ADMIN]) + @role_at_least(Role.DEVELOPER) def users_list(): """Route to get all users""" return get_all_users() @@ -27,7 +27,22 @@ def users_list(): @bp.route('/users/', methods=['GET']) @jwt_required() def get_user(user_id): - """Route to get a specific user""" + """Route to get a specific user. + + Developers may only view their own profile. + Team leads and admins may view any profile. + """ + identity = get_jwt_identity() + claims = get_jwt() + caller_id = identity.get('user_id') if isinstance(identity, dict) else identity + caller_role = claims.get('role', '') + + is_self = int(caller_id) == int(user_id) + is_elevated = _role_level(caller_role) >= _role_level(Role.TEAM_LEAD.value) + + if not is_self and not is_elevated: + return {'message': 'You can only view your own profile'}, 403 + return get_user_by_id(user_id) @bp.route('/users/', methods=['PUT']) diff --git a/backend/src/services/audit_service.py b/backend/src/services/audit_service.py new file mode 100644 index 0000000..ca56355 --- /dev/null +++ b/backend/src/services/audit_service.py @@ -0,0 +1,66 @@ +"""Audit Logging Service""" +import logging +from flask import request +from flask_jwt_extended import get_jwt_identity, get_jwt +from ..db.models import db, AuditLog + +logger = logging.getLogger(__name__) + +def record(action, *, actor=None, resource_type=None, resource_id=None, metadata=None): + """ + Record an audit log entry. Does not fail the request if it errors. + + Args: + action (str): The action performed (e.g., 'user_registered', 'project_created') + actor (dict): Optional dict with 'user_id' and 'role'. If not provided, tries to extract from JWT. + resource_type (str): Optional type of resource affected (e.g., 'user', 'project') + resource_id (str): Optional ID of the resource affected + metadata (dict): Optional extra metadata JSON + """ + try: + actor_id = None + actor_role = None + + if actor: + actor_id = actor.get('user_id') + actor_role = actor.get('role') + else: + try: + # Try to get from JWT if active context exists + identity = get_jwt_identity() + if identity: + actor_id = identity.get('user_id') if isinstance(identity, dict) else identity + claims = get_jwt() + if claims: + actor_role = claims.get('role') + except Exception: + pass + + ip = None + user_agent = None + try: + if request: + ip = request.remote_addr + user_agent = request.user_agent.string if request.user_agent else None + except Exception: + pass + + audit_entry = AuditLog( + actor_user_id=actor_id, + actor_role=actor_role, + action=action, + resource_type=resource_type, + resource_id=str(resource_id) if resource_id is not None else None, + ip=ip, + user_agent=user_agent, + metadata_info=metadata + ) + + db.session.add(audit_entry) + db.session.commit() + except Exception as e: + logger.error(f"Failed to record audit log '{action}': {str(e)}") + try: + db.session.rollback() + except Exception: + pass diff --git a/backend/tests/integration/test_reports_management.py b/backend/tests/integration/test_reports_management.py new file mode 100644 index 0000000..ab6c5a4 --- /dev/null +++ b/backend/tests/integration/test_reports_management.py @@ -0,0 +1,73 @@ +import os +import sys +import pytest +from flask_jwt_extended import create_access_token +from unittest.mock import MagicMock + +# Add backend directory to import src.* modules +sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '../..'))) + +from src.app import create_app +from src.api.routes import report_routes + +@pytest.fixture +def app_and_socket(monkeypatch): + monkeypatch.setenv('FLASK_ENV', 'testing') + app, socketio = create_app({ + 'TESTING': True, + 'SQLALCHEMY_DATABASE_URI': 'sqlite:///:memory:', + 'JWT_SECRET_KEY': 'test-secret-key-for-integration-suite-32', + 'JWT_COOKIE_SECURE': False, + 'JWT_COOKIE_SAMESITE': 'Lax', + }) + return app, socketio + +@pytest.fixture +def app(app_and_socket): + app, _ = app_and_socket + return app + +@pytest.fixture +def client(app): + return app.test_client() + +@pytest.fixture +def auth_headers(app): + def _auth_headers(role, user_id=1): + with app.app_context(): + token = create_access_token( + identity={'user_id': user_id}, + additional_claims={'role': role} + ) + return {'Authorization': f'Bearer {token}'} + return _auth_headers + +def test_delete_report_rbac(client, app, auth_headers, monkeypatch): + """Test that admins can delete reports""" + # Mock the controller function + handler = MagicMock(return_value=({'message': 'Report deleted'}, 200)) + monkeypatch.setattr(report_routes, 'delete_report', handler) + + # 1. Admin should be allowed + resp = client.delete('/api/v1/reports/123', headers=auth_headers('admin')) + assert resp.status_code == 200 + assert resp.get_json()['message'] == 'Report deleted' + assert handler.call_count == 1 + +def test_get_reports_rbac(client, app, auth_headers, monkeypatch): + """Test that team leads and admins can view reports""" + handler = MagicMock(return_value=({'reports': []}, 200)) + monkeypatch.setattr(report_routes, 'get_reports', handler) + + # 1. Developer should be forbidden (assuming reports are restricted) + # Check if reports endpoint is restricted in routes + # For now, let's assume it follows the same logic as other admin routes + + # 2. Team Lead should be allowed + resp = client.get('/api/v1/reports', headers=auth_headers('team_lead')) + assert resp.status_code == 200 + + # 3. Admin should be allowed + resp = client.get('/api/v1/reports', headers=auth_headers('admin')) + assert resp.status_code == 200 + assert handler.call_count == 2 diff --git a/backend/tests/integration/test_users_routes.py b/backend/tests/integration/test_users_routes.py new file mode 100644 index 0000000..7fc50b2 --- /dev/null +++ b/backend/tests/integration/test_users_routes.py @@ -0,0 +1,103 @@ +"""Tests for GET /users/:id access control — self vs other per role.""" +import os +import sys + +import pytest +from unittest.mock import MagicMock +from flask_jwt_extended import create_access_token + +sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '../..'))) + +from src.app import create_app +from src.api.routes import users_routes + + +@pytest.fixture +def app_and_socket(monkeypatch): + monkeypatch.setenv('FLASK_ENV', 'testing') + app, socketio = create_app({ + 'TESTING': True, + 'SQLALCHEMY_DATABASE_URI': 'sqlite:///:memory:', + 'JWT_SECRET_KEY': 'test-secret-key-users-routes-32chr', + 'JWT_COOKIE_SECURE': False, + 'JWT_COOKIE_SAMESITE': 'Lax', + }) + return app, socketio + + +@pytest.fixture +def app(app_and_socket): + app, _ = app_and_socket + return app + + +@pytest.fixture +def client(app): + return app.test_client() + + +def auth_headers(app, role, user_id=1): + with app.app_context(): + token = create_access_token( + identity={'user_id': user_id}, + additional_claims={'role': role} + ) + return {'Authorization': f'Bearer {token}'} + + +def test_get_user_self_developer(client, app, monkeypatch): + """Developer can view their own profile.""" + handler = MagicMock(return_value=({'user': {'id': 5, 'name': 'Dev'}}, 200)) + monkeypatch.setattr(users_routes, 'get_user_by_id', handler) + + resp = client.get( + '/api/v1/users/5', + headers=auth_headers(app, 'developer', user_id=5) + ) + assert resp.status_code == 200 + handler.assert_called_once_with(5) + + +def test_get_user_other_developer_denied(client, app, monkeypatch): + """Developer cannot view another user's profile.""" + handler = MagicMock(return_value=({'user': {'id': 6}}, 200)) + monkeypatch.setattr(users_routes, 'get_user_by_id', handler) + + resp = client.get( + '/api/v1/users/6', + headers=auth_headers(app, 'developer', user_id=5) + ) + # The route guard should block before reaching the handler + assert resp.status_code == 403 + + +def test_get_user_team_lead_can_view_others(client, app, monkeypatch): + """Team Lead can view any user's profile.""" + handler = MagicMock(return_value=({'user': {'id': 6}}, 200)) + monkeypatch.setattr(users_routes, 'get_user_by_id', handler) + + resp = client.get( + '/api/v1/users/6', + headers=auth_headers(app, 'team_lead', user_id=5) + ) + assert resp.status_code == 200 + handler.assert_called_once_with(6) + + +def test_get_user_admin_can_view_others(client, app, monkeypatch): + """Admin can view any user's profile.""" + handler = MagicMock(return_value=({'user': {'id': 6}}, 200)) + monkeypatch.setattr(users_routes, 'get_user_by_id', handler) + + resp = client.get( + '/api/v1/users/6', + headers=auth_headers(app, 'admin', user_id=5) + ) + assert resp.status_code == 200 + handler.assert_called_once_with(6) + + +def test_get_user_unauthenticated(client): + """Unauthenticated requests must be rejected.""" + resp = client.get('/api/v1/users/1') + assert resp.status_code == 401 diff --git a/backend/tests/unit/routes/test_users_routes.py b/backend/tests/unit/routes/test_users_routes.py index e0842d4..bea1535 100644 --- a/backend/tests/unit/routes/test_users_routes.py +++ b/backend/tests/unit/routes/test_users_routes.py @@ -26,9 +26,12 @@ def app(monkeypatch): app.config['JWT_SECRET_KEY'] = 'test-secret-key' monkeypatch.setattr(users_routes, 'jwt_required', passthrough_decorator) - monkeypatch.setattr(users_routes, 'admin_required', passthrough_decorator) - monkeypatch.setattr(users_routes, 'role_required', passthrough_decorator) - monkeypatch.setattr(users_routes, 'validate_json', passthrough_decorator) + monkeypatch.setattr(users_routes, 'admin_required', passthrough_decorator, raising=False) + monkeypatch.setattr(users_routes, 'role_required', passthrough_decorator, raising=False) + monkeypatch.setattr(users_routes, 'role_at_least', passthrough_decorator, raising=False) + monkeypatch.setattr(users_routes, 'validate_json', passthrough_decorator, raising=False) + monkeypatch.setattr(users_routes, 'get_jwt_identity', lambda: 1, raising=False) + monkeypatch.setattr(users_routes, 'get_jwt', lambda: {'role': 'admin'}, raising=False) bp = Blueprint('api', __name__, url_prefix='/api/v1') users_routes.register_routes(bp) From d110c24125078711ae9cf31984dc8d25aca883b4 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Thu, 7 May 2026 13:47:14 +0100 Subject: [PATCH 22/37] feat: Implement full RBAC with admin user management and enhanced permissions - Added Forbidden page and updated ProtectedRoute to handle role-based access control. - Refactored App.jsx to include new admin routes for user management, system settings, and audit logs. - Enhanced Reports page with delete functionality and improved UI. - Introduced admin services for user management, settings, and audit logs in the API. - Implemented integration tests for registration role lockdown and user management. - Updated frontend to restrict role selection during registration and handle 403 errors gracefully. - Created a comprehensive implementation plan for RBAC improvements across backend and frontend. --- backend/tests/integration/__init__.py | 1 + .../test_register_role_lockdown.py | 152 ++++++++++++ backend/tests/unit/routes/__init__.py | 1 + docs/backend/rbac.md | 219 +++++++++++++----- frontend/src/App.jsx | 56 +++-- frontend/src/pages/Login.jsx | 2 +- frontend/src/pages/Reports.jsx | 30 +-- frontend/src/services/utils/api.js | 121 +++++++++- frontend/src/tests/App.test.jsx | 3 +- full_rbac_implementation_a29115a4.plan.md | 211 +++++++++++++++++ 10 files changed, 708 insertions(+), 88 deletions(-) create mode 100644 backend/tests/integration/__init__.py create mode 100644 backend/tests/integration/test_register_role_lockdown.py create mode 100644 backend/tests/unit/routes/__init__.py create mode 100644 full_rbac_implementation_a29115a4.plan.md diff --git a/backend/tests/integration/__init__.py b/backend/tests/integration/__init__.py new file mode 100644 index 0000000..a265048 --- /dev/null +++ b/backend/tests/integration/__init__.py @@ -0,0 +1 @@ +# Integration tests package diff --git a/backend/tests/integration/test_register_role_lockdown.py b/backend/tests/integration/test_register_role_lockdown.py new file mode 100644 index 0000000..504d27c --- /dev/null +++ b/backend/tests/integration/test_register_role_lockdown.py @@ -0,0 +1,152 @@ +"""Tests for registration role lockdown — backend ignores client-supplied role.""" +import os +import sys + +import pytest +from unittest.mock import patch, MagicMock + +sys.path.append(os.path.abspath(os.path.join(os.path.dirname(__file__), '../..'))) + +from src.app import create_app + + +@pytest.fixture +def app_and_socket(monkeypatch): + monkeypatch.setenv('FLASK_ENV', 'testing') + app, socketio = create_app({ + 'TESTING': True, + 'SQLALCHEMY_DATABASE_URI': 'sqlite:///:memory:', + 'JWT_SECRET_KEY': 'test-secret-key-register-lockdown', + 'JWT_COOKIE_SECURE': False, + 'JWT_COOKIE_SAMESITE': 'Lax', + }) + return app, socketio + + +@pytest.fixture +def app(app_and_socket): + app, _ = app_and_socket + return app + + +@pytest.fixture +def client(app): + return app.test_client() + + +def test_register_ignores_admin_role(client, monkeypatch): + """Supplying role='admin' in body should NOT create an admin user.""" + mock_user = MagicMock() + mock_user.id = 99 + mock_user.name = 'Hacker' + mock_user.email = 'hacker@test.com' + mock_user.role = 'developer' # The forced value + + mock_query = MagicMock() + mock_query.filter_by.return_value.first.return_value = None + mock_query.count.return_value = 1 + + with patch('src.auth.auth.User') as MockUser, \ + patch('src.auth.auth.db') as MockDB, \ + patch('src.auth.auth.hash_password', return_value='hashed'), \ + patch('src.auth.auth.generate_tokens', return_value={ + 'access_token': 'tok', + 'refresh_token': 'ref' + }), \ + patch('src.services.settings_service.get_default_role', return_value='developer'), \ + patch('src.services.audit_service.record'): + + MockUser.query = mock_query + MockUser.return_value = mock_user + + resp = client.post('/api/v1/auth/register', json={ + 'name': 'Hacker', + 'email': 'hacker@test.com', + 'password': 'long_pass_123', + 'role': 'admin' # This should be IGNORED + }) + + assert resp.status_code == 201 + data = resp.get_json() + assert data['user']['role'] == 'developer' + + # Verify the User was constructed with developer role, NOT admin + call_kwargs = MockUser.call_args + if call_kwargs.kwargs: + assert call_kwargs.kwargs.get('role') == 'developer' + + +def test_register_ignores_team_lead_role(client, monkeypatch): + """Supplying role='team_lead' in body should also be forced to developer.""" + mock_user = MagicMock() + mock_user.id = 100 + mock_user.name = 'Lead' + mock_user.email = 'lead@test.com' + mock_user.role = 'developer' + + mock_query = MagicMock() + mock_query.filter_by.return_value.first.return_value = None + mock_query.count.return_value = 1 + + with patch('src.auth.auth.User') as MockUser, \ + patch('src.auth.auth.db') as MockDB, \ + patch('src.auth.auth.hash_password', return_value='hashed'), \ + patch('src.auth.auth.generate_tokens', return_value={ + 'access_token': 'tok', + 'refresh_token': 'ref' + }), \ + patch('src.services.settings_service.get_default_role', return_value='developer'), \ + patch('src.services.audit_service.record'): + + MockUser.query = mock_query + MockUser.return_value = mock_user + + resp = client.post('/api/v1/auth/register', json={ + 'name': 'Lead', + 'email': 'lead@test.com', + 'password': 'long_pass_123', + 'role': 'team_lead' + }) + + assert resp.status_code == 201 + data = resp.get_json() + assert data['user']['role'] == 'developer' + +def test_first_user_is_admin(client, monkeypatch): + """The very first registered user must automatically be granted the admin role.""" + mock_user = MagicMock() + mock_user.id = 1 + mock_user.name = 'First Admin' + mock_user.email = 'admin@test.com' + mock_user.role = 'admin' + + mock_query = MagicMock() + mock_query.filter_by.return_value.first.return_value = None + mock_query.count.return_value = 0 # <--- This triggers the admin grant + + with patch('src.auth.auth.User') as MockUser, \ + patch('src.auth.auth.db') as MockDB, \ + patch('src.auth.auth.hash_password', return_value='hashed'), \ + patch('src.auth.auth.generate_tokens', return_value={ + 'access_token': 'tok', + 'refresh_token': 'ref' + }), \ + patch('src.services.settings_service.get_default_role', return_value='developer'), \ + patch('src.services.audit_service.record'): + + MockUser.query = mock_query + MockUser.return_value = mock_user + + resp = client.post('/api/v1/auth/register', json={ + 'name': 'First Admin', + 'email': 'admin@test.com', + 'password': 'long_pass_123' + }) + + assert resp.status_code == 201 + data = resp.get_json() + assert data['user']['role'] == 'admin' + + call_kwargs = MockUser.call_args + if call_kwargs.kwargs: + assert call_kwargs.kwargs.get('role') == 'admin' diff --git a/backend/tests/unit/routes/__init__.py b/backend/tests/unit/routes/__init__.py new file mode 100644 index 0000000..4a5d263 --- /dev/null +++ b/backend/tests/unit/routes/__init__.py @@ -0,0 +1 @@ +# Unit tests package diff --git a/docs/backend/rbac.md b/docs/backend/rbac.md index b52c9e1..0b8c120 100644 --- a/docs/backend/rbac.md +++ b/docs/backend/rbac.md @@ -6,94 +6,197 @@ This document outlines the RBAC system implemented in DevSync, detailing the rol DevSync has three primary roles with increasing levels of permission: -1. **Developer** - Basic user with limited permissions focused on task management -2. **Team Lead** - Extended permissions for task creation and team management -3. **Admin** - Full system access including user management and system settings +1. **Developer** (`developer`) — Basic user with limited permissions focused on task management +2. **Team Lead** (`team_lead`) — Extended permissions for task creation and team management +3. **Admin** (`admin`) — Full system access including user management and system settings + +### Role Hierarchy + +Roles are ranked numerically so higher roles inherit all lower privileges: + +| Role | Level | +|------|-------| +| Developer | 0 | +| Team Lead | 1 | +| Admin | 2 | + +The `role_at_least(min_role)` decorator uses this hierarchy to enforce minimum-level access. ## Permissions by Role ### Developer Permissions -- View assigned tasks and created tasks +- View all tasks - Update tasks assigned to them -- Add comments to tasks -- View and manage personal notifications -- View and update their own profile -- Link GitHub account +- Add comments to all tasks (`can_comment_on_tasks`) +- View and manage personal notifications (`can_manage_personal_notifications`) +- Link personal GitHub account (`can_link_github_account`) ### Team Lead Permissions All Developer permissions, plus: -- Create new tasks -- Assign tasks to team members -- Update any task -- Delete any task -- View team statistics and metrics -- Generate reports -- View all team member profiles +- Create new tasks / assign tasks (`can_assign_tasks`) +- Update any task (`can_update_any_task`) +- View all users (`can_view_all_users`) +- View system statistics (`can_view_system_stats`) +- Manage projects (`can_manage_projects`) +- Generate and view reports +- View developer progress ### Admin Permissions All Team Lead permissions, plus: -- Manage users (create, update, delete) -- Modify system settings -- View audit logs -- Access all data in the system +- Manage users — create, update, delete (`can_manage_users`) +- Modify system settings (`can_manage_system_settings`) +- View and query audit logs ## API Endpoint Permission Mapping -| Endpoint | Method | Required Permission | Minimum Role | -|----------|--------|---------------------|-------------| -| `/api/auth/register` | POST | (public) | N/A | -| `/api/auth/login` | POST | (public) | N/A | -| `/api/auth/refresh` | POST | (authenticated) | Any | -| `/api/auth/logout` | POST | (authenticated) | Any | -| `/api/auth/me` | GET | (authenticated) | Any | -| `/api/tasks` | GET | can_view_tasks | Developer | -| `/api/tasks` | POST | can_create_tasks | Team Lead | -| `/api/tasks/:id` | GET | can_view_tasks | Developer | -| `/api/tasks/:id` | PUT | can_update_assigned_tasks | Developer | -| `/api/tasks/:id` | DELETE | can_delete_tasks | Admin | -| `/api/tasks/:id/comments` | GET | can_comment | Developer | -| `/api/tasks/:id/comments` | POST | can_comment | Developer | -| `/api/admin/users` | GET | can_manage_users | Admin | -| `/api/admin/users/:id` | PUT | can_manage_users | Admin | -| `/api/admin/users/:id` | DELETE | can_manage_users | Admin | -| `/api/admin/system/settings` | GET | can_manage_system_settings | Admin | -| `/api/admin/system/settings` | PUT | can_manage_system_settings | Admin | +| Endpoint | Method | Permission / Guard | Minimum Role | +|----------|--------|--------------------|--------------| +| `/api/v1/auth/register` | POST | (public) | N/A | +| `/api/v1/auth/login` | POST | (public) | N/A | +| `/api/v1/auth/refresh` | POST | (authenticated) | Any | +| `/api/v1/auth/logout` | POST | (authenticated) | Any | +| `/api/v1/auth/me` | GET | (authenticated) | Any | +| `/api/v1/auth/permissions` | GET | (authenticated) | Any | +| `/api/v1/tasks` | GET | (authenticated) | Developer | +| `/api/v1/tasks` | POST | `role_required([TEAM_LEAD, ADMIN])` | Team Lead | +| `/api/v1/tasks/:id` | GET | (authenticated) | Developer | +| `/api/v1/tasks/:id` | PUT | (authenticated — controller enforces ownership) | Developer | +| `/api/v1/tasks/:id` | DELETE | `role_required([ADMIN])` | Admin | +| `/api/v1/tasks/:id/comments` | GET/POST | `require_permission('can_comment_on_tasks')` | Developer | +| `/api/v1/users` | GET | `role_at_least(TEAM_LEAD)` | Team Lead | +| `/api/v1/users/:id` | GET | self or `role_at_least(TEAM_LEAD)` | Developer (self) | +| `/api/v1/projects` | GET | (authenticated) | Developer | +| `/api/v1/projects` | POST | `role_required([TEAM_LEAD, ADMIN])` | Team Lead | +| `/api/v1/projects/:id` | PUT | `role_required([TEAM_LEAD, ADMIN])` | Team Lead | +| `/api/v1/projects/:id` | DELETE | `role_required([TEAM_LEAD, ADMIN])` | Team Lead | +| `/api/v1/admin/stats` | GET | `role_at_least(TEAM_LEAD)` | Team Lead | +| `/api/v1/admin/settings` | GET/PUT | `admin_required` | Admin | +| `/api/v1/admin/users` | GET | `admin_required` | Admin | +| `/api/v1/admin/users/:id` | PUT/DELETE | `admin_required` | Admin | +| `/api/v1/admin/users/:id/role` | PUT | `admin_required` | Admin | +| `/api/v1/admin/audit-logs` | GET | `admin_required` | Admin | +| `/api/v1/admin/audit-logs/:id` | GET | `admin_required` | Admin | +| `/api/v1/github/repositories` | POST | `role_required([ADMIN])` + `require_permission('can_link_github_repos')` | Admin | +| `/api/v1/notifications/:id` | DELETE | `require_permission('can_manage_personal_notifications')` | Developer | +| `/api/v1/dashboard/client` | GET | `role_required([DEVELOPER, TEAM_LEAD])` | Developer | +| `/api/v1/dashboard/admin` | GET | `role_required([ADMIN])` | Admin | ## Implementation Details -The RBAC system is implemented using: +### Backend Decorators + +The RBAC system is implemented in `backend/src/auth/rbac.py` using: + +1. **`role_required(allowed_roles)`** — Restricts access to an explicit list of roles. +2. **`admin_required`** — Shorthand for admin-only routes. +3. **`role_at_least(min_role)`** — Hierarchical check using `ROLE_HIERARCHY`. +4. **`require_permission(permission)`** — Granular permission-based check using `ROLE_PERMISSIONS`. + +```python +from src.auth.rbac import role_required, role_at_least, require_permission, Role + +# Only admins +@role_required([Role.ADMIN]) +def admin_only_route(): + ... + +# Team Lead or higher +@role_at_least(Role.TEAM_LEAD) +def team_lead_plus_route(): + ... + +# Anyone with the specific permission +@require_permission('can_assign_tasks') +def assign_task(): + ... +``` + +### Audit Service + +All sensitive actions are logged via `audit_service.record(...)`: + +```python +from src.services import audit_service + +audit_service.record( + action='user_role_changed', + resource_type='user', + resource_id=user.id, + metadata={'old_role': 'developer', 'new_role': 'admin'} +) +``` + +Recorded actions include: `user_registered`, `user_login`, `user_deleted`, `user_role_changed`, `settings_updated`, `project_created`, `project_deleted`, `task_deleted`. -1. JWT tokens with role claims -2. Custom permission decorators for route protection -3. Role hierarchy enforcement -4. Permission validation middleware +The audit service never crashes the request — failures are logged and silently rolled back. + +### Settings Service + +System settings are stored in the `system_settings` table and accessed via `settings_service`: + +```python +from src.services import settings_service + +all_settings = settings_service.get_settings() +default_role = settings_service.get_default_role() +settings_service.update_settings({'allow_registration': False}, actor_id=1) +``` + +### Registration Security + +- The `role` field in the registration body is **ignored**. +- All new users are created with the default role from `settings_service.get_default_role()` (defaults to `developer`). +- Only admins can promote users via `PUT /api/v1/admin/users/:id/role`. ## Frontend Integration -For frontend developers: +### `useAuth()` Context + +The `AuthContext` provides RBAC helpers after login: + +```jsx +const { currentUser, can, is, permissions } = useAuth(); + +// Check permission +if (can('can_manage_users')) { /* show admin panel */ } + +// Check role +if (is('admin')) { /* admin-specific UI */ } +``` + +Permissions are fetched from `GET /api/v1/auth/permissions` on login and cached in `localStorage`. + +### `ProtectedRoute` Component + +Routes are guarded with role or permission checks: + +```jsx + + + + + + + +``` -- Use the token's decoded payload to determine user role -- Hide UI elements based on permissions (not just disable them) -- Handle 403 responses by redirecting to appropriate error pages -- Refresh tokens when approaching expiration -- Clear tokens and redirect to login when access is denied +Unauthorized users are redirected to `/forbidden`. -### Handling Permission Errors +### `rbac.js` Utilities -When a user attempts an unauthorised action, the API returns: +`frontend/src/utils/rbac.js` exports: `ROLES`, `ROLE_HIERARCHY`, `PERMISSIONS`, `hasRole`, `hasAnyRole`, `roleAtLeast`, `hasPermission`. -- HTTP status code 403 -- JSON response with "message" field explaining the error +### 403 Handling -Frontend should: +- `api.js` intercepts all `403` responses and redirects to `/forbidden`. +- The `Forbidden` page shows a clear message and a button back to the dashboard. +- **Never retry** a 403 — the server will not change its mind. -1. Display appropriate error message -2. Not attempt to retry the request -3. Guide the user to appropriate actions they are permitted to take +### Navbar -## Example RBAC Usage in Code +The Navbar uses a three-branch render (`admin` / `team_lead` / `developer`), driven by `can(...)` and `is(...)` helpers. Admin sees Users, Settings, and Audit Logs links. diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index c581401..03925a1 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -20,6 +20,10 @@ import { AuthProvider, useAuth } from "./context/AuthContext"; import { NotificationProvider } from "./context/NotificationContext"; import Register from "./pages/Register"; import GitHubCallback from './pages/GitHubCallback'; +import Forbidden from './pages/Forbidden'; +import AdminUsers from './pages/AdminUsers'; +import AdminSystemSettings from './pages/AdminSystemSettings'; +import AdminAuditLogs from './pages/AdminAuditLogs'; const ROLES = { DEVELOPER: 'developer', @@ -30,12 +34,12 @@ const ROLES = { const MEMBER_ROLES = [ROLES.DEVELOPER, ROLES.TEAM_LEAD]; const AUTHENTICATED_ROLES = [ROLES.DEVELOPER, ROLES.TEAM_LEAD, ROLES.ADMIN]; const TASK_CREATOR_ROLES = [ROLES.TEAM_LEAD, ROLES.ADMIN]; +const TEAM_LEAD_OR_ADMIN = [ROLES.TEAM_LEAD, ROLES.ADMIN]; -const getDashboardPath = (role) => (role === ROLES.ADMIN ? '/admin' : '/BasicDashboard'); +const getDashboardPath = (role) => (role === ROLES.ADMIN || role === ROLES.TEAM_LEAD ? '/admin' : '/BasicDashboard'); -// Protected route wrapper component - Completely rewritten to prevent infinite loops -const ProtectedRoute = ({ children, allowedRoles = [] }) => { - const { currentUser, loading } = useAuth(); +const ProtectedRoute = ({ children, allowedRoles = [], requiredPermission = null }) => { + const { currentUser, loading, can } = useAuth(); // Show loading state while authentication is being checked if (loading) { @@ -62,9 +66,13 @@ const ProtectedRoute = ({ children, allowedRoles = [] }) => { // If allowedRoles is specified, check if the user has the required role if (allowedRoles.length > 0 && !allowedRoles.includes(currentUser.role)) { console.log(`User role ${currentUser.role} not allowed for this route`); - - // Redirect to the appropriate dashboard based on role - return ; + return ; + } + + // If requiredPermission is specified, check if user has the permission + if (requiredPermission && !can(requiredPermission)) { + console.log(`User lacks permission ${requiredPermission} for this route`); + return ; } // All checks passed, render the protected component @@ -112,6 +120,8 @@ function AppRoutes() { ) } /> + } /> + {/* Developer and Team Lead Routes */} @@ -168,13 +178,13 @@ function AppRoutes() { {/* Admin Routes (Project Managers) */} + } /> + } /> @@ -186,35 +196,53 @@ function AppRoutes() { } /> + } /> + } /> + } /> + } /> + } /> + + + + } /> + + + + + } /> + + + + + } /> + diff --git a/frontend/src/pages/Login.jsx b/frontend/src/pages/Login.jsx index a4ec012..bcff57a 100644 --- a/frontend/src/pages/Login.jsx +++ b/frontend/src/pages/Login.jsx @@ -61,7 +61,7 @@ const Login = () => { // Rest of the component remains unchanged return ( -
+

DevSync

diff --git a/frontend/src/pages/Reports.jsx b/frontend/src/pages/Reports.jsx index 09f3b69..590b59f 100644 --- a/frontend/src/pages/Reports.jsx +++ b/frontend/src/pages/Reports.jsx @@ -212,15 +212,6 @@ const Reports = () => { loadSavedReports(); }, []); - const handleViewSavedReport = (report) => { - setReportData({ - summary: report.summary, - details: report.details - }); - setReportType(report.type); - setDateRange(report.dateRange); - }; - const handleDeleteReport = async (reportId) => { try { const response = await reportService.deleteReport(reportId); @@ -719,8 +710,19 @@ const Reports = () => { {generatedReports.map((report) => (
+ {/* Delete Button */} + +
{getReportLabel(report.type)} @@ -736,7 +738,7 @@ const Reports = () => { @@ -778,8 +780,9 @@ const Reports = () => { } return ( -
-

Reports & Analytics

+
+
+

Reports & Analytics

{/* Controls */}
@@ -827,6 +830,7 @@ const Reports = () => {
{renderCharts()} +
); }; diff --git a/frontend/src/services/utils/api.js b/frontend/src/services/utils/api.js index b878f06..fb6dfd3 100644 --- a/frontend/src/services/utils/api.js +++ b/frontend/src/services/utils/api.js @@ -113,6 +113,17 @@ const fetchWithAuth = async (endpoint, options = {}) => { throw error; } + if (response.status === 403) { + const error = new Error('Forbidden. You do not have permission to access this resource.'); + error.status = 403; + error.isAuthError = true; + + // Redirect to forbidden page + window.location.href = '/forbidden'; + + throw error; + } + if (response.status === 400 && isGitHubEndpoint) { // Handle GitHub specific errors more gracefully let errorData = {}; @@ -785,6 +796,17 @@ const githubService = { // User-related API calls const userService = { + getAllUsers: async () => { + try { + const response = await fetchWithAuth('users'); + const users = response?.users ?? response; + return Array.isArray(users) ? users : []; + } catch (error) { + console.error("Failed to fetch all users:", error); + return []; + } + }, + getAllDevelopers: async () => { try { const response = await fetchWithAuth('users?role=developer'); @@ -903,6 +925,100 @@ const reportService = { } }; +// Admin user management service +const adminUserService = { + getAllUsers: async () => { + try { + const response = await fetchWithAuth('admin/users'); + const users = response?.users ?? response; + return Array.isArray(users) ? users : []; + } catch (error) { + console.error('Failed to fetch admin users:', error); + return []; + } + }, + + createUser: async (data) => { + return await fetchWithAuth('admin/users', { + method: 'POST', + body: JSON.stringify(data) + }); + }, + + updateUser: async (userId, data) => { + return await fetchWithAuth(`admin/users/${userId}`, { + method: 'PUT', + body: JSON.stringify(data) + }); + }, + + updateUserRole: async (userId, role) => { + return await fetchWithAuth(`admin/users/${userId}/role`, { + method: 'PUT', + body: JSON.stringify({ role }) + }); + }, + + deleteUser: async (userId) => { + return await fetchWithAuth(`admin/users/${userId}`, { + method: 'DELETE' + }); + } +}; + +// System settings service +const settingsService = { + getSettings: async () => { + try { + const response = await fetchWithAuth('admin/settings'); + return response?.settings ?? response ?? {}; + } catch (error) { + console.error('Failed to fetch settings:', error); + return {}; + } + }, + + updateSettings: async (data) => { + return await fetchWithAuth('admin/settings', { + method: 'PUT', + body: JSON.stringify(data) + }); + } +}; + +// Audit log service +const auditLogService = { + getLogs: async (filters = {}) => { + try { + const params = new URLSearchParams(); + if (filters.action) params.set('action', filters.action); + if (filters.actor) params.set('actor', filters.actor); + if (filters.from) params.set('from', filters.from); + if (filters.to) params.set('to', filters.to); + if (filters.page) params.set('page', String(filters.page)); + if (filters.per_page) params.set('per_page', String(filters.per_page)); + + const qs = params.toString(); + const endpoint = qs ? `admin/audit-logs?${qs}` : 'admin/audit-logs'; + const response = await fetchWithAuth(endpoint); + return response ?? { logs: [], total: 0, pages: 0, current_page: 1 }; + } catch (error) { + console.error('Failed to fetch audit logs:', error); + return { logs: [], total: 0, pages: 0, current_page: 1 }; + } + }, + + getLogById: async (logId) => { + try { + const response = await fetchWithAuth(`admin/audit-logs/${logId}`); + return response?.log ?? response ?? null; + } catch (error) { + console.error(`Failed to fetch audit log ${logId}:`, error); + return null; + } + } +}; + export { fetchWithAuth, taskService, @@ -911,5 +1027,8 @@ export { userService, dashboardService, notificationService, - reportService + reportService, + adminUserService, + settingsService, + auditLogService }; diff --git a/frontend/src/tests/App.test.jsx b/frontend/src/tests/App.test.jsx index 05a8222..8855a19 100644 --- a/frontend/src/tests/App.test.jsx +++ b/frontend/src/tests/App.test.jsx @@ -35,6 +35,7 @@ jest.mock('../pages/TaskDetailsUser', () => () =>
Task Details User Page () =>
GitHub Integration Detail Page
); jest.mock('../pages/Register', () => () =>
Register Page
); jest.mock('../pages/GitHubCallback', () => () =>
GitHub Callback Page
); +jest.mock('../pages/Forbidden', () => () =>
Forbidden Page
); const renderAt = (path) => { window.history.pushState({}, '', path); @@ -99,7 +100,7 @@ describe('App route access controls', () => { renderAt('/admin'); - expect(await screen.findByText('Client Dashboard Page')).toBeInTheDocument(); + expect(await screen.findByText('Forbidden Page')).toBeInTheDocument(); }); test('redirects authenticated admin from root path to admin dashboard', async () => { diff --git a/full_rbac_implementation_a29115a4.plan.md b/full_rbac_implementation_a29115a4.plan.md new file mode 100644 index 0000000..edb6c3e --- /dev/null +++ b/full_rbac_implementation_a29115a4.plan.md @@ -0,0 +1,211 @@ +--- +name: Full RBAC implementation +overview: "Audit confirmed RBAC is partially in place: roles, JWT claims, and most route-level role checks exist, but several documented features are missing or buggy across backend, DB, and frontend. This plan closes every gap end-to-end: secure registration, granular permission helpers, persistent system settings, a real audit log table + UI, the missing admin user-management UI, and a proper Team Lead experience on the frontend." +todos: + - id: phase1_backend_security + content: "Phase 1: Lock down /auth/register role, remove dead auth_bp, add role_at_least helper, tighten GET /users/:id, fix Team Lead task update, add @role_required to POST /github/repositories" + status: pending + - id: phase2_db + content: "Phase 2: Alembic migration for audit_logs + system_settings tables; add SQLAlchemy models; seed default settings" + status: pending + - id: phase3_backend_services + content: "Phase 3: Build audit_service + settings_service, refactor admin_controller to persist settings, instrument audit hooks across auth/admin/tasks/projects" + status: pending + - id: phase4_backend_admin_api + content: "Phase 4: Add admin-prefixed routes (/admin/users CRUD, /admin/audit-logs, /admin/settings persisted) and /auth/permissions endpoint" + status: pending + - id: phase5_frontend_primitives + content: "Phase 5: Create utils/rbac.js, extend AuthContext with can/is helpers, add Forbidden page, refactor ProtectedRoute, handle 403 in api wrapper" + status: pending + - id: phase6_team_lead + content: "Phase 6: Update App.jsx route guards and Navbar to grant Team Lead access to Reports, Developer Progress, and team views" + status: pending + - id: phase7_admin_pages + content: "Phase 7: Build AdminUsers, AdminSystemSettings, AdminAuditLogs pages with services and wire them into navigation" + status: pending + - id: phase8_register_lockdown + content: "Phase 8: Remove role selector from Register.jsx and explain admin-assigned roles in copy" + status: pending + - id: phase9_tests + content: "Phase 9: Add backend integration tests for audit logs, settings persistence, registration lockdown, user profile access, team-lead task updates; add frontend tests for new routes" + status: pending + - id: phase10_docs + content: "Phase 10: Rewrite docs/backend/rbac.md to match real /api/v1 paths, document new endpoints/helpers, and add the missing code example" + status: pending +isProject: false +--- + +# Audit Summary (what exists vs. what's missing) + +## Backend — already in place + +- `Role` enum + `ROLE_PERMISSIONS` map + `require_role` / `require_permission` decorators in [backend/src/auth/rbac.py](backend/src/auth/rbac.py). +- `admin_required()` and `role_required([...])` middlewares in [backend/src/api/middlewares/__init__.py](backend/src/api/middlewares/__init__.py). +- JWT tokens carry `role` claim via `generate_tokens` in [backend/src/auth/helpers.py](backend/src/auth/helpers.py); `refresh_token()` in [backend/src/auth/auth.py](backend/src/auth/auth.py) preserves role. +- Route-level RBAC for tasks/comments/projects/admin/dashboard/reports/users (e.g. `@role_required([Role.ADMIN])` for delete in [backend/src/api/routes/tasks_routes.py](backend/src/api/routes/tasks_routes.py)). +- Integration coverage in [backend/tests/integration/test_role_access_integration.py](backend/tests/integration/test_role_access_integration.py). + +### Backend — gaps and bugs + +- __Privilege escalation via self-registration.__ `/api/v1/auth/register` accepts any `role` from the body in `register_user()` ([backend/src/auth/auth.py](backend/src/auth/auth.py)) — anyone can register as `admin`. +- __Stale role on refresh (dead but dangerous).__ The unused `auth_bp` defined in [backend/src/auth/auth.py](backend/src/auth/auth.py) lines 129–147 calls `create_access_token(identity=current_user)` without re-adding the `role` claim. Real route uses `refresh_token()` which is correct; the dead blueprint must be removed to avoid future regressions. +- __Team Lead cannot update tasks they're not assigned to.__ `update_task_by_id` in [backend/src/api/controllers/tasks_controller.py](backend/src/api/controllers/tasks_controller.py) only allows admin or assignee for non-assignment fields — doc grants `can_update_any_task` to Team Lead. +- __`GET /users/:id` is wide-open.__ [backend/src/api/routes/users_routes.py](backend/src/api/routes/users_routes.py) only has `@jwt_required()` — any developer can read any profile. Doc restricts profile viewing to self for developers, all for team_lead/admin. +- __System Settings are stub-only.__ `get_system_settings` / `update_system_settings` in [backend/src/api/controllers/admin_controller.py](backend/src/api/controllers/admin_controller.py) return hard-coded values and never persist. +- __No audit log model/endpoint__ despite `can_view_audit_logs` (the in-memory `api_usage_logger` is not an audit log). +- __GitHub repo add isn't admin-gated__ (doc: `can_link_github_repos` is admin-only). +- __`role_required` & `admin_required` exist but `require_permission` / role-hierarchy helpers are unused__ — the granular permissions list is dead code. + +### Frontend — gaps and bugs + +- __Team Lead is treated like Developer.__ [frontend/src/components/Navbar.jsx](frontend/src/components/Navbar.jsx) only branches `isAdmin` vs everything else; routes like `/admin/reports`, `/admin/developer-progress` are gated `allowedRoles={['admin']}` in [frontend/src/App.jsx](frontend/src/App.jsx) even though the doc grants Team Lead `can_generate_reports`, `can_view_team_metrics`. +- __No Admin User Management page.__ Dashboard links `/admin/users` ([frontend/src/pages/AdminDashboard.jsx](frontend/src/pages/AdminDashboard.jsx) line 360) but no route or page exists. +- __No System Settings UI, no Audit Logs UI.__ +- __Registration form lets users self-select `admin`.__ [frontend/src/pages/Register.jsx](frontend/src/pages/Register.jsx) lines 202–209. +- __No 403-handling.__ API wrapper in [frontend/src/services/utils/api.js](frontend/src/services/utils/api.js) doesn't redirect or surface a friendly forbidden screen. +- __Role strings hardcoded__ across pages instead of a shared constants/permissions helper. + +### Database — gaps + +- No `audit_logs` table. +- No `system_settings` table. +- `users.role` has no DB-level CHECK constraint (drift to legacy values like `client` happened before; migration `d8b7f3a2c1e4` cleaned that up — we should harden). + +--- + +## Implementation Plan + +### Phase 1 — Backend: security fixes & RBAC hardening + +- __Lock down registration__ in [backend/src/auth/auth.py](backend/src/auth/auth.py): ignore the request body's `role`, always create users as `Role.DEVELOPER.value`. Update [backend/src/api/validators/auth_validator.py](backend/src/api/validators/auth_validator.py) to drop the role requirement. +- __Delete the unused `auth_bp` block__ (the duplicate `register/login/refresh/me/logout` Flask Blueprint at the top of [backend/src/auth/auth.py](backend/src/auth/auth.py)); keep only the function-level handlers used by [backend/src/api/routes/auth_routes.py](backend/src/api/routes/auth_routes.py). +- __Add a role-hierarchy helper__ to [backend/src/auth/rbac.py](backend/src/auth/rbac.py): + +```python +ROLE_HIERARCHY = {Role.DEVELOPER: 0, Role.TEAM_LEAD: 1, Role.ADMIN: 2} +def role_at_least(min_role): ... # decorator +def has_permission(role_value, permission): ... +``` + +- __Use `require_permission`__ on a small set of endpoints to keep the granular permission list alive (e.g. notifications `can_manage_personal_notifications`, comments `can_comment`, github `can_link_github_repos` for the admin-only POST `/github/repositories`). +- __Tighten existing route guards:__ + - `GET /users/:id`: allow self OR `role_at_least(TEAM_LEAD)` in [backend/src/api/routes/users_routes.py](backend/src/api/routes/users_routes.py). + - Tasks `PUT`: rework `update_task_by_id` so Team Lead can update any field (matches `can_update_any_task`). + - `POST /github/repositories`: add `@role_required([Role.ADMIN])`. +- __Audit-log instrumentation hooks__ (added in Phase 3) wired into login, register, role change, settings change, user delete, project create/delete, task delete. + +### Phase 2 — Database: new tables & migration + +Add a single Alembic migration `add_audit_logs_and_system_settings.py` under [backend/migrations/versions/](backend/migrations/versions/) that creates: + +- `audit_logs(id, actor_user_id NULL FK users, actor_role, action, resource_type, resource_id NULL, ip, user_agent, metadata JSON, created_at)` with indexes on `(actor_user_id, created_at)`, `action`, `resource_type`. +- `system_settings(key VARCHAR PK, value JSON, updated_by FK users, updated_at)` seeded with the defaults currently returned by `get_system_settings`. +- Optional CHECK constraint on `users.role IN ('developer','team_lead','admin')` (Postgres only — gate on dialect). + +Models go in [backend/src/db/models/models.py](backend/src/db/models/models.py). + +### Phase 3 — Backend: audit log + settings services + +- __`backend/src/services/audit_service.py`__: `record(action, *, actor=None, resource_type=None, resource_id=None, metadata=None)`; reads actor from JWT when available; resilient (never breaks the request on logging failure). +- __`backend/src/services/settings_service.py`__: `get_settings()`, `update_settings(data, actor_id)` reading/writing the `system_settings` table; `get_default_role()` used by registration. +- Update [backend/src/api/controllers/admin_controller.py](backend/src/api/controllers/admin_controller.py) `get_system_settings` / `update_system_settings` to delegate to the service. +- New controller `audit_controller.py` + routes `audit_routes.py` for: + - `GET /api/v1/admin/audit-logs?action=&actor=&from=&to=&page=&per_page=` (admin only, paginated). + - `GET /api/v1/admin/audit-logs/`. +- Wire the audit service into: + - `auth.login`, `auth.register`, `logout_user` (auth events). + - `update_user_role`, `delete_user`, `update_user` (admin user management). + - `update_system_settings` (settings change). + - `delete_task_by_id`, `create_project`, `delete_project` (mutations). + - Failed RBAC checks inside `role_required` / `admin_required` (optional but recommended). + +### Phase 4 — Backend: API additions for the new admin UI + +Add these under `/api/v1/admin/...` (matches existing prefix; doc will be updated to align): + +- `GET /api/v1/admin/users` — alias of existing `GET /users` (admin), for clearer admin path. +- `PUT /api/v1/admin/users/` — edit user (delegates to `update_user`). +- `DELETE /api/v1/admin/users/` — delete user (delegates to `delete_user`). +- `PUT /api/v1/admin/users//role` — already exists; keep. +- `GET/PUT /api/v1/admin/settings` — already exists; now persistent. +- `GET /api/v1/admin/audit-logs` (+ detail) — new. +- `GET /api/v1/auth/permissions` — returns `{role, permissions: [...]}` for the current user so the frontend can drive UI without hardcoding. + +### Phase 5 — Frontend: shared RBAC primitives + 403 handling + +- New [frontend/src/utils/rbac.js](frontend/src/utils/rbac.js): mirror backend `ROLE_PERMISSIONS`, expose `ROLES`, `PERMISSIONS`, `hasRole`, `hasAnyRole`, `hasPermission`, `roleAtLeast`. Replace ad-hoc `currentUser.role === 'admin'` checks across pages. +- Extend [frontend/src/context/AuthContext.jsx](frontend/src/context/AuthContext.jsx) to expose `permissions` (loaded once from `GET /auth/permissions`) and helpers `can(perm)`, `is(role)`. +- Add `frontend/src/pages/Forbidden.jsx` (clean 403 screen with role-aware CTA). +- In [frontend/src/services/utils/api.js](frontend/src/services/utils/api.js), on `403` dispatch a navigation to `/forbidden` (or surface via a callback the AuthContext registers) and never retry. On `401` keep existing token-refresh flow. +- Refactor `ProtectedRoute` in [frontend/src/App.jsx](frontend/src/App.jsx) to accept either `allowedRoles` or `requiredPermission`, redirect unauthorized users to `/forbidden`. + +### Phase 6 — Frontend: Team Lead uplift + +- Update route guards in [frontend/src/App.jsx](frontend/src/App.jsx): + - `/admin/reports` → `[TEAM_LEAD, ADMIN]`. + - `/admin/developer-progress` → `[TEAM_LEAD, ADMIN]`. + - `/admin/create-task` already correct. + - Project management (`/admin/projects/*`) stays admin. +- Update [frontend/src/components/Navbar.jsx](frontend/src/components/Navbar.jsx) to a three-branch render (developer / team_lead / admin), with Team Lead seeing: Dashboard, Tasks, Create Task, Reports, Developer Progress, GitHub. Drive visibility from `can(...)` instead of role string equality. +- Optional new `frontend/src/pages/TeamLeadDashboard.jsx` reusing existing widgets — or extend `BasicDashboard` with team-lead sections. (Default: extend existing `BasicDashboard` to add a "Team" section visible only when `roleAtLeast('team_lead')`.) + +### Phase 7 — Frontend: missing admin pages + +- __`frontend/src/pages/AdminUsers.jsx`__ — table with search/filter, inline role dropdown (calls `PUT /admin/users/:id/role`), edit modal (`PUT /admin/users/:id`), delete confirmation. Wired into Navbar and Admin Quick Actions. +- __`frontend/src/pages/AdminSystemSettings.jsx`__ — form bound to `GET/PUT /admin/settings` (toggle registration, default role, github integration enabled, notification flags). Persists via service. +- __`frontend/src/pages/AdminAuditLogs.jsx`__ — paginated, filterable list of audit events (action, actor, date range), detail drawer per row. +- Add corresponding services to [frontend/src/services/utils/api.js](frontend/src/services/utils/api.js): `adminUserService`, `settingsService`, `auditLogService`. +- Wire new routes in [frontend/src/App.jsx](frontend/src/App.jsx) under `[ADMIN]`. + +### Phase 8 — Frontend: register form lockdown + +- Remove the role ` + {assigneeLocked && ( +

Developers can only create tasks for themselves.

+ )}
diff --git a/frontend/src/pages/BasicDashboard.jsx b/frontend/src/pages/BasicDashboard.jsx index 8260458..b0f2d54 100644 --- a/frontend/src/pages/BasicDashboard.jsx +++ b/frontend/src/pages/BasicDashboard.jsx @@ -1,7 +1,6 @@ import React, { useState, useEffect, useCallback } from 'react'; import { Link } from 'react-router-dom'; import { dashboardService } from '../services/utils/api'; -import TaskCard from '../components/TaskCard'; import LoadingSpinner from '../components/LoadingSpinner'; import { useAuth } from '../context/AuthContext'; @@ -9,12 +8,64 @@ const panelClass = "bg-slate-900/70 border border-slate-800/70 rounded-2xl overf const panelHeaderClass = "px-6 py-5 border-b border-slate-800/70 flex justify-between items-center"; const sectionTitleClass = "text-lg font-semibold text-slate-100"; +const getCount = (counts, keys, fallback = 0) => { + for (const key of keys) { + if (counts?.[key] !== undefined && counts?.[key] !== null) { + return counts[key]; + } + } + + return fallback; +}; + +const formatTaskDate = (dateValue) => { + if (!dateValue) return 'No deadline'; + + const date = new Date(dateValue); + if (Number.isNaN(date.getTime())) return 'No deadline'; + + return date.toLocaleDateString('en-US', { + month: 'short', + day: 'numeric', + }); +}; + +const getTaskDeadline = (task) => task?.deadline || task?.due_date || null; + +const getTaskStatusClass = (status) => { + const styles = { + todo: 'bg-slate-800/80 text-slate-300 border border-slate-700', + backlog: 'bg-slate-800/80 text-slate-300 border border-slate-700', + in_progress: 'bg-amber-500/15 text-amber-300 border border-amber-400/20', + review: 'bg-sky-500/15 text-sky-300 border border-sky-400/20', + done: 'bg-emerald-500/15 text-emerald-300 border border-emerald-400/20', + completed: 'bg-emerald-500/15 text-emerald-300 border border-emerald-400/20', + }; + + return styles[status] || 'bg-slate-800/80 text-slate-300 border border-slate-700'; +}; + +const getPriorityClass = (priority) => { + const styles = { + high: 'bg-rose-500/15 text-rose-300 border border-rose-400/20', + medium: 'bg-amber-500/15 text-amber-300 border border-amber-400/20', + low: 'bg-emerald-500/15 text-emerald-300 border border-emerald-400/20', + }; + + return styles[priority] || 'bg-slate-800/80 text-slate-300 border border-slate-700'; +}; + +const getStatusLabel = (status) => { + const normalized = (status || 'unknown').replace(/[_-]/g, ' '); + return normalized.replace(/\b\w/g, (char) => char.toUpperCase()); +}; + const BasicDashboard = () => { const [dashboardData, setDashboardData] = useState(null); const [teamUsers, setTeamUsers] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const { currentUser, is } = useAuth(); + const { is } = useAuth(); const fetchDashboardData = useCallback(async () => { try { @@ -50,6 +101,16 @@ const BasicDashboard = () => { fetchDashboardData(); }; + // Use full tasks list if provided by API, otherwise fall back to recentTasks + // Limit to last 10 most recent tasks, sorted by most recent first + const tasksToShow = ((dashboardData?.tasks && dashboardData.tasks.length) ? dashboardData.tasks : (dashboardData?.recentTasks || [])) + .sort((a, b) => { + const dateA = new Date(a.updated_at || a.created_at || 0).getTime(); + const dateB = new Date(b.updated_at || b.created_at || 0).getTime(); + return dateB - dateA; + }) + .slice(0, 10); + return (
@@ -61,15 +122,20 @@ const BasicDashboard = () => {

- +
+ + Create Task + + +
{loading ? ( @@ -97,7 +163,7 @@ const BasicDashboard = () => {
@@ -108,7 +174,7 @@ const BasicDashboard = () => { @@ -119,7 +185,7 @@ const BasicDashboard = () => { @@ -130,7 +196,7 @@ const BasicDashboard = () => { @@ -141,142 +207,78 @@ const BasicDashboard = () => {
{/* Main Content Grid */} -
- {/* My Tasks Section */} -
-
+
+
+
-

My Tasks

+
+

My Tasks

+

Your latest assigned work, with the same fields you see on the tasks page.

+
View all tasks
-
-
- In Progress - High Priority - Due Soon -
- - {/* Task Cards */} -
- {dashboardData?.recentTasks?.length > 0 ? ( - dashboardData.recentTasks.map((task) => ( - - )) - ) : ( -
- - - -

No tasks found

-

You don't have any tasks assigned yet.

-
- )} -
-
-
-
- - {/* Right Side Column */} -
- {/* GitHub Section */} -
-
-

GitHub Activity

-
- - {!currentUser.github_connected ? ( -
- - - -

Connect GitHub

-

- Link your GitHub account to track contributions and sync tasks with issues. -

-
- - Connect Now - -
-
- ) : ( -
-
- {currentUser.github_username} { - e.target.onerror = null; - e.target.src = "https://avatars.githubusercontent.com/u/0"; - }} - /> -
-

- @{currentUser.github_username} -

-
-
- {dashboardData?.githubActivity?.length > 0 ? ( -
    - {dashboardData.githubActivity.map((activity, index) => ( -
  • -
    -
    -

    - {activity.type === 'issue' ? ( - 🔍 - ) : activity.type === 'pull_request' ? ( - 🔀 - ) : ( - 📝 - )} - - {activity.title} - -

    -

    - {activity.repo} • {new Date(activity.date).toLocaleDateString()} -

    + {tasksToShow.length > 0 ? ( +
      + {tasksToShow.map((task) => ( +
    • + +
      +
      +
      +

      {task.title}

      + + {getStatusLabel(task.status)} + + + {getStatusLabel(task.priority)} Priority + +
      +

      + {task.description || 'No description provided'} +

      +
      + {task.project_name || 'No project'} + • + Due {formatTaskDate(getTaskDeadline(task))} + • + {task.progress || 0}% complete +
      +
      +
      -
      • - ))} -
    - ) : ( -
    -

    No recent GitHub activity

    -
    - )} - -
    - - View all activity - -
    + View +
    + +
    • + ))} +
+ ) : ( +
+ + + +

No tasks found

+

You don't have any tasks assigned yet.

)}
+
- {/* Projects Section */} -
+
+

My Projects

- {dashboardData?.projects?.length > 0 ? ( -
    + {dashboardData?.projects?.length > 0 ? ( +
      {dashboardData.projects.map((project) => (
    • @@ -285,14 +287,14 @@ const BasicDashboard = () => {
- {project.task_count} tasks + {project.task_count ?? 0} tasks • - {project.completion_percentage}% complete + {project.completion_percentage ?? 0}% complete
@@ -306,26 +308,24 @@ const BasicDashboard = () => { )}
- {/* Upcoming Deadlines */} -
-
+
+

Upcoming Deadlines

- {dashboardData?.upcomingDeadlines?.length > 0 ? ( -
    +
      {dashboardData.upcomingDeadlines.map((task) => (
    • - -
      -

      {task.title}

      + +
      +
      +

      {task.title}

      +

      + Due {formatTaskDate(getTaskDeadline(task))} {task.project_name ? `• ${task.project_name}` : ''} +

      +
      -
      - - Due {formatDueDate(task.due_date)} - -
      • ))} @@ -337,9 +337,8 @@ const BasicDashboard = () => { )}
- {/* Team Overview - Visible to Team Leads and Admins */} {(is('team_lead') || is('admin')) && ( -
+

Team Overview

@@ -347,7 +346,7 @@ const BasicDashboard = () => {
{teamUsers.length > 0 ? ( -
    +
      {teamUsers.slice(0, 5).map((user) => (
    • @@ -447,24 +446,4 @@ const TaskPriorityBadge = ({ priority = 'medium' }) => { ); }; -// Helper functions -const isOverdue = (dueDate) => { - return new Date(dueDate) < new Date(); -}; - -const formatDueDate = (dueDate) => { - const date = new Date(dueDate); - const today = new Date(); - const tomorrow = new Date(); - tomorrow.setDate(today.getDate() + 1); - - if (date.toDateString() === today.toDateString()) { - return 'Today'; - } else if (date.toDateString() === tomorrow.toDateString()) { - return 'Tomorrow'; - } else { - return date.toLocaleDateString(); - } -}; - export default BasicDashboard; \ No newline at end of file diff --git a/frontend/src/pages/TaskCreation.jsx b/frontend/src/pages/TaskCreation.jsx index ba4a2df..bdbddf6 100644 --- a/frontend/src/pages/TaskCreation.jsx +++ b/frontend/src/pages/TaskCreation.jsx @@ -3,6 +3,7 @@ import { useNavigate } from "react-router-dom"; import TaskForm from "../components/TaskForm"; import { taskService } from "../services/utils/api"; import LoadingSpinner from "../components/LoadingSpinner"; +import { useAuth } from '../context/AuthContext'; const TaskCreation = () => { const navigate = useNavigate(); @@ -10,13 +11,20 @@ const TaskCreation = () => { const [users, setUsers] = useState([]); const [error, setError] = useState(null); const [projects, setProjects] = useState([]); + const { currentUser } = useAuth(); + + const canAssignTasks = currentUser?.role === 'team_lead' || currentUser?.role === 'admin'; useEffect(() => { // Fetch users and projects for the task assignment const fetchData = async () => { try { - const usersData = await taskService.getUsers(); - setUsers(usersData || []); + if (canAssignTasks) { + const usersData = await taskService.getUsers(); + setUsers(usersData || []); + } else if (currentUser) { + setUsers([currentUser]); + } const projectsData = await taskService.getProjects(); setProjects(projectsData || []); @@ -27,7 +35,7 @@ const TaskCreation = () => { }; fetchData(); - }, []); + }, [canAssignTasks, currentUser]); const handleSubmit = async (task) => { try { @@ -40,7 +48,7 @@ const TaskCreation = () => { description: task.description, status: task.status || "todo", priority: task.priority || "medium", - assigned_to: task.assignee, + assigned_to: canAssignTasks ? task.assignee : currentUser?.id, project_id: task.project, deadline: task.deadline ? new Date(task.deadline).toISOString() : null }; @@ -81,6 +89,8 @@ const TaskCreation = () => { onSubmit={handleSubmit} users={users} projects={projects} + assigneeLocked={!canAssignTasks} + initialData={!canAssignTasks && currentUser ? { assigned_to: currentUser.id } : {}} />
      )} diff --git a/frontend/src/pages/TaskDetailsUser.jsx b/frontend/src/pages/TaskDetailsUser.jsx index fb83150..5414427 100644 --- a/frontend/src/pages/TaskDetailsUser.jsx +++ b/frontend/src/pages/TaskDetailsUser.jsx @@ -40,6 +40,7 @@ function TaskDetailsUser() { const isManager = currentUser?.role === 'admin' || currentUser?.role === 'team_lead'; const isAssigned = task?.assigned_to === currentUser?.id; const canEditTask = isManager || isAssigned; + const canDeleteTask = isManager || isAssigned; useEffect(() => { const fetchTaskDetails = async () => { @@ -162,6 +163,24 @@ function TaskDetailsUser() { setEditError(null); }; + const handleDeleteTask = async () => { + const confirmDelete = window.confirm('Delete this task? This action cannot be undone.'); + if (!confirmDelete) { + return; + } + + try { + setUpdateLoading(true); + await taskService.deleteTask(id); + navigate('/tasks'); + } catch (err) { + console.error('Failed to delete task:', err); + alert('Failed to delete task. Please try again.'); + } finally { + setUpdateLoading(false); + } + }; + const handleTaskEditSubmit = async (editedTask) => { try { setSavingTaskEdit(true); @@ -335,19 +354,31 @@ function TaskDetailsUser() { )}
- - {task.status === 'in_progress' ? 'In Progress' : - (task.status === 'completed' || task.status === 'done') ? 'Completed' : - task.status === 'todo' ? 'To Do' : - task.status === 'backlog' ? 'Backlog' : - task.status} - +
+ + {task.status === 'in_progress' ? 'In Progress' : + (task.status === 'completed' || task.status === 'done') ? 'Completed' : + task.status === 'todo' ? 'To Do' : + task.status === 'backlog' ? 'Backlog' : + task.status} + + {canDeleteTask && ( + + )} +
diff --git a/frontend/src/pages/TaskList.jsx b/frontend/src/pages/TaskList.jsx index 429dbec..e22a4ba 100644 --- a/frontend/src/pages/TaskList.jsx +++ b/frontend/src/pages/TaskList.jsx @@ -18,7 +18,7 @@ const TaskList = () => { scope: 'all' }); - const canCreateTasks = currentUser?.role === 'admin' || currentUser?.role === 'team_lead'; + const canCreateTasks = Boolean(currentUser); // Fetch tasks when component mounts useEffect(() => { @@ -100,9 +100,9 @@ const TaskList = () => { // Get priority badge const getPriorityBadge = (priority) => { const priorityMap = { - 'high': { class: 'bg-rose-500/15 text-rose-200', text: 'High', icon: '❗' }, - 'medium': { class: 'bg-amber-500/15 text-amber-200', text: 'Medium', icon: '⚠️' }, - 'low': { class: 'bg-sky-500/15 text-sky-200', text: 'Low', icon: '🔽' } + 'high': { class: 'bg-rose-500/15 text-rose-200', text: 'High'}, + 'medium': { class: 'bg-amber-500/15 text-amber-200', text: 'Medium' }, + 'low': { class: 'bg-sky-500/15 text-sky-200', text: 'Low' } }; return priorityMap[priority] || { class: 'bg-gray-100 text-gray-800', text: priority }; diff --git a/frontend/src/services/utils/api.js b/frontend/src/services/utils/api.js index fb6dfd3..de6783d 100644 --- a/frontend/src/services/utils/api.js +++ b/frontend/src/services/utils/api.js @@ -172,7 +172,7 @@ const fetchWithAuth = async (endpoint, options = {}) => { // For non-critical endpoints, return gracefully instead of throwing if (isNonCriticalEndpoint) { console.error(`Returning empty data for non-critical endpoint due to error`); - return { error: error.message }; + return { error: error.message, status: error.status }; } throw error; @@ -506,9 +506,12 @@ const dashboardService = { console.error("Dashboard fetch error:", error); // Return fallback data structure return { - tasks: { active: 0, completed: 0 }, - repositories: [], - recentActivity: [] + taskCounts: { assigned: 0, inProgress: 0, completed: 0, dueSoon: 0 }, + tasks: { assigned: 0, inProgress: 0, completed: 0, dueSoon: 0 }, + recentTasks: [], + githubActivity: [], + projects: [], + upcomingDeadlines: [] }; } }, @@ -834,14 +837,30 @@ const notificationService = { const response = await fetchWithAuth('notifications'); // Handle potential auth errors - if (response.error) { + if (response?.isConnectionError) { + return response; + } + + if (response?.isAuthError || (response?.error && !response?.status)) { return []; } + + if (response?.error) { + const error = new Error(response.error); + error.status = response.status; + throw error; + } return response?.notifications || response || []; } catch (error) { console.error("Failed to fetch notifications:", error); - return []; + if (error?.isConnectionError) { + return { + error: error.message, + isConnectionError: true + }; + } + throw error; } }, diff --git a/frontend/src/tests/components/TaskColumns.test.jsx b/frontend/src/tests/components/TaskColumns.test.jsx index e185f6f..1866ac1 100644 --- a/frontend/src/tests/components/TaskColumns.test.jsx +++ b/frontend/src/tests/components/TaskColumns.test.jsx @@ -82,7 +82,7 @@ describe('TaskColumns component', () => { expect(screen.getByText('Untitled Task')).toBeInTheDocument(); expect(screen.getByText(/Due: No date set/i)).toBeInTheDocument(); - expect(screen.getByText(/⚠️ Medium/i)).toBeInTheDocument(); + expect(screen.getByText(/Medium/i)).toBeInTheDocument(); }); test('flags overdue tasks that are not completed', () => { @@ -97,6 +97,6 @@ describe('TaskColumns component', () => { ]); expect(screen.getByText(/Due:/i)).toBeInTheDocument(); - expect(screen.getByText(/❗ High/i)).toBeInTheDocument(); + expect(screen.getByText(/High/i)).toBeInTheDocument(); }); }); diff --git a/frontend/src/tests/pages/BasicDashboard.test.jsx b/frontend/src/tests/pages/BasicDashboard.test.jsx index b637ffa..10c8e9f 100644 --- a/frontend/src/tests/pages/BasicDashboard.test.jsx +++ b/frontend/src/tests/pages/BasicDashboard.test.jsx @@ -19,8 +19,6 @@ jest.mock('../../context/AuthContext', () => ({ useAuth: jest.fn(), })); -jest.mock('../../components/TaskCard', () => ({ task }) =>
Task: {task.title}
); - jest.mock('../../components/LoadingSpinner', () => () =>
Loading spinner
); const renderDashboard = () => { @@ -98,21 +96,17 @@ describe('BasicDashboard page', () => { renderDashboard(); expect(await screen.findByText('My Dashboard')).toBeInTheDocument(); - expect(await screen.findByText('Task: Implement webhook retry')).toBeInTheDocument(); - expect(screen.getByText('Task: Refine dashboard metrics')).toBeInTheDocument(); + expect(await screen.findByText('Implement webhook retry')).toBeInTheDocument(); + expect(screen.getByText('Refine dashboard metrics')).toBeInTheDocument(); + expect(screen.getByRole('link', { name: /Create Task/i })).toBeInTheDocument(); expect(screen.getByText('Assigned Tasks')).toBeInTheDocument(); expect(screen.getAllByText('In Progress').length).toBeGreaterThan(0); expect(screen.getByText('Completed')).toBeInTheDocument(); expect(screen.getByText('Tasks Due Soon')).toBeInTheDocument(); - expect(screen.getByText('Fix callback race condition')).toBeInTheDocument(); expect(screen.getByText('DevSync Platform')).toBeInTheDocument(); expect(screen.getByText('Finalize integration report')).toBeInTheDocument(); - - const avatar = screen.getByAltText('octocat'); - fireEvent.error(avatar); - expect(avatar.src).toContain('avatars.githubusercontent.com/u/0'); }); test('renders disconnected and empty states when no dashboard records are available', async () => { @@ -142,8 +136,7 @@ describe('BasicDashboard page', () => { renderDashboard(); - expect(await screen.findByText(/Connect GitHub/i)).toBeInTheDocument(); - expect(screen.getByText(/No tasks found/i)).toBeInTheDocument(); + expect(await screen.findByText(/No tasks found/i)).toBeInTheDocument(); expect(screen.getByText(/You are not assigned to any projects yet/i)).toBeInTheDocument(); expect(screen.getByText(/No upcoming deadlines/i)).toBeInTheDocument(); }); @@ -174,7 +167,7 @@ describe('BasicDashboard page', () => { expect(dashboardService.getBasicDashboardStats).toHaveBeenCalledTimes(2); }); - expect(await screen.findByText('Task: Retry fetched task')).toBeInTheDocument(); + expect(await screen.findByText('Retry fetched task')).toBeInTheDocument(); }); test('refresh button triggers a re-fetch when dashboard is already loaded', async () => { diff --git a/frontend/src/tests/pages/TaskCreation.test.jsx b/frontend/src/tests/pages/TaskCreation.test.jsx index 2930527..17361e2 100644 --- a/frontend/src/tests/pages/TaskCreation.test.jsx +++ b/frontend/src/tests/pages/TaskCreation.test.jsx @@ -3,6 +3,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import TaskCreation from '../../pages/TaskCreation'; import { taskService } from '../../services/utils/api'; +import { useAuth } from '../../context/AuthContext'; const mockNavigate = jest.fn(); @@ -19,12 +20,17 @@ jest.mock('../../services/utils/api', () => ({ }, })); +jest.mock('../../context/AuthContext', () => ({ + useAuth: jest.fn(), +})); + jest.mock('../../components/LoadingSpinner', () => () =>
Loading spinner
); -jest.mock('../../components/TaskForm', () => ({ onSubmit, users, projects }) => ( +jest.mock('../../components/TaskForm', () => ({ onSubmit, users, projects, assigneeLocked }) => (
users loaded: {users.length}
projects loaded: {projects.length}
+
assignee locked: {String(assigneeLocked)}
- setShowNotifications(false)} - onMarkRead={markAsRead} - onMarkAllRead={markAllAsRead} - /> +
+ setShowNotifications(false)} + onMarkRead={markAsRead} + onMarkAllRead={markAllAsRead} + /> +
)}
diff --git a/frontend/src/components/ReportTable.jsx b/frontend/src/components/ReportTable.jsx index a1da999..647bc7f 100644 --- a/frontend/src/components/ReportTable.jsx +++ b/frontend/src/components/ReportTable.jsx @@ -91,7 +91,7 @@ const ReportTable = ({ data = [], type }) => { case 'Issues': return item.open_issues ?? item.open_issues_count ?? 0; case 'PRs': - return item.open_prs || 0; + return item.total_prs || 0; case 'Commits': return item.recent_commits || 0; case 'Last Updated': diff --git a/frontend/src/context/AuthContext.jsx b/frontend/src/context/AuthContext.jsx index d4a5b89..1d5c5a3 100644 --- a/frontend/src/context/AuthContext.jsx +++ b/frontend/src/context/AuthContext.jsx @@ -1,6 +1,7 @@ import { createContext, useState, useContext, useEffect, useRef } from 'react'; import { authApi } from '../services/utils/auth'; import { githubService } from '../services/github'; +import { dashboardService } from '../services/utils/api'; import { useNavigate, useLocation } from 'react-router-dom'; import { hasRole, hasPermission } from '../utils/rbac'; @@ -10,6 +11,7 @@ export const useAuth = () => useContext(AuthContext); const VALID_ROLES = new Set(["developer", "team_lead", "admin"]); const DEFAULT_ROLE = "developer"; +const REPORT_WARMUP_ROLES = new Set(["team_lead", "admin"]); const hasValidRole = (user) => user?.role && VALID_ROLES.has(user.role); @@ -43,6 +45,7 @@ export const AuthProvider = ({ children }) => { // Keep a ref to track initialization const isInitialized = useRef(false); + const reportWarmupKeyRef = useRef(null); const navigate = useNavigate(); const location = useLocation(); @@ -257,6 +260,7 @@ export const AuthProvider = ({ children }) => { } catch (err) { console.error("Logout error:", err); } finally { + dashboardService.clearReportDataCache(); localStorage.removeItem("user"); setCurrentUser(null); setPermissions([]); @@ -297,6 +301,7 @@ export const AuthProvider = ({ children }) => { // Update GitHub connection status if that information is included if (userData && "github_connected" in userData) { + dashboardService.clearReportDataCache(); setGithubConnected(userData.github_connected); if (userData.github_connected) { // If GitHub is now connected, make sure to hide the prompt @@ -426,6 +431,48 @@ export const AuthProvider = ({ children }) => { verify(); }, [currentUserId, currentGithubConnected]); + useEffect(() => { + const canAccessReports = currentUser?.role && REPORT_WARMUP_ROLES.has(currentUser.role); + const isGithubReady = Boolean(currentUser?.github_connected || githubConnected); + + if (!currentUserId || !canAccessReports || !isGithubReady) { + return undefined; + } + + const warmupKey = `${currentUserId}:github:week`; + if (reportWarmupKeyRef.current === warmupKey) { + return undefined; + } + + let cancelled = false; + const runWarmup = () => { + if (cancelled) { + return; + } + + reportWarmupKeyRef.current = warmupKey; + dashboardService.prefetchReportData('github', 'week').catch((error) => { + console.error('Error warming GitHub report data', error); + }); + }; + + if (typeof window !== 'undefined' && typeof window.requestIdleCallback === 'function') { + const idleId = window.requestIdleCallback(runWarmup, { timeout: 5000 }); + return () => { + cancelled = true; + if (typeof window.cancelIdleCallback === 'function') { + window.cancelIdleCallback(idleId); + } + }; + } + + const timeoutId = window.setTimeout(runWarmup, 1000); + return () => { + cancelled = true; + window.clearTimeout(timeoutId); + }; + }, [currentUserId, currentUser?.role, currentUser?.github_connected, githubConnected]); + // RBAC Helpers const can = (permission) => hasPermission(permissions, permission); const is = (role) => hasRole(currentUser?.role, role); diff --git a/frontend/src/pages/Reports.jsx b/frontend/src/pages/Reports.jsx index 590b59f..2c59422 100644 --- a/frontend/src/pages/Reports.jsx +++ b/frontend/src/pages/Reports.jsx @@ -35,6 +35,7 @@ const Reports = () => { const [reportType, setReportType] = useState('tasks'); const [dateRange, setDateRange] = useState('week'); const [generatedReports, setGeneratedReports] = useState([]); + const [refreshing, setRefreshing] = useState(false); const reportCacheRef = useRef({}); const latestRequestRef = useRef(0); @@ -106,7 +107,7 @@ const Reports = () => { }); } else if (report.type === 'github') { details.slice(0, 8).forEach((repo) => { - lines.push(`- ${repo.name || 'Repo'} (${repo.owner || 'owner'}) issues: ${repo.open_issues || 0} prs: ${repo.open_prs || 0}`); + lines.push(`- ${repo.name || 'Repo'} (${repo.owner || 'owner'}) issues: ${repo.open_issues || 0} prs: ${repo.total_prs || 0}`); }); } else { details.slice(0, 8).forEach((item) => { @@ -153,13 +154,13 @@ const Reports = () => { return new Blob([pdf], { type: 'application/pdf' }); }; - const loadReportData = useCallback(async () => { + const loadReportData = useCallback(async ({ forceRefresh = false } = {}) => { const cacheKey = `${reportType}:${dateRange}`; const cachedReport = reportCacheRef.current[cacheKey]; const requestId = latestRequestRef.current + 1; latestRequestRef.current = requestId; - if (cachedReport) { + if (cachedReport && reportType !== 'github' && !forceRefresh) { setReportData(cachedReport); setLoading(false); setError(null); @@ -167,9 +168,13 @@ const Reports = () => { } try { - setLoading(true); + if (forceRefresh) { + setRefreshing(true); + } else { + setLoading(true); + } setError(null); - const data = await dashboardService.getReportData(reportType, dateRange); + const data = await dashboardService.getReportData(reportType, dateRange, { forceRefresh }); // Ignore stale responses from previous report/date selections. if (requestId !== latestRequestRef.current) { return null; @@ -187,6 +192,7 @@ const Reports = () => { } finally { if (requestId === latestRequestRef.current) { setLoading(false); + setRefreshing(false); } } }, [reportType, dateRange]); @@ -262,6 +268,10 @@ const Reports = () => { setGeneratedReports((prev) => [reportEntry, ...prev]); }; + const handleRefreshReport = async () => { + await loadReportData({ forceRefresh: true }); + }; + const handleDownloadPdf = (report) => { const lines = buildPdfLines(report); const blob = createPdfBlob(lines); @@ -348,12 +358,12 @@ const Reports = () => { if (type === 'github') { const repos = Array.isArray(details) ? details : []; const openIssues = repos.reduce((sum, repo) => sum + getGithubRepoMetrics(repo).openIssues, 0); - const openPrs = repos.reduce((sum, repo) => sum + getGithubRepoMetrics(repo).openPrs, 0); + const totalPrs = repos.reduce((sum, repo) => sum + getGithubRepoMetrics(repo).totalPrs, 0); const recentCommits = repos.reduce((sum, repo) => sum + getGithubRepoMetrics(repo).recentCommits, 0); return { repos: summary?.repos ?? repos.length, open_issues: summary?.open_issues ?? openIssues, - open_prs: summary?.open_prs ?? openPrs, + total_prs: summary?.total_prs ?? totalPrs, recent_commits: summary?.recent_commits ?? recentCommits }; } @@ -376,12 +386,21 @@ const Reports = () => { }; const hasNonZero = (values = []) => values.some((v) => Number(v) > 0); + const getGithubUpdatedLabel = () => { + if (reportType !== 'github') { + return null; + } + + const fetchedAt = reportData?.meta?.fetched_at; + return fetchedAt ? `GitHub stats updated ${formatGeneratedAt(fetchedAt)}` : 'GitHub stats ready for live refresh'; + }; + const getRepoLabel = (repo) => repo?.name || repo?.full_name || 'Repo'; const getGithubRepoMetrics = (repo) => { const openIssues = Number(repo?.open_issues ?? repo?.open_issues_count ?? 0) || 0; - const openPrs = Number(repo?.open_prs ?? 0) || 0; + const totalPrs = Number(repo?.total_prs ?? 0) || 0; const recentCommits = Number(repo?.recent_commits ?? 0) || 0; - return { openIssues, openPrs, recentCommits, total: openIssues + openPrs + recentCommits }; + return { openIssues, totalPrs, recentCommits, total: openIssues + totalPrs + recentCommits }; }; // ─── Chart theme tokens ─────────────────────────────────────────────────── @@ -532,7 +551,7 @@ const Reports = () => { } /> - } /> { labels: topRepos.map((r) => r.label), datasets: [ { label: 'Open Issues', data: topRepos.map((r) => r.openIssues), backgroundColor: chartPalette.blue }, - { label: 'Open PRs', data: topRepos.map((r) => r.openPrs), backgroundColor: chartPalette.green }, + { label: 'Total PRs', data: topRepos.map((r) => r.totalPrs), backgroundColor: chartPalette.green }, { label: 'Recent Commits', data: topRepos.map((r) => r.recentCommits), backgroundColor: chartPalette.yellow } ] }; const issuesTotal = repos.reduce((s, r) => s + getGithubRepoMetrics(r).openIssues, 0); - const prsTotal = repos.reduce((s, r) => s + getGithubRepoMetrics(r).openPrs, 0); + const prsTotal = repos.reduce((s, r) => s + getGithubRepoMetrics(r).totalPrs, 0); const commitsTotal = repos.reduce((s, r) => s + getGithubRepoMetrics(r).recentCommits, 0); const githubMixValues = [issuesTotal, prsTotal, commitsTotal]; const githubMixData = { - labels: ['Open Issues', 'Open PRs', 'Recent Commits'], + labels: ['Open Issues', 'Total PRs', 'Recent Commits'], datasets: [{ data: githubMixValues, backgroundColor: [chartPalette.blue, chartPalette.green, chartPalette.yellow], @@ -819,13 +838,31 @@ const Reports = () => {
-
- +
+
+ {reportType === 'github' && ( + {refreshing ? 'Refreshing GitHub stats...' : getGithubUpdatedLabel()} + )} +
+ +
+ {reportType === 'github' && ( + + )} + +
@@ -835,4 +872,4 @@ const Reports = () => { ); }; -export default Reports; \ No newline at end of file +export default Reports; diff --git a/frontend/src/services/utils/api.js b/frontend/src/services/utils/api.js index de6783d..158fcad 100644 --- a/frontend/src/services/utils/api.js +++ b/frontend/src/services/utils/api.js @@ -390,14 +390,14 @@ const normalizeGithubRepository = (repository = {}) => { repository.open_issues ?? repository.open_issues_count, 0 ); - const openPrs = toSafeMetricValue(repository.open_prs, 0); + const totalPrs = toSafeMetricValue(repository.total_prs, 0); const recentCommits = toSafeMetricValue(repository.recent_commits, 0); return { ...repository, open_issues: openIssues, open_issues_count: toSafeMetricValue(repository.open_issues_count, openIssues), - open_prs: openPrs, + total_prs: totalPrs, recent_commits: recentCommits, last_updated: repository.last_updated || repository.pushed_at || repository.updated_at || null, }; @@ -477,6 +477,117 @@ const buildDeveloperProgress = (users, tasks) => { }); }; +const GITHUB_REPORT_CACHE_TTL_MS = 2 * 60 * 1000; +const reportDataCache = new Map(); + +const getReportCacheScope = () => { + try { + const user = JSON.parse(localStorage.getItem('user') || 'null'); + return user?.id ? `user:${user.id}` : 'anonymous'; + } catch { + return 'anonymous'; + } +}; + +const getReportCacheKey = (reportType, dateRange) => `${getReportCacheScope()}:${reportType}:${dateRange}`; + +const addReportMeta = (reportData, meta = {}) => ({ + ...reportData, + meta: { + ...(reportData?.meta || {}), + ...meta, + }, +}); + +const getFreshCachedReport = (cacheKey) => { + const cached = reportDataCache.get(cacheKey); + if (!cached?.data || !cached.fetchedAt) { + return null; + } + + if (Date.now() - cached.fetchedAt > GITHUB_REPORT_CACHE_TTL_MS) { + return null; + } + + return addReportMeta(cached.data, { cache_hit: true }); +}; + +const getReportDataFromNetwork = async (reportType = 'tasks', dateRange = 'week') => { + if (reportType === 'github') { + const githubStatus = await fetchWithAuth('github/status').catch(() => ({ connected: false })); + const connected = Boolean(githubStatus?.connected); + const activityWindowDays = getActivityWindowDays(dateRange); + const repositories = connected + ? await githubService.getUserRepos({ + perPage: 100, + fetchAll: true, + activityWindowDays + }) + : []; + + const openIssuesTotal = repositories.reduce((sum, repo) => sum + (repo.open_issues || 0), 0); + const totalPrsTotal = repositories.reduce((sum, repo) => sum + (repo.total_prs || 0), 0); + const recentCommitsTotal = repositories.reduce((sum, repo) => sum + (repo.recent_commits || 0), 0); + + return { + summary: { + repos: repositories.length, + open_issues: openIssuesTotal, + total_prs: totalPrsTotal, + recent_commits: recentCommitsTotal + }, + details: repositories + }; + } + + const rangeStart = getDateRangeStart(dateRange); + const [tasks, usersResponse] = await Promise.all([ + taskService.getAllTasks(), + fetchWithAuth('users').catch(() => ({ users: [] })) + ]); + + const users = Array.isArray(usersResponse?.users) ? usersResponse.users : []; + const scopedTasks = tasks.filter((task) => isWithinDateRange(task.created_at, rangeStart)); + + if (reportType === 'developers') { + const developers = buildDeveloperProgress(users, tasks); + const totalTasks = developers.reduce((sum, developer) => sum + (developer.total_tasks || 0), 0); + const completedTasks = developers.reduce((sum, developer) => sum + (developer.completed_tasks || 0), 0); + const avgCompletion = totalTasks > 0 ? Math.round((completedTasks / totalTasks) * 100) : 0; + + return { + summary: { + developers: developers.length, + avg_tasks: developers.length > 0 ? Math.round(totalTasks / developers.length) : 0, + avg_completion: avgCompletion, + active_devs: developers.filter((developer) => developer.active_tasks > 0).length + }, + details: developers + }; + } + + const completed = scopedTasks.filter((task) => normalizeTaskStatus(task.status) === 'done').length; + const inProgress = scopedTasks.filter((task) => normalizeTaskStatus(task.status) === 'in_progress').length; + const overdue = scopedTasks.filter((task) => { + if (!task.deadline) { + return false; + } + + return new Date(task.deadline) < new Date() && normalizeTaskStatus(task.status) !== 'done'; + }).length; + + return { + summary: { + total: scopedTasks.length, + completed, + in_progress: inProgress, + overdue, + team_members: users.length + }, + details: scopedTasks + }; +}; + // Dashboard service for admin and member dashboards const dashboardService = { getAdminDashboardStats: async (timeRange = 'week') => { @@ -533,90 +644,89 @@ const dashboardService = { } }, - getReportData: async (reportType = 'tasks', dateRange = 'week') => { - try { - const rangeStart = getDateRangeStart(dateRange); - const [tasks, usersResponse] = await Promise.all([ - taskService.getAllTasks(), - fetchWithAuth('users').catch(() => ({ users: [] })) - ]); - - const users = Array.isArray(usersResponse?.users) ? usersResponse.users : []; - const scopedTasks = tasks.filter((task) => isWithinDateRange(task.created_at, rangeStart)); + getReportData: async (reportType = 'tasks', dateRange = 'week', options = {}) => { + const cacheKey = getReportCacheKey(reportType, dateRange); + const isGithubReport = reportType === 'github'; + const forceRefresh = Boolean(options?.forceRefresh); - if (reportType === 'developers') { - const developers = buildDeveloperProgress(users, tasks); - const totalTasks = developers.reduce((sum, developer) => sum + (developer.total_tasks || 0), 0); - const completedTasks = developers.reduce((sum, developer) => sum + (developer.completed_tasks || 0), 0); - const avgCompletion = totalTasks > 0 ? Math.round((completedTasks / totalTasks) * 100) : 0; + if (isGithubReport && !forceRefresh) { + const cachedReport = getFreshCachedReport(cacheKey); + if (cachedReport) { + return cachedReport; + } - return { - summary: { - developers: developers.length, - avg_tasks: developers.length > 0 ? Math.round(totalTasks / developers.length) : 0, - avg_completion: avgCompletion, - active_devs: developers.filter((developer) => developer.active_tasks > 0).length - }, - details: developers - }; + const pendingReport = reportDataCache.get(cacheKey)?.promise; + if (pendingReport) { + return pendingReport; } + } - if (reportType === 'github') { - const githubStatus = await fetchWithAuth('github/status').catch(() => ({ connected: false })); - const connected = Boolean(githubStatus?.connected); - const activityWindowDays = getActivityWindowDays(dateRange); - const repositories = connected - ? await githubService.getUserRepos({ - perPage: 100, - fetchAll: true, - activityWindowDays - }) - : []; - - // Calculate totals from enriched repo data - const openIssuesTotal = repositories.reduce((sum, repo) => sum + (repo.open_issues || 0), 0); - const openPrsTotal = repositories.reduce((sum, repo) => sum + (repo.open_prs || 0), 0); - const recentCommitsTotal = repositories.reduce((sum, repo) => sum + (repo.recent_commits || 0), 0); + const reportPromise = (async () => { + try { + const data = await getReportDataFromNetwork(reportType, dateRange); + const fetchedAt = Date.now(); + const report = addReportMeta(data, { + cache_hit: false, + fetched_at: new Date(fetchedAt).toISOString(), + live: true, + }); + + if (isGithubReport) { + reportDataCache.set(cacheKey, { + data: report, + fetchedAt, + }); + } + + return report; + } catch (error) { + console.error('Failed to fetch report data:', error); + if (isGithubReport) { + reportDataCache.delete(cacheKey); + } return { - summary: { - repos: repositories.length, - open_issues: openIssuesTotal, - open_prs: openPrsTotal, - recent_commits: recentCommitsTotal + summary: {}, + details: [], + meta: { + cache_hit: false, + fetched_at: new Date().toISOString(), + live: false, }, - details: repositories }; } + })(); - const completed = scopedTasks.filter((task) => normalizeTaskStatus(task.status) === 'done').length; - const inProgress = scopedTasks.filter((task) => normalizeTaskStatus(task.status) === 'in_progress').length; - const overdue = scopedTasks.filter((task) => { - if (!task.deadline) { - return false; - } + if (isGithubReport) { + reportDataCache.set(cacheKey, { + promise: reportPromise, + fetchedAt: 0, + }); + } - return new Date(task.deadline) < new Date() && normalizeTaskStatus(task.status) !== 'done'; - }).length; + return reportPromise; + }, - return { - summary: { - total: scopedTasks.length, - completed, - in_progress: inProgress, - overdue, - team_members: users.length - }, - details: scopedTasks - }; + prefetchReportData: async (reportType = 'github', dateRange = 'year') => { + try { + return await dashboardService.getReportData(reportType, dateRange); } catch (error) { - console.error('Failed to fetch report data:', error); - return { - summary: {}, - details: [] - }; + console.error('Failed to prefetch report data:', error); + return null; } - } + }, + + invalidateReportData: (reportType = 'github', dateRange = 'year') => { + reportDataCache.delete(getReportCacheKey(reportType, dateRange)); + }, + + clearReportDataCache: () => { + reportDataCache.clear(); + }, + + refreshReportData: async (reportType = 'github', dateRange = 'year') => ( + dashboardService.getReportData(reportType, dateRange, { forceRefresh: true }) + ) }; // Enhanced GitHub service with better error handling diff --git a/frontend/src/tests/components/ReportTable.test.jsx b/frontend/src/tests/components/ReportTable.test.jsx index 0000c30..019cd57 100644 --- a/frontend/src/tests/components/ReportTable.test.jsx +++ b/frontend/src/tests/components/ReportTable.test.jsx @@ -55,7 +55,7 @@ describe('ReportTable', () => { name: 'devsync', owner: 'ahmedikram', open_issues_count: 3, - open_prs: 2, + total_prs: 2, recent_commits: 6, updated_at: '2099-03-01T00:00:00.000Z', html_url: 'https://github.com/AhmedIkram05/devsync', diff --git a/frontend/src/tests/context/AuthContext.test.jsx b/frontend/src/tests/context/AuthContext.test.jsx index 1c98e2c..0d087b7 100644 --- a/frontend/src/tests/context/AuthContext.test.jsx +++ b/frontend/src/tests/context/AuthContext.test.jsx @@ -4,6 +4,7 @@ import { MemoryRouter } from 'react-router-dom'; import { AuthProvider, useAuth } from '../../context/AuthContext'; import { authApi } from '../../services/utils/auth'; +import { dashboardService } from '../../services/utils/api'; import { githubService } from '../../services/github'; jest.mock('../../services/utils/auth', () => ({ @@ -15,6 +16,13 @@ jest.mock('../../services/utils/auth', () => ({ }, })); +jest.mock('../../services/utils/api', () => ({ + dashboardService: { + prefetchReportData: jest.fn(), + clearReportDataCache: jest.fn(), + }, +})); + jest.mock('../../services/github', () => ({ githubService: { initiateOAuthFlow: jest.fn(), @@ -105,6 +113,9 @@ describe('AuthContext', () => { authApi.login.mockReset(); authApi.register.mockReset(); authApi.logout.mockReset(); + dashboardService.prefetchReportData.mockReset(); + dashboardService.prefetchReportData.mockResolvedValue(null); + dashboardService.clearReportDataCache.mockReset(); githubService.initiateOAuthFlow.mockReset(); githubService.completeOAuthFlow.mockReset(); githubService.checkConnection.mockResolvedValue({ connected: false }); @@ -344,7 +355,9 @@ describe('AuthContext', () => { renderWithProvider(); fireEvent.click(screen.getByRole('button', { name: 'Login' })); - await waitFor(() => screen.getByTestId('show-prompt').textContent === 'true'); + await waitFor(() => { + expect(screen.getByTestId('show-prompt')).toHaveTextContent('true'); + }); // Clicking prompt with connect=true should call connectGitHub const acceptBtn = screen.queryByRole('button', { name: /Skip Prompt/i }); diff --git a/frontend/src/tests/pages/Reports.test.jsx b/frontend/src/tests/pages/Reports.test.jsx index 23fb00b..e066341 100644 --- a/frontend/src/tests/pages/Reports.test.jsx +++ b/frontend/src/tests/pages/Reports.test.jsx @@ -39,7 +39,7 @@ describe('Reports page', () => { summary: { repos: 4, open_issues: 12, - open_prs: 3, + total_prs: 3, recent_commits: 18, }, details: [ @@ -48,7 +48,7 @@ describe('Reports page', () => { name: 'devsync', owner: 'ahmedikram', open_issues_count: 12, - open_prs: 3, + total_prs: 3, recent_commits: 18, }, ], @@ -101,7 +101,7 @@ describe('Reports page', () => { render(); await waitFor(() => { - expect(dashboardService.getReportData).toHaveBeenCalledWith('tasks', 'week'); + expect(dashboardService.getReportData).toHaveBeenCalledWith('tasks', 'week', { forceRefresh: false }); }); expect(await screen.findByText('Reports & Analytics')).toBeInTheDocument(); @@ -122,7 +122,7 @@ describe('Reports page', () => { }); await waitFor(() => { - expect(dashboardService.getReportData).toHaveBeenCalledWith('github', 'week'); + expect(dashboardService.getReportData).toHaveBeenCalledWith('github', 'week', { forceRefresh: false }); }); expect(await screen.findByText('Connected Repos')).toBeInTheDocument(); @@ -134,7 +134,7 @@ describe('Reports page', () => { }); await waitFor(() => { - expect(dashboardService.getReportData).toHaveBeenCalledWith('github', 'month'); + expect(dashboardService.getReportData).toHaveBeenCalledWith('github', 'month', { forceRefresh: false }); }); fireEvent.change(await screen.findByDisplayValue('GitHub Activity'), { @@ -142,7 +142,7 @@ describe('Reports page', () => { }); await waitFor(() => { - expect(dashboardService.getReportData).toHaveBeenCalledWith('developers', 'month'); + expect(dashboardService.getReportData).toHaveBeenCalledWith('developers', 'month', { forceRefresh: false }); }); expect(await screen.findByText('Team Members')).toBeInTheDocument(); @@ -198,4 +198,25 @@ describe('Reports page', () => { createElementSpy.mockRestore(); clickSpy.mockRestore(); }); + + test('github report displays total_prs summary card', async () => { + render(); + + expect(await screen.findByText('Reports & Analytics')).toBeInTheDocument(); + + // Switch to GitHub report using the report type select + const reportSelects = screen.getAllByRole('combobox'); + fireEvent.change(reportSelects[0], { target: { value: 'github' } }); + + // Verify GitHub report is displayed with correct summary cards + await waitFor(() => { + expect(dashboardService.getReportData).toHaveBeenCalledWith('github', 'week', { forceRefresh: false }); + }); + + // Verify all GitHub summary cards are rendered including Total PRs + expect(await screen.findByText('Total PRs')).toBeInTheDocument(); + expect(screen.getByText('Connected Repos')).toBeInTheDocument(); + expect(screen.getByText('Open Issues')).toBeInTheDocument(); + expect(screen.getByText('Recent Commits')).toBeInTheDocument(); + }); }); diff --git a/frontend/src/tests/services/api.test.js b/frontend/src/tests/services/api.test.js index e40b29f..1783973 100644 --- a/frontend/src/tests/services/api.test.js +++ b/frontend/src/tests/services/api.test.js @@ -29,6 +29,7 @@ describe('api utilities', () => { localStorage.clear(); global.fetch = jest.fn(); + dashboardService.clearReportDataCache(); }); afterEach(() => { @@ -341,8 +342,6 @@ describe('api utilities', () => { test('getReportData github branch when connected returns normalized repos', async () => { global.fetch - .mockResolvedValueOnce(buildResponse({ tasks: [] })) - .mockResolvedValueOnce(buildResponse({ users: [] })) .mockResolvedValueOnce(buildResponse({ connected: true })) .mockResolvedValueOnce( buildResponse({ @@ -351,7 +350,7 @@ describe('api utilities', () => { id: 1, full_name: 'org/r', open_issues_count: 5, - open_prs: 2, + total_prs: 2, recent_commits: 4, updated_at: '2099-01-01T00:00:00.000Z', }, @@ -362,11 +361,13 @@ describe('api utilities', () => { const report = await dashboardService.getReportData('github', 'month'); expect(report.summary.repos).toBe(1); expect(report.summary.open_issues).toBe(5); + expect(report.summary.total_prs).toBe(2); expect(report.details[0].open_issues).toBe(5); + expect(report.details[0].total_prs).toBe(2); expect(report.details[0].last_updated).toBe('2099-01-01T00:00:00.000Z'); - expect(global.fetch.mock.calls[3][0]).toContain('include_activity=true'); - expect(global.fetch.mock.calls[3][0]).toContain('all_pages=true'); - expect(global.fetch.mock.calls[3][0]).toContain('per_page=100'); + expect(global.fetch.mock.calls[1][0]).toContain('include_activity=true'); + expect(global.fetch.mock.calls[1][0]).toContain('all_pages=true'); + expect(global.fetch.mock.calls[1][0]).toContain('per_page=100'); }); test('buildDeveloperProgress: task with no assigned_to is skipped', async () => { @@ -387,6 +388,85 @@ describe('api utilities', () => { expect(dev.total_tasks).toBe(1); }); + test('getReportData github: aggregates total_prs across multiple repositories', async () => { + global.fetch + .mockResolvedValueOnce(buildResponse({ connected: true })) + .mockResolvedValueOnce( + buildResponse({ + repositories: [ + { + id: 1, + name: 'repo1', + owner: 'org', + full_name: 'org/repo1', + open_issues_count: 5, + total_prs: 3, + recent_commits: 10, + updated_at: '2099-01-01T00:00:00.000Z', + }, + { + id: 2, + name: 'repo2', + owner: 'org', + full_name: 'org/repo2', + open_issues_count: 2, + total_prs: 8, + recent_commits: 5, + updated_at: '2099-01-02T00:00:00.000Z', + }, + { + id: 3, + name: 'repo3', + owner: 'user', + full_name: 'user/repo3', + open_issues_count: 10, + total_prs: 0, + recent_commits: 2, + updated_at: '2099-01-03T00:00:00.000Z', + }, + ], + }) + ); + + const report = await dashboardService.getReportData('github', 'month'); + + expect(report.summary.repos).toBe(3); + expect(report.summary.open_issues).toBe(17); // 5 + 2 + 10 + expect(report.summary.total_prs).toBe(11); // 3 + 8 + 0 + expect(report.summary.recent_commits).toBe(17); // 10 + 5 + 2 + expect(report.details).toHaveLength(3); + expect(report.details[0].total_prs).toBe(3); + expect(report.details[1].total_prs).toBe(8); + expect(report.details[2].total_prs).toBe(0); + }); + + test('prefetchReportData warms the GitHub report cache', async () => { + localStorage.setItem('user', JSON.stringify({ id: 77, token: 'token-77' })); + global.fetch + .mockResolvedValueOnce(buildResponse({ connected: true })) + .mockResolvedValueOnce( + buildResponse({ + repositories: [ + { + id: 1, + name: 'repo1', + open_issues_count: 1, + total_prs: 6, + recent_commits: 2, + }, + ], + }) + ); + + const warmedReport = await dashboardService.prefetchReportData('github', 'week'); + const cachedReport = await dashboardService.getReportData('github', 'week'); + + expect(global.fetch).toHaveBeenCalledTimes(2); + expect(warmedReport.summary.total_prs).toBe(6); + expect(cachedReport.summary.total_prs).toBe(6); + expect(cachedReport.meta.cache_hit).toBe(true); + }); + test('buildDeveloperProgress: admin role is excluded, team_lead included', async () => { global.fetch.mockResolvedValueOnce( buildResponse({ users: [ @@ -575,7 +655,7 @@ describe('api utilities', () => { id: 5, open_issues_count: 4, open_issues: 4, - open_prs: 0, + total_prs: 0, recent_commits: 0, pushed_at: '2099-02-01T00:00:00.000Z', last_updated: '2099-02-01T00:00:00.000Z', From ef9dee4b3ee8cac9311d73ebafb59cf8e4655b48 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:10:35 +0100 Subject: [PATCH 28/37] feat: update BasicDashboard to enhance role-based views and remove team overview card --- .../api/controllers/dashboard_controller.py | 71 +++++++----- frontend/src/pages/BasicDashboard.jsx | 102 ++++++++---------- .../src/tests/pages/BasicDashboard.test.jsx | 15 +-- 3 files changed, 96 insertions(+), 92 deletions(-) diff --git a/backend/src/api/controllers/dashboard_controller.py b/backend/src/api/controllers/dashboard_controller.py index 1de08ec..cb3ce98 100644 --- a/backend/src/api/controllers/dashboard_controller.py +++ b/backend/src/api/controllers/dashboard_controller.py @@ -80,15 +80,21 @@ def get_user_tasks(user_id): logger.error(f"Error fetching user tasks: {str(e)}") return [] -def get_tasks_due_soon(user_id): - """Helper function to get tasks due soon for a user""" +def get_tasks_due_soon(user_id=None, project_ids=None): + """Helper function to get tasks due soon for a user or a set of projects""" try: today = datetime.now().date() week_later = today + timedelta(days=7) - return Task.query.filter_by(assigned_to=user_id)\ - .filter(Task.deadline.isnot(None))\ + query = Task.query.filter(Task.deadline.isnot(None))\ .filter(Task.deadline.between(today, week_later))\ - .filter(~Task.status.in_(COMPLETED_TASK_STATUSES)).all() + .filter(~Task.status.in_(COMPLETED_TASK_STATUSES)) + + if project_ids: + query = query.filter(Task.project_id.in_(project_ids)) + elif user_id is not None: + query = query.filter(Task.assigned_to == user_id) + + return query.all() except Exception as e: logger.error(f"Error fetching tasks due soon: {str(e)}") return [] @@ -245,41 +251,58 @@ def get_client_dashboard(): if not user: logger.error(f"User not found: {user_id}") return jsonify({'message': 'User not found'}), 404 - - # Get tasks assigned to this user - assigned_tasks = get_user_tasks(user_id) - # Get tasks due soon - tasks_due_soon = get_tasks_due_soon(user_id) + + user_projects = list(user.projects.all()) + if user_role == Role.TEAM_LEAD.value: + seen_project_ids = {project.id for project in user_projects} + for project in Project.query.filter_by(created_by=user_id).all(): + if project.id not in seen_project_ids: + user_projects.append(project) + seen_project_ids.add(project.id) + + project_ids = [project.id for project in user_projects] + is_team_lead = user_role == Role.TEAM_LEAD.value + + # Get tasks in the correct scope for the current role + if is_team_lead: + scoped_tasks = Task.query.filter(Task.project_id.in_(project_ids)).all() if project_ids else [] + tasks_due_soon = get_tasks_due_soon(project_ids=project_ids) + else: + scoped_tasks = get_user_tasks(user_id) + tasks_due_soon = get_tasks_due_soon(user_id=user_id) recent_tasks = sorted( - assigned_tasks, + scoped_tasks, key=lambda task: getattr(task, 'updated_at', None) or getattr(task, 'created_at', None) or datetime.min, reverse=True, - )[:5] + ) + if not is_team_lead: + recent_tasks = recent_tasks[:10] task_stats = { - 'total': len(assigned_tasks), - 'assigned': len(assigned_tasks), - 'todo': _count(assigned_tasks, lambda task: getattr(task, 'status', None) == 'todo'), - 'in_progress': _count(assigned_tasks, lambda task: getattr(task, 'status', None) == 'in_progress'), - 'review': _count(assigned_tasks, lambda task: getattr(task, 'status', None) == 'review'), - 'done': _count(assigned_tasks, lambda task: getattr(task, 'status', None) in {'done', 'completed'}), + 'total': len(scoped_tasks), + 'assigned': len(scoped_tasks), + 'todo': _count(scoped_tasks, lambda task: getattr(task, 'status', None) == 'todo'), + 'in_progress': _count(scoped_tasks, lambda task: getattr(task, 'status', None) == 'in_progress'), + 'review': _count(scoped_tasks, lambda task: getattr(task, 'status', None) == 'review'), + 'done': _count(scoped_tasks, lambda task: getattr(task, 'status', None) in {'done', 'completed'}), 'due_soon': len(tasks_due_soon), } github_activity = [] try: - recent_links = TaskGitHubLink.query.join(Task).outerjoin(GitHubRepository).filter( - Task.assigned_to == user_id - ).order_by(TaskGitHubLink.created_at.desc()).limit(5).all() + recent_links_query = TaskGitHubLink.query.join(Task).outerjoin(GitHubRepository) + if is_team_lead and project_ids: + recent_links_query = recent_links_query.filter(Task.project_id.in_(project_ids)) + else: + recent_links_query = recent_links_query.filter(Task.assigned_to == user_id) + + recent_links = recent_links_query.order_by(TaskGitHubLink.created_at.desc()).limit(5).all() github_activity = [_github_activity_to_item(link) for link in recent_links] except Exception as e: logger.error(f"Error fetching GitHub activity for client dashboard: {str(e)}") github_activity = [] - # Get projects user is part of - user_projects = user.projects.all() - # Format response data dashboard_data = { 'taskCounts': task_stats, diff --git a/frontend/src/pages/BasicDashboard.jsx b/frontend/src/pages/BasicDashboard.jsx index b0f2d54..f2e0ea2 100644 --- a/frontend/src/pages/BasicDashboard.jsx +++ b/frontend/src/pages/BasicDashboard.jsx @@ -62,10 +62,15 @@ const getStatusLabel = (status) => { const BasicDashboard = () => { const [dashboardData, setDashboardData] = useState(null); - const [teamUsers, setTeamUsers] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); - const { is } = useAuth(); + const { is, currentUser } = useAuth(); + const isTeamLead = is('team_lead'); + const isAdmin = is('admin'); + const dashboardTitle = isTeamLead ? 'Team Lead Workspace' : 'My Dashboard'; + const dashboardSubtitle = isTeamLead + ? 'Track your own work and keep an eye on the team without switching into the admin console.' + : 'View your tasks, projects, and GitHub activity'; const fetchDashboardData = useCallback(async () => { try { @@ -73,17 +78,6 @@ const BasicDashboard = () => { const data = await dashboardService.getBasicDashboardStats(); setDashboardData(data); - // Fetch team data if Team Lead or Admin - if (is('team_lead') || is('admin')) { - try { - const { userService } = await import('../services/utils/api'); - const users = await userService.getAllUsers(); - setTeamUsers(users); - } catch (err) { - console.error("Failed to fetch team users:", err); - } - } - setError(null); } catch (err) { console.error("Dashboard fetch error:", err); @@ -91,7 +85,7 @@ const BasicDashboard = () => { } finally { setLoading(false); } - }, [is]); + }, []); useEffect(() => { fetchDashboardData(); @@ -103,23 +97,21 @@ const BasicDashboard = () => { // Use full tasks list if provided by API, otherwise fall back to recentTasks // Limit to last 10 most recent tasks, sorted by most recent first - const tasksToShow = ((dashboardData?.tasks && dashboardData.tasks.length) ? dashboardData.tasks : (dashboardData?.recentTasks || [])) + const orderedTasksToShow = ((dashboardData?.tasks && dashboardData.tasks.length) ? dashboardData.tasks : (dashboardData?.recentTasks || [])) .sort((a, b) => { const dateA = new Date(a.updated_at || a.created_at || 0).getTime(); const dateB = new Date(b.updated_at || b.created_at || 0).getTime(); return dateB - dateA; - }) - .slice(0, 10); + }); + const tasksToShow = isTeamLead ? orderedTasksToShow : orderedTasksToShow.slice(0, 10); return (
-

My Dashboard

-

- View your tasks, projects, and GitHub activity -

+

{dashboardTitle}

+

{dashboardSubtitle}

@@ -159,6 +151,36 @@ const BasicDashboard = () => {
) : (
+ {(isTeamLead || isAdmin) && ( +
+
+
+

Management Snapshot

+

Keep the team moving without leaving this dashboard

+

+ {isAdmin + ? 'Admin users can manage users, reports, and progress from the admin console.' + : 'Team Leads can review progress, spot blockers, and create tasks from the same workspace.'} +

+
+ +
+ + Developer progress + + + Reports + + {currentUser?.role === 'admin' && ( + + Manage users + + )} +
+
+
+ )} + {/* Stats Summary */}
{
-
+

My Projects

@@ -308,7 +330,7 @@ const BasicDashboard = () => { )}
-
+

Upcoming Deadlines

@@ -337,40 +359,6 @@ const BasicDashboard = () => { )}
- {(is('team_lead') || is('admin')) && ( -
-
-

Team Overview

- - View Progress - -
- {teamUsers.length > 0 ? ( -
    - {teamUsers.slice(0, 5).map((user) => ( -
  • -
    -
    - {user.name?.charAt(0) || 'U'} -
    -
    -

    {user.name}

    -

    {user.role}

    -
    -
    -
    - Active -
    -
    • - ))} -
- ) : ( -
-

No team members found

-
- )} -
- )}
diff --git a/frontend/src/tests/pages/BasicDashboard.test.jsx b/frontend/src/tests/pages/BasicDashboard.test.jsx index 10c8e9f..0a64c9c 100644 --- a/frontend/src/tests/pages/BasicDashboard.test.jsx +++ b/frontend/src/tests/pages/BasicDashboard.test.jsx @@ -195,13 +195,7 @@ describe('BasicDashboard page', () => { }); }); - test('renders Team Overview for users with team_lead role', async () => { - const { userService } = require('../../services/utils/api'); - userService.getAllUsers.mockResolvedValue([ - { id: 1, name: 'Alice Developer', role: 'developer' }, - { id: 2, name: 'Bob Developer', role: 'developer' }, - ]); - + test('shows the TL management snapshot without the old team overview card', async () => { useAuth.mockReturnValue({ currentUser: { id: 10, name: 'Lead User', role: 'team_lead' }, is: (role) => role === 'team_lead', @@ -218,9 +212,8 @@ describe('BasicDashboard page', () => { renderDashboard(); - expect(await screen.findByText('Team Overview')).toBeInTheDocument(); - expect(await screen.findByText('Alice Developer')).toBeInTheDocument(); - expect(screen.getByText('Bob Developer')).toBeInTheDocument(); - expect(screen.getByText('View Progress')).toBeInTheDocument(); + expect(await screen.findByText('Management Snapshot')).toBeInTheDocument(); + expect(screen.queryByText('Team Overview')).not.toBeInTheDocument(); + expect(screen.getByText('Developer progress')).toBeInTheDocument(); }); }); From a9a2e0672ddd7f7c53f2fbe944c8c7cacda7ac30 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:10:59 +0100 Subject: [PATCH 29/37] feat: enhance project and task serialization with additional fields; update DeveloperProgress for role-based data fetching --- .../api/controllers/projects_controller.py | 5 ++++ .../src/api/controllers/tasks_controller.py | 1 + .../controllers/test_dashboard_controller.py | 30 ++++++++----------- frontend/src/pages/AdminDashboard.jsx | 12 +++++--- frontend/src/pages/DeveloperProgress.jsx | 14 +++++++-- .../tests/pages/DeveloperProgress.test.jsx | 21 +++++++++++++ 6 files changed, 58 insertions(+), 25 deletions(-) diff --git a/backend/src/api/controllers/projects_controller.py b/backend/src/api/controllers/projects_controller.py index 9501ac0..b510acc 100644 --- a/backend/src/api/controllers/projects_controller.py +++ b/backend/src/api/controllers/projects_controller.py @@ -59,6 +59,11 @@ def get_all_projects(): 'status': project.status, 'github_repo': project.github_repo, 'created_by': project.created_by, + 'team_members': [{ + 'id': member.id, + 'name': member.name, + 'role': member.role, + } for member in _relationship_items(project.team_members)], 'created_at': project.created_at.isoformat() if project.created_at else None, 'updated_at': project.updated_at.isoformat() if project.updated_at else None } for project in projects] diff --git a/backend/src/api/controllers/tasks_controller.py b/backend/src/api/controllers/tasks_controller.py index 802a307..f02cb65 100644 --- a/backend/src/api/controllers/tasks_controller.py +++ b/backend/src/api/controllers/tasks_controller.py @@ -69,6 +69,7 @@ def _serialize_task(task): 'status': _task_value(task, 'status'), 'priority': _task_value(task, 'priority', 'medium'), 'progress': _task_value(task, 'progress', 0), + 'project_id': _task_value(task, 'project_id'), 'assigned_to': _task_value(task, 'assigned_to'), 'created_by': _task_value(task, 'created_by'), 'deadline': _task_datetime(task, 'deadline'), diff --git a/backend/tests/unit/controllers/test_dashboard_controller.py b/backend/tests/unit/controllers/test_dashboard_controller.py index f92d048..a03f9f0 100644 --- a/backend/tests/unit/controllers/test_dashboard_controller.py +++ b/backend/tests/unit/controllers/test_dashboard_controller.py @@ -248,18 +248,18 @@ def test_get_tasks_due_soon_success(self, mock_logger, mock_datetime, mock_task) mock_now.date.return_value = today_date mock_datetime.now.return_value = mock_now - # Create the filter chain properly (deep mocking) - mock_filter_by = MagicMock() + # Create the filter chain properly (deep mocking) for 4 filters + all() mock_filter1 = MagicMock() mock_filter2 = MagicMock() mock_filter3 = MagicMock() + mock_filter4 = MagicMock() - # Link the mocks in the chain - mock_task.query.filter_by.return_value = mock_filter_by - mock_filter_by.filter.return_value = mock_filter1 + # Link the mocks in the chain: query.filter().filter().filter().filter().all() + mock_task.query.filter.return_value = mock_filter1 mock_filter1.filter.return_value = mock_filter2 mock_filter2.filter.return_value = mock_filter3 - mock_filter3.all.return_value = expected_tasks + mock_filter3.filter.return_value = mock_filter4 + mock_filter4.all.return_value = expected_tasks # Call the function result = get_tasks_due_soon(user_id=1) @@ -269,9 +269,6 @@ def test_get_tasks_due_soon_success(self, mock_logger, mock_datetime, mock_task) self.assertEqual(result, expected_tasks) self.assertEqual(result[0].id, 1) self.assertEqual(result[1].id, 2) - - # Verify the Task query was called correctly - mock_task.query.filter_by.assert_called_once_with(assigned_to=1) @patch('backend.src.api.controllers.dashboard_controller.Task') @patch('backend.src.api.controllers.dashboard_controller.datetime') @@ -287,18 +284,18 @@ def test_get_tasks_due_soon_no_tasks(self, mock_logger, mock_datetime, mock_task mock_now.date.return_value = today_date mock_datetime.now.return_value = mock_now - # Create the filter chain properly (deep mocking) that returns empty list - mock_filter_by = MagicMock() + # Create the filter chain properly (deep mocking) for 4 filters + all() mock_filter1 = MagicMock() mock_filter2 = MagicMock() mock_filter3 = MagicMock() + mock_filter4 = MagicMock() - # Link the mocks in the chain - mock_task.query.filter_by.return_value = mock_filter_by - mock_filter_by.filter.return_value = mock_filter1 + # Link the mocks in the chain: query.filter().filter().filter().filter().all() + mock_task.query.filter.return_value = mock_filter1 mock_filter1.filter.return_value = mock_filter2 mock_filter2.filter.return_value = mock_filter3 - mock_filter3.all.return_value = [] + mock_filter3.filter.return_value = mock_filter4 + mock_filter4.all.return_value = [] # Call the function result = get_tasks_due_soon(user_id=1) @@ -306,9 +303,6 @@ def test_get_tasks_due_soon_no_tasks(self, mock_logger, mock_datetime, mock_task # Assertions self.assertEqual(len(result), 0) self.assertEqual(result, []) - - # Verify the Task query was called correctly - mock_task.query.filter_by.assert_called_once_with(assigned_to=1) @patch('backend.src.api.controllers.dashboard_controller.Task') @patch('backend.src.api.controllers.dashboard_controller.datetime') diff --git a/frontend/src/pages/AdminDashboard.jsx b/frontend/src/pages/AdminDashboard.jsx index 6b6fd4e..ff7ae15 100644 --- a/frontend/src/pages/AdminDashboard.jsx +++ b/frontend/src/pages/AdminDashboard.jsx @@ -115,10 +115,14 @@ const AdminDashboard = () => { console.error('Failed to fetch team users:', userErr); } - // Fetch recent audit logs + // Fetch recent audit logs (admin-only) try { - const auditResponse = await auditLogService.getLogs({ per_page: 3, page: 1 }); - setRecentAuditLogs(auditResponse?.logs || []); + if (currentUser?.role === 'admin') { + const auditResponse = await auditLogService.getLogs({ per_page: 3, page: 1 }); + setRecentAuditLogs(auditResponse?.logs || []); + } else { + setRecentAuditLogs([]); + } } catch (auditErr) { console.error('Failed to fetch audit logs:', auditErr); } @@ -130,7 +134,7 @@ const AdminDashboard = () => { } finally { setLoading(false); } - }, [timeRange]); + }, [currentUser?.role, timeRange]); useEffect(() => { fetchDashboardData(); }, [fetchDashboardData]); diff --git a/frontend/src/pages/DeveloperProgress.jsx b/frontend/src/pages/DeveloperProgress.jsx index ca57f06..56fa509 100644 --- a/frontend/src/pages/DeveloperProgress.jsx +++ b/frontend/src/pages/DeveloperProgress.jsx @@ -2,12 +2,15 @@ import React, { useState, useEffect } from 'react'; import { Link } from 'react-router-dom'; import { dashboardService } from '../services/utils/api'; import LoadingSpinner from '../components/LoadingSpinner'; +import { useAuth } from '../context/AuthContext'; const DeveloperProgress = () => { const [developers, setDevelopers] = useState([]); const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [filter, setFilter] = useState('all'); // all, active, completed + const { currentUser, is } = useAuth(); + const isTeamLead = is('team_lead'); useEffect(() => { const fetchDevelopersProgress = async () => { @@ -15,7 +18,7 @@ const DeveloperProgress = () => { setLoading(true); // Fetch developers with their tasks and progress stats - const developersData = await dashboardService.getDeveloperProgressStats(); + const developersData = await dashboardService.getDeveloperProgressStats({ currentUser }); setDevelopers(developersData || []); } catch (err) { @@ -27,7 +30,7 @@ const DeveloperProgress = () => { }; fetchDevelopersProgress(); - }, []); + }, [currentUser]); // Filter developers based on selected filter const filteredDevelopers = developers.filter(developer => { @@ -62,7 +65,12 @@ const DeveloperProgress = () => { return (
-

Developer Progress

+
+

Developer Progress

+

+ {isTeamLead ? 'Showing developers on your shared project teams.' : 'Showing all developers.'} +

+
{/* Filter Controls */}
diff --git a/frontend/src/tests/pages/DeveloperProgress.test.jsx b/frontend/src/tests/pages/DeveloperProgress.test.jsx index 3444d0b..517c592 100644 --- a/frontend/src/tests/pages/DeveloperProgress.test.jsx +++ b/frontend/src/tests/pages/DeveloperProgress.test.jsx @@ -4,6 +4,7 @@ import { MemoryRouter } from 'react-router-dom'; import DeveloperProgress from '../../pages/DeveloperProgress'; import { dashboardService } from '../../services/utils/api'; +import { useAuth } from '../../context/AuthContext'; jest.mock('../../services/utils/api', () => ({ dashboardService: { @@ -11,6 +12,10 @@ jest.mock('../../services/utils/api', () => ({ }, })); +jest.mock('../../context/AuthContext', () => ({ + useAuth: jest.fn(), +})); + jest.mock('../../components/LoadingSpinner', () => () =>
Loading spinner
); const renderDeveloperProgress = () => { @@ -25,6 +30,11 @@ describe('DeveloperProgress page', () => { beforeEach(() => { jest.spyOn(console, 'error').mockImplementation(() => {}); + useAuth.mockReturnValue({ + currentUser: { id: 1, role: 'admin' }, + is: jest.fn().mockReturnValue(false), + }); + dashboardService.getDeveloperProgressStats.mockReset(); dashboardService.getDeveloperProgressStats.mockResolvedValue([ { @@ -125,4 +135,15 @@ describe('DeveloperProgress page', () => { expect(await screen.findByText(/Failed to load developer progress data. Please try again./i)).toBeInTheDocument(); }); + + test('shows team lead scope message when the user is a team lead', async () => { + useAuth.mockReturnValue({ + currentUser: { id: 2, role: 'team_lead' }, + is: (role) => role === 'team_lead', + }); + + renderDeveloperProgress(); + + expect(await screen.findByText(/Showing developers on your shared project teams/i)).toBeInTheDocument(); + }); }); From 7e1e5d44de6cf641a7a35fc8dbbc6cbabcabeff8 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:11:11 +0100 Subject: [PATCH 30/37] feat: enhance dashboard functionality for team leads; update routing and task visibility --- .../test_auth_socket_dashboard_integration.py | 71 +++++++++ frontend/src/App.jsx | 3 +- frontend/src/components/Navbar.jsx | 12 +- frontend/src/services/utils/api.js | 146 ++++++++++++++++-- frontend/src/tests/components/Navbar.test.jsx | 5 +- frontend/src/tests/services/api.test.js | 7 +- 6 files changed, 213 insertions(+), 31 deletions(-) diff --git a/backend/tests/integration/test_auth_socket_dashboard_integration.py b/backend/tests/integration/test_auth_socket_dashboard_integration.py index 9adbf02..99a1e1b 100644 --- a/backend/tests/integration/test_auth_socket_dashboard_integration.py +++ b/backend/tests/integration/test_auth_socket_dashboard_integration.py @@ -312,6 +312,77 @@ class StubUser: assert payload['projects'][0]['name'] == 'Project A' +def test_dashboard_client_route_scopes_team_leads_to_their_projects(client, app, monkeypatch): + shared_project = SimpleNamespace(id=5, name='Shared Project', status='active', created_by=21) + created_project = SimpleNamespace(id=8, name='Created Project', status='active', created_by=21) + user = SimpleNamespace( + id=21, + name='Team Lead', + role='team_lead', + projects=SimpleNamespace(all=lambda: [shared_project]), + ) + + scoped_tasks = [ + SimpleNamespace(id=1, title='One', status='todo', project_id=5, updated_at=datetime(2099, 1, 3), created_at=datetime(2099, 1, 2), deadline=datetime(2099, 1, 6), assigned_to=None), + SimpleNamespace(id=2, title='Two', status='done', project_id=8, updated_at=datetime(2099, 1, 4), created_at=datetime(2099, 1, 1), deadline=datetime(2099, 1, 7), assigned_to=None), + SimpleNamespace(id=3, title='Three', status='in_progress', project_id=8, updated_at=datetime(2099, 1, 5), created_at=datetime(2099, 1, 5), deadline=datetime(2099, 1, 8), assigned_to=None), + ] + due_tasks = [SimpleNamespace(id=2, title='Two', deadline=datetime(2099, 1, 7), status='done', project_id=8)] + github_link = SimpleNamespace(id=1, task=SimpleNamespace(title='Two'), repository=SimpleNamespace(repo_name='Repo', repo_url='https://github.com/org/repo'), pull_request_number=None, issue_number=7, created_at=datetime(2099, 1, 6)) + + class StubUser: + query = MagicMock() + + class StubProject: + query = MagicMock() + + class StubTask: + query = MagicMock() + project_id = MagicMock() + assigned_to = MagicMock() + + class StubLink: + query = MagicMock() + + class StubRepo: + pass + + StubUser.query.get.return_value = user + StubProject.query.filter_by.return_value.all.return_value = [created_project] + + task_query = MagicMock() + task_query.filter.return_value = task_query + task_query.all.return_value = scoped_tasks + StubTask.query = task_query + StubTask.project_id.in_.return_value = MagicMock() + + link_query = MagicMock() + link_query.join.return_value = link_query + link_query.outerjoin.return_value = link_query + link_query.filter.return_value = link_query + link_query.order_by.return_value = link_query + link_query.limit.return_value = link_query + link_query.all.return_value = [github_link] + StubLink.query = link_query + + monkeypatch.setattr(dashboard_controller, 'User', StubUser) + monkeypatch.setattr(dashboard_controller, 'Project', StubProject) + monkeypatch.setattr(dashboard_controller, 'Task', StubTask) + monkeypatch.setattr(dashboard_controller, 'TaskGitHubLink', StubLink) + monkeypatch.setattr(dashboard_controller, 'GitHubRepository', StubRepo) + monkeypatch.setattr(dashboard_controller, 'get_tasks_due_soon', MagicMock(return_value=due_tasks)) + + response = client.get('/api/v1/dashboard/client', headers=auth_headers(app, role='team_lead', user_id=21)) + + assert response.status_code == 200 + payload = response.get_json() + assert payload['tasks']['total'] == 3 + assert payload['tasks']['done'] == 1 + assert [project['name'] for project in payload['projects']] == ['Shared Project', 'Created Project'] + assert len(payload['recentTasks']) == 3 + assert payload['tasks_due_soon'][0]['id'] == 2 + + def test_dashboard_admin_route_returns_user_and_task_totals(client, app, monkeypatch): admin_user = SimpleNamespace(id=1, name='Admin User', role='admin') users = [ diff --git a/frontend/src/App.jsx b/frontend/src/App.jsx index f7bf61c..1566284 100644 --- a/frontend/src/App.jsx +++ b/frontend/src/App.jsx @@ -36,7 +36,8 @@ const AUTHENTICATED_ROLES = [ROLES.DEVELOPER, ROLES.TEAM_LEAD, ROLES.ADMIN]; const TASK_CREATOR_ROLES = [ROLES.DEVELOPER, ROLES.TEAM_LEAD, ROLES.ADMIN]; const TEAM_LEAD_OR_ADMIN = [ROLES.TEAM_LEAD, ROLES.ADMIN]; -const getDashboardPath = (role) => (role === ROLES.ADMIN || role === ROLES.TEAM_LEAD ? '/admin' : '/BasicDashboard'); +// Team Leads should see the basic developer dashboard by default. +const getDashboardPath = (role) => (role === ROLES.ADMIN ? '/admin' : '/BasicDashboard'); const ProtectedRoute = ({ children, allowedRoles = [], requiredPermission = null }) => { const { currentUser, loading, can } = useAuth(); diff --git a/frontend/src/components/Navbar.jsx b/frontend/src/components/Navbar.jsx index 3ffbdea..6265fe8 100644 --- a/frontend/src/components/Navbar.jsx +++ b/frontend/src/components/Navbar.jsx @@ -47,7 +47,7 @@ const Navbar = () => { const isAdmin = is('admin'); const isTeamLead = is('team_lead'); - const canCreateTasks = isAdmin || isTeamLead; + const dashboardPath = isAdmin ? '/admin' : '/BasicDashboard'; return (
@@ -63,7 +63,6 @@ const Navbar = () => { Dashboard Tasks Projects - Create Task Developer Progress Reports
@@ -81,10 +80,9 @@ const Navbar = () => { ) : isTeamLead ? ( <> - Dashboard + Dashboard Tasks Projects - Create Task Developer Progress Reports GitHub @@ -177,7 +175,6 @@ const Navbar = () => { {isAdmin ? ( <> Dashboard - Create Task Developer Progress Reports Users @@ -187,11 +184,8 @@ const Navbar = () => { ) : isTeamLead ? ( <> - Dashboard + Dashboard Tasks - {canCreateTasks && ( - Create Task - )} Reports GitHub diff --git a/frontend/src/services/utils/api.js b/frontend/src/services/utils/api.js index 158fcad..7405c98 100644 --- a/frontend/src/services/utils/api.js +++ b/frontend/src/services/utils/api.js @@ -200,8 +200,23 @@ const fetchWithAuth = async (endpoint, options = {}) => { // Task related API calls const taskService = { getAllTasks: async () => { + getAllTasks: async (params) => { try { - const response = await fetchWithAuth('tasks'); + let endpoint = 'tasks'; + if (params) { + if (typeof params === 'string') { + endpoint = `tasks${params.startsWith('?') ? params : `?${params}`}`; + } else if (typeof params === 'object') { + const ps = new URLSearchParams(); + Object.keys(params).forEach((k) => { + if (params[k] !== undefined && params[k] !== null) ps.set(k, String(params[k])); + }); + const qs = ps.toString(); + if (qs) endpoint = `tasks?${qs}`; + } + } + + const response = await fetchWithAuth(endpoint); const tasks = response?.tasks ?? response; return Array.isArray(tasks) ? tasks : []; } catch (error) { @@ -433,26 +448,34 @@ const normalizeAdminDashboardTasks = (tasks = {}) => { }; const buildDeveloperProgress = (users, tasks) => { - const progressTrackingRoles = new Set(['developer', 'team_lead']); + const progressTrackingRoles = new Set(['admin', 'developer', 'team_lead']); const tasksByAssignee = new Map(); - tasks.forEach((task) => { - if (task?.assigned_to === null || task?.assigned_to === undefined) { - return; - } - - const assigneeKey = String(task.assigned_to); - if (!tasksByAssignee.has(assigneeKey)) { - tasksByAssignee.set(assigneeKey, []); - } + const getAssigneeId = (task) => { + if (!task) return null; + if (task.assigned_to !== undefined && task.assigned_to !== null) return task.assigned_to; + if (task.assignedTo !== undefined && task.assignedTo !== null) return task.assignedTo; + if (task.assignee !== undefined && task.assignee !== null) return task.assignee; + return null; + }; + tasks.forEach((task) => { + const assignee = getAssigneeId(task); + if (assignee === null || assignee === undefined) return; + const assigneeKey = String(assignee); + if (!tasksByAssignee.has(assigneeKey)) tasksByAssignee.set(assigneeKey, []); tasksByAssignee.get(assigneeKey).push(task); }); + console.log('[buildDeveloperProgress] Tasks by assignee:', tasksByAssignee); + console.log('[buildDeveloperProgress] Total tasks with assignees:', Array.from(tasksByAssignee.values()).flat().length); + return users .filter((user) => progressTrackingRoles.has(user.role)) .map((user) => { const userTasks = tasksByAssignee.get(String(user.id)) || []; + console.log(`[buildDeveloperProgress] User ${user.id} (${user.name}): ${userTasks.length} tasks`); + let completedCount = 0; userTasks.forEach((task) => { @@ -627,17 +650,110 @@ const dashboardService = { } }, - getDeveloperProgressStats: async () => { + getDeveloperProgressStats: async (options = {}) => { try { - const [usersResponse, tasks] = await Promise.all([ + const [usersResponse, tasks, projects] = await Promise.all([ fetchWithAuth('users'), - taskService.getAllTasks() + taskService.getAllTasks(), + projectService.getAllProjects() ]); const users = Array.isArray(usersResponse?.users) ? usersResponse.users : []; const allTasks = Array.isArray(tasks) ? tasks : []; + const allProjects = Array.isArray(projects) ? projects : []; + const currentUser = options.currentUser || null; + + console.log('[getDeveloperProgressStats] Users:', users.length); + console.log('[getDeveloperProgressStats] Tasks:', allTasks.length, allTasks.slice(0, 3)); + console.log('[getDeveloperProgressStats] Projects:', allProjects.length); + console.log('[getDeveloperProgressStats] currentUser:', currentUser); + + const scopedProjectIds = new Set(); + if (currentUser?.role === 'team_lead' && currentUser?.id) { + allProjects.forEach((project) => { + const teamMembers = Array.isArray(project?.team_members) ? project.team_members : []; + const teamMemberIds = teamMembers.map((member) => Number(member?.id)).filter(Number.isFinite); + const isProjectCreator = Number(project?.created_by) === Number(currentUser.id); + const isOnProjectTeam = teamMemberIds.includes(Number(currentUser.id)); + + if (isProjectCreator || isOnProjectTeam) { + scopedProjectIds.add(Number(project.id)); + } + }); + } + + const isTeamLead = currentUser?.role === 'team_lead'; + // Debug: show sample task/project fields to help diagnose scoping issues + try { + console.log('[getDeveloperProgressStats] Sample tasks:', allTasks.slice(0, 5).map(t => ({ id: t.id, project_id: t.project_id, assigned_to: t.assigned_to }))); + console.log('[getDeveloperProgressStats] Sample projects:', allProjects.map(p => ({ id: p.id, created_by: p.created_by, team_members: (p.team_members || []).map(m => ({ id: m.id, role: m.role })) })).slice(0,5)); + } catch (e) { + console.warn('[getDeveloperProgressStats] Debug logging failed', e); + } + + if (isTeamLead && scopedProjectIds.size === 0) { + console.log('[getDeveloperProgressStats] TL with no scoped projects, returning []'); + return []; + } + + const scopedUsers = isTeamLead && scopedProjectIds.size > 0 + ? users.filter((user) => { + if (!['admin', 'developer', 'team_lead'].includes(user.role)) { + return false; + } + + return allProjects.some((project) => { + if (!scopedProjectIds.has(Number(project.id))) { + return false; + } + + return Array.isArray(project?.team_members) + ? project.team_members.some((member) => Number(member?.id) === Number(user.id) && ['admin', 'developer', 'team_lead'].includes(member?.role)) + : false; + }); + }) + : users; + + const getTaskProjectId = (task) => { + if (!task) return null; + if (task.project_id !== undefined && task.project_id !== null) return task.project_id; + if (task.projectId !== undefined && task.projectId !== null) return task.projectId; + if (task.project && task.project.id !== undefined && task.project.id !== null) return task.project.id; + return null; + }; + + const scopedUserIds = new Set((isTeamLead ? scopedUsers.map(u => Number(u.id)).filter(Number.isFinite) : [])); + + const getTaskAssigneeId = (task) => { + if (!task) return null; + if (task.assigned_to !== undefined && task.assigned_to !== null) return task.assigned_to; + if (task.assignedTo !== undefined && task.assignedTo !== null) return task.assignedTo; + if (task.assignee !== undefined && task.assignee !== null) return task.assignee; + return null; + }; + + const scopedTasks = isTeamLead && scopedProjectIds.size > 0 + ? allTasks.filter((task) => { + const pid = getTaskProjectId(task); + const aid = getTaskAssigneeId(task); + const inProject = pid !== null && scopedProjectIds.has(Number(pid)); + const assignedToTeam = aid !== null && scopedUserIds.has(Number(aid)); + return inProject || assignedToTeam; + }) + : allTasks; + if (isTeamLead && scopedProjectIds.size > 0 && scopedTasks.length === 0) { + console.warn('[getDeveloperProgressStats] No scoped tasks found for TL - inspecting all tasks vs scopedProjectIds', Array.from(scopedProjectIds)); + allTasks.forEach((task) => { + console.log('[getDeveloperProgressStats] task:', task.id, 'project_id:', task.project_id, 'Number(project_id):', Number(task.project_id)); + }); + } + + console.log('[getDeveloperProgressStats] Scoped users:', scopedUsers.length); + console.log('[getDeveloperProgressStats] Scoped tasks:', scopedTasks.length, scopedTasks.slice(0, 3)); - return buildDeveloperProgress(users, allTasks); + const result = buildDeveloperProgress(scopedUsers, scopedTasks); + console.log('[getDeveloperProgressStats] Result:', result); + return result; } catch (error) { console.error('Failed to fetch developer progress stats:', error); return []; diff --git a/frontend/src/tests/components/Navbar.test.jsx b/frontend/src/tests/components/Navbar.test.jsx index ae65398..57e867c 100644 --- a/frontend/src/tests/components/Navbar.test.jsx +++ b/frontend/src/tests/components/Navbar.test.jsx @@ -91,7 +91,6 @@ describe('Navbar component', () => { renderNavbar(); expect(screen.getAllByText('Dashboard').length).toBeGreaterThan(0); - expect(screen.getAllByText('Create Task').length).toBeGreaterThan(0); expect(screen.getAllByText('Developer Progress').length).toBeGreaterThan(0); expect(screen.getAllByText('Reports').length).toBeGreaterThan(0); expect(screen.getAllByText('GitHub').length).toBeGreaterThan(0); @@ -145,7 +144,7 @@ describe('Navbar component', () => { expect(screen.queryByText('Create Task')).not.toBeInTheDocument(); }); - test('shows task creation link for team leads', () => { + test('does not show task creation link for team leads', () => { useAuth.mockReturnValue({ currentUser: { id: 3, @@ -160,7 +159,7 @@ describe('Navbar component', () => { renderNavbar(); - expect(screen.getAllByText('Create Task').length).toBeGreaterThan(0); + expect(screen.queryByText('Create Task')).not.toBeInTheDocument(); }); test('shows logging out state while logout is in progress', async () => { diff --git a/frontend/src/tests/services/api.test.js b/frontend/src/tests/services/api.test.js index 1783973..32baffb 100644 --- a/frontend/src/tests/services/api.test.js +++ b/frontend/src/tests/services/api.test.js @@ -234,8 +234,9 @@ describe('api utilities', () => { const progress = await dashboardService.getDeveloperProgressStats(); - expect(progress).toHaveLength(2); + expect(progress).toHaveLength(3); expect(progress.find((item) => item.id === 1).completed_tasks).toBe(1); + expect(progress.find((item) => item.id === 2)).toBeDefined(); expect(progress.find((item) => item.id === 3).completed_tasks).toBe(1); }); @@ -467,7 +468,7 @@ describe('api utilities', () => { expect(cachedReport.meta.cache_hit).toBe(true); }); - test('buildDeveloperProgress: admin role is excluded, team_lead included', async () => { + test('buildDeveloperProgress: admin role is included along with team_lead', async () => { global.fetch.mockResolvedValueOnce( buildResponse({ users: [ { id: 1, name: 'Admin', role: 'admin' }, @@ -477,7 +478,7 @@ describe('api utilities', () => { global.fetch.mockResolvedValueOnce(buildResponse({ tasks: [] })); const progress = await dashboardService.getDeveloperProgressStats(); - expect(progress.find((p) => p.id === 1)).toBeUndefined(); + expect(progress.find((p) => p.id === 1)).toBeDefined(); expect(progress.find((p) => p.id === 2)).toBeDefined(); }); From d934d3cd7e0693e75f83b75b6015b5428d3271f9 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:11:17 +0100 Subject: [PATCH 31/37] refactor: clean up task status styling in TaskList component --- docs/backend/rbac.md | 1 - frontend/src/pages/TaskList.jsx | 10 +++++----- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/backend/rbac.md b/docs/backend/rbac.md index 7fc1222..5f8006f 100644 --- a/docs/backend/rbac.md +++ b/docs/backend/rbac.md @@ -39,7 +39,6 @@ All Developer permissions, plus: - Create new tasks / assign tasks (`can_assign_tasks`) - Update any task (`can_update_any_task`) - View all users (`can_view_all_users`) -- View system statistics (`can_view_system_stats`) - Manage projects (`can_manage_projects`) - Generate and view reports - View developer progress diff --git a/frontend/src/pages/TaskList.jsx b/frontend/src/pages/TaskList.jsx index e22a4ba..0693e38 100644 --- a/frontend/src/pages/TaskList.jsx +++ b/frontend/src/pages/TaskList.jsx @@ -87,11 +87,11 @@ const TaskList = () => { // Get status badge color and format status text const getStatusInfo = (status) => { const statusMap = { - 'todo': { class: 'bg-slate-800/70 text-slate-300', text: 'To Do' }, - 'backlog': { class: 'bg-slate-800/70 text-slate-300', text: 'Backlog' }, - 'in_progress': { class: 'bg-amber-500/15 text-amber-200', text: 'In Progress' }, - 'review': { class: 'bg-sky-500/15 text-sky-200', text: 'Review' }, - 'completed': { class: 'bg-emerald-500/15 text-emerald-200', text: 'Completed' } + 'todo': { class: 'text-slate-300', text: 'To Do' }, + 'backlog': { class: 'text-slate-300', text: 'Backlog' }, + 'in_progress': { class: 'text-amber-200', text: 'In Progress' }, + 'review': { class: 'text-sky-200', text: 'Review' }, + 'completed': { class: 'text-emerald-200', text: 'Completed' } }; return statusMap[status] || { class: 'bg-gray-100 text-gray-800', text: status }; From 49a2d89c65df144a233f7326195670fd2e9c666a Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:17:38 +0100 Subject: [PATCH 32/37] feat: enhance task fetching with support for query parameters and deep-linking --- frontend/src/pages/TaskList.jsx | 31 ++++++++++++++++++++++++------ frontend/src/services/utils/api.js | 18 ++++++++++------- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/frontend/src/pages/TaskList.jsx b/frontend/src/pages/TaskList.jsx index 0693e38..cbdd899 100644 --- a/frontend/src/pages/TaskList.jsx +++ b/frontend/src/pages/TaskList.jsx @@ -20,15 +20,27 @@ const TaskList = () => { const canCreateTasks = Boolean(currentUser); - // Fetch tasks when component mounts + const urlParams = new URLSearchParams(window.location.search); + const requestedAssignee = urlParams.get('assigned_to') || urlParams.get('assignee'); + + // Fetch tasks once; honor deep-link assignee query if provided useEffect(() => { + const initialUrlParams = new URLSearchParams(window.location.search); + const initialAssignee = initialUrlParams.get('assigned_to') || initialUrlParams.get('assignee'); + + if (initialAssignee) { + setFilters((prev) => ({ ...prev, scope: 'my' })); + fetchTasks({ assigned_to: initialAssignee }); + return; + } + fetchTasks(); }, []); - const fetchTasks = async () => { + const fetchTasks = async (params = null) => { try { setLoading(true); - const tasksData = await taskService.getAllTasks(); + const tasksData = await taskService.getAllTasks(params); setTasks(Array.isArray(tasksData) ? tasksData : []); setError(null); } catch (err) { @@ -126,8 +138,15 @@ const TaskList = () => { } // Filter by scope (My Tasks vs All Tasks) - if (filters.scope === 'my' && task.assigned_to !== currentUser?.id) { - return false; + if (filters.scope === 'my') { + const targetAssigneeId = requestedAssignee ?? currentUser?.id; + if (targetAssigneeId === undefined || targetAssigneeId === null) { + return false; + } + + if (Number(task.assigned_to) !== Number(targetAssigneeId)) { + return false; + } } return true; @@ -277,7 +296,7 @@ const TaskList = () => {

No tasks found matching your filters

{filters.status !== 'all' || filters.priority !== 'all' || filters.search ? (
+
+ +
); })} diff --git a/frontend/src/context/NotificationContext.jsx b/frontend/src/context/NotificationContext.jsx index c815c2a..fd35f99 100644 --- a/frontend/src/context/NotificationContext.jsx +++ b/frontend/src/context/NotificationContext.jsx @@ -358,6 +358,23 @@ export const NotificationProvider = ({ children }) => { refreshNotifications(true); } }; + + const deleteNotification = async (notificationId) => { + if (serverDown) { + console.log('Server appears to be down, cannot delete notification'); + return; + } + + const previousNotifications = notifications; + + try { + setNotifications((prev) => prev.filter((notification) => notification.id !== notificationId)); + await notificationService.deleteNotification(notificationId); + } catch (error) { + console.error('Failed to delete notification:', error); + setNotifications(previousNotifications); + } + }; // Mark all notifications as read const markAllAsRead = async () => { @@ -391,6 +408,7 @@ export const NotificationProvider = ({ children }) => { notifications, unreadCount, markAsRead, + deleteNotification, markAllAsRead, refreshNotifications, isConnected, diff --git a/frontend/src/services/utils/api.js b/frontend/src/services/utils/api.js index e140d7b..e2ebbdb 100644 --- a/frontend/src/services/utils/api.js +++ b/frontend/src/services/utils/api.js @@ -518,6 +518,29 @@ const getReportCacheScope = () => { const getReportCacheKey = (reportType, dateRange) => `${getReportCacheScope()}:${reportType}:${dateRange}`; +const normalizeTaskReportDetails = (tasks = [], users = []) => { + const usersById = new Map( + (Array.isArray(users) ? users : []).map((user) => [Number(user?.id), user?.name]) + ); + + return (Array.isArray(tasks) ? tasks : []).map((task) => { + const assigneeId = task?.assigned_to ?? task?.assignedTo ?? null; + const explicitAssigneeName = typeof task?.assignee === 'object' + ? task?.assignee?.name + : null; + + return { + ...task, + assignee_name: + task?.assignee_name + || task?.assigneeName + || explicitAssigneeName + || usersById.get(Number(assigneeId)) + || null, + }; + }); +}; + const addReportMeta = (reportData, meta = {}) => ({ ...reportData, meta: { @@ -611,7 +634,7 @@ const getReportDataFromNetwork = async (reportType = 'tasks', dateRange = 'week' overdue, team_members: users.length }, - details: scopedTasks + details: normalizeTaskReportDetails(scopedTasks, users) }; }; @@ -1104,6 +1127,12 @@ const notificationService = { return await fetchWithAuth('notifications/read-all', { method: 'PUT' }); + }, + + deleteNotification: async (notificationId) => { + return await fetchWithAuth(`notifications/${notificationId}`, { + method: 'DELETE' + }); } }; @@ -1279,5 +1308,6 @@ export { reportService, adminUserService, settingsService, - auditLogService + auditLogService, + normalizeTaskReportDetails }; From 64151537c27bcd5e2a61029187e46cc49267f380 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:37:41 +0100 Subject: [PATCH 35/37] feat: enhance audit log functionality with actor name resolution and add tests for AdminAuditLogs --- .../src/api/controllers/audit_controller.py | 60 +++++---- .../unit/controllers/test_audit_controller.py | 123 ++++++++++++++++++ frontend/src/components/ReportTable.jsx | 6 +- frontend/src/pages/AdminAuditLogs.jsx | 4 +- frontend/src/pages/Reports.jsx | 12 +- .../src/tests/components/ReportTable.test.jsx | 1 + .../src/tests/pages/AdminAuditLogs.test.jsx | 68 ++++++++++ .../src/tests/pages/AdminProjects.test.jsx | 9 ++ .../src/tests/services/utils/api.test.jsx | 24 ++++ 9 files changed, 274 insertions(+), 33 deletions(-) create mode 100644 backend/tests/unit/controllers/test_audit_controller.py create mode 100644 frontend/src/tests/pages/AdminAuditLogs.test.jsx create mode 100644 frontend/src/tests/services/utils/api.test.jsx diff --git a/backend/src/api/controllers/audit_controller.py b/backend/src/api/controllers/audit_controller.py index 8c996ea..3a98064 100644 --- a/backend/src/api/controllers/audit_controller.py +++ b/backend/src/api/controllers/audit_controller.py @@ -1,7 +1,35 @@ """Audit Log Controller""" from flask import request, jsonify -from ...db.models import AuditLog +from ...db.models import AuditLog, User + + +def _build_actor_name_map(logs): + actor_ids = {log.actor_user_id for log in logs if getattr(log, 'actor_user_id', None) is not None} + if not actor_ids: + return {} + + users = User.query.filter(User.id.in_(actor_ids)).all() + return {user.id: user.name for user in users} + + +def _serialize_audit_log(log, actor_name_map=None): + actor_name_map = actor_name_map or {} + actor_user_id = log.actor_user_id + + return { + 'id': log.id, + 'actor_user_id': actor_user_id, + 'actor_name': actor_name_map.get(actor_user_id), + 'actor_role': log.actor_role, + 'action': log.action, + 'resource_type': log.resource_type, + 'resource_id': log.resource_id, + 'ip': log.ip, + 'user_agent': log.user_agent, + 'metadata': log.metadata_info, + 'created_at': log.created_at.isoformat() if log.created_at else None + } def get_audit_logs(): """Get paginated and filtered audit logs""" @@ -28,18 +56,8 @@ def get_audit_logs(): page=page, per_page=per_page, error_out=False ) - logs_data = [{ - 'id': log.id, - 'actor_user_id': log.actor_user_id, - 'actor_role': log.actor_role, - 'action': log.action, - 'resource_type': log.resource_type, - 'resource_id': log.resource_id, - 'ip': log.ip, - 'user_agent': log.user_agent, - 'metadata': log.metadata_info, - 'created_at': log.created_at.isoformat() if log.created_at else None - } for log in pagination.items] + actor_name_map = _build_actor_name_map(pagination.items) + logs_data = [_serialize_audit_log(log, actor_name_map) for log in pagination.items] return jsonify({ 'logs': logs_data, @@ -51,18 +69,14 @@ def get_audit_logs(): def get_audit_log_by_id(log_id): """Get a specific audit log""" log = AuditLog.query.get_or_404(log_id) + actor_name = None + + if log.actor_user_id is not None: + actor = User.query.get(log.actor_user_id) + actor_name = actor.name if actor else None return jsonify({ 'log': { - 'id': log.id, - 'actor_user_id': log.actor_user_id, - 'actor_role': log.actor_role, - 'action': log.action, - 'resource_type': log.resource_type, - 'resource_id': log.resource_id, - 'ip': log.ip, - 'user_agent': log.user_agent, - 'metadata': log.metadata_info, - 'created_at': log.created_at.isoformat() if log.created_at else None + **_serialize_audit_log(log, {log.actor_user_id: actor_name} if actor_name else {}), } }) diff --git a/backend/tests/unit/controllers/test_audit_controller.py b/backend/tests/unit/controllers/test_audit_controller.py new file mode 100644 index 0000000..b8e3a80 --- /dev/null +++ b/backend/tests/unit/controllers/test_audit_controller.py @@ -0,0 +1,123 @@ +"""Tests for audit controller name resolution.""" +from types import SimpleNamespace + +import pytest + +from src.api.controllers import audit_controller + + +class FakePagination: + def __init__(self, items): + self.items = items + self.total = len(items) + self.pages = 1 + + +class FakeQuery: + def __init__(self, items): + self.items = items + + def filter(self, *args, **kwargs): + return self + + def filter_by(self, *args, **kwargs): + return self + + def order_by(self, *args, **kwargs): + return self + + def paginate(self, *args, **kwargs): + return FakePagination(self.items) + + def get_or_404(self, log_id): + for item in self.items: + if item.id == log_id: + return item + raise LookupError(log_id) + + +class FakeUserColumn: + def in_(self, values): + return values + + +class FakeAuditColumn: + def desc(self): + return self + + def ilike(self, value): + return value + + +class FakeUserQuery: + def __init__(self, users): + self.users = users + + def filter(self, *args, **kwargs): + return self + + def all(self): + return self.users + + def get(self, user_id): + for user in self.users: + if user.id == user_id: + return user + return None + + +@pytest.fixture +def audit_logs(): + return [ + SimpleNamespace( + id=1, + actor_user_id=7, + actor_role='admin', + action='user_created', + resource_type='user', + resource_id='42', + ip='127.0.0.1', + user_agent='pytest', + metadata_info=None, + created_at=None, + ) + ] + + +@pytest.fixture +def users(): + return [SimpleNamespace(id=7, name='Admin User')] + + +@pytest.fixture +def fake_models(monkeypatch, audit_logs, users): + monkeypatch.setattr( + audit_controller, + 'AuditLog', + SimpleNamespace(query=FakeQuery(audit_logs), created_at=FakeAuditColumn(), action=FakeAuditColumn()), + ) + monkeypatch.setattr( + audit_controller, + 'User', + SimpleNamespace(query=FakeUserQuery(users), id=FakeUserColumn()), + ) + + +def test_get_audit_logs_includes_actor_name(app, client, monkeypatch, fake_models): + with app.test_request_context('/api/v1/admin/audit-logs'): + response = audit_controller.get_audit_logs() + + assert response.status_code == 200 + data = response.get_json() + assert data['logs'][0]['actor_name'] == 'Admin User' + assert data['logs'][0]['actor_user_id'] == 7 + + +def test_get_audit_log_by_id_includes_actor_name(app, client, monkeypatch, fake_models): + with app.test_request_context('/api/v1/admin/audit-logs/1'): + response = audit_controller.get_audit_log_by_id(1) + + assert response.status_code == 200 + data = response.get_json() + assert data['log']['actor_name'] == 'Admin User' + assert data['log']['actor_user_id'] == 7 diff --git a/frontend/src/components/ReportTable.jsx b/frontend/src/components/ReportTable.jsx index 647bc7f..df9951c 100644 --- a/frontend/src/components/ReportTable.jsx +++ b/frontend/src/components/ReportTable.jsx @@ -173,15 +173,15 @@ const ReportTable = ({ data = [], type }) => { if (!data || data.length === 0) { return ( -
+
No data available for this report
); } return ( -
-
+
+
diff --git a/frontend/src/pages/AdminAuditLogs.jsx b/frontend/src/pages/AdminAuditLogs.jsx index f91810c..1e9fb4d 100644 --- a/frontend/src/pages/AdminAuditLogs.jsx +++ b/frontend/src/pages/AdminAuditLogs.jsx @@ -88,7 +88,7 @@ const AdminAuditLogs = () => {
- {log.actor_user_id ? `User ${log.actor_user_id}` : 'System'} + {log.actor_name || (log.actor_user_id ? `User ${log.actor_user_id}` : 'System')} {log.actor_role && ({log.actor_role})} @@ -129,7 +129,7 @@ const AdminAuditLogs = () => {

Audit Log Detail

- {[['ID', selectedLog.id], ['Action', selectedLog.action], ['Actor', selectedLog.actor_user_id], + {[['ID', selectedLog.id], ['Action', selectedLog.action], ['Actor', selectedLog.actor_name || selectedLog.actor_user_id], ['Role', selectedLog.actor_role], ['Resource', `${selectedLog.resource_type || ''} ${selectedLog.resource_id || ''}`], ['IP', selectedLog.ip], ['User Agent', selectedLog.user_agent], ['Timestamp', selectedLog.created_at ? new Date(selectedLog.created_at).toLocaleString() : '—'] diff --git a/frontend/src/pages/Reports.jsx b/frontend/src/pages/Reports.jsx index 2c59422..a8df2e6 100644 --- a/frontend/src/pages/Reports.jsx +++ b/frontend/src/pages/Reports.jsx @@ -706,20 +706,22 @@ const Reports = () => {
{/* Report Details + Generated Reports */} -
-
+
+

Report Details

- +
+ +
-
+

Generated Reports

PDF downloads
-
+
{generatedReports.length === 0 ? (

No generated reports yet. Use Generate Report to create one. diff --git a/frontend/src/tests/components/ReportTable.test.jsx b/frontend/src/tests/components/ReportTable.test.jsx index 019cd57..420a5a3 100644 --- a/frontend/src/tests/components/ReportTable.test.jsx +++ b/frontend/src/tests/components/ReportTable.test.jsx @@ -45,6 +45,7 @@ describe('ReportTable', () => { expect(screen.getByText('Review')).toBeInTheDocument(); expect(screen.getByText('Backlog')).toBeInTheDocument(); expect(screen.getByText('Completed')).toBeInTheDocument(); + expect(screen.getByText('Assignee 2')).toBeInTheDocument(); expect(screen.getAllByText('View').length).toBe(5); expect(screen.getAllByText('Unassigned').length).toBeGreaterThan(0); }); diff --git a/frontend/src/tests/pages/AdminAuditLogs.test.jsx b/frontend/src/tests/pages/AdminAuditLogs.test.jsx new file mode 100644 index 0000000..5372d17 --- /dev/null +++ b/frontend/src/tests/pages/AdminAuditLogs.test.jsx @@ -0,0 +1,68 @@ +import React from 'react'; +import { fireEvent, render, screen, waitFor } from '@testing-library/react'; + +import AdminAuditLogs from '../../pages/AdminAuditLogs'; +import { auditLogService } from '../../services/utils/api'; + +jest.mock('../../services/utils/api', () => ({ + auditLogService: { + getLogs: jest.fn(), + getLogById: jest.fn(), + }, +})); + +describe('AdminAuditLogs', () => { + beforeEach(() => { + auditLogService.getLogs.mockResolvedValue({ + logs: [ + { + id: 1, + actor_user_id: 7, + actor_name: 'Admin User', + actor_role: 'admin', + action: 'user_created', + resource_type: 'user', + resource_id: '42', + created_at: '2026-05-08T10:00:00.000Z', + }, + ], + total: 1, + pages: 1, + current_page: 1, + }); + + auditLogService.getLogById.mockResolvedValue({ + id: 1, + actor_user_id: 7, + actor_name: 'Admin User', + actor_role: 'admin', + action: 'user_created', + resource_type: 'user', + resource_id: '42', + ip: '127.0.0.1', + user_agent: 'pytest', + metadata: null, + created_at: '2026-05-08T10:00:00.000Z', + }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + test('renders actor names instead of raw ids when available', async () => { + render(); + + expect(await screen.findByText('Admin User')).toBeInTheDocument(); + expect(screen.queryByText('User 7')).not.toBeInTheDocument(); + + fireEvent.click(screen.getByRole('button', { name: /view/i })); + + await waitFor(() => { + expect(auditLogService.getLogById).toHaveBeenCalledWith(1); + }); + + expect(await screen.findByText(/Audit Log Detail/i)).toBeInTheDocument(); + expect(screen.getAllByText('Admin User').length).toBeGreaterThan(0); + }); +}); diff --git a/frontend/src/tests/pages/AdminProjects.test.jsx b/frontend/src/tests/pages/AdminProjects.test.jsx index b858cf6..3403688 100644 --- a/frontend/src/tests/pages/AdminProjects.test.jsx +++ b/frontend/src/tests/pages/AdminProjects.test.jsx @@ -3,6 +3,7 @@ import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom'; import AdminProjects from '../../pages/AdminProjects'; +import { useAuth } from '../../context/AuthContext'; import { projectService } from '../../services/utils/api'; jest.mock('../../services/utils/api', () => ({ @@ -13,6 +14,10 @@ jest.mock('../../services/utils/api', () => ({ jest.mock('../../components/LoadingSpinner', () => () =>

Loading spinner
); +jest.mock('../../context/AuthContext', () => ({ + useAuth: jest.fn(), +})); + const renderAdminProjects = () => { return render( @@ -25,6 +30,10 @@ describe('AdminProjects page', () => { beforeEach(() => { jest.spyOn(console, 'error').mockImplementation(() => {}); + useAuth.mockReturnValue({ + currentUser: { id: 1, role: 'admin' }, + }); + projectService.getAllProjects.mockReset(); projectService.getAllProjects.mockResolvedValue([ { diff --git a/frontend/src/tests/services/utils/api.test.jsx b/frontend/src/tests/services/utils/api.test.jsx new file mode 100644 index 0000000..0832c3e --- /dev/null +++ b/frontend/src/tests/services/utils/api.test.jsx @@ -0,0 +1,24 @@ +import { normalizeTaskReportDetails } from '../../../services/utils/api'; + +describe('normalizeTaskReportDetails', () => { + test('hydrates assignee_name from the user list when task rows only expose assigned_to ids', () => { + const tasks = [ + { id: 1, title: 'Task A', assigned_to: 9 }, + { id: 2, title: 'Task B', assigned_to: 10 }, + { id: 3, title: 'Task C', assigned_to: null }, + ]; + + const users = [ + { id: 9, name: 'Developer One' }, + { id: 10, name: 'Developer Two' }, + ]; + + const normalized = normalizeTaskReportDetails(tasks, users); + + expect(normalized).toEqual([ + { id: 1, title: 'Task A', assigned_to: 9, assignee_name: 'Developer One' }, + { id: 2, title: 'Task B', assigned_to: 10, assignee_name: 'Developer Two' }, + { id: 3, title: 'Task C', assigned_to: null, assignee_name: null }, + ]); + }); +}); From 99b11a22f7408ae2720ce4634a62800ad5e2d13c Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:43:17 +0100 Subject: [PATCH 36/37] fix: increase maximum height of task panel in BasicDashboard --- frontend/src/pages/BasicDashboard.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/pages/BasicDashboard.jsx b/frontend/src/pages/BasicDashboard.jsx index f2e0ea2..e57ff7c 100644 --- a/frontend/src/pages/BasicDashboard.jsx +++ b/frontend/src/pages/BasicDashboard.jsx @@ -231,7 +231,7 @@ const BasicDashboard = () => { {/* Main Content Grid */}
-
+

My Tasks

From d99e09619224a21057ee6432ddc915491fe84120 Mon Sep 17 00:00:00 2001 From: Ahmed Ikram <2571642@dundee.ac.uk> Date: Fri, 8 May 2026 00:49:23 +0100 Subject: [PATCH 37/37] refactor: consolidate imports from githubService and taskService in GitHubIntegrationDetail --- frontend/src/pages/GithubIntegrationDetail.jsx | 3 +-- frontend/src/tests/pages/GithubIntegrationDetail.test.jsx | 8 ++------ 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/frontend/src/pages/GithubIntegrationDetail.jsx b/frontend/src/pages/GithubIntegrationDetail.jsx index 6bcefe4..e1142a4 100644 --- a/frontend/src/pages/GithubIntegrationDetail.jsx +++ b/frontend/src/pages/GithubIntegrationDetail.jsx @@ -1,7 +1,6 @@ import React, { useState, useEffect } from 'react'; import { useParams, useNavigate, Link } from 'react-router-dom'; -import { githubService } from '../services/github'; -import { taskService } from '../services/utils/api'; +import { githubService, taskService } from '../services/utils/api'; import LoadingSpinner from '../components/LoadingSpinner'; const panelClass = "bg-slate-900/70 border border-slate-800/70 rounded-2xl p-6 shadow-[0_10px_30px_rgba(0,0,0,0.25)]"; diff --git a/frontend/src/tests/pages/GithubIntegrationDetail.test.jsx b/frontend/src/tests/pages/GithubIntegrationDetail.test.jsx index 5b8ec04..11790bf 100644 --- a/frontend/src/tests/pages/GithubIntegrationDetail.test.jsx +++ b/frontend/src/tests/pages/GithubIntegrationDetail.test.jsx @@ -2,8 +2,7 @@ import React from 'react'; import { fireEvent, render, screen, waitFor } from '@testing-library/react'; import GitHubIntegrationDetail from '../../pages/GithubIntegrationDetail'; -import { githubService } from '../../services/github'; -import { taskService } from '../../services/utils/api'; +import { githubService, taskService } from '../../services/utils/api'; const mockNavigate = jest.fn(); const mockUseParams = jest.fn(); @@ -19,16 +18,13 @@ jest.mock('react-router-dom', () => ({ useParams: () => mockUseParams(), })); -jest.mock('../../services/github', () => ({ +jest.mock('../../services/utils/api', () => ({ githubService: { getUserRepos: jest.fn(), getIssues: jest.fn(), getPullRequests: jest.fn(), linkTaskToGithub: jest.fn(), }, -})); - -jest.mock('../../services/utils/api', () => ({ taskService: { getAllTasks: jest.fn(), },