fix: Bunkx dashboard integration with corrected error handling and session management#18
Closed
vishnunathasuresh wants to merge 2 commits into
Closed
Conversation
… accidentally added to the bunkialo-landing page
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThe PR removes the in-memory session store, associated API route handlers for session creation and consumption, the bunkialo page UI, and payload validation module, while adding a new end-to-end test script that validates the full session handoff flow. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (3 warnings)
✏️ 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 |
|
Preview |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/scripts/test-bunkx-sid-e2e.mjs (2)
218-224: Minor duplication withsafeJsonParse.This inline try-catch duplicates the
safeJsonParsehelper defined earlier. Consider reusing it for consistency.♻️ Suggested refactor
- const bodyText = await response.text(); - let bodyJson = null; - try { - bodyJson = JSON.parse(bodyText); - } catch { - bodyJson = null; - } + const clonedResponse = response.clone(); + const bodyText = await response.text(); + const bodyJson = await safeJsonParse(clonedResponse);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/test-bunkx-sid-e2e.mjs` around lines 218 - 224, The code parses response.text() into bodyJson using an inline try-catch that duplicates the existing helper; replace the manual parse with the safeJsonParse helper by calling safeJsonParse(bodyText) and assigning its return to bodyJson (instead of the try/catch block) so the logic consistently uses the safeJsonParse function; update references to bodyText/bodyJson accordingly where needed.
262-320: Test lacks automated pass/fail assertions.The test collects data and logs it but doesn't assert expected outcomes. For example, after checking
page.hasReceivedText(line 309) orconsume.firstStatus(line 315), the test should fail if the results don't match expectations. Currently, a human must inspect the output to determine success.Consider adding assertions to make this a proper automated test:
♻️ Suggested assertion additions
const page = await checkLaunchPage(sid); console.log(`Page status: ${page.status}`); console.log(`Page has 'Received X records': ${page.hasReceivedText}`); console.log( `Page has 'Session unavailable': ${page.hasSessionUnavailableText}`, ); + + if (page.status !== 200) { + console.error(`Expected page status 200, got ${page.status}`); + process.exit(1); + } const consume = await checkConsumeApi(sid); console.log(`Consume first status: ${consume.firstStatus}`); console.log(`Consume second status: ${consume.secondStatus}`); console.log(`Consume first body head: ${consume.firstBodyHead}`); console.log(`Consume second body head: ${consume.secondBodyHead}`); + + // Verify single-use session behavior + if (consume.firstStatus !== 200) { + console.error(`Expected first consume status 200, got ${consume.firstStatus}`); + process.exit(1); + } console.log("=== E2E TEST COMPLETE ==="); + console.log("=== ALL ASSERTIONS PASSED ===");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/scripts/test-bunkx-sid-e2e.mjs` around lines 262 - 320, The main function currently only logs results; add automated assertions after key checks (after create.bodyJson?.sid, after checkLaunchPage result properties like page.hasReceivedText and page.hasSessionUnavailableText, and after checkConsumeApi results like consume.firstStatus/secondStatus and any expected body content) so the script exits non-zero on failure; specifically, in main validate that sid exists (already checked) but also assert page.hasReceivedText is true (or false if appropriate) and page.hasSessionUnavailableText is false, and assert consume.firstStatus/secondStatus equal expected HTTP codes and that consume.firstBodyHead/secondBodyHead contain expected substrings, calling process.exit(1) on any failed assertion and logging a clear error message referencing the symbols createBunkxSession, checkLaunchPage, checkConsumeApi, page.hasReceivedText, page.hasSessionUnavailableText, consume.firstStatus, and consume.secondStatus.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/scripts/test-bunkx-sid-e2e.mjs`:
- Around line 218-224: The code parses response.text() into bodyJson using an
inline try-catch that duplicates the existing helper; replace the manual parse
with the safeJsonParse helper by calling safeJsonParse(bodyText) and assigning
its return to bodyJson (instead of the try/catch block) so the logic
consistently uses the safeJsonParse function; update references to
bodyText/bodyJson accordingly where needed.
- Around line 262-320: The main function currently only logs results; add
automated assertions after key checks (after create.bodyJson?.sid, after
checkLaunchPage result properties like page.hasReceivedText and
page.hasSessionUnavailableText, and after checkConsumeApi results like
consume.firstStatus/secondStatus and any expected body content) so the script
exits non-zero on failure; specifically, in main validate that sid exists
(already checked) but also assert page.hasReceivedText is true (or false if
appropriate) and page.hasSessionUnavailableText is false, and assert
consume.firstStatus/secondStatus equal expected HTTP codes and that
consume.firstBodyHead/secondBodyHead contain expected substrings, calling
process.exit(1) on any failed assertion and logging a clear error message
referencing the symbols createBunkxSession, checkLaunchPage, checkConsumeApi,
page.hasReceivedText, page.hasSessionUnavailableText, consume.firstStatus, and
consume.secondStatus.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bed255d-e182-405d-be4b-c217a6ceb478
📒 Files selected for processing (6)
bunkialo-landing/src/app/api/bunkx/session/[sid]/route.tsbunkialo-landing/src/app/api/bunkx/session/route.tsbunkialo-landing/src/app/bunkialo/page.tsxbunkialo-landing/src/lib/bunkx-payload.tsbunkialo-landing/src/lib/bunkx-session-store.tssrc/scripts/test-bunkx-sid-e2e.mjs
💤 Files with no reviewable changes (5)
- bunkialo-landing/src/lib/bunkx-payload.ts
- bunkialo-landing/src/app/api/bunkx/session/route.ts
- bunkialo-landing/src/app/api/bunkx/session/[sid]/route.ts
- bunkialo-landing/src/app/bunkialo/page.tsx
- bunkialo-landing/src/lib/bunkx-session-store.ts
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
1 participant
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.
Changes
This PR introduces the Bunkx integration for the dashboard, allowing users to launch Bunkx with their attendance data via a secure session handoff mechanism.
Key Features
Files Changed
src/app/(tabs)/index.tsx- Dashboard UI with Bunkx launch buttonsrc/services/bunkx-session.ts- Session management and validationsrc/utils/bunkx-payload.ts- Attendance payload serializationsrc/types/attendance.ts- Updated type definitionsNote
Testing
Run the e2e test to verify the handoff: