Repository navigation
Feature/t230 polish UI - #69
Conversation
…and safe pendingTasks fallback
…shboard approval totals
…g by route permissions
…g by route permissions
…dashboard template ref fix
…ch network error guard
…dition on tour close
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughPR bổ sung modal chào mừng, guided tour, dashboard theo quyền, trang hồ sơ, route ChangesTrải nghiệm sản phẩm
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/views/DashboardView.vue (1)
254-418: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winTrùng lặp logic xử lý lỗi 401 nhiều lần.
Khối code sau lặp lại 7 lần trong
loadDashboardData(Line 294-299, 311-316, 324-329, 337-342, 351-356, 370-375, 401-406):isLoading.value = false; isRetrying.value = false; router.replace("/login"); return;Việc lặp lại này làm hàm khó bảo trì. Nếu sau này cần sửa hành vi khi hết phiên (ví dụ thêm thông báo, xóa thêm state), phải sửa 7 chỗ và dễ bỏ sót.
♻️ Đề xuất rút hàm dùng chung
+function handleUnauthorized() { + isLoading.value = false; + isRetrying.value = false; + router.replace("/login"); +}Sau đó thay từng khối bằng:
- if (overviewResultValue.reason?.status === 401) { - isLoading.value = false; - isRetrying.value = false; - router.replace("/login"); - return; - } + if (overviewResultValue.reason?.status === 401) { + handleUnauthorized(); + return; + }Áp dụng tương tự cho các khối còn lại.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/views/DashboardView.vue` around lines 254 - 418, Extract the repeated 401 session-expiration handling from loadDashboardData into a shared helper that resets isLoading and isRetrying, redirects with router.replace("/login"), and provides the required early-return behavior. Replace every matching 401 block in loadDashboardData—including the overview, product, warehouse, warning, and pending-data error paths—with this helper while preserving non-401 handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/GuidedTourOverlay.vue`:
- Around line 44-113: Cập nhật cơ chế hủy trong updateTarget: đặt
pendingNavigationCancelled = true trong cả closeTour và skipTour trước khi kết
thúc tour. Ở hai nhánh kiểm tra sau router.push và nextTick, chỉ revert về
pendingNavigationPrevRoute khi route hiện tại vẫn là step.route, không ghi đè
điều hướng khác của người dùng. Giữ nguyên việc xóa spotlight và thoát xử lý khi
navigation bị hủy.
In `@src/components/WelcomeModal.vue`:
- Around line 155-162: Phân biệt hành vi của hai nút trong WelcomeModal: giữ nút
“Bắt đầu” để đóng modal và tiếp tục mở guided tour, còn “Để sau” phải phát tín
hiệu bỏ qua tour. Cập nhật closeWelcomeModal trong App.vue để nhận trạng thái
này và chỉ gọi logic mở tour khi người dùng chọn “Bắt đầu”, đồng thời giữ việc
đóng modal cho cả hai lựa chọn.
In `@src/services/profileService.js`:
- Line 39: Update the error handling around normalizeProfileError in the
profile-loading loop to use a neutral fallback such as “Không thể tải thông tin
hồ sơ.” for non-401 failures, including server errors and missing response
messages. Only assign the session-expired message when normalizedError.status is
401, while preserving the existing normalized error flow and null handling.
- Around line 41-58: Update the fallback loop in the profile service around the
endpoint error handling so HTTP 4xx responses other than 404 use continue
instead of break, allowing subsequent endpoints to be attempted. Preserve the
existing early exits for authentication failures and missing network responses.
---
Outside diff comments:
In `@src/views/DashboardView.vue`:
- Around line 254-418: Extract the repeated 401 session-expiration handling from
loadDashboardData into a shared helper that resets isLoading and isRetrying,
redirects with router.replace("/login"), and provides the required early-return
behavior. Replace every matching 401 block in loadDashboardData—including the
overview, product, warehouse, warning, and pending-data error paths—with this
helper while preserving non-401 handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 666198dd-7299-494b-a99e-a0dbad73a947
📒 Files selected for processing (21)
src/App.vuesrc/components/AppSidebar.vuesrc/components/ConfirmDialog.vuesrc/components/DataTable.vuesrc/components/EmptyState.vuesrc/components/GuidedTourOverlay.vuesrc/components/PageHeader.vuesrc/components/StatusBadge.vuesrc/components/WelcomeModal.vuesrc/router/index.jssrc/services/dashboardService.jssrc/services/permissionService.jssrc/services/productService.jssrc/services/profileService.jssrc/services/warehouseService.jssrc/style.csssrc/views/ApprovalsView.vuesrc/views/DashboardView.vuesrc/views/ProductsView.vuesrc/views/ProfileView.vuesrc/views/WarehousesView.vue
| <div class="welcome-actions"> | ||
| <button class="btn btn-ghost" type="button" @click="close"> | ||
| Để sau | ||
| </button> | ||
| <button class="btn btn-primary" type="button" @click="close"> | ||
| Bắt đầu | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Nút "Để sau" và "Bắt đầu" có hành vi giống hệt nhau.
Cả hai nút chỉ gọi close(), tức chỉ emit "close". Trong App.vue, closeWelcomeModal() luôn mở guided tour ngay sau khi đóng modal nếu tour chưa xem (shouldShowGuidedTour()). Vậy dù người dùng bấm "Để sau" (ý định hoãn lại), guided tour vẫn hiện ngay lập tức, giống hệt khi bấm "Bắt đầu". Nhãn nút không khớp hành vi thực tế.
Nếu ý định là "Để sau" = bỏ qua tour luôn, cần phân biệt 2 event, ví dụ:
💡 Đề xuất phân biệt hành vi 2 nút
const emit = defineEmits(["close"]);
+
+function closeAndSkipTour() {
+ emit("close", { skipTour: true });
+}- <button class="btn btn-ghost" type="button" `@click`="close">
+ <button class="btn btn-ghost" type="button" `@click`="closeAndSkipTour">
Để sau
</button>Và trong App.vue, closeWelcomeModal cần nhận tham số này để quyết định có mở tour tiếp hay không.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/WelcomeModal.vue` around lines 155 - 162, Phân biệt hành vi
của hai nút trong WelcomeModal: giữ nút “Bắt đầu” để đóng modal và tiếp tục mở
guided tour, còn “Để sau” phải phát tín hiệu bỏ qua tour. Cập nhật
closeWelcomeModal trong App.vue để nhận trạng thái này và chỉ gọi logic mở tour
khi người dùng chọn “Bắt đầu”, đồng thời giữ việc đóng modal cho cả hai lựa
chọn.
…allback loop, and shared 401 handler
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/WelcomeModal.vue (1)
67-84: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winKhông thêm listener sau khi modal đã đóng hoặc unmount.
Watcher chờ
nextTick()trước khi thêm listener. Nếuopenchuyển thànhfalsehoặc component unmount trong thời gian này, callback vẫn tiếp tục và thêmfocuslistener sau bước cleanup. Listener đó tồn tại đến lần đóng modal tiếp theo hoặc đến khi app unmount.Sau
await nextTick(), kiểm tra lạiprops.openvàdialogRef.valuetrước khi focus hoặc thêm listener.Đề xuất sửa
previouslyFocused = document.activeElement; await nextTick(); + if (!props.open || !dialogRef.value) return; + const focusable = getFocusableElements();As per path instructions: "Kiểm tra ... cleanup timer/listener/watcher".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/WelcomeModal.vue` around lines 67 - 84, Update the open branch of the watcher on props.open so that after await nextTick() it verifies the modal is still open and dialogRef.value still exists before focusing or registering the focus and keydown listeners. If either condition is no longer true, return without adding listeners, preserving the existing cleanup behavior in the closed branch.Source: Path instructions
src/App.vue (1)
76-121: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winLọc bước guided tour theo quyền truy cập.
guidedTourStepsluôn có bước đến/inventory. Trong khi đóDashboardView.vuechỉ dùng route này khicanAccessRoute("/inventory")cho phép. Người dùng không có quyền vẫn có thể bịGuidedTourOverlayđiều hướng đến màn hình này.Lọc các bước có
routebằngcanAccessRoute(step.route)trước khi truyền vào overlay.Đề xuất sửa
+import { canAccessRoute } from "./services/permissionService"; -const guidedTourSteps = [ +const guidedTourSteps = computed(() => + [ // các bước hiện có -]; + ].filter((step) => !step.route || canAccessRoute(step.route)), +);As per path instructions: "UI phải phản ánh đúng quyền".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.vue` around lines 76 - 121, Filter the route-based entries in guidedTourSteps using canAccessRoute(step.route) before passing the steps to GuidedTourOverlay, while retaining steps without a route. Ensure users lacking /inventory access never receive or get navigated to that tour step, consistent with the existing DashboardView permission behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/App.vue`:
- Around line 76-121: Filter the route-based entries in guidedTourSteps using
canAccessRoute(step.route) before passing the steps to GuidedTourOverlay, while
retaining steps without a route. Ensure users lacking /inventory access never
receive or get navigated to that tour step, consistent with the existing
DashboardView permission behavior.
In `@src/components/WelcomeModal.vue`:
- Around line 67-84: Update the open branch of the watcher on props.open so that
after await nextTick() it verifies the modal is still open and dialogRef.value
still exists before focusing or registering the focus and keydown listeners. If
either condition is no longer true, return without adding listeners, preserving
the existing cleanup behavior in the closed branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4ff4e7e1-c648-46d2-ada3-389ae65172ad
📒 Files selected for processing (11)
src/App.vuesrc/components/ConfirmDialog.vuesrc/components/EmptyState.vuesrc/components/GuidedTourOverlay.vuesrc/components/PageHeader.vuesrc/components/StatusBadge.vuesrc/components/WelcomeModal.vuesrc/services/profileService.jssrc/views/DashboardView.vuesrc/views/ProductsView.vuesrc/views/WarehousesView.vue
… race condition fix
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/App.vue (2)
136-143: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReset state onboarding khi chuyển sang auth layout.
isWelcomeModalOpenvàisGuidedTourOpenvẫn giữ giá trị khiisAuthLayoutchuyển sangtrue. Template chỉ unmount component. Nếu logout khi modal hoặc tour đang mở rồi đăng nhập user khác, state cũ có thể mở lại cùng onboarding của user mới.Reset cả hai ref khi vào auth layout. Trước khi tính trạng thái onboarding cho phiên mới, cũng reset cả hai ref.
Đề xuất sửa
watch(isAuthLayout, (newVal, oldVal) => { + if (newVal) { + isWelcomeModalOpen.value = false; + isGuidedTourOpen.value = false; + return; + } + if (oldVal === true && newVal === false) { + isWelcomeModalOpen.value = false; + isGuidedTourOpen.value = false; if (shouldShowWelcomeModal()) { isWelcomeModalOpen.value = true;As per path instructions, cần kiểm tra trạng thái auth/main và state người dùng trong
src/App.vue.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.vue` around lines 136 - 143, Reset isWelcomeModalOpen and isGuidedTourOpen when isAuthLayout changes to true, before evaluating onboarding state for the new session. Update the watcher around isAuthLayout so the auth-layout transition clears both refs, while preserving the existing welcome-modal and guided-tour checks when transitioning back to the main layout.Source: Path instructions
90-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winThêm điều kiện role cho step “Pending Approval” trong guided tour.
DashboardView.vuechỉ hiển thị card chờ duyệt nhập/xuất vớiADMINvàMANAGER. Step này không córoute, nên ở vai tròEMPLOYEEtour vẫn hiển thị nhưng card thứ 1 trên.dashboard-section-gridcó thể là mục khác. Thêm checkcanAccessRoute("/approvals")hoặc thay step thành “Alerts” choEMPLOYEE.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/App.vue` around lines 90 - 95, Update the “Pending Approval” guided-tour step in the tour configuration to render only when canAccessRoute("/approvals") is true, matching DashboardView.vue’s ADMIN/MANAGER visibility; ensure EMPLOYEE users receive the appropriate “Alerts” step instead of targeting the first dashboard card.Source: Path instructions
src/components/WelcomeModal.vue (1)
67-94: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKhởi tạo focus khi modal mount với
open=true.
src/App.vuecó thể thiếtisWelcomeModalOpen = truetại thời điểmWelcomeModalvào nhánh main, nhưng watcher ởsrc/components/WelcomeModal.vue:67-94không có{ immediate: true }, nên callback không chạy khi modal được mount vớiprops.openđúng trước. Lúc đó không focus phần tử đầu tiên, không thêmenforceFocus, và không gắnhandleDialogKeydown; người dùng bàn phím có thể nhấn Tab ra ngoài dialog.Thêm
{ immediate: true }, hoặc chạy cùng logic khởi tạo trongonMountedkhiprops.open === true.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/WelcomeModal.vue` around lines 67 - 94, Update the watcher observing props.open in WelcomeModal.vue to run immediately on mount by enabling its immediate option, so the existing open-state initialization executes when the modal is created already open. Preserve the current focus setup, enforceFocus listener, and handleDialogKeydown registration logic.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/App.vue`:
- Around line 136-143: Reset isWelcomeModalOpen and isGuidedTourOpen when
isAuthLayout changes to true, before evaluating onboarding state for the new
session. Update the watcher around isAuthLayout so the auth-layout transition
clears both refs, while preserving the existing welcome-modal and guided-tour
checks when transitioning back to the main layout.
- Around line 90-95: Update the “Pending Approval” guided-tour step in the tour
configuration to render only when canAccessRoute("/approvals") is true, matching
DashboardView.vue’s ADMIN/MANAGER visibility; ensure EMPLOYEE users receive the
appropriate “Alerts” step instead of targeting the first dashboard card.
In `@src/components/WelcomeModal.vue`:
- Around line 67-94: Update the watcher observing props.open in WelcomeModal.vue
to run immediately on mount by enabling its immediate option, so the existing
open-state initialization executes when the modal is created already open.
Preserve the current focus setup, enforceFocus listener, and handleDialogKeydown
registration logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3f01c3d3-8d15-490c-ac4a-571860d56f83
📒 Files selected for processing (2)
src/App.vuesrc/components/WelcomeModal.vue
…modal immediate watcher
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.vue`:
- Around line 125-129: The visibleGuidedTourSteps computed value must react to
authentication and role changes instead of relying on the non-reactive
getCurrentRoleCode() localStorage lookup. Update the computed logic to use the
auth store’s reactive role/ref when calling canAccessRoute, or explicitly depend
on the relevant auth state so the /approvals step refreshes after login, logout,
or role changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 19402be8-233d-4918-8c2e-ad2289474ffb
📒 Files selected for processing (2)
src/App.vuesrc/components/WelcomeModal.vue
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/App.vue`:
- Around line 128-135: Update GuidedTourOverlay to watch props.steps and
synchronize the active tour when visibleGuidedTourSteps changes: clamp the
current index to a valid step, reset it when the current step is removed, or
close the tour when no permitted steps remain. Preserve the existing open-tour
reset behavior and ensure highlighting uses the updated step list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4d3cbfcb-326a-483d-ae55-2b655819e1fb
📒 Files selected for processing (1)
src/App.vue
2 participants
check giúp tui
Summary by CodeRabbit