feat(support): dashboard backend (statistics, escalation alerts, clerk mapping)#3942
Conversation
joshuakrueger-dfx
requested review from
TaprootFreak and
davidleomay
as code owners
joshuakrueger-dfx
force-pushed
the
feat/support-dashboard-backend
branch
4 times, most recently
from
f3c59b5 to
043dce7
Compare
…k mapping)
- add GET support/issue/statistics?days=N with role-scoped, period-based aggregates (total, avg messages/ticket, tickets/day, day/month trend, avg resolution time per type)
- notify a Telegram group when a ticket escalates (customer waiting beyond the SLA) and for each new limit increase request; auto-bind + per-cycle de-duplication; chat id + state via SettingService; token from SUPPORT_TELEGRAM_BOT_TOKEN
- resolve the logged-in agent's clerk via GET support/issue/clerk, backed by the registered supportClerkAccounts setting ([{ account, name }])
joshuakrueger-dfx
force-pushed
the
feat/support-dashboard-backend
branch
from
043dce7 to
a768c04
Compare
TaprootFreak
left a comment
TaprootFreak
left a comment
There was a problem hiding this comment.
Review notes on the support dashboard backend — solid overall (clean Setting/entity patterns, guarded token usage, role-scoped endpoints). A few items inline, mainly the OPEN_STATES interaction with the new states from #3950, the monthly-trend bucket math, and the missing .env.example entry.
| const CHAT_ID_KEY = 'supportEscalationChatId'; | ||
| const NOTIFIED_KEY = 'supportEscalationNotified'; | ||
| const LIMIT_NOTIFIED_KEY = 'supportLimitRequestNotified'; | ||
| const OPEN_STATES = [SupportIssueInternalState.CREATED, SupportIssueInternalState.PENDING]; |
There was a problem hiding this comment.
OPEN_STATES excludes the new ticket states introduced by #3950
The escalation cron only scans find({ where: { state: In(OPEN_STATES) } }) with OPEN_STATES = [CREATED, PENDING], and notifyNewLimitRequests reuses that same issues set. Once #3950 lets staff set InProgress / InClarification, a customer who then waits beyond the SLA is never escalated to Telegram, because the ticket is no longer in the queried set — and closeIssue/customer auto-reopen only resets Completed/OnHold/Canceled to Pending, so those states never return to an open state on their own. The services overview hardcodes the identical pair, so the ticket also vanishes from the dashboard. Suggest "all non-terminal states" and keeping both repos in sync.
| if (department) trendQb.andWhere('issue.department = :department', { department }); | ||
| const rows = await trendQb.getRawMany<{ y: number; m: number; count: string }>(); | ||
| const map = new Map(rows.map((r) => [`${r.y}-${pad(r.m)}`, +r.count])); | ||
| const months = Math.max(1, Math.round(days / 30)); |
There was a problem hiding this comment.
Monthly trend buckets cover fewer months than the period, so trend never sums to total
months = Math.max(1, Math.round(days / 30)) emits that many whole calendar months ending in the current month, while total counts every ticket with created >= from (from = now - days*24h). The oldest partial month(s) of the window therefore get no bucket: their tickets are in total but dropped from trend. Worst near the daily cutoff — e.g. days=32 → round(32/30)=1 bucket although the window spans 2–3 months; days=365 → 12 buckets although the window touches 13 months. The client mirror in services has the same off-by-one. Consider deriving the bucket list from from/now rather than round(days/30).
| }, | ||
| issueOnHoldExpiry: 14, //days | ||
| escalation: { | ||
| telegramBotToken: process.env.SUPPORT_TELEGRAM_BOT_TOKEN, |
There was a problem hiding this comment.
New env var not added to .env.example
SUPPORT_TELEGRAM_BOT_TOKEN is introduced here but .env.example (which lists the other SUPPORT_* / token vars) is not updated in this PR. Per CONTRIBUTING → PR Completeness → Environment/Infrastructure updates, new env vars must ship in the same PR ("Missing any of these = changes requested"). Add a SUPPORT_TELEGRAM_BOT_TOKEN= entry near the other support vars.
| // Reads recent updates and returns the group/supergroup chats the bot has seen. | ||
| async getGroupChats(): Promise<TelegramChat[]> { | ||
| if (!this.token) return []; | ||
| const res = await this.http.get<{ result: any[] }>(this.apiUrl('getUpdates')); |
There was a problem hiding this comment.
any violates the no-any rule (CONTRIBUTING → Type safety). The Telegram getUpdates payload is typed { result: any[] }, and the update loop var is implicitly any. A small interface for the fields actually read (message.chat, my_chat_member.chat, channel_post.chat with id / type / title) keeps it type-safe.
| @GetJwt() jwt: JwtPayload, | ||
| @Query('days') days?: string, | ||
| ): Promise<SupportIssueStatisticsDto> { | ||
| return this.supportIssueService.getSupportIssueStatistics(jwt.role, days ? +days : undefined); |
There was a problem hiding this comment.
Non-numeric ?days bypasses the clamp and yields NaN
days ? +days : undefined forwards NaN for a truthy non-numeric value (e.g. ?days=abc). The service default only applies on undefined, and the intended clamp Math.min(Math.max(Math.round(periodDays), 1), 366) propagates NaN (so it doesn't sanitize) → from becomes Invalid Date and perDay/periodDays come back NaN. Guard with Number.isFinite (or coerce to the default) before clamping. Low severity since the endpoint is role-gated.
| // Convenience: pick the most recently seen group and store it as the escalation target. | ||
| async bindGroupChat(): Promise<TelegramChat | undefined> { | ||
| const chats = await this.getGroupChats(); | ||
| const target = chats[chats.length - 1]; |
There was a problem hiding this comment.
Auto-bind picks an arbitrary group, risking customer PII to an unintended chat
bindGroupChat stores chats[chats.length - 1] — the last group seen via getUpdates. If the bot is in (or briefly added to) more than one group, escalation messages — which include customer names and issue types — can be sent to the wrong chat. Consider binding only on an explicit "bot added to group" event for the intended group, or confirming the bound chat out-of-band before sending.
…nt escalation env var - replace the any-typed getUpdates payload with TelegramUpdate/TelegramApiChat interfaces - coerce a non-finite ?days to the default so the stats clamp cannot produce an Invalid Date - add SUPPORT_TELEGRAM_BOT_TOKEN to .env.example
…issue tests - include InProgress/InClarification in escalation OPEN_STATES so actively-handled tickets keep escalating - span every calendar month the stats window touches so the monthly trend sums to total (was round(days/30)) - align getSupportIssueStatistics department scoping with the getVisibleDepartments helper its siblings use (RoleDepartmentMap path was dropped on develop) - replace the tautological sort-injection test with a real DTO @IsEnum rejection test - cover the anonymous UID close path and the CANCELED idempotency case - document the SupportIssueListOrderBy column-name enum values
…stics tests - replace MSSQL YEAR()/MONTH() with EXTRACT(... FROM ...) so the monthly statistics query runs on Postgres (the default /statistics path errored otherwise) - include the oldest partial day in the daily trend so it sums to total, mirroring the monthly span - add getSupportIssueStatistics coverage (daily bucket span, monthly granularity, non-numeric days fallback) - describe the list DTO updated field by what it is, not a stats implementation detail
…aming and contracts
- group the statistics trend by CAST(created AS DATE) and roll up to months in JS, replacing the Postgres-incompatible YEAR()/MONTH() (and the alias-guard-failing EXTRACT) so the default /statistics path runs
- rename SupportClerkDto -> SupportClerkAccountDto, SupportIssueStatBucketDto -> SupportIssueStatisticsBucketDto, the German label maps to ...Map, and getMyClerk -> getSupportIssueClerk for consistency
- wrap the telegram-bind response as { chat } to match the sibling endpoints
… real data - derive trend map keys from the same local date parts as the bucket loop (and dedupe the shared day-grouped query), so daily and monthly buckets align in any timezone - replace the all-zero statistics assertion with a non-empty case that pins counts to their calendar-day buckets and a non-trivial sum-to-total
…r resolution and scoping
- anchor the daily trend's first bucket to from's calendar day (not a fixed count) so it stays correct across DST/timezone shifts and always sums to total
- use getObj's default-value argument instead of ?? {} / ?? [] in the escalation cron, matching the codebase idiom
- add statistics tests for resolution-time-per-type averaging and department scoping
…overage - rename getSupportClerkForAccount -> getSupportIssueClerkForAccount to match the getSupportIssue* method family - extract a ticketLink helper and read the bound chat id via getBoundChatId in the escalation cron - drop the redundant updated Swagger description so it matches the sibling created field - add a monthly trend test asserting count placement and sum-to-total (mirroring the daily case)
|
@TaprootFreak Danke fürs Review — alle sechs Punkte sind adressiert:
Lint + Specs lokal grün (29 Tests in den beiden betroffenen Spec-Files), CI läuft auf dem neuen Commit. |
Auto-bind only when exactly one group has explicitly invited the bot (my_chat_member -> joined status), instead of picking the last group the bot happened to see. Add an explicit chatId path so an operator can bind a reviewed group out-of-band. Prevents escalation messages (which carry customer names/issue types) from being sent to an unintended chat.
joshuakrueger-dfx
force-pushed
the
feat/support-dashboard-backend
branch
from
8a5b5db to
ec6ee84
Compare
The daily/monthly bucket keys align because the app and DB both run UTC; soften the comments that overclaimed correctness in any timezone.
|
Please stop force-pushing this branch while it's being co-developed This branch was force-pushed again today (~19:12 CEST), rewriting an already-pushed commit (the escalation-bind change) to fix its formatting. Since a couple of us have been pushing here in parallel, this is a real process problem, not a nitpick:
Per our contributing guidelines, a force-push is only acceptable for a genuine rebase of an unmerged feature branch onto its base — never to amend an already-pushed commit. For a follow-up fix (e.g. a formatting tweak), push a new commit; everything is squashed at merge anyway, so a tidy intermediate history is no reason to rewrite a shared branch. Going forward, please do not force-push this branch — no Thanks 🙏 |
Add checkEscalations cases: an overdue customer-waiting ticket escalates exactly once and records the waiting cycle; a second run on the same cycle is de-duplicated (no send) while keeping the marker.
- add a checkEscalations case asserting no escalation when support (not the customer) wrote the last message - assert the daily trend spans from's calendar day through today instead of a fixed length, so it stays correct across timezone/DST shifts
|
You're completely right, and I apologize — the Understood and adopted going forward on this branch:
I'll leave the current head ( |
…ten injection test - order the support-escalation src imports path-alphabetically (models before services), matching sibling services - rename SupportIssueResolutionDto -> SupportIssueStatisticsResolutionDto so both nested statistics shapes share the parent's Statistics infix - assert the orderBy injection test fails specifically on the @IsEnum constraint
…at id - the cron no longer auto-binds; escalations run only against a chat an operator deliberately bound (closes a PII-leak race where an outsider could be the sole bot-inviter during the deploy-before-bind window) - on a supergroup migration, rebind to the new chat id and resend; on a permanently unreachable chat (group deleted / bot removed), drop the binding so an operator rebinds, instead of erroring every 5-minute run forever - cover both self-healing paths plus the unbound no-op with unit tests
2 participants
Support dashboard — backend
Adds the backend for the new support dashboard overview: ticket statistics, Telegram escalation alerts, and account→clerk resolution.
What's included
GET support/issue/statistics?days=N: role-scoped, period-based aggregates (total, avg messages/ticket, tickets/day, and a day- or month-bucketed trend depending on the period). MSSQL-side aggregation, no row dump.Config.support.escalation.slaHours= 24h). Auto-binds to the group it was invited to, de-duplicates per waiting cycle, and posts a clickable link to the ticket. Bot token fromSUPPORT_TELEGRAM_BOT_TOKEN; chat id + notified state viaSettingService(no schema change).GET support/issue/clerkreturns the clerk mapped to the logged-in account via the new registeredsupportClerkAccountssetting ([{ account, name }]). Replaces client-side identity.Setup (settings / ops)
SUPPORT_TELEGRAM_BOT_TOKEN.supportClerks(names for the assign dropdown):["Jana","Vesa","Cyrill","Josh"]supportClerkAccounts(account→name for "my tickets"):[{ "account": <JanaId>, "name": "Jana" }, { "account": <VesaId>, "name": "Vesa" }, { "account": <CyrillId>, "name": "Cyrill" }, { "account": <JoshId>, "name": "Josh" }]Release Checklist
Pre-Release
Post-Release
POST support/issue/escalation/telegram-test)