From 8bc951b0ff8ad2b67502c93f9c8761faa6cd6126 Mon Sep 17 00:00:00 2001 From: Chris Date: Tue, 4 Nov 2025 14:29:31 +0200 Subject: [PATCH] Replace ?uid with /:uid (#482) * Replace ?uid with /:uid * fixup! Replace ?uid with /:uid --- backend/routes/tasks.js | 84 ++----------------- .../integration/permissions-tasks.test.js | 4 +- .../recurring-display-fixes.test.js | 8 +- .../tests/integration/recurring-tasks.test.js | 2 +- frontend/utils/tasksService.ts | 2 +- 5 files changed, 14 insertions(+), 86 deletions(-) diff --git a/backend/routes/tasks.js b/backend/routes/tasks.js index 12e9feb..fd2b30b 100644 --- a/backend/routes/tasks.js +++ b/backend/routes/tasks.js @@ -1342,93 +1342,24 @@ router.get('/tasks', async (req, res) => { } }); -// GET /api/task?uid=... -router.get('/task', async (req, res) => { - try { - const { uid } = req.query; - - if (_.isEmpty(uid)) { - return res - .status(400) - .json({ error: 'uid query parameter is required' }); - } - - const task = await Task.findOne({ - where: { - uid: uid, - }, - include: [ - { - model: Tag, - attributes: ['id', 'name', 'uid'], - through: { attributes: [] }, - }, - { - model: Project, - attributes: ['id', 'name', 'uid'], - required: false, - }, - { - model: Task, - as: 'Subtasks', - required: false, - include: [ - { - model: Tag, - attributes: ['id', 'name', 'uid'], - through: { attributes: [] }, - }, - ], - }, - ], - }); - - if (!task) { - return res.status(404).json({ error: 'Task not found.' }); - } - - // Ensure read access to the task - const access = await permissionsService.getAccess( - req.currentUser.id, - 'task', - task.uid - ); - if (access === 'none') { - return res.status(403).json({ error: 'Forbidden' }); - } - - const serializedTask = await serializeTask( - task, - req.currentUser.timezone, - { skipDisplayNameTransform: true } - ); - - res.json(serializedTask); - } catch (error) { - logError('Error fetching task by UID:', error); - res.status(500).json({ error: 'Internal server error' }); - } -}); - -// GET /api/task/:id +// GET /api/task/:uid - fetch task by UID only router.get( - '/task/:id', + '/task/:uid', hasAccess( 'ro', 'task', async (req) => { - const t = await Task.findOne({ - where: { id: req.params.id }, - attributes: ['uid'], - }); - return t?.uid; + // Return the UID directly for permission checking + return req.params.uid; }, { notFoundMessage: 'Task not found.' } ), async (req, res) => { try { + const { uid } = req.params; + const task = await Task.findOne({ - where: { id: req.params.id }, + where: { uid }, include: [ { model: Tag, @@ -1443,6 +1374,7 @@ router.get( { model: Task, as: 'Subtasks', + required: false, include: [ { model: Tag, diff --git a/backend/tests/integration/permissions-tasks.test.js b/backend/tests/integration/permissions-tasks.test.js index e6237a7..d1f5bd0 100644 --- a/backend/tests/integration/permissions-tasks.test.js +++ b/backend/tests/integration/permissions-tasks.test.js @@ -31,13 +31,13 @@ describe('Tasks Permissions', () => { expect(res.body.error).toBe('Forbidden'); }); - it("GET /api/task?uid=... should return 403 for other user's task", async () => { + it("GET /api/task/:uid should return 403 for other user's task", async () => { const otherTask = await Task.create({ name: 'Other Task', user_id: otherUser.id, }); - const res = await agent.get(`/api/task?uid=${otherTask.uid}`); + const res = await agent.get(`/api/task/${otherTask.uid}`); expect(res.status).toBe(403); expect(res.body.error).toBe('Forbidden'); }); diff --git a/backend/tests/integration/recurring-display-fixes.test.js b/backend/tests/integration/recurring-display-fixes.test.js index b260d9d..6304f41 100644 --- a/backend/tests/integration/recurring-display-fixes.test.js +++ b/backend/tests/integration/recurring-display-fixes.test.js @@ -286,9 +286,7 @@ describe('Recurring Task Display Fixes', () => { priority: Task.PRIORITY.MEDIUM, }); - const response = await agent.get( - `/api/task?uid=${recurringTask.uid}` - ); + const response = await agent.get(`/api/task/${recurringTask.uid}`); expect(response.status).toBe(200); expect(response.body.name).toBe('My Weekly Review'); @@ -306,9 +304,7 @@ describe('Recurring Task Display Fixes', () => { priority: Task.PRIORITY.MEDIUM, }); - const response = await agent.get( - `/api/task?uid=${monthlyTask.uid}` - ); + const response = await agent.get(`/api/task/${monthlyTask.uid}`); expect(response.status).toBe(200); expect(response.body.name).toBe('Monthly Budget Review'); diff --git a/backend/tests/integration/recurring-tasks.test.js b/backend/tests/integration/recurring-tasks.test.js index b9d8f7e..94286f9 100644 --- a/backend/tests/integration/recurring-tasks.test.js +++ b/backend/tests/integration/recurring-tasks.test.js @@ -545,7 +545,7 @@ describe('Recurring Tasks API', () => { }); it('should return recurring task with all recurrence fields', async () => { - const response = await agent.get(`/api/task/${recurringTask.id}`); + const response = await agent.get(`/api/task/${recurringTask.uid}`); expect(response.status).toBe(200); expect(response.body.name).toBe('Test Recurring Task'); diff --git a/frontend/utils/tasksService.ts b/frontend/utils/tasksService.ts index 90bc570..c075454 100644 --- a/frontend/utils/tasksService.ts +++ b/frontend/utils/tasksService.ts @@ -100,7 +100,7 @@ export const fetchTaskById = async (taskId: number): Promise => { }; export const fetchTaskByUid = async (uid: string): Promise => { - const response = await fetch(`/api/task?uid=${encodeURIComponent(uid)}`, { + const response = await fetch(`/api/task/${encodeURIComponent(uid)}`, { credentials: 'include', headers: getDefaultHeaders(), });