fix: improve Slack unavailable messaging for end users#94
Conversation
…h user-friendly text PR #91 made the Slack section always visible, but when the server reports available=false, end users saw "Contact your administrator to set up the Slack app" — a confusing admin-targeted message they can't act on. Root cause: the server lumped "tables not ready" (transient) and "OAuth not configured" (needs admin) into one generic unavailableReason. Now the API returns specific reasons ("tables-not-ready" vs "oauth-not-configured") so the client can show appropriate messages: - tables-not-ready: "temporarily unavailable, try again in a moment" - oauth-not-configured: "not available for this service" (neutral, no admin jargon) https://claude.ai/code/session_01RSvtzrtyhYtfcAb9Mm4BR4
Export SlackStatus from use-slack.ts and import it in slack-display-state.ts instead of maintaining two separate copies. https://claude.ai/code/session_01RSvtzrtyhYtfcAb9Mm4BR4
…ranching - Derive SlackStatus type from the Zod schema in shared/routes.ts via z.infer so adding a new unavailableReason only requires a single change - Replace 5-deep ternary chain with IIFE + early returns, calling getSlackDisplayState once instead of 4 times per render https://claude.ai/code/session_01RSvtzrtyhYtfcAb9Mm4BR4
- Zod schema validation: accepts valid reasons, rejects the old "unavailable" value, rejects missing required fields - Server endpoint: tables-not-ready when OAuth IS configured (isolated table failure, not just the combined-missing case) - Display state: available=false takes precedence over connected=true https://claude.ai/code/session_01RSvtzrtyhYtfcAb9Mm4BR4
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe PR refactors Slack integration handling to distinguish between missing database tables ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary
PR #91 made the Slack notification card always visible, but when the server reports
available: false, end users now see "Contact your administrator to set up the Slack app" — a confusing admin-targeted message they cannot act on. This PR replaces that generic message with specific, user-friendly text by splitting the server's singleunavailableReasoninto two distinct reasons.Changes
API contract (
shared/routes.ts)unavailableReason: "unavailable"enum with"tables-not-ready" | "oauth-not-configured"Server (
server/routes.ts)!tablesReady || !oauthReadycheck into two sequential early returns, each with its own reasonClient display logic (
slack-display-state.ts,use-slack.ts)"not-ready"display state for transient table failuresSlackStatustype from Zod schema viaz.infer<...>(single source of truth)SlackStatusinterface (was defined in two files)UI (
NotificationChannelsPanel.tsx)tables-not-ready→ "Slack notifications are temporarily unavailable. Please try again in a moment."oauth-not-configured→ "Slack notifications are not available for this service."getSlackDisplayStateonce instead of 4×)Tests (
slack-display-state.test.ts,routes.notificationChannels.test.ts,routes.test.ts)available: falseoverconnected: trueHow to test
SLACK_CLIENT_ID/SLACK_CLIENT_SECRETenv vars setSLACK_CLIENT_IDandSLACK_CLIENT_SECRETto valid values, restartnpm run test— all 1161 tests passnpm run build— production build succeedshttps://claude.ai/code/session_01RSvtzrtyhYtfcAb9Mm4BR4
Summary by CodeRabbit
New Features
UI Improvements