Fix activity feed race condition that flashes empty state#848
Open
marcodejongh wants to merge 13 commits intomainfrom
Open
Fix activity feed race condition that flashes empty state#848marcodejongh wants to merge 13 commits intomainfrom
marcodejongh wants to merge 13 commits intomainfrom
Conversation
Fix broken queue-persistence tests for queue bridge architecture: - waitForBoardPage: wait for climb card instead of queue bar - addClimbToQueue: use dblclick (required in list mode) - Replace queue-item count checks with queue bar visibility - Use client-side navigation instead of page.goto for state preservation - Fix global bar click test to use thumbnail link Add comprehensive bottom-tab-bar e2e tests: - Visibility on all pages (home, board, settings, notifications, library) - Navigation between tabs - Active state verification via Mui-selected class - Queue bar + bottom tab bar coexistence Add data-testid attributes to BottomNavigation and bottom bar wrapper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Strengthen climb name assertions: all tests now capture the climb name
and verify it stays correct after each navigation, not just bar visibility
- Fix hardcoded localhost: replace /localhost:3000\/($|\?)/ with
toHaveURL('/') using Playwright's baseURL-relative matching
- Fix fragile .last() selector: scope Home button (and all tab buttons) to
[data-testid="bottom-tab-bar"] via bottomTabButton() helper
- Keep Mui-selected class for active state (MUI BottomNavigationAction does
not render aria-selected); added comment explaining why
- Add test.slow() for multi-page navigation tests that exceed 30s default
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The branches filter on pull_request matches the base branch, which prevented workflows from running on PRs targeting feature branches (e.g. better_global_queue_control_bar). Paths filtering alone is sufficient to scope when workflows run. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The e2e workflow was failing because it used a bare PostgreSQL image without board data and ran drizzle-kit migrate from packages/web/ which has no drizzle config. Switch to the pre-built dev-db image and run migrations from packages/db/ where drizzle.config.ts lives. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The dev-db image has data baked into the 'main' database with password 'password'. POSTGRES_DB is ignored when the image already has an initialized data directory, so the migration was failing with "database boardsesh_test does not exist". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Rewrite beforeEach to use direct URL navigation instead of obsolete combobox board selection. Update all test selectors to match the redesigned UI (search drawer, queue bar, user drawer). Update FAQ text in help-content.tsx to reference current UI flows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The Next.js app uses the Neon serverless driver which connects via HTTP fetch through a proxy. Added the local-neon-http-proxy service and set the runtime DATABASE_URL to db.localtest.me so the Neon config routes queries through the proxy at localhost:4444. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The SSR climb search makes GraphQL HTTP requests to the backend at port 8080. Without the backend running, pages degrade to empty results and tests fail waiting for climb cards. - Build and start the backend before the web server - Set NEXT_PUBLIC_WS_URL at build time so it gets baked into the bundle - Wait for backend /health endpoint before starting the web server - Add packages/backend/** to workflow path triggers - Redis is optional; the backend gracefully degrades without it Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Switch from installing Playwright browsers (5+ min) to running the job inside mcr.microsoft.com/playwright:v1.57.0-noble which has Chromium pre-installed. Since container jobs access services by hostname instead of localhost, add an /etc/hosts entry mapping db.localtest.me to the neon-proxy service IP so the Neon driver's fetch endpoint reaches the proxy. Migration step uses postgres hostname directly (pg driver). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The /health endpoint only responds to GET requests, but wait-on defaults to HEAD for http:// URLs. Use http-get:// prefix to force GET requests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The dev-db image needs more shared memory than the default 64mb. Queries were failing with "could not resize shared memory segment: No space left on device". Matches the shm_size in docker-compose.yml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The activity feed at `/` sporadically flashed items then showed "Follow climbers to see their activity here" even for users with follows. This was caused by two interacting race conditions: 1. `useSession()` status `'loading'` was treated as unauthenticated, firing a trending fetch before auth resolved. 2. No mechanism to discard stale in-flight fetch results, so the trending and authenticated fetches would race and overwrite each other. Fix: add `authSessionLoading` prop to gate fetching until session resolves, and a monotonic fetch ID ref to discard stale responses. Includes unit tests (7 cases) and e2e tests for regression. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Claude Review✅ Ready to merge - No significant issues found. The race condition fix is well-implemented with proper stale fetch protection. Minor observations (non-blocking):
Test coverage:The unit tests (7 cases) effectively cover:
E2E tests provide good coverage of the race condition scenario and bottom tab bar integration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e29f93440
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }); | ||
|
|
||
| // Navigate to home page | ||
| await page.goto('/'); |
There was a problem hiding this comment.
In activity feed loads without flashing empty state, calling page.goto('/') after installing the MutationObserver recreates the document, so the observer and window.__emptyStateFlashed are lost before the assertion runs. In CI (where this authenticated test is not skipped), this can make the check fail with undefined !== false or miss real flashes entirely, so the race-condition regression test is not reliable as written.
Useful? React with 👍 / 👎.
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.
Summary
useSession()status'loading'was treated as unauthenticated, firing a premature trending fetchauthSessionLoadingprop to gate fetching until session resolvesfetchIdRefto discard stale in-flight fetch resultsdata-testidattributes for e2e testing (activity-feed-loading,activity-feed-empty-state,activity-feed-items)Test plan
npm run typecheckpasses (no new errors)npx vitest run app/components/activity-feed/__tests__/activity-feed.test.tsxnpm run test:e2e --workspace=@boardsesh/web(requires dev server + db)/repeatedly as authenticated user, verify no flash of empty state🤖 Generated with Claude Code