580-feat: Dedicated mentorship page #609
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (11)
src/core/services/api/index.ts (1)
12-12: LGTM! Consider adding a comment.The new mentorship entry follows the established pattern. Consider documenting its purpose and expected data structure.
+ // Holds mentorship program data and resources mentorship: [],src/views/mentorship/mentorship-courses-loader.ts (1)
1-6: Consider using a more specific import pathThe 'data' import path is quite generic. Consider using a more specific path that clearly indicates the source of these mentorship-related constants and types.
src/app/mentorship/page.tsx (1)
5-9: Consider adding more metadata fields for SEOWhile the title is set correctly, consider adding description, keywords, and OpenGraph metadata for better SEO optimization.
export async function generateMetadata(): Promise<Metadata> { const title = `Mentorship · The Rolling Scopes School`; + const description = 'Learn about our mentorship program at The Rolling Scopes School'; - return { title }; + return { + title, + description, + openGraph: { + title, + description, + }, + }; }src/app/mentorship/courses/page.tsx (2)
1-3: Use consistent import paths.Consider using the same import pattern (
@/) for the data import as used for the views import.-import { mentorshipCourses } from 'data'; +import { mentorshipCourses } from '@/data';
7-11: Enhance SEO with additional metadata.Consider adding more metadata fields such as description, keywords, and OpenGraph tags for better SEO and social sharing.
export async function generateMetadata(): Promise<Metadata> { const title = `Mentorship · The Rolling Scopes School`; - return { title }; + return { + title, + description: 'Learn about our mentorship program and how to get involved', + openGraph: { + title, + description: 'Learn about our mentorship program and how to get involved', + }, + }; }src/views/javascript-en.tsx (1)
30-30: Consider using courseName prop for pathInstead of hardcoding "javascript", consider using the courseName prop to maintain consistency and reduce potential maintenance issues.
- <MemberActivity path="javascript" type="marked" /> + <MemberActivity path={courseName.toLowerCase()} type="marked" />src/views/mentorship/mentorship.tsx (3)
11-11: Consider moving hardcoded path to constantsThe
studyPathNameshould be moved to the shared constants file for consistency and maintainability.-const studyPathName = 'mentorship';Add to
@/shared/constants.ts:export const STUDY_PATHS = { MENTORSHIP: 'mentorship' } as const;
13-15: Add JSDoc documentation for the props typeAdding documentation will improve code maintainability and IDE support.
+/** + * Props for the Mentorship component + * @property {MentorshipCourse} mentorshipData - Data containing mentorship course details + */ type MentorshipProps = { mentorshipData: MentorshipCourse; };
22-40: Reduce prop drilling for languageThe
langprop is passed to multiple components. Consider using React Context for shared values.// Create a new context const LanguageContext = React.createContext<string | undefined>(undefined); // Wrap the component tree export const Mentorship = ({ mentorshipData }: MentorshipProps) => { return ( <LanguageContext.Provider value={mentorshipData.lang}> {/* Child components can now use useContext(LanguageContext) */} </LanguageContext.Provider> ); };src/core/styles/_constants.scss (1)
40-42: LGTM! Consider adding a comment block for mentorship colors.The new color variables follow the project's naming and value format conventions. They're appropriately placed in the blue colors section.
Consider adding a descriptive comment block for better organization:
// Blue colors +// Mentorship theme colors $color-mentorship-banner: hsl(198deg 83% 70%); // #73ccf2 $color-mentorship-header: hsl(198deg 86% 55%); // #2BB4EFsrc/core/base-layout/components/header/header.tsx (1)
47-52: Simplify the header color logic.The current implementation with if-block is unnecessarily verbose. The usePathname hook always returns a string, making the null check redundant.
- // const headerAccentColor = pathname.includes(ROUTES.MENTORSHIP) ? 'blue' : 'gray'; - let headerAccentColor = 'gray'; - - if (pathname) { - headerAccentColor = pathname.includes(ROUTES.MENTORSHIP) ? 'blue' : 'gray'; - } + const headerAccentColor = pathname.includes(ROUTES.MENTORSHIP) ? 'blue' : 'gray';
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (41)
dev-data/about-course.data.tsx(3 hunks)dev-data/communication.data.ts(1 hunks)dev-data/contribute-options.data.ts(2 hunks)dev-data/courses-data.types.ts(1 hunks)dev-data/hero-page.data.ts(2 hunks)dev-data/index.ts(2 hunks)dev-data/mentors-register.data.ts(1 hunks)dev-data/mentorship-data.types.ts(1 hunks)dev-data/mentorship.data.ts(1 hunks)src/app/mentorship/courses/page.tsx(1 hunks)src/app/mentorship/page.tsx(1 hunks)src/core/base-layout/components/header/header.module.scss(1 hunks)src/core/base-layout/components/header/header.test.tsx(1 hunks)src/core/base-layout/components/header/header.tsx(5 hunks)src/core/const/index.ts(1 hunks)src/core/services/api/index.ts(1 hunks)src/core/styles/_constants.scss(1 hunks)src/shared/constants.ts(1 hunks)src/shared/icons/discord-icon.tsx(1 hunks)src/shared/types.ts(1 hunks)src/views/angular.tsx(2 hunks)src/views/aws-developer.tsx(2 hunks)src/views/courses.tsx(2 hunks)src/views/home.tsx(2 hunks)src/views/javascript-en.tsx(2 hunks)src/views/javascript-ru.tsx(2 hunks)src/views/mentorship/mentorship-courses-loader.ts(1 hunks)src/views/mentorship/mentorship.tsx(1 hunks)src/widgets/about-mentors/ui/about-mentors.tsx(1 hunks)src/widgets/benefits/ui/benefit-item/benefit-item.tsx(1 hunks)src/widgets/hero-page/ui/hero-page.tsx(2 hunks)src/widgets/member-activity/member-activity.test.tsx(5 hunks)src/widgets/member-activity/ui/member-activity.tsx(2 hunks)src/widgets/mentors-docs/ui/mentors-docs/mentors-docs.tsx(1 hunks)src/widgets/mentors-register/ui/mentors-register.tsx(1 hunks)src/widgets/mentors-wanted/ui/mentors-wanted.test.tsx(2 hunks)src/widgets/mentors-wanted/ui/mentors-wanted.tsx(2 hunks)src/widgets/mobile-view/ui/mobile-view.tsx(1 hunks)src/widgets/school-menu/school-menu.test.tsx(1 hunks)src/widgets/school-menu/ui/school-item/school-item.tsx(1 hunks)src/widgets/school-menu/ui/school-menu.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (22)
- dev-data/communication.data.ts
- dev-data/contribute-options.data.ts
- dev-data/courses-data.types.ts
- dev-data/hero-page.data.ts
- dev-data/index.ts
- dev-data/mentors-register.data.ts
- dev-data/mentorship-data.types.ts
- dev-data/mentorship.data.ts
- src/shared/constants.ts
- src/shared/icons/discord-icon.tsx
- src/shared/types.ts
- src/widgets/about-mentors/ui/about-mentors.tsx
- src/widgets/benefits/ui/benefit-item/benefit-item.tsx
- src/widgets/hero-page/ui/hero-page.tsx
- src/widgets/member-activity/member-activity.test.tsx
- src/widgets/mentors-docs/ui/mentors-docs/mentors-docs.tsx
- src/widgets/mentors-register/ui/mentors-register.tsx
- src/widgets/mentors-wanted/ui/mentors-wanted.test.tsx
- src/widgets/mentors-wanted/ui/mentors-wanted.tsx
- src/widgets/mobile-view/ui/mobile-view.tsx
- src/widgets/school-menu/ui/school-item/school-item.tsx
- src/widgets/school-menu/ui/school-menu.tsx
🧰 Additional context used
🪛 GitHub Check: CI
dev-data/about-course.data.tsx
[warning] 1-1:
Reaching to "@/core/const" is not allowed
🔇 Additional comments (25)
src/core/services/api/index.ts (1)
12-12: Verify DataMap type definition.
Let's ensure the DataMap type includes the new mentorship property.
✅ Verification successful
DataMap type definition is correctly defined with mentorship property
The DataMap type in dev-data/courses-data.types.ts already includes the mentorship property of type MentorActivities[].
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for DataMap type definition
ast-grep --pattern 'type DataMap = {
$$$
}'
Length of output: 569
src/app/mentorship/page.tsx (2)
11-13: LGTM!
The component implementation is clean and follows React best practices.
1-3: Verify the 'data' import path configuration
The import from 'data' uses a shorthand path. Ensure that this path is properly configured in your tsconfig.json or build configuration.
✅ Verification successful
Import path 'data' is correctly configured
The import path is properly set up in tsconfig.json with an alias mapping "data" to "../dev-data". The mentorshipCoursesDefault is exported from dev-data/mentorship.data.ts and re-exported through dev-data/index.ts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the import path is configured
rg -l "\"data\"" "tsconfig*.json"
Length of output: 88
Script:
#!/bin/bash
# Let's first check if there's a tsconfig.json file with any name pattern
fd -g "tsconfig*"
# Check for any path aliases or module configurations in package.json
fd "package.json" --exec cat {}
# Let's also check where this data module is actually located
fd "mentorshipCoursesDefault"
Length of output: 3144
Script:
#!/bin/bash
# Let's check the content of tsconfig.json for path configurations
cat tsconfig.json
# Let's also search for the data directory and its contents
fd -t d "data"
fd -t f -p "data/" "mentorshipCoursesDefault"
# Search for any file that exports mentorshipCoursesDefault
rg "export.*mentorshipCoursesDefault"
Length of output: 1551
src/views/courses.tsx (2)
7-7: LGTM!
Clean import statement following consistent pattern.
18-18: Verify MemberActivity component integration.
The component replacement looks good, but let's verify the integration since this is part of a broader change replacing StudyPath across multiple files.
src/views/home.tsx (2)
5-5: LGTM: Clean import addition
The Benefits import follows the established import organization pattern.
25-25: Verify Benefits component integration
The replacement of Mentoring with Benefits aligns with the PR's objective of restructuring mentorship content. Let's verify the component's implementation:
✅ Verification successful
Let me check the actual implementation of the Benefits component and its imports to ensure proper integration.
Benefits component is properly integrated
The Benefits component is correctly imported and used in home.tsx. It's a well-implemented component that:
- Uses default props from benefitMentorshipHome data
- Renders a list of benefits with proper accessibility attributes
- Supports flexible layouts through grid/flex options
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify Benefits component implementation and data usage
echo "Checking Benefits component implementation..."
ast-grep --pattern 'export const Benefits: FC = () => {
$$$
}'
echo "\nChecking benefitMentorshipHome data usage..."
rg -l "benefitMentorshipHome" --type ts
Length of output: 461
Script:
#!/bin/bash
# Check Benefits component implementation details
echo "Checking Benefits component in benefits.tsx..."
cat src/widgets/benefits/ui/benefits/benefits.tsx
echo "\nChecking home.tsx imports..."
rg -p "import.*Benefits" src/views/home.tsx
Length of output: 1712
src/core/const/index.ts (1)
32-32: LGTM! Verifying route usage.
The new MENTORSHIP route constant follows the established naming pattern and is well-positioned within the ROUTES object.
Let's verify the route usage across the codebase:
✅ Verification successful
Route constant is properly integrated across the codebase
The ROUTES.MENTORSHIP constant is correctly used throughout the application:
- Used in navigation components (header, mobile view)
- Integrated in mentorship-related views and components
- Properly referenced in route definitions and links
- No direct string usage of the route path found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of the new MENTORSHIP route
# Test: Search for direct string usage of 'mentorship'.
# Expect: Only usage through ROUTES.MENTORSHIP
rg -i "mentorship" --type ts --type tsx
# Test: Search for ROUTES.MENTORSHIP usage
# Expect: Consistent usage in components mentioned in the summary
rg "ROUTES\.MENTORSHIP" --type ts --type tsx
Length of output: 142
Script:
#!/bin/bash
# Search for mentorship usage without file type filters
rg -i "mentorship"
# Search for ROUTES.MENTORSHIP usage
rg "ROUTES\.MENTORSHIP"
# List files containing mentorship in their content
rg -l "mentorship"
Length of output: 15536
src/views/aws-developer.tsx (2)
7-7: LGTM!
The import statement follows the project's import pattern and maintains alphabetical ordering.
28-28: Consider using a typed constant for the path prop.
The hard-coded string "awsDev" could be error-prone. Consider using a typed constant from a centralized location to ensure consistency across views.
Let's check if other views use the same pattern:
src/views/angular.tsx (2)
1-1: Import changes look good!
The switch from LINKS to ROUTES and addition of MemberActivity import align well with the routing reorganization.
Also applies to: 9-9
31-31: Component updates look good, let's verify the routes.
The component changes align with the mentorship page requirements.
Let's verify the route construction:
Also applies to: 33-33
✅ Verification successful
Route construction is correctly implemented
The route construction /${ROUTES.MENTORSHIP}/${ROUTES.ANGULAR} is properly defined and consistently used:
- ROUTES.MENTORSHIP is defined as 'mentorship' in constants
- ROUTES.ANGULAR is defined as 'angular' in constants
- The route pattern is used consistently in both angular.tsx and mentorship.data.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify ROUTES constant usage and route construction
rg -A 1 "ROUTES\s*=\s*{"
rg "/${ROUTES.MENTORSHIP}/${ROUTES.ANGULAR}"
Length of output: 224
Script:
#!/bin/bash
# Search for ROUTES constant definition
rg "export const ROUTES" -A 5
# Search for route path usage in angular.tsx
rg -l "angular\.tsx$"
rg "path=\"angular\"" -A 2 -B 2
# Search for MemberActivity component definition and usage
ast-grep --pattern 'const MemberActivity = $_'
Length of output: 4467
Script:
#!/bin/bash
# Search for ANGULAR route constant and mentorship route usage
rg "ANGULAR\s*[:=]" -A 1
rg "MENTORSHIP\s*[:=]" -A 1
# Find where the mentorship route is used with angular
rg "MENTORSHIP.*ANGULAR"
Length of output: 670
src/views/javascript-en.tsx (2)
1-1: LGTM: Import statements are properly organized
The new imports for mentorship-related components follow the project's import conventions.
Also applies to: 9-10
32-32: LGTM: Proper route construction
The MentorsWanted component correctly uses ROUTES constants for constructing the navigation path.
src/views/javascript-ru.tsx (1)
1-1: LGTM: Import changes align with mentorship page requirements
The new imports and routing constant usage are consistent with the PR's objective of creating a dedicated mentorship page.
Also applies to: 9-10
src/core/base-layout/components/header/header.module.scss (2)
19-21: Clean implementation of the new header style
The .blue modifier follows the established pattern and is well-integrated with existing styles.
19-21: Verify the color variable definition
Please ensure that $color-mentorship-header is defined in your color variables file.
✅ Verification successful
Color variable is properly defined and used
The $color-mentorship-header variable is defined in src/core/styles/_constants.scss with the value hsl(198deg 86% 55%). The implementation is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the color variable definition
rg '\$color-mentorship-header' --type scss
Length of output: 73
Script:
#!/bin/bash
# Search for the color variable definition using ripgrep without file type filter
rg '\$color-mentorship-header'
# Also search in common SASS/SCSS variable file locations
fd -e scss -e sass | grep -i 'var\|color\|theme'
Length of output: 279
src/core/base-layout/components/header/header.test.tsx (1)
38-38: LGTM: Updated header link count.
The expectation is correctly updated to 4 links to accommodate the new mentorship menu item.
src/core/base-layout/components/header/header.tsx (1)
33-37: LGTM: Navigation link addition follows consistent patterns.
The new Mentorship navigation item maintains consistency with existing navigation items in terms of structure and component usage.
src/widgets/member-activity/ui/member-activity.tsx (4)
17-18: LGTM: Well-typed interface extension.
The optional activities prop is properly typed and aligns with the mentorship feature requirements.
36-45: Consider moving mentorship data to a dedicated file.
The localized mentorship data should be moved to dev-data/mentorship.data.ts for better separation of concerns.
47-47: LGTM: Improved type safety for language context.
The context now uses the shared Language type, improving type consistency.
54-64: Simplify the routing and content logic.
The current implementation has multiple conditional assignments that could be simplified using a mapping object, as suggested in the previous review.
dev-data/about-course.data.tsx (2)
43-43: LGTM! Verify route constants are defined.
The link updates correctly implement the new routing structure for mentorship pages.
#!/bin/bash
# Verify ROUTES.MENTORSHIP, ROUTES.JS, and ROUTES.JS_RU are defined
rg -A 1 "export const ROUTES = {" | rg "MENTORSHIP|JS|JS_RU"Also applies to: 53-53
1-1: Verify import path configuration.
The static analysis tool flags the import path "@/core/const". While the change from ANCHORS to ROUTES is correct, ensure the path alias "@" is properly configured in your build setup.
✅ Verification successful
Import path configuration is correct
The @ alias is properly configured in tsconfig.json under the paths section, mapping @/* to src/*. The import of ROUTES from @/core/const is valid and follows the project's configuration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if tsconfig.json or other config files define the "@" alias
fd -t f -e json -e js "tsconfig|webpack|vite" -x cat {} \; | grep -A 5 "paths\|alias"
Length of output: 199
Script:
#!/bin/bash
# Search for ROUTES constant definition
rg -l "export.*ROUTES"
# Search for @ alias usage in imports
rg "@/" --type ts --type tsx
# Check for tsconfig.json existence and content
fd "tsconfig.json" -x cat {}
Length of output: 1373
🧰 Tools
🪛 GitHub Check: CI
[warning] 1-1:
Reaching to "@/core/const" is not allowed
|
Lighthouse Report:
|
|
Lighthouse Report:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
src/app/[...mentorship]/page.tsx (3)
10-14: Consider enhancing SEO metadata.Add description and other relevant meta tags for better SEO.
export async function generateMetadata(): Promise<Metadata> { const title = `Mentorship · The Rolling Scopes School`; - - return { title }; + return { + title, + description: 'Learn about our mentorship program, eligibility criteria, and how to become a mentor.', + openGraph: { + title, + description: 'Learn about our mentorship program, eligibility criteria, and how to become a mentor.', + }, + }; }
16-18: Resolve the 404 page handling issue.The TODO comment indicates that 404 page handling isn't working as expected. This should be addressed for proper error handling.
Would you like me to help implement proper 404 page handling for invalid routes?
20-29: Consider using a configuration object for routes.Extract routes to a configuration object for better maintainability.
+const MENTORSHIP_ROUTES = [ + 'mentorship', + 'reactjs', + 'angular', + 'javascript', + 'javascript-ru', +] as const; export async function generateStaticParams() { - return [ - { mentorship: ['mentorship'] }, - { mentorship: ['mentorship', 'reactjs'] }, - { mentorship: ['mentorship', 'angular'] }, - { mentorship: ['mentorship', 'javascript'] }, - { mentorship: ['mentorship', 'javascript-ru'] }, - ]; + return [ + { mentorship: ['mentorship'] }, + ...MENTORSHIP_ROUTES.slice(1).map(route => ({ + mentorship: ['mentorship', route] + })), + ]; }src/views/mentorship.tsx (2)
11-15: Consider a more specific constant name.The constant
studyPathNamecould be renamed toMENTORSHIP_PATHto better reflect its specific usage and follow the naming convention of constants.-const studyPathName = 'mentorship'; +const MENTORSHIP_PATH = 'mentorship';
17-43: Component structure looks good, but consider adding error handling.The component follows React best practices and properly passes props to child components. Consider adding error boundaries and loading states for better user experience.
export const Mentorship = ({ mentorshipData }: MentorshipProps) => { + if (!mentorshipData) { + return <div>Loading mentorship data...</div>; + } + return ( <> <HeroPage pageName={PAGE_NAMES.MENTORSHIP} />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
src/app/[...mentorship]/page.tsx(1 hunks)src/views/mentorship.tsx(1 hunks)
🔇 Additional comments (4)
src/app/[...mentorship]/page.tsx (2)
1-8: LGTM! Clean and well-organized imports.
31-33: LGTM! Type definition is clear and properly typed.
src/views/mentorship.tsx (2)
1-9: LGTM! Imports are well-organized and properly utilized.
27-27: Verify the Benefits component conditional rendering logic.
The Benefits component is shown when there's no title. Is this the intended behavior?
✅ Verification successful
Benefits component rendering logic is correct
The conditional rendering {!mentorshipData.title && <Benefits {...benefitMentorshipMentors} />} is intentional. The Benefits component is shown only on the general mentorship page (when there's no specific course title) to display mentor-specific benefits, while it's hidden when viewing specific mentorship courses. This pattern aligns with the page's component hierarchy where AboutMentors and MemberActivity components handle course-specific content.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of Benefits component usage
rg -A 5 "Benefits.*benefitMentorshipMentors"
Length of output: 414
Script:
#!/bin/bash
# Let's check the Benefits component implementation and its usage patterns
ast-grep --pattern 'const Benefits = $_'
# Also check for mentorshipData structure and usage
rg -A 5 "mentorshipData.*title"
Length of output: 3226
Script:
#!/bin/bash
# Let's check the full mentorship view implementation to understand the component structure
rg -A 10 "function Mentorship|const Mentorship"
# And check for any other Benefits component usage patterns
rg "Benefits.*benefit" -A 2
Length of output: 2851
|
Lighthouse Report:
|
|
Lighthouse Report:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/widgets/school-menu/school-menu.test.tsx (3)
8-46: LGTM! Consider enhancing mock data coverage.The mock data structure is well-defined and properly typed. However, consider adding test cases for different language combinations and enrollment states to improve test coverage.
const mockedCourse: Course[] = [ { // ... existing course language: ['ru'], + language: ['ru', 'en'], // Test multi-language support // ... rest of the properties }, { // ... second course + enroll: null, // Test course without enrollment // ... rest of the properties }, ];
Line range hint
61-89: Consolidate ESLint rule disabling.Instead of disabling the
vitest/no-commented-out-testsrule for each test, move it to the top of the describe block.describe('SchoolMenu', () => { + // eslint-disable-next-line vitest/no-commented-out-tests const [aws, react] = mockedCourse; - // eslint-disable-next-line vitest/no-commented-out-tests it('renders without crashing and displays "rs school" heading', () => { // ... test code }); - // eslint-disable-next-line vitest/no-commented-out-tests it('displays correct links and descriptions with "rs school" props', () => { // ... test code });
Line range hint
98-130: Add missing test cases for mentorship functionality.The test suite lacks coverage for mentorship-specific features mentioned in the PR objectives.
Add these test cases:
it('displays mentorship heading when "mentorship" is passed', () => { renderWithRouter(<SchoolMenu heading="mentorship" />); expect(screen.getByRole('heading', { name: /mentorship/i })).toBeInTheDocument(); }); it('renders correct mentorship enrollment links', () => { renderWithRouter(<SchoolMenu heading="mentorship" />); const enrollLinks = screen.getAllByRole('link'); enrollLinks.forEach((link) => { expect(link).toHaveAttribute('href', expect.stringContaining('wearecommunity.io')); }); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/widgets/school-menu/school-menu.test.tsx(6 hunks)
🔇 Additional comments (1)
src/widgets/school-menu/school-menu.test.tsx (1)
49-55: LGTM! Mock setup follows best practices.
The mock import setup correctly preserves actual imports while overriding the courses data.
|
Lighthouse Report:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/widgets/school-menu/school-menu.test.tsx (2)
8-46: Improve mock data structure.The mock data setup has a few areas for improvement:
- The
enrollURLs are hardcoded and duplicate across courses- The
[mentorshipId]placeholder in the React course title should be replaced with a meaningful valueConsider using constants for URLs and removing the placeholder:
- enroll: 'https://wearecommunity.io/events/js-stage0-rs-2024q2', + enroll: MOCK_ENROLL_URL, - title: 'React JS [mentorshipId]', + title: 'React JS',
Line range hint
57-124: Add test coverage for mentorship scenarios.While the current tests cover basic functionality, consider adding tests for:
- Mentorship-specific routing
- Error handling (e.g., missing course data)
- User interactions (e.g., click handlers)
Example test case:
it('redirects to mentorship page when mentorship link is clicked', () => { const { user } = renderWithRouter(<SchoolMenu heading="all courses" />); const mentorshipLink = screen.getByRole('link', { name: /React JS/i }); await user.click(mentorshipLink); expect(window.location.pathname).toBe('/mentorship'); });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/widgets/school-menu/school-menu.test.tsx(4 hunks)
🔇 Additional comments (1)
src/widgets/school-menu/school-menu.test.tsx (1)
49-55: LGTM! Clean mock setup.
The mock import setup follows best practices by preserving original imports and only overriding the necessary data.
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
In progress
Related Tickets & Documents
Screenshots, Recordings
Added/updated tests?
[optional] Are there any post deployment tasks we need to perform?
[optional] What gif best describes this PR or how it makes you feel?
Summary by CodeRabbit
Release Notes
New Features
DiscordIconcomponent to enhance social media integration.MemberActivitycomponent to display mentorship activities.Benefitscomponent highlighting the advantages of mentorship.Improvements
HeroPagecomponent to include mentorship-specific content.Bug Fixes
Tests