Repository navigation
T121-T123: Export receipt UI and integration - #37
Conversation
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughPR thêm module Phiếu xuất kho (Export Receipt) cho frontend: service ChangesModule Phiếu xuất kho
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 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: 10
🤖 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 `@docs/README_T121.md`:
- Around line 38-40: Điều chỉnh rule điều hướng để chỉ áp dụng cho luồng
tạo/sửa/submit thành công, không mô tả redirect từ màn danh sách. Cập nhật phần
liên quan trong `README_T121.md` để phản ánh đúng flow của
`ExportReceiptList.vue`, nơi action trên list chỉ refresh dữ liệu tại chỗ sau
submit/hủy và không gọi router redirect; giữ wording tập trung vào các màn
T122/T120 và bỏ ngụ ý chuyển trang từ list.
In `@docs/README_T122.md`:
- Around line 16-17: The product-selection rule in the draft-edit flow is too
strict and can hide previously chosen rows when inventory reaches zero, breaking
state consistency. Update the spec around the product dropdown so the “Inventory
> 0” filter applies only to new selections, while existing lines in an editable
draft remain visible and selectable in the current form state even if their
stock is now zero. Use the draft-edit vs create-new behavior distinction in the
product selection requirements to guide the change.
- Around line 18-20: The “Validate Số lượng xuất” requirement only covers
real-time UI checks, but backend must also re-validate inventory before
save/submit to protect data integrity. Update the spec around the inventory
validation requirement to state that the backend flow (for the save/approval
path) must compare the requested quantity against current stock, or enforce
version/optimistic locking, using the relevant inventory and submit/save
workflow as the source of truth. Keep the UI warning behavior, but make it clear
that client-side validation is not sufficient on its own.
In `@docs/superpowers/specs/2026-06-28-import-receipt-ux-ui-design.md`:
- Around line 30-35: Spec hiện đang gộp chung chặn chuyển route và refresh/đóng
tab, nhưng onBeforeRouteLeave chỉ xử lý điều hướng nội bộ. Hãy tách rõ hai cơ
chế: onBeforeRouteLeave cho route change trong app và beforeunload cho F5/đóng
tab, để mô tả đúng behavior bảo vệ dữ liệu. Cập nhật phần Unsaved Changes Guard
để nêu rõ khi nào Modal/Dialog xuất hiện và luồng confirm/discard tương ứng, đảm
bảo state consistency và async flow nhất quán.
- Around line 23-29: Role gating and self-approval behavior are inconsistent
with the current action rendering in StockDocumentsView, so update the spec to
first define the exact role matrix for who can see the Approve/Reject actions,
then describe the disabled state only for the case where Manager is viewing
their own receipt. Make the wording align with the actual conditional rendering
in StockDocumentsView so implementation can support a visible-but-disabled
button state for authorized roles rather than implying visibility for roles that
are currently excluded.
In `@src/components/ExportReceiptDetailPanel.vue`:
- Around line 112-129: `executeSubmit` (and the matching `executeCancel`)
currently only surfaces `err.message` and does not handle 401 the way
`loadReceiptDetails` does, so expired sessions do not redirect to login. Update
these handlers to detect `err.status === 401` in their catch blocks, call
`router.replace('/login')` like `loadReceiptDetails`, and keep the existing
error fallback for non-401 cases. Use the existing `executeSubmit`,
`executeCancel`, and `loadReceiptDetails` logic in
`ExportReceiptDetailPanel.vue` as the reference for consistent auth handling.
In `@src/components/ExportReceiptForm.vue`:
- Around line 87-112: The form handlers in ExportReceiptForm.vue are missing 401
handling, so expired-token failures leave the user stuck on the page. Update
loadReceiptDetails, saveReceipt, and saveAndSubmit to detect err.status === 401
and redirect with router.replace('/login'), matching the behavior already used
in ExportReceiptList and ExportReceiptDetailPanel. Keep the existing
errorMessage fallback for non-401 errors, and make sure the redirect logic is
applied in the same catch blocks for these methods.
- Around line 266-268: The delayed navigation in ExportReceiptForm’s submit flow
uses setTimeout without cleanup, so it can still call router.push('/stock-out')
after the user has already left the page. Store the timeout handle in the
component state inside the affected submit handler(s), and clear it when the
component unmounts or before starting a new timeout; apply the same cleanup
pattern to both places using this delayed redirect.
- Around line 289-293: The create-and-submit flow in ExportReceiptForm’s draft
handling is split between createDraft() and submitForApproval(), which can leave
a newly created draft unreconciled if approval submission fails. Update the
non-edit path in ExportReceiptForm so it preserves the created receiptIdToSubmit
and transitions the UI to the newly created draft (or otherwise exposes its
ID/state) when submitForApproval() errors, instead of staying in create mode and
allowing a second createDraft() on retry.
In `@src/services/exportReceiptService.js`:
- Around line 6-11: The axios client in exportReceiptClient is missing a
timeout, so requests can hang indefinitely when the backend is slow or
unresponsive. Update the axios.create configuration in exportReceiptClient to
include a reasonable timeout value alongside baseURL and headers, keeping the
existing setup intact.
🪄 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: 264acaa1-db11-40b9-8e0d-f0292f778b0e
📒 Files selected for processing (12)
docs/README_T121.mddocs/README_T122.mddocs/README_T123.mddocs/superpowers/specs/2026-06-28-import-receipt-ux-ui-design.mdexport-receipt-plan.mdsrc/components/ExportReceiptDetailPanel.vuesrc/components/ExportReceiptForm.vuesrc/components/ExportReceiptList.vuesrc/services/exportReceiptService.jssrc/views/StockDocumentCreateView.vuesrc/views/StockDocumentDetailView.vuesrc/views/StockDocumentsView.vue
| ### 2.3 Self-Approval Restriction (Chặn Duyệt Chéo) | ||
| - **Scenario:** A user with the `Manager` role views a receipt that they originally created themselves. | ||
| - **UI Behavior:** | ||
| - The "Approve / Reject" (Duyệt / Từ chối) action buttons will still be visible to maintain layout consistency. | ||
| - However, the buttons will be visibly **DISABLED** (greyed out). | ||
| - Hovering over the disabled buttons will display a clear Tooltip: *"Bạn không thể tự duyệt phiếu do chính mình tạo"*. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Chốt lại role matrix trước khi mô tả nút “disabled”.
Spec đang nói Manager vẫn thấy nút rồi mới disable, nhưng src/views/StockDocumentsView.vue:250-271 hiện chỉ render action cho role ADMIN/EMPLOYEE. Nếu giữ nguyên mô tả này thì implementation sẽ không bao giờ có trạng thái “disabled” cho Manager.
Gợi ý chỉnh spec
- The "Approve / Reject" action buttons will still be visible to maintain layout consistency.
- However, the buttons will be visibly DISABLED (greyed out).
+ Chỉ mô tả disabled-state nếu role hiện tại thật sự render được nút.
+ Nếu không, hãy ẩn hẳn action theo role matrix để tránh lệch UI.As per path instructions: ưu tiên correctness và role gating.
📝 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.
| ### 2.3 Self-Approval Restriction (Chặn Duyệt Chéo) | |
| - **Scenario:** A user with the `Manager` role views a receipt that they originally created themselves. | |
| - **UI Behavior:** | |
| - The "Approve / Reject" (Duyệt / Từ chối) action buttons will still be visible to maintain layout consistency. | |
| - However, the buttons will be visibly **DISABLED** (greyed out). | |
| - Hovering over the disabled buttons will display a clear Tooltip: *"Bạn không thể tự duyệt phiếu do chính mình tạo"*. | |
| ### 2.3 Self-Approval Restriction (Chặn Duyệt Chéo) | |
| - **Scenario:** A user with the `Manager` role views a receipt that they originally created themselves. | |
| - **UI Behavior:** | |
| - Chỉ mô tả disabled-state nếu role hiện tại thật sự render được nút. | |
| - Nếu không, hãy ẩn hẳn action theo role matrix để tránh lệch UI. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 23-23: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 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 `@docs/superpowers/specs/2026-06-28-import-receipt-ux-ui-design.md` around
lines 23 - 29, Role gating and self-approval behavior are inconsistent with the
current action rendering in StockDocumentsView, so update the spec to first
define the exact role matrix for who can see the Approve/Reject actions, then
describe the disabled state only for the case where Manager is viewing their own
receipt. Make the wording align with the actual conditional rendering in
StockDocumentsView so implementation can support a visible-but-disabled button
state for authorized roles rather than implying visibility for roles that are
currently excluded.
Source: Path instructions
| ### 2.4 Unsaved Changes Guard (Cảnh báo mất dữ liệu) | ||
| - **Scenario:** The user is filling out the "Create Draft" or "Inspect" form and attempts to navigate away (e.g., clicking Back, refreshing the page, or clicking another sidebar menu) without saving. | ||
| - **UI Behavior:** | ||
| - The browser/router will intercept the navigation. | ||
| - A Modal/Dialog will appear warning: *"Bạn có dữ liệu chưa lưu, chắc chắn muốn rời đi?"*. | ||
| - The user must explicitly confirm to discard changes. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Tách guard cho route change và refresh/đóng tab.
onBeforeRouteLeave chỉ chặn chuyển route nội bộ. Nếu muốn bảo vệ lúc F5 hoặc đóng tab, spec cần thêm cơ chế beforeunload riêng; nếu không người dùng vẫn có thể mất dữ liệu dù mô tả nói “browser/router sẽ intercept”.
Gợi ý chỉnh spec
- The browser/router will intercept the navigation.
+ The router will intercept internal navigation.
+ A separate beforeunload guard will handle refresh/close tab.As per path instructions: ưu tiên state consistency và async flow.
📝 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.
| ### 2.4 Unsaved Changes Guard (Cảnh báo mất dữ liệu) | |
| - **Scenario:** The user is filling out the "Create Draft" or "Inspect" form and attempts to navigate away (e.g., clicking Back, refreshing the page, or clicking another sidebar menu) without saving. | |
| - **UI Behavior:** | |
| - The browser/router will intercept the navigation. | |
| - A Modal/Dialog will appear warning: *"Bạn có dữ liệu chưa lưu, chắc chắn muốn rời đi?"*. | |
| - The user must explicitly confirm to discard changes. | |
| ### 2.4 Unsaved Changes Guard (Cảnh báo mất dữ liệu) | |
| - **Scenario:** The user is filling out the "Create Draft" or "Inspect" form and attempts to navigate away (e.g., clicking Back, refreshing the page, or clicking another sidebar menu) without saving. | |
| - **UI Behavior:** | |
| - The router will intercept internal navigation. | |
| - A separate beforeunload guard will handle refresh/close tab. | |
| - A Modal/Dialog will appear warning: *"Bạn có dữ liệu chưa lưu, chắc chắn muốn rời đi?"*. | |
| - The user must explicitly confirm to discard changes. |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 30-30: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🤖 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 `@docs/superpowers/specs/2026-06-28-import-receipt-ux-ui-design.md` around
lines 30 - 35, Spec hiện đang gộp chung chặn chuyển route và refresh/đóng tab,
nhưng onBeforeRouteLeave chỉ xử lý điều hướng nội bộ. Hãy tách rõ hai cơ chế:
onBeforeRouteLeave cho route change trong app và beforeunload cho F5/đóng tab,
để mô tả đúng behavior bảo vệ dữ liệu. Cập nhật phần Unsaved Changes Guard để
nêu rõ khi nào Modal/Dialog xuất hiện và luồng confirm/discard tương ứng, đảm
bảo state consistency và async flow nhất quán.
Source: Path instructions
2 participants
Summary by CodeRabbit