835-fix: Stale data on the website #843
Conversation
github-actions
bot
changed the title
Quiddlee
changed the title
|
Lighthouse Report:
|
|
Lighthouse Report:
|
Quiddlee
changed the title
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
|
Lighthouse Report:
|
KristiBo
requested review from
andron13,
dzmitry-varabei and
natanchik
as code owners
📝 WalkthroughWalkthroughThis change introduces new React components for rendering course lists, refactors course menu and card rendering to use these components, and enhances stale data handling with mentorship-specific logic in the Changes
Sequence Diagram(s)sequenceDiagram
participant Page as Widget/Page
participant FreshCourses as FreshCourses
participant getActualData as getActualData
participant CourseMenuItemsFresh as CourseMenuItemsFresh
Page->>FreshCourses: Render with courses, options
FreshCourses->>getActualData: Filter/sort courses (mentorship, stale, etc.)
getActualData-->>FreshCourses: Processed courses
FreshCourses-->>Page: Rendered course nodes
Page->>CourseMenuItemsFresh: Render with courses
CourseMenuItemsFresh->>FreshCourses: Render with courses and render prop
FreshCourses->>getActualData: Filter/sort courses
getActualData-->>FreshCourses: Processed courses
FreshCourses-->>CourseMenuItemsFresh: Rendered menu items
CourseMenuItemsFresh-->>Page: Rendered menu
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Possibly related PRs
Suggested reviewers
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (4)
src/shared/ui/course-menu-items-fresh/course-menu-items-fresh.tsx (1)
8-13: Consider renaming the type for better clarity.The type name
AllCoursesPropsdoesn't clearly indicate it's for menu items specifically.-type AllCoursesProps = { +type CourseMenuItemsFreshProps = { courses: Course[]; icon?: 'iconSmall' | 'iconFooter'; color?: Color; onClose?: () => void; };src/widgets/hero-course/ui/registration-link.tsx (1)
9-14: Rename type to avoid naming conflict.The type
RegistrationLinkhas the same name as the component, which can cause confusion.-type RegistrationLink = { +type RegistrationLinkProps = { enrollLink: string | null; startDate: string; registrationEndDate: string; language: Language; };And update the component parameter:
-}: RegistrationLink) => { +}: RegistrationLinkProps) => {src/widgets/upcoming-courses/ui/course-items.tsx (1)
15-18: Consider moving empty text outside component.The
emptyTextobject is recreated on every render. Consider moving it outside the component scope for better performance.+const emptyText = { + part1: `Looks like the course board is empty right now. But don't worry — we're cooking up something exciting! Subscribe to our `, + part2: ` Announcement channel to be the first to know when fresh courses are served.`, +}; + export const CourseItems = ({ courses }: CourseItems) => { - const emptyText = { - part1: `Looks like the course board is empty right now. But don't worry — we're cooking up something exciting! Subscribe to our `, - part2: ` Announcement channel to be the first to know when fresh courses are served.`, - };src/widgets/hero-course/ui/hero-course.test.tsx (1)
1-2: Minor: Consider more specific ESLint disable comment.The broad
import/no-namespacedisable could be more targeted to specific imports if only certain namespace imports are needed.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/shared/helpers/get-actual-data.ts(2 hunks)src/shared/ui/course-menu-items-fresh/course-menu-items-fresh.tsx(1 hunks)src/shared/ui/course-menu-items-fresh/index.ts(1 hunks)src/shared/ui/fresh-courses/fresh-courses.tsx(1 hunks)src/shared/ui/fresh-courses/index.ts(1 hunks)src/shared/ui/short-info-panel/course-start-label.tsx(1 hunks)src/shared/ui/short-info-panel/short-info-panel.tsx(3 hunks)src/widgets/courses/ui/course-items-fresh.tsx(1 hunks)src/widgets/courses/ui/courses.tsx(2 hunks)src/widgets/footer/ui/desktop-view.tsx(2 hunks)src/widgets/header/header.tsx(2 hunks)src/widgets/hero-course/ui/availability-status.tsx(1 hunks)src/widgets/hero-course/ui/hero-course.test.tsx(5 hunks)src/widgets/hero-course/ui/hero-course.tsx(3 hunks)src/widgets/hero-course/ui/registration-link.tsx(1 hunks)src/widgets/mentorship-courses/course-items-fresh.tsx(1 hunks)src/widgets/mentorship-courses/mentorship-courses.tsx(1 hunks)src/widgets/mobile-view/ui/mobile-view.tsx(2 hunks)src/widgets/upcoming-courses/ui/course-items.tsx(1 hunks)src/widgets/upcoming-courses/ui/upcoming-courses.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
src/widgets/upcoming-courses/ui/upcoming-courses.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`.
src/widgets/footer/ui/desktop-view.tsx (2)
Learnt from: Quiddlee
PR: rolling-scopes/site#659
File: src/core/base-layout/components/footer/desktop-view.tsx:14-22
Timestamp: 2024-12-14T10:58:02.008Z
Learning: In `src/core/base-layout/components/footer/desktop-view.tsx`, the `schoolMenuStaticLinks` array represents a static menu where the order will never change.
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`.
src/widgets/header/header.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`.
src/widgets/courses/ui/course-items-fresh.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`.
src/widgets/mentorship-courses/mentorship-courses.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`.
src/widgets/courses/ui/courses.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`.
src/widgets/upcoming-courses/ui/course-items.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 (10)
src/widgets/courses/ui/course-items-fresh.tsx (1)
src/shared/ui/fresh-courses/fresh-courses.tsx (1)
FreshCourses(19-38)
src/widgets/mentorship-courses/mentorship-courses.tsx (2)
src/entities/course/model/store.ts (1)
courseStore(32-32)src/widgets/courses/ui/course-items-fresh.tsx (1)
CourseItemsFresh(10-18)
src/widgets/hero-course/ui/availability-status.tsx (3)
src/widgets/hero-course/helpers/get-course-status.ts (1)
getCourseStatus(7-38)src/shared/helpers/day-js.ts (1)
dayJS(8-8)src/shared/ui/section-label/section-label.tsx (1)
SectionLabel(14-20)
src/widgets/courses/ui/courses.tsx (1)
src/widgets/courses/ui/course-items-fresh.tsx (1)
CourseItemsFresh(10-18)
src/shared/ui/short-info-panel/short-info-panel.tsx (2)
src/shared/ui/short-info-panel/course-start-label.tsx (1)
CourseStartLabel(16-39)src/shared/constants.ts (1)
LABELS(17-25)
src/shared/ui/short-info-panel/course-start-label.tsx (1)
src/shared/helpers/get-course-date.ts (1)
calculateFreshDate(13-18)
src/widgets/hero-course/ui/hero-course.test.tsx (4)
src/shared/helpers/get-course-date.ts (1)
getCourseDate(5-11)src/widgets/hero-course/ui/hero-course.tsx (1)
HeroCourse(20-67)src/shared/__tests__/utils/render-with-router/render-with-router.tsx (1)
renderWithRouter(38-49)src/shared/constants.ts (1)
TO_BE_DETERMINED(6-6)
src/widgets/hero-course/ui/registration-link.tsx (3)
src/shared/helpers/get-course-date.ts (1)
calculateFreshDate(13-18)src/shared/constants.ts (1)
TO_BE_DETERMINED(6-6)src/shared/ui/link-custom/link-custom.tsx (1)
LinkCustom(45-91)
src/shared/helpers/get-actual-data.ts (3)
src/shared/helpers/day-js.ts (1)
dayJS(8-8)src/shared/helpers/get-course-date.ts (1)
getCourseDate(5-11)src/shared/constants.ts (1)
TO_BE_DETERMINED(6-6)
src/widgets/hero-course/ui/hero-course.tsx (2)
src/widgets/hero-course/ui/availability-status.tsx (1)
AvailabilityStatus(12-16)src/widgets/hero-course/ui/registration-link.tsx (1)
RegistrationLink(16-37)
🔇 Additional comments (39)
src/shared/ui/course-menu-items-fresh/index.ts (1)
1-1: Standard export pattern - looks good.Clean barrel export following TypeScript conventions.
src/shared/ui/fresh-courses/index.ts (1)
1-1: Standard export pattern - looks good.Clean barrel export following TypeScript conventions.
src/widgets/footer/ui/desktop-view.tsx (2)
3-3: Import added for centralized component.Good addition supporting the course rendering refactoring.
31-31: Centralized course rendering - excellent refactoring.Replacing manual mapping with
CourseMenuItemsFreshcomponent centralizes course data handling, which should help address the stale data issues mentioned in the PR objectives.src/widgets/header/header.tsx (2)
10-10: Import added for centralized component.Good addition supporting the course rendering refactoring.
91-91: Consistent centralized course rendering.Replacing manual course mapping with
CourseMenuItemsFreshfollows the same pattern as other components, ensuring consistent course data handling across the application.src/widgets/mobile-view/ui/mobile-view.tsx (1)
6-6: LGTM! Good refactoring for consistency.The replacement of inline course mapping with the centralized
CourseMenuItemsFreshcomponent improves maintainability and ensures consistent course data handling across the application.Also applies to: 62-62
src/widgets/courses/ui/course-items-fresh.tsx (1)
1-18: Well-structured wrapper component.Clean implementation that effectively wraps
FreshCourseswith sensible defaults. The render prop pattern forCourseCardrendering is appropriate and maintains flexibility.src/widgets/mentorship-courses/mentorship-courses.tsx (1)
3-3: Effective simplification through delegation.The refactoring successfully delegates course rendering to the specialized
CourseItemsFreshcomponent while maintaining the mentorship-specific functionality. UsingcourseStore.loadCourses()directly simplifies the data flow.Also applies to: 5-5, 12-12, 19-19
src/widgets/hero-course/ui/availability-status.tsx (1)
1-16: Clean encapsulation of status logic.Well-implemented component that correctly calculates course status using the day difference between registration end and start dates. The integration with
getCourseStatushelper andSectionLabelrendering is appropriate.src/widgets/mentorship-courses/course-items-fresh.tsx (1)
1-28: LGTM! Clean wrapper component for mentorship course rendering.The component properly delegates to
FreshCourseswith mentorship-specific configuration and correctly shows the mentoring start date for mentorship courses.src/widgets/courses/ui/courses.tsx (2)
3-5: Good refactoring to centralize course rendering logic.The import changes properly reflect the delegation of course rendering to the specialized
CourseItemsFreshcomponent.
19-19: Proper delegation to CourseItemsFresh component.The raw courses array is correctly passed to
CourseItemsFresh, which will handle filtering stale data and rendering individual course cards.src/widgets/upcoming-courses/ui/upcoming-courses.tsx (2)
4-7: Clean import refactoring for simplified component structure.The import changes properly reflect the delegation to the
CourseItemscomponent.
22-22:✅ Verification successful
Verify CourseItems vs CourseItemsFresh component choice.
The component uses
CourseItemswhile other widgets useCourseItemsFresh. Ensure this difference is intentional and thatCourseItemsprovides the appropriate filtering and rendering logic for upcoming courses.
🏁 Script executed:
#!/bin/bash # Description: Verify the CourseItems component exists and understand its implementation # Expected: CourseItems component should handle upcoming course filtering and rendering # Search for CourseItems component definition ast-grep --pattern 'export const CourseItems = $_' # Search for its import and usage rg -A 5 "CourseItems"Length of output: 8347
Component choice confirmed:
CourseItemsis intentional and correct.
CourseItemsappliesgetActualData({ filterStale: true })and enforces the upcoming‐courses limit, whereasCourseItemsFreshdelegates toFreshCoursesfor general listings. No change needed here.src/shared/ui/course-menu-items-fresh/course-menu-items-fresh.tsx (1)
15-36: Excellent reusable component for course menu items.The component properly handles optional props with sensible defaults, uses dynamic icon selection, and delegates to
FreshCoursesfor consistent data handling. This should help ensure stale course data is properly filtered in menu items.src/shared/ui/short-info-panel/short-info-panel.tsx (3)
8-8: Good refactoring to use specialized component.Nice addition of the
CourseStartLabelimport to support the component refactoring.
50-56: Clean delegation to CourseStartLabel component.The refactoring properly delegates date label rendering to the specialized component with appropriate props.
74-84: Verify conditional registrationEndDate logic.The upcoming view conditionally passes
registrationEndDateonly whenstartDateexists. This could potentially hide registration end dates when start date is missing.registrationEndDate={startDate ? registrationEndDate : null}Consider if this behavior is intentional or if
registrationEndDateshould always be passed when available.src/widgets/hero-course/ui/registration-link.tsx (2)
22-31: Solid stale course detection logic.The implementation correctly calculates fresh dates and determines staleness using the
TO_BE_DETERMINEDconstant. The conditional text logic properly handles both disabled states.
32-36: Clean LinkCustom integration.The component properly passes all necessary props to
LinkCustomwith appropriate disabled state and styling.src/shared/ui/fresh-courses/fresh-courses.tsx (2)
11-17: Well-designed component interface.The props interface is clean and provides good flexibility with optional filtering, sorting, and mentorship modes. The render prop pattern enables reusable course rendering logic.
19-38: Excellent centralization of course filtering logic.The component properly delegates to
getActualDatafor filtering/sorting and handles mentorship transformation appropriately. This centralizes stale data handling across the application.src/widgets/upcoming-courses/ui/course-items.tsx (2)
20-38: Solid course filtering and rendering logic.The implementation properly filters stale courses and limits the display quantity. The destructuring and key assignment in the map function is clean and efficient.
40-60: Excellent empty state handling.The conditional rendering provides a user-friendly empty state with helpful guidance and a call-to-action link to the Telegram channel.
src/widgets/hero-course/ui/hero-course.test.tsx (4)
9-14: Good addition of test dependencies.The new imports properly support the enhanced testing approach with date mocking capabilities.
45-56: Test logic enhancement with proper mocking.The test now properly controls date-dependent behavior by mocking
calculateFreshDate. This ensures deterministic test results regardless of when tests run.
80-80: Consistent mock usage for stale date scenarios.Good use of
TO_BE_DETERMINEDmock return value to test the "registration will open soon" behavior consistently.Also applies to: 94-94
23-23:✅ Verification successful
Verify mock spy setup correctness.
The mock spy for
calculateFreshDatelooks correct, but ensure this function exists in thegetCourseDatemodule.
🏁 Script executed:
#!/bin/bash # Description: Verify calculateFreshDate function exists in get-course-date module # Expected: Function should be exported from the module ast-grep --pattern 'export const calculateFreshDate = $_'Length of output: 564
✅ Mock Spy Verified for calculateFreshDate
calculateFreshDateis exported fromsrc/shared/helpers/get-course-date.ts, so thevi.spyOn(getCourseDate, 'calculateFreshDate')setup in your test is correct.src/shared/ui/short-info-panel/course-start-label.tsx (3)
8-14: Well-defined TypeScript interface.The props interface properly extends
PropsWithChildrenand includes appropriate nullable types for dates.
24-26: Robust null handling for fresh date calculation.The component correctly handles cases where either
startDateorregistrationEndDatemight be null, preventing runtime errors.
28-38: Clean component rendering with proper prop delegation.The component effectively wraps
DateSimplewith computed values while maintaining flexibility through children and optional props.src/widgets/hero-course/ui/hero-course.tsx (3)
9-10: Good modular imports for specialized components.The new imports support the refactored architecture with dedicated components for availability status and registration links.
43-43: Excellent separation of concerns with AvailabilityStatus.Replacing inline status logic with a dedicated component improves maintainability and reusability.
57-62: Clean delegation to RegistrationLink component.The registration link rendering is now properly encapsulated in a dedicated component with all necessary props passed through.
src/shared/helpers/get-actual-data.ts (4)
14-15: Well-designed optional parameters with sensible defaults.The new parameters provide flexibility while maintaining backward compatibility through appropriate default values.
20-26: Clean conditional logic for mentorship and sorting.The function properly handles the new parameters with clear conditional branches for mentorship mapping and optional sorting.
Also applies to: 29-31, 37-39
64-68: Robust type checking for mentorship data.The early return for Event data prevents incorrect processing when the function receives non-Course data.
69-95:✅ Verification successful
Verify mentorship date field handling logic.
The mentorship mapping logic processes
personalMentoringStartDateandpersonalMentoringEndDate, but ensure these fields exist on the Course type and the staleness calculation is correct.
🏁 Script executed:
#!/bin/bash # Description: Verify Course type includes personalMentoringStartDate and personalMentoringEndDate fields # Expected: Course interface should include these optional date fields ast-grep --pattern 'interface Course { $$$ personalMentoringStartDate$_ $$$ }' ast-grep --pattern 'type Course = { $$$ personalMentoringStartDate$_ $$$ }'Length of output: 170
🏁 Script executed:
#!/bin/bash # Locate Course type/interface declarations rg -n "interface Course" rg -n "type Course" # Find any references to the mentoring date fields rg -n "personalMentoringStartDate" rg -n "personalMentoringEndDate"Length of output: 8019
No action needed on mentorship date handling
TheCoursetype insrc/entities/course/types.tsdoes include bothpersonalMentoringStartDateandpersonalMentoringEndDateasstring | null, and the Day.js diff call correctly calculates days between end and start dates. Staleness logic ingetCourseDatealigns with expectations.
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