Repository navigation
Feature/t152 integrate reject modal - #43
Conversation
|
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 thêm luồng phiếu xuất chờ duyệt gồm API chuẩn hóa dữ liệu, danh sách phân trang, xem chi tiết, duyệt/từ chối, hỗ trợ hàng có thể nhấp và cập nhật quyền cùng điều hướng. ChangesLuồng duyệt phiếu xuất
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/AppSidebar.vue`:
- Around line 8-38: Update the Import Excel entry in the items array of
AppSidebar.vue to include the "admin" role tag so visibleItems hides it from
non-ADMIN roles. Leave the existing ADMIN access rule for /import-excel in
src/router/index.js lines 67-73 unchanged; no direct change is required there.
In `@src/components/StockOutApprovalDetail.vue`:
- Around line 186-190: Update the props.receiptId watcher to reset rejectState
whenever the receipt changes, alongside actionMessage and actionError, before
calling loadDetail. Ensure any open reject modal and its previous reason are
cleared so confirmReject cannot submit stale data for the new receipt.
- Around line 139-154: Update the rejection flow around rejectExportReceipt to
use its response directly, matching handleApprove, and remove the subsequent
loadDetail call. Ensure a successful rejection closes and resets rejectState and
shows the success message even when refreshing detail data would fail; retain
the existing error handling for an actual rejectExportReceipt failure.
In `@src/views/PendingExportApprovalsView.vue`:
- Around line 134-139: Thêm thuộc tính :clickable="true" vào component DataTable
trong PendingExportApprovalsView, giữ nguyên cấu hình `@row-click`="goToDetail" để
sự kiện nhấp vào hàng được phát ra và điều hướng đến trang chi tiết hoạt động.
- Around line 73-85: Update openDetail to guard against concurrent requests
using detailLoading, preventing a second receipt selection while a detail
request is already in progress. Keep the existing loading, error,
selectedReceipt, and finally-state behavior unchanged for the accepted request.
🪄 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: 31683ba3-0dc0-42cb-b63b-ff14365f8fdd
📒 Files selected for processing (7)
src/components/AppSidebar.vuesrc/components/DataTable.vuesrc/components/StockOutApprovalDetail.vuesrc/router/index.jssrc/services/stockOutApprovalService.jssrc/views/PendingExportApprovalsView.vuesrc/views/StockDocumentDetailView.vue
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 (1)
src/components/AppSidebar.vue (1)
11-11: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winThêm check
/partnerstrongsrc/router/index.js. Sidebar đã ẩn mục này với EMPLOYEE, nhưngcanAccessRoute()vẫn cho phép/partners, nên mở thẳng URL vẫn vào được màn Nhà cung cấp.🤖 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/AppSidebar.vue` at line 11, Update canAccessRoute in the router configuration to explicitly restrict the /partners route for EMPLOYEE users, matching the existing visibility rule in AppSidebar. Preserve access for roles that are allowed to manage suppliers while preventing direct navigation to /partners by EMPLOYEE.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/components/AppSidebar.vue`:
- Line 11: Update canAccessRoute in the router configuration to explicitly
restrict the /partners route for EMPLOYEE users, matching the existing
visibility rule in AppSidebar. Preserve access for roles that are allowed to
manage suppliers while preventing direct navigation to /partners by EMPLOYEE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 247a716b-8270-47d2-9c5b-2fd4ad396efc
📒 Files selected for processing (3)
src/components/AppSidebar.vuesrc/components/StockOutApprovalDetail.vuesrc/views/PendingExportApprovalsView.vue
…table click guards
b530842 to
718b8df
Compare
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/AppSidebar.vue`:
- Around line 30-38: Remove the hardcoded role checks from the visibleItems
computed property and centralize route-access rules in a shared authorization
helper or service. Update both visibleItems and the router’s canAccessRoute to
reuse that common authorization logic, preserving the existing ADMIN, MANAGER,
and approval/manage access behavior.
In `@src/components/StockOutApprovalDetail.vue`:
- Around line 58-76: Thêm bộ đếm request dùng chung, chẳng hạn loadDetailId cạnh
các ref, và tăng giá trị mỗi khi loadDetail bắt đầu. Lưu ID của lần gọi hiện tại
rồi chỉ cập nhật receipt, error và trạng thái loading nếu ID đó vẫn là request
mới nhất; bỏ qua response hoặc lỗi stale khi props.receiptId thay đổi nhanh, đảm
bảo không hiển thị dữ liệu phiếu cũ.
🪄 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: d50c2c7b-9978-4d79-9be2-e884c0c9213c
📒 Files selected for processing (7)
src/components/AppSidebar.vuesrc/components/DataTable.vuesrc/components/StockOutApprovalDetail.vuesrc/router/index.jssrc/services/stockOutApprovalService.jssrc/views/PendingExportApprovalsView.vuesrc/views/StockDocumentDetailView.vue
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/StockOutApprovalDetail.vue (1)
158-160: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKhi reject lỗi, thông báo lỗi hiển thị sau modal nên user không thấy.
Nhánh
catchchỉ setrejectState.submitting = false(modal vẫn mở) rồi gán lỗi vàoactionError— chỗ này nằm ở khu "Hành động" phía sau modal. Người dùng vẫn thấy modal mở nhưng không thấy báo lỗi bên trong, dễ tưởng không có gì xảy ra. Nên gán vàorejectState.errorđể lỗi hiện ngay trong modal.🔧 Đề xuất sửa
} catch (err) { - rejectState.value.submitting = false; - actionError.value = err.message || "Không thể từ chối phiếu xuất."; + rejectState.value.submitting = false; + rejectState.value.error = err.message || "Không thể từ chối phiếu xuất."; } finally {🤖 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/StockOutApprovalDetail.vue` around lines 158 - 160, Update the rejection error handling in the catch branch of the reject action to assign the failure message to rejectState.error instead of actionError.value, so the still-open rejection modal displays the error immediately; keep resetting rejectState.submitting and the existing fallback message unchanged.src/router/index.js (1)
64-65: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winGuard này chỉ hỗ trợ UX, không thay thế phân quyền ở backend.
canAccessRoutetrảtruecho path không khớp rule, nên các route chi tiết (vd/pending-export-approvals/:id,/stock-out/:id) vẫn vào được với mọi role. Không có redirect loop vì/dashboardluôn được phép — phần này ổn. Nhưng cần đảm bảo backend cũng enforce quyền cho các endpoint chi tiết/duyệt/từ chối, không dựa vào guard FE.As per path instructions: "Nhắc rõ frontend guard chỉ hỗ trợ UX, không thay thế authorization ở backend."
🤖 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/router/index.js` around lines 64 - 65, Keep the frontend guard around canAccessRoute and getCurrentRoleCode as UX-only navigation protection, and add or verify backend authorization for detailed, approval, and rejection endpoints such as pending-export-approvals/:id and stock-out/:id. Ensure unauthorized roles are rejected server-side rather than relying on the guard, while preserving the existing /dashboard redirect 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.
Inline comments:
In `@src/services/permissionService.js`:
- Around line 41-60: Update canAccessRoute to add explicit rules for the
stock-in and stock-out detail routes matching /stock-in/:id and /stock-out/:id,
instead of allowing them through the default return true path. Apply the
intended authorized-role restriction consistently with the existing stock route
rules while preserving current behavior for create/edit and unrelated routes.
---
Outside diff comments:
In `@src/components/StockOutApprovalDetail.vue`:
- Around line 158-160: Update the rejection error handling in the catch branch
of the reject action to assign the failure message to rejectState.error instead
of actionError.value, so the still-open rejection modal displays the error
immediately; keep resetting rejectState.submitting and the existing fallback
message unchanged.
In `@src/router/index.js`:
- Around line 64-65: Keep the frontend guard around canAccessRoute and
getCurrentRoleCode as UX-only navigation protection, and add or verify backend
authorization for detailed, approval, and rejection endpoints such as
pending-export-approvals/:id and stock-out/:id. Ensure unauthorized roles are
rejected server-side rather than relying on the guard, while preserving the
existing /dashboard redirect behavior.
🪄 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: 6892d30d-5343-4c71-b77e-3bcc01102429
📒 Files selected for processing (4)
src/components/AppSidebar.vuesrc/components/StockOutApprovalDetail.vuesrc/router/index.jssrc/services/permissionService.js
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/services/permissionService.js`:
- Around line 56-58: Update the route authorization logic in canAccessRoute so
MANAGER is allowed to access the stock-out detail route (/stock-out/:id) used
for approval, while preserving the existing ADMIN/EMPLOYEE restrictions for
stock-out creation and editing routes.
🪄 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: 58198ab0-a76b-4abf-8404-c3c57c6ba06e
📒 Files selected for processing (2)
src/components/StockOutApprovalDetail.vuesrc/services/permissionService.js
2 participants
check và merge code giúp tui
Summary by CodeRabbit