-
Notifications
You must be signed in to change notification settings - Fork 36
feat(egh): add createEggheadLessonVersion function and integrate egghead lesson versioning in post updates #481
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ead lesson versioning in post updates
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis change introduces explicit version tracking for Egghead lessons by adding a function to create lesson version records and updating relevant workflows to call it. The update also extends the lesson update logic to include a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PostService
participant LessonService
participant Database
User->>PostService: Create or update post (with Egghead lesson)
PostService->>LessonService: updateEggheadLesson(..., published)
LessonService->>Database: Update lessons table (set published)
alt New post or content hash changed
PostService->>LessonService: createEggheadLessonVersion(lessonId, note)
LessonService->>Database: Insert lesson_versions row
LessonService->>Database: Update lessons.current_lesson_version_id
end
PostService->>User: Return result
Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
Scope: all 3 workspace projects This error happened while installing a direct dependency of /tmp/eslint/packages/config/eslint-config Packages found in the workspace: ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/egghead/src/lib/egghead/lesson.ts
(5 hunks)apps/egghead/src/lib/posts-new-query.ts
(2 hunks)apps/egghead/src/lib/posts/write.ts
(4 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/egghead/src/lib/posts-new-query.ts (1)
apps/egghead/src/lib/egghead/lesson.ts (1)
createEggheadLessonVersion
(264-304)
apps/egghead/src/lib/posts/write.ts (1)
apps/egghead/src/lib/egghead/lesson.ts (1)
createEggheadLessonVersion
(264-304)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: format
- GitHub Check: build
- GitHub Check: e2e-node (course-builder-web)
🔇 Additional comments (8)
apps/egghead/src/lib/egghead/lesson.ts (2)
98-98
: LGTM: Function signature updated correctly.The addition of the
published: boolean
parameter toupdateEggheadLesson
aligns with the PR objective to track lesson published state. The parameter is properly destructured and used in the SQL query.Also applies to: 108-108, 145-145
132-132
: LGTM: SQL query properly updated with published field.The SQL query correctly adds the
published
column update using parameterized queries, maintaining security best practices.Also applies to: 145-145
apps/egghead/src/lib/posts-new-query.ts (2)
15-19
: LGTM: Import added correctly.The import of
createEggheadLessonVersion
is properly added to the existing import statement.
135-137
: LGTM: Lesson version creation properly integrated.The conditional call to
createEggheadLessonVersion
is correctly placed after post version creation and uses an appropriate note "Initial version" for new lessons. The conditional check ensures it only runs when an egghead lesson exists.apps/egghead/src/lib/posts/write.ts (4)
40-40
: LGTM: Import added correctly.The import of
createEggheadLessonVersion
is properly added to the existing import statement from the egghead module.
358-358
: LGTM: Published state correctly derived.The
isPublished
boolean is correctly derived from the post state comparison, providing a clear and readable way to determine the published status.
391-391
: LGTM: Published parameter correctly passed.The
published
parameter is correctly passed to theupdateEggheadLesson
function using the derivedisPublished
value, integrating well with the updated function signature.
471-476
: LGTM: Lesson versioning correctly integrated into update workflow.The conditional call to
createEggheadLessonVersion
is properly placed after content hash change detection and new post version creation. This ensures lesson versions are created only when actual content changes occur, with an appropriate note "Post metadata updated".
export async function createEggheadLessonVersion( | ||
eggheadLessonId: number, | ||
versionNote: string, | ||
) { | ||
const { ability, session } = await getServerAuthSession() | ||
|
||
if (!session?.user?.id || ability.cannot('manage', 'all')) { | ||
throw new Error('Unauthorized') | ||
} | ||
|
||
const eggheadUser = await getEggheadUserProfile(session?.user?.id) | ||
|
||
const columns = ['lesson_id', 'notes', 'user_id', 'created_at'] | ||
|
||
const values = [eggheadLessonId, versionNote, eggheadUser.id, new Date()] | ||
|
||
const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') | ||
|
||
const query = ` | ||
INSERT INTO lesson_versions (${columns.join(', ')}) | ||
VALUES (${placeholders}) | ||
RETURNING id | ||
` | ||
const eggheadLessonVersionResult = await eggheadPgQuery(query, values) | ||
|
||
const eggheadLessonVersionId = eggheadLessonVersionResult.rows[0].id | ||
|
||
console.log('[createEggheadLessonVersion]', { | ||
eggheadLessonVersionId, | ||
eggheadLessonId, | ||
}) | ||
|
||
await eggheadPgQuery( | ||
`UPDATE lessons SET | ||
current_lesson_version_id = $1 | ||
WHERE id = $2`, | ||
[eggheadLessonVersionId, eggheadLessonId], | ||
) | ||
|
||
return eggheadLessonVersionId | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address error handling and remove debug logging.
The createEggheadLessonVersion
function implementation looks good overall, but has a few concerns:
- Potential null reference: Line 274 could throw if
getEggheadUserProfile
returns null/undefined, but there's no validation. - Debug logging: Lines 291-294 contain console.log statements that should be replaced with proper logging or removed.
Apply this diff to improve error handling:
const eggheadUser = await getEggheadUserProfile(session?.user?.id)
+
+ if (!eggheadUser?.id) {
+ throw new Error('Egghead user profile not found')
+ }
Apply this diff to replace debug logging:
- console.log('[createEggheadLessonVersion]', {
- eggheadLessonVersionId,
- eggheadLessonId,
- })
Consider using a proper logging library instead of console.log for production code.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
export async function createEggheadLessonVersion( | |
eggheadLessonId: number, | |
versionNote: string, | |
) { | |
const { ability, session } = await getServerAuthSession() | |
if (!session?.user?.id || ability.cannot('manage', 'all')) { | |
throw new Error('Unauthorized') | |
} | |
const eggheadUser = await getEggheadUserProfile(session?.user?.id) | |
const columns = ['lesson_id', 'notes', 'user_id', 'created_at'] | |
const values = [eggheadLessonId, versionNote, eggheadUser.id, new Date()] | |
const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') | |
const query = ` | |
INSERT INTO lesson_versions (${columns.join(', ')}) | |
VALUES (${placeholders}) | |
RETURNING id | |
` | |
const eggheadLessonVersionResult = await eggheadPgQuery(query, values) | |
const eggheadLessonVersionId = eggheadLessonVersionResult.rows[0].id | |
console.log('[createEggheadLessonVersion]', { | |
eggheadLessonVersionId, | |
eggheadLessonId, | |
}) | |
await eggheadPgQuery( | |
`UPDATE lessons SET | |
current_lesson_version_id = $1 | |
WHERE id = $2`, | |
[eggheadLessonVersionId, eggheadLessonId], | |
) | |
return eggheadLessonVersionId | |
} | |
export async function createEggheadLessonVersion( | |
eggheadLessonId: number, | |
versionNote: string, | |
) { | |
const { ability, session } = await getServerAuthSession() | |
if (!session?.user?.id || ability.cannot('manage', 'all')) { | |
throw new Error('Unauthorized') | |
} | |
const eggheadUser = await getEggheadUserProfile(session?.user?.id) | |
if (!eggheadUser?.id) { | |
throw new Error('Egghead user profile not found') | |
} | |
const columns = ['lesson_id', 'notes', 'user_id', 'created_at'] | |
const values = [eggheadLessonId, versionNote, eggheadUser.id, new Date()] | |
const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') | |
const query = ` | |
INSERT INTO lesson_versions (${columns.join(', ')}) | |
VALUES (${placeholders}) | |
RETURNING id | |
` | |
const eggheadLessonVersionResult = await eggheadPgQuery(query, values) | |
const eggheadLessonVersionId = eggheadLessonVersionResult.rows[0].id | |
await eggheadPgQuery( | |
`UPDATE lessons SET | |
current_lesson_version_id = $1 | |
WHERE id = $2`, | |
[eggheadLessonVersionId, eggheadLessonId], | |
) | |
return eggheadLessonVersionId | |
} |
🤖 Prompt for AI Agents
In apps/egghead/src/lib/egghead/lesson.ts between lines 264 and 304, add a check
after calling getEggheadUserProfile to verify that eggheadUser is not null or
undefined and throw an appropriate error if it is, to prevent potential null
reference errors. Also, remove the console.log debug statements around lines
291-294 and replace them with calls to a proper logging library or remove them
entirely if logging is not needed.
lesson progress was not working properly for users because we weren't creating lesson versions in rails. This adds lesson versions so that when a lesson is created or updated in builder it will push a lesson version to rails as well.
I also updated the affected (~140) lessons with proper lesson versions as well as added lesson version ids to the lesson views users completed on those 140 lessons (~7.5k records)
#EGG-406
Summary by CodeRabbit