Skip to content
16 changes: 6 additions & 10 deletions app/(app)/feature-flag-example/_client.tsx
Original file line number Diff line number Diff line change
@@ -1,24 +1,20 @@
"use client";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole folder should have been left where it was an untouched. It is just to use as an example to show how to do feature flags.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was meant to be kept as an example. I don't think I was clear enough in the issue so that's my fault. But it is meant to be in app/(app)/courses so when a user visits /courses they can find this page.

import { type Session } from "next-auth";
import { notFound } from "next/navigation";
import { isFlagEnabled, FEATURE_FLAGS } from "@/utils/flags";
import CoursesLanding from "@/components/Course";

const Content = ({ session }: { session: Session | null }) => {
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.FEATURE_FLAG_TEST);
const flagEnabled = isFlagEnabled(FEATURE_FLAGS.COURSES_LANDING); // Adjust to the correct flag

if (!flagEnabled) {
notFound();
notFound(); // Show 404 page if the feature flag is not enabled
}

return (
<div className="mx-auto max-w-2xl">
<h1 className="text-lg">
This page is behind a feature flag. It will work in development or when
the flag is turned on.
</h1>
<p className="mt-8 text-sm">
{session ? "User is logged in" : "User is not logged in"}
</p>
<div className="mx-auto max-w-6xl">
<CoursesLanding session={session} />
</div>
);
};
Expand Down
6 changes: 3 additions & 3 deletions app/(app)/feature-flag-example/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import Content from "./_client";
import { getServerAuthSession } from "@/server/auth";

export const metadata = {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again this should be in the /courses path

title: "This is a feature flag example",
title: "Courses Landing Page",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Inconsistency between page title and file path

The metadata title has been updated to "Courses Landing Page", which aligns with the PR objectives. However, the file is still located in the feature-flag-example directory. This inconsistency could lead to confusion.

Consider moving this file to a more appropriate location, such as app/(app)/courses/page.tsx, to match its new purpose. If you decide to keep it in the current location, please provide a comment explaining why it remains in the feature flag example directory.

};

export default async function Page() {
// Example of grabbing session in case it is needed
export default async function CoursesPage() {
// Get session if needed for authentication purposes
const session = await getServerAuthSession();

return <Content session={session} />;
Expand Down
42 changes: 42 additions & 0 deletions components/Course/CourseCard.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@

import React from "react";
import { Course } from "./type";

interface CourseCardProps {
course: Course;
onStartCourse: (courseId: number) => void; // Change to number to match your mock data
}

const CourseCard: React.FC<CourseCardProps> = ({ course, onStartCourse }) => {
return (
<div
className={`rounded-lg p-6 shadow-md ${course.featured ? "bg-orange-100 dark:bg-orange-900" : "bg-gray-100 dark:bg-gray-800"}`}
>
{/* Course Title */}
<h2 className="mb-4 text-2xl font-bold">{course.title}</h2>

{/* Course Description */}
<p className="mb-4 text-lg text-gray-700 dark:text-gray-300">
{course.description}
</p>

{/* Course Progress */}
<div className="flex items-center gap-4">
<p className="text-lg">Progress:</p>
<div className="flex h-16 w-16 items-center justify-center rounded-full bg-pink-500 text-lg font-bold">
{course.progress}%
</div>
</div>

{/* Start Course Button */}
<button
className="mt-4 rounded bg-orange-500 px-4 py-2 text-white hover:bg-orange-600"
onClick={() => onStartCourse(Number(course.id))} // Call the passed function with course ID
>
Start Course
</button>
</div>
);
};

export default CourseCard;
39 changes: 39 additions & 0 deletions components/Course/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@

import React from "react";
import { useRouter } from "next/router";
import { mockCourses } from "./mock";
import CourseCard from "./CourseCard";
import { type Session } from "next-auth";

const CoursesLanding = ({ session }: { session: Session | null }) => {
const router = useRouter(); // Initialize the useRouter hook

const handleStartCourse = (courseId: number) => {
// Navigate to the course content page
router.push(`/courses/${courseId}`);

};

return (
<div className="flex min-h-screen flex-col gap-8 px-4 py-8 dark:bg-gray-900 dark:text-white lg:px-16 lg:py-12">
{/* Page Title */}
<h1 className="mb-6 text-3xl font-bold lg:text-4xl">Courses</h1>

{/* Courses List with aria-label for accessibility */}
<div
className="grid grid-cols-1 gap-8 lg:grid-cols-2"
aria-label="List of available courses"
>
{mockCourses.map((course) => (
<CourseCard
key={course.id}
course={course}
onStartCourse={handleStartCourse}
/>
))}
</div>
</div>
);
};

export default CoursesLanding;
61 changes: 61 additions & 0 deletions components/Course/mock.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Course } from "./type";

// Define the UserProgress type
type UserProgress = {
coursesProgress: Array<{
courseId: number; // Use string for the ID
progress: number; // Progress percentage
featured: boolean; // Include the featured status if needed
}>;
};
Comment on lines +4 to +10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance the UserProgress type definition

While the UserProgress type provides a good structure, consider the following improvements:

  1. Use more specific types for progress and featured:

    • progress could be constrained to a range (0-100).
    • featured could use a literal type.
  2. Consider adding optional fields that might be useful in the future, such as lastAccessedDate or completionDate.

Here's a suggested improvement:

type UserProgress = {
  coursesProgress: Array<{
    courseId: string; // Assuming this will be changed as per previous comment
    progress: number & { __brand: 'Percent' }; // Custom type to ensure 0-100 range
    featured: boolean;
    lastAccessedDate?: Date;
    completionDate?: Date;
  }>;
};

// Helper function to ensure progress is within 0-100 range
function createPercent(n: number): number & { __brand: 'Percent' } {
  if (n < 0 || n > 100) throw new Error('Progress must be between 0 and 100');
  return n as number & { __brand: 'Percent' };
}

This enhancement improves type safety and future-proofs the type definition.


// Mock courses data
export const mockCourses: Course[] = [
{
id: 1, // Changed to string
title: 'Introduction to Web Development',
description: 'Learn the basics of HTML, CSS, and JavaScript to build responsive websites.',
progress: 90,
featured: true,
},
{
id: 2,
title: 'Advanced JavaScript Concepts',
description: 'Deep dive into JavaScript with a focus on ES6+ features and asynchronous programming.',
progress: 65,
featured: false,
},
{
id: 3,
title: 'React for Beginners',
description: 'Understand the fundamentals of React and how to build interactive UIs.',
progress: 30,
featured: true,
},
{
id: 4,
title: 'Full-Stack Web Development',
description: 'Learn to build full-stack applications using modern technologies like Node.js and Express.',
progress: 45,
featured: false,
},
{
id: 5,
title: 'Version Control with Git and GitHub',
description: 'Master version control using Git and learn how to collaborate on GitHub.',
progress: 80,
featured: true,
},
];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Correct the id type and consider adding a 0% progress course

  1. The id property for each course is currently a number, but the comment on line 15 suggests it should be a string. This inconsistency should be resolved.

  2. Consider adding a course with 0% progress to cover this edge case in the mock data.

Update the mockCourses array to use string IDs and add a 0% progress course:

export const mockCourses: Course[] = [
  {
-    id: 1, // Changed to string
+    id: '1',
    title: 'Introduction to Web Development',
    description: 'Learn the basics of HTML, CSS, and JavaScript to build responsive websites.',
    progress: 90,
    featured: true,
  },
  // ... (update other course IDs similarly)
+  {
+    id: '6',
+    title: 'Mobile App Development with React Native',
+    description: 'Learn to build cross-platform mobile apps using React Native.',
+    progress: 0,
+    featured: false,
+  },
];

Committable suggestion was skipped due to low confidence.


// Function to generate user progress
export const generateUserProgress = (courses: Course[]): UserProgress => ({
coursesProgress: courses.map(course => ({
courseId: course.id, // Use the course ID
progress: course.progress, // Use the course progress
featured: course.featured, // Include featured status if needed
})),
});

// Generate the user progress based on mockCourses
export const userProgress: UserProgress = generateUserProgress(mockCourses);
11 changes: 11 additions & 0 deletions components/Course/type.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@




export interface Course {
id: number;
title: string;
description?: string;
progress: number;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using a more specific type for the progress property

The progress property is currently typed as number, which allows for any numeric value. However, since this likely represents a percentage, it might be beneficial to use a more specific type or add a comment to clarify the expected range.

You could consider one of these options:

  1. Use a type alias with a comment:

    /** Percentage of course completion (0-100) */
    progress: number;
  2. Or, for stricter typing, create a custom type:

    type Percentage = number & { __brand: 'percentage' };
    // ...
    progress: Percentage;

    This would require a helper function to create valid percentages:

    function createPercentage(value: number): Percentage {
      if (value < 0 || value > 100) {
        throw new Error('Percentage must be between 0 and 100');
      }
      return value as Percentage;
    }

The choice depends on the level of type safety you want to enforce in your codebase.

featured: boolean;
}
6 changes: 3 additions & 3 deletions utils/flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { posthog } from "posthog-js";

export const FEATURE_FLAGS = {
FEATURE_FLAG_TEST: "feature-flag-test",
// Add more feature flags as needed
COURSES_LANDING: "courses-landing",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perfect! Thanks

} as const;

export type FeatureFlagName =
Expand All @@ -17,11 +17,11 @@ export function isDevEnvironment() {

export const isFlagEnabled = (
featureFlag: FeatureFlagName,
disableDevCheck = false, // Disable dev check to force feature flag to be checked always
disableDevCheck = false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add back in the comment here as it's a note to developer

) => {
if (!disableDevCheck && isDevEnvironment()) {
console.log("Feature flag check skipped in development environment");
return true;
return true; // Always true in dev environments unless you want to test differently
}
return posthog.isFeatureEnabled(featureFlag);
};