feat(notifications): add mark-all-as-read and fix mobile card view

[Paul-AUB]

Closes #1303

🤔 What

• Add a "Mark all as read" button on the notifications page (/ui/notifications), placed in the page header alongside the title — consistent with other page-level actions in the app (e.g. "New" on Entrances)
• Add a "Mark all as read" icon button in the notification dropdown header, next to the unread count badge
• Fix: in card (mobile) view of the notifications page, clicking a card now correctly marks the notification as read (it was only working in table view)
• Fix: the "See all notifications" link in the dropdown is now sticky (pinned footer), visible without scrolling
• Improve the notifications table: better column order (Name → Type → Action → From → Date → Read), date now formatted as DD/MM/YYYY HH:MM, card title uses the entity name instead of the raw ISO timestamp
• Replace the hardcoded >4700 massifs placeholder on the homepage with a live count from the new API endpoint

Note — selection feature skipped intentionally: The API endpoint (PUT /api/v1/notifications/read) supports passing specific ids to do a partial batch mark-as-read. This PR does not implement checkbox selection — it is all-or-nothing. The selection feature can be added in a follow-up once the UX is defined.

🤷‍♂️ Why

Users with many notifications (after a vacation, or a burst of activity in subscribed areas) had no way to clear them efficiently without clicking each one individually. The mobile card view silently never marked notifications as read, which was a regression. The >4700 massifs value on the homepage was stale and would only get more inaccurate over time.

🔍 How

• New Redux action ReadAllNotifications calls PUT /api/v1/notifications/read with an empty body (marks all unread for the authenticated user)
• New reducer ReadAllNotificationsReducer tracks loading/success/failure state
• CountUnreadNotificationsReducer handles READ_ALL_NOTIFICATIONS_SUCCESS to reset the badge to 0 immediately, without waiting for a re-fetch
• Mobile fix: onRowClick prop is now threaded through EntityTable → MobileEntityList → MobileEntityCard, so the callback fires on card tap before navigation
• Dropdown sticky footer: notification list is wrapped in a Box with overflow-y: auto / maxHeight: 400px; header and footer are flexShrink: 0 outside the scroll area
• massifs key added to dynamicNumbersUrl map pointing to GET /api/v1/massifs/count; staticValue: '>4700' removed from HeroStats

🔗 Related API PR

• API issue: feat(notification): batch mark notifications as read grottocenter-api#1559 (batch mark-as-read endpoint)
• API issue: Add GET /api/v1/massifs/count endpoint grottocenter-api#1700 (massifs count endpoint, already closed/merged)

🧪 Testing

1. Log in as a user with unread notifications
2. Open the notification dropdown → confirm "Mark all as read" icon is visible in the header, disabled when all are read
3. Click the icon → badge resets to 0, all items show as read in the list
4. Navigate to /ui/notifications → confirm button is in the page header, disabled when nothing unread
5. Click "Mark all as read" → list refreshes, all rows show read state
6. Switch to card view (mobile or toggle) → tap a card → confirm it is marked as read
7. Open the dropdown with 10+ notifications → confirm "Voir toutes les notifications" is always visible without scrolling
8. Check the homepage → massif count is a real number (not >4700)

📸 Previews

[Paul-AUB]

Need to have the backend merged before to be tested

[ClemRz]

Issues (Must Fix)

• [packages/web-app/src/pages/Notifications/index.jsx:53-56] The useEffect that triggers on readAllStatus === REDUCER_STATUS.SUCCEEDED will also fire on mount if the user previously clicked "Mark all as read" in the dropdown (since Redux state persists across navigation). This causes a redundant double-fetch alongside the initial useEffect([], []) on line 44. Consider either resetting the reducer status back to IDLE after handling success, or tracking a local "has just completed" flag instead.

  // Option A: Reset status in reducer after handling
  // Add a RESET_READ_ALL_NOTIFICATIONS action type that sets status back to IDLE
  // and dispatch it after the re-fetch.
  
  // Option B: Use a ref to track previous status
  const prevReadAllStatus = useRef(readAllStatus);
  useEffect(() => {
    if (
      prevReadAllStatus.current !== REDUCER_STATUS.SUCCEEDED &&
      readAllStatus === REDUCER_STATUS.SUCCEEDED
    ) {
      dispatch(fetchNotifications({ limit: 50, skip: 0 }));
      dispatch(countUnreadNotifications());
    }
    prevReadAllStatus.current = readAllStatus;
  }, [dispatch, readAllStatus]);
  

• [packages/web-app/src/components/appli/NotificationMenu/index.jsx:91-93] After readAllNotifications() succeeds, the badge resets to 0 but the notification list items in the dropdown are not refreshed — they still appear as unread visually (since state.menuNotifications is not re-fetched). Consider dispatching fetchMenuNotifications on success, similar to how the Notifications page re-fetches after success. A useEffect watching readAllStatus (or cross-reducer handling of READ_ALL_NOTIFICATIONS_SUCCESS in the menu reducer) would solve this.

Suggestions (Should Consider)

• [packages/web-app/src/components/common/EntityTable/entitiesConfig.jsx:12-22] DateTimeCell is a React component but is missing PropTypes validation, which is required by project conventions. Consider adding:

  import PropTypes from 'prop-types';
  // ... after the component definition:
  DateTimeCell.propTypes = {
    value: PropTypes.string
  };
  

• [packages/web-app/src/actions/Notifications/ReadAllNotifications.js:40-44] The readAllNotificationsActionFailure function signature accepts a single error parameter, but the dispatch call passes two arguments (makeErrorMessage(...) and error.message). The second argument is silently ignored. This is a copy-paste from ReadNotification.js which has the same issue. Harmless, but worth cleaning up for clarity:

        dispatch(
          readAllNotificationsActionFailure(
            makeErrorMessage(error.message, 'Reading all notifications')
          )
        );
  

• [packages/web-app/src/actions/Notifications/ReadAllNotifications.js:3] makeErrorMessage is imported from the deprecated helpers/ folder. While the same pattern exists in ReadNotification.js, new code ideally shouldn't add more usages of deprecated modules. This is not blocking since it mirrors an existing pattern, but worth noting for future cleanup.

• [packages/web-app/src/pages/Notifications/index.jsx:63-69] The error effect depends on readAllError reference equality. If the action is dispatched multiple times with the same error message, the toast won't re-trigger because the makeErrorMessage creates a new object each time (so reference does change). This is fine — just noting that error state isn't cleared when a new attempt starts, so navigating away and back could re-show the toast. Consider clearing error in the LOADING case of the reducer (which is already done: error: undefined on LOADING — good).

Nitpicks (Optional)

• [packages/web-app/src/components/common/EntityTable/entitiesConfig.jsx:75-79] The notifications column definitions now exceed 80 characters per line (e.g., { visible: true, field: 'entityName', label: 'Name', sortable: false, isTitle: true }). Consider breaking these into multi-line objects to stay within Prettier's 80-char width, which would also match the style of other column definitions in the file.

• [packages/web-app/src/components/common/EntityTable/MobileEntityList.jsx:41-42] The if (onRowClick) onRowClick(doc) is nested inside an else block of if (onToggle). Adding braces for the inner if would improve readability:

      } else {
        if (onRowClick) {
          onRowClick(doc);
        }
        openLink(link(doc));
      }
  

[ClemRz]

API endpoint ready 🎉

The backend for batch mark-as-read is implemented and open for review: GrottoCenter/grottocenter-api#1709

Endpoint contract

PUT /api/v1/notifications/read
Authorization: Bearer <token>
Content-Type: application/json

Mark all unread:

{}

or

{ "ids": [] }

Mark specific notifications:

{ "ids": [1, 2, 3] }

Responses

Status	Meaning
204	Success (no body)
400	Invalid ids (not positive integers, or >1000 items)
401	Not authenticated
403	One or more IDs don't belong to the authenticated user (or don't exist)

Notes for front-end integration

1. The current ReadAllNotifications action sends a PUT with no body — that works perfectly, it will mark all unread as read.
2. If you later add checkbox selection, just send { "ids": [selected] } to the same URL.
3. The endpoint is idempotent — calling it multiple times with the same IDs is safe.
4. Max 1000 IDs per request. If you ever need to batch more, paginate on the client side.

[Paul-AUB]

Code review follow-up

Must Fix — both addressed

useEffect firing on mount when Redux state persists (SUCCEEDED from a previous session)

Fixed in Notifications/index.jsx and NotificationMenu/index.jsx with the same pattern: a useRef initialized to the current status is compared against the new status on each render cycle. The re-fetch (and any side effect) only triggers on a transition into SUCCEEDED, not when the component mounts with SUCCEEDED already in the store.

const prevReadAllStatus = useRef(readAllStatus);

useEffect(() => {
  if (
    readAllStatus === REDUCER_STATUS.SUCCEEDED &&
    prevReadAllStatus.current !== REDUCER_STATUS.SUCCEEDED
  ) {
    // ...side effects
  }
  prevReadAllStatus.current = readAllStatus;
}, [dispatch, readAllStatus]);

Dropdown not re-fetching after readAllNotifications() succeeds

Fixed in NotificationMenu/index.jsx: added useSelector for state.readAllNotifications.status and the ref-guarded useEffect above, which dispatches both fetchMenuNotifications and countUnreadNotifications on success. Notification items in the dropdown now visually refresh to "read" state without requiring the user to close and reopen the menu.

---

Should Consider — all addressed

DateTimeCell missing PropTypes

Added PropTypes import to entitiesConfig.jsx and declared DateTimeCell.propTypes = { value: PropTypes.string }.

readAllNotificationsActionFailure called with two arguments

The action creator only accepts one argument; the second (error.message) was silently dropped. Removed the extra argument from the catch block in ReadAllNotifications.js.

---

Not Fixed — intentional

makeErrorMessage from deprecated helpers/ folder

This is pre-existing tech debt shared across many action files. Fixing it in isolation would be an unrelated refactor; out of scope for this PR.

Column definitions exceeding 80 chars / missing braces on single-line if

Style nitpicks that conflict with the existing codebase style (Airbnb config allows omitting braces for single-statement bodies, and the 80-char limit is a soft guideline in this project). Not changed.

[ClemRz]

Suggestions (Should Consider)

• [packages/web-app/src/pages/Notifications/index.jsx:70-74] The error useEffect depends on readAllError reference equality. However, makeErrorMessage returns a new string each time, so this works correctly for repeated errors. The subtle issue is that onError and formatMessage are stable (memoized), but they're still in the dependency array — which is fine for correctness but adds noise. More importantly, if the user navigates away and back while readAllError is still in the store (status is FAILED, error is set), the toast will fire again on mount. Consider clearing the error on the LOADING transition (which is already done: error: undefined on LOADING — good), but also note that if the user never retries, the stale error persists across navigations. A minor UX concern, not blocking.

• [packages/web-app/src/components/appli/NotificationMenu/index.jsx:157-165] The Tooltip wrapping the IconButton always shows "Mark all as read" as its title, even when the button is disabled (no unread notifications). On the Notifications page, you show a different tooltip ("No unread notifications") when disabled. Consider applying the same pattern here for consistency — showing why the button is disabled improves discoverability.

• [packages/web-app/src/components/common/EntityTable/entitiesConfig.jsx:14-26] DateTimeCell uses useIntl() which means it can only be rendered inside a React tree (not as a plain function call). Since cellsRender.dateTime wraps it inside JSX (<DateTimeCell value={value} />), this works. However, the component is defined at module scope alongside plain render functions — consider adding a brief comment clarifying it's a component (vs. the other plain-function renderers) so future contributors don't accidentally try to call it as a function.

Nitpicks (Optional)

• [packages/web-app/src/components/common/EntityTable/entitiesConfig.jsx:78-97] The notification column definition objects on lines 78, 97, etc. are single-line and exceed 80 characters. The existing pattern in the file uses multi-line objects for columns with render functions. Consider breaking the longer lines for consistency:

  {
    visible: true,
    field: 'entityName',
    label: 'Name',
    sortable: false,
    isTitle: true
  },
  

• [packages/web-app/src/pages/Notifications/index.jsx:82-90] The markAllButton is defined as a JSX expression stored in a variable. This is fine and readable, but the button's onClick creates a new arrow function on every render (() => dispatch(readAllNotifications())). Since Layout likely doesn't memoize action, this is harmless in practice — just noting it for awareness.

• [packages/web-app/src/actions/Notifications/ReadAllNotifications.js:3] makeErrorMessage is imported from the deprecated helpers/ folder. Pre-existing pattern shared with ReadNotification.js — not introduced by this PR, just noting for future cleanup.

---

Overall this is a clean, well-structured PR. The previous review's must-fix items (useEffect firing on mount, dropdown not re-fetching) have been properly addressed with the useRef pattern. The new action/reducer follow established conventions, translations are sorted and complete across all 15 languages, and the mobile card fix correctly threads onRowClick through the component hierarchy. Good work.

[Paul-AUB]

Second code review follow-up

Suggestions — all addressed

Stale error toast on navigation back (Notifications/index.jsx)

Applied the same ref pattern to the error effect. prevReadAllError holds the previous error reference; the toast only fires when readAllError is both truthy and different from the previous value. Since makeErrorMessage returns a new string on each call and the reducer clears error to undefined on LOADING, repeated retries still trigger the toast correctly — and navigating back to the page with a stale FAILED state no longer re-fires it.

Tooltip inconsistency in NotificationMenu

The Tooltip title now mirrors the Notifications page: "Mark all as read" when there are unread notifications, "No unread notifications" when the button is disabled.

DateTimeCell comment

Added a one-line comment above the component clarifying it must be rendered as JSX (uses useIntl), not called as a plain function — so future contributors don't accidentally copy the pattern from the surrounding cellsRender plain-function renderers.

Multi-line column objects (nitpick)

Expanded the two single-line column objects (entityName and dateInscription) in the notifications config to match the multi-line style used by the rest of the file.

---

Not Fixed — intentional

onClick arrow function in markAllButton

() => dispatch(readAllNotifications()) creates a new function reference on each render, but Layout passes action directly as a prop without memoizing it — so useCallback here would provide no actual benefit. Left as-is.

[github-actions]

Azure Static Web Apps: Your stage site is ready! Visit it here: https://orange-rock-0d4f87503-1427.westeurope.azurestaticapps.net

[Paul-AUB]

@urien Deployed on staging env, ready to be reviewed :)

[urien]

Quand je clique sur une notification elle apparait comme lue dans la petite fenêtre mais il n'y a pas de mise a jour dans la liste (tableau). peut être dans la carte il faudrait garder le point orange comme indication de message non lu sur les cartes

Tout le reste est parfait

[urien]

Voir le commentaire

[Paul-AUB]

J'ai corrigé normalement

[github-actions]

Azure Static Web Apps: Your stage site is ready! Visit it here: https://orange-rock-0d4f87503-1427.westeurope.azurestaticapps.net

[github-actions]

Azure Static Web Apps: Your stage site is ready! Visit it here: https://orange-rock-0d4f87503-1427.westeurope.azurestaticapps.net

[urien]

Je n'ai plus de notification. Est ce que tu peux modifier une fiche de cavité en France ?

[Paul-AUB]

Fait !

corrigé

[urien]

tout marche Merci @Paul-AUB