feat(notifications): give in-app notifications somewhere to go - #29
Merged
Merged
Conversation
In-app notifications were text with no destination. A candidate told "your interview starts soon" had to work out for themselves that the join button lives on the home page, and a message longer than two lines was clamped with no way to read the rest — the rows were inert divs, so there was nothing to click and nothing to focus. Adds a pure resolver, src/lib/notificationLinks.ts, that turns a row into the places its recipient can actually go, plus a detail dialog for the full text. Two rules shape the links, and both are load-bearing: - Destinations are filtered through getRequiredRolesForPath, the same table middleware and RoleGuard read, rather than a second list that would drift from it. This is not hypothetical: `candidate` holds no permissions at all, so the obvious "Submit feedback" -> /dashboard/interviews link would have landed them on a denial panel. An unknown role — useUserRole reports undefined until the Convex query settles — withholds protected routes for the same reason. - Whatever survives that filter is promoted to primary, so a list that loses its lead action to a role check still reads as a call to action instead of a row of equally quiet links. getMyNotifications now joins each row to the slice of its interview the UI needs, chiefly streamCallId, without which "Join interview" cannot be built at all. Read once per interview rather than once per notification, since one scheduled round fans out a create, a reminder and a feedback nudge. streamCallId is not a capability: joining requires a Stream token minted for the caller's own streamUserId and Stream enforces call membership, so this hands out a destination, not access to it. buildMeetingHref duplicates resolveJoinTarget from #28 on purpose — that branch is unmerged, so importing it would not build here. Collapse the two once it lands. Rows are now buttons rather than divs, so the summary is keyboard reachable and announced as activatable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In-app notifications were text with no destination. A candidate told "your interview starts soon" had to work out for themselves that the join button lives on the home page, and a message longer than two lines was clamped with no way to read the rest — the rows were inert
divs, so there was nothing to click and nothing to focus.What this adds
src/lib/notificationLinks.ts— a pure, dependency-light resolver turning a notification row into the destinations its recipient can actually reach.NotificationDetailDialog— the full message, untruncated and with its line breaks intact, plus parsed metadata (reschedule times, cancellation reason, feedback due date) and every available action.Two rules shape the links
Destinations are filtered through
getRequiredRolesForPath— the same table middleware andRoleGuardread — rather than a second hand-maintained list that would drift from it. This is not hypothetical:candidateholds no permissions at all, so the obvious "Submit feedback" →/dashboard/interviewslink would have landed them on a denial panel. An unknown role (useUserRolereportsundefineduntil the Convex query settles) withholds protected routes for the same reason, so no button flashes and then turns into a denial page.Whatever survives the filter is promoted to primary, so a list that loses its lead action to a role check still reads as a call to action rather than a row of equally quiet links.
Backend
getMyNotificationsnow joins each row to the slice of its interview the UI needs — chieflystreamCallId, without which "Join interview" cannot be built at all. Read once per interview rather than once per notification, since one scheduled round fans out a create, a reminder and a feedback nudge.On access:
streamCallIdis not a capability. Joining requires a Stream token minted for the caller's ownstreamUserId, and Stream enforces call membership — so this hands out a destination, not access to it. The recipient was a participant when the notification was written, and the title and time are already in the message text on the row.Known duplication
buildMeetingHrefduplicatesresolveJoinTargetfrom #28 deliberately: that branch is unmerged, so importing it would not build here. There is a comment saying to collapse the two once #28 lands. Whichever of the two merges second should do that.Testing
26 new cases in
src/lib/notificationLinks.test.tscovering the join window (early/late/ended/cancelled), role filtering per route, the unknown-role case, malformed and non-object metadata, and path-escape encoding of call ids.npm run ci:validateexits 0 locally — typecheck clean, 291/291 tests pass, build compiles.