add unit + integration coverage for Course Builder slug gating #1550
Conversation
…guide-to-react-broken-lesson-titles] Add tests to ensure Course Builder DB queries only use LIKE matching when a slug contains a "~<id>" suffix, preventing legacy slugs from accidentally matching and overwriting titles.
…rs-guide-to-react-broken-lesson-titles] Covers the full loadLesson and loadResourcesForCourse flows with mocked GraphQL, Sanity, and Course Builder DB. Ensures legacy slugs never trigger LIKE-based Course Builder matching and titles cannot be overwritten by unrelated records.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds integration tests that verify legacy slug handling avoids LIKE-based DB queries for lessons and courses; increases playlist GraphQL page size from 25 to 200; and makes legacy-lesson fetch in [post].tsx resilient to non-OK responses and exceptions. Changes
Sequence Diagram(s)(Skipped — changes are tests, a pagination constant tweak, and UI fetch error handling; no new multi-component control flow requiring sequence diagrams.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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)
No actionable comments were generated in the recent review. 🎉 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 |
…for courses to 3600 seconds Updated the loadAllPlaylistsByPage function to fetch 200 playlists per request instead of 25. Additionally, modified the getStaticProps function in the courses index to revalidate every hour, improving data freshness.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/pages/courses/index.tsx (1)
16-18: ISR implementation looks good, but appears unrelated to PR objectives.Adding
revalidate: 3600enables hourly Incremental Static Regeneration, which is appropriate for a courses index that doesn't require real-time updates. The implementation is correct.However, this optimization appears unrelated to the PR's stated objectives (adding unit and integration test coverage for Course Builder slug gating). If this change was discovered as a necessary improvement during testing, consider documenting the connection in the PR description.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/playlists.ts(1 hunks)src/pages/courses/index.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,mjs,cjs,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/logging-session.mdc)
**/*.{js,mjs,cjs,ts,tsx}: On application startup, generate a timestamp YYYY-MM-DD-HH-MM-SS and create logs/.log; append all log output to this file
Each application run must create a new log file, preserving previous logs
Use a logger (e.g., pino or winston) or file writes to implement logging
Record start/end of major operations, successful paths, handled errors, and unexpected failures
Include environment info (e.g., CLI arguments and Node version) in logs
Provide a toggle to disable file logging: LOG_LEVEL=off or --no-log; when disabled, do not create the log file and suppress log output
Prefer structured log lines (JSON or clearly delimited) with level, timestamp, and message in each entry
Logs must capture both happy paths and errors while maintaining readability and sufficient detail for troubleshooting
Files:
src/lib/playlists.tssrc/pages/courses/index.tsx
{src,app,lib,packages}/**
📄 CodeRabbit inference engine (.cursor/rules/project-update-user-rules.mdc)
Examine main source directories (e.g., src/, app/, lib/, packages/) to understand module organization and architectural patterns
Files:
src/lib/playlists.tssrc/pages/courses/index.tsx
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use the @/ path alias for imports from the src directory
Files:
src/lib/playlists.tssrc/pages/courses/index.tsx
src/lib/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place database queries and data-fetching logic in src/lib/[feature] and mark server-only files with 'use server'
Files:
src/lib/playlists.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-rules.mdc)
**/*.{tsx,jsx}: Do not include non-memoized functions in React useEffect dependency arrays to avoid infinite re-renders
For effects that should only run on mount/unmount, use an empty dependency array [] in useEffect
If a function must be a dependency in useEffect, memoize it with useCallback so its identity is stable
Files:
src/pages/courses/index.tsx
src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.tsx: Prefer Tailwind utility classes for styling in components; use CSS Modules only for complex styles
Use trpc.useQuery() for client-side data fetching
Files:
src/pages/courses/index.tsx
src/pages/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
In legacy pages, keep files focused on orchestration/layout and import extracted modules during migration
Files:
src/pages/courses/index.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: skillrecordings/egghead-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T17:59:16.787Z
Learning: Applies to src/lib/**/*course-builder*.ts : Use flexible slug matching in SQL: id = ? OR JSON_UNQUOTE(JSON_EXTRACT(fields, '$.slug')) = ?
Learnt from: CR
Repo: skillrecordings/egghead-next PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-02T17:59:16.787Z
Learning: Applies to src/lib/**/*course-builder*.ts : Identify content types as specified (courses: type='post' with fields.postType='course'; videos: type='videoResource') and model relationships via egghead_ContentResourceResource
🔇 Additional comments (1)
src/lib/playlists.ts (1)
53-53: Document GraphQL endpoint pagination limits or justify the 200-item page size choice.This change was intentional per the commit history, but the per_page value of 200 lacks inline documentation or endpoint specifications. While the retry logic (5 attempts) indicates the change works in practice, without egghead's GraphQL API documentation, the endpoint's actual capacity limits cannot be verified. Consider: (1) Adding a code comment explaining why 200 was chosen, (2) Documenting the endpoint's pagination limits in a config or README, or (3) Moving this optimization to a separate PR/issue if it's orthogonal to the slug gating feature.
Eliminated the revalidation parameter from the getStaticProps function in the courses index, simplifying the data fetching logic.
| } = await request(config.graphQLEndpoint, query, { | ||
| page: currentPage, | ||
| per_page: 25, | ||
| per_page: 200, |
There was a problem hiding this comment.
loadAllPlaylistsByPage fetches 25 items per page sequentially. This causes many slow network roundtrips.
There was a problem hiding this comment.
this is probably too much data and will be slower, this has worked for literally years so that's interesting to me that it'd be an issue now
Enhanced the getStaticProps function to include error handling when fetching legacy lesson data, ensuring better resilience and logging of fetch failures.
joelhooks
deleted the
cp/beginners-guide-to-react-broken-lesson-titles
branch
2 participants
Summary
LIKEmatching in Course Builder DB lookups.loadLessonandloadResourcesForCourseto ensure legacy lesson/course titles and ordering are not polluted by unrelated Course Builder records.Test plan
pnpm test:ci -- src/lib/__tests__/get-course-builder-metadata.test.tspnpm test:ci -- src/lib/__tests__/load-lesson.integration.test.ts src/lib/__tests__/course-resources.integration.test.tsSummary by CodeRabbit
Bug Fixes
Performance
Tests