Skip to content

TJK: In-app notification bell with per-user history#8

Open
gqcorneby wants to merge 24 commits into
release/est/tjk/0.9.7from
feature/in-app-notifications
Open

TJK: In-app notification bell with per-user history#8
gqcorneby wants to merge 24 commits into
release/est/tjk/0.9.7from
feature/in-app-notifications

Conversation

@gqcorneby
Copy link
Copy Markdown

@gqcorneby gqcorneby commented May 25, 2026

📌 References

Purpose

  • Email-only notifications left no history — once an alert email was missed or deleted, there was no way to recover it from the application.
  • Users without email addresses were silently excluded from all alerts.
  • For each notification sent via email, create an in app notification

📝 Implementation

  • CustomNotification domain class + custom_notification Liquibase migration — stores one record per user per event (user-keyed, never email-keyed)
  • CustomNotificationServicenotifyUsers(), listForUser(), markRead(), markAllRead(), countUnread()
  • NotificationDispatcherService — single entry point: records in-app notification and optionally sends email; replaces all direct mailService.sendHtmlMail calls in NotificationService
  • NotificationType enum — SHIPMENT, REQUISITION, FULFILLMENT, STOCK_ALERT, USER_ACCOUNT, PRODUCT, SYSTEM, EMAIL_TRIGGER
  • CustomNotificationController + REST API (GET/PUT /api/custom/notifications, mark-read, mark-all-read, unread count)
  • Bell UI: _bell.gsp partial for GSP pages + NotificationBell.jsx React component for React pages — polling unread count, dropdown list, mark-read on click
  • Hooked into all existing notification flows: shipment shipped/received, per-item shipped/received, requisition updates, fulfillment, stock/expiry alerts, user signup confirmation, user activation/deactivation, new product created
  • UserController.toggleActivation: added @Transactional to fix pre-existing upstream bug (flush:true save in a non-transactional controller action)

✨ Description of Change

Adds a persistent in-app notification inbox to complement (not replace) email alerts. Every event that previously sent an email now also writes a custom_notification row per recipient. Users see a bell icon with an unread badge; clicking opens a dropdown of recent notifications with mark-read support.

Key design decisions:

  • User-keyed, not email-keyed: recipients without an email address still receive in-app notifications
  • withTransaction over withNewSession: participates in the caller's transaction when one exists (avoids MySQL lock-timeout on signup), creates a new session when called from background threads (Quartz/GPars)
  • sendEmail=false escape hatch: per-recipient flows (shipment items shipped/received) handle their own email send; the dispatcher records in-app only
  • Upstream isolation: all new code under org.pih.warehouse.custom.notifications.* and src/js/custom/notifications/; upstream files have surgical, documented edits

📷 Screenshots & Recordings (optional)

2026-05-25.15-00-15.trimmed.trimmed.mp4

🔥 Notes to the tester

cd docker && docker compose up -d db
./gradlew bootRun -Dserver.port=8081
  1. Stock alert: trigger SendStockAlertsJob via Admin > Quartz > Jobs — bell badge increments, dropdown shows alert
  2. Shipment shipped: ship a stock movement — users with ROLE_SHIPMENT_NOTIFICATION at origin/destination each get a notification; per-item recipients get an individual one
  3. Shipment received: receive a shipment — same role-based recipients notified
  4. Requisition update: approve or update a requisition — requester gets a notification
  5. Fulfillment: issue (ship) a requisition-backed movement — requester gets a FULFILLMENT notification
  6. User signup: complete a self-registration flow — new user gets a USER_ACCOUNT confirmation notification
  7. User activation/deactivation: toggle the Active checkbox on a user via Admin > Users > edit — user and admins get a USER_ACCOUNT notification
  8. Product created: create a new product — users with ROLE_PRODUCT_NOTIFICATION get a PRODUCT notification
  9. Mark read: click a notification in the dropdown — unread badge decrements
  10. Mark all read: click "Mark all as read" — badge clears

gqcorneby added 16 commits May 25, 2026 10:56
…gnup

withNewSession opened a separate Hibernate session while handleSignup
held a write lock on the user row. The FK check in the notification
insert then blocked until lock timeout.

withTransaction alone is sufficient: it participates in the caller's
session when one exists (request threads) and creates a new one when
there isn't one (Quartz/GPars background threads).
Feature is always-on; a config flag adds complexity for no gain.
Removing the flag and the grailsApplication dependency it required.
…eation

- Add PRODUCT NotificationType enum value
- ProductController.sendProductCreatedNotification: replace mailService
  call with notificationDispatcherService (handles both in-app + email)
- UserController.sendUserStatusChanged: same replacement for
  user activation/deactivation emails (USER_ACCOUNT type)
- UserController.toggleActivation: add @transactional to fix pre-existing
  upstream bug (save flush:true in a controller with no transaction)
@gqcorneby gqcorneby changed the base branch from develop to release/est/tjk/0.9.7 May 25, 2026 05:54
gqcorneby added 5 commits May 25, 2026 13:58
Flag was removed in a prior refactor. Clean up the stale comment in
NotificationDispatcherService, the dead integration test that toggled
the flag, and the commented-out block in the client config template.
sandbox="" blocked all navigation so links redirected inside the
sessionless frame → login screen. allow-popups lets clicks escape;
base target="_blank" forces all anchors into a real browser tab.
- Badge top/right set to 2px/2px on both GSP and React bells for
  consistent positioning across dashboard and other pages
- Add X-Requested-With: XMLHttpRequest to GSP bell fetch calls so
  Spring Security returns 401 instead of redirecting to login, keeping
  the saved redirect URL on the actual page the user was on
- Map body column as TEXT in GORM static mapping (Liquibase already creates
  it as TEXT; without this GORM would generate VARCHAR(255) on schema-create)
- Add X-Requested-With header to GSP bell PUT calls for session-expiry parity
- Show fetch error message in dropdown when load fails and list is empty
- Remove identity-wrapper handlers in NotificationBell; pass markRead /
  markAllRead directly as props
- Clarify notifyUsers catch comment: Hibernate constraint violations can still
  roll back the full batch
- Fix notificationsApi tests: assert offset in all getNotifications cases,
  add forwarded-offset test case
- Add notifications.bell.error i18n key (EN / RU / TG)
gqcorneby added 3 commits May 25, 2026 16:24
Archive in-app-notifications and notifications-decouple-email to
openspec/changes/archive/. Promote the synced capability spec to
openspec/specs/in-app-notifications/, reflecting the user-keyed,
email-decoupled dispatcher design. Drop the inApp.enabled toggle
requirement (flag was removed when toggle-off scope grew to hiding
the bell).
Drop the never-populated linkUrl deep-link slot across the notification
domain, controller DTO, React modal/dropdown, GSP bell, i18n bundles,
and migration. No production caller ever set the value, so both UI
guards (isSameOriginPath) were unreachable.
Sync the in-app-notifications capability spec: drop link_url navigation
from the click scenarios and assert the payload carries no linkUrl.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant