feat(notification): add batch mark-as-read endpoint

[ClemRz]

🤔 What

• Add PUT /api/v1/notifications/read endpoint for batch marking notifications as read
• Closes feat(notification): batch mark notifications as read #1559

🤷‍♂️ Why

Users with many unread notifications (after a vacation or a burst of activity in subscribed areas) have no efficient way to clear them. The front-end (GrottoCenter/grottocenter-front#1427) needs a batch endpoint to support a "mark all as read" action and future checkbox selection.

🔍 How

• Controller (api/controllers/v1/notification/mark-as-read-batch.js):
  • If ids is provided and non-empty: validates they are positive integers, enforces a max of 1000 IDs, verifies ownership via a single count() query, then batch-updates only unread ones
  • If ids is omitted or empty: marks all unread notifications for the authenticated user as read
  • Returns 403 for both non-owned and non-existent IDs (prevents ID enumeration)
  • Returns 204 No Content on success (idempotent)
• Route: PUT /api/v1/notifications/read → v1/notification/mark-as-read-batch
• Policy: tokenAuth (JWT required)
• Swagger: OpenAPI 3.0 documentation added

🧪 Testing

9 integration tests covering:

• Unauthenticated access (401)
• Mark all unread (empty body / empty ids array)
• Mark specific IDs only
• Ownership validation (403 for other user's notifications)
• Non-existent IDs (403)
• Idempotency (already-read notifications silently skipped)
• Invalid input (400 for non-integer/negative IDs)
• Max length exceeded (400 for >1000 IDs)

All 3097 existing tests pass with no regressions.

[Paul-AUB]

Clean, well-scoped implementation that matches the issue spec closely. The ownership check via a single count() query is an efficient approach, and returning 403 for both non-owned and non-existent IDs neatly prevents ID enumeration. Route registration (PUT vs. the existing POST — no method collision), policy, Swagger, and the 9 integration tests all look solid. A few non-blocking points below.

Suggestions (Should Consider)

1. [mark-as-read-batch.js:24-33] Duplicate IDs in the request can produce a false 403. If the client sends { ids: [1, 1, 2] }, count() returns 2 (distinct rows) while ids.length is 3, so the strict ownedCount !== ids.length check rejects a request whose IDs all legitimately belong to the user. Consider de-duplicating up front, e.g. const ids = [...new Set(rawIds)], before the length/ownership comparisons. This also makes the MAX_IDS ceiling reflect distinct IDs.

2. [mark-as-read-batch.js:24] mark-as-read.js casts the token id with Number(req.token.id) when comparing ownership, whereas here notified: req.token.id is passed to Waterline raw. It mirrors count-unread.js, so it's consistent and the tests pass — just flagging the divergence from mark-as-read.js in case a future change relies on a numeric type. No action needed if Waterline coercion is being relied on intentionally.

Nitpicks (Optional)

3. [mark-as-read-batch.js] Unlike count-unread.js (which wraps its logic in try/catch + ErrorService), this controller has no explicit error handling. It follows the mark-as-read.js style and Sails will surface a 500 on rejection, so this is fine — noting only for consistency awareness across the notification controllers.

[ClemRz]

Thanks for the review @Paul-AUB!

Item 1 (duplicate IDs) — Good catch. Fixed in 0108e72: the raw input is now de-duplicated with new Set() before the length check and ownership query. Validation still runs on the raw array so invalid values are caught regardless.

Item 2 (token id coercion) — Acknowledged, intentionally consistent with count-unread.js. Leaving as-is.

Item 3 (no try/catch) — Acknowledged, follows mark-as-read.js style. Leaving as-is.