bug fixers 10/20
This commit is contained in:
@@ -0,0 +1,196 @@
|
||||
# Notification Flow — Current State & Analysis
|
||||
|
||||
How notifications are raised, configured, stored and shown in ERPCore today, every event that
|
||||
raises one, and the gaps found while tracing the code (2026-10-20).
|
||||
|
||||
> **Scope:** the in-app notification system (the bell in the header) and its configuration
|
||||
> screen (**Settings → Notification Config**). The separate **Day-End Owner Report** (SMS + email
|
||||
> to the owner) is covered in `DAY_END_OWNER_REPORT_PLAN.md`; how it relates is in §8.
|
||||
|
||||
---
|
||||
|
||||
## 1. Overview
|
||||
|
||||
```
|
||||
Business action (create GRN, post sale, approve PO, stock drops …)
|
||||
│
|
||||
▼
|
||||
Service ──► INotificationService.RaiseAsync(moduleKey, title, message, entityType, entityId)
|
||||
│ or NotifyUsersAsync(userIds, moduleKey, …) (targeted: approvals)
|
||||
▼
|
||||
NotificationService.DeliverAsync
|
||||
│ 1. recipients = active users (all of them, or the given ids)
|
||||
│ 2. channels = NotificationConfigService.ResolveEnabledChannelsAsync(moduleKey)
|
||||
│ 3. In-App off (or module unknown) ──► STOP, nothing stored
|
||||
▼
|
||||
notifications (1 row) + notification_recipients (1 row per user, IsRead = false)
|
||||
│
|
||||
▼
|
||||
Frontend Header bell ──► GET /api/v1/notifications?take=20 (on page load + when the bell opens)
|
||||
│ → latest 20 for the signed-in user + unread count
|
||||
└──────────────────► POST /api/v1/notifications/mark-all-read
|
||||
```
|
||||
|
||||
Only the **In-App** channel is delivered. **Email** and **SMS** can be switched on per module in
|
||||
Settings, but nothing sends them (§7, G2).
|
||||
|
||||
---
|
||||
|
||||
## 2. Components
|
||||
|
||||
| Layer | File | Role |
|
||||
|---|---|---|
|
||||
| Module registry | `Domain/Notifications/Modules/NotificationModules.cs` | One `[NotificationModule(key, label, description, sortOrder)]` class per notification type |
|
||||
| | `Domain/Notifications/NotificationModuleAttribute.cs`, `NotificationModuleDefinition.cs` | Attribute + DTO for the registry |
|
||||
| | `Services/NotificationModuleCatalog.cs` | Finds every `[NotificationModule]` by reflection at startup; throws on duplicate keys |
|
||||
| Channels | `Domain/Notifications/NotificationChannel.cs` | `Email`, `Sms`, `InApp` |
|
||||
| Data | `Domain/Entities/Notification.cs` | The event: `ModuleKey`, `Title`, `Message`, `RelatedEntityType`, `RelatedEntityId`, `CreatedAt` |
|
||||
| | `Domain/Entities/NotificationRecipient.cs` | Per-user copy: `UserId`, `IsRead`, `ReadAt` |
|
||||
| | `Domain/Entities/NotificationConfig.cs` | Singleton settings row (`ConfigId = 1`, `RowVersion` → ETag) |
|
||||
| | `Domain/Entities/NotificationConfigModule.cs` | Per module: `EmailEnabled`, `SmsEnabled`, `InAppEnabled` |
|
||||
| Delivery | `Services/NotificationService.cs` | `RaiseAsync`, `NotifyUsersAsync`, `GetFeedAsync`, `MarkAllReadAsync` |
|
||||
| Settings | `Services/NotificationConfigService.cs` | Get/update settings, `ResolveEnabledChannelsAsync(moduleKey)` |
|
||||
| Low stock | `Services/Stock/LowStockTracker.cs`, `LowStockAlertService.cs`, `Infra/Filters/LowStockAlertFilter.cs` | Collects stock issues during a request, raises `stock.low` after it commits |
|
||||
| API | `Controllers/NotificationsController.cs` | `GET /api/v1/notifications`, `POST /api/v1/notifications/mark-all-read` |
|
||||
| | `Controllers/NotificationConfigController.cs` | `GET/PUT /api/v1/notification-config` (policy `settings`) |
|
||||
| Frontend | `components/Layouts/Header.tsx` | Bell icon, unread badge, popover list, "mark all read" |
|
||||
| | `app/dashboard/settings/notification-config/page.tsx` | Per-module Email / SMS / In-App switches |
|
||||
| | `lib/api/notifications.ts`, `lib/api/notification-config.ts` | API clients |
|
||||
|
||||
---
|
||||
|
||||
## 3. Data model
|
||||
|
||||
```
|
||||
notification_config (1 row)
|
||||
└─< notification_config_modules (ModuleKey, EmailEnabled, SmsEnabled, InAppEnabled)
|
||||
|
||||
notifications (NotificationId, ModuleKey, Title, Message, RelatedEntityType, RelatedEntityId, CreatedAt)
|
||||
└─< notification_recipients (NotificationId, UserId, IsRead, ReadAt)
|
||||
```
|
||||
|
||||
- One `notifications` row per event; one `notification_recipients` row **per user** who should see it.
|
||||
- Read state is per user (`IsRead`, `ReadAt`).
|
||||
- Nothing is ever deleted or archived — both tables grow forever (§7, G7).
|
||||
|
||||
---
|
||||
|
||||
## 4. Configuration flow
|
||||
|
||||
1. **Registry.** Each notification type is a class with `[NotificationModule("grn.created", "GRN Created", "…", 30)]`. The catalog is built once by reflection, so **adding a module = adding one class**; it then appears on the settings screen automatically.
|
||||
2. **Seed.** `DataSeeder.SeedNotificationConfigAsync` creates the singleton `notification_config` row **with no module rows**.
|
||||
3. **Settings screen** (`/dashboard/settings/notification-config`): lists every catalog module with three switches (Email, SMS, In-App). Saving sends **every** module (`PUT`, `If-Match` ETag); the server rejects unknown keys (422) and missing keys (422), then replaces all module rows.
|
||||
4. **At send time** `ResolveEnabledChannelsAsync(moduleKey)` reads that module's row. **No row ⇒ no channels ⇒ nothing is delivered.**
|
||||
|
||||
> Consequence: on a fresh database **every notification is off** until an admin opens Settings →
|
||||
> Notification Config and switches modules on (§7, G3).
|
||||
|
||||
---
|
||||
|
||||
## 5. Delivery flow (backend)
|
||||
|
||||
### 5.1 Broadcast — `RaiseAsync`
|
||||
Used by almost every trigger. Recipients = **every active user** in the system — no role, branch
|
||||
or permission filter (§7, G4).
|
||||
|
||||
### 5.2 Targeted — `NotifyUsersAsync(userIds, …)`
|
||||
Used by the approval flow only: the approvers of a pending request, or the requester when it is
|
||||
approved / rejected. Inactive users in the list are dropped.
|
||||
|
||||
### 5.3 Shared steps (`DeliverAsync`)
|
||||
1. Stop if there are no active recipients.
|
||||
2. Resolve channels for the module; stop unless **In-App** is enabled.
|
||||
3. Insert one `Notification` + one `NotificationRecipient` per user, `SaveChangesAsync`.
|
||||
|
||||
### 5.4 Timing
|
||||
Most services call `RaiseAsync` **after** their own transaction commits (e.g. `GrnService.CreateAsync`
|
||||
commits the GRN, then raises `grn.created`). So a notification never exists for a rolled-back
|
||||
document — but if saving the notification throws, the request fails **after** the document was
|
||||
already saved (§7, G6).
|
||||
|
||||
### 5.5 Low-stock alerts (`stock.low`)
|
||||
```
|
||||
FifoCostingService.ConsumeAsync (every stock issue: sale, transfer, adjustment, return, production)
|
||||
└─► LowStockTracker.Track(itemId, warehouseId, qty) (per request, in memory)
|
||||
LowStockAlertFilter (MVC action filter, after the action succeeds)
|
||||
└─► LowStockAlertService: for each tracked item/warehouse with a reorder rule,
|
||||
if stock crossed from above the reorder point to at/below it in this request
|
||||
→ RaiseAsync("stock.low", "Low stock: <item>", "<sku> — <item> is down to <qty> …")
|
||||
```
|
||||
It fires only on the crossing (not on every later sale while still low). Unlike the other
|
||||
triggers this path is already **best-effort**: a failed action drains the tracker (nothing alerted),
|
||||
and any error in the check is caught and logged without failing the request.
|
||||
|
||||
---
|
||||
|
||||
## 6. Every trigger
|
||||
|
||||
| Module key | Registered | Raised by | When | Recipients |
|
||||
|---|---|---|---|---|
|
||||
| `sale.create` | ✅ | `SalesSlipService` | Sales slip created | All active users |
|
||||
| `sale.posted` | ✅ | `SalesSlipService` | Sales slip posted | All |
|
||||
| `grn.created` | ✅ | `GrnService` | GRN created | All |
|
||||
| `grn.confirmed` | ✅ | `GrnService` | GRN confirmed | All |
|
||||
| `purchase.order.created` | ✅ | `PurchaseOrderService` | PO created | All |
|
||||
| `purchase.order.pending_approval` | ❌ **not in catalog** | `PurchaseOrderService` | PO submitted for approval | **Never delivered** (G1) |
|
||||
| `purchase.order.approved` | ✅ | `PurchaseOrderService` | PO approved | All |
|
||||
| `purchase.order.rejected` | ❌ **not in catalog** | `PurchaseOrderService` | PO rejected | **Never delivered** (G1) |
|
||||
| `purchase.return.created` | ✅ | `PurchaseReturnService` | Purchase return created | All |
|
||||
| `stock.adjustment.created` | ✅ | `AdjustmentService` | Adjustment created / posted | All |
|
||||
| `stock.transfer.created` | ✅ | `TransferService` | Transfer created | All |
|
||||
| `stock.low` | ✅ | `LowStockAlertService` | Stock crosses a reorder point | All |
|
||||
| `approval.pending` | ✅ | `ApprovalService` | Stock/price adjustment awaits approval | The approvers |
|
||||
| `approval.approved` | ✅ | `AdjustmentService`, `PriceAdjustmentService` | Request fully approved | The requester |
|
||||
| `approval.rejected` | ✅ | `AdjustmentService`, `PriceAdjustmentService` | Request rejected | The requester |
|
||||
| `item.created` | ✅ | `ItemService` | Item created | All |
|
||||
| `item.updated` | ✅ | `ItemService` | Item updated | All |
|
||||
|
||||
**Not covered at all:** sales invoices (create / post / payment), expenses, bank deposits, purchase
|
||||
invoices & payments, requisitions, RFQs, stock counts, production runs, HRM (leave, payroll),
|
||||
day-end close, customer credit limit, overdue invoices.
|
||||
|
||||
---
|
||||
|
||||
## 7. Gaps & risks
|
||||
|
||||
| # | Gap | Impact | Suggested fix |
|
||||
|---|---|---|---|
|
||||
| **G1** | `purchase.order.pending_approval` and `purchase.order.rejected` are raised but **not registered** as modules. `ResolveEnabledChannelsAsync` finds no row → returns no channels → silently dropped. | Approvers are never told a PO is waiting; requesters never see a rejection. | Add two `[NotificationModule]` classes; ideally send pending-approval only to PO approvers (`NotifyUsersAsync`) like `approval.pending`. Add a startup check that every key passed to `RaiseAsync` exists (constants class instead of string literals). |
|
||||
| **G2** | **Email / SMS switches do nothing.** `DeliverAsync` only checks In-App; no sender is called. | Admins switch on email/SMS and nothing happens — misleading. | Reuse the owner-report messaging path (`IMessageSender` → AuthHex `/api/notify/email|sms`), using each user's email / mobile; or hide the two columns until implemented. |
|
||||
| **G3** | Seed creates the config **without module rows** ⇒ everything off on a new install. | New deployments look like notifications are broken. | Seed every catalog module with In-App on (and add rows for modules added later on startup). |
|
||||
| **G4** | `RaiseAsync` broadcasts to **every active user** — no role, permission or branch filter. | Cashiers get PO/GRN/item noise; users see other branches' documents; the bell fills up and gets ignored. | Per-module recipient rules: by permission code (e.g. `receiving` for GRN), by role, and by branch of the document. |
|
||||
| **G5** | Bell shows the latest 20 only; **no per-item read**, no link to the record (`RelatedEntityType/Id` are stored but unused), no "view all" page. | Users can't open the GRN/PO from the alert or work through older ones. | Map `RelatedEntityType` → route (as `SEARCH_TYPE_META` in `Header.tsx` already does for search); mark one as read on click; add `/dashboard/notifications` with paging. |
|
||||
| **G6** | Notification is saved after the business commit, outside it, and exceptions aren't caught (all triggers except `stock.low`, which is already best-effort). | A notification DB error returns 500 for a document that **was** saved → user retries → duplicate. | Wrap `RaiseAsync` calls in try/catch + log (notifications are best-effort), or write an outbox row inside the same transaction and deliver after. |
|
||||
| **G7** | No retention. One recipient row per active user per event. | 50 users × a few hundred events/day ≈ tens of thousands of rows a day. | Purge read notifications older than N days (background job, like `OwnerReportJob`); index `(UserId, IsRead)`. |
|
||||
| **G8** | No real-time push — the bell refreshes only on page load and when opened. | Unread badge is stale while a user stays on one page. | Poll every 60 s while the tab is visible, or SignalR later. |
|
||||
| **G9** | `ResolveEnabledChannelsAsync` queries the DB on every notification. | An extra query per event (minor). | Cache the module map, invalidate on `PUT /notification-config`. |
|
||||
| **G10** | Titles/messages are hard-coded English strings in each service. | No templates, no Sinhala/Tamil, inconsistent wording. | Optional: per-module templates in the catalog. |
|
||||
|
||||
---
|
||||
|
||||
## 8. Relation to the Day-End Owner Report
|
||||
|
||||
| | In-app notifications (this doc) | Day-End Owner Report |
|
||||
|---|---|---|
|
||||
| Trigger | Business events, as they happen | Once a day, at a configured time (`OwnerReportJob`) |
|
||||
| Recipients | ERP users (all, or targeted ids) | Owner contacts entered in Settings (not ERP users) |
|
||||
| Channels | In-App only (Email/SMS switches not implemented) | SMS + email through AuthHex |
|
||||
| Send log | none | `owner_report_dispatches` (status, attempts, error) |
|
||||
| Config | Settings → Notification Config | Settings → Configuration → Day-End Owner Report |
|
||||
|
||||
The owner report already solved what G2 needs — `IMessageSender` + AuthHex `/api/notify/*` with no
|
||||
credentials in ERPCore — so wiring Email/SMS for notifications can reuse it rather than add a second
|
||||
mail path. Note the providers themselves currently fail (Hostinger SMTP outbound disabled; SMS
|
||||
gateway out of credit/limit) — see the owner report send log.
|
||||
|
||||
---
|
||||
|
||||
## 9. Recommended order of work
|
||||
|
||||
1. **G1** register the two missing PO modules (+ key constants) — small, fixes silent loss.
|
||||
2. **G3** seed every module with In-App on.
|
||||
3. **G6** make notification delivery best-effort (try/catch + log).
|
||||
4. **G5** clickable notifications + mark one read + "view all" page.
|
||||
5. **G4** recipient rules by permission / role / branch.
|
||||
6. **G2** Email/SMS delivery via `IMessageSender` (after the SMTP/SMS provider accounts work).
|
||||
7. **G7 / G8 / G9** retention job, polling, config cache.
|
||||
Reference in New Issue
Block a user