Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/components/cards/LearningModule.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { LearningModule } from "@/types/course";
/**
* Component that displays related learning material with a title, description, and "Start now" button.
*/
export function LearningModuleCard({ data }: { data: LearningModule }): JSX.Element {
export function LearningModuleCard({ data, url }: { data: LearningModule, url: string }): JSX.Element {
const { t } = useTranslation()
const { challenge, community, colors } = useMultiSelector<any, any>({
challenge: (state: IRootState) => state.challenges.current,
Expand Down Expand Up @@ -66,7 +66,7 @@ export function LearningModuleCard({ data }: { data: LearningModule }): JSX.Elem
<></>}

<div className="w-full mb-0 justify-self-end">
<Link href={`/communities/${community.slug}/challenges/${challenge?.id}/learning-modules/${data.id}`}>
<Link href={url}>
<ArrowButton communityStyles={true} variant="outline-primary">
{t("communities.overview.challenge.learning.start")}
</ArrowButton>
Expand Down
8 changes: 6 additions & 2 deletions src/components/sections/challenges/Learning.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Accordion from "@/components/ui/accordion/Accordion";
import Section from "@/components/sections/communities/_partials/Section";
import { LearningModuleCard } from "@/components/cards/LearningModule";
import CourseCard from "@/components/cards/course";
import { Course, LearningModule } from "@/types/course";
import { Challenge } from "@/types/course";
import { Community } from "@/types/community";
import { useTranslation } from "next-i18next";

Expand All @@ -11,8 +11,11 @@ import { useTranslation } from "next-i18next";
*
* @returns {JSX.Element} The Learning component JSX element.
*/
export default function Learning({ courses, learningModules, community }: { courses: Course[]; learningModules: LearningModule[]; community: Community }): JSX.Element {
export default function Learning({ challenge, community }: { challenge: Challenge; community: Community }): JSX.Element {
const { t } = useTranslation();
const courses = challenge?.courses
const learningModules = challenge?.learningModules
Comment on lines +16 to +17
Copy link

Choose a reason for hiding this comment

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

Logic change approved; handle potential null cases.

The logic to derive courses and learningModules from the challenge prop is correct. Consider adding checks or default values to handle cases where challenge might be null or undefined to prevent runtime errors.

-  const courses = challenge?.courses
-  const learningModules = challenge?.learningModules
+  const courses = challenge?.courses || []
+  const learningModules = challenge?.learningModules || []
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.

Suggested change
const courses = challenge?.courses
const learningModules = challenge?.learningModules
const courses = challenge?.courses || []
const learningModules = challenge?.learningModules || []


return (
<Section>
<Accordion
Expand Down Expand Up @@ -40,6 +43,7 @@ export default function Learning({ courses, learningModules, community }: { cour
<LearningModuleCard
key={`related-learning-card-${learning.id}`}
data={learning}
url={`/communities/${community.slug}/challenges/${challenge?.id}/learning-modules/${learning.id}`}
/>
))}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ export default function LearningMaterialsOverview() {
return <LearningModuleCard
key={`related-learning-card-${module.id}`}
data={module}
url={`/communities/${community.slug}/learning-modules/${module.id}`}
/>
})}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export default function ChallengePage() {
<Rewards />
<Objectives />
{challenge.isTeamChallenge && <TeamChallenge />}
<Learning courses={challenge.courses} learningModules={challenge.learningModules} community={community} />
<Learning challenge={challenge} community={community} />
<RatingRubric ratingCriteria={challenge?.ratingCriteria} selected={[]} />
<BestSubmissions />

Expand Down
81 changes: 81 additions & 0 deletions src/pages/communities/[slug]/learning-modules/[id].tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import DefaultLayout from "@/components/layout/Default";
import PageNavigation from "@/components/sections/courses/PageNavigation";
import Wrapper from "@/components/sections/courses/Wrapper";
import LearningModuleSection from "@/components/sections/learning-modules";
import Header from "@/components/sections/learning-modules/Header";
import useNavigation from "@/hooks/useNavigation";
import { useDispatch } from "@/hooks/useTypedDispatch";
import { useMultiSelector } from "@/hooks/useTypedSelector";
import { IRootState, wrapper } from "@/store";
import { initLearningModuleNavigationMenu } from "@/store/feature/communities/navigation.slice";
import { fetchCurrentCommunity } from "@/store/services/community.service";
import { findLearningModule } from "@/store/services/learningModules.service";
import { LearningModule } from "@/types/course";
import { NotFoundError } from "@/utilities/errors/NotFoundError";
import { serverSideTranslations } from "next-i18next/serverSideTranslations";
import { useRouter } from "next/router";
import { ReactElement, useEffect, useMemo } from "react";

interface LearningModuleMultiselector {
learningModule: LearningModule,
loading: boolean
}

export default function LearningModulePage(): ReactElement {
const dispatch = useDispatch();
const navigation = useNavigation()
const { learningModule } = useMultiSelector<unknown, LearningModuleMultiselector>({
learningModule: (state: IRootState) => state.learningModules.current,
loading: (state: IRootState) => state.learningModules.loading
})
Comment on lines +19 to +30
Copy link

Choose a reason for hiding this comment

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

Review of Interface and MultiSelector Usage

The LearningModuleMultiselector interface is well-defined, providing clear typing for the state management. The use of useMultiSelector is appropriate for selecting multiple pieces of state. However, ensure that the generic type <unknown> is replaced with a more specific type if possible to enhance type safety.

Consider specifying a more accurate type than unknown for better type safety and clarity in the useMultiSelector hook usage.


const router = useRouter()
const { query, locale } = router
const paths = useMemo(() => [learningModule?.title], [learningModule?.title]);

useEffect(() => {
dispatch(initLearningModuleNavigationMenu(navigation.community));
dispatch(findLearningModule({ id: query?.id as string, locale }))
}, [dispatch, locale]);
Comment on lines +32 to +39
Copy link

Choose a reason for hiding this comment

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

Review of useEffect Hook Implementation

The useEffect hook is used effectively to dispatch actions for initializing navigation and fetching the learning module based on the router's query parameters. This is a critical part of the page's functionality, ensuring that the necessary data is loaded when the component mounts. However, the dependency array includes locale but not navigation.community or query.id, which might lead to bugs if these values change and the component does not re-fetch the data.

Add navigation.community and query.id to the dependency array of the useEffect hook to ensure that the component reacts appropriately to changes in these values.


return (
<Wrapper paths={paths}>
<div className="py-8 flex flex-col space-y-8 text-gray-700">
<Header />
<div className="w-full">
<LearningModuleSection learningModule={learningModule} />
<PageNavigation />
</div>
</div>
</Wrapper>
)
Comment on lines +41 to +51
Copy link

Choose a reason for hiding this comment

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

Review of Component Rendering Logic

The rendering logic uses the Wrapper and LearningModuleSection components effectively, passing the necessary props. The use of dynamic paths for the Wrapper component based on the learning module's title is a good touch for dynamic navigation. However, ensure that there is error handling in case learningModule is null or undefined to prevent runtime errors.

Implement error handling for the learningModule prop to ensure the component handles cases where the learning module data is not available.

}

LearningModulePage.getLayout = function (page: ReactElement) {
return <DefaultLayout footerBackgroundColor={false}>{page}</DefaultLayout>;
};

export const getServerSideProps = wrapper.getServerSideProps((store) => async ({ params, locale }) => {

const learningModuleId = params?.id as string
try {
const communitySlug = params?.slug as string;
const [{ data: community }, { data: learningModule }, translations] = await Promise.all([
store.dispatch(fetchCurrentCommunity({ slug: communitySlug, locale })),
store.dispatch(findLearningModule({ id: learningModuleId, locale })),
serverSideTranslations(locale as string),
]);
if (!community || !learningModule) throw new NotFoundError();
return {
props: {
community,
learningModule,
...translations,
},
};
} catch (error) {
return {
notFound: true,
};
}
});
Comment on lines +58 to +81
Copy link

Choose a reason for hiding this comment

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

Review of getServerSideProps Function

The getServerSideProps function is well-implemented, fetching necessary data for the community and learning module based on URL parameters. The use of Promise.all for concurrent data fetching is efficient. However, the error handling could be improved by logging the error or sending it to a monitoring service to aid in debugging and maintaining the application.

Enhance error handling in the getServerSideProps function by logging the error or integrating with a monitoring service.

19 changes: 18 additions & 1 deletion src/store/feature/communities/navigation.slice.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createSlice, Dispatch, PayloadAction } from "@reduxjs/toolkit";
import { NextRouter } from "next/router";
import { Challenge, Course } from "@/types/course";
import { Challenge, Course, LearningModule } from "@/types/course";
import { Community } from "@/types/community";
import { List } from "@/utilities/CommunityNavigation";
import remarkParse from "remark-parse";
Expand Down Expand Up @@ -119,6 +119,23 @@ export const initCourseNavigationMenu = (navigation: any) => (dispatch: Dispatch
dispatch(setNavigationList(menus));
};


/**
* Iniate the learning module page navigation found at communities/:slug/learning-module/:id
*
* @param {*} navigation
* @returns {(dispatch: Dispatch, getState: any) => void}
*/
export const initLearningModuleNavigationMenu = (navigation: any) => (dispatch: Dispatch, getState: any) => {
dispatch(setNavigationList([]))
const community = getState().communities.current as Community;
const learningModule = getState().learningModules.current as LearningModule
const menus: List[] = navigation.initForLearningModule({
community,
learningModule
})
dispatch(setNavigationList(menus));
Comment on lines +129 to +137
Copy link

Choose a reason for hiding this comment

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

Suggest improvements for error handling and type safety.

The function initLearningModuleNavigationMenu effectively initializes the navigation for learning modules. However, it could be improved by:

  • Adding error handling for cases where community or learningModule might be undefined.
  • Ensuring type safety rather than using type assertions, which could lead to runtime errors if the assumptions are incorrect.

Consider adding checks and throwing appropriate errors if the required data is not available.

}
/**
* Hide navigation action
* @date 4/20/2023 - 4:09:46 PM
Expand Down
1 change: 1 addition & 0 deletions src/types/course.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ export type LearningModule = {
interactiveModules: InteractiveModule[];
courses: Course[],
level?: number,
challenges?: Challenge[]
};

export type InteractiveModule = {
Expand Down
86 changes: 85 additions & 1 deletion src/utilities/CommunityNavigation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { Community } from "@/types/community";
import { Challenge, Course } from "@/types/course";
import { Challenge, Course, LearningModule } from "@/types/course";
import Slugger from "github-slugger";

type QueryRoute = {
Expand Down Expand Up @@ -115,6 +115,18 @@ export default class CommunityNavigation {
return this.cleanupUrl(this.coursePath(`learning-modules/${path}`, courseSlug, communitySlug));
}


/**
* Path for a single learning module from the community found at communities/:slug/learning-module/:id
*
* @param {string} path
* @param {(string | undefined)} [communitySlug=this.params().slug]
* @returns {string}
*/
singleLearningModulePath(path: string, communitySlug: string | undefined = this.params().slug): string {
return this.cleanupUrl(this.communityPath(`learning-modules/${path}`, communitySlug))
}

/**
* Get the path for the learning module that if from the challenge
* @date 8/8/2023 - 12:09:57 PM
Expand Down Expand Up @@ -315,4 +327,76 @@ export default class CommunityNavigation {
}
return communityNavigationMenuList;
}

initForLearningModule({ community, learningModule }: { community: Community; learningModule: LearningModule }): List[] {

const learningModuleNavigationMenuList: List[] = [
{
id: "introduction",
title: "Introduction",
hideTitle: true,
items: [
{
label: "communities.navigation.overview",
exact: true,
link: "",
},
],
},
];

const modules = {
id: "learning-modules",
title: "communities.navigation.learning-modules",
hideTitle: false,
items: [{
id: learningModule.id,
label: `item ${learningModule.title}`,
link: this.singleLearningModulePath(learningModule.id, community?.slug),
exact: false,
subitems: []
}],
};

const challenges =
learningModule?.challenges?.map((challenge) => {
return {
id: challenge?.id,
label: challenge.name,
link: this.challengePath(challenge?.id, community.slug),
exact: false,
};
}) || [];

const courses = learningModule?.courses?.map((course) => {
return {
id: course?.id,
label: course.name,
link: this.coursePath("", course.slug, community?.slug),
exact: false,
};
}) || [];

learningModuleNavigationMenuList.push(modules)

// Add connected challenges
if (challenges.length) {
learningModuleNavigationMenuList.push({
id: "challenges",
title: "communities.navigation.challenge",
items: challenges,
});
}

// Add related courses
if (courses.length) {
learningModuleNavigationMenuList.push({
id: "courses",
title: "communities.overview.courses.title",
items: courses,
});
}

return learningModuleNavigationMenuList
}
Comment on lines +330 to +401
Copy link

Choose a reason for hiding this comment

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

Review of initForLearningModule method.

This method effectively initializes a navigation structure for a learning module, incorporating links to related challenges and courses. The use of singleLearningModulePath within this method ensures consistency in link generation.

However, there are a few areas for improvement:

  1. The method lacks detailed documentation, which is crucial for understanding its purpose and usage.
  2. The method constructs a navigation menu but does not handle potential null values gracefully in the destructuring of community and learningModule. This could lead to runtime errors if either is undefined.
  • Enhance the method documentation to clearly describe its functionality and parameters.
  • Implement null checks for community and learningModule to prevent potential runtime errors.
  /**
   * Initializes navigation for a specific learning module within a community.
   * Constructs a navigation menu with links to the learning module, associated challenges, and related courses.
   *
   * @param {{ community: Community; learningModule: LearningModule }} { community, learningModule } - The community and learning module for which to initialize navigation.
   * @returns {List[]} - A list of navigation items structured for the learning module page.
   */
  initForLearningModule({ community, learningModule }: { community: Community; learningModule: LearningModule }): List[] {
+   if (!community || !learningModule) {
+     throw new Error("Community and learning module must be provided.");
+   }
    const learningModuleNavigationMenuList: List[] = [
      {
        id: "introduction",
        title: "Introduction",
        hideTitle: true,
        items: [
          {
            label: "communities.navigation.overview",
            exact: true,
            link: "",
          },
        ],
      },
    ];
    // Remaining implementation...
  }
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.

Suggested change
initForLearningModule({ community, learningModule }: { community: Community; learningModule: LearningModule }): List[] {
const learningModuleNavigationMenuList: List[] = [
{
id: "introduction",
title: "Introduction",
hideTitle: true,
items: [
{
label: "communities.navigation.overview",
exact: true,
link: "",
},
],
},
];
const modules = {
id: "learning-modules",
title: "communities.navigation.learning-modules",
hideTitle: false,
items: [{
id: learningModule.id,
label: `item ${learningModule.title}`,
link: this.singleLearningModulePath(learningModule.id, community?.slug),
exact: false,
subitems: []
}],
};
const challenges =
learningModule?.challenges?.map((challenge) => {
return {
id: challenge?.id,
label: challenge.name,
link: this.challengePath(challenge?.id, community.slug),
exact: false,
};
}) || [];
const courses = learningModule?.courses?.map((course) => {
return {
id: course?.id,
label: course.name,
link: this.coursePath("", course.slug, community?.slug),
exact: false,
};
}) || [];
learningModuleNavigationMenuList.push(modules)
// Add connected challenges
if (challenges.length) {
learningModuleNavigationMenuList.push({
id: "challenges",
title: "communities.navigation.challenge",
items: challenges,
});
}
// Add related courses
if (courses.length) {
learningModuleNavigationMenuList.push({
id: "courses",
title: "communities.overview.courses.title",
items: courses,
});
}
return learningModuleNavigationMenuList
}
/**
* Initializes navigation for a specific learning module within a community.
* Constructs a navigation menu with links to the learning module, associated challenges, and related courses.
*
* @param {{ community: Community; learningModule: LearningModule }} { community, learningModule } - The community and learning module for which to initialize navigation.
* @returns {List[]} - A list of navigation items structured for the learning module page.
*/
initForLearningModule({ community, learningModule }: { community: Community; learningModule: LearningModule }): List[] {
if (!community || !learningModule) {
throw new Error("Community and learning module must be provided.");
}
const learningModuleNavigationMenuList: List[] = [
{
id: "introduction",
title: "Introduction",
hideTitle: true,
items: [
{
label: "communities.navigation.overview",
exact: true,
link: "",
},
],
},
];
const modules = {
id: "learning-modules",
title: "communities.navigation.learning-modules",
hideTitle: false,
items: [{
id: learningModule.id,
label: `item ${learningModule.title}`,
link: this.singleLearningModulePath(learningModule.id, community?.slug),
exact: false,
subitems: []
}],
};
const challenges =
learningModule?.challenges?.map((challenge) => {
return {
id: challenge?.id,
label: challenge.name,
link: this.challengePath(challenge?.id, community.slug),
exact: false,
};
}) || [];
const courses = learningModule?.courses?.map((course) => {
return {
id: course?.id,
label: course.name,
link: this.coursePath("", course.slug, community?.slug),
exact: false,
};
}) || [];
learningModuleNavigationMenuList.push(modules)
// Add connected challenges
if (challenges.length) {
learningModuleNavigationMenuList.push({
id: "challenges",
title: "communities.navigation.challenge",
items: challenges,
});
}
// Add related courses
if (courses.length) {
learningModuleNavigationMenuList.push({
id: "courses",
title: "communities.overview.courses.title",
items: courses,
});
}
return learningModuleNavigationMenuList
}

}