diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 368c9d91..b4342ae4 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -60,9 +60,9 @@ jobs: # integration/webhookDelivery — supertest fixture # services/backupService.enhanced — knex mock chain # routes/__tests__/adminAuth — supertest fixture - # routes/__tests__/adminNotifications — supertest fixture + # (adminNotifications was excluded; #597 fix re-enables it.) npx jest \ - --testPathIgnorePatterns='/node_modules/|adminSettings\.logo\.test|integration/adminPhotos\.reference|integration/webhookDelivery|backupService\.enhanced|routes/__tests__/adminAuth|routes/__tests__/adminNotifications' \ + --testPathIgnorePatterns='/node_modules/|adminSettings\.logo\.test|integration/adminPhotos\.reference|integration/webhookDelivery|backupService\.enhanced|routes/__tests__/adminAuth' \ --ci frontend: diff --git a/backend/src/routes/__tests__/adminNotifications.test.js b/backend/src/routes/__tests__/adminNotifications.test.js index d1416545..0cbe62f3 100644 --- a/backend/src/routes/__tests__/adminNotifications.test.js +++ b/backend/src/routes/__tests__/adminNotifications.test.js @@ -28,6 +28,13 @@ jest.mock('../../middleware/auth', () => ({ adminAuth: (_req, _res, next) => next(), })); +// requirePermission is its own module — without this mock the real +// implementation runs, queries role_permissions on the mocked db, and +// 403s before we ever reach the handler. +jest.mock('../../middleware/permissions', () => ({ + requirePermission: () => (_req, _res, next) => next(), +})); + const { db } = require('../../database/db'); const notificationsRouter = require('../adminNotifications'); diff --git a/backend/src/routes/adminNotifications.js b/backend/src/routes/adminNotifications.js index bd87a780..b7d97f97 100644 --- a/backend/src/routes/adminNotifications.js +++ b/backend/src/routes/adminNotifications.js @@ -98,62 +98,22 @@ router.put('/read-all', adminAuth, requirePermission('settings.edit'), async (re } }); -// Delete old notifications (older than 30 days and read) -router.delete('/clear-old', adminAuth, requirePermission('settings.edit'), async (req, res) => { +// Clear all notifications (#597). +// +// The frontend AdminHeader "Clear All" button hits this — its service +// at `notifications.service.ts` does DELETE /admin/notifications/clear-all. +// The previous /clear-old route was named for an "older than 30 days +// and read" semantic but had a fallback that deleted EVERYTHING when +// nothing matched the date filter, so it was effectively a confusingly +// named Clear All anyway. Drop the rename and the branching, return +// the simple deletedCount the existing test (and frontend toast) expect. +router.delete('/clear-all', adminAuth, requirePermission('settings.edit'), async (req, res) => { try { - // Use database-agnostic date calculation - const thirtyDaysAgo = new Date(); - thirtyDaysAgo.setDate(thirtyDaysAgo.getDate() - 30); - - let deletedCount = 0; - const client = db?.client?.config?.client; - - if (client === 'pg') { - const primaryResult = await db.raw( - ` - WITH deleted AS ( - DELETE FROM activity_logs - WHERE read_at IS NOT NULL OR created_at < ? - RETURNING id - ) - SELECT COUNT(*)::int AS count FROM deleted - `, - [thirtyDaysAgo.toISOString()] - ); - deletedCount = primaryResult.rows?.[0]?.count || 0; - - if (deletedCount === 0) { - const fallbackResult = await db.raw( - ` - WITH deleted AS ( - DELETE FROM activity_logs - RETURNING id - ) - SELECT COUNT(*)::int AS count FROM deleted - ` - ); - deletedCount = fallbackResult.rows?.[0]?.count || 0; - } - } else { - deletedCount = await db('activity_logs') - .where(function () { - this.whereNotNull('read_at') - .orWhere('created_at', '<', thirtyDaysAgo); - }) - .delete(); - - if (deletedCount === 0) { - deletedCount = await db('activity_logs').delete(); - } - } - - res.json({ - message: deletedCount > 0 ? 'Old notifications cleared' : 'No notifications to clear', - deletedCount - }); + const deletedCount = await db('activity_logs').delete(); + res.json({ message: 'All notifications cleared', deletedCount }); } catch (error) { - console.error('Clear old notifications error:', error); - res.status(500).json({ error: 'Failed to clear old notifications' }); + console.error('Clear notifications error:', error); + res.status(500).json({ error: 'Failed to clear notifications' }); } });