Migrate common UI components to packages/ui #3049
Conversation
WalkthroughThis update centralizes and refactors UI components for course and module management by replacing local implementations with shared package imports, restructures localization keys, and introduces utility and data table enhancements. Several redundant or local files are deleted, and new or updated components and utilities are provided from shared packages, improving modularity and consistency across the codebase. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AppPage
participant SharedUI
participant Utils
User->>AppPage: Navigates to course/module page
AppPage->>SharedUI: Renders shared CourseHeader, CourseForm, ModuleForm, etc.
AppPage->>SharedUI: Uses shared CustomDataTable for listing
AppPage->>SharedUI: Uses shared resource/file management components
AppPage->>Utils: Calls joinPath/popPath for path handling
SharedUI->>Utils: Uses joinPath/popPath if needed
AppPage->>SharedUI: Uses ViewToggle to switch between card/table view
User->>AppPage: Interacts with UI (edit, delete, upload, etc.)
AppPage->>SharedUI: Triggers modals/actions via shared components
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/columns.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/content/page.tsxOops! Something went wrong! :( ESLint: 9.28.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs
✨ 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
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
…/platform into chore/education-packages
Implement tiptap editor to ttr
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🔭 Outside diff range comments (2)
packages/ui/src/components/ui/custom/education/modules/module-toggle.tsx (1)
44-49:⚠️ Potential issueInitialize loading state and add user feedback on errors.
handlePublishedChangenever callssetLoading(true), soloadingstaysfalseand the checkbox isn’t disabled during the update. AddsetLoading(true)before the Supabase call and wrap in atry/catchto display a toast (via@tuturuuu/ui/hooks/use-toast) on failure.apps/upskii/messages/en.json (1)
1-4143: 💡 Verification agent🧩 Analysis chain
Verify duplicate localization entries have been removed at the root.
The PR summary notes that top-level
no_courses_foundandno_description_providedentries were removed. Please ensure no stray definitions remain outsidews-courses:
🏁 Script executed:
#!/bin/bash # Search for duplicate localization keys in this file rg -n '"no_courses_found"' apps/upskii/messages/en.json rg -n '"no_description_provided"' apps/upskii/messages/en.jsonLength of output: 233
Action Required: Remove stray top-level
no_courses_foundandno_description_providedentriesThe PR summary indicated these keys were removed, but they still exist at the root of
apps/upskii/messages/en.json:
- Line 3528:
"no_courses_found": "No courses found",- Line 3529:
"no_description_provided": "No description provided"Please delete these two lines so that those messages only exist under
ws-courses.
🧹 Nitpick comments (16)
packages/ui/src/components/ui/custom/education/modules/module-toggle.tsx (3)
8-18: Remove unusedisPublicprop and commented code.The
isPublicprop remains in the prop type but isn’t destructured or used, and its handler is fully commented out. To reduce confusion and dead code, either removeisPublicfrom the props type or re-enable the public toggle logic if it's still required.
26-42: Clean up or re-enable the public toggle handler.All of the
handlePublicChangecode is commented out. If the public-toggle feature is deprecated, drop these lines entirely; otherwise, re-enable and wire it up with its own checkbox.
76-80: Use unique IDs for accessibility.The hard-coded
id="isPublished"may collide if multiple toggles are rendered. Consider using a template literal, e.g.id={isPublished-${moduleId}}, to guarantee uniqueness.packages/ui/src/components/ui/custom/education/modules/resources/delete-resource.tsx (1)
9-28: Avoid parameter shadowing and improve user feedback.The inner
deleteResource(path: string)parameter shadows the outerpathprop—rename it (e.g.,filePath) for clarity. Also, surface success/error to the user via a toast (use-toast) instead of silently refreshing or just resetting loading.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx (1)
10-12: Verify shared component imports and add coverage.The local
CourseSection,FileDisplay, andYoutubeEmbedimports have been replaced with package imports. Confirm these paths match the UI library’s exports, and consider adding UI/integration tests to cover these new imports (static analysis flagged them as uncovered).🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-12: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx#L10-L12
Added lines #L10 - L12 were not covered by testspackages/ui/src/components/ui/custom/education/modules/course-module-form.tsx (1)
34-38: Confirm named export and update consumers.
CourseModuleFormwas converted from a default to a named export—ensure all consuming modules import it correctly. You might also consider adding aContent-Type: application/jsonheader to the fetch call and showing a success toast to improve UX.apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx (1)
3-5: Validate new YouTube component APIs and tests.Imports for
DeleteLinkButton,YoutubeEmbed, andYouTubeLinkFormwere updated to the shared UI package. Verify their prop signatures match the updated components, and add tests to cover these lines (they’re currently untested).🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-5: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx#L3-L5
Added lines #L3 - L5 were not covered by testspackages/ui/src/components/ui/custom/education/courses/course-header.tsx (2)
7-13: Consider enhancing the props interface with better TypeDocs.The component interface is functional but could benefit from documentation for better developer experience.
export function CourseHeader({ href, data, }: { + /** URL for the course navigation link */ href: string; + /** Course data object containing name and description */ data: UserGroup; }) {
19-22: Consider extracting the badge styling to a reusable component.The graduation cap badge has specific styling that could be reused across other education-related components.
Consider creating a reusable
CourseBadgecomponent:const CourseBadge = ({ children }: { children: ReactNode }) => ( <div className="flex items-center gap-2 rounded-lg border border-dynamic-blue/20 bg-dynamic-blue/10 px-2 text-lg text-dynamic-blue max-md:hidden"> <GraduationCap className="h-6 w-6" /> {children} </div> );packages/ui/src/components/ui/custom/tables/custom-data-table.tsx (2)
34-35: Remove redundant fallback values in props.The pageIndex and pageSize already have default values calculated above, making the
|| 0and|| 10fallbacks redundant.- pageIndex={pageIndex || 0} - pageSize={pageSize || 10} + pageIndex={pageIndex} + pageSize={pageSize}
38-42: Consider improving the search logic for better UX.The current search implementation immediately resets all parameters when the query is empty, which might not be the desired behavior for users.
onSearch={(query: string) => - query - ? searchParams.set({ q: query, page: '1' }) - : searchParams.reset() + query.trim() + ? searchParams.set({ q: query.trim(), page: '1' }) + : searchParams.remove(['q', 'page']) }This change:
- Trims whitespace from search queries
- Only removes search-related parameters instead of resetting everything
- Preserves other URL parameters like pageSize
apps/web/messages/en.json (2)
287-291: Typo: inconsistent “GitHub” casing
The value uses “Github” but elsewhere we use “GitHub” (e.g.,continue_with_github). Align casing for consistency.- "sign_in_with_github": "Sign in with Github", + "sign_in_with_github": "Sign in with GitHub",
1980-1987: Consistency: key naming and cross-app parity
Inws-coursesyou addeddelete_confirm_title, but the common pattern usesconfirm_delete_title. Consider renaming to match (confirm_delete_title) or document the intentional change. Also ensure all new course-related keys (edit_description,course_description,card_view,table_view,no_courses_found,no_courses_found_description,no_description_provided,delete_confirm_title) are synchronized across bothapps/webandapps/upskiilocale files.apps/upskii/messages/vi.json (1)
3530-3530: Typographical consistency: diacritics in “xóa”
The verb “xóa” is conventionally written with the accent on “o” (xóa), not on “á” (xoá). Consider updating to:- "delete_confirm_title": "Xác nhận xoá \"{name}\"" + "delete_confirm_title": "Xác nhận xóa \"{name}\""packages/ui/src/hooks/useSearchParams.tsx (1)
96-108: Consider using Map or Object.fromEntries for cleaner implementation.The
getAllmethod works correctly but could be more concise.const getAll = useCallback(() => { - const params = new URLSearchParams(searchParams); - const entries = params.entries(); - const result: Record<string, string[]> = {}; - for (const [key, value] of entries) { - if (result[key]) { - result[key]?.push(value); - } else { - result[key] = [value]; - } - } - return result; + const result: Record<string, string[]> = {}; + searchParams.forEach((value, key) => { + result[key] = result[key] ? [...result[key], value] : [value]; + }); + return result; }, [searchParams]);apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/page.tsx (1)
36-43: Consider adding radix parameter to parseInt calls.While the current implementation works correctly with default values, it's a best practice to specify the radix explicitly for parseInt calls to avoid potential parsing issues with non-decimal numbers.
- const currentPage = parseInt(page); - const currentPageSize = parseInt(pageSize); + const currentPage = parseInt(page, 10); + const currentPageSize = parseInt(pageSize, 10);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
apps/upskii/messages/en.json(2 hunks)apps/upskii/messages/vi.json(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/columns.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/content/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/layout.tsx(2 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/resources/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/toggles.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/embed.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx(1 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/columns.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/new-actions.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/page.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/row-actions.tsx(0 hunks)apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/table.tsx(0 hunks)apps/upskii/src/components/custom-data-table.tsx(1 hunks)apps/upskii/src/utils/path-helper.ts(0 hunks)apps/web/messages/en.json(2 hunks)apps/web/messages/vi.json(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/drive/form.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/drive/new-actions.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/columns.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/form.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/link-button.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx(4 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/delete-resource.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/file-display.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/page.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/pdf-viewer.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/delete-link.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/form.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/page.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/row-actions.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/section.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/columns.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/form.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/page.tsx(6 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/row-actions.tsx(0 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/education/quiz-sets/[setId]/layout.tsx(1 hunks)apps/web/src/components/custom-data-table.tsx(1 hunks)packages/ui/package.json(2 hunks)packages/ui/src/components/ui/custom/education/courses/course-card-view.tsx(2 hunks)packages/ui/src/components/ui/custom/education/courses/course-form.tsx(1 hunks)packages/ui/src/components/ui/custom/education/courses/course-header.tsx(1 hunks)packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx(2 hunks)packages/ui/src/components/ui/custom/education/modules/course-module-form.tsx(1 hunks)packages/ui/src/components/ui/custom/education/modules/course-module-row-actions.tsx(2 hunks)packages/ui/src/components/ui/custom/education/modules/module-toggle.tsx(1 hunks)packages/ui/src/components/ui/custom/education/modules/resources/delete-resource.tsx(1 hunks)packages/ui/src/components/ui/custom/education/modules/resources/file-display.tsx(1 hunks)packages/ui/src/components/ui/custom/education/modules/resources/file-upload-form.tsx(2 hunks)packages/ui/src/components/ui/custom/tables/custom-data-table.tsx(1 hunks)packages/ui/src/components/ui/text-editor/content-editor.tsx(1 hunks)packages/ui/src/hooks/useSearchParams.tsx(1 hunks)packages/utils/src/path-helper.ts(1 hunks)
💤 Files with no reviewable changes (20)
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/link-button.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/new-actions.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/delete-resource.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/embed.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/form.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/row-actions.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/table.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/row-actions.tsx
- apps/upskii/src/utils/path-helper.ts
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/form.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/form.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/columns.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/section.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/page.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/pdf-viewer.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/delete-link.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/file-display.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/toggles.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/drive/form.tsx
- apps/upskii/src/app/[locale]/(dashboard)/[wsId]/drive/row-actions.tsx
🧰 Additional context used
🧬 Code Graph Analysis (5)
packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx (1)
packages/ui/src/components/ui/custom/education/courses/course-form.tsx (1)
CourseForm(34-125)
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx (1)
packages/ui/src/components/ui/custom/education/courses/course-header.tsx (1)
CourseHeader(7-38)
packages/ui/src/components/ui/custom/education/courses/course-header.tsx (1)
packages/types/src/primitives/UserGroup.ts (1)
UserGroup(3-10)
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx (1)
packages/ui/src/components/ui/custom/education/courses/course-header.tsx (1)
CourseHeader(7-38)
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/page.tsx (5)
packages/ui/src/components/ui/custom/education/courses/course-form.tsx (1)
CourseForm(34-125)packages/ui/src/components/ui/custom/view-toggle.tsx (1)
ViewToggle(19-62)packages/ui/src/components/ui/custom/education/courses/course-card-view.tsx (1)
CourseCardView(18-123)packages/ui/src/components/ui/custom/education/courses/course-pagination.tsx (1)
CoursePagination(15-56)packages/types/src/db.ts (1)
WorkspaceCourse(37-39)
🪛 GitHub Check: codecov/patch
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx
[warning] 3-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx#L3-L5
Added lines #L3 - L5 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx
[warning] 5-8: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx#L5-L8
Added lines #L5 - L8 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/content/page.tsx
[warning] 5-7: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/content/page.tsx#L5-L7
Added lines #L5 - L7 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/page.tsx
[warning] 5-5: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/page.tsx#L5
Added line #L5 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/page.tsx
[warning] 7-7: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/page.tsx#L7
Added line #L7 was not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/page.tsx
[warning] 2-4: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/page.tsx#L2-L4
Added lines #L2 - L4 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx
[warning] 10-10: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx#L10
Added line #L10 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx
[warning] 10-12: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/page.tsx#L10-L12
Added lines #L10 - L12 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/columns.tsx
[warning] 5-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/columns.tsx#L5
Added line #L5 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/resources/page.tsx
[warning] 2-4: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/resources/page.tsx#L2-L4
Added lines #L2 - L4 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx
[warning] 3-5: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx#L3-L5
Added lines #L3 - L5 were not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx
[warning] 3-3: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx#L3
Added line #L3 was not covered by tests
[warning] 23-23: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx#L23
Added line #L23 was not covered by tests
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/layout.tsx
[warning] 44-44: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/layout.tsx#L44
Added line #L44 was not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/columns.tsx
[warning] 5-5: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/columns.tsx#L5
Added line #L5 was not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx
[warning] 6-7: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx#L6-L7
Added lines #L6 - L7 were not covered by tests
[warning] 42-42: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx#L42
Added line #L42 was not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx
[warning] 3-3: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx#L3
Added line #L3 was not covered by tests
[warning] 23-25: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx#L23-L25
Added lines #L23 - L25 were not covered by tests
apps/upskii/src/components/custom-data-table.tsx
[warning] 3-7: apps/upskii/src/components/custom-data-table.tsx#L3-L7
Added lines #L3 - L7 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx
[warning] 5-6: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L5-L6
Added lines #L5 - L6 were not covered by tests
[warning] 9-14: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L9-L14
Added lines #L9 - L14 were not covered by tests
[warning] 16-17: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L16-L17
Added lines #L16 - L17 were not covered by tests
[warning] 20-36: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L20-L36
Added lines #L20 - L36 were not covered by tests
[warning] 56-60: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L56-L60
Added lines #L56 - L60 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx
[warning] 9-12: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L9-L12
Added lines #L9 - L12 were not covered by tests
[warning] 22-23: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L22-L23
Added lines #L22 - L23 were not covered by tests
[warning] 73-73: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L73
Added line #L73 was not covered by tests
[warning] 80-84: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L80-L84
Added lines #L80 - L84 were not covered by tests
[warning] 195-195: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L195
Added line #L195 was not covered by tests
🔇 Additional comments (59)
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/quiz-sets/[setId]/layout.tsx (1)
3-3: VerifyLinkButtonimport and usage.Importing
LinkButtonfrom the shared UI package centralizes component usage. Confirm that the public API forLinkButton(props likehref,title,icon, andclassName) hasn't changed with this migration so that this file compiles and behaves as expected.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx (4)
6-6: Approved: CentralizedCourseCardViewimport
The local card view component has been replaced with the shared package variant, improving consistency and maintainability across apps.
7-7: Approved: CentralizedCourseFormimport
Pulling in the form component from the UI package reduces duplication and ensures a uniform user experience.
8-8: Approved: CentralizedCoursePaginationimport
Standardizing pagination through the shared package will streamline styling and behavior across different listings.
10-10: Add test coverage for the newViewToggleimport
Static analysis flagged this line as uncovered. Please add unit or integration tests to validate that toggling between “card” and “table” views updates the URL search parameter and renders the correct component.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 10-10: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/page.tsx#L10
Added line #L10 was not covered by testsapps/web/src/app/[locale]/(dashboard)/[wsId]/drive/new-actions.tsx (1)
4-7: LGTM! Clean migration to shared UI package.The import change correctly moves from local components to the centralized
@tuturuuu/uipackage, aligning perfectly with the PR objectives.packages/ui/src/components/ui/custom/education/modules/resources/file-upload-form.tsx (2)
20-20: LGTM! Consistent migration to shared utilities.The import change correctly uses the centralized path utilities from
@tuturuuu/utils, maintaining consistency with the broader refactoring effort.
320-320: Good UX improvement with proper text handling.The addition of
max-w-[400px]andbreak-allclasses improves the display of long file names by constraining width and ensuring text wraps properly.packages/utils/src/path-helper.ts (2)
1-51: Excellent robust path joining implementation.The
joinPathfunction demonstrates comprehensive edge case handling:
- Proper empty input handling
- Preservation of relative/absolute path semantics
- Correct trailing slash behavior
- Filtering of invalid segments
The logic correctly handles complex scenarios like mixed relative/absolute paths while maintaining path integrity.
53-93: Well-implemented parent path extraction.The
popPathfunction provides robust parent directory extraction with:
- Proper path normalization
- Correct handling of relative vs absolute paths
- Edge case management for root and empty paths
- Preservation of original path format
The implementation thoughtfully handles various path scenarios while maintaining semantic correctness.
packages/ui/package.json (1)
60-60:❓ Verification inconclusive
Verify Framer Motion’s peerDependencies against React version
framer-motion@^12.16.0may have a React peerDependency (e.g., React 18). Please confirm it’s compatible with your React^19.1.0to avoid runtime mismatches.
🌐 Web query:
What are the peerDependencies of framer-motion@12.16.0?💡 Result:
The
framer-motionpackage version 12.16.0 specifies the following peer dependencies:
@emotion/is-prop-validreactreact-domThese dependencies are essential for the proper functioning of
framer-motion. The@emotion/is-prop-validpackage is optional, as indicated by its absence in thepeerDependenciesMetasection. (app.unpkg.com)It's important to note that
framer-motionis compatible with React versions 18 and 19. However, there have been reported issues with React 19, such as incompatibilities affecting type definitions and element animations. (github.com) Therefore, if you're using React 19, ensure that you have the latest versions of bothreactandreact-domto mitigate potential issues.Citations:
- 1: https://app.unpkg.com/framer-motion%4011.9.0/files/package.json?utm_source=openai
- 2: [BUG] Incompatible with React 19 motiondivision/motion#2668
Verify Framer Motion’s peerDependencies and React 19 compatibility
- File:
packages/ui/package.json(line 60)framer-motion@^12.16.0declares peerDependencies:
reactreact-dom@emotion/is-prop-valid- Although v12.16.0 supports React 18 and 19, there are known issues with React 19 (type definitions and element animations). Ensure your
reactandreact-dompackages are updated to ≥19.1.0 and test key animations to avoid runtime mismatches.apps/upskii/src/components/custom-data-table.tsx (1)
3-3: Re-export simplifies component usage
TheCustomDataTableis now delegated to the shared UI library. Confirm that the export path (@tuturuuu/ui/custom/tables/custom-data-table) is valid in thepackages/uipackage.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-7: apps/upskii/src/components/custom-data-table.tsx#L3-L7
Added lines #L3 - L7 were not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/content/page.tsx (1)
5-5: Centralize the editor import
Replacing the localModuleContentEditorwith the shared component from@tuturuuu/ui/text-editor/content-editoris correct. Ensure the external component’s API (courseId,moduleId,content) aligns with your usage here.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 5-7: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/content/page.tsx#L5-L7
Added lines #L5 - L7 were not covered by testsapps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/page.tsx (1)
5-5: Migrate to shared CourseModuleForm
ImportingCourseModuleFormfrom@tuturuuu/ui/custom/education/modules/course-module-formreplaces the local implementation appropriately. Verify that the new form component supports thewsIdandcourseIdprops as passed in<CourseModuleForm wsId={wsId} courseId={courseId} />.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 5-5: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/page.tsx#L5
Added line #L5 was not covered by testsapps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/columns.tsx (1)
5-5: Use centralized row actions
Updating the import to@tuturuuu/ui/custom/education/courses/course-row-actionsforWorkspaceCourseRowActionsaligns with the new package structure. Ensure the external component’s API matches its usage in theactionscolumn.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx (1)
5-5: Align row actions import
Switching to the sharedWorkspaceCourseRowActionsfrom@tuturuuu/ui/custom/education/courses/course-row-actionsis correct. Confirm that the package export and the component props remain compatible with this table’sactionscell.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 5-8: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/columns.tsx#L5-L8
Added lines #L5 - L8 were not covered by testsapps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/columns.tsx (1)
5-5: Import shared row actions
Switched to using the centralizedWorkspaceCourseModuleRowActionsfrom the@tuturuuu/uipackage, improving consistency and reducing duplication.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 5-5: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/columns.tsx#L5
Added line #L5 was not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/page.tsx (1)
7-7: Use shared CourseModuleForm component
Replaced local form import with the package version to align with the shared UI library migration.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 7-7: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/page.tsx#L7
Added line #L7 was not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/columns.tsx (1)
5-5: Centralize module row actions import
Updated import to pullWorkspaceCourseModuleRowActionsfrom the shared UI package for consistency.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 5-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/columns.tsx#L5
Added line #L5 was not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx (1)
3-5: Migrate YouTube components to shared UI package
All YouTube-related components (DeleteLinkButton,YoutubeEmbed,YouTubeLinkForm) now come from the@tuturuuu/uilibrary, reducing local duplication.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-5: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/youtube-links/page.tsx#L3-L5
Added lines #L3 - L5 were not covered by testsapps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/resources/page.tsx (1)
2-4: Switch resource components to shared package
Replaced local resource UI components (DeleteResourceButton,FileDisplay,StorageObjectForm) with imports from@tuturuuu/uito consolidate implementations.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-4: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/resources/page.tsx#L2-L4
Added lines #L2 - L4 were not covered by testsapps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/page.tsx (1)
2-4:✅ Verification successful
Import migration looks good - verify component availability.
The import path updates successfully migrate these components to the centralized UI package, aligning with the PR objectives. The component usage remains unchanged, which ensures functional consistency.
Run this script to verify the migrated components exist at their new package locations:
🏁 Script executed:
#!/bin/bash # Description: Verify that the migrated UI components exist in the packages/ui directory echo "Checking for DeleteResourceButton component..." fd "delete-resource" packages/ui --type f --exec cat {} \; | head -10 echo -e "\nChecking for FileDisplay component..." fd "file-display" packages/ui --type f --exec cat {} \; | head -10 echo -e "\nChecking for StorageObjectForm component..." fd "file-upload-form" packages/ui --type f --exec cat {} \; | head -10Length of output: 1483
Import migration successful – all components verified
I confirmed that the following components are exported from the new
@tuturuuu/uipackage paths:
DeleteResourceButtoninpackages/ui/custom/education/modules/resources/delete-resourceFileDisplayinpackages/ui/custom/education/modules/resources/file-displayStorageObjectForminpackages/ui/custom/education/modules/resources/file-upload-formNo further changes needed.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 2-4: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/resources/page.tsx#L2-L4
Added lines #L2 - L4 were not covered by testspackages/ui/src/components/ui/custom/education/courses/course-card-view.tsx (1)
78-79: Consistent scoped translation key usage.The fallback text properly uses the new scoped translation key, maintaining consistency with the no_courses_found key update.
packages/ui/src/components/ui/text-editor/content-editor.tsx (1)
16-16:✅ Verification successful
Export standardization improves consistency.
The change from default to named export aligns with the standardization pattern across the migrated UI components.
Run this script to verify all imports of ModuleContentEditor use the correct named import syntax:
🏁 Script executed:
#!/bin/bash # Description: Verify that all imports of ModuleContentEditor use named import syntax echo "Searching for ModuleContentEditor imports..." rg "import.*ModuleContentEditor" -A 1 -B 1 echo -e "\nChecking for any remaining default imports that need updating..." rg "import ModuleContentEditor" --type ts --type tsxLength of output: 1379
Let’s adjust the import check to use glob patterns so we include both .ts and .tsx files:
🏁 Script executed:
#!/bin/bash echo "Re-verifying default imports for ModuleContentEditor across TS and TSX files..." rg "import\s\+ModuleContentEditor\b" -g '*.ts' -g '*.tsx' || echo "No default imports found; all imports are correctly named."Length of output: 355
All ModuleContentEditor imports now use named exports
Verification across
.tsand.tsxfiles confirms no remaining default imports. The change in
packages/ui/src/components/ui/text-editor/content-editor.tsxis fully propagated:
- apps/web/src/app/[locale]/…/content/page.tsx
- apps/upskii/src/app/[locale]/…/content/page.tsx
No further action required—approving this change.
packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx (1)
6-6: Shared CourseForm import applied
Import updated to the centralized UI package path. ConfirmCourseFormis a named export in@tuturuuu/ui/custom/education/courses/course-form.apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/layout.tsx (1)
6-7: Shared UI import migration validated
TheLinkButtondefault import andModuleTogglesnamed import now reference the shared UI package. Ensure the package exports match these import styles.packages/ui/src/components/ui/custom/education/modules/course-module-row-actions.tsx (1)
6-6: CourseModuleForm import migration approved
Import updated to the shared UI package. Confirm@tuturuuu/ui/custom/education/modules/course-module-formexportsCourseModuleFormas a named export.apps/upskii/messages/en.json (1)
3527-3530: Scoped localization keys are correctly added under ws-courses.Adding
table_view,no_courses_found, andno_description_providedunder thews-coursesnamespace aligns with the shared UI package and prevents key collisions.apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx (2)
6-7: LGTM! Import paths successfully migrated to centralized UI package.The imports have been correctly updated to use the centralized UI components from
@tuturuuu/ui/custom/education/modules, which improves code modularity and reusability across the codebase.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 6-7: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx#L6-L7
Added lines #L6 - L7 were not covered by tests
42-42: Good refactoring! Path construction improved using commonHref variable.Using the
commonHrefvariable for path construction reduces duplication and makes the code more maintainable compared to the previous manual string concatenation approach.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 42-42: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/layout.tsx#L42
Added line #L42 was not covered by testspackages/ui/src/components/ui/custom/education/modules/resources/file-display.tsx (2)
11-14: LGTM! Dynamic import path correctly updated for centralized PDF viewer.The dynamic import has been properly updated to reference the PDFViewer component from its new location in the UI package, maintaining the same SSR disabling behavior.
20-20: Export style changed from default to named export.The component export has been changed from default to named export. This is a breaking change, but appears to be part of the coordinated UI migration effort where all consumers have been updated accordingly.
apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx (2)
3-3: LGTM! CourseHeader import correctly added from UI package.The import path for the CourseHeader component follows the expected pattern for the centralized UI components.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-3: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx#L3
Added line #L3 was not covered by tests
23-23: LGTM! CourseHeader usage is correct.The CourseHeader component is properly instantiated with the required props:
data(UserGroup type from getData function)href(correctly constructed course URL path)This matches the expected interface shown in the relevant code snippet.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 23-23: apps/upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/layout.tsx#L23
Added line #L23 was not covered by testsapps/web/messages/vi.json (2)
285-289: New common UI translation keys added correctly.The entries for
"sidebar","whats_the_title","write_something","error_saving_content", and"not_completed"align with the corresponding English keys and follow the existing JSON structure without syntax issues.
1992-1998: ws-courses localization keys are correctly scoped.The new keys under
"ws-courses"—including"edit_description","course_description","card_view","table_view","no_courses_found","no_description_provided", and"delete_confirm_title"—match their English counterparts and properly escape the{name}placeholder.apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx (1)
3-3: LGTM! Clean migration to shared component.The import of the shared
CourseHeadercomponent aligns well with the PR objective of migrating common UI components to the packages/ui directory.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 3-3: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/layout.tsx#L3
Added line #L3 was not covered by testspackages/ui/src/components/ui/custom/education/courses/course-header.tsx (1)
27-27:✅ Verification successful
Verify the translation key 'common.unknown' exists.
Ensure the fallback translation key is available across all locales to prevent missing translation errors.
🏁 Script executed:
#!/bin/bash # Description: Verify the translation key exists in all locale files # Search for the common.unknown translation key rg -A 2 -B 2 '"unknown"' apps/*/messages/*.json packages/*/messages/*.json || echo "Translation key not found" # Check the structure of common translations rg -A 5 '"common"' apps/*/messages/*.json packages/*/messages/*.json | head -20Length of output: 5404
common.unknown translation verified across all locales
All applications (calendar, famigo, nova, rewise, upskii, web) definecommon.unknownin bothen.jsonandvi.jsonunder theircommonsection. No missing‐translation risk detected—no further action required.apps/web/messages/en.json (1)
288-291: Review new common keys for translation consistency
The keyswhats_the_title,write_something,error_saving_content, andnot_completedare newly added undercommon. Please verify they are also present (and translated) in all other locale files (e.g.,vi.json,apps/upskii/messages/en.json).apps/upskii/messages/vi.json (3)
3528-3529: Approve translations for empty-state messages
The translations forno_courses_found(“Chưa có khóa học nào”) andno_description_provided(“Không có mô tả”) are accurate and contextually appropriate.
3527-3527:✅ Verification successful
Verify
table_viewkey alignment
Ensure thetable_viewkey matches the corresponding key inen.json(it might betable-viewortable_view) to avoid locale mismatches.
🏁 Script executed:
rg '"table[-_]view"' -n apps/upskii/messages/en.jsonLength of output: 91
No action needed:
table_viewkey is aligned
Thetable_viewkey in apps/upskii/messages/vi.json matches the corresponding entry in apps/upskii/messages/en.json (both at line 3527). No changes required.
1-4138:✅ Verification successful
Verify removal of deprecated top-level keys
Confirm that the old top-level entries"no-courses-found"and"no-description-provided"have been removed to prevent fallback translation conflicts.
🏁 Script executed:
rg -e '"no-courses-found"' -e '"no-description-provided"' -n apps/upskii/messages/vi.jsonLength of output: 91
Removal of deprecated keys confirmed
No occurrences of"no-courses-found"or"no-description-provided"remain inapps/upskii/messages/vi.json.apps/web/src/components/custom-data-table.tsx (1)
3-3: LGTM! Clean migration to shared UI package.The re-export pattern maintains backward compatibility while centralizing the
CustomDataTableimplementation in the shared UI library. This improves maintainability and code reuse across the application.apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx (3)
9-12: LGTM! Proper migration to shared UI components.The import updates to use components from
@tuturuuu/ui/custom/education/modulesalign well with the centralization effort, improving code reusability and maintainability.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 9-12: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L9-L12
Added lines #L9 - L12 were not covered by tests
73-73: Good UX improvement with Accordion wrapper.Using
Accordionwithtype="multiple"allows users to expand multiple course sections simultaneously, providing a better user experience for exploring module content.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 73-73: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L73
Added line #L73 was not covered by tests
80-84: Proper integration of RichTextEditor for content display.The
RichTextEditorwithreadOnlyprop and proper typing asJSONContentis well-implemented. The scrollable container with max height prevents excessive vertical space usage.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 80-84: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/page.tsx#L80-L84
Added lines #L80 - L84 were not covered by testspackages/ui/src/hooks/useSearchParams.tsx (2)
39-62: Excellent parameter manipulation methods with proper performance optimization.The
set,add, andremovemethods are well-implemented with:
- Proper use of
useCallbackfor performance- Correct dependency arrays
- Appropriate handling of array values
- Optional refresh parameter for flexibility
112-124: Well-structured return object with comprehensive API.The hook provides a clean, comprehensive API with:
- Consistent naming conventions
- Alias for
clear→reset- Good balance of utility methods
- Proper TypeScript typing
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/page.tsx (6)
2-2: Mock data import commented out appropriately.Good practice to keep the mock import commented for potential development use while maintaining clean production code.
6-10: Well-structured migration to shared UI components.The import changes properly migrate from local components to the shared @tuturuuu/ui package, which aligns perfectly with the PR objective. The new components (CourseCardView, CourseForm, CoursePagination, ViewToggle) provide enhanced functionality for the course management interface.
20-20: Type-safe view parameter added.The union type
'card' | 'table'provides good type safety for the view toggle functionality.
53-93: Excellent implementation of view toggle functionality.The conditional rendering logic is clean and well-structured. The layout properly accommodates both card and table views with appropriate spacing and components. The pagination integration is correctly implemented for the card view.
107-125: Mock data logic properly preserved for development.The commented mock data implementation is well-structured and provides a good fallback for development environments. This allows for testing pagination and filtering logic without requiring database access.
159-159: Excellent type safety improvement.The explicit return type annotation enhances type safety and makes the function's contract clear, especially with the added
modulesproperty. This helps prevent type-related issues and improves IDE support.apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx (5)
1-1: LGTM! Import migration looks good.The migration from local imports to shared
@tuturuuu/uipackage imports aligns well with the PR objectives for centralizing UI components.Also applies to: 5-6
9-14: LGTM! Proper typing for Next.js app router.The
Propsinterface correctly types the async params pattern used in Next.js 13+ app router.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 9-14: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L9-L14
Added lines #L9 - L14 were not covered by tests
16-17: LGTM! Correct async pattern for Next.js app router.The function signature and params destructuring follow the proper Next.js app router async component pattern.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 16-17: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L16-L17
Added lines #L16 - L17 were not covered by tests
56-60: LGTM! Clean component integration.The
ModuleContentEditorintegration looks clean with proper prop passing ofcourseId,moduleId, and fetchedcontent.🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 56-60: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L56-L60
Added lines #L56 - L60 were not covered by tests
20-26:❓ Verification inconclusive
Consider adding authorization checks.
The database query fetches content based on
moduleIdandcourseIdbut doesn't verify user permissions. Ensure that upstream middleware or route guards validate user access to this specific course and module.
🏁 Script executed:
#!/bin/bash # Description: Check for authorization/permission validation in the route # Expected: Find middleware, guards, or permission checks for course/module access rg -A 5 -B 5 "auth|permission|guard|middleware" apps/web/src/app/\[locale\]/\(dashboard\)/\[wsId\]/education/courses/\[courseId\]/modules/\[moduleId\]/Length of output: 145
I’ll search for any Next.js middleware or route handler files in the app directory to see if access is being enforced elsewhere.
🏁 Script executed:
#!/bin/bash # Search for Next.js middleware and route handlers in the app directory find apps/web/src/app -type f \( -iname 'middleware.ts' -o -iname 'route.ts' -o -iname 'route.tsx' \)Length of output: 14386
I’ll inspect the dashboard layout for any auth guards and also pull in the
createClient()implementation to see if it already ties into user sessions.
🏁 Script executed:
#!/bin/bash # 1) Show the (dashboard) layout.tsx to check for any AuthGuard or session checks find apps/web/src/app -type f -path 'apps/web/src/app/*/(dashboard)/layout.tsx' -print -exec head -n 50 {} \; # 2) Locate and print the wrapper for createClient() to see how it handles auth grep -R "export function createClient" -n apps/web/src grep -R "createClient(" -n apps/web/src/lib apps/web/srcLength of output: 65485
Verify and enforce workspace/course access before loading module content
I didn’t find any permission checks in either the content-page or your global middleware. While authentication is enforced, there’s no validation that the signed-in user belongs to this workspace or is enrolled in the specified course/module.
• apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx
• apps/web/src/middleware.tsPlease confirm that Supabase RLS policies restrict
workspace_course_modulesto authorized users, or add one of the following:
- a filter on
workspace_id(or a membership join) in your query- a guard in the relevant layout.tsx or custom middleware
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-36: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L20-L36
Added lines #L20 - L36 were not covered by tests
packages/ui/src/components/ui/custom/education/courses/course-card-view.tsx
Show resolved
Hide resolved
packages/ui/src/components/ui/custom/education/courses/course-row-actions.tsx
Show resolved
Hide resolved
.../upskii/src/app/[locale]/(dashboard)/[wsId]/courses/[courseId]/modules/[moduleId]/layout.tsx
Show resolved
Hide resolved
| } | ||
| description={ | ||
| <div className="mt-2 line-clamp-2"> | ||
| {data.description || t('ws-courses.no_description_provided')} |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the translation key 'ws-courses.no_description_provided' exists.
Ensure this specific translation key is available to prevent runtime errors when course descriptions are missing.
🏁 Script executed:
#!/bin/bash
# Description: Verify the ws-courses translation keys exist
# Search for ws-courses translation namespace
rg -A 10 '"ws-courses"' apps/*/messages/*.json packages/*/messages/*.json
# Look for the specific no_description_provided key
rg '"no_description_provided"' apps/*/messages/*.json packages/*/messages/*.jsonLength of output: 8811
Add missing translation key ws-courses.no_description_provided across all apps
The key is defined in:
- apps/web/messages/{en.json, vi.json}
- apps/upskii/messages/{en.json, vi.json}
But it’s missing in:
- apps/calendar/messages/{en.json, vi.json}
- apps/famigo/messages/{en.json, vi.json}
- apps/nova/messages/{en.json, vi.json}
- apps/rewise/messages/{en.json, vi.json}
Please add the following under the ws-courses namespace in each of those files:
"no_description_provided": "No description provided"(and the corresponding localized text in vi.json)
🤖 Prompt for AI Agents
In packages/ui/src/components/ui/custom/education/courses/course-header.tsx at
line 33, the translation key 'ws-courses.no_description_provided' is used but
missing in some app localization files. To fix this, add the key
"no_description_provided": "No description provided" under the "ws-courses"
namespace in the en.json and vi.json files of apps/calendar, apps/famigo,
apps/nova, and apps/rewise to ensure consistent translations and prevent runtime
errors.
| const pageSize = Number(searchParams.get('pageSize') || 10); | ||
| const page = Number(searchParams.get('page') || 0); | ||
| const pageIndex = page > 0 ? page - 1 : 0; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix potential issues with URL parameter parsing and page indexing.
The current implementation has several concerns:
- No validation for invalid number inputs
- Page indexing logic may be confusing (page 0 vs page 1)
- Default values could be inconsistent
- const pageSize = Number(searchParams.get('pageSize') || 10);
- const page = Number(searchParams.get('page') || 0);
- const pageIndex = page > 0 ? page - 1 : 0;
+ const pageSize = Math.max(1, Number(searchParams.get('pageSize')) || 10);
+ const page = Math.max(1, Number(searchParams.get('page')) || 1);
+ const pageIndex = page - 1; // Convert 1-based page to 0-based indexThis ensures:
- Minimum page size of 1
- 1-based page numbering for URLs (more user-friendly)
- Consistent conversion to 0-based indexing
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const pageSize = Number(searchParams.get('pageSize') || 10); | |
| const page = Number(searchParams.get('page') || 0); | |
| const pageIndex = page > 0 ? page - 1 : 0; | |
| const pageSize = Math.max(1, Number(searchParams.get('pageSize')) || 10); | |
| const page = Math.max(1, Number(searchParams.get('page')) || 1); | |
| const pageIndex = page - 1; // Convert 1-based page to 0-based index |
🤖 Prompt for AI Agents
In packages/ui/src/components/ui/custom/tables/custom-data-table.tsx around
lines 23 to 25, improve the parsing of URL parameters by validating that
pageSize and page are valid positive numbers, defaulting pageSize to at least 1.
Adjust the page variable to use 1-based indexing from the URL, then convert it
consistently to 0-based pageIndex for internal use. This ensures robust handling
of invalid inputs and clearer, user-friendly page numbering.
| <Suspense fallback={null}> | ||
| <DataTable | ||
| t={t} | ||
| hideToolbar={hideToolbar} | ||
| hidePagination={hidePagination} | ||
| namespace={namespace} | ||
| pageIndex={pageIndex || 0} | ||
| pageSize={pageSize || 10} | ||
| onRefresh={() => router.refresh()} | ||
| defaultQuery={searchParams.getSingle('q', '')} | ||
| onSearch={(query: string) => | ||
| query | ||
| ? searchParams.set({ q: query, page: '1' }) | ||
| : searchParams.reset() | ||
| } | ||
| setParams={(params) => searchParams.set(params)} | ||
| resetParams={() => searchParams.reset()} | ||
| isEmpty={searchParams.isEmpty} | ||
| newObjectTitle={t('common.create')} | ||
| className={className} | ||
| {...props} | ||
| /> | ||
| </Suspense> | ||
| ); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify the useSearchParams hook implementation.
The component heavily relies on a custom useSearchParams hook that provides methods like set, reset, getSingle, etc. Ensure this hook is properly implemented and tested.
🏁 Script executed:
#!/bin/bash
# Description: Verify the useSearchParams hook implementation and usage
# Find the useSearchParams hook implementation
fd "useSearchParams.tsx" packages/ui/src/hooks/ --exec cat {} \;
# Check for other usages of this hook to understand patterns
rg -A 5 -B 2 "useSearchParams" packages/ui/src/ --type tsx --type tsLength of output: 3321
🏁 Script executed:
#!/bin/bash
# Check for existing tests referencing useSearchParams
rg "useSearchParams" --glob "*.{test,spec}.{ts,tsx}" -n packages/uiLength of output: 67
Add unit tests for useSearchParams hook
We rely heavily on the custom hook at packages/ui/src/hooks/useSearchParams.tsx (methods: has, get, getSingle, set, add, remove, reset/clear, getAll, isEmpty), but no tests currently cover its behavior. Please add unit tests to verify:
- Fallback values returned by
getSingleandget - URL updates and refresh calls in
set,add, andremove - Reset behavior via
reset/clear - Correct mapping from
getAll isEmptyflag toggles as expected
Path to cover:
- packages/ui/src/hooks/useSearchParams.tsx
🤖 Prompt for AI Agents
In packages/ui/src/hooks/useSearchParams.tsx, add comprehensive unit tests to
cover the custom hook's behavior. Write tests to verify fallback values returned
by getSingle and get methods, ensure URL updates and refresh calls occur
correctly when using set, add, and remove methods, validate reset and clear
methods properly reset the state, confirm getAll returns the correct mapping of
parameters, and check that the isEmpty flag toggles as expected. Use a testing
framework like Jest and mock necessary dependencies such as router or URL
handling to simulate and assert these behaviors.
| const get = useCallback( | ||
| (data: { key: string; fallbackValue?: string | string[] } | string) => { | ||
| if (typeof data === 'string') return searchParams.getAll(data) || ''; | ||
|
|
||
| const { key, fallbackValue } = data; | ||
| return searchParams.getAll(key).length > 0 | ||
| ? searchParams.getAll(key) || fallbackValue | ||
| : fallbackValue; | ||
| }, | ||
| [searchParams] | ||
| ); |
There was a problem hiding this comment.
Fix inconsistent return types and logic in the get method.
The get method has several issues:
- Inconsistent return types: Returns
string[]orstring | string[] | undefineddepending on the input - Logic flaw: Line 22 returns
string[]but the fallback''is a string - Redundant condition: Line 25-26 checks length > 0 then calls
getAllagain
const get = useCallback(
- (data: { key: string; fallbackValue?: string | string[] } | string) => {
- if (typeof data === 'string') return searchParams.getAll(data) || '';
-
- const { key, fallbackValue } = data;
- return searchParams.getAll(key).length > 0
- ? searchParams.getAll(key) || fallbackValue
- : fallbackValue;
+ (data: { key: string; fallbackValue?: string[] } | string): string[] => {
+ if (typeof data === 'string') {
+ return searchParams.getAll(data);
+ }
+
+ const { key, fallbackValue = [] } = data;
+ const values = searchParams.getAll(key);
+ return values.length > 0 ? values : fallbackValue;
},
[searchParams]
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const get = useCallback( | |
| (data: { key: string; fallbackValue?: string | string[] } | string) => { | |
| if (typeof data === 'string') return searchParams.getAll(data) || ''; | |
| const { key, fallbackValue } = data; | |
| return searchParams.getAll(key).length > 0 | |
| ? searchParams.getAll(key) || fallbackValue | |
| : fallbackValue; | |
| }, | |
| [searchParams] | |
| ); | |
| const get = useCallback( | |
| (data: { key: string; fallbackValue?: string[] } | string): string[] => { | |
| if (typeof data === 'string') { | |
| return searchParams.getAll(data); | |
| } | |
| const { key, fallbackValue = [] } = data; | |
| const values = searchParams.getAll(key); | |
| return values.length > 0 ? values : fallbackValue; | |
| }, | |
| [searchParams] | |
| ); |
🤖 Prompt for AI Agents
In packages/ui/src/hooks/useSearchParams.tsx around lines 20 to 30, the get
method returns inconsistent types by mixing string arrays and strings, returns
an empty string fallback where a string array is expected, and redundantly calls
searchParams.getAll twice. Fix this by ensuring the method consistently returns
string arrays or the fallback, replace the empty string fallback with an empty
array, and store the result of searchParams.getAll in a variable to avoid
duplicate calls and simplify the conditional logic.
| const getContent = async (courseId: string, moduleId: string) => { | ||
| const supabase = await createClient(); | ||
| const { data, error } = await supabase | ||
| .from('workspace_course_modules') | ||
| .select('content') | ||
| .eq('id', moduleId) | ||
| .eq('course_id', courseId); | ||
|
|
||
| if (error) { | ||
| console.error(error); | ||
| } | ||
|
|
||
| return data?.[0]?.content as JSONContent; | ||
| }; | ||
|
|
||
| const content = await getContent(courseId, moduleId); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and consider function placement.
The content fetching logic has several areas for improvement:
- Error handling: Currently only logs errors to console, which isn't sufficient for production
- Function placement: Defining the function inside the component isn't optimal
- Type safety: The
as JSONContentassertion could be unsafe without validation
Consider this refactor:
- const getContent = async (courseId: string, moduleId: string) => {
- const supabase = await createClient();
- const { data, error } = await supabase
- .from('workspace_course_modules')
- .select('content')
- .eq('id', moduleId)
- .eq('course_id', courseId);
-
- if (error) {
- console.error(error);
- }
-
- return data?.[0]?.content as JSONContent;
- };
+ const supabase = await createClient();
+ const { data, error } = await supabase
+ .from('workspace_course_modules')
+ .select('content')
+ .eq('id', moduleId)
+ .eq('course_id', courseId)
+ .single();
+
+ if (error) {
+ console.error('Failed to fetch module content:', error);
+ // Consider redirecting to error page or showing error UI
+ throw new Error('Failed to load module content');
+ }
+
+ const content = data?.content as JSONContent;Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: codecov/patch
[warning] 20-36: apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx#L20-L36
Added lines #L20 - L36 were not covered by tests
🤖 Prompt for AI Agents
In
apps/web/src/app/[locale]/(dashboard)/[wsId]/education/courses/[courseId]/modules/[moduleId]/content/page.tsx
around lines 20 to 36, improve error handling by throwing or properly handling
the error instead of just logging it, move the getContent function outside the
component to avoid redefining it on each render, and add validation or type
checking before asserting the returned data as JSONContent to ensure type
safety.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Chores