Conversation
…d add POS page scaffold
…nd customer complaints
…king and CRUD operations
There was a problem hiding this comment.
Pull request overview
This PR expands unit-conversion/packaging-variant support across the product manager UI and inventory services, while also adjusting CRM ticket permissions/UI and tightening database seeding behavior.
Changes:
- Frontend: enhance unit conversion display to surface the linked “converted variant” details and management actions; show only base-unit variants in Product Detail; lock sell price editing in the variant modal.
- Backend: add syncing of converted-variant stock from base-variant stock during import/sale/refund/PO receive; adjust conversion-derived variant stock calculation.
- Ops/CRM: introduce opt-in legacy fallback for seeding; simplify complaint UI and update delete authorization.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/pages/Products/ProductManager/UnitConversionSection.jsx | Renders richer conversion rows including mapped converted-variant info and actions. |
| frontend/src/pages/Products/ProductManager/ProductDetail.jsx | Splits base vs converted variants; passes conversion→variant mapping into UnitConversionSection and hides converted variants from main table. |
| frontend/src/pages/Products/ProductManager/EditVariantModal.jsx | Disables sell-price editing and limits conversion management to base-unit variants. |
| frontend/src/pages/Pos/pos.jsx | Formatting-only changes. |
| frontend/src/pages/Pos/complain.jsx | Removes priority UI and refactors role-based permissions; uses shared ConfirmModal. |
| frontend/src/pages/Pos/TransactionHistory.jsx | Fixes stray CRLF artifact in markup. |
| deploy/scripts/reset-db-and-seed.sh | Adds ENABLE_LEGACY_FALLBACK gating and stricter failure on empty critical tables. |
| backend/src/main/resources/data.sql | Adds explicit column lists for inserts (schema-alignment). |
| backend/src/main/java/com/smalltrend/service/products/UnitConversionService.java | Changes default sell price behavior when creating conversions. |
| backend/src/main/java/com/smalltrend/service/products/ProductVariantService.java | Uses conversion-derived flag to compute stock quantities from base variant. |
| backend/src/main/java/com/smalltrend/service/inventory/shared/InventoryStockService.java | Adds syncConvertedStocksFromBase and calls it on stock transitions; removes direct stock deduction/restock for non-base units. |
| backend/src/main/java/com/smalltrend/service/inventory/purchase/PurchaseOrderService.java | Syncs converted stocks after PO stock-in. |
| backend/src/main/java/com/smalltrend/controller/CRM/TicketController.java | Expands delete permission to include MANAGER. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {conversionVariant?.is_active ? ( | ||
| <Badge variant="outline" className="text-[10px] px-1.5 bg-green-50 border-green-200 text-green-700">Biến thể đang hoạt động</Badge> | ||
| ) : ( | ||
| <Badge variant="outline" className="text-[10px] px-1.5 bg-gray-100 border-gray-200 text-gray-500">Biến thể đang tắt</Badge> |
There was a problem hiding this comment.
conversionVariant?.is_active ? ... : ... currently renders “Biến thể đang tắt” even when conversionVariant is undefined (i.e., not found / not synced). That’s misleading because the block below correctly shows a “Chưa đồng bộ…” warning in that case. Consider only rendering the active/inactive badge when conversionVariant exists, and otherwise render an explicit “Chưa đồng bộ”/“Không tìm thấy biến thể” badge (or omit the badge).
| {conversionVariant?.is_active ? ( | |
| <Badge variant="outline" className="text-[10px] px-1.5 bg-green-50 border-green-200 text-green-700">Biến thể đang hoạt động</Badge> | |
| ) : ( | |
| <Badge variant="outline" className="text-[10px] px-1.5 bg-gray-100 border-gray-200 text-gray-500">Biến thể đang tắt</Badge> | |
| {conversionVariant && ( | |
| conversionVariant.is_active ? ( | |
| <Badge variant="outline" className="text-[10px] px-1.5 bg-green-50 border-green-200 text-green-700">Biến thể đang hoạt động</Badge> | |
| ) : ( | |
| <Badge variant="outline" className="text-[10px] px-1.5 bg-gray-100 border-gray-200 text-gray-500">Biến thể đang tắt</Badge> | |
| ) |
| src={conversionVariant.image_url.startsWith('http') ? conversionVariant.image_url : `${import.meta.env.PROD ? "" : "http://localhost:8081"}${conversionVariant.image_url.startsWith('/') ? '' : '/'}${conversionVariant.image_url}`} | ||
| alt={conversionVariant.name} |
There was a problem hiding this comment.
The image URL resolver here hardcodes http://localhost:8081 for non-PROD, which bypasses the existing VITE_API_BASE_URL convention and the shared helpers in frontend/src/utils/inventory.js (getApiOrigin / resolveImageUrl). This can break dev setups that don’t run the backend on 8081 or use a different proxy. Prefer using the shared resolver helper (or VITE_API_BASE_URL-derived origin) instead of a hardcoded host.
| const canEditComplaint = hasAnyRole(user, [...MANAGER_ROLES, ...CASHIER_ROLES]); | ||
| const canDeleteComplaint = hasAnyRole(user, MANAGER_ROLES); | ||
| const canCreateComplaint = hasAnyRole(user, [...MANAGER_ROLES, ...CASHIER_ROLES]); |
There was a problem hiding this comment.
canEditComplaint / canCreateComplaint / canDeleteComplaint don’t include ADMIN_ROLES, so an ADMIN user would be blocked from CRUD actions in the UI even though admins typically have full permissions (and rolePermissions.canPerform grants admins all actions). Consider including ADMIN_ROLES in these checks or switching to a single permission helper to keep frontend role gating consistent.
| BigDecimal resolvedSellPrice = request.getSellPrice() != null | ||
| ? request.getSellPrice() | ||
| : baseVariant.getSellPrice(); | ||
| if (resolvedSellPrice == null) { | ||
| throw new RuntimeException("Giá bán quy đổi không được để trống."); | ||
| } | ||
| : BigDecimal.ZERO; | ||
|
|
There was a problem hiding this comment.
Defaulting resolvedSellPrice to BigDecimal.ZERO when the request doesn’t provide a sell price will silently create packaging variants / conversions priced at 0. This also changes downstream behavior: e.g. BarcodeLookupService uses uc.getSellPrice() when it’s non-null, so a stored 0 will override the variant price and produce a 0 unit price at POS. Consider defaulting to baseVariant.getSellPrice() (or leaving it null and handling null explicitly) instead of forcing 0.
| || conversion.getConversionFactor() == null | ||
| || conversion.getConversionFactor().intValue() <= 0) { | ||
| continue; | ||
| } | ||
|
|
||
| ProductVariant convertedVariant = findConvertedVariant(baseVariant, conversion.getToUnit().getId()); | ||
| if (convertedVariant == null || convertedVariant.getId() == null) { | ||
| continue; | ||
| } | ||
|
|
||
| int convertedQty = baseStockAtLocation / conversion.getConversionFactor().intValue(); | ||
| ProductBatch convertedBatch = resolveConvertedBatch(convertedVariant, baseBatch); |
There was a problem hiding this comment.
conversionFactor is a BigDecimal with scale=4, but this sync logic uses intValue() and integer division (baseStockAtLocation / factor). If conversion_factor is not an integer (or larger than Integer.MAX_VALUE), intValue() will truncate and produce incorrect derived quantities. Consider either enforcing an integer-only conversion factor (e.g., validate scale==0 and use intValueExact()), or performing the calculation using BigDecimal with an explicit rounding strategy.
| if [ "$ENABLE_LEGACY_FALLBACK" = "true" ]; then | ||
| log "WARN" "Critical product tables are empty after data.sql. Running legacy fallback import..." | ||
| if [ -f "$FIX_SEED_FILE" ]; then | ||
| seed_with_fix_seed || true | ||
| else | ||
| seed_with_backup_files || true | ||
| fi | ||
| else | ||
| seed_with_backup_files || true | ||
| log "ERROR" "Critical product tables are empty after data.sql. Legacy fallback is disabled (ENABLE_LEGACY_FALLBACK=false)." | ||
| exit 1 |
There was a problem hiding this comment.
This change makes the fallback seed path opt-in (ENABLE_LEGACY_FALLBACK=false by default) and exits with code 1 when critical tables remain empty after data.sql. Since the CD workflow calls this script without setting ENABLE_LEGACY_FALLBACK, a schema mismatch or partial seed will now fail the deploy instead of attempting the legacy backup/fix seed. If that’s intended, it would help to (a) clearly document the env var at the top of the script and/or (b) update the caller to explicitly set ENABLE_LEGACY_FALLBACK=true when you still want the old behavior.
No description provided.