725-feat: Sync wearecommunity link with api data #726
Conversation
github-actions
bot
changed the title
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
SpaNb4
requested review from
andron13,
dzmitry-varabei and
natanchik
as code owners
📝 WalkthroughWalkthroughThis pull request introduces enhancements to course registration and localization across multiple components. The changes primarily focus on adding support for displaying a "Registration will open soon" message when course enrollment is not available, synchronizing Wearecommunity links from API data, and updating type definitions to accommodate nullable enrollment links. Changes
Assessment against linked issues
Possibly related PRs
Suggested Labels
Suggested Reviewers
✨ Finishing Touches
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: 2
🧹 Nitpick comments (2)
src/entities/course/helpers/sync-with-api-data.ts (1)
22-22: Add null check before assignment.Consider adding a null check before assigning
wearecommunityUrlto handle potential API inconsistencies.- clonedCourse.enroll = currApiCourse.wearecommunityUrl; + clonedCourse.enroll = currApiCourse.wearecommunityUrl ?? null;src/widgets/training-program/ui/training-program.tsx (1)
41-44: Remove redundant course check.The
coursecheck is unnecessary sincecourseis already guaranteed to exist from theselectCoursecall above.- {course && ( <LinkCustom href={enrollHref} variant="primary" external disabled={!course.enroll}> {registrationLinkText} </LinkCustom> - )} + <LinkCustom href={enrollHref} variant="primary" external disabled={!course.enroll}> + {registrationLinkText} + </LinkCustom>
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
dev-data/about-course.data.tsx(2 hunks)dev-data/courses.data.ts(9 hunks)dev-data/hero-course.data.ts(1 hunks)dev-data/training-program.data.tsx(2 hunks)src/core/styles/_constants.scss(1 hunks)src/entities/course/helpers/sync-with-api-data.ts(1 hunks)src/entities/course/types.ts(2 hunks)src/shared/__tests__/constants.ts(2 hunks)src/shared/constants.ts(1 hunks)src/shared/ui/link-custom/link-custom.module.scss(1 hunks)src/shared/ui/link-custom/link-custom.tsx(3 hunks)src/widgets/about-course/ui/about-course/about-course.test.tsx(2 hunks)src/widgets/about-course/ui/about-course/about-course.tsx(2 hunks)src/widgets/hero-course/ui/hero-course.test.tsx(2 hunks)src/widgets/hero-course/ui/hero-course.tsx(2 hunks)src/widgets/training-program/ui/training-program.test.tsx(2 hunks)src/widgets/training-program/ui/training-program.tsx(2 hunks)
🔇 Additional comments (17)
dev-data/hero-course.data.ts (1)
4-11: LGTM! Clean localization implementation.The addition of
noLinkLabelfor both languages follows good localization practices.src/shared/constants.ts (1)
5-6: LGTM! Well-structured constants.Clear and consistent naming for internationalized messages.
src/entities/course/types.ts (1)
25-25: LGTM! Type definitions are well-structured.The type changes properly handle nullable values and maintain type safety.
Also applies to: 42-42
src/widgets/hero-course/ui/hero-course.tsx (1)
35-38: LGTM!Clean implementation that follows the consistent pattern across components.
Also applies to: 64-65
src/widgets/about-course/ui/about-course/about-course.test.tsx (2)
7-7: LGTM! Clean setup of test dependencies.The import of localization constants and mock course setup look good.
Also applies to: 11-13
65-89: LGTM! Comprehensive test coverage for registration message.The test cases effectively verify both English and Russian localization scenarios when enrollment links are not available.
src/widgets/hero-course/ui/hero-course.test.tsx (2)
6-6: LGTM! Clean import.The import statement follows the project's conventions.
63-87: LGTM! Consistent test implementation.The test cases maintain consistency with other components while thoroughly verifying the registration message functionality.
src/widgets/training-program/ui/training-program.test.tsx (2)
7-7: LGTM! Clean import.The import statement follows the project's conventions.
91-115: LGTM! Consistent test implementation.The test cases effectively verify the registration message functionality while maintaining consistency with other components.
dev-data/courses.data.ts (1)
38-38: LGTM! Consistent enrollment link handling.Setting all enrollment links to
nullaligns with the PR objective to sync wearecommunity links with API data.Also applies to: 58-58, 78-78, 97-97, 116-116, 135-135, 154-154, 174-174, 193-193
src/shared/__tests__/constants.ts (1)
80-80: LGTM! Test data updated correctly.The changes to set
enroll: nullfor specific courses align with the feature's objective to handle courses where registration is not available.Also applies to: 177-177
dev-data/about-course.data.tsx (1)
8-8: LGTM! Localization properly implemented.The addition of
noLinkLabelwith appropriate localized messages enhances the user experience when registration is not available.Also applies to: 21-21, 27-27, 33-33
src/shared/ui/link-custom/link-custom.module.scss (1)
93-96: LGTM! Disabled state properly implemented.The
.disabledclass correctly implements both visual feedback and interaction prevention.src/core/styles/_constants.scss (1)
81-81: LGTM! Opacity constant added correctly.The new
$opacity-30variable follows the established pattern and fills a logical gap in the opacity scale.dev-data/training-program.data.tsx (2)
18-18: LGTM!The import statement follows the project's conventions and the constants are used appropriately.
306-313: Consider adding type definitions for the trainingProgramLink object.The object structure has been updated with new properties, but type definitions are missing. This could lead to type errors in consuming components.
Let's verify the impact and type definitions:
✅ Verification successful
Explicit type definitions would be beneficial but not critical
The object's structure is consistent and TypeScript will infer types correctly. The only consumer (training-program.tsx) properly handles both properties.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for type definitions and usages of trainingProgramLink echo "Searching for type definitions..." rg -t typescript -t tsx "type.*trainingProgramLink|interface.*trainingProgramLink" echo -e "\nSearching for usages..." rg -t typescript -t tsx "trainingProgramLink" -B 2 -A 2Length of output: 350
Script:
#!/bin/bash # Search for files with .ts, .tsx extensions and grep for patterns echo "Searching for type definitions..." fd -e ts -e tsx | xargs rg "type.*trainingProgramLink|interface.*trainingProgramLink" echo -e "\nSearching for usages..." fd -e ts -e tsx | xargs rg "trainingProgramLink" -B 2 -A 2Length of output: 2061
What type of PR is this? (select all that apply)
Description
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
New Features
Bug Fixes
Style
.disabledCSS class for visual indication of disabled stateChores