680-refactor: Lang props #681
Conversation
github-actions
bot
changed the title
|
run visual now |
|
Lighthouse Report:
|
|
run visual now |
KristiBo
requested review from
andron13,
dzmitry-varabei,
SpaNb4,
ansivgit and
natanchik
as code owners
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive refactoring of course-related components and data structures. The primary changes involve transforming the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
Documentation and Community
|
![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
🧹 Nitpick comments (11)
src/widgets/training-program/ui/training-program.tsx (1)
41-41: Language-based content caution.
Accessing "localizedContent[language]" may throw if "language" is unexpected. Provide a default label for safety.src/widgets/communication/ui/communication.tsx (1)
48-48: Validate language key.
Ensure "communicationText[language]" is safe to access. Provide a default to avoid undefined references.src/shared/helpers/get-course-language.ts (1)
1-9: Centralized language retrieval.
This helper succinctly encapsulates the logic for fetching a course's language. Consider a fallback if "selectCourse" fails.src/shared/__tests__/setup-tests.tsx (1)
29-30: Maintenance reminder for Image mock.
This minimal Image mock helps your tests run quickly, but remember to revisit if advanced image behavior is needed.src/views/angular.tsx (1)
30-30: Inline await usage
Calling "getCourseLanguage" inline is fine for a single usage. For multiple calls, consider caching or pre-fetching.src/widgets/mentors-wanted/ui/mentors-wanted.test.tsx (1)
11-14: Consider ensuring handling of async complexities in beforeEach
Having an async call in beforeEach is fine, but be aware it could lead to unhandled promise rejections if the component load fails. You may want to handle or log potential errors to keep tests robust.src/widgets/mentors-wanted/ui/mentors-wanted.tsx (1)
17-17: Prefer mandatory courseName if possible
If this component cannot function suitably without a valid courseName, consider making it a required prop to avoid null scenarios.src/widgets/communication/ui/communication.test.tsx (1)
7-16: Highlighted approach for language variants
Iterating over mockLangVariants is a neat, maintainable strategy. Keep in mind test coverage if new languages are added.src/widgets/hero-course/ui/hero-course.tsx (1)
38-43: Test presence of data-testid
Including data-testid="hero-image" helps ensure robust testing coverage.src/widgets/about-course/ui/about-course/about-course.test.tsx (1)
8-8: Handle the possibility of not finding a course
If no matching course is found, this line returns undefined. Consider gracefully handling a missing course scenario for improved test robustness.src/widgets/training-program/ui/training-program.test.tsx (1)
37-37: Hardcoded enroll URL.
Consider extracting "/enroll" into a constant to facilitate future changes and maintain consistency.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (46)
dev-data/courses.data.ts(9 hunks)dev-data/index.ts(1 hunks)src/app/courses/angular/page.tsx(1 hunks)src/app/courses/aws-cloud-developer/page.tsx(1 hunks)src/app/courses/aws-devops/page.tsx(1 hunks)src/app/courses/aws-fundamentals/page.tsx(1 hunks)src/app/courses/javascript-preschool-ru/page.tsx(1 hunks)src/app/courses/javascript-ru/page.tsx(1 hunks)src/app/courses/javascript/page.tsx(1 hunks)src/app/courses/nodejs/page.tsx(1 hunks)src/app/courses/reactjs/page.tsx(1 hunks)src/entities/course/types.ts(2 hunks)src/entities/course/ui/course-card/course-card.test.tsx(2 hunks)src/entities/course/ui/course-item/course-item.test.tsx(1 hunks)src/entities/course/ui/course-item/course-item.tsx(1 hunks)src/shared/__tests__/constants.ts(6 hunks)src/shared/__tests__/setup-tests.tsx(1 hunks)src/shared/helpers/get-course-language.ts(1 hunks)src/shared/helpers/getActualData.test.ts(3 hunks)src/shared/helpers/is-training-program.ts(1 hunks)src/shared/ui/date-lang/date-lang.test.tsx(2 hunks)src/shared/ui/date-lang/date-lang.tsx(2 hunks)src/views/angular.tsx(2 hunks)src/views/aws-developer.tsx(2 hunks)src/views/aws-devops.tsx(1 hunks)src/views/aws-fundamentals.tsx(1 hunks)src/views/javascript-en.tsx(2 hunks)src/views/javascript-preschool-ru.tsx(1 hunks)src/views/javascript-ru.tsx(2 hunks)src/views/nodejs.tsx(1 hunks)src/views/react.tsx(1 hunks)src/widgets/about-course/ui/about-course/about-course.test.tsx(3 hunks)src/widgets/about-course/ui/about-course/about-course.tsx(2 hunks)src/widgets/about-video/ui/about-video.tsx(1 hunks)src/widgets/communication/ui/communication.test.tsx(2 hunks)src/widgets/communication/ui/communication.tsx(3 hunks)src/widgets/hero-course/ui/hero-course.test.tsx(1 hunks)src/widgets/hero-course/ui/hero-course.tsx(3 hunks)src/widgets/member-activity/ui/member-activity.tsx(1 hunks)src/widgets/mentors-wanted/ui/mentors-wanted.test.tsx(1 hunks)src/widgets/mentors-wanted/ui/mentors-wanted.tsx(2 hunks)src/widgets/school-menu/school-menu.test.tsx(2 hunks)src/widgets/trainers/ui/trainers.test.tsx(3 hunks)src/widgets/trainers/ui/trainers.tsx(1 hunks)src/widgets/training-program/ui/training-program.test.tsx(4 hunks)src/widgets/training-program/ui/training-program.tsx(2 hunks)
🔇 Additional comments (122)
src/widgets/about-course/ui/about-course/about-course.tsx (1)
3-3: Good import usage.
Importing "selectCourse" aligns with the new approach of fetching data by name.
src/widgets/training-program/ui/training-program.tsx (4)
3-3: Solid separation of concerns.
Using a helper function for type checking keeps the logic reusable and clear.
7-7: Good data import.
Relying on centralized data ensures maintainability and consistency across the app.
12-13: Type definitions look consistent.
Switching to "CourseNamesKeys" matches the standardized approach for referencing courses.
27-27: Map existence check.
If "contentName" doesn't exist in "contentMap," consider a fallback or error message to prevent runtime errors.
src/widgets/communication/ui/communication.tsx (1)
4-4: Consistent data fetching approach.
Centralizing data retrieval with "selectCourse" maintains a uniform pattern across components.
src/shared/helpers/is-training-program.ts (1)
1-10: Straightforward type guard approach.
The function is concise. If new training program types arise, keep this array updated.
src/app/courses/reactjs/page.tsx (1)
13-13: Nicely Simplified.
Removing the course object dependency in favor of directly using the courseName prop streamlines the data flow. This approach minimizes overhead and improves clarity.
src/app/courses/nodejs/page.tsx (1)
13-13: Clear and Consistent Refactor.
Passing only courseName is consistent with the overall refactoring goals, reducing complexity and sticking to a single source of truth for the course data.
src/app/courses/angular/page.tsx (1)
13-13: Adheres to the New Data Flow.
By passing only the courseName prop, you maintain a straightforward interface for the Angular component. This refactor aligns with the removal of language-specific properties and improves maintainability.
src/app/courses/javascript-ru/page.tsx (1)
13-13: Streamlined Prop Usage.
Exposing only courseName ensures that the JavaScriptRu component determines its own data requirements, consistent with the rest of the refactor.
src/app/courses/javascript/page.tsx (1)
13-13: Transition to using only courseName looks good!
You are now passing just the courseName to the JavaScriptEn component. Please verify that this component no longer requires a full course object.
Use this script to confirm no references to a removed course prop exist in JavaScriptEn:
✅ Verification successful
Transition to courseName prop is correctly implemented
The JavaScriptEn component's type definition and implementation show it only expects and uses the courseName prop of type CourseNames['JS_EN']. No references to a removed course prop exist.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg -A 5 'JavaScriptEn'
Length of output: 1078
src/app/courses/aws-devops/page.tsx (1)
13-13: Consistent data flow for AwsDevOps component.
Passing only courseName parallels the PR objective. Double-check that AwsDevOps no longer relies on a course object.
Run this script to verify no references to the removed course prop remain:
✅ Verification successful
AwsDevOps component correctly uses courseName prop
The component only accepts and uses courseName prop, with no references to a course object. The implementation in src/views/aws-devops.tsx confirms this through its prop type definition and usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg -A 5 'AwsDevOps'
Length of output: 1075
src/app/courses/aws-cloud-developer/page.tsx (1)
13-13: Straightforward change for AwsDeveloper component.
The code now passes courseName instead of a course object, aligning with the refactor aim.
Use this script to check for references to a removed course prop:
✅ Verification successful
Verified: AwsDeveloper component correctly accepts courseName prop
The codebase shows the AwsDeveloper component is properly typed with courseName prop of type CourseNames['AWS_CLOUD_DEVELOPER'] and is used consistently across all references.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg -A 5 'AwsDeveloper'
Length of output: 1532
src/app/courses/aws-fundamentals/page.tsx (1)
13-13: Simplified prop usage in AwsFundamentals component.
Only courseName is passed, which is consistent with the streamlined approach.
Search for references to a removed course object to confirm no lingering dependencies:
✅ Verification successful
No lingering dependencies found after prop simplification
The codebase shows consistent usage of only the courseName prop for the AwsFundamentals component, with no references to a removed course object. The component's type definition and all its usages align with this simplified prop pattern.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
rg -A 5 'AwsFundamentals'
Length of output: 4730
src/app/courses/javascript-preschool-ru/page.tsx (1)
13-13: Ensure consistent naming across the codebase.
Since you have standardized to use only "courseName", verify that all related imports and references don't expect a "course" object.
✅ Verification successful
Let me verify this by searching for any references to both "courseName" and "course" objects in the codebase.
Let me try a different approach using ripgrep to search for these patterns.
No issues with using courseName prop
The codebase consistently uses courseName prop across all components and there are no instances where a course object is expected instead. The course object is only used internally after being retrieved via selectCourse(courseName) function.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for courseName prop usage
ast-grep --pattern 'courseName={$_}'
# Search for course prop usage
ast-grep --pattern 'course={$_}'
# Search for courseName parameter in function definitions
ast-grep --pattern 'function $_($$$courseName$$$) {'
# Search for course parameter in function definitions
ast-grep --pattern 'function $_($$$course$$$) {'
Length of output: 179
Script:
#!/bin/bash
# Search for courseName usage
rg "courseName" -A 2 -B 2
# Search for references to course object
rg "course\." -A 2 -B 2
# Search for course prop assignments
rg "course=" -A 2 -B 2
Length of output: 52092
src/views/react.tsx (2)
14-14: Refactor confirmed.
Removing "course" from props and only passing "courseName" simplifies data flow.
17-17: Keep an eye on null or invalid courseName.
Ensure that callers never pass an undefined or empty courseName, since these child components rely on it.
Also applies to: 19-19, 20-20, 23-23
✅ Verification successful
Let me verify how courseName is being used and validated in the codebase.
Let me check how selectCourse handles invalid course names.
Let me try a different approach to find the selectCourse implementation.
Let me check the type definition for CourseNamesKeys and how courses are fetched.
Validation for courseName is properly implemented
The codebase has proper type safety and error handling for courseName:
courseNameis typed asCourseNamesKeyswhich is a union type of valid course namesselectCoursefunction fetches data via API and throws an error if something goes wrong- TypeScript's type system ensures only valid course names can be passed at compile time
The concern about null/invalid courseNames is already addressed through the type system and error handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find the definition and usage of HeroCourse component
ast-grep --pattern 'const HeroCourse = $_'
# Find where courseName prop is used within components
ast-grep --pattern 'courseName={$_}'
# Find any prop type definitions or validation for courseName
rg "courseName.*Props" -A 2
Length of output: 7422
Script:
#!/bin/bash
# Find selectCourse implementation and error handling
ast-grep --pattern 'selectCourse = $_'
# Find any type definitions for courseName
rg "type.*Props.*courseName"
Length of output: 71
Script:
#!/bin/bash
# Find selectCourse function definition with a more flexible pattern
ast-grep --pattern 'function selectCourse'
# Also try to find it as an exported const
ast-grep --pattern 'export const selectCourse'
# Look for the file where selectCourse might be defined
rg "selectCourse" --type ts
Length of output: 2675
Script:
#!/bin/bash
# Find CourseNamesKeys type definition
rg "type CourseNamesKeys" -A 5
# Find getCourses implementation
ast-grep --pattern 'export const getCourses'
# Look for course type definitions
rg "type Course" -A 5
Length of output: 7732
src/views/nodejs.tsx (2)
15-15: Straightforward improvement.
Removing "course" reduces coupling to a large object and keeps the component more focused.
18-18: Confirm fallback or error handling.
If "courseName" is invalid, confirm there's a fallback or error boundary so the widgets don't break.
Also applies to: 20-20, 21-21, 25-25
✅ Verification successful
Type safety and error handling are properly implemented
The HeroCourse component has proper TypeScript typing through CourseNamesKeys enum, and the courseName prop is validated at compile-time. The component uses async/await with proper data fetching through selectCourse, and the values are type-safe. The courseName values are strictly controlled through CourseNames enum, making invalid values impossible at runtime.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for HeroCourse component definition and its error handling
ast-grep --pattern 'const HeroCourse = ({ $_ }: { $_ }) => {
$$$
}'
# Search for any error boundaries or fallback handling
rg -A 5 "ErrorBoundary|fallback"
# Look for PropTypes or TypeScript interface/type definitions related to HeroCourse
rg -A 3 "interface.*HeroCourse|type.*HeroCourse|HeroCourse.propTypes"
Length of output: 979
Script:
#!/bin/bash
# Check CourseNamesKeys type definition and possible values
rg -A 5 "type.*CourseNamesKeys|enum.*CourseNamesKeys"
# Look for HeroCourse component implementation
cat src/widgets/hero-course/ui/hero-course.tsx
# Check for any null checks or error handling in the component usage
rg -A 3 "courseName(?:\s+|[^a-zA-Z])" src/views/
Length of output: 13416
src/views/aws-devops.tsx (2)
15-15: Consistent changes.
Replaced "course" with "courseName" to unify the component interface. This aligns well with the overall refactor goal.
18-18: Double-check for leftover references.
Scan the codebase to ensure no references to "course" remain that might cause runtime errors.
Also applies to: 20-20, 21-21, 25-25
✅ Verification successful
Let me check for specific references to the courseName prop and any potential issues in the HeroCourse component.
The courseName prop is used consistently across components
The courseName prop is properly used in the HeroCourse component across all course views, and it's consistently passed down to other components. No leftover or incorrect references were found that could cause runtime errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching all references to the removed "course" prop in the codebase
rg -A 3 'course\s*:'
Length of output: 2235
Script:
#!/bin/bash
# Search for HeroCourse component definition and usage
ast-grep --pattern 'const HeroCourse = ({ $_ }) => { $$$ }'
# Search for courseName prop usage
rg -A 2 'courseName='
Length of output: 9523
src/shared/__tests__/setup-tests.tsx (1)
22-26: Mock usage looks appropriate.
The mocked selectCourse function is well-structured for testing. It correctly resolves a matching course from mockedCourses.
src/views/javascript-preschool-ru.tsx (1)
16-27: Props refactor aligns with simpler flow.
Dropping the direct lang and course props in favor of courseName simplifies prop handling. The updated children components also correctly accept just courseName.
src/shared/ui/date-lang/date-lang.tsx (1)
Line range hint 5-16: Single Language type is preferred.
Switching from string[] to Language is more consistent and robust. This helps avoid accidental array usage and clarifies language usage throughout.
src/views/aws-developer.tsx (1)
1-1: Asynchronous approach and minimal props are successfully consolidated.
Making AwsDeveloper async is a clean way to fetch language info. The shift to courseName-based props aligns with the broader refactor, keeping the component simpler.
Also applies to: 17-28
src/widgets/trainers/ui/trainers.tsx (5)
3-3: Good import structure
Importing "selectCourse" is consistent with the new approach of retrieving course data by name, ensuring a single entry point. No issues here.
6-6: Import usage clarity
Importing CourseNamesKeys from "data" is clear and self-explanatory.
13-13: Prop type modification
Switching from a language-specific prop to a courseName-based prop is a suitable change, clarifying that language (and other details) come from the Course data.
25-25: Title retrieval looks good
Using trainersTitle[language] neatly aligns the displayed text with the chosen language from the fetched course data.
17-20: Leverage async logic
Utilizing async and awaiting selectCourse(courseName) is a straightforward approach for fetching course data. Confirm that the parent component can handle the asynchronous nature of this component (e.g., with Suspense, server components, or other strategies).
src/shared/ui/date-lang/date-lang.test.tsx (3)
17-17: Transition to string prop
Using "language='en'" in tests ensures coverage for the new string-based implementation.
27-27: Mode rendering
Testing that "Online" mode is correctly displayed is valid. Switching language to a string is consistent with the broader refactor.
32-32: Icon existence check
Verifying icons with "language='en'" is aligned with the new structure. Looks correct.
src/entities/course/ui/course-item/course-item.tsx (1)
41-41: Handling language as a string
Replacing language[0] with language.toUpperCase() is an excellent move, eliminating potential index issues. Confirm that language is guaranteed to be non-empty.
✅ Verification successful
Let me verify if the language prop is properly validated before being used.
Let me try a different approach to find the component definition and usage of the language prop.
Let me check the CourseItemData type definition to verify the language prop validation.
Let me check the base Course type to see how the language field is defined.
Let me check for the actual language field in the Course type and any related type definitions.
Let me check the Language type definition.
Language prop is type-safe and non-empty
The language prop is typed as 'en' | 'ru', making it impossible to be empty or undefined. The toUpperCase() operation is safe to use.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find the component definition and props interface
ast-grep --pattern 'interface CourseItemProps {
$$$
}'
# Find usage of language prop in the component
ast-grep --pattern 'function CourseItem($props) {
$$$
}'
# Search for any prop validation or default value assignment
rg "language.*prop" -A 2
Length of output: 490
Script:
#!/bin/bash
# Find the CourseItem component file content
cat src/entities/course/ui/course-item/course-item.tsx
# Find any type definitions or interfaces in the file
rg -t typescript "interface.*Props" src/entities/course/ui/course-item/
Length of output: 1626
Script:
#!/bin/bash
# Find the CourseItemData type definition
rg "CourseItemData" -A 10 src/entities/course/
Length of output: 3966
Script:
#!/bin/bash
# Find the Course type definition
rg "type Course" -A 10 src/entities/course/types.ts
Length of output: 871
Script:
#!/bin/bash
# Search for language field definition in types
rg "language.*:" src/entities/course/types.ts
# Check for language-related constants or enums
rg "language" src/entities/course/
Length of output: 1305
Script:
#!/bin/bash
# Search for Language type definition
rg "type Language" -A 5 src/
rg "enum Language" -A 5 src/
Length of output: 117
src/widgets/about-video/ui/about-video.tsx (2)
3-3: Centralizing type definitions
Importing Language from '@/shared/types' clarifies the shared domain model and avoids string-literal duplication.
11-11: AboutVideoProps refinement
Using lang?: Language is consistent with the rest of the codebase’s shift to typed single-language usage. This avoids array-based confusion.
src/views/angular.tsx (5)
2-2: Import check
The addition of "getCourseLanguage" looks good. No issues found.
23-23: HeroCourse prop update
Passing "courseName" aligns with the new design of focusing on courseName rather than a full course object.
25-25: TrainingProgram prop
This is a straightforward change to align with "courseName" usage.
27-27: AboutCourse prop
Continuing the "courseName" pattern keeps consistency across components.
33-33: Trainers prop
Referencing "angular" from data and passing "courseName" is aligned with the refactor strategy.
src/views/javascript-ru.tsx (4)
2-2: Import check
"getCourseLanguage" import is consistent with the codebase’s refactor strategy.
25-25: Props alignment
Using "courseName" keeps components simpler and consistent with the new data flow.
Also applies to: 27-28
30-32: Language usage
"Communication", "AboutVideo", and "MemberActivity" rely on the asynchronously fetched language. This approach is straightforward and clear.
34-35: Route & trainers
Using "courseName" for route building and passing "javaScriptRu" as trainers is an effective approach.
src/views/javascript-en.tsx (4)
2-2: Import check
"getCourseLanguage" import follows the new pattern.
24-27: Props alignment
Transitioning to "courseName" for these widgets is in line with the refactor plan.
30-31: Language usage
Providing "lang" to "AboutVideo" and "MemberActivity" is a neat solution.
33-34: Route & trainers
Good usage of "courseName" for the route and referencing "javaScriptEn" for trainers.
src/widgets/mentors-wanted/ui/mentors-wanted.tsx (4)
5-5: Imports look consistent
The import for selectCourse is aligned with the new approach of fetching course data.
9-9: Ensure data consistency
By centralizing course data, you reduce duplication and potential mismatch across the app. Good approach!
38-38: Paragraph rendering verified
Displaying text based on the language property is correct and straightforward.
40-40: Link label logic is correct
Using mentorsWantedData[language] properly ensures consistent button labeling.
src/widgets/communication/ui/communication.test.tsx (9)
3-3: Centralized mock data
Good practice to unify test course data in mockedCourses, enhancing reusability.
5-5: Imports are well-managed
Using data constants (COURSE_TITLES, DISCORD_LINKS, communicationText) clarifies test readability.
18-23: Comprehensive coverage for course variants
Mapping over mockedCourses ensures that all courses are tested with relevant links.
25-27: Well-structured test name
Scripted with it.each for clarity, ensuring multiple data points get tested seamlessly.
30-30: Async usage
Using async/await for Communication ensures the component data is loaded before assertion—clean approach.
32-32: Rendering approach
renderWithRouter(widget) remains consistent with other tests. Good uniformity.
42-42: Assert link correctness
Verifying the link attribute ensures that the correct Discord link is used for each language.
47-50: Clarity in test name
Explicit test name helps identify purpose swiftly in logs.
51-56: Cleanup usage
Calling cleanup is prudent to avoid shared state between tests.
src/widgets/hero-course/ui/hero-course.tsx (6)
5-5: Imports are consistent with refactor
Using selectCourse aligns with the broader changes in this refactor.
10-10: Data import synergy
Importing heroCourseLocalized and CourseNamesKeys from 'data' supports consistent type usage.
17-17: Promote clarity in prop naming
Making it clear that the prop is a “courseName” ensures the consumer knows exactly what to provide.
47-51: Subtitle rendering
Conditionally rendering subTitle is a good pattern. Minimal overhead, maximum clarity.
60-60: Localizing the link label
heroCourseLocalized[language].linkLabel is a consistent approach for i18n.
20-21: Potential invalid courseName
Consider verifying the result of selectCourse. If an invalid courseName is supplied, the remaining fields might be undefined.
src/widgets/about-course/ui/about-course/about-course.test.tsx (4)
4-4: Good use of a shared mock data import
This improves maintainability by consolidating course data in one place.
12-15: Asynchronous rendering approach is effective
Awaiting component creation ensures the component is fully rendered before tests run.
38-40: Consistency in calling the component
Passing the AWS_DEVOPS title ensures test coverage for different courses. Implementation looks solid.
50-52: Verifying localized content
Ensures that the component properly renders RU-specific text. Nicely done.
src/widgets/trainers/ui/trainers.test.tsx (5)
4-4: Centralized course titles
Importing from COURSE_TITLES maintains consistent naming across tests.
13-19: Asynchronously rendering widget
This approach confirms the component is ready before assertions run. Good use of async/await.
25-31: Verifying trainer details
Clear checks for trainer fields ensure correct data binding.
48-54: Ensuring RU title
Validates that RU content is displayed accurately. Solid test coverage.
60-66: Testing multiple trainers
Properly checks that all trainers are rendered. Good thoroughness.
src/widgets/hero-course/ui/hero-course.test.tsx (7)
3-3: Shared constants for test data
Reduces duplication and keeps course data unified.
7-12: Fetching correct mocked course
Using find() avoids hardcoded data, tightening test maintainability.
16-19: Asynchronous rendering
Ensures the component is fully populated with the fetched course before rendering.
23-25: Content checks
Ensures the title, label, and subtitle are rendered correctly. Straightforward and effective.
Also applies to: 28-28, 31-31
35-35: Testing enroll button behaviors
Confirms the button label and link attribute for a valid course workflow.
Also applies to: 38-38, 40-40
42-42: Hero image validation
Checks the rendered image source and alt text. Good coverage.
Also applies to: 46-46
51-55: No subtitle scenario
Ensures correct handling of courses that don't have a subtitle.
dev-data/index.ts (1)
8-16: Structured type exports
Grouping related types under mentorship-data.types clarifies where each type originates.
src/widgets/school-menu/school-menu.test.tsx (1)
68-68: Great move to dynamic length.
Using the mock size directly ensures flexibility when the number of courses changes.
src/widgets/training-program/ui/training-program.test.tsx (3)
9-12: Async test initialization is fine.
Waiting for the component setup allows data fetching or side-effects before rendering.
48-51: Consistent async approach.
Aligns with the pattern introduced in the Angular test block. Looks good.
77-77: Uniform enrollment URL confirmed.
Matches the updated approach for registration links.
src/widgets/member-activity/ui/member-activity.tsx (1)
20-20: Flexible language type.
Switching from literal union to Language type fosters scalability if other locales are added in the future.
src/shared/helpers/getActualData.test.ts (3)
42-42: Single-string language format.
Removing the array structure is consistent with the refactor.
74-74: Consistent language property usage.
Aligns with the updated approach applied throughout this PR.
106-106: Uniform standardization for language.
Ensures data consistency in tests and the overall implementation.
src/shared/__tests__/constants.ts (11)
32-32: Consistency check of language property
The switch to a single string 'ru' looks correct and aligns with the refactor goals.
39-39: Consistent altTitle
Changing altTitle to 'JavaScript / Front-end' is fine for uniform naming.
52-52: Language property: 'en'
Implementation matches the new single-language approach.
59-59: Refined altTitle usage
Ensures altTitle is consistent across courses.
72-72: Language set to 'ru'
Good adherence to the new format in line with the directory changes.
79-79: Matched altTitle
Keeps naming consistent for display.
92-92: Language set to 'en'
Reflects the updated language property approach.
111-111: Consistency for Angular course
Language set to 'en' is aligned with the new single-language standard.
130-130: AWS Fundamentals language
Maintaining 'en' as a simple string is correct for the new structure.
142-160: New AWS Cloud Developer course
All fields (including single-string language) correctly align with the refactor. Keep verifying references to ensure coverage in tests.
161-179: New AWS DevOps course
Implementation follows the single-string language refactor seamlessly.
dev-data/courses.data.ts (9)
35-35: language property switched to 'ru'
Smooth transition from array to single string; looks good.
55-55: language set to 'en'
Correctly mirrors the refactor.
75-75: language set to 'ru'
In line with the single-language design choice.
94-94: language set to 'en'
Maintains consistency for React course.
113-113: language set to 'en' for Angular
No issues spotted, matches the project standard.
132-132: language for Node
Refactor to single string is correct.
151-151: language set to 'en' for AWS Fundamentals
Works with the new type.
171-171: language set to 'en' for AWS Cloud Developer
Properly follows the new approach.
190-190: language set to 'en' for AWS DevOps
Consistent, concluding the single-language refactor in this file.
src/views/aws-fundamentals.tsx (4)
14-14: Updated AwsFundamentals signature
Dropping the course object in favor of the simpler 'courseName' prop is a clear improvement in alignment with this PR’s objective.
17-17: HeroCourse usage
Using 'courseName' keeps the data flow concise and uniform.
19-20: TrainingProgram & AboutCourse
Minimal, focused approach to get data by 'courseName' – consistent with the broader refactor.
24-25: Second TrainingProgram & Trainers
Appropriately passing 'courseName' and 'awsFundamentals' helps maintain clarity.
src/entities/course/types.ts (2)
3-3: Import Language
Pulling Language from @/shared/types ensures a single source of truth for language codes.
37-37: Replaced array of languages with a single Language
Simplifies type usage and maintains consistency across the app.
src/entities/course/ui/course-card/course-card.test.tsx (2)
38-38: LGTM: Test assertion properly adapted
Test assertion correctly updated to check the new string format of the language property.
16-16: LGTM: Language property format updated correctly
The change from array to string format aligns with the refactoring goal.
✅ Verification successful
Language property type is consistent with the change
The codebase shows that language is consistently defined as a string type across the application:
- In Course type:
language: Language - In test files and mock data: using string values like
'en'and'ru' - The change to string format in the test file aligns with the type definition and usage patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify CourseCardProps type definition matches the new format
ast-grep --pattern 'type CourseCardProps = {
$$$
language: $_
$$$
}'
Length of output: 76
Script:
#!/bin/bash
# Search for CourseCard component props definition
rg -A 5 "interface.*CourseCard.*Props|type.*CourseCard.*Props" --type typescript
# Search for language prop usage in CourseCard
rg "language.*:" --type typescript -A 2 -B 2
Length of output: 199
Script:
#!/bin/bash
# Search for CourseCard component props definition
rg -A 5 "interface.*CourseCard.*Props|type.*CourseCard.*Props"
# Search for language prop usage in CourseCard
rg "language.*:" -A 2 -B 2
Length of output: 11523
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
src/widgets/training-program/ui/training-program.test.tsx (1)
Line range hint
7-28: Add error case tests for async course selectionWhile the async setup is correct, tests for error cases are missing.
Add tests for:
- Course not found scenario
- selectCourse rejection scenario
♻️ Duplicate comments (1)
src/widgets/training-program/ui/training-program.tsx (1)
19-23:⚠️ Potential issueAdd error handling for async course selection
The async course selection lacks error handling which could lead to runtime errors if the course is not found or if selectCourse fails.
Apply this diff to add error handling:
-export const TrainingProgram = async ({ courseName, specify = '' }: TrainingProgramProps) => { - const course = await selectCourse(courseName); - const { language } = course; +export const TrainingProgram = async ({ courseName, specify = '' }: TrainingProgramProps) => { + try { + const course = await selectCourse(courseName); + if (!course) { + throw new Error(`Course not found: ${courseName}`); + } + const { language } = course;
🧹 Nitpick comments (1)
src/widgets/training-program/ui/training-program.tsx (1)
23-26: Make fallback behavior more explicitThe content name determination logic could be more explicit about its fallback behavior.
Consider this more explicit approach:
-let contentName = isTrainingProgramType(programName) ? programName : courseName; +const contentName = isTrainingProgramType(programName) + ? programName // Use full program name for training programs + : courseName; // Fall back to course name for regular courses
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
dev-data/courses.data.ts(9 hunks)dev-data/index.ts(1 hunks)src/widgets/mobile-view/ui/mobile-view.tsx(1 hunks)src/widgets/school-menu/ui/school-menu/school-menu.test.tsx(3 hunks)src/widgets/training-program/ui/training-program.test.tsx(4 hunks)src/widgets/training-program/ui/training-program.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/widgets/mobile-view/ui/mobile-view.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- dev-data/courses.data.ts
- dev-data/index.ts
🔇 Additional comments (7)
src/widgets/school-menu/ui/school-menu/school-menu.test.tsx (3)
88-88: Good to see dynamic test coverage.
Ensuring the images length matches mockedCourses.length is a good step toward making tests resilient to future changes.
108-108: Likewise for descriptions.
This check ensures alignment between test data and rendered output, avoiding brittle, hardcoded expectations.
129-129: Confirm environment compatibility with .at().
Negative indexing in .at() is a newer JavaScript feature. Ensure the testing environment supports it; otherwise, consider links[links.length - 3].
src/widgets/training-program/ui/training-program.tsx (2)
4-16: LGTM: Props refactoring aligns with centralized language handling
The removal of the language prop and switch to CourseNamesKeys type properly supports the goal of retrieving language from Course objects.
38-38: LGTM: Properly uses course language for link label
The change correctly uses the language from the course object for localization.
src/widgets/training-program/ui/training-program.test.tsx (2)
59-62: LGTM: Consistent test setup for AWS course
The async test setup properly matches the pattern used in other tests.
84-84: LGTM: Updated assertion matches new course name
The alt text assertion correctly reflects the AWS Cloud Developer course name.
|
run visual now |
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/widgets/training-program/ui/training-program.tsx (1)
19-22:⚠️ Potential issueAdd error handling for course selection.
The
selectCoursefunction lacks error handling in two scenarios:
- If
getCourses()promise rejects- If no course is found (returns undefined)
Current implementation forcefully type casts the result to
Coursewhich could lead to runtime errors.
🧹 Nitpick comments (2)
src/widgets/training-program/ui/training-program.tsx (2)
15-16: Add JSDoc comments to document the props.The props look good, but adding documentation would help other developers understand their purpose.
type TrainingProgramProps = { + /** The unique identifier for the course */ courseName: CourseNamesKeys; + /** Optional specification to append to the course name */ specify?: string; };
Line range hint
43-47: Fix potential null reference in Image alt prop.The
course?.titlein the Image alt prop uses optional chaining, but the parent condition already ensures course exists. Consider removing the optional chaining.<Image data-testid="image" src={image} - alt={course?.title} + alt={course.title} className={cx('image', { badge: isCourseWithBadge })} />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/views/aws-developer.tsx(2 hunks)src/widgets/training-program/ui/training-program.tsx(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/views/aws-developer.tsx
🔇 Additional comments (1)
src/widgets/training-program/ui/training-program.tsx (1)
4-8: LGTM! Import changes align with the refactoring goals.
|
Lighthouse Report:
|
What type of PR is this? (select all that apply)
Description
delete lang props for course-page, and get language from Course-obj
UPD: send only courseName into page and widgets. Add call selectCourse inside widgets. Refactor tests
Related Tickets & Documents
Screenshots, Recordings
Please replace this line with any relevant images for UI changes.
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
Language and Course Data Refactoring
Component Enhancements
Testing Improvements
Type System Refinements
Languagetype.These changes represent a significant refactoring of the course and language handling throughout the application, focusing on improved type safety, flexibility, and maintainability.