feat: notification categories by rebelchris · Pull Request #3958 · dailydotdev/daily-api

rebelchris · 2026-06-22T14:00:57Z

Adding categories to notifications so we can filter them out frontend wise

pulumi · 2026-06-22T14:02:19Z

🍹 The Update (preview) for dailydotdev/api/prod (at b1c542e) was successful.

✨ Neo Code Review

This is a standard new-image rollout adding notification category filtering, with two moderate concerns: the non-concurrent index build inside the migration transaction (should be pre-built concurrently in prod) and a Kubernetes Secret replacement that removes OneSignal keys, creating a brief replacement window. 🟡 Moderate Risk.

This deployment ships the notification category filtering feature: a new type column on user_notification, a composite index on (userId, type, date), and a GraphQL category argument on the notifications query. The DB migration job (AddUserNotificationType) runs ALTER TABLE ... ADD COLUMN IF NOT EXISTS (nullable, no default — an instant metadata-only change on Postgres) and creates the index non-concurrently inside the migration transaction.

🟡 Warning — Index creation under migration transaction: The IDX_user_notification_userId_type_date index is created NON-CONCURRENTLY inside the TypeORM migration. On a large user_notification table this will hold an ACCESS SHARE lock for the duration of index build, which can block DDL but not normal reads/writes. The migration notes acknowledge this and recommend pre-building it with CREATE INDEX CONCURRENTLY in production before this deploys so the migration becomes a no-op. Confirm that has been done.

🟡 Warning — Secret replacement (vpc-native-k8s-secret): Two keys (ONESIGNAL_WEB_API_KEY, ONESIGNAL_WEB_APP_ID) are being removed from the Kubernetes Secret, triggering a create-replacement. Any pods that currently mount this secret and reference those keys will fail if they restart during the brief window between old secret deletion and new secret creation. Verify no running workload still consumes those OneSignal keys.

🔵 Info — Historical rows will have type = NULL until the backfill script is run. The Updates category filter uses <> ALL(...) which will exclude NULL rows (SQL three-valued logic), meaning pre-migration notifications won't appear under any category filter until backfilled. The "all activity" path (no category arg) is unaffected.

Resource Changes

    Name                                                       Type                           Operation
~   vpc-native-clean-gifted-plus-cron                          kubernetes:batch/v1:CronJob    update
~   vpc-native-update-current-streak-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-update-tag-materialized-views-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-daily-digest-cron                               kubernetes:batch/v1:CronJob    update
~   vpc-native-rotate-weekly-quests-cron                       kubernetes:batch/v1:CronJob    update
~   vpc-native-calculate-top-readers-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-update-source-public-threshold-cron             kubernetes:batch/v1:CronJob    update
+-  vpc-native-k8s-secret                                      kubernetes:core/v1:Secret      create-replacement
~   vpc-native-squad-posts-analytics-refresh-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-materialize-yearly-best-post-archives-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-images-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-hourly-notification-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-validate-active-users-cron                      kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-opportunities-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-clickhouse-cron          kubernetes:batch/v1:CronJob    update
~   vpc-native-expire-super-agent-trial-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-post-lifecycle-state-clickhouse-cron            kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-history-day-clickhouse-cron      kubernetes:batch/v1:CronJob    update
~   vpc-native-update-achievement-rarity-cron                  kubernetes:batch/v1:CronJob    update
+   vpc-native-api-db-migration-5e12561c                       kubernetes:batch/v1:Job        create
~   vpc-native-update-highlighted-views-cron                   kubernetes:batch/v1:CronJob    update
~   vpc-native-sync-subscription-with-cio-cron                 kubernetes:batch/v1:CronJob    update
~   vpc-native-update-trending-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-users-cron                         kubernetes:batch/v1:CronJob    update
~   vpc-native-worker-job-deployment                           kubernetes:apps/v1:Deployment  update
~   vpc-native-generate-search-invites-cron                    kubernetes:batch/v1:CronJob    update
~   vpc-native-clean-zombie-user-companies-cron                kubernetes:batch/v1:CronJob    update
~   vpc-native-temporal-deployment                             kubernetes:apps/v1:Deployment  update
~   vpc-native-clean-stale-user-transactions-cron              kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-updated-sync-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-check-analytics-report-cron                     kubernetes:batch/v1:CronJob    update
~   vpc-native-deployment                                      kubernetes:apps/v1:Deployment  update
-   vpc-native-api-clickhouse-migration-3571d872               kubernetes:batch/v1:Job        delete
~   vpc-native-personalized-digest-deployment                  kubernetes:apps/v1:Deployment  update
~   vpc-native-update-tags-str-cron                            kubernetes:batch/v1:CronJob    update
~   vpc-native-post-analytics-clickhouse-cron                  kubernetes:batch/v1:CronJob    update
~   vpc-native-personalized-digest-cron                        kubernetes:batch/v1:CronJob    update
~   vpc-native-materialize-monthly-best-post-archives-cron     kubernetes:batch/v1:CronJob    update
~   vpc-native-user-profile-analytics-history-clickhouse-cron  kubernetes:batch/v1:CronJob    update
-   vpc-native-api-db-migration-3571d872                       kubernetes:batch/v1:Job        delete
~   vpc-native-bg-deployment                                   kubernetes:apps/v1:Deployment  update
... and 15 other changes

idoshamun · 2026-06-22T14:11:03Z

+  // by type using a (userId, type, date) index without scanning a heavy user's
+  // rows and discarding non-matching types post-join.
+  @Column({ type: 'text', nullable: true })
+  type?: NotificationType;


Do we need index?

idoshamun · 2026-06-22T14:12:12Z

+-- user_notification is one of the largest tables, so this is intentionally a
+-- bounded loop rather than a single UPDATE.
+
+DO $$


I'm not sure on the consequences, but i'd do the limit per user and not 10k globally

idoshamun · 2026-06-22T14:14:45Z

@rebelchris another approach, instead of materializing the category, maybe we should just store the notification types to category mapping and then in sql we will just look for specific notification types instead of category? this will make it easy to change categories

capJavert · 2026-06-23T06:36:55Z

+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(/* sql */ `
+      ALTER TABLE "user_notification" ADD COLUMN IF NOT EXISTS "type" text;


why do we need type here, would join not work?

capJavert · 2026-06-23T06:37:36Z

+const notificationCategoryToTypes: Partial<
+  Record<NotificationFilterCategory, NotificationType[]>
+> = {
+  [NotificationFilterCategory.Upvotes]: [
+    NotificationType.ArticleUpvoteMilestone,
+    NotificationType.CommentUpvoteMilestone,
+  ],


We don't have this kind of mapping somewhere else, eg. per CIO topics for example?

capJavert · 2026-06-23T06:38:43Z

+          // (userId, type, date) index can drive the query for heavy users.
+          if (categoryFilter && 'include' in categoryFilter) {
+            builder.queryBuilder.andWhere(`un."type" = ANY(:types)`, {
+              types: categoryFilter.include,


we need to check categoryFilter.include is non empty, if it is it will break SQL syntax i think, same below

feat: notification categories

b1c542e

rebelchris requested review from capJavert and idoshamun June 22, 2026 14:00

idoshamun reviewed Jun 22, 2026

View reviewed changes

capJavert reviewed Jun 23, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat: notification categories#3958

feat: notification categories#3958
rebelchris wants to merge 1 commit into
mainfrom
feat-notification-categories

rebelchris commented Jun 22, 2026

Uh oh!

pulumi Bot commented Jun 22, 2026

Uh oh!

idoshamun Jun 22, 2026

Uh oh!

idoshamun Jun 22, 2026

Uh oh!

idoshamun commented Jun 22, 2026

Uh oh!

capJavert Jun 23, 2026

Uh oh!

capJavert Jun 23, 2026

Uh oh!

capJavert Jun 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

rebelchris commented Jun 22, 2026

Uh oh!

pulumi Bot commented Jun 22, 2026

✨ Neo Code Review

Resource Changes

Uh oh!

idoshamun Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

idoshamun Jun 22, 2026

Choose a reason for hiding this comment

Uh oh!

idoshamun commented Jun 22, 2026

Uh oh!

capJavert Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

capJavert Jun 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants