869-refactor: Update angular mark #875
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
YulikK
requested review from
andron13,
dzmitry-varabei,
SpaNb4,
Quiddlee,
ansivgit and
natanchik
as code owners
github-actions
bot
changed the title
📝 Walkthrough## Walkthrough
Eligibility descriptions for the Angular course were broadened from "JS/FE Stage 2 graduates only" to "graduates of any RS School course" across data, UI, and tests. A new constant (`RS_GRADUATED_ONLY`) was introduced for subtitle consistency. Related test logic and props were updated to align with these changes.
## Changes
| Files/Group | Change Summary |
|---------------------------------------------------------------------------------------------|--------------------------------------------------------------------------------------------------------|
| dev-data/about-course.data.tsx, dev-data/training-program.data.tsx | Broadened Angular course eligibility description from "JS/FE Stage 2 graduates only" to "any RS School course". |
| dev-data/courses.data.ts | Updated Angular course's `subTitle` to use new constant; changed `startDate` to a constant. |
| src/shared/constants.ts | Added `RS_GRADUATED_ONLY` constant. |
| src/entities/course/ui/course-card/course-card.tsx | Added optional `subTitle` prop; conditionally displays subtitle for Angular course. |
| src/entities/course/ui/course-card/course-card.test.tsx | Refactored tests to iterate over multiple courses; included `subTitle` in test props. |
| src/widgets/courses/ui/courses.test.tsx | Adjusted test logic to match course titles with possible subtitles, especially for Angular. |
| src/widgets/training-program/ui/training-program.test.tsx | Updated test data to reflect new Angular course eligibility text. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant CoursesWidget
participant CourseCard
User->>CoursesWidget: View list of courses
CoursesWidget->>CourseCard: Render each course with title and subTitle
CourseCard->>CourseCard: If Angular course and no mentoring start date, append subTitle
CourseCard-->>User: Display course title (with or without subtitle)Assessment against linked issues
Possibly related PRs
Suggested reviewers
Learnt from: Quiddlee |
![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 (3)
src/shared/constants.ts (1)
7-7: Add missing Russian translation for the new subtitle constant.We introduced
RS_GRADUATED_ONLY = 'Alumni only'but did not add a corresponding Russian localization. Consider adding anRS_GRADUATED_ONLY_RUconstant or integrating this into the i18n pipeline for consistency across locales.src/widgets/training-program/ui/training-program.test.tsx (1)
12-12: Avoid regex pitfalls in test.The test uses
new RegExp(p, 'i')on a string with punctuation, which can introduce unintended regex behavior. Preferscreen.getByText(p)for exact matching or escape special characters before constructing theRegExp.dev-data/training-program.data.tsx (1)
120-121: Centralize eligibility text.Hardcoding the Angular eligibility description here duplicates the literal. For maintainability, import and render
RS_GRADUATED_ONLYwithin the<Paragraph>instead of repeating the string.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
dev-data/about-course.data.tsx(1 hunks)dev-data/courses.data.ts(2 hunks)dev-data/training-program.data.tsx(1 hunks)src/entities/course/ui/course-card/course-card.test.tsx(1 hunks)src/entities/course/ui/course-card/course-card.tsx(6 hunks)src/shared/constants.ts(1 hunks)src/widgets/courses/ui/courses.test.tsx(2 hunks)src/widgets/training-program/ui/training-program.test.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/widgets/courses/ui/courses.test.tsx (1)
Learnt from: Quiddlee
PR: rolling-scopes/site#572
File: src/pages/home.tsx:25-25
Timestamp: 2024-09-30T12:07:39.137Z
Learning: The `Courses` component in `src/widgets/courses/other-courses.test.tsx`, `src/pages/courses.tsx`, and `dev-data/courses-path.data.ts` is a different component from the one replaced in `src/pages/home.tsx`.
🧬 Code Graph Analysis (2)
dev-data/courses.data.ts (1)
src/shared/constants.ts (3)
RS_GRADUATED_ONLY(7-7)COURSE_LINKS(34-44)TO_BE_DETERMINED(6-6)
src/widgets/courses/ui/courses.test.tsx (1)
src/shared/__tests__/constants.ts (1)
mockedCourses(27-207)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: CI
🔇 Additional comments (11)
dev-data/courses.data.ts (3)
10-10: Good - Import standardizationAdding
RS_GRADUATED_ONLYto imports helps maintain consistency with the new eligibility labeling across the application.
108-108: Good - Broadened eligibility criteriaChanged from hardcoded "JS/FE Stage 2 graduates only" to a more inclusive
RS_GRADUATED_ONLYconstant ('Alumni only'), opening the Angular course to all RS School graduates.
114-114: Good - Using constants for date valuesReplacing hardcoded strings with the
TO_BE_DETERMINEDconstant improves consistency and makes future updates easier.src/widgets/courses/ui/courses.test.tsx (2)
6-6: Good - Using constants for course titlesAdding the
COURSE_TITLESimport allows for more robust title matching in tests.
28-32: Good - Updated test logic for Angular subtitleThe test now correctly handles the special case where Angular course titles include a subtitle suffix in parentheses, ensuring proper test coverage of the UI changes.
src/entities/course/ui/course-card/course-card.tsx (6)
11-11: Good - Using central course title constantsImporting
COURSE_TITLESenables consistent title handling and specific logic for Angular courses.
20-20: Good - Added subTitle property typeProperly extended the type definition to include the subTitle property from Course.
38-38: Good - Destructured subTitle propAdded subTitle to the destructured props to make it available in the component.
56-56: Good - Clean Angular course subtitle logicThe condition to display the Angular course subtitle is clearly defined and handles the special case appropriately.
71-73: Good - Proper subtitle display implementationUpdated the Subtitle component to include the title with optional subtitle suffix for Angular courses.
93-93: Good - Code consolidationConsolidated the ShortInfoPanel component usage to a single line, improving code readability without changing functionality.
|
Lighthouse Report:
|
|
Lighthouse Report:
|
Quiddlee
approved these changes
May 7, 2025
|
Lighthouse Report:
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this? (select all that apply)
Description
Update Angular mark
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?
Our alumni are running to register for the course 🙃
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests