feat(egh): core instructor invite flow #435
Conversation
- Added `InvitePage` component to handle displaying and accepting instructor invitations. - Created `AcceptInviteForm` component for users to submit their email and accept the invitation. - Implemented `acceptInstructorInvite` action to validate and update the invite state in the database. - Updated email template to provide a clickable link for accepting the invitation. - Enhanced invite creation process to ensure proper email handling and state management.
- Added server actions for accepting instructor invites and creating instructor profiles. - Created onboarding page and form components for new instructors to complete their profiles. - Implemented logic to validate invite states and handle user creation in the database. - Enhanced email template for instructor invites to streamline the onboarding process. - Introduced a completed onboarding page to confirm successful profile creation.
- Updated `InvitePage` to redirect verified invites to onboarding. - Modified `AcceptInviteForm` to handle toast notifications based on invite acceptance. - Implemented `createInstructorProfile` action to send instructor invite completion events. - Added `instructorInviteCompleted` function to handle user creation and role assignment. - Enhanced onboarding completion page with status checks and loading indicators. - Introduced `invitedById` to track who sent the invite in the database schema.
…/instructor-invite-flow
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces multiple enhancements to the instructor invitation and onboarding flows. It expands the remote image configuration for Cloudinary, adds several new React components, pages, and server-side actions to handle invitation acceptance and instructor onboarding, updates the database schema to include an inviter identifier, and revises event handling with new Inngest functions and types. Additional improvements include updates to email templates and Egghead API integrations for user management. Changes
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: Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (26)
apps/egghead/src/app/(user)/invites/[inviteId]/page.tsx (3)
8-8: Consider simplifying the Params typeThe
Paramstype is defined as a Promise, which is unusual for route parameters. In Next.js App Router, params are typically plain objects, not promises.-type Params = Promise<{ inviteId: string }> +type Params = { inviteId: string }
12-18: Consider adding try/catch for database queryWhile the component handles cases where the invite doesn't exist, it doesn't handle potential database query errors. Adding error handling would make the component more robust.
- const invite = await db.query.invites.findFirst({ - where: eq(invites.id, inviteId), - columns: { - inviteEmail: true, - inviteState: true, - }, - }) + let invite + try { + invite = await db.query.invites.findFirst({ + where: eq(invites.id, inviteId), + columns: { + inviteEmail: true, + inviteState: true, + }, + }) + } catch (error) { + console.error('Error fetching invite:', error) + return <div className="container mx-auto max-w-2xl py-16"> + <h1 className="mb-8 text-3xl font-bold">Error</h1> + <p>There was an error processing your invitation. Please try again later.</p> + </div> + }
20-26: Consider adding loading stateSince this is an async component that fetches data from the database, it would be beneficial to show a loading state or skeleton UI while data is being fetched, especially for users on slower connections.
You could implement this with React Suspense or a simple loading indicator component. Here's a basic example of how you might approach this:
// In a separate Loading component export function InvitePageLoading() { return ( <div className="container mx-auto max-w-2xl py-16"> <div className="h-8 w-64 bg-gray-200 rounded animate-pulse mb-8"></div> <div className="space-y-4"> <div className="h-4 w-20 bg-gray-200 rounded animate-pulse"></div> <div className="h-10 bg-gray-200 rounded animate-pulse"></div> <div className="h-10 w-32 bg-gray-200 rounded animate-pulse"></div> </div> </div> ) } // Then use with Suspense in the parent component or routeapps/egghead/src/app/(user)/invites/[inviteId]/onboarding/page.tsx (2)
10-22: Add error handling for database operationsThe code properly handles the case when an invite is not found or not in the correct state, but it doesn't handle potential exceptions from the database query. Consider adding a try/catch block around the database operation to gracefully handle any unexpected errors.
export default async function OnboardingPage(props: { params: Params }) { const { inviteId } = await props.params + try { const invite = await db.query.invites.findFirst({ where: eq(invites.id, inviteId), columns: { acceptedEmail: true, inviteState: true, }, }) if (!invite || invite.inviteState !== 'VERIFIED') { notFound() } + } catch (error) { + console.error('Error fetching invite:', error) + notFound() + }
29-29: Avoid non-null assertion on acceptedEmailThe code uses a non-null assertion (
!) oninvite.acceptedEmail. While this might be safe if the database schema guarantees that a 'VERIFIED' invite always has anacceptedEmail, it's better to handle potential null/undefined values explicitly for type safety.<InstructorOnboardingForm inviteId={inviteId} - acceptedEmail={invite.acceptedEmail!} + acceptedEmail={invite.acceptedEmail || ''} />apps/egghead/src/app/(user)/invites/[inviteId]/actions.ts (2)
17-32: Add explicit email validation before proceedingWhile form validation might happen client-side, it's good practice to also validate the email format server-side before using it for critical operations. Consider adding email format validation to ensure the provided email is valid.
export async function acceptInstructorInvite({ inviteId, email, }: AcceptInstructorInviteParams) { + // Basic email validation + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ + if (!emailRegex.test(email)) { + throw new Error('Invalid email format') + } const invite = await db.query.invites.findFirst({ where: eq(invites.id, inviteId), columns: { inviteEmail: true, inviteState: true, }, }) if (!invite) { throw new Error('Invite not found') } if (invite.inviteState !== 'INITIATED') { throw new Error('Invite has already been used or expired') }
33-41: Add error handling for database update operationThe code doesn't handle potential exceptions from the database update operation. Consider adding a try/catch block to gracefully handle any unexpected errors during the update.
+ try { await db .update(invites) .set({ inviteState: 'VERIFIED', acceptedEmail: email, confirmedAt: new Date(), }) .where(eq(invites.id, inviteId)) + } catch (error) { + console.error('Error updating invite:', error) + throw new Error('Failed to verify invite') + }apps/egghead/src/inngest/functions/instructor-invite-created.ts (2)
22-33: Add error handling for database operationsThe function doesn't handle potential exceptions when inserting the invite record. Consider adding error handling to ensure the function doesn't fail silently if there's a database issue.
const newInviteId = await step.run('create invite', async () => { const inviteId = nanoid() + try { await db.insert(invites).values({ id: inviteId, inviteState: 'INITIATED', inviteEmail: event.data.email, invitedById: event.data.invitedById, createdAt: new Date(), }) + } catch (error) { + console.error('Error creating invite:', error) + throw new NonRetriableError(`Failed to create invite: ${error.message}`) + } return inviteId })
37-49: Add validation checks for environment variablesThe code uses environment variables for constructing the invite URL and setting email addresses, but doesn't validate their existence. Consider adding validation to ensure these critical environment variables are defined.
+if (!process.env.COURSEBUILDER_URL) { + throw new NonRetriableError('COURSEBUILDER_URL environment variable is not defined') +} const inviteUrl = `${process.env.COURSEBUILDER_URL}/invites/${newInviteId}` const sendResponse = await step.run('send the invite email', async () => { + if (!process.env.NEXT_PUBLIC_SUPPORT_EMAIL) { + throw new NonRetriableError('NEXT_PUBLIC_SUPPORT_EMAIL environment variable is not defined') + } return await sendAnEmail({ Component: InstructorInviteEmail, componentProps: { inviteUrl, }, Subject: 'You have been invited to join egghead as an instructor', To: event.data.email, ReplyTo: process.env.NEXT_PUBLIC_SUPPORT_EMAIL, From: process.env.NEXT_PUBLIC_SUPPORT_EMAIL, type: 'transactional', }) })apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/actions.ts (2)
36-50: Add error handling for the Inngest event sendingThe function doesn't handle potential exceptions when sending the Inngest event. Consider adding a try/catch block to gracefully handle any unexpected errors.
export async function createInstructorProfile({ inviteId, firstName, lastName, email, twitter, website, bluesky, bio, profileImageUrl, }: CreateInstructorProfileParams) { + try { await inngest.send({ name: INSTRUCTOR_INVITE_COMPLETED_EVENT, data: { inviteId, firstName, lastName, email, twitter, website, bio, bluesky, profileImageUrl, }, }) + } catch (error) { + console.error('Error sending instructor invite completed event:', error) + throw new Error('Failed to create instructor profile') + } redirect(`/invites/${inviteId}/onboarding/completed`) }
13-23: Add input validation for required fieldsThe function accepts several parameters but doesn't validate them before sending the event. Consider adding validation for required fields to ensure data integrity.
export async function createInstructorProfile({ inviteId, firstName, lastName, email, twitter, website, bluesky, bio, profileImageUrl, }: CreateInstructorProfileParams) { + // Validate required fields + if (!inviteId || !firstName || !lastName || !email) { + throw new Error('Missing required fields') + } + + // Validate email format + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ + if (!emailRegex.test(email)) { + throw new Error('Invalid email format') + }apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/cloudinary-profile-uploader.tsx (2)
14-19: Consider adding cleanup for Cloudinary references.The component correctly initializes the Cloudinary reference, but it's good practice to clear references in the cleanup function of useEffect to prevent memory leaks.
React.useEffect(() => { cloudinaryRef.current = (window as any).cloudinary + return () => { + // Clean up references when component unmounts + cloudinaryRef.current = null + widgetRef.current = null + } }, [])
45-45: Remove commented code.There's a commented line referring to an inline container that's not being used.
-// inline_container: '#cloudinary-upload-widget-container',apps/egghead/src/lib/egghead/auth.ts (3)
86-111: Remove debug console logs before production release.The function includes console logs that should be removed or converted to proper logging before going to production.
return await fetch( `https://app.egghead.io/api/v1/users/${email}?by_email=true&support=true`, { headers: { Authorization: `Bearer ${process.env.EGGHEAD_ADMIN_TOKEN}`, Accept: 'application/json', 'Content-Type': 'application/json', }, }, ).then(async (res) => { if (!res.ok) { - console.error('Full response:', { - status: res.status, - statusText: res.statusText, - headers: Object.fromEntries(res.headers.entries()), - }) throw new Error( `Failed to get egghead user: ${res.status} ${res.statusText}`, ) } const data = await res.json() - console.log('egghead user data', data) return data })
87-110: Consider using more specific error types.The function currently throws generic errors. Consider extending the EggheadApiError class that's already imported and used elsewhere in the file.
return await fetch( `https://app.egghead.io/api/v1/users/${email}?by_email=true&support=true`, { headers: { Authorization: `Bearer ${process.env.EGGHEAD_ADMIN_TOKEN}`, Accept: 'application/json', 'Content-Type': 'application/json', }, }, ).then(async (res) => { if (!res.ok) { console.error('Full response:', { status: res.status, statusText: res.statusText, headers: Object.fromEntries(res.headers.entries()), }) - throw new Error( - `Failed to get egghead user: ${res.status} ${res.statusText}`, - ) + throw new EggheadApiError(res.statusText, res.status) } const data = await res.json() console.log('egghead user data', data) return data })
118-140: Consider adding rate limiting or retry logic.API calls to external services should typically include retry logic for transient failures, especially for critical user flows like registration.
You could implement a simple retry mechanism:
export async function createEggheadUser(email: string) { + const maxRetries = 3; + let retries = 0; + + async function attemptRequest() { return await fetch('https://app.egghead.io/api/v1/users/send_token', { method: 'POST', headers: { 'Content-Type': 'application/json', Accept: 'application/json', }, body: JSON.stringify({ email, }), }).then(async (res) => { if (!res.ok) { + if (res.status >= 500 && retries < maxRetries) { + retries++; + // Exponential backoff + const delay = 1000 * Math.pow(2, retries); + await new Promise(resolve => setTimeout(resolve, delay)); + return attemptRequest(); + } throw new Error( `Failed to create egghead user: ${res.status} ${res.statusText}`, ) } const data = await res.json() if (!data) { throw new Error('No data returned from egghead API') } return data }) + } + + return attemptRequest(); }apps/egghead/src/app/(user)/invites/[inviteId]/_components/accept-invite-form.tsx (2)
9-13: Ensure alignment between prop names and usage.
The propinviteEmailis directly stored in local state as
46-67: UI/UX improvement suggestion.
Disable the entire form (including labels and hints) whenisSubmittingto clearly indicate that submission is ongoing and prevent accidental edits.apps/egghead/src/inngest/functions/instructor-invite-completed.ts (1)
28-154: Consider using transactional or compensating operations for partial failures.
Currently, if one of the steps mid-process fails, the initial data insertions won’t be reverted, leaving the data in a partially updated state. A transaction or distinct rollback mechanism could ensure consistency if any error occurs.apps/egghead/src/emails/instructor-invite-email.tsx (1)
31-35: Refine the invitation link to a styled button.
You already importButtonfrom@react-email/components, but you’re using a plain<a>tag. Switching to theButtoncomponent may enhance accessibility and styling consistency.apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/completed/page.tsx (3)
13-13: Consider using more descriptive naming for the Params typeThe
Paramstype is defined as a Promise containing an inviteId, which is a bit unusual. Typically, route parameters in Next.js aren't Promise types. Consider renaming to something more descriptive likeRouteParamsand removing the Promise wrapper if not needed.-type Params = Promise<{ inviteId: string }> +type RouteParams = { inviteId: string }
16-64: Good implementation of InviteStatus component, consider adding enum for invite statesThe component effectively handles different states of the invitation process and provides appropriate UI feedback. To improve code maintainability:
- Consider using an enum for invite states instead of string literals
- Add explicit error handling for database query failures
+ // At the top of the file or in a separate types file + enum InviteState { + PENDING = 'PENDING', + ACCEPTED = 'ACCEPTED', + VERIFIED = 'VERIFIED', + COMPLETED = 'COMPLETED' + } async function InviteStatus({ // ... existing parameters }) { // ... existing code try { const invite = await db.query.invites.findFirst({ // ... existing query }) // ... existing checks and rendering - if (invite.inviteState === 'VERIFIED') { + if (invite.inviteState === InviteState.VERIFIED) { // ... existing rendering } // ... rest of the component + } catch (error) { + console.error('Failed to fetch invite status:', error); + return ( + <div className="text-destructive"> + An error occurred while checking your invite status. Please try again. + </div> + ); + } }
66-93: Validate inviteId early to prevent unnecessary downstream processingThe component nicely fetches providers and CSRF token, but consider validating the inviteId parameter early to prevent unnecessary processing if it's invalid.
export default async function OnboardingCompletedPage(props: { params: Params }) { const { inviteId } = await props.params + + // Early validation of inviteId format + if (!inviteId || typeof inviteId !== 'string' || inviteId.length === 0) { + notFound() + } const providers = await getProviders() const csrfToken = await getCsrf() // Rest of the component remains the same }apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/instructor-onboarding-form.tsx (2)
21-23: Externalize default image URL to configurationThe default profile image URL is hardcoded. Consider moving this to a configuration constant or environment variable to make it easier to update across the application.
+ // At the top of the file or in a constants file + const DEFAULT_PROFILE_IMAGE = 'https://res.cloudinary.com/dg3gyk0gu/image/upload/v1566948117/transcript-images/Eggo_Notext.png'; export function InstructorOnboardingForm({ inviteId, acceptedEmail, }: InstructorOnboardingFormProps) { const [isSubmitting, setIsSubmitting] = useState(false) const [imageUrl, setImageUrl] = useState( - 'https://res.cloudinary.com/dg3gyk0gu/image/upload/v1566948117/transcript-images/Eggo_Notext.png', + DEFAULT_PROFILE_IMAGE, )
32-43: Add form data validation before submissionThe form currently relies on HTML's basic validation (required attributes). Consider adding more robust validation for fields like Twitter, BlueSky, and website format before submission.
async function onSubmit(e: React.FormEvent<HTMLFormElement>) { e.preventDefault() setIsSubmitting(true) const formData = new FormData(e.currentTarget) + const firstName = formData.get('firstName') as string + const lastName = formData.get('lastName') as string + const email = formData.get('email') as string + const twitter = formData.get('twitter') as string + const bluesky = formData.get('bluesky') as string + const website = formData.get('website') as string + const bio = formData.get('bio') as string + + // Simple validation + if (!firstName || !lastName || !email) { + toast({ + title: 'Missing required fields', + description: 'Please fill out all required fields.', + variant: 'destructive', + }) + setIsSubmitting(false) + return + } + + // Validate Twitter format if provided + if (twitter && !twitter.match(/^@?[\w\d]+$/)) { + toast({ + title: 'Invalid Twitter handle', + description: 'Please enter a valid Twitter handle.', + variant: 'destructive', + }) + setIsSubmitting(false) + return + } try { await createInstructorProfile({ inviteId, - firstName: formData.get('firstName') as string, - lastName: formData.get('lastName') as string, - email: formData.get('email') as string, - twitter: formData.get('twitter') as string, - bluesky: formData.get('bluesky') as string, - website: formData.get('website') as string, - bio: formData.get('bio') as string, + firstName, + lastName, + email, + twitter, + bluesky, + website, + bio, profileImageUrl: imageUrl, })apps/egghead/src/lib/egghead/instructor.ts (1)
97-122: Make revenue split percentage configurableThe revenue split percentage (0.2 or 20%) is hardcoded. Consider making it configurable.
export async function addRevenueSplitToEggheadInstructor({ eggheadInstructorId, + percentage = 0.2, // Default to 20% }: { eggheadInstructorId: string + percentage?: number }) { const revenueSplitQuery = ` INSERT INTO instructor_revenue_splits ( instructor_id, credit_to_instructor_id, percentage, from_date ) VALUES ( $1, $2, $3, $4 ); ` + // Validate percentage is between 0 and 1 + const validPercentage = Math.max(0, Math.min(1, percentage)) await eggheadPgQuery(revenueSplitQuery, [ eggheadInstructorId, null, - 0.2, + validPercentage, new Date(), ]) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/egghead/next.config.mjs(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/_components/accept-invite-form.tsx(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/actions.ts(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/cloudinary-profile-uploader.tsx(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/instructor-onboarding-form.tsx(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/actions.ts(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/completed/page.tsx(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/page.tsx(1 hunks)apps/egghead/src/app/(user)/invites/[inviteId]/page.tsx(1 hunks)apps/egghead/src/app/admin/instructors/invite/actions.ts(1 hunks)apps/egghead/src/db/schemas/invites.ts(2 hunks)apps/egghead/src/emails/instructor-invite-email.tsx(1 hunks)apps/egghead/src/inngest/events/instructor-invite-completed.ts(1 hunks)apps/egghead/src/inngest/events/instructor-invite-created.ts(1 hunks)apps/egghead/src/inngest/functions/instructor-invite-completed.ts(1 hunks)apps/egghead/src/inngest/functions/instructor-invite-created.ts(2 hunks)apps/egghead/src/inngest/inngest.config.ts(2 hunks)apps/egghead/src/inngest/inngest.server.ts(2 hunks)apps/egghead/src/lib/egghead/auth.ts(1 hunks)apps/egghead/src/lib/egghead/index.ts(1 hunks)apps/egghead/src/lib/egghead/instructor.ts(1 hunks)packages/utils-email/src/send-an-email.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (10)
apps/egghead/src/app/admin/instructors/invite/actions.ts (2)
apps/egghead/src/inngest/inngest.server.ts (1) (1)
inngest(90:94)apps/egghead/src/inngest/events/instructor-invite-created.ts (1) (1)
INSTRUCTOR_INVITE_CREATED_EVENT(1:1)
apps/egghead/src/inngest/functions/instructor-invite-created.ts (1)
apps/egghead/src/emails/instructor-invite-email.tsx (1) (1)
InstructorInviteEmail(19:43)
apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/actions.ts (2)
apps/egghead/src/inngest/inngest.server.ts (1) (1)
inngest(90:94)apps/egghead/src/inngest/events/instructor-invite-completed.ts (1) (1)
INSTRUCTOR_INVITE_COMPLETED_EVENT(1:1)
apps/egghead/src/inngest/inngest.server.ts (1)
apps/egghead/src/inngest/events/instructor-invite-completed.ts (2) (2)
INSTRUCTOR_INVITE_COMPLETED_EVENT(1:1)InstructorInviteCompleted(2:15)
apps/egghead/src/app/(user)/invites/[inviteId]/page.tsx (1)
apps/egghead/src/app/(user)/invites/[inviteId]/_components/accept-invite-form.tsx (1) (1)
AcceptInviteForm(14:69)
apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/page.tsx (1)
apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/instructor-onboarding-form.tsx (1) (1)
InstructorOnboardingForm(16:150)
apps/egghead/src/inngest/functions/instructor-invite-completed.ts (3)
apps/egghead/src/inngest/events/instructor-invite-completed.ts (1) (1)
INSTRUCTOR_INVITE_COMPLETED_EVENT(1:1)apps/egghead/src/lib/egghead/auth.ts (1) (1)
getEggheadUserByEmail(86:111)apps/egghead/src/lib/egghead/instructor.ts (3) (3)
createEggheadInstructor(34:91)addInstructorRoleToEggheadUser(9:20)addRevenueSplitToEggheadInstructor(97:122)
apps/egghead/src/app/(user)/invites/[inviteId]/_components/accept-invite-form.tsx (1)
apps/egghead/src/app/(user)/invites/[inviteId]/actions.ts (1) (1)
acceptInstructorInvite(13:43)
apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/instructor-onboarding-form.tsx (2)
apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/actions.ts (1) (1)
createInstructorProfile(25:52)apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/_components/cloudinary-profile-uploader.tsx (1) (1)
CloudinaryUploadButton(9:64)
apps/egghead/src/inngest/inngest.config.ts (1)
apps/egghead/src/inngest/functions/instructor-invite-completed.ts (1) (1)
instructorInviteCompleted(20:155)
🔇 Additional comments (14)
apps/egghead/src/lib/egghead/index.ts (1)
26-26: Export addition looks good!Adding the instructor module export integrates well with the existing pattern in this file. This makes all instructor-related functionality available through the egghead library's API, which is essential for the instructor invitation flow being implemented.
apps/egghead/src/inngest/inngest.config.ts (1)
6-6: Appropriate integration of the instructor invite completion handlerThe addition of the
instructorInviteCompletedfunction to the Inngest configuration is well-structured and follows the existing pattern. This function is crucial for processing completed instructor invitations as part of the invitation flow.Also applies to: 31-31
apps/egghead/src/db/schemas/invites.ts (1)
25-25: Schema enhancement looks goodAdding the
invitedByIdfield to track who initiated an invitation is valuable for analytics and accountability. The field is properly typed and sized in both the Zod schema and the Drizzle table definition.Also applies to: 49-49
apps/egghead/src/app/(user)/invites/[inviteId]/page.tsx (1)
10-34: Well-structured invite page componentThe page component correctly handles different invite states, with appropriate redirects and error handling. The flow is logical:
- Fetch the invite using the ID
- Redirect to onboarding if already verified
- Show 404 if not found or not in the proper state
- Present the acceptance form if valid
apps/egghead/src/app/(user)/invites/[inviteId]/onboarding/actions.ts (1)
51-51: Consider providing feedback on success before redirectingThe function immediately redirects after sending the event without waiting for confirmation that the profile was successfully created. Consider providing feedback to the user or ensuring the profile creation is completed before redirecting.
- redirect(`/invites/${inviteId}/onboarding/completed`) + // Add a small delay to ensure the event is processed + await new Promise(resolve => setTimeout(resolve, 500)) + + // Check if the profile was created successfully + // This is a simplified example - you might need to implement a different approach + const profileCreated = await checkProfileCreated(email) + + if (profileCreated) { + redirect(`/invites/${inviteId}/onboarding/completed`) + } else { + throw new Error('Profile creation in progress. Please try again in a moment.') + }Do you have an existing mechanism to verify that the profile was created successfully before redirecting? If not, this suggestion might require additional implementation work.
apps/egghead/src/inngest/events/instructor-invite-completed.ts (1)
1-15: Well-structured event type for instructor onboarding.The event structure provides a clear contract for the instructor invite completion event with appropriate required fields (id, name, email) and optional profile data.
apps/egghead/src/inngest/inngest.server.ts (2)
31-34: Proper import of new instructor invite event.The import is correctly structured following the established pattern in the file.
64-64: Correctly registered new event in the Events type.The new instructor invite completed event has been properly integrated into the event system.
apps/egghead/src/app/(user)/invites/[inviteId]/_components/accept-invite-form.tsx (1)
22-44: Consider validating user-provided email before submission.
If users change the pre-filled email field, the server-side logic could reject their request if the back end is strict about matching the invite's email. You might add client-side form validation and an appropriate error message to guide users.apps/egghead/src/inngest/events/instructor-invite-created.ts (1)
6-6: Tracking who initiated an instructor inviteThe addition of the
invitedByIdfield to theInstructorInviteCreatedevent type enables proper tracking of which user initiated an instructor invitation. This enhancement supports audit trails and adds context to the invitation process.apps/egghead/next.config.mjs (1)
40-44: Added Cloudinary to remote image patternsThe addition of Cloudinary's domain to the remote patterns list properly enables the application to load and display images from Cloudinary. This configuration is necessary for the instructor profile image upload functionality in the onboarding flow.
apps/egghead/src/app/admin/instructors/invite/actions.ts (3)
5-5: Added server authentication importAppropriate addition of the authentication utility to verify the current user's session.
8-10: Added session validationGood implementation of session validation before proceeding with the invite process.
14-17: Capturing inviter ID with instructor inviteThis change ensures that each instructor invitation is linked to the admin who created it, enhancing audit capabilities and data integrity in the system.
| @@ -52,7 +52,7 @@ export async function sendAnEmail<ComponentPropsType = any>({ | |||
| const { render } = await import('@react-email/render') | |||
|
|
|||
| // The email html needs to be rendered from the React component | |||
| const emailHtml = render(Component(componentProps)) | |||
| const emailHtml = await render(Component(componentProps)) | |||
There was a problem hiding this comment.
Critical fix for asynchronous rendering!
The change correctly updates the render function call to use await, ensuring proper handling of the asynchronous rendering process. Without this fix, the email HTML would contain a stringified Promise instead of the actual rendered content, which would result in broken emails.
| (error: any, result: any) => { | ||
| if (!error && result && result.event === 'success') { | ||
| console.debug('Done! Here is the image info: ', result.info) | ||
| onImageUploadedAction(result.info.secure_url) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in widget callback.
The callback doesn't handle errors, potentially leaving users without feedback when uploads fail.
(error: any, result: any) => {
if (!error && result && result.event === 'success') {
console.debug('Done! Here is the image info: ', result.info)
onImageUploadedAction(result.info.secure_url)
+ } else if (error) {
+ console.error('Error during upload:', error)
+ // Consider adding user-facing error feedback here
}
},📝 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.
| (error: any, result: any) => { | |
| if (!error && result && result.event === 'success') { | |
| console.debug('Done! Here is the image info: ', result.info) | |
| onImageUploadedAction(result.info.secure_url) | |
| } | |
| (error: any, result: any) => { | |
| if (!error && result && result.event === 'success') { | |
| console.debug('Done! Here is the image info: ', result.info) | |
| onImageUploadedAction(result.info.secure_url) | |
| } else if (error) { | |
| console.error('Error during upload:', error) | |
| // Consider adding user-facing error feedback here | |
| } | |
| }, |
| onClick={() => { | ||
| widgetRef.current = cloudinaryRef.current.createUploadWidget( | ||
| { | ||
| cloudName: env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME, | ||
| uploadPreset: env.NEXT_PUBLIC_CLOUDINARY_UPLOAD_PRESET, | ||
| // inline_container: '#cloudinary-upload-widget-container', | ||
| folder: `${dir}/${id}`, | ||
| }, | ||
| (error: any, result: any) => { | ||
| if (!error && result && result.event === 'success') { | ||
| console.debug('Done! Here is the image info: ', result.info) | ||
| onImageUploadedAction(result.info.secure_url) | ||
| } | ||
| }, | ||
| ) | ||
| widgetRef.current.open() | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance error handling for Cloudinary widget initialization.
The code assumes that cloudinaryRef.current will always be defined when the button is clicked, but this could lead to runtime errors if the script hasn't loaded properly.
onClick={() => {
+ if (!cloudinaryRef.current) {
+ console.error('Cloudinary script not loaded')
+ return
+ }
widgetRef.current = cloudinaryRef.current.createUploadWidget(
{
cloudName: env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME,
uploadPreset: env.NEXT_PUBLIC_CLOUDINARY_UPLOAD_PRESET,
// inline_container: '#cloudinary-upload-widget-container',
folder: `${dir}/${id}`,
},
(error: any, result: any) => {
if (!error && result && result.event === 'success') {
console.debug('Done! Here is the image info: ', result.info)
onImageUploadedAction(result.info.secure_url)
+ } else if (error) {
+ console.error('Error uploading image:', error)
}
},
)
widgetRef.current.open()
}}📝 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.
| onClick={() => { | |
| widgetRef.current = cloudinaryRef.current.createUploadWidget( | |
| { | |
| cloudName: env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME, | |
| uploadPreset: env.NEXT_PUBLIC_CLOUDINARY_UPLOAD_PRESET, | |
| // inline_container: '#cloudinary-upload-widget-container', | |
| folder: `${dir}/${id}`, | |
| }, | |
| (error: any, result: any) => { | |
| if (!error && result && result.event === 'success') { | |
| console.debug('Done! Here is the image info: ', result.info) | |
| onImageUploadedAction(result.info.secure_url) | |
| } | |
| }, | |
| ) | |
| widgetRef.current.open() | |
| }} | |
| onClick={() => { | |
| if (!cloudinaryRef.current) { | |
| console.error('Cloudinary script not loaded') | |
| return | |
| } | |
| widgetRef.current = cloudinaryRef.current.createUploadWidget( | |
| { | |
| cloudName: env.NEXT_PUBLIC_CLOUDINARY_CLOUD_NAME, | |
| uploadPreset: env.NEXT_PUBLIC_CLOUDINARY_UPLOAD_PRESET, | |
| // inline_container: '#cloudinary-upload-widget-container', | |
| folder: `${dir}/${id}`, | |
| }, | |
| (error: any, result: any) => { | |
| if (!error && result && result.event === 'success') { | |
| console.debug('Done! Here is the image info: ', result.info) | |
| onImageUploadedAction(result.info.secure_url) | |
| } else if (error) { | |
| console.error('Error uploading image:', error) | |
| } | |
| }, | |
| ) | |
| widgetRef.current.open() | |
| }} |
| export async function createEggheadUser(email: string) { | ||
| return await fetch('https://app.egghead.io/api/v1/users/send_token', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| Accept: 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| email, | ||
| }), | ||
| }).then(async (res) => { | ||
| if (!res.ok) { | ||
| throw new Error( | ||
| `Failed to create egghead user: ${res.status} ${res.statusText}`, | ||
| ) | ||
| } | ||
| const data = await res.json() | ||
| if (!data) { | ||
| throw new Error('No data returned from egghead API') | ||
| } | ||
| return data | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for the returned user data.
The function should validate the structure of the returned data, similar to how getEggheadUserProfile uses a schema to validate the response.
+ import { z } from "zod";
+
+ const EggheadUserSchema = z.object({
+ // Define expected fields here based on API response
+ id: z.string(),
+ email: z.string().email(),
+ // Add other fields as needed
+ });
export async function createEggheadUser(email: string) {
return await fetch('https://app.egghead.io/api/v1/users/send_token', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
Accept: 'application/json',
},
body: JSON.stringify({
email,
}),
}).then(async (res) => {
if (!res.ok) {
throw new Error(
`Failed to create egghead user: ${res.status} ${res.statusText}`,
)
}
const data = await res.json()
if (!data) {
throw new Error('No data returned from egghead API')
}
+ const parsedData = EggheadUserSchema.safeParse(data);
+ if (!parsedData.success) {
+ throw new Error('Invalid user data returned from egghead API', {
+ cause: parsedData.error.flatten().fieldErrors,
+ });
+ }
- return data
+ return parsedData.data
})
}📝 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 createEggheadUser(email: string) { | |
| return await fetch('https://app.egghead.io/api/v1/users/send_token', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Accept: 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| email, | |
| }), | |
| }).then(async (res) => { | |
| if (!res.ok) { | |
| throw new Error( | |
| `Failed to create egghead user: ${res.status} ${res.statusText}`, | |
| ) | |
| } | |
| const data = await res.json() | |
| if (!data) { | |
| throw new Error('No data returned from egghead API') | |
| } | |
| return data | |
| }) | |
| } | |
| import { z } from "zod"; | |
| const EggheadUserSchema = z.object({ | |
| // Define expected fields here based on API response | |
| id: z.string(), | |
| email: z.string().email(), | |
| // Add other fields as needed | |
| }); | |
| export async function createEggheadUser(email: string) { | |
| return await fetch('https://app.egghead.io/api/v1/users/send_token', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| Accept: 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| email, | |
| }), | |
| }).then(async (res) => { | |
| if (!res.ok) { | |
| throw new Error( | |
| `Failed to create egghead user: ${res.status} ${res.statusText}`, | |
| ) | |
| } | |
| const data = await res.json() | |
| if (!data) { | |
| throw new Error('No data returned from egghead API') | |
| } | |
| const parsedData = EggheadUserSchema.safeParse(data); | |
| if (!parsedData.success) { | |
| throw new Error('Invalid user data returned from egghead API', { | |
| cause: parsedData.error.flatten().fieldErrors, | |
| }); | |
| } | |
| return parsedData.data | |
| }) | |
| } |
| if (!eggheadUser && !eggheadUser.id) { | ||
| throw new NonRetriableError('Failed to find/create egghead user') | ||
| } |
There was a problem hiding this comment.
Fix null check logic to avoid runtime error.
Using && in this condition could trigger a crash when eggheadUser is null because eggheadUser.id would be accessed. Switch to an OR condition to correctly handle both possibilities.
- if (!eggheadUser && !eggheadUser.id) {
+ if (!eggheadUser || !eggheadUser.id) {
throw new NonRetriableError('Failed to find/create egghead user')
}📝 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.
| if (!eggheadUser && !eggheadUser.id) { | |
| throw new NonRetriableError('Failed to find/create egghead user') | |
| } | |
| if (!eggheadUser || !eggheadUser.id) { | |
| throw new NonRetriableError('Failed to find/create egghead user') | |
| } |
| if ((error as Error).message === 'NEXT_REDIRECT') { | ||
| toast({ | ||
| title: 'Invitation accepted!', | ||
| description: 'Your instructor profile has been created successfully.', | ||
| }) | ||
| } else { | ||
| toast({ | ||
| title: 'Error', | ||
| description: 'Failed to accept invitation. Please try again.', | ||
| variant: 'destructive', | ||
| }) | ||
| } | ||
| } finally { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider alternative approach for handling redirects
The current error handling catches NEXT_REDIRECT errors, which is a side effect of Next.js's redirect function. This approach is brittle as it relies on implementation details of Next.js.
Consider returning a result object from the server action instead of relying on redirect errors:
// In actions.ts
export async function createInstructorProfile({...}) {
+ try {
await inngest.send({...})
+ return { success: true }
+ } catch (error) {
+ console.error('Failed to create instructor profile:', error)
+ return { success: false, error: (error as Error).message }
+ }
- redirect(`/invites/${inviteId}/onboarding/completed`)
}
// In your component
try {
- await createInstructorProfile({...})
+ const result = await createInstructorProfile({...})
+ if (result.success) {
+ toast({
+ title: 'Invitation accepted!',
+ description: 'Your instructor profile has been created successfully.',
+ })
+ // Use router.push instead of redirect for client-side navigation
+ router.push(`/invites/${inviteId}/onboarding/completed`)
+ } else {
+ throw new Error(result.error || 'Unknown error')
+ }
} catch (error) {
- if ((error as Error).message === 'NEXT_REDIRECT') {
- toast({...}) // Success toast
- } else {
toast({...}) // Error toast
- }
}📝 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.
| if ((error as Error).message === 'NEXT_REDIRECT') { | |
| toast({ | |
| title: 'Invitation accepted!', | |
| description: 'Your instructor profile has been created successfully.', | |
| }) | |
| } else { | |
| toast({ | |
| title: 'Error', | |
| description: 'Failed to accept invitation. Please try again.', | |
| variant: 'destructive', | |
| }) | |
| } | |
| } finally { | |
| export async function createInstructorProfile({...}) { | |
| + try { | |
| await inngest.send({...}) | |
| + return { success: true } | |
| + } catch (error) { | |
| + console.error('Failed to create instructor profile:', error) | |
| + return { success: false, error: (error as Error).message } | |
| + } | |
| - redirect(`/invites/${inviteId}/onboarding/completed`) | |
| } |
| if ((error as Error).message === 'NEXT_REDIRECT') { | |
| toast({ | |
| title: 'Invitation accepted!', | |
| description: 'Your instructor profile has been created successfully.', | |
| }) | |
| } else { | |
| toast({ | |
| title: 'Error', | |
| description: 'Failed to accept invitation. Please try again.', | |
| variant: 'destructive', | |
| }) | |
| } | |
| } finally { | |
| try { | |
| - await createInstructorProfile({...}) | |
| + const result = await createInstructorProfile({...}) | |
| + if (result.success) { | |
| + toast({ | |
| + title: 'Invitation accepted!', | |
| + description: 'Your instructor profile has been created successfully.', | |
| + }) | |
| + // Use router.push instead of redirect for client-side navigation | |
| + router.push(`/invites/${inviteId}/onboarding/completed`) | |
| + } else { | |
| + throw new Error(result.error || 'Unknown error') | |
| + } | |
| } catch (error) { | |
| - if ((error as Error).message === 'NEXT_REDIRECT') { | |
| - toast({ | |
| - title: 'Invitation accepted!', | |
| - description: 'Your instructor profile has been created successfully.', | |
| - }) | |
| - } else { | |
| - toast({ | |
| - title: 'Error', | |
| - description: 'Failed to accept invitation. Please try again.', | |
| - variant: 'destructive', | |
| - }) | |
| - } | |
| + toast({ | |
| + title: 'Error', | |
| + description: 'Failed to accept invitation. Please try again.', | |
| + variant: 'destructive', | |
| + }) | |
| } finally { | |
| // any finalization code remains unchanged | |
| } |
| const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') | ||
|
|
||
| const query = ` | ||
| INSERT INTO instructors (${columns.join(', ')}) | ||
| VALUES (${placeholders}) | ||
| RETURNING id | ||
| ` | ||
|
|
||
| const eggheadInstructorResult = await eggheadPgQuery(query, values) | ||
|
|
||
| return eggheadInstructorResult.rows[0].id |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for database operations
The database query doesn't have error handling, which could lead to uncaught exceptions.
const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ')
const query = `
INSERT INTO instructors (${columns.join(', ')})
VALUES (${placeholders})
RETURNING id
`
+ try {
const eggheadInstructorResult = await eggheadPgQuery(query, values)
+
+ if (!eggheadInstructorResult.rows || eggheadInstructorResult.rows.length === 0) {
+ throw new Error('Failed to create instructor: No ID returned')
+ }
return eggheadInstructorResult.rows[0].id
+ } catch (error) {
+ console.error('Error creating instructor in database:', error)
+ throw new Error(`Failed to create instructor: ${(error as Error).message}`)
+ }📝 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.
| const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') | |
| const query = ` | |
| INSERT INTO instructors (${columns.join(', ')}) | |
| VALUES (${placeholders}) | |
| RETURNING id | |
| ` | |
| const eggheadInstructorResult = await eggheadPgQuery(query, values) | |
| return eggheadInstructorResult.rows[0].id | |
| const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') | |
| const query = ` | |
| INSERT INTO instructors (${columns.join(', ')}) | |
| VALUES (${placeholders}) | |
| RETURNING id | |
| ` | |
| try { | |
| const eggheadInstructorResult = await eggheadPgQuery(query, values) | |
| if (!eggheadInstructorResult.rows || eggheadInstructorResult.rows.length === 0) { | |
| throw new Error('Failed to create instructor: No ID returned') | |
| } | |
| return eggheadInstructorResult.rows[0].id | |
| } catch (error) { | |
| console.error('Error creating instructor in database:', error) | |
| throw new Error(`Failed to create instructor: ${(error as Error).message}`) | |
| } |
| const values = [ | ||
| firstName, | ||
| lastName, | ||
| `${firstName}-${lastName}`, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve slug generation to avoid collisions
The current slug generation is too simple and could easily lead to collisions for instructors with the same name.
+ import { slugify } from 'some-slug-library'; // Import a robust slugify function
+ // Or implement a more robust slug generation function
+ function generateUniqueSlug(firstName: string, lastName: string): string {
+ const base = `${firstName.toLowerCase()}-${lastName.toLowerCase()}`;
+ const timestamp = Date.now().toString(36).slice(-4);
+ return `${base}-${timestamp}`;
+ }
const values = [
firstName,
lastName,
- `${firstName}-${lastName}`,
+ generateUniqueSlug(firstName, lastName),
userId,📝 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.
| const values = [ | |
| firstName, | |
| lastName, | |
| `${firstName}-${lastName}`, | |
| import { slugify } from 'some-slug-library'; // Import a robust slugify function | |
| // Or implement a more robust slug generation function | |
| function generateUniqueSlug(firstName: string, lastName: string): string { | |
| const base = `${firstName.toLowerCase()}-${lastName.toLowerCase()}`; | |
| const timestamp = Date.now().toString(36).slice(-4); | |
| return `${base}-${timestamp}`; | |
| } | |
| const values = [ | |
| firstName, | |
| lastName, | |
| generateUniqueSlug(firstName, lastName), | |
| userId, | |
| ]; |
| if ((error as Error).message === 'NEXT_REDIRECT') { | ||
| toast({ | ||
| title: 'Invitation accepted!', | ||
| description: 'You will be redirected to complete your profile.', | ||
| }) | ||
| } else { |
There was a problem hiding this comment.
for some reason next redirects show up as errors so the 'error' toast was popping up even though the invite was successfully accepted. Had to check for NEXT_REDIRECT in the message
| await db | ||
| .update(invites) | ||
| .set({ | ||
| inviteState: 'VERIFIED', | ||
| acceptedEmail: email, | ||
| confirmedAt: new Date(), | ||
| }) | ||
| .where(eq(invites.id, inviteId)) |
There was a problem hiding this comment.
not much happens when an email is accepted, we use the invite.acceptedEmail later onto check for egghead account and create a new one if it doesn't exist
|
|
||
| import { Button } from '@coursebuilder/ui' | ||
|
|
||
| export const CloudinaryUploadButton: React.FC<{ |
There was a problem hiding this comment.
created a onboarding specific upload button that doesn't do an auth check and saves the uploaded profile picture in state to send with the rest of the profile information. This button doesn't save the uploaded profile picture as a resource as there is no user to associate it to
| await inngest.send({ | ||
| name: INSTRUCTOR_INVITE_COMPLETED_EVENT, | ||
| data: { | ||
| inviteId, | ||
| firstName, | ||
| lastName, | ||
| email, | ||
| twitter, | ||
| website, | ||
| bio, | ||
| bluesky, | ||
| profileImageUrl, | ||
| }, | ||
| }) |
There was a problem hiding this comment.
server action kicks off inngest flow
| data: { email }, | ||
| data: { | ||
| email, | ||
| invitedById: session.user.id, |
There was a problem hiding this comment.
adds an invitedById to the invite so we know who invited who
| let eggheadUser = null | ||
| eggheadUser = await step.run('attempt to get egghead user', async () => { | ||
| return await getEggheadUserByEmail(acceptedEmail) | ||
| }) | ||
|
|
||
| if (!eggheadUser || !eggheadUser.id) { | ||
| eggheadUser = await step.run('create egghead user', async () => { | ||
| await createEggheadUser(acceptedEmail) | ||
| return await getEggheadUserByEmail(acceptedEmail) | ||
| }) | ||
| } | ||
|
|
||
| if (!eggheadUser && !eggheadUser.id) { | ||
| throw new NonRetriableError('Failed to find/create egghead user') | ||
| } |
There was a problem hiding this comment.
check for existing egghead user, if none then create one and get egghead user again
| `https://app.egghead.io/api/v1/users/${email}?by_email=true&support=true`, | ||
| { | ||
| headers: { | ||
| Authorization: `Bearer ${process.env.EGGHEAD_ADMIN_TOKEN}`, |
There was a problem hiding this comment.
getting egghead user in this flow requires admin token because there is no user to auth with
| export async function addInstructorRoleToEggheadUser({ | ||
| userId, | ||
| }: { | ||
| userId: string | ||
| }) { | ||
| const instructorRoleQuery = ` | ||
| INSERT INTO users_roles (user_id, role_id) | ||
| VALUES ($1, $2) | ||
| ` | ||
|
|
||
| await eggheadPgQuery(instructorRoleQuery, [userId, 8]) | ||
| } |
There was a problem hiding this comment.
instructor role is added to egghead user object through join table
| export async function addRevenueSplitToEggheadInstructor({ | ||
| eggheadInstructorId, | ||
| }: { | ||
| eggheadInstructorId: string | ||
| }) { | ||
| const revenueSplitQuery = ` | ||
| INSERT INTO instructor_revenue_splits ( | ||
| instructor_id, | ||
| credit_to_instructor_id, | ||
| percentage, | ||
| from_date | ||
| ) VALUES ( | ||
| $1, | ||
| $2, | ||
| $3, | ||
| $4 | ||
| ); | ||
| ` | ||
|
|
||
| await eggheadPgQuery(revenueSplitQuery, [ | ||
| eggheadInstructorId, | ||
| null, | ||
| 0.2, | ||
| new Date(), | ||
| ]) | ||
| } |
There was a problem hiding this comment.
revenue split is also created when an instructor object is initially created
| const emailHtml = render(Component(componentProps)) | ||
| const emailHtml = await render(Component(componentProps)) |
There was a problem hiding this comment.
sending emails in course builder wasn't working with the error below. The render was returning a promise so we need to await it to get the actual html string
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
apps/egghead/src/lib/egghead/instructor.ts (4)
11-22: Add error handling and duplicate prevention to role assignmentThis function executes a database query without any error handling. Consider adding try/catch blocks and checking if the role already exists to prevent duplicate entries.
export async function addInstructorRoleToEggheadUser({ userId, }: { userId: string }) { + try { + // Check if the role already exists for this user to prevent duplicates + const checkRoleQuery = ` + SELECT id FROM users_roles WHERE user_id = $1 AND role_id = $2 + ` + const existingRole = await eggheadPgQuery(checkRoleQuery, [userId, INSTRUCTOR_ROLE_ID]) + + if (existingRole.rows.length > 0) { + console.log(`User ${userId} already has instructor role`) + return + } + const instructorRoleQuery = ` INSERT INTO users_roles (user_id, role_id) VALUES ($1, $2) ` await eggheadPgQuery(instructorRoleQuery, [userId, INSTRUCTOR_ROLE_ID]) + } catch (error) { + console.error(`Failed to add instructor role to user ${userId}:`, error) + throw new Error(`Failed to add instructor role: ${(error as Error).message}`) + } }
99-124: Add error handling to revenue split creationSimilar to other functions, this function lacks error handling for database operations.
export async function addRevenueSplitToEggheadInstructor({ eggheadInstructorId, }: { eggheadInstructorId: string }) { + try { const revenueSplitQuery = ` INSERT INTO instructor_revenue_splits ( instructor_id, credit_to_instructor_id, percentage, from_date ) VALUES ( $1, $2, $3, $4 ); ` await eggheadPgQuery(revenueSplitQuery, [ eggheadInstructorId, null, 0.2, new Date(), ]) + } catch (error) { + console.error(`Failed to add revenue split for instructor ${eggheadInstructorId}:`, error) + throw new Error(`Failed to add revenue split: ${(error as Error).message}`) + } }
118-123: Consider making the revenue split percentage configurableThe revenue split percentage is hardcoded to 0.2 (20%). Consider making this configurable or at least defining it as a constant at the top of the file.
+ // Define constants at the top of the file + const DEFAULT_INSTRUCTOR_REVENUE_PERCENTAGE = 0.2 // 20% // ... export async function addRevenueSplitToEggheadInstructor({ eggheadInstructorId, + percentage = DEFAULT_INSTRUCTOR_REVENUE_PERCENTAGE, }: { eggheadInstructorId: string + percentage?: number }) { // ... await eggheadPgQuery(revenueSplitQuery, [ eggheadInstructorId, null, - 0.2, + percentage, new Date(), ])
36-54: Consider input validation for required fieldsThere's no validation for the input parameters, which could lead to unexpected behavior if some fields are empty or in incorrect formats.
export async function createEggheadInstructor({ userId, email, firstName, lastName, twitter, website, bio, profileImageUrl, }: { userId: string firstName: string lastName: string email: string twitter: string website: string bio: string profileImageUrl: string }) { + // Basic validation for required fields + if (!userId || !email || !firstName || !lastName) { + throw new Error('Missing required fields: userId, email, firstName, and lastName are required') + } + + // Validate email format + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ + if (!emailRegex.test(email)) { + throw new Error('Invalid email format') + } // add instructor to egghead user
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/egghead/src/lib/egghead/instructor.ts(1 hunks)
🔇 Additional comments (2)
apps/egghead/src/lib/egghead/instructor.ts (2)
69-73: Improve slug generation to avoid collisionsThe current slug generation is too simple and could easily lead to collisions for instructors with the same name.
+ import { nanoid } from 'nanoid'; // Or another UUID library + // Or implement a more robust slug generation function + function generateUniqueSlug(firstName: string, lastName: string): string { + const base = `${firstName.toLowerCase()}-${lastName.toLowerCase()}`; + const uniqueId = nanoid(6); // Generate a short unique ID + return `${base}-${uniqueId}`; + } const values = [ firstName, lastName, - `${firstName}-${lastName}`, + generateUniqueSlug(firstName, lastName), userId,
82-93: Add error handling for database operationsThe database query doesn't have any error handling, which could lead to uncaught exceptions.
const placeholders = columns.map((_, index) => `$${index + 1}`).join(', ') const query = ` INSERT INTO instructors (${columns.join(', ')}) VALUES (${placeholders}) RETURNING id ` + try { const eggheadInstructorResult = await eggheadPgQuery(query, values) + + if (!eggheadInstructorResult.rows || eggheadInstructorResult.rows.length === 0) { + throw new Error('Failed to create instructor: No ID returned') + } return eggheadInstructorResult.rows[0].id + } catch (error) { + console.error('Error creating instructor in database:', error) + throw new Error(`Failed to create instructor: ${(error as Error).message}`) + }
| 'use server' | ||
|
|
||
| import { eggheadPgQuery } from '@/db/eggheadPostgres' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using a database transaction for related operations
The instructor creation process involves multiple operations (creating instructor, adding role, adding revenue split) that should ideally be handled within a transaction to ensure data consistency.
Consider creating a higher-level function that uses transactions to handle the entire instructor creation process:
export async function createInstructorWithRoleAndRevenueSplit(params) {
// Start a transaction
const client = await eggheadPgPool.connect()
try {
await client.query('BEGIN')
// Create instructor
const instructorId = await createEggheadInstructor(params, client)
// Add role
await addInstructorRoleToEggheadUser({userId: params.userId}, client)
// Add revenue split
await addRevenueSplitToEggheadInstructor({eggheadInstructorId: instructorId}, client)
await client.query('COMMIT')
return instructorId
} catch (error) {
await client.query('ROLLBACK')
throw error
} finally {
client.release()
}
}This would require modifying the existing functions to accept an optional client parameter for transaction support.
This implements the core instructor invite flow letting a person accept an invite with their email and fill out profile information
Step 0: receive email invite
Step 1: Accept invite with desired email
/invites/:inviteIdStep 2: fill out instructor profile information
/invites/:inviteId/onboardingStep 3: profile is created via inngest flow
/invites/:inviteId/onboarding/completedStep 4: prompted to login
/invites/:inviteId/onboarding/completedSummary by CodeRabbit