[#534] Add judging room communications and announcements - #535
[#534] Add judging room communications and announcements#535DVidal1205 wants to merge 9 commits into
Conversation
DVidal1205
added
Feature
DVidal1205
added
Feature
|
Warning Review limit reachedNext included review available in 36 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: QUIET Plan: Team Run ID: 📒 Files selected for processing (1)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: QUIET Plan: Team Run ID: 📒 Files selected for processing (22)
💤 Files with no reviewable changes (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughAdds optional Discord communications for judging rooms, including persistent room threads, judge and guest notices, QR delivery, revocation notices, and announcements. Stores Discord configuration, room threads, and announcement history. Adds API procedures, delivery status reporting, validation, audit policies, and migration changes. Adds Command Center controls and Blade announcement banners or urgent dialogs with polling and dismissal persistence. Updates judge display-name resolution and adds tests for delivery, permissions, safety, persistence, and UI behavior. Merge Risk: ⚪ Minimal · up to Current announcement validation and Send QR feedback handle the previously identified cases, with no actionable merge-blocking risk remaining. 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 5.63% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 71 functions across 28 files. (4 skipped: 4 unsupported.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
|
@coderabbitai review |
|
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
8cac730 to
0b562cc
Compare
|
Live Discord acceptance is complete with T.K in Dev@KnightHacks The run covered room-thread provisioning, I also changed member-facing Discord copy to use the linked Blade member profile name. The mention still targets the right Discord account, but alerts now say Current QR delivery
Guest arrival and revocation
Member profile name on room entry
The branch is rebased onto |
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>
Announcement and identity acceptance passThe latest pass adds Blade and Discord announcements, preserves the full accepted message during Discord escaping and batching, and applies the Member full-name rule throughout judging. Officer view
The room card now exposes both urgency and audience without opening the editor. The live roster resolves the current Member profile name. Authenticated judge view
The judging shell uses Guest view
The unskippable identity field now says Live Discord delivery
Both messages came from the local Blade server through T.K in Dev@KnightHacks. The complete screenshot set and flow notes are in the updated PR body. |
|
@coderabbitai review |
Co-authored-by: Codex <codex@openai.com>
|
CI identified that the new announcement table was missing from the explicit development-backup policy. Commit |
DVidal1205
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (4)
packages/db/drizzle/0049_real_redwing.sql-12-12 (1)
12-12: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winReject all-whitespace announcements.
btrim()removes spaces by default, so tab-only and newline-only messages pass this check. Usemessage ~ '[^[:space:]]'in the migration andpackages/db/src/schemas/knight-hacks.ts. Add regression cases for tab-only and newline-only messages.Source: MCP tools
packages/api/src/routers/judging.ts-681-690 (1)
681-690: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA Discord outage produces a wrong validation message.
validateJudgingDiscordChannelreturnsfalsefor any thrown error, including a 429, a 5xx, or a network failure. The officer then reads "Choose a text channel from the configured Knight Hacks server." while the selected channel is valid. Return a distinct outcome for lookup failure and map it to a retry message.apps/blade/src/app/_components/judging/judging-control-panel.tsx-1039-1049 (1)
1039-1049: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a fallback branch for other
discordDeliveryvalues.The handler toasts only for
"delivered"and"failed". Any other status, for example"not_configured"after the configuration changes between render and click, produces no feedback. The officer sees no result.🛠️ Proposed fix
if (result.discordDelivery === "delivered") { toast.success( "QR sent to current room judges.", ); } else if ( result.discordDelivery === "failed" ) { toast.error( "The QR is still active, but Discord delivery failed.", ); + } else { + toast.error( + "Connect a Discord channel to send this QR.", + ); }apps/blade/src/app/_components/judging/judging-control-panel.tsx-777-778 (1)
777-778: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winShow the configured channel when the channel list fails to load.
items={channels.data ?? []}leaves the combobox empty on a query error.ResponsiveComboBoxresolves its label by findingvalueinsideitems, so the trigger falls back to "Choose a text channel" while the badge still reports "Connected". The officer then cannot see which channel is configured. Add the saved channel id as a fallback item.🛠️ Proposed fix
- items={channels.data ?? []} + items={ + channels.data ?? + (commsChannelId + ? [{ id: commsChannelId, name: commsChannelId }] + : []) + }
🧹 Nitpick comments (5)
packages/api/src/utils/judging/discord-comms.ts (1)
676-676: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAccept a gateway parameter for consistency and testability.
Every other exported delivery function takes
gateway: JudgingDiscordGateway = liveJudgingDiscordGateway.provisionJudgingRoomThreadsdoes not, sosetCommsChannelandprovisionRoomThreadsinpackages/api/src/routers/judging.tscannot be tested without live Discord calls.♻️ Proposed signature change
-export async function provisionJudgingRoomThreads(hackathonId: string) { +export async function provisionJudgingRoomThreads( + hackathonId: string, + gateway: JudgingDiscordGateway = liveJudgingDiscordGateway, +) { @@ - const threadId = await ensureJudgingRoomThread(room.id); + const threadId = await ensureJudgingRoomThread(room.id, gateway);packages/api/src/tests/integration/judging-access.test.ts (1)
385-390: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the
"not_configured"assertion independent of test order.This expectation holds only while no earlier test has set
judgingCommsChannelIdforHACKATHON. The tests at lines 602 and 651 configure that channel and never clear it. Reordering or running thisitalone after those tests changes the result. Delete the row forHACKATHONinHackathonJudgingConfiguration, or setjudgingCommsChannelIdtonull, immediately before line 385.As per path instructions for
**/*.test.*: "Check for meaningful descriptions, proper assertions, and no skipped tests without explanation."Source: Path instructions
packages/api/src/routers/judging.ts (1)
1634-1640: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPass
txinstead of opening a second connection inside the transaction.
resolveCurrentJudgeDisplayNamesuses the module-leveldb, so this call takes a second pool connection while the surrounding transaction holdsJudgingRoomPresenceFOR UPDATE. Add an executor parameter toresolveCurrentJudgeDisplayNamesinpackages/api/src/utils/member/display-name.tsand passtxhere.apps/blade/src/tests/projects/judging-announcements.test.tsx (1)
57-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for dismissal persistence across mounts.
beforeEachclearssessionStorage, but no test proves thatreadDismissedsuppresses an announcement after a remount. That persistence is the purpose ofDISMISSED_KEY. Add a case that dismisses an announcement, unmounts, renders again with the sameinitialAnnouncements, and asserts the message stays hidden.As per path instructions: "Test files. Check for meaningful descriptions, proper assertions, and no skipped tests without explanation."
Source: Path instructions
packages/api/src/tests/judging/discord-comms.test.ts (1)
58-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert nonce uniqueness.
The test checks only the charset and the length. A constant return value would pass, yet Discord uses the nonce for deduplication, so a repeated nonce drops messages. Add an assertion that separate calls differ.
🛠️ Proposed fix
it("uses a Discord-safe nonce", () => { const nonce = judgingDiscordNonce(); expect(nonce).toMatch(/^[A-Za-z0-9_-]+$/); expect(nonce.length).toBeLessThanOrEqual(25); + expect(new Set(Array.from({ length: 50 }, judgingDiscordNonce)).size).toBe( + 50, + ); });As per path instructions: "Test files. Check for meaningful descriptions, proper assertions, and no skipped tests without explanation."
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: QUIET
Plan: Team
Run ID: 4abc4a72-10b0-444c-ab85-061b377f16dc
⛔ Files ignored due to path filters (1)
packages/api/src/tests/root/__snapshots__/api-surface.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (35)
.forge/features/judging-discord-comms/spec.md.forge/features/judging-discord-comms/srd.md.forge/features/judging-discord-comms/status.md.forge/features/judging-discord-comms/test-cases.mdapps/blade/src/app/_components/judging/guest-name-gate.tsxapps/blade/src/app/_components/judging/judging-announcements.tsxapps/blade/src/app/_components/judging/judging-control-panel.tsxapps/blade/src/app/_components/projects/judge-project-workspace.tsxapps/blade/src/app/_components/shared/authenticated-shell.tsxapps/blade/src/app/judge/layout.tsxapps/blade/src/tests/admin/authenticated-shell.test.tsxapps/blade/src/tests/projects/guest-name-gate.test.tsxapps/blade/src/tests/projects/judging-announcements.test.tsxapps/blade/src/tests/projects/project-judge-privacy.test.tsxdocs/DATABASE-USAGE.mdpackages/api/src/judging-access.server.tspackages/api/src/routers/judging-scores.tspackages/api/src/routers/judging.tspackages/api/src/tests/integration/judging-access.test.tspackages/api/src/tests/judging/discord-comms.test.tspackages/api/src/utils/audit/coverage.tspackages/api/src/utils/judging/discord-comms.tspackages/api/src/utils/judging/principal.tspackages/api/src/utils/member/display-name.tspackages/db/drizzle/0048_little_tarot.sqlpackages/db/drizzle/0049_real_redwing.sqlpackages/db/drizzle/meta/0048_snapshot.jsonpackages/db/drizzle/meta/0049_snapshot.jsonpackages/db/drizzle/meta/_journal.jsonpackages/db/src/schemas/knight-hacks.tspackages/db/src/tests/migration-lineage.test.tspackages/ui/src/dialog.tsxpackages/validators/src/audit.tspackages/validators/src/judging.tspackages/validators/src/tests/judging.test.ts
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.
Co-authored-by: Codex <codex@openai.com>
|
Co-authored-by: Codex <codex@openai.com>
|
Review fixes are pushed in af7dd86. This pass separates Discord delivery from database transactions, preserves room threads during transient Discord failures, serializes room and announcement operations, distinguishes skipped deliveries, hardens announcement dismissal storage, expands migration and boundary tests, and labels guest identity as Full Name. The PR body now includes the new guest dialog capture and updated verification counts. All six inline review threads are resolved. |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
Why
Judges need a quiet way to coordinate while hackers are presenting. Walkie-talkies interrupt pitches, while authenticated organizers already have Discord on their phones. Guest QR activity and operational announcements also need to reach the right room without exposing unrelated judging data.
What
Closes: #534
Open threadaction.Send QRaction. Every send resolves the room's authenticated judges at delivery time.0048_little_tarotand0049_real_redwing, audit coverage, principal-scoped reads, and the approved Forge feature bundle.Flow
Interface and live delivery evidence
Screenshots are hosted in the PR evidence gist. None are committed to Forge.
Communications setup and room controls
The channel uses the existing searchable combobox and environment-aware guild.
Send QRkeeps the current link, while rotation revokes it and generates a replacement.Authenticated judge room entry and human names
The judging shell, room roster, score feedback, evaluation history, and organizer messages resolve the current Member full name. The Discord account remains the mention target, not the visible identity label.
Guest identity gate
Scanning the QR alone is quiet. The arrival message is sent only after the guest submits their full name.
Global announcement controls and standard notice
Standard notices do not expire. Dismissal is scoped to the announcement ID, so a replacement in the same scope appears again.
Urgent announcement
Escape and outside clicks do not close an urgent notice. The judge must select
I understand.Room-scoped and guest announcement views
A guest can receive an included global notice and the included notice for their signed QR room. Room announcements never cross room boundaries.
Live T.K delivery in Dev@KnightHacks
These messages were sent by the local Blade dev server through T.K to the dynamic development guild and
#bot-testing.Test Plan
pnpm verify:precommitpassed: React analysis, formatting, lint, and 33 type-check tasks.pnpm --filter=@forge/validators test -- judging auditpassed: 17 tests.pnpm --filter=@forge/api test -- judgingpassed: 25 tests.pnpm --filter=@forge/db test -- migrationpassed: 65 tests.pnpm --filter=@forge/db test -- judging-schemapassed: 11 tests.pnpm --filter=@forge/api testpassed: 112 files and 797 tests.pnpm --filter=@forge/db testpassed: 148 tests, including the development-backup policy gate.pnpm --filter=@forge/blade test -- authenticated-shell project-judge-privacy guest-name-gate judging-announcementspassed: 29 tests.pnpm --filter=@forge/blade buildpassed: 58 routes generated, including server-rendered judging routes.git diff --checkpassed.origin/mainatbd97fccb, which includes PR [#1] Fix forms callback delivery and action feedback #533.Checklist
pnpm db:generateand committed0048_little_tarot.sql,0049_real_redwing.sql, their snapshots, and the journal entries.