Skip to content

Conversation

@sonyccd
Copy link
Owner

@sonyccd sonyccd commented Jan 14, 2026

Summary

Fixes a bug where the community promo toast was incorrectly showing for users who already have a forum_username.

Root Cause:

  1. Component mounts with profile still loading (forumUsername is undefined)
  2. undefined is falsy, so the guard if (forumUsername) passed through
  3. Toast was scheduled before profile actually loaded with the real value

Fix:
Explicitly check for undefined and wait until profile loads before deciding whether to show the toast:

  • undefined = profile still loading, don't proceed yet
  • null = profile loaded, user has no forum username, show toast
  • string = profile loaded, user has forum username, don't show toast

Test plan

  • Added 3 new tests for profile loading race condition scenarios
  • Verify users with forum_username don't see the toast
  • Verify users without forum_username still see the toast after 3 days

🤖 Generated with Claude Code

The toast was incorrectly showing for users with forum_username because:
1. Component mounts with profile still loading (undefined)
2. undefined is falsy, so the guard passed through
3. Toast scheduled before profile actually loaded

Fix: Explicitly check for undefined and wait until profile loads.
Added tests for profile loading race condition scenarios.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vercel
Copy link
Contributor

vercel bot commented Jan 14, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
openhamprep Ready Ready Preview, Comment Jan 14, 2026 8:00pm

@supabase
Copy link

supabase bot commented Jan 14, 2026

Updates to Preview Branch (feature/community-promo-toast) ↗︎

Deployments Status Updated
Database Wed, 14 Jan 2026 20:00:28 UTC
Services Wed, 14 Jan 2026 20:00:28 UTC
APIs Wed, 14 Jan 2026 20:00:28 UTC

Tasks are run on every commit but only new migration files are pushed.
Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks Status Updated
Configurations Wed, 14 Jan 2026 20:00:37 UTC
Migrations Wed, 14 Jan 2026 20:00:48 UTC
Seeding Wed, 14 Jan 2026 20:00:57 UTC
Edge Functions Wed, 14 Jan 2026 20:01:01 UTC

View logs for this Workflow Run ↗︎.
Learn more about Supabase for Git ↗︎.

@sonyccd sonyccd merged commit c42731b into main Jan 14, 2026
11 checks passed
@sonyccd sonyccd deleted the feature/community-promo-toast branch January 14, 2026 20:01
@sentry
Copy link

sentry bot commented Jan 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.91%. Comparing base (df68641) to head (a946004).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #161   +/-   ##
=======================================
  Coverage   86.91%   86.91%           
=======================================
  Files         117      117           
  Lines       14684    14689    +5     
  Branches     2262     2264    +2     
=======================================
+ Hits        12762    12767    +5     
  Misses       1922     1922           
Flag Coverage Δ
unittests 86.91% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/hooks/useCommunityPromoToast.ts 99.12% <100.00%> (+0.04%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@claude
Copy link

claude bot commented Jan 14, 2026

PR Review: Fix Community Promo Toast Race Condition

Summary

This PR addresses a legitimate race condition bug where the community promo toast was incorrectly showing for users who already have a forum_username. The fix is clean, well-tested, and follows the project's best practices.


✅ Strengths

1. Excellent Problem Identification

  • The root cause analysis is spot-on: the hook was making decisions before profile data loaded
  • The distinction between undefined (loading), null (no value), and string (has value) is the correct pattern for async state

2. Outstanding Test Coverage
The 3 new tests comprehensively cover the race condition:

  • useCommunityPromoToast.test.ts:86 - Verifies no toast when profile is loading
  • useCommunityPromoToast.test.ts:99 - Tests the happy path: undefined → null shows toast
  • useCommunityPromoToast.test.ts:122 - Tests the fix: undefined → string does NOT show toast

These tests use rerender() to simulate the actual user flow, which is the right approach.

3. Minimal, Focused Change

  • Only 6 lines added to the implementation (lines 36-40)
  • No unnecessary refactoring or scope creep
  • Clear inline comments explaining the tri-state logic

4. Follows Project Conventions

  • Matches the existing guard pattern in the hook
  • Consistent with TypeScript's relaxed settings (no strict null checks required)
  • Tests follow the established patterns in the test file

🔍 Code Quality Observations

Implementation (useCommunityPromoToast.ts:36-40)

Correct placement: This guard is positioned perfectly - after auth check but before the if (forumUsername) check
Clear documentation: The comment explicitly explains the tri-state meaning
Type-safe: Uses strict equality (===) for undefined check


🔒 Security & Performance

No security concerns: No XSS, injection, or data exposure risks
No performance impact: The additional guard is O(1) and exits early
No memory leaks: Cleanup logic remains unchanged and correct


📊 Integration Check

The hook is called in src/components/AppLayout.tsx:84-88:

✅ This confirms the fix is necessary - profile?.forum_username will be undefined until the profile query resolves.


📝 Final Recommendation

APPROVE

This PR is production-ready. The fix is correct, well-tested, and follows all project conventions from CLAUDE.md. No blocking issues or changes required.

Optional follow-up: After this merges, consider documenting this tri-state pattern in TESTING.md under "Common Pitfalls" so other developers can learn from this pattern.


Reviewed with ❤️ by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants