fix(notifications): restore /clear-all route the frontend already calls (#597)
The AdminHeader "Clear All" notifications button has been 404'ing for
a while: frontend `notifications.service.ts` calls
`DELETE /admin/notifications/clear-all`, backend only defined
`DELETE /admin/notifications/clear-old`.
The /clear-old route was misleadingly named anyway — it tried to
delete read OR >30-days-old rows, then had a fallback that nuked
EVERY row when nothing matched. Both the frontend and the existing
test expect a simple Clear All shape, so just rename to /clear-all,
drop the tiered logic, and return the plain
`{ message, deletedCount }` payload the test asserts on.
The test (adminNotifications.test.js) was hiding the breakage —
it was on CI's --testPathIgnorePatterns ignore list and so never
ran. Two reasons it failed locally before this fix:
1. Route path mismatch (the actual #597 bug).
2. The mock only stubbed adminAuth — requirePermission lives in
its own middleware module and ran for real, 403'ing before
the handler. Add a passthrough mock for that too.
With both fixed, the test passes. Drop adminNotifications from the
CI ignore list so future regressions in this route fail loudly
instead of going to ground.
This commit is contained in:
@@ -60,9 +60,9 @@ jobs:
|
|||||||
# integration/webhookDelivery — supertest fixture
|
# integration/webhookDelivery — supertest fixture
|
||||||
# services/backupService.enhanced — knex mock chain
|
# services/backupService.enhanced — knex mock chain
|
||||||
# routes/__tests__/adminAuth — supertest fixture
|
# routes/__tests__/adminAuth — supertest fixture
|
||||||
# routes/__tests__/adminNotifications — supertest fixture
|
# (adminNotifications was excluded; #597 fix re-enables it.)
|
||||||
npx jest \
|
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
|
--ci
|
||||||
|
|
||||||
frontend:
|
frontend:
|
||||||
|
|||||||
@@ -28,6 +28,13 @@ jest.mock('../../middleware/auth', () => ({
|
|||||||
adminAuth: (_req, _res, next) => next(),
|
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 { db } = require('../../database/db');
|
||||||
const notificationsRouter = require('../adminNotifications');
|
const notificationsRouter = require('../adminNotifications');
|
||||||
|
|
||||||
|
|||||||
@@ -98,62 +98,22 @@ router.put('/read-all', adminAuth, requirePermission('settings.edit'), async (re
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
// Delete old notifications (older than 30 days and read)
|
// Clear all notifications (#597).
|
||||||
router.delete('/clear-old', adminAuth, requirePermission('settings.edit'), async (req, res) => {
|
//
|
||||||
|
// 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 {
|
try {
|
||||||
// Use database-agnostic date calculation
|
const deletedCount = await db('activity_logs').delete();
|
||||||
const thirtyDaysAgo = new Date();
|
res.json({ message: 'All notifications cleared', deletedCount });
|
||||||
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
|
|
||||||
});
|
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
console.error('Clear old notifications error:', error);
|
console.error('Clear notifications error:', error);
|
||||||
res.status(500).json({ error: 'Failed to clear old notifications' });
|
res.status(500).json({ error: 'Failed to clear notifications' });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user