diff options
| author | Christophe Besson <cbesson@gmail.com> | 2026-09-02 12:55:05 +0200 |
|---|---|---|
| committer | Christophe Besson <cbesson@gmail.com> | 2026-09-02 12:55:05 +0200 |
| commit | 16b5e5c463ec998c6434bbf2ef39113ed01da801 (patch) | |
| tree | e8c7f65517af3235013a660641079d02251e679f /packages/meshbay-hub/src/meshbay_hub/api | |
| parent | a194b333169efb5e25bc05c94567600eec8bb823 (diff) | |
| download | meshbay-16b5e5c463ec998c6434bbf2ef39113ed01da801.tar.gz | |
fix(hub): dismissing a notification deletes it
The previous commit filtered the list to unread, which corrected what the
reader saw and left every dismissed row in the table, invisible for ever. That
is a place to hide the disagreement rather than a resolution, and the operator
said so: "elles s'accumulent pour rien."
So dismissing drops the row. It is the reasoning `purge_notifications` has
carried all along — "these are signals, not a record: the group is still
there, the message is still in the chat, the invitation is still an
invitation" — applied one at a time instead of only in bulk.
- `DELETE /v1/notifications/{id}` is the honest name and what the SPA calls.
- `POST /{id}/read` reaches the same handler and now deletes too. It has to
keep working: the interface ships inside the desktop package, so a hub is
always answering some client older than itself, and giving the old path the
new behaviour means those clients stop hoarding as well rather than only the
updated ones.
- `read-all` deletes rather than marking, which makes it `DELETE ""` under an
older name. Marking would have made it the one route still filling the
table. Nothing in this repo calls it, but a reachable endpoint is one that
can be called.
`Notification.read` is now vestigial — nothing stored can be read, because
reading it deletes it. It stays because dropping a column is a migration for
no gain, and `unread_only` stays because a SPA newer than its hub still needs
it to be right. Both are said in the module docstring rather than left to be
worked out.
Two existing tests encoded the old semantics and now assert the opposite;
test_notification_dismissal.py gains one for the old `/read` path, because
version skew is the normal case here and not the exception. 617 hub tests
pass. docs/USERGUIDE.md's endpoint table updated in both places it lists them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014UtzVrzM7e2tG9fSpkR9ML
Diffstat (limited to 'packages/meshbay-hub/src/meshbay_hub/api')
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/api/notifications.py | 61 |
1 files changed, 49 insertions, 12 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/api/notifications.py b/packages/meshbay-hub/src/meshbay_hub/api/notifications.py index ca56620..9d5c125 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/notifications.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/notifications.py @@ -1,9 +1,28 @@ -"""Notification endpoints — /v1/notifications/*""" +""" +Notification endpoints — /v1/notifications/* + +**Dismissing one deletes it.** These are signals, not a record: the group is +still there, the message is still in the chat, the invitation is still an +invitation, so nothing is lost by dropping the row — which is what +`purge_notifications` below has always said, now applied to one at a time. + +That resolves a disagreement between two halves that were each defensible +alone. The interface treats a click as "this is gone" and removes the entry; +the hub marked it read and kept it; and the next launch listed read entries +too, so everything dismissed came back. Filtering the list to unread fixed +what the user saw and left the rows accumulating for nothing, invisible for +ever — which is the state this replaces. + +`Notification.read` is therefore **vestigial**: nothing stored can be read, +because reading it deletes it. It stays because dropping a column is a +migration for no gain, and `unread_only` stays because it is what an older +interface asks for and it still answers correctly — every row is unread. +""" from fastapi import APIRouter, Depends, HTTPException from datetime import datetime, timezone -from sqlalchemy import delete, func, select, update +from sqlalchemy import delete, func, select from sqlalchemy.ext.asyncio import AsyncSession from meshbay_hub.api.deps import get_current_user @@ -52,16 +71,27 @@ async def list_notifications( } +@router.delete("/{notification_id}") @router.post("/{notification_id}/read") -async def mark_read( +async def dismiss( notification_id: int, current_user: User = Depends(get_current_user), db: AsyncSession = Depends(get_db), ): + """ + Dismiss one. The row goes. + + Two paths to the same handler. `DELETE /{id}` says what happens and is what + the interface calls; `POST /{id}/read` is what every already-installed + client calls, and it has to keep working — the SPA ships inside the desktop + package, so a hub is always talking to some interface older than itself. + Giving the old path the new behaviour means those clients stop accumulating + rows too, rather than only the ones that have been updated. + """ notif = await db.get(Notification, notification_id) if not notif or notif.user_id != current_user.id: raise HTTPException(status_code=404, detail="Notification not found") - notif.read = True + await db.delete(notif) await db.commit() return {"status": "ok"} @@ -76,7 +106,8 @@ async def purge_notifications( These are signals, not a record: the group is still there, the message is still in the chat, the invitation is still an invitation. Nothing is lost by - clearing the list, so it clears rather than marking a hundred rows read. + clearing the list, so it clears rather than marking a hundred rows read — + the reasoning the whole module now follows. """ result = await db.execute( delete(Notification).where(Notification.user_id == current_user.id)) @@ -85,17 +116,23 @@ async def purge_notifications( @router.post("/read-all") -async def mark_all_read( +async def dismiss_all( current_user: User = Depends(get_current_user), db: AsyncSession = Depends(get_db), ): - await db.execute( - update(Notification) - .where(Notification.user_id == current_user.id, Notification.read == False) # noqa: E712 - .values(read=True) - ) + """ + Dismiss every one — the same thing as `DELETE ""`, under the name an older + client knows it by. + + Marking them read instead would put back exactly what this change removes: + rows the list can never show again. Nothing in this repository calls it, + but an endpoint that is reachable is an endpoint that can be called, and it + should not be the one route that still hoards. + """ + result = await db.execute( + delete(Notification).where(Notification.user_id == current_user.id)) await db.commit() - return {"status": "ok"} + return {"status": "ok", "removed": result.rowcount} async def create_notification( |