Notifications backend by davidwaroquiers · Pull Request #1904 · datalab-org/datalab

davidwaroquiers · 2026-07-06T10:14:00Z

Backend notifications API

What this PR does

This adds the backend pieces for in-app notifications (see #1875)

Notifications are stored per user/recipient: even when one API request targets multiple users, the backend creates one notification document per recipient. This is intentional, because it keeps read/archive/delete state user-specific and avoids shared notification state becoming awkward. We are also not foreseeing a lot of messages that are sent to a lot of users except for update/maintenance notifications.

The feature is behind ENABLE_NOTIFICATIONS, which is still disabled by default. When enabled, the backend exposes routes to create, list, read/unread, archive/unarchive, delete, count unread notifications, and mark everything as read.

The PR also adds grouped notifications, so repeated events from the same source can update one existing notification instead of creating a new notification every time.

How to use it

Currently, only admins can create notifications with:

POST /notifications
{
  "recipient_ids": ["..."],
  "title": "Import finished",
  "summary": "2 rows need attention",
  "message": "Some imported rows could not be matched.",
  "level": "important"
}

To send to every active user:

{
  "send_all_users": true,
  "title": "Scheduled maintenance"
}

Users can then use:

GET /notifications
GET /notifications/unread-count
PATCH /notifications/
POST /notifications/mark-all-read
DELETE /notifications/

Users can only access their own notifications.

Grouping behavior

A notification can be grouped by passing a grouping object:

{
  "recipient_ids": ["..."],
  "title": "Equipment anomaly",
  "summary": "Drift detected",
  "message": "The latest ingestion run detected drift.",
  "level": "critical",
  "grouping": {
    "key": "ingestion:equipment:eq-1",
    "policy": "once",
    "max_occurrences": 100
  }
}

Grouping "identity" is based on:

recipient
title
grouping key
grouping policy
window_seconds for window grouping

Archived notifications are not reused.
For grouped notifications:

new occurrences are appended to occurrences
summary and message are updated to the latest occurrence
level keeps the highest priority level seen so far
a read notification becomes unread again when a new occurrence is added
each occurrence has is_new, meaning “new since this notification was last read.”
max_occurrences is owned by the stored grouped notification. So if the first grouped notification is created with max_occurrences=2, later matching events with max_occurrences=100 still group only until the stored cap of 2 is reached. The incoming cap only matters when a new grouped notification document is created.

Notification levels

Current notification levels are:

low
normal
important
urgent
critical

They have explicit priorities in the model, so grouped notifications can keep the highest-priority level.

No frontend changes are included in this PR.

The helper API create_notification(...) remains available for direct backend callers.

Keep it for now with the minimal features required.

codecov · 2026-07-06T10:16:33Z

Codecov Report

❌ Patch coverage is 93.10345% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.48%. Comparing base (2b24653) to head (12efc5f).
⚠️ Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
...datalab/src/pydatalab/routes/v0_1/notifications.py	90.97%	13 Missing ⚠️
pydatalab/src/pydatalab/notifications.py	93.47%	3 Missing ⚠️
pydatalab/src/pydatalab/permissions.py	87.50%	3 Missing ⚠️
pydatalab/src/pydatalab/models/notifications.py	98.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1904      +/-   ##
==========================================
+ Coverage   79.94%   80.48%   +0.53%     
==========================================
  Files          83       86       +3     
  Lines        7481     7782     +301     
==========================================
+ Hits         5981     6263     +282     
- Misses       1500     1519      +19

Files with missing lines	Coverage Δ
pydatalab/src/pydatalab/config.py	`80.13% <100.00%> (+0.13%)`	⬆️
pydatalab/src/pydatalab/feature_flags.py	`76.47% <100.00%> (+1.16%)`	⬆️
pydatalab/src/pydatalab/main.py	`90.67% <100.00%> (+0.24%)`	⬆️
pydatalab/src/pydatalab/models/__init__.py	`100.00% <100.00%> (ø)`
pydatalab/src/pydatalab/mongo.py	`85.96% <100.00%> (+0.51%)`	⬆️
pydatalab/src/pydatalab/routes/v0_1/__init__.py	`100.00% <100.00%> (ø)`
pydatalab/src/pydatalab/models/notifications.py	`98.30% <98.30%> (ø)`
pydatalab/src/pydatalab/notifications.py	`93.47% <93.47%> (ø)`
pydatalab/src/pydatalab/permissions.py	`89.13% <87.50%> (-0.35%)`	⬇️
...datalab/src/pydatalab/routes/v0_1/notifications.py	`90.97% <90.97%> (ø)`

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gpetretto

Overall it looks good to me. I left some comments.

gpetretto · 2026-07-15T12:51:39Z

+
+                if flask_mongo.db.users.find_one(
+                    {"_id": recipient_object_id},
+                    {"_id": 1},


For the option to send to all recipients the selected users are active. Should it be used also here in the search? Or is it fine even if it sent to inactive users?

I guess here I would allow even deactivated users as it is an explicit send. In principle deactivated users are allowed to authenticate but not allowed to edit. I'd maybe refer to @ml-evs about what he thinks about it when we undraft the PR

ok. In practice I don't think it makes really a big difference. The only difference would be for the sender of the notification that does not get a warning that the user is deactivated.

Ok, I'll keep this conversation open for now

once using a transaction.

davidwaroquiers · 2026-07-15T17:14:13Z

Thanks for the review @gpetretto I think I addressed the comments or asked some question. Feel free to resolve or propose anything you deem right.

davidwaroquiers · 2026-07-16T09:32:56Z

This is ready for review @ml-evs @be-smith @OMWalmsley

I addressed @gpetretto 's comments, with one being left up for discussion.

As mentioned in the main text of the PR, this is only the backend part for the notifications. I will make the frontend in another PR when this is approved and merged. Note that I have already made a working version of the frontend that uses all this backend functionality. I can show it in case this helps of course.

OMWalmsley

I think it generally looks good, just a few improvements that could be done in the notifications route.

OMWalmsley · 2026-07-24T10:24:17Z

+    created_by = current_user.person.immutable_id
+
+    def create_for_recipients(session=None):
+        notification_results = []


notification_results shadows name from outer scope

Thanks, done.

OMWalmsley · 2026-07-24T10:24:23Z

+
+    def create_for_recipients(session=None):
+        notification_results = []
+        for recipient_id in recipient_ids:


recipient_id shadows name from outer scope

Thanks, done.

OMWalmsley · 2026-07-24T10:24:28Z

+                    grouping=grouping,
+                    session=session,
+                )
+            except (ValidationError, ValueError) as exc:


exc also shadows name from outer scope

Thanks, done.

OMWalmsley · 2026-07-24T10:24:31Z

+    try:
+        hello = flask_mongo.cx.admin.command("hello")
+        supports_transactions = hello.get("msg") == "isdbgrid" or "setName" in hello
+    except Exception:


Is it possible to narrow down what this Exception should be?

(e.g. pymongo.errors.PyMongoError)

Thanks, done.

OMWalmsley · 2026-07-24T10:24:38Z

+    except Exception:
+        supports_transactions = False
+
+    if supports_transactions:


supports_transactions could be removed completely and the body of this if statement could be moved into else statement after try...except

Can be indeed. I did it as it is probably cleaner. I would just wondering initially that we could actually make this test once at deployment time somehow and not having to test it everytime (and it would/could be read from current deployment config file or similar). Not sure if that's worth anyway so I propose we keep it as you propose for now.

OMWalmsley · 2026-07-24T10:24:41Z

+            with session.start_transaction():
+                notification_results = create_for_recipients(session=session)
+    else:
+        notification_results = create_for_recipients()


Could move this into the except statement

Refactored (see above comment)

OMWalmsley · 2026-07-24T10:24:48Z

+@NOTIFICATIONS.route("/notifications/<notification_id>", methods=["PATCH"])
+@notification_recipient_only
+def update_notification(
+    notification_id: str,


Similar to other comment (about notification_id).

Thanks, done.

OMWalmsley · 2026-07-24T10:24:58Z

+@NOTIFICATIONS.route("/notifications/<notification_id>", methods=["DELETE"])
+@notification_recipient_only
+def delete_notification(
+    notification_id: str,


notification_id is not used in this function, could possibly have decorator (@notification_recipient_only) remove this from **kwargs. Since the only two instances using it, do not make use of the notification_id.

Thanks, done.

Removed supports_transaction and refactored the try transaction and fallback to no transactions part.

update_notification. Consumes notification_id from the kwargs in the notification_recipient_only decorator. Narrowed down exceptions in try/except for ObjectId.

davidwaroquiers

@OMWalmsley Thanks for the comments. I think I addressed all of them.

davidwaroquiers · 2026-07-24T13:35:45Z

+    created_by = current_user.person.immutable_id
+
+    def create_for_recipients(session=None):
+        notification_results = []


Thanks, done.

davidwaroquiers · 2026-07-24T13:35:56Z

+
+    def create_for_recipients(session=None):
+        notification_results = []
+        for recipient_id in recipient_ids:


Thanks, done.

davidwaroquiers · 2026-07-24T13:36:00Z

+                    grouping=grouping,
+                    session=session,
+                )
+            except (ValidationError, ValueError) as exc:


Thanks, done.

davidwaroquiers · 2026-07-24T13:43:55Z

+    try:
+        hello = flask_mongo.cx.admin.command("hello")
+        supports_transactions = hello.get("msg") == "isdbgrid" or "setName" in hello
+    except Exception:


Thanks, done.

davidwaroquiers · 2026-07-24T13:51:48Z

+    except Exception:
+        supports_transactions = False
+
+    if supports_transactions:


Can be indeed. I did it as it is probably cleaner. I would just wondering initially that we could actually make this test once at deployment time somehow and not having to test it everytime (and it would/could be read from current deployment config file or similar). Not sure if that's worth anyway so I propose we keep it as you propose for now.

davidwaroquiers · 2026-07-24T13:52:35Z

+            with session.start_transaction():
+                notification_results = create_for_recipients(session=session)
+    else:
+        notification_results = create_for_recipients()


Refactored (see above comment)

davidwaroquiers · 2026-07-24T14:14:23Z

+@NOTIFICATIONS.route("/notifications/<notification_id>", methods=["PATCH"])
+@notification_recipient_only
+def update_notification(
+    notification_id: str,


Thanks, done.

davidwaroquiers · 2026-07-24T14:14:35Z

+@NOTIFICATIONS.route("/notifications/<notification_id>", methods=["DELETE"])
+@notification_recipient_only
+def delete_notification(
+    notification_id: str,


Thanks, done.

davidwaroquiers · 2026-07-24T14:16:37Z

I think it generally looks good, just a few improvements that could be done in the notifications route.

Thanks for your review. I believe I've addressed your comments. Feel free to check and resolve if it looks fine to you.

not enabled.

davidwaroquiers added 2 commits July 6, 2026 11:48

Added backend model and routes for notifications.

8a0faa3

Keep it for now with the minimal features required.

Merge branch 'main' into dw/notifications_backend

6b781f0

davidwaroquiers requested review from gpetretto and jbouquiaux July 6, 2026 10:14

Merge branch 'main' into dw/notifications_backend

f978bcb

gpetretto reviewed Jul 15, 2026

View reviewed changes

davidwaroquiers added 4 commits July 15, 2026 17:08

Merge branch 'main' into dw/notifications_backend

8db10f8

Some fixes after review.

08985a6

Fixed possible problem with failure to send multiple notifications at

b40ec15

once using a transaction.

More changes based on Guido's review.

bd0b224

davidwaroquiers requested a review from gpetretto July 15, 2026 17:14

davidwaroquiers mentioned this pull request Jul 16, 2026

Enable transactions #1926

Open

Removed create_notification function.

1236de2

davidwaroquiers marked this pull request as ready for review July 16, 2026 09:30

OMWalmsley reviewed Jul 24, 2026

View reviewed changes

davidwaroquiers added 3 commits July 24, 2026 15:36

Renamed variables to prevent shadowing name from outer scope.

a89f2e7

Narrowed down the exception for transaction support test.

7c089f9

Removed supports_transaction and refactored the try transaction and fallback to no transactions part.

Removed notification_id from delete_notification and

c8d4027

update_notification. Consumes notification_id from the kwargs in the notification_recipient_only decorator. Narrowed down exceptions in try/except for ObjectId.

davidwaroquiers commented Jul 24, 2026

View reviewed changes

Raising NotFound instead of calling abort(404) when notifications are

12efc5f

not enabled.

Uh oh!

Conversation

davidwaroquiers commented Jul 6, 2026

Backend notifications API

What this PR does

How to use it

Grouping behavior

Notification levels

Uh oh!

codecov Bot commented Jul 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

gpetretto left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

davidwaroquiers commented Jul 15, 2026

Uh oh!

davidwaroquiers commented Jul 16, 2026

Uh oh!

OMWalmsley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

davidwaroquiers left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented Jul 6, 2026 •

edited

Loading