Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive refactor of the admin page, introducing modern UI components, improved authentication flows, and a new post editor experience.
Key Changes:
- Replaced CKEditor with BlockNote editor for enhanced content editing
- Implemented dashboards for both admin and author roles with charts and statistics
- Added JWT refresh token mechanism with queue-based request handling
- Enhanced admin authentication UI with modern animations and styling
Reviewed changes
Copilot reviewed 39 out of 41 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
src/services/authServices/AdminAuthServices.js |
Refactored to use centralized API endpoints and added refresh token method |
src/configs/axiosConfig.js |
Completely rewritten to support JWT refresh token logic with request queuing |
src/configs/axiosUserConfig.js |
New file separating user-specific axios configuration |
src/pages/auth-pages/admin-auth/Login.jsx |
Redesigned with modern UI, enhanced validation, and role-based routing |
src/pages/app-pages/admin-pages/Dashboard.jsx |
New admin dashboard with statistics cards and chart visualizations |
src/pages/app-pages/author-pages/Dashboard.jsx |
New author dashboard with performance metrics and recent posts |
src/pages/app-pages/admin-pages/post/Create.jsx |
Integrated BlockNote editor and added live preview functionality |
src/pages/app-pages/admin-pages/post/Edit.jsx |
Integrated BlockNote editor with live preview and improved UX |
src/components/BlockNoteEditor/index.jsx |
New rich text editor component with fullscreen mode and HTML conversion |
src/components/PostPreview/index.jsx |
New component for real-time post preview during editing |
src/layouts/auth-layouts/admin-layouts/index.jsx |
Redesigned with gradient backgrounds and modern card styling |
src/layouts/app-layouts/admin-layouts/SideBar.jsx |
Enhanced navigation with proper URL synchronization and state management |
src/constants/routeConstants.js |
New centralized route constants for better maintainability |
src/configs/apiEndpoints.js |
Centralized API endpoint definitions |
src/validation/auth/adminAuthValidation.js |
Comprehensive validation rules for admin authentication |
src/styles/admin-auth.css |
New CSS animations for login page |
src/services/adminServices/dashboardServices.js |
Service layer for dashboard data with date formatting utilities |
src/components/Dashboard/StatCard.jsx |
Reusable statistics card component with gradient support |
src/components/Dashboard/ChartCard.jsx |
Reusable chart wrapper component |
package.json |
Added BlockNote, Chart.js, and styled-components dependencies |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onChange: () => { | ||
| handleEditorChange(); | ||
| }, |
There was a problem hiding this comment.
The editor change handler is called directly in the onChange callback, which triggers on every keystroke. This approach bypasses the debouncing logic defined in the useEffect hook (lines 85-112). Consider removing the direct call here and relying solely on the debounced subscription to avoid performance issues with rapid state updates.
| onChange: () => { | |
| handleEditorChange(); | |
| }, | |
| // Removed direct onChange handler to rely on debounced logic in useEffect |
|
|
||
| try { | ||
| const blocks = editor.document; | ||
| const html =editor.blocksToHTMLLossy(blocks); |
There was a problem hiding this comment.
The typo in line 72: editor.blocksToHTMLLossy is missing a space after html =. It should be const html = editor.blocksToHTMLLossy(blocks);
| const html =editor.blocksToHTMLLossy(blocks); | |
| const html = editor.blocksToHTMLLossy(blocks); |
| if (timeoutId) { | ||
| clearTimeout(timeoutId); | ||
| } | ||
| if (unsubscribe) { |
There was a problem hiding this comment.
The unsubscribe function might not exist if editor.onChange doesn't return an unsubscribe function. Consider checking if unsubscribe is a function before calling it: if (typeof unsubscribe === 'function') { unsubscribe(); }
| if (unsubscribe) { | |
| if (typeof unsubscribe === 'function') { |
| setRecentPosts([ | ||
| { | ||
| id: 1, | ||
| title: "Hướng dẫn sử dụng React Hooks hiệu quả", | ||
| status: "published", | ||
| views: 1250, | ||
| created_at: "2025-10-20T10:30:00Z", | ||
| category: "Lập trình" | ||
| }, | ||
| { | ||
| id: 2, | ||
| title: "10 mẹo tối ưu hóa hiệu suất website", | ||
| status: "draft", | ||
| views: 0, | ||
| created_at: "2025-10-19T15:45:00Z", | ||
| category: "Web Development" | ||
| }, | ||
| { | ||
| id: 3, | ||
| title: "Xu hướng công nghệ 2025", | ||
| status: "published", | ||
| views: 890, | ||
| created_at: "2025-10-18T09:20:00Z", | ||
| category: "Công nghệ" | ||
| }, | ||
| { | ||
| id: 4, | ||
| title: "Machine Learning cơ bản cho người mới", | ||
| status: "published", | ||
| views: 2100, | ||
| created_at: "2025-10-17T14:10:00Z", | ||
| category: "AI/ML" | ||
| } | ||
| ]); |
There was a problem hiding this comment.
The hardcoded date "2025-10-20T10:30:00Z" and similar dates in this mock data are in the past relative to the current date (December 9, 2025). This might cause confusion when displaying "recent" posts. Consider using dynamic dates relative to the current date, like new Date().toISOString() or calculating dates relative to today.
| try { | ||
| const stored = localStorage.getItem("admin"); | ||
| return stored ? JSON.parse(stored) : null; | ||
| } catch (e) { |
There was a problem hiding this comment.
The error handling for JSON parsing uses a try-catch block which is good, but it returns null on error without logging or notifying the user. This could lead to silent failures when localStorage contains corrupted data. Consider adding error logging: console.error('Failed to parse admin data from localStorage:', e);
| } catch (e) { | |
| } catch (e) { | |
| console.error('Failed to parse admin data from localStorage:', e); |
src/components/Form/FormField.jsx
Outdated
| @@ -0,0 +1,60 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
The component imports React but never uses it. With modern React (17+), this import is unnecessary. Consider removing it.
| import React from 'react'; |
| return tag?.label || id; | ||
| }); | ||
| }; | ||
|
|
There was a problem hiding this comment.
[nitpick] The variable name editorError has been removed but editorContent state is still being used. However, the validation is now handled by the form rules. Consider clarifying the comment or documentation to indicate that content validation is now managed by the Form component's validation system rather than custom state.
| // Note: Content validation is now managed by the Form component's validation system, | |
| // not by custom state or error variables. The editorContent state is used for preview and data handling. |
| // title: () => <ColumnSort type="id" title="ID" handleSort={handleSort} />, | ||
| dataIndex: "id", | ||
| key: "id", | ||
| width: 200, |
There was a problem hiding this comment.
[nitpick] The width property is set to a fixed pixel value (200px) which may not work well on smaller screens or with responsive layouts. Consider using a percentage-based width or making it configurable, or testing this value across different viewport sizes to ensure it doesn't cause layout issues.
| }); | ||
| } else { | ||
| if (error?.response?.status === 401) { | ||
| if (error?.response?.data?.cause === 'JsonWebTokenError') { |
There was a problem hiding this comment.
Security concern: When JWT token errors occur (line 58-61), the code removes the admin token and reloads the page without any user notification or proper logout flow. This abrupt behavior could be confusing for users. Consider showing a notification message before reload: notification.warning({ message: 'Phiên đăng nhập hết hạn', description: 'Vui lòng đăng nhập lại' }); before window.location.reload();
| if (error?.response?.data?.cause === 'JsonWebTokenError') { | |
| if (error?.response?.data?.cause === 'JsonWebTokenError') { | |
| notification.warning({ | |
| message: 'Phiên đăng nhập hết hạn', | |
| description: 'Vui lòng đăng nhập lại' | |
| }); |
| @@ -1,11 +1,11 @@ | |||
| import adminAuthServices from "@/services/authServices/adminAuthServices"; | |||
| import AdminAuthServices from "@/services/authServices/AdminAuthServices"; | |||
There was a problem hiding this comment.
Unused import AdminAuthServices.
7198119 to
7ce6d53
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 41 changed files in this pull request and generated 16 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/context/AdminContext.jsx
Outdated
| } | ||
| }); | ||
|
|
||
| const contextValue = useMemo(() => ({ admin, setAdmin }), [admin, setAdmin]); |
There was a problem hiding this comment.
The useMemo dependency array includes setAdmin, but setAdmin from context/state setters should be stable and doesn't need to be in the dependency array. Including it is unnecessary and could cause extra re-renders if the reference changes.
| const contextValue = useMemo(() => ({ admin, setAdmin }), [admin, setAdmin]); | |
| const contextValue = useMemo(() => ({ admin, setAdmin }), [admin]); |
| useEffect(() => { | ||
| clearAuthData(); | ||
| }, []); |
There was a problem hiding this comment.
The useEffect dependency array is missing setAdmin and navigate. While these are typically stable references, it's better to include all dependencies or use ESLint disable comments with justification to avoid potential issues.
| processQueue(refreshError, null); | ||
| failedQueue = []; // Clear queue on error | ||
| localStorage.removeItem("admin"); | ||
| globalThis.location.reload(); |
There was a problem hiding this comment.
Using globalThis.location.reload() forces a full page reload. Consider navigating to the login page instead to preserve the SPA experience and avoid losing application state unnecessarily.
| try { | ||
| const response = await AdminAuthServices.refreshToken(); | ||
| const token = response?.accessToken; | ||
|
|
||
| if (token) { | ||
| const authData = { token }; | ||
| const authString = JSON.stringify(authData); | ||
| localStorage.setItem("admin", authString); | ||
|
|
||
| // Cập nhật header cho request hiện tại | ||
| originalRequest.headers.Authorization = `Bearer ${token}`; | ||
|
|
||
| // Xử lý tất cả requests đang chờ | ||
| processQueue(null, token); | ||
|
|
||
| // Retry request ban đầu | ||
| return axiosInstance(originalRequest); |
There was a problem hiding this comment.
Security concern: The token refresh logic could be exploited if an attacker can force 401 errors. The code doesn't validate the refresh token response before storing it. Add validation to ensure the response contains a valid token structure before saving to localStorage.
| </Form.Item> | ||
| </Flex> | ||
| <UploadPhotoInput propUpload={propUpload} /> | ||
| <ButtonAddForm loading={loading} /> |
There was a problem hiding this comment.
Missing PropTypes validation for propUpload prop. This component receives a propUpload prop but doesn't validate it, which could lead to runtime errors if incorrect data is passed.
| thumbnail: response?.photo || null | ||
| }); | ||
| } catch (error) { | ||
| console.log(error); |
There was a problem hiding this comment.
Empty catch block silently swallows errors. At minimum, log the error to help with debugging. Consider also showing a user-friendly error message.
| console.log(error); | |
| console.log(error); | |
| message.error("Không thể tải dữ liệu bài viết. Vui lòng thử lại sau."); |
| const getOpenKeysFromSelectedKey = (key) => { | ||
| if (!key) return []; | ||
|
|
||
| // Tìm parent key từ selected key | ||
| // Ví dụ: "2-1" -> parent là "2" | ||
| const parentKey = key.split('-')[0]; | ||
|
|
||
| // Chỉ return parent key nếu selected key có dấu gạch ngang (là submenu) | ||
| if (key.includes('-')) { | ||
| return [parentKey]; | ||
| } | ||
| return []; |
There was a problem hiding this comment.
The function getOpenKeysFromSelectedKey assumes a specific key format ("2-1") but doesn't validate the input. If the key format changes or is malformed, this could lead to unexpected behavior. Consider adding validation or documentation about the expected key format.
| const getOpenKeysFromSelectedKey = (key) => { | |
| if (!key) return []; | |
| // Tìm parent key từ selected key | |
| // Ví dụ: "2-1" -> parent là "2" | |
| const parentKey = key.split('-')[0]; | |
| // Chỉ return parent key nếu selected key có dấu gạch ngang (là submenu) | |
| if (key.includes('-')) { | |
| return [parentKey]; | |
| } | |
| return []; | |
| // Expected key format: "parent-child" (e.g., "2-1"). Returns [parent] if valid, [] otherwise. | |
| const getOpenKeysFromSelectedKey = (key) => { | |
| if (!key) return []; | |
| // Validate key format: must be two non-empty segments separated by a single dash | |
| const keyParts = key.split('-'); | |
| if (keyParts.length !== 2 || !keyParts[0] || !keyParts[1]) { | |
| return []; | |
| } | |
| // Return parent key if valid | |
| return [keyParts[0]]; |
| localStorage.removeItem("admin"); | ||
| setAdmin(null); | ||
| navigate("/auth/admin/login"); | ||
| } |
There was a problem hiding this comment.
Missing semicolon at the end of the statement. While JavaScript allows this, it's inconsistent with the coding style in other parts of the file.
| message: 'Phiên đăng nhập hết hạn', | ||
| description: 'Vui lòng đăng nhập lại' | ||
| }); | ||
| globalThis.location.reload(); |
There was a problem hiding this comment.
Using globalThis.location.reload() forces a full page reload which will lose any unsaved state. Consider navigating to the login page instead using navigate('/auth/admin/login') to preserve the SPA experience.
| useEffect(() => { | ||
| if (!editor) return; | ||
|
|
||
| let timeoutId = null; | ||
|
|
||
| // Debounce the change handler to avoid rapid successive calls | ||
| const debouncedHandler = () => { | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId); | ||
| } | ||
|
|
||
| timeoutId = setTimeout(() => { | ||
| handleEditorChange(); | ||
| }, 100); | ||
| }; | ||
|
|
||
| // Subscribe to editor changes | ||
| const unsubscribe = editor.onChange(debouncedHandler); | ||
|
|
||
| return () => { | ||
| if (timeoutId) { | ||
| clearTimeout(timeoutId); | ||
| } | ||
| if (typeof unsubscribe === 'function') { | ||
| unsubscribe(); | ||
| } | ||
| }; | ||
| }, [editor, handleEditorChange]); | ||
|
|
There was a problem hiding this comment.
Duplicate change handler subscription. The editor's onChange is already set in useCreateBlockNote (line 43), but then another subscription is created in the useEffect (line 102). This will cause the change handler to fire twice for every edit, which is inefficient and could lead to performance issues or unexpected behavior.
| useEffect(() => { | |
| if (!editor) return; | |
| let timeoutId = null; | |
| // Debounce the change handler to avoid rapid successive calls | |
| const debouncedHandler = () => { | |
| if (timeoutId) { | |
| clearTimeout(timeoutId); | |
| } | |
| timeoutId = setTimeout(() => { | |
| handleEditorChange(); | |
| }, 100); | |
| }; | |
| // Subscribe to editor changes | |
| const unsubscribe = editor.onChange(debouncedHandler); | |
| return () => { | |
| if (timeoutId) { | |
| clearTimeout(timeoutId); | |
| } | |
| if (typeof unsubscribe === 'function') { | |
| unsubscribe(); | |
| } | |
| }; | |
| }, [editor, handleEditorChange]); | |
| // Removed duplicate editor.onChange subscription useEffect. |
7ce6d53 to
8fdfe70
Compare
No description provided.