Feature/issue management - #14
Conversation
|
Warning Review limit reached
Next review available in: 9 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: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThis PR adds server-side ticket management APIs (merge/unmerge, urgent tickets, admin status update, feedback), new admin Issue Management and Merge Management UI, ticket feedback and bulk-cancel flows on Tracking, supporting hooks/services, a Prisma rating type migration, and a broad CSS import path reorganization with new design tokens. ChangesTicket Management Feature Set
Component Style Reorganization
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Admin
participant AuditIssues
participant MergeManagementPanel
participant ticketService
participant Server
Admin->>AuditIssues: select tickets, confirm merge
AuditIssues->>MergeManagementPanel: pass selection/state
AuditIssues->>ticketService: mergeTickets(primary, duplicates)
ticketService->>Server: PATCH /api/ticketManagement/mergeTickets
Server-->>ticketService: merge result
ticketService-->>AuditIssues: response
AuditIssues->>AuditIssues: refetch tickets and groups
sequenceDiagram
participant User
participant Tracking
participant FeedbackModal
participant ticketService
participant Server
User->>Tracking: click Feedback on ticket
Tracking->>FeedbackModal: open(ticketId)
User->>FeedbackModal: submit rating/comment
FeedbackModal->>Tracking: onSubmit(ticketId, payload)
Tracking->>ticketService: submitFeedback(ticketId, payload)
ticketService->>Server: POST /api/tickets/submitFeedback/:id
Server-->>ticketService: success
Tracking->>Tracking: updateTicketStatus(resolved), refetch, refresh sidebar counts
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (10)
client/src/index.css (2)
118-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNaming:
--text-size-basevs--text-size-normalis ambiguous.Having both
base(14px) andnormal(16px) alongsidesmall/largemakes the scale's intent unclear at call sites. Consider a single consistent naming scheme (e.g., xs/sm/base/lg or a numeric scale).🤖 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 `@client/src/index.css` around lines 118 - 120, The text-size CSS variables use an inconsistent naming scheme, which makes the scale unclear at call sites. Update the custom properties in the root token set to follow one consistent pattern in the same section as --text-size-small, --text-size-base, and --text-size-normal, and rename any related usages throughout the client styles to match the chosen convention.
79-86: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew
--status-reject-*tokens reuse a low-contrast color pair.
--status-reject-bg:#e74c3cwith `--status-reject-text: `#ffffffyields roughly 3.8:1 contrast, below the WCAG AA minimum of 4.5:1 for normal text. This mirrors the pre-existing--button-red-bg/white pairing elsewhere in the file, so it's a pre-existing pattern rather than a new mistake, but worth reconsidering for the new status label usage.🤖 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 `@client/src/index.css` around lines 79 - 86, The new `--status-reject-*` tokens in `index.css` use a red/white pair that does not meet WCAG AA contrast for normal text. Update the `--status-reject-bg` and/or `--status-reject-text` values to a higher-contrast combination while keeping the new status label styling consistent, and verify the same token pair is used wherever the reject status is rendered.client/src/pages/pageStyles/AddIssue.css (1)
148-171: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded font-size breaks the new design-token pattern.
.form-section h2now usesvar(--text-size-large)at Line 27, but its1536pxbreakpoint override at Line 158 falls back to a hardcoded18px. Consider using a token (e.g., a smaller--text-size-*variable) here too for consistency.♻️ Suggested fix
.form-section h2 { - font-size: 18px; + font-size: var(--text-size-normal); margin-bottom: 15px; }🤖 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 `@client/src/pages/pageStyles/AddIssue.css` around lines 148 - 171, The responsive override for .form-section h2 in AddIssue.css is still using a hardcoded font size, which breaks the design-token approach used by the base rule. Update the 1536px media query so the .form-section h2 font-size also uses an appropriate text-size token instead of 18px, keeping it consistent with the existing var(--text-size-large) usage and the rest of the AddIssue styles.client/src/pages/pageStyles/Dashboard.css (1)
93-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge duplicate
.ticket-pending-listselector in the same media block.The selector is declared twice back-to-back; consolidating avoids redundant rule blocks.
♻️ Suggested fix
`@media` (max-width: 1536px) { .ticket-pending-list { grid-template-columns: repeat(3, 1fr); - } - .ticket-pending-list { gap: 15px; padding: 10px 80px; } }🤖 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 `@client/src/pages/pageStyles/Dashboard.css` around lines 93 - 101, The `@media` block contains two back-to-back .ticket-pending-list rules, which should be merged into a single selector. Consolidate the properties in the Dashboard.css media query so .ticket-pending-list defines both grid-template-columns and the gap/padding together, removing the redundant duplicate block.client/src/pages/adminPage/IssueManagement.jsx (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport references a mistyped filename (
TIcketStatusFilter.jsx).Matches the actual (typo'd) filename, so it works, but the typo should eventually be fixed at the source across all importers.
🤖 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 `@client/src/pages/adminPage/IssueManagement.jsx` at line 23, The import in IssueManagement.jsx is pointing to a mistyped module name, so update the TicketStatusFilter import to use the correctly spelled filename and keep the symbol TicketStatusFilter unchanged. Check all other importers of TicketStatusFilter as well and make them reference the corrected component path so the typo is fixed consistently at the source.client/src/components/componentsAdmin/MergeManagementPanel.jsx (1)
66-207: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated ticket-item markup across four render sites.
The badge/title/meta JSX for main-ticket, sub-ticket (merge tab), and group main/sub-ticket (manage tab) is repeated near-verbatim four times. Extracting a small
TicketListItem(or similar) helper component parameterized by variant (main/sub) and action button would reduce this duplication and ease future styling/behavior changes.🤖 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 `@client/src/components/componentsAdmin/MergeManagementPanel.jsx` around lines 66 - 207, The ticket-item JSX is duplicated across the merge and manage render branches, including the badge/title/meta blocks and action buttons. Extract the repeated markup into a reusable helper component such as TicketListItem in MergeManagementPanel and parameterize it for main vs sub variants, ticket data, and optional actions like remove/unlink/disband so the four render sites share one implementation.client/src/components/componentsStyles/TicketCategoryFilter.css (1)
1-57: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsolidate triplicated filter stylesheet.
This file,
TicketLocationFilter.css, andTicketStatusFilter.cssare essentially copy-pasted (identical select/wrapper/focus/disabled rules and SVG background-image), differing only in class prefix and width values. Consider a shared base class (e.g.,.ticket-filter-wrapper/.ticket-filter-select) applied alongside a per-type modifier class for width overrides, to avoid three-way maintenance drift going forward.♻️ Example consolidation approach
.ticket-filter-wrapper { position: relative; display: flex; align-items: center; } .ticket-filter-select { width: 100%; padding: 12px 35px 12px 40px; border: 2px solid `#e0e0e0`; border-radius: 25px; background-color: `#ffffff`; font-size: 15px; color: `#334155`; cursor: pointer; appearance: none; background-image: url("..."); background-repeat: no-repeat; background-position: right 16px top 50%; background-size: 12px auto; transition: all 0.3s ease; box-shadow: 0 2px 5px rgba(0,0,0,0.05); } .ticket-filter-select:focus { outline: none; border-color: var(--tbs-green); box-shadow: 0 4px 10px rgba(0, 123, 255, 0.1); } .ticket-filter-select:disabled { background-color: `#f1f5f9`; color: `#94a3b8`; cursor: not-allowed; opacity: 0.8; } .category-filter-wrapper { width: 220px; } .location-filter-wrapper, .status-filter-wrapper { width: 180px; }🤖 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 `@client/src/components/componentsStyles/TicketCategoryFilter.css` around lines 1 - 57, The filter CSS rules are duplicated across the ticket category, location, and status stylesheets, so consolidate the shared wrapper/select/focus/disabled/SVG background styles into one base set (for example, a shared `.ticket-filter-wrapper` and `.ticket-filter-select`). Keep only the type-specific width overrides in `TicketCategoryFilter.css` and the other filter styles, and update the corresponding components to apply the shared base class plus any modifier class needed for the width differences.client/src/pages/adminPage/AuditIssues.css (1)
6-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFlex-child widths + margin sum beyond container width.
.audit-issues-card-list(width: 60%+margin-right: 2rem) and.audit-right-panel(width: 40%) sit in the same flex row (.audit-issues-content) with nogapdefined, so their combined footprint exceeds 100% of the container. This currently relies on implicit flex-shrink rather than explicit spacing, and could cause squeezing/overflow at some viewport widths. Consider usinggapon.audit-issues-contentinstead ofmargin-righton the card list.♻️ Proposed fix
.audit-issues-content { display: flex; + gap: 2rem; } .audit-issues-card-list { display: grid; grid-template-columns: repeat(auto-fill, minmax(320px, 1fr)); border-radius: 2rem; gap: 20px; - margin-right: 2rem; width: 60%; align-content: start; }🤖 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 `@client/src/pages/adminPage/AuditIssues.css` around lines 6 - 33, The flex layout in .audit-issues-content is overcommitted because .audit-issues-card-list uses width: 60% plus margin-right while .audit-right-panel uses width: 40%, which can cause squeezing or overflow. Move the spacing responsibility to .audit-issues-content by adding a gap and remove the right margin from .audit-issues-card-list; keep the two child widths in .audit-issues-card-list and .audit-right-panel aligned so the row fits cleanly.server/controllers/ticketManagementControllers.js (1)
258-296: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winFull ticket payload fetched for all pending tickets just to compute a 5-item ranking.
getUrgentTicketsloadslocation,floor,room,imagesfor everypendingticket to compute an in-memory urgency score, then discards all but the top 5. As pending-ticket volume grows this becomes an unnecessary I/O/memory cost on every call.♻️ Suggested two-step approach
- const pendingTickets = await prisma.ticket.findMany({ - where: { ticketStatus: 'pending' }, - include: { - location: true, - floor: true, - room: true, - images: true, - _count: { select: { subTickets: true } }, - upvotes: { select: { upvoteId: true } } - } - }); + // 1) Lightweight pass to compute scores + const scoreCandidates = await prisma.ticket.findMany({ + where: { ticketStatus: 'pending' }, + select: { + ticketId: true, + _count: { select: { subTickets: true, upvotes: true } } + } + }); + const top5Ids = scoreCandidates + .map(t => ({ ticketId: t.ticketId, score: (t._count.subTickets * 5) + t._count.upvotes })) + .filter(t => t.score > 0) + .sort((a, b) => b.score - a.score) + .slice(0, 5); + // 2) Fetch full data only for the winners + const pendingTickets = await prisma.ticket.findMany({ + where: { ticketId: { in: top5Ids.map(t => t.ticketId) } }, + include: { location: true, floor: true, room: true, images: 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 `@server/controllers/ticketManagementControllers.js` around lines 258 - 296, The getUrgentTickets controller is over-fetching full ticket payloads for every pending ticket just to rank and return the top 5. Refactor this logic to use a two-step approach in ticketManagementControllers: first fetch only the minimal fields needed to compute urgencyScore (for example ticketId, subTickets count, and upvote count), sort and select the top 5, then load the expanded relations (location, floor, room, images) only for those selected tickets before sending the response.server/controllers/ticketControllers.js (1)
21-21: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRedundant initial
generateTicketId()call.
customTicketIdgenerated at line 21 is only ever used to builddataToCreate.ticketId, but the retry loop (line 89-90) always regenerates and overwritesdataToCreate.ticketIdbefore the firstcreateattempt, making the outer generation a wasted async call.♻️ Proposed fix
- const files = req.files; // รับไฟล์จาก multer - const customTicketId = await generateTicketId(); + const files = req.files; // รับไฟล์จาก multerThen initialize
dataToCreate.ticketIdinside the retry loop only (already done at line 90), or remove the redundant field from the initialdataToCreateobject until the loop assigns it.Also applies to: 89-90
🤖 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 `@server/controllers/ticketControllers.js` at line 21, Remove the redundant outer generateTicketId() call in ticketControllers.js: the initial customTicketId is immediately overwritten by the retry logic before the first create attempt. Update the dataToCreate initialization so ticketId is assigned only inside the retry loop in the ticket creation flow, using the existing generateTicketId() assignment there and dropping the unused initial field.
🤖 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 `@client/src/components/componentsAdmin/MergeManagementPanel.css`:
- Around line 443-452: The keyframe identifier in the `@keyframes slideDown`
block should use kebab-case to match the codebase/stylelint convention. Rename
the animation definition to a kebab-case name and update any matching
`animation` references in `MergeManagementPanel` styles so they still point to
the renamed keyframes.
In `@client/src/components/componentsStyles/FeedbackModal.css`:
- Around line 9-15: The FeedbackModal styles have a blank line before the
animation declaration, which violates the stylelint formatting rule. Remove the
empty line in the modal/container CSS block so the animation property sits
directly with the other declarations, and apply the same cleanup to the other
flagged block in FeedbackModal.css. Keep the change limited to the relevant
selector blocks in the stylesheet.
- Around line 189-197: The keyframe names in FeedbackModal.css use camelCase,
which violates the kebab-case stylelint rule. Rename the `@keyframes` definitions
from fadeIn and slideUp to kebab-case names, and update the matching animation
references in the modal styles to use the new names so the component still
animates correctly.
In `@client/src/components/FeedbackModal.jsx`:
- Around line 93-104: The star rating in FeedbackModal is mouse-only, so make
the interactive star wrappers keyboard-accessible and screen-reader friendly.
Update the star wrapper render in the star list mapping to use semantic
button-like behavior or equivalent by adding an appropriate role, tab focus, and
keyboard handlers alongside the existing mouse handlers. Ensure the rating can
be changed with Enter/Space and that the current selection is announced so
handlePreSubmit can be satisfied without a mouse.
In `@client/src/components/TIcketStatusFilter.jsx`:
- Line 4: The import path in TicketStatusFilter.jsx is pointing to the wrong
stylesheet directory name, causing module resolution to fail. Update the CSS
import to use the actual componentsStyles folder, and apply the same correction
in TicketCategoryFilter.jsx and TicketLocationFilter.jsx where the same typo
appears. Keep the import paths consistent with the existing componentStyles
naming used by the project.
In `@client/src/hooks/useInfiniteScroll.js`:
- Around line 11-29: The early return in useInfiniteScroll’s lastElementRef
skips disconnecting the existing IntersectionObserver when node is cleared
during loading, which can leave a stale observer attached. Move the
observerRef.current.disconnect() cleanup to happen before the isLoading /
isFetchingNextPage return, and keep the rest of lastElementRef’s observe/setup
logic unchanged so null callback refs always clean up properly.
In `@client/src/hooks/useTicketDetail.js`:
- Around line 22-26: The useTicketDetail hook currently handles only the success
path after ticketService.getTicketById(ticketId), so when response.success is
false the UI never gets an error state. Update the logic in useTicketDetail to
add an explicit else branch alongside setTicket(response.data) that sets a
meaningful error message (and leaves ticket unset) when the API returns success:
false, so the hook can distinguish a failed response from a loading or empty
state.
In `@client/src/hooks/useTickets.js`:
- Around line 108-114: The refetch function in useTickets.js is using a fixed
timeout to resolve, so callers may continue before fetchTickets has actually
finished updating state. Replace the setTimeout-based resolution in refetch with
a completion-driven approach by using refetchResolversRef.current and resolving
any pending promises from the finally block in fetchTickets after loading ends.
Keep the change localized around refetch, fetchTickets, and the existing
refetchResolversRef logic so awaiting refetch() only completes once the refresh
is truly done.
In `@client/src/pages/adminPage/AuditIssues.css`:
- Around line 14-16: The block comment in AuditIssues.css is flagged by
stylelint because the comment delimiters are missing the expected internal
whitespace. Update the existing section comment so it follows the standard
block-comment spacing used by the stylesheet, keeping the same meaning while
fixing the formatting in the AuditIssues.css comment block.
In `@client/src/pages/adminPage/AuditIssues.jsx`:
- Around line 248-262: `MergeManagementPanel` is always receiving
`isLoading={false}`, so its reset/confirm disabled state never reflects the real
request status. Update `AuditIssues` to pass the actual loading flag from
`useLoadingState` (the `loading.isLoading` value or equivalent) into
`MergeManagementPanel` instead of a hardcoded false, so `handleResetSelection`
and `handleConfirmMerge` stay guarded during in-flight merge/unmerge actions.
- Around line 93-148: In submitMerge and submitUnmerge in AuditIssues.jsx,
handle the case where ticketService.mergeTickets and
ticketService.unmergeTickets resolve with success false instead of throwing;
currently only the success path clears the loading state and closes the modal.
Add an explicit else path after checking result.success that sets an error
message, stops the loading/progress state, and closes the confirm modal (or
otherwise resets it) so the UI does not մն stay stuck on "กำลังประมวลผล...". Use
the existing submitMerge, submitUnmerge, setError, setConfirmMergeModal,
setConfirmUnmergeModal, and startLoading flow as the entry points.
In `@client/src/pages/adminPage/IssueManagement.jsx`:
- Line 104: The loading state in IssueManagement is unreachable because the
`isLoading && normalTickets.length === 0` skeleton is nested inside the
`normalTickets.length === 0 ? ... : ...` branch, so it never renders when
needed. Restore the early-loading guard in `IssueManagement` before the
empty-state check, and remove the dead loading skeleton logic from the
`normalTickets` conditional so first render and filter changes show the loading
indicator instead of the false “no issues” message.
In `@client/src/pages/adminPage/IssueManagementDetail.jsx`:
- Around line 133-135: The conditional in IssueManagementDetail’s ticket image
fallback has an operator precedence bug, so the “no images” message is skipped
when ticket.images is missing or undefined. Update the JSX condition around the
ticket.images check to explicitly group the null/empty cases before rendering
the frame-no-images block, using the same ticket.images reference in
IssueManagementDetail so the fallback displays whenever there are no images.
- Around line 36-53: In submitUpdateStatus, handle the non-success response path
from ticketService.updateTicketStatusAdmin the same way as failures so the UI
does not stay loading forever. If result.success is falsy, set an error message,
close confirmModal, and ensure loading is cleared after the request finishes;
keep the existing success path and catch block behavior in
IssueManagementDetail.jsx. Also make sure any loading state started by
startLoading is always paired with a completion/reset path regardless of
outcome.
In `@client/src/pages/EditIssue.jsx`:
- Around line 344-355: The disabled-state guard in EditIssue.jsx is missing
formData.equipmentCode, so changes to equipment code are treated as “no changes”
and keep the Submit button disabled. Update the no-change comparison in the
button’s disabled condition to also compare formData.equipmentCode against
ticket.equipment?.equipmentCode, alongside the existing
categoryId/title/locationId/floorId/roomId/description/image checks, so
equipment-category edits are recognized as changes.
In `@client/src/pages/Tracking.jsx`:
- Around line 194-218: `handleConfirmCancelSubmit` currently uses `Promise.all`,
so one rejected `ticketService.cancelTicket` call prevents successful
cancellations in `selectedToCancel` from being removed locally. Update the bulk
cancel flow to use `Promise.allSettled` (or equivalent per-ticket handling) so
you can remove only the tickets that were actually canceled, then keep the
existing `removeTicket`, `refetch`, and `fetchSidebarCounts` steps in sync with
the partial success state. Make sure `setError` still reports failures for
rejected items while preserving the UI updates for fulfilled cancellations.
In `@client/src/services/ticketService.js`:
- Around line 144-150: The axios.patch call in ticketService’s
updateTicketStatusAdmin request is manually setting the multipart Content-Type
header for FormData, which can prevent the boundary from being added correctly.
Remove the explicit 'Content-Type': 'multipart/form-data' header from this
upload path and let axios/browser infer it automatically while keeping the
withCredentials option and existing request structure intact.
In `@server/controllers/getTicketControllers.js`:
- Around line 103-139: The getTicketControllers response select object has a
duplicate equipment key, so the earlier equipmentName selection is being
overwritten and never returned. Update the ticket query in the controller to
merge both equipmentName and equipmentCode into a single equipment select entry,
keeping the existing nested select shape intact so the response includes all
intended equipment fields.
In `@server/controllers/ticketManagementControllers.js`:
- Around line 336-351: The Cloudinary upload flow in the ticket status update
path can leak successful uploads when one file fails because `Promise.all`
rejects before `cloudinaryResults.forEach` can record rollback IDs. Update the
`uploadToCloudinary` handling in this block to use a failure-tolerant approach
such as `Promise.allSettled` (or equivalent per-file try/catch) so every
successful upload is captured in `uploadedImagesForRollback` and later cleanup
can remove any partial uploads.
In
`@server/prisma/migrations/20260606071851_change_rating_to_float/migration.sql`:
- Around line 19-38: The migration in the tickets/floors/rooms/equipments
foreign key section is too blocking for production because the rating type
change and immediate FK creation can take exclusive locks. Update the migration
flow to avoid table-wide blocking by splitting each foreign key add into a NOT
VALID creation followed by a separate VALIDATE CONSTRAINT step, and keep the
tickets.rating type change in mind when ordering operations in this migration
file.
---
Nitpick comments:
In `@client/src/components/componentsAdmin/MergeManagementPanel.jsx`:
- Around line 66-207: The ticket-item JSX is duplicated across the merge and
manage render branches, including the badge/title/meta blocks and action
buttons. Extract the repeated markup into a reusable helper component such as
TicketListItem in MergeManagementPanel and parameterize it for main vs sub
variants, ticket data, and optional actions like remove/unlink/disband so the
four render sites share one implementation.
In `@client/src/components/componentsStyles/TicketCategoryFilter.css`:
- Around line 1-57: The filter CSS rules are duplicated across the ticket
category, location, and status stylesheets, so consolidate the shared
wrapper/select/focus/disabled/SVG background styles into one base set (for
example, a shared `.ticket-filter-wrapper` and `.ticket-filter-select`). Keep
only the type-specific width overrides in `TicketCategoryFilter.css` and the
other filter styles, and update the corresponding components to apply the shared
base class plus any modifier class needed for the width differences.
In `@client/src/index.css`:
- Around line 118-120: The text-size CSS variables use an inconsistent naming
scheme, which makes the scale unclear at call sites. Update the custom
properties in the root token set to follow one consistent pattern in the same
section as --text-size-small, --text-size-base, and --text-size-normal, and
rename any related usages throughout the client styles to match the chosen
convention.
- Around line 79-86: The new `--status-reject-*` tokens in `index.css` use a
red/white pair that does not meet WCAG AA contrast for normal text. Update the
`--status-reject-bg` and/or `--status-reject-text` values to a higher-contrast
combination while keeping the new status label styling consistent, and verify
the same token pair is used wherever the reject status is rendered.
In `@client/src/pages/adminPage/AuditIssues.css`:
- Around line 6-33: The flex layout in .audit-issues-content is overcommitted
because .audit-issues-card-list uses width: 60% plus margin-right while
.audit-right-panel uses width: 40%, which can cause squeezing or overflow. Move
the spacing responsibility to .audit-issues-content by adding a gap and remove
the right margin from .audit-issues-card-list; keep the two child widths in
.audit-issues-card-list and .audit-right-panel aligned so the row fits cleanly.
In `@client/src/pages/adminPage/IssueManagement.jsx`:
- Line 23: The import in IssueManagement.jsx is pointing to a mistyped module
name, so update the TicketStatusFilter import to use the correctly spelled
filename and keep the symbol TicketStatusFilter unchanged. Check all other
importers of TicketStatusFilter as well and make them reference the corrected
component path so the typo is fixed consistently at the source.
In `@client/src/pages/pageStyles/AddIssue.css`:
- Around line 148-171: The responsive override for .form-section h2 in
AddIssue.css is still using a hardcoded font size, which breaks the design-token
approach used by the base rule. Update the 1536px media query so the
.form-section h2 font-size also uses an appropriate text-size token instead of
18px, keeping it consistent with the existing var(--text-size-large) usage and
the rest of the AddIssue styles.
In `@client/src/pages/pageStyles/Dashboard.css`:
- Around line 93-101: The `@media` block contains two back-to-back
.ticket-pending-list rules, which should be merged into a single selector.
Consolidate the properties in the Dashboard.css media query so
.ticket-pending-list defines both grid-template-columns and the gap/padding
together, removing the redundant duplicate block.
In `@server/controllers/ticketControllers.js`:
- Line 21: Remove the redundant outer generateTicketId() call in
ticketControllers.js: the initial customTicketId is immediately overwritten by
the retry logic before the first create attempt. Update the dataToCreate
initialization so ticketId is assigned only inside the retry loop in the ticket
creation flow, using the existing generateTicketId() assignment there and
dropping the unused initial field.
In `@server/controllers/ticketManagementControllers.js`:
- Around line 258-296: The getUrgentTickets controller is over-fetching full
ticket payloads for every pending ticket just to rank and return the top 5.
Refactor this logic to use a two-step approach in ticketManagementControllers:
first fetch only the minimal fields needed to compute urgencyScore (for example
ticketId, subTickets count, and upvote count), sort and select the top 5, then
load the expanded relations (location, floor, room, images) only for those
selected tickets before sending the response.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49ef2d50-41cb-425c-9007-c980b8e9d279
⛔ Files ignored due to path filters (2)
client/package-lock.jsonis excluded by!**/package-lock.jsonserver/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (81)
client/package.jsonclient/src/App.jsxclient/src/components/CardFinishProblem.jsxclient/src/components/CardPendingProblem.cssclient/src/components/CardPendingProblem.jsxclient/src/components/ConfirmButton.jsxclient/src/components/DateRangeFilter.jsxclient/src/components/FeedbackModal.jsxclient/src/components/FilterProblem.jsxclient/src/components/ImageUploader.jsxclient/src/components/LoadingSpinner.jsxclient/src/components/Navbar.jsxclient/src/components/SearchBar.jsxclient/src/components/SimilarTickets.jsxclient/src/components/StarRating.jsxclient/src/components/TIcketStatusFilter.jsxclient/src/components/TicketCategoryFilter.jsxclient/src/components/TicketLocationFilter.jsxclient/src/components/TrackingSidebar.cssclient/src/components/TrackingSidebar.jsxclient/src/components/componentsAdmin/AdminSidebar.jsxclient/src/components/componentsAdmin/Adminsidebar.cssclient/src/components/componentsAdmin/MergeManagementPanel.cssclient/src/components/componentsAdmin/MergeManagementPanel.jsxclient/src/components/componentsAdmin/TicketActionPanel.cssclient/src/components/componentsAdmin/TicketActionPanel.jsxclient/src/components/componentsStyles/CardPendingProblem.cssclient/src/components/componentsStyles/ConfirmButton.cssclient/src/components/componentsStyles/DateRangeFilter.cssclient/src/components/componentsStyles/FeedbackModal.cssclient/src/components/componentsStyles/FilterProblem.cssclient/src/components/componentsStyles/ImageUploader.cssclient/src/components/componentsStyles/LoadingSpinner.cssclient/src/components/componentsStyles/Navbar.cssclient/src/components/componentsStyles/SearchBar.cssclient/src/components/componentsStyles/SimilarTickets.cssclient/src/components/componentsStyles/TicketCategoryFilter.cssclient/src/components/componentsStyles/TicketLocationFilter.cssclient/src/components/componentsStyles/TicketStatusFilter.cssclient/src/components/componentsStyles/TrackingSidebar.cssclient/src/hooks/useEquipmentValidation.jsclient/src/hooks/useInfiniteScroll.jsclient/src/hooks/useMasterData.jsclient/src/hooks/useTicketDetail.jsclient/src/hooks/useTicketGroups.jsclient/src/hooks/useTickets.jsclient/src/hooks/useUrgentTickets.jsclient/src/index.cssclient/src/main.jsxclient/src/pages/AddIssue.jsxclient/src/pages/Dashboard.jsxclient/src/pages/DetailTicket.jsxclient/src/pages/EditIssue.jsxclient/src/pages/Login.jsxclient/src/pages/Tracking.jsxclient/src/pages/adminPage/AssetManagement.jsxclient/src/pages/adminPage/AuditIssues.cssclient/src/pages/adminPage/AuditIssues.jsxclient/src/pages/adminPage/IssueManagement.cssclient/src/pages/adminPage/IssueManagement.jsxclient/src/pages/adminPage/IssueManagementDetail.cssclient/src/pages/adminPage/IssueManagementDetail.jsxclient/src/pages/pageStyles/AddIssue.cssclient/src/pages/pageStyles/Dashboard.cssclient/src/pages/pageStyles/DetailTicket.cssclient/src/pages/pageStyles/EditIssue.cssclient/src/pages/pageStyles/Login.cssclient/src/pages/pageStyles/Tracking.cssclient/src/services/ticketService.jsclient/src/utils/timeline.jsserver/controllers/getTicketControllers.jsserver/controllers/managementControllers.jsserver/controllers/ticketControllers.jsserver/controllers/ticketManagementControllers.jsserver/package.jsonserver/prisma/migrations/20260606071851_change_rating_to_float/migration.sqlserver/prisma/schema.prismaserver/routes/managementRoutes.jsserver/routes/ticketManagementRoutes.jsserver/routes/ticketRoutes.jsserver/src/index.js
💤 Files with no reviewable changes (3)
- client/src/components/TrackingSidebar.css
- client/src/components/CardPendingProblem.css
- server/controllers/managementControllers.js
| @keyframes slideDown { | ||
| from { | ||
| opacity: 0; | ||
| transform: translateY(-10px); | ||
| } | ||
| to { | ||
| opacity: 1; | ||
| transform: translateY(0); | ||
| } | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keyframe name not kebab-case.
Stylelint flags slideDown; rename to kebab-case for consistency with the rest of the codebase's naming convention.
🔧 Proposed fix
-@keyframes slideDown {
+@keyframes slide-down {
from {
opacity: 0;
transform: translateY(-10px);
}
to {
opacity: 1;
transform: translateY(0);
}
}And update the reference:
- animation: slideDown 0.3s ease-out forwards;
+ animation: slide-down 0.3s ease-out forwards;🧰 Tools
🪛 Stylelint (17.14.0)
[error] 443-443: Expected keyframe name "slideDown" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 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 `@client/src/components/componentsAdmin/MergeManagementPanel.css` around lines
443 - 452, The keyframe identifier in the `@keyframes slideDown` block should
use kebab-case to match the codebase/stylelint convention. Rename the animation
definition to a kebab-case name and update any matching `animation` references
in `MergeManagementPanel` styles so they still point to the renamed keyframes.
Source: Linters/SAST tools
| display: flex; | ||
| align-items: center; | ||
| justify-content: center; | ||
| z-index: 1000; | ||
|
|
||
| animation: fadeIn 0.2s ease-in-out; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stylelint: remove blank line before animation declaration.
🛠️ Proposed fix
display: flex;
align-items: center;
justify-content: center;
z-index: 1000;
-
animation: fadeIn 0.2s ease-in-out;
}
/* กล่องเนื้อหา Modal */
.feedback-modal-content {
background-color: `#ffffff`;
width: 100%;
max-width: 450px;
border-radius: 20px;
padding: 30px;
position: relative;
box-shadow: 0 10px 40px rgba(0, 0, 0, 0.1);
-
animation: slideUp 0.3s ease-out;
}Also applies to: 25-28
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 14-14: Expected no empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 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 `@client/src/components/componentsStyles/FeedbackModal.css` around lines 9 -
15, The FeedbackModal styles have a blank line before the animation declaration,
which violates the stylelint formatting rule. Remove the empty line in the
modal/container CSS block so the animation property sits directly with the other
declarations, and apply the same cleanup to the other flagged block in
FeedbackModal.css. Keep the change limited to the relevant selector blocks in
the stylesheet.
Source: Linters/SAST tools
| @keyframes fadeIn { | ||
| from { opacity: 0; } | ||
| to { opacity: 1; } | ||
| } | ||
|
|
||
| @keyframes slideUp { | ||
| from { opacity: 0; transform: translateY(20px); } | ||
| to { opacity: 1; transform: translateY(0); } | ||
| } No newline at end of file |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Stylelint: keyframe names must be kebab-case.
🛠️ Proposed fix
-@keyframes fadeIn {
+@keyframes fade-in {
from { opacity: 0; }
to { opacity: 1; }
}
-@keyframes slideUp {
+@keyframes slide-up {
from { opacity: 0; transform: translateY(20px); }
to { opacity: 1; transform: translateY(0); }
}And update the two animation: references at lines 14 and 27 accordingly (fadeIn → fade-in, slideUp → slide-up).
📝 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.
| @keyframes fadeIn { | |
| from { opacity: 0; } | |
| to { opacity: 1; } | |
| } | |
| @keyframes slideUp { | |
| from { opacity: 0; transform: translateY(20px); } | |
| to { opacity: 1; transform: translateY(0); } | |
| } | |
| `@keyframes` fade-in { | |
| from { opacity: 0; } | |
| to { opacity: 1; } | |
| } | |
| `@keyframes` slide-up { | |
| from { opacity: 0; transform: translateY(20px); } | |
| to { opacity: 1; transform: translateY(0); } | |
| } |
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 189-189: Expected keyframe name "fadeIn" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
[error] 194-194: Expected keyframe name "slideUp" to be kebab-case (keyframes-name-pattern)
(keyframes-name-pattern)
🤖 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 `@client/src/components/componentsStyles/FeedbackModal.css` around lines 189 -
197, The keyframe names in FeedbackModal.css use camelCase, which violates the
kebab-case stylelint rule. Rename the `@keyframes` definitions from fadeIn and
slideUp to kebab-case names, and update the matching animation references in the
modal styles to use the new names so the component still animates correctly.
Source: Linters/SAST tools
| return ( | ||
| <div | ||
| key={index} | ||
| className="star-wrapper" | ||
| onMouseMove={(e) => handleMouseMove(e, index)} | ||
| onMouseLeave={handleMouseLeave} | ||
| onClick={(e) => handleStarClick(e, index)} | ||
| > | ||
| <StarIcon color={color} size={32} className="star-icon-svg" /> | ||
| </div> | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Star rating is not keyboard-accessible.
The rating control is a required field (handlePreSubmit blocks submission below 0.5 stars) but each star is a plain <div> reachable only via mouse (onMouseMove/onClick), with no role, tabIndex, or onKeyDown. Keyboard-only and screen-reader users cannot complete this form.
🛠️ Proposed fix
return (
<div
key={index}
className="star-wrapper"
+ role="button"
+ tabIndex={0}
+ aria-label={`ให้คะแนน ${index} ดาว`}
onMouseMove={(e) => handleMouseMove(e, index)}
onMouseLeave={handleMouseLeave}
onClick={(e) => handleStarClick(e, index)}
+ onKeyDown={(e) => {
+ if (e.key === 'Enter' || e.key === ' ') {
+ e.preventDefault();
+ setRating(index);
+ setErrorMsg('');
+ }
+ }}
>
<StarIcon color={color} size={32} className="star-icon-svg" />
</div>
);📝 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.
| return ( | |
| <div | |
| key={index} | |
| className="star-wrapper" | |
| onMouseMove={(e) => handleMouseMove(e, index)} | |
| onMouseLeave={handleMouseLeave} | |
| onClick={(e) => handleStarClick(e, index)} | |
| > | |
| <StarIcon color={color} size={32} className="star-icon-svg" /> | |
| </div> | |
| ); | |
| }); | |
| return ( | |
| <div | |
| key={index} | |
| className="star-wrapper" | |
| role="button" | |
| tabIndex={0} | |
| aria-label={`ให้คะแนน ${index} ดาว`} | |
| onMouseMove={(e) => handleMouseMove(e, index)} | |
| onMouseLeave={handleMouseLeave} | |
| onClick={(e) => handleStarClick(e, index)} | |
| onKeyDown={(e) => { | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| setRating(index); | |
| setErrorMsg(''); | |
| } | |
| }} | |
| > | |
| <StarIcon color={color} size={32} className="star-icon-svg" /> | |
| </div> | |
| ); | |
| }); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 100-100: A list component should have a key to prevent re-rendering
Context:
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.
(list-component-needs-key)
🤖 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 `@client/src/components/FeedbackModal.jsx` around lines 93 - 104, The star
rating in FeedbackModal is mouse-only, so make the interactive star wrappers
keyboard-accessible and screen-reader friendly. Update the star wrapper render
in the star list mapping to use semantic button-like behavior or equivalent by
adding an appropriate role, tab focus, and keyboard handlers alongside the
existing mouse handlers. Ensure the rating can be changed with Enter/Space and
that the current selection is announced so handlePreSubmit can be satisfied
without a mouse.
| import React from 'react'; | ||
| import { useMasterData } from '../hooks/useMasterData.js'; | ||
| import { FaFilter } from 'react-icons/fa'; | ||
| import '../components/componentStyles/TicketStatusFilter.css'; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Broken import path: componentStyles vs actual componentsStyles directory.
This import references ../components/componentStyles/TicketStatusFilter.css, but per the file layout the stylesheet actually lives under componentsStyles (with an "s" after "component"). This is a module-resolution failure at build time. The same typo is repeated in TicketCategoryFilter.jsx and TicketLocationFilter.jsx.
🐛 Proposed fix
-import '../components/componentStyles/TicketStatusFilter.css';
+import './componentsStyles/TicketStatusFilter.css';📝 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.
| import '../components/componentStyles/TicketStatusFilter.css'; | |
| import './componentsStyles/TicketStatusFilter.css'; |
🤖 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 `@client/src/components/TIcketStatusFilter.jsx` at line 4, The import path in
TicketStatusFilter.jsx is pointing to the wrong stylesheet directory name,
causing module resolution to fail. Update the CSS import to use the actual
componentsStyles folder, and apply the same correction in
TicketCategoryFilter.jsx and TicketLocationFilter.jsx where the same typo
appears. Keep the import paths consistent with the existing componentStyles
naming used by the project.
| const handleConfirmCancelSubmit = async () => { | ||
| startLoading(); | ||
| try { | ||
| const cancelPromises = selectedToCancel.map(id => ticketService.cancelTicket(id)); | ||
| await Promise.all(cancelPromises); | ||
|
|
||
| selectedToCancel.forEach(ticketId => { | ||
| removeTicket(ticketId); | ||
| }); | ||
|
|
||
| setSuccess(`ยกเลิกรายการแจ้งปัญหาจำนวน ${selectedToCancel.length} รายการสำเร็จ`); | ||
|
|
||
| setIsCancelMode(false); | ||
| setSelectedToCancel([]); | ||
| setConfirmCancelSubmit({ isOpen: false }); | ||
| // ✅ รอให้ Backend sync ข้อมูลเสร็จ | ||
| await refetch(); | ||
| fetchSidebarCounts(); | ||
|
|
||
| } catch (error) { | ||
| console.error("Error bulk canceling tickets:", error); | ||
| setError(error.response?.data?.message || "เกิดข้อผิดพลาดในการยกเลิกรายการ"); | ||
| setConfirmCancelSubmit({ isOpen: false }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Bulk cancel uses Promise.all; a single failure discards all local state updates even for tickets that succeeded server-side.
If one cancelTicket call rejects, Promise.all rejects immediately, so the catch block runs and none of selectedToCancel are removed locally — even though other tickets in the batch may have already been canceled on the backend. This leaves the UI showing stale (already-canceled) tickets as still active until a manual refresh.
🛠️ Proposed fix using `Promise.allSettled`
const handleConfirmCancelSubmit = async () => {
startLoading();
try {
- const cancelPromises = selectedToCancel.map(id => ticketService.cancelTicket(id));
- await Promise.all(cancelPromises);
-
- selectedToCancel.forEach(ticketId => {
- removeTicket(ticketId);
- });
-
- setSuccess(`ยกเลิกรายการแจ้งปัญหาจำนวน ${selectedToCancel.length} รายการสำเร็จ`);
+ const results = await Promise.allSettled(
+ selectedToCancel.map(id => ticketService.cancelTicket(id))
+ );
+
+ const succeeded = selectedToCancel.filter((_, i) => results[i].status === 'fulfilled');
+ succeeded.forEach(ticketId => removeTicket(ticketId));
+
+ const failedCount = results.length - succeeded.length;
+ if (failedCount > 0) {
+ setError(`ยกเลิกสำเร็จ ${succeeded.length} รายการ, ล้มเหลว ${failedCount} รายการ`);
+ } else {
+ setSuccess(`ยกเลิกรายการแจ้งปัญหาจำนวน ${selectedToCancel.length} รายการสำเร็จ`);
+ }
setIsCancelMode(false);
setSelectedToCancel([]);
setConfirmCancelSubmit({ isOpen: false });
// ✅ รอให้ Backend sync ข้อมูลเสร็จ
await refetch();
fetchSidebarCounts();
-
} catch (error) {
console.error("Error bulk canceling tickets:", error);
setError(error.response?.data?.message || "เกิดข้อผิดพลาดในการยกเลิกรายการ");
setConfirmCancelSubmit({ isOpen: false });
}
};📝 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.
| const handleConfirmCancelSubmit = async () => { | |
| startLoading(); | |
| try { | |
| const cancelPromises = selectedToCancel.map(id => ticketService.cancelTicket(id)); | |
| await Promise.all(cancelPromises); | |
| selectedToCancel.forEach(ticketId => { | |
| removeTicket(ticketId); | |
| }); | |
| setSuccess(`ยกเลิกรายการแจ้งปัญหาจำนวน ${selectedToCancel.length} รายการสำเร็จ`); | |
| setIsCancelMode(false); | |
| setSelectedToCancel([]); | |
| setConfirmCancelSubmit({ isOpen: false }); | |
| // ✅ รอให้ Backend sync ข้อมูลเสร็จ | |
| await refetch(); | |
| fetchSidebarCounts(); | |
| } catch (error) { | |
| console.error("Error bulk canceling tickets:", error); | |
| setError(error.response?.data?.message || "เกิดข้อผิดพลาดในการยกเลิกรายการ"); | |
| setConfirmCancelSubmit({ isOpen: false }); | |
| } | |
| }; | |
| const handleConfirmCancelSubmit = async () => { | |
| startLoading(); | |
| try { | |
| const results = await Promise.allSettled( | |
| selectedToCancel.map(id => ticketService.cancelTicket(id)) | |
| ); | |
| const succeeded = selectedToCancel.filter((_, i) => results[i].status === 'fulfilled'); | |
| succeeded.forEach(ticketId => removeTicket(ticketId)); | |
| const failedCount = results.length - succeeded.length; | |
| if (failedCount > 0) { | |
| setError(`ยกเลิกสำเร็จ ${succeeded.length} รายการ, ล้มเหลว ${failedCount} รายการ`); | |
| } else { | |
| setSuccess(`ยกเลิกรายการแจ้งปัญหาจำนวน ${selectedToCancel.length} รายการสำเร็จ`); | |
| } | |
| setIsCancelMode(false); | |
| setSelectedToCancel([]); | |
| setConfirmCancelSubmit({ isOpen: false }); | |
| // ✅ รอให้ Backend sync ข้อมูลเสร็จ | |
| await refetch(); | |
| fetchSidebarCounts(); | |
| } catch (error) { | |
| console.error("Error bulk canceling tickets:", error); | |
| setError(error.response?.data?.message || "เกิดข้อผิดพลาดในการยกเลิกรายการ"); | |
| setConfirmCancelSubmit({ isOpen: false }); | |
| } | |
| }; |
🤖 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 `@client/src/pages/Tracking.jsx` around lines 194 - 218,
`handleConfirmCancelSubmit` currently uses `Promise.all`, so one rejected
`ticketService.cancelTicket` call prevents successful cancellations in
`selectedToCancel` from being removed locally. Update the bulk cancel flow to
use `Promise.allSettled` (or equivalent per-ticket handling) so you can remove
only the tickets that were actually canceled, then keep the existing
`removeTicket`, `refetch`, and `fetchSidebarCounts` steps in sync with the
partial success state. Make sure `setError` still reports failures for rejected
items while preserving the UI updates for fulfilled cancellations.
| const response = await axios.patch(`/api/ticketManagement/updateTicketStatusAdmin/${ticketId}`, formData, { | ||
| withCredentials: true, | ||
| headers: { | ||
| 'Content-Type': 'multipart/form-data' | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== ticketService.js context ==\n'
if [ -f client/src/services/ticketService.js ]; then
nl -ba client/src/services/ticketService.js | sed -n '120,170p'
else
echo "client/src/services/ticketService.js not found"
fi
printf '\n== package.json axios versions ==\n'
for f in package.json client/package.json frontend/package.json; do
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,220p' "$f" | grep -n '"axios"' || true
fi
done
printf '\n== repo-wide multipart/FormData usage ==\n'
rg -n --hidden --glob '!node_modules' --glob '!dist' --glob '!build' "multipart/form-data|FormData" client/src . 2>/dev/null | sed -n '1,200p'Repository: CNPJB/trouble-tracking-system-project
Length of output: 1997
🌐 Web query:
axios FormData Content-Type boundary browser do not manually set multipart/form-data documentation
💡 Result:
When using FormData with axios in a browser, web worker, or React Native environment, you should not manually set the Content-Type header [1][2][3]. If you set the Content-Type header manually to multipart/form-data, it often lacks the required multipart boundary string, which causes the request to fail because the browser cannot automatically inject the correct boundary [4][5][6]. The browser runtime needs to generate and append this unique boundary automatically to correctly format the multipart request [1][2][3]. To send FormData correctly, simply pass the FormData instance directly as the payload to your axios request [2][7]: axios.post('https://your-api-endpoint.com/upload', formData); Axios will automatically detect the payload type and ensure the correct Content-Type header (including the necessary boundary) is set [1][2][7]. If you are using shortcut methods such as axios.postForm, axios.putForm, or axios.patchForm, these are designed specifically for this purpose and will handle the headers automatically [8][9][7]. Recent versions of axios include fixes to help prevent issues where manually set, incomplete headers might interfere with this automatic process [5][10][11].
Citations:
- 1: https://axios.rest/pages/advanced/multipart-form-data-format
- 2: https://github.com/axios/axios?tab=readme-ov-file
- 3: https://github.com/axios/axios
- 4: Axios fetch adapter: Missing boundary when Content-Type is set manually with FormData axios/axios#7054
- 5: axios/axios@62f6281
- 6: https://stackoverflow.com/questions/75742226/cant-set-content-type-header-for-axios-with-formdata
- 7: https://www.mintlify.com/axios/axios/advanced/form-data
- 8: https://github.com/axios/axios-docs/blob/master/posts/en/multipart.md
- 9: https://github.com/axios/axios/blob/v1.15.0/docs/pages/advanced/multipart-form-data-format.md
- 10: fix: remove forced multipart/form-data placeholder header in postForm/putForm/patchForm axios/axios#10980
- 11: fix(fetch): remove Content-Type without boundary for FormData axios/axios#7314
Drop the manual multipart Content-Type header
For FormData, let axios/browser set Content-Type so the multipart boundary is added automatically; hard-coding multipart/form-data can break parsing on this upload path.
🤖 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 `@client/src/services/ticketService.js` around lines 144 - 150, The axios.patch
call in ticketService’s updateTicketStatusAdmin request is manually setting the
multipart Content-Type header for FormData, which can prevent the boundary from
being added correctly. Remove the explicit 'Content-Type': 'multipart/form-data'
header from this upload path and let axios/browser infer it automatically while
keeping the withCredentials option and existing request structure intact.
| select: { | ||
| ticketId: true, | ||
| category: { select: { ticketCtgName: true } }, | ||
| location: { select: { locationName: true } }, | ||
| floor: { select: { floorLevel: true } }, | ||
| room: { select: { roomId: true, roomName: true } }, | ||
| equipment: { select: { equipmentName: true } }, | ||
| title: true, | ||
| description: true, | ||
| ticketStatus: true, | ||
| parentTicketId: true, | ||
| adminId: true, | ||
| adminNote: true, | ||
| rating: true, | ||
| comment: true, | ||
| createdAt: true, | ||
| updatedAt: true, | ||
| images: { select: { imageUrl: true, imageType: true } }, | ||
| ticketCtgId: true, | ||
| locationId: true, | ||
| floorId: true, | ||
| roomId: true, | ||
| equipment: { select: { equipmentCode: true } }, | ||
| upvotes: { select: { userId: true } }, | ||
| user: { select: { userId: true, fullName: true } }, | ||
| admin: { select: { userId: true, fullName: true } }, | ||
| timestampInprogress: true, | ||
| timestampFinished: true, | ||
| _count: { | ||
| select: { subTickets: true } | ||
| }, | ||
| subTickets: { | ||
| include: { | ||
| user: true | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Duplicate equipment key silently drops equipmentName from the response.
equipment is defined twice in this select object (line 109: { select: { equipmentName: true } }, line 125: { select: { equipmentCode: true } }). JS object literals keep only the last duplicate key, so only equipmentCode is ever returned — equipmentName is silently lost even though search filters on equipment.equipmentName. Flagged by Biome (noDuplicateObjectKeys).
🐛 Proposed fix: merge into a single key
- equipment: { select: { equipmentName: true } },
title: true,
@@
- equipment: { select: { equipmentCode: true } },
+ equipment: { select: { equipmentName: true, equipmentCode: true } },📝 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.
| select: { | |
| ticketId: true, | |
| category: { select: { ticketCtgName: true } }, | |
| location: { select: { locationName: true } }, | |
| floor: { select: { floorLevel: true } }, | |
| room: { select: { roomId: true, roomName: true } }, | |
| equipment: { select: { equipmentName: true } }, | |
| title: true, | |
| description: true, | |
| ticketStatus: true, | |
| parentTicketId: true, | |
| adminId: true, | |
| adminNote: true, | |
| rating: true, | |
| comment: true, | |
| createdAt: true, | |
| updatedAt: true, | |
| images: { select: { imageUrl: true, imageType: true } }, | |
| ticketCtgId: true, | |
| locationId: true, | |
| floorId: true, | |
| roomId: true, | |
| equipment: { select: { equipmentCode: true } }, | |
| upvotes: { select: { userId: true } }, | |
| user: { select: { userId: true, fullName: true } }, | |
| admin: { select: { userId: true, fullName: true } }, | |
| timestampInprogress: true, | |
| timestampFinished: true, | |
| _count: { | |
| select: { subTickets: true } | |
| }, | |
| subTickets: { | |
| include: { | |
| user: true | |
| } | |
| } | |
| } | |
| select: { | |
| ticketId: true, | |
| category: { select: { ticketCtgName: true } }, | |
| location: { select: { locationName: true } }, | |
| floor: { select: { floorLevel: true } }, | |
| room: { select: { roomId: true, roomName: true } }, | |
| equipment: { select: { equipmentName: true, equipmentCode: true } }, | |
| title: true, | |
| description: true, | |
| ticketStatus: true, | |
| parentTicketId: true, | |
| adminId: true, | |
| adminNote: true, | |
| rating: true, | |
| comment: true, | |
| createdAt: true, | |
| updatedAt: true, | |
| images: { select: { imageUrl: true, imageType: true } }, | |
| ticketCtgId: true, | |
| locationId: true, | |
| floorId: true, | |
| roomId: true, | |
| upvotes: { select: { userId: true } }, | |
| user: { select: { userId: true, fullName: true } }, | |
| admin: { select: { userId: true, fullName: true } }, | |
| timestampInprogress: true, | |
| timestampFinished: true, | |
| _count: { | |
| select: { subTickets: true } | |
| }, | |
| subTickets: { | |
| include: { | |
| user: true | |
| } | |
| } | |
| } |
🧰 Tools
🪛 Biome (2.5.1)
[error] 109-109: This property is later overwritten by an object member with the same name.
(lint/suspicious/noDuplicateObjectKeys)
🤖 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 `@server/controllers/getTicketControllers.js` around lines 103 - 139, The
getTicketControllers response select object has a duplicate equipment key, so
the earlier equipmentName selection is being overwritten and never returned.
Update the ticket query in the controller to merge both equipmentName and
equipmentCode into a single equipment select entry, keeping the existing nested
select shape intact so the response includes all intended equipment fields.
Source: Linters/SAST tools
| const uploadedImagesData = []; | ||
| if (ticketStatus === 'resolved' && files && files.length > 0) { | ||
| // ใช้เทคนิค Promise.all เหมือนใน addTicket[cite: 27] | ||
| const uploadPromises = files.map((file) => uploadToCloudinary(file.buffer, 'TTS-img')); | ||
| const cloudinaryResults = await Promise.all(uploadPromises); | ||
|
|
||
| cloudinaryResults.forEach((result) => { | ||
| uploadedImagesData.push({ | ||
| imageUrl: result.secure_url, | ||
| imageType: "after", // กำหนดว่าเป็นรูป "หลังซ่อม" | ||
| imagePublicId: result.public_id, | ||
| }); | ||
| // เก็บ Public ID ไว้เผื่อ Database พัง จะได้ตามไปลบทิ้งได้[cite: 27] | ||
| uploadedImagesForRollback.push(result.public_id); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Orphaned Cloudinary uploads on partial Promise.all failure.
If one file in files fails to upload, Promise.all rejects immediately; the .forEach at line 342 that populates uploadedImagesForRollback never runs, so any uploads that did succeed before the failure are never tracked and never cleaned up — leaking storage/cost in Cloudinary indefinitely.
🐛 Proposed fix using allSettled
- const uploadPromises = files.map((file) => uploadToCloudinary(file.buffer, 'TTS-img'));
- const cloudinaryResults = await Promise.all(uploadPromises);
-
- cloudinaryResults.forEach((result) => {
- uploadedImagesData.push({
- imageUrl: result.secure_url,
- imageType: "after",
- imagePublicId: result.public_id,
- });
- uploadedImagesForRollback.push(result.public_id);
- });
+ const uploadResults = await Promise.allSettled(
+ files.map((file) => uploadToCloudinary(file.buffer, 'TTS-img'))
+ );
+ const failed = uploadResults.filter(r => r.status === 'rejected');
+
+ uploadResults.forEach((r) => {
+ if (r.status === 'fulfilled') {
+ uploadedImagesData.push({
+ imageUrl: r.value.secure_url,
+ imageType: "after",
+ imagePublicId: r.value.public_id,
+ });
+ uploadedImagesForRollback.push(r.value.public_id);
+ }
+ });
+
+ if (failed.length > 0) {
+ await Promise.all(uploadedImagesForRollback.map(deleteFromCloudinary)).catch(() => {});
+ throw new Error("IMAGE_UPLOAD_FAILED");
+ }📝 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.
| const uploadedImagesData = []; | |
| if (ticketStatus === 'resolved' && files && files.length > 0) { | |
| // ใช้เทคนิค Promise.all เหมือนใน addTicket[cite: 27] | |
| const uploadPromises = files.map((file) => uploadToCloudinary(file.buffer, 'TTS-img')); | |
| const cloudinaryResults = await Promise.all(uploadPromises); | |
| cloudinaryResults.forEach((result) => { | |
| uploadedImagesData.push({ | |
| imageUrl: result.secure_url, | |
| imageType: "after", // กำหนดว่าเป็นรูป "หลังซ่อม" | |
| imagePublicId: result.public_id, | |
| }); | |
| // เก็บ Public ID ไว้เผื่อ Database พัง จะได้ตามไปลบทิ้งได้[cite: 27] | |
| uploadedImagesForRollback.push(result.public_id); | |
| }); | |
| } | |
| const uploadedImagesData = []; | |
| if (ticketStatus === 'resolved' && files && files.length > 0) { | |
| // ใช้เทคนิค Promise.all เหมือนใน addTicket[cite: 27] | |
| const uploadResults = await Promise.allSettled( | |
| files.map((file) => uploadToCloudinary(file.buffer, 'TTS-img')) | |
| ); | |
| const failed = uploadResults.filter(r => r.status === 'rejected'); | |
| uploadResults.forEach((r) => { | |
| if (r.status === 'fulfilled') { | |
| uploadedImagesData.push({ | |
| imageUrl: r.value.secure_url, | |
| imageType: "after", // กำหนดว่าเป็นรูป "หลังซ่อม" | |
| imagePublicId: r.value.public_id, | |
| }); | |
| // เก็บ Public ID ไว้เผื่อ Database พัง จะได้ตามไปลบทิ้งได้[cite: 27] | |
| uploadedImagesForRollback.push(r.value.public_id); | |
| } | |
| }); | |
| if (failed.length > 0) { | |
| await Promise.all(uploadedImagesForRollback.map(deleteFromCloudinary)).catch(() => {}); | |
| throw new Error("IMAGE_UPLOAD_FAILED"); | |
| } | |
| } |
🤖 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 `@server/controllers/ticketManagementControllers.js` around lines 336 - 351,
The Cloudinary upload flow in the ticket status update path can leak successful
uploads when one file fails because `Promise.all` rejects before
`cloudinaryResults.forEach` can record rollback IDs. Update the
`uploadToCloudinary` handling in this block to use a failure-tolerant approach
such as `Promise.allSettled` (or equivalent per-file try/catch) so every
successful upload is captured in `uploadedImagesForRollback` and later cleanup
can remove any partial uploads.
| -- AlterTable | ||
| ALTER TABLE "tickets" ALTER COLUMN "rating" SET DATA TYPE DOUBLE PRECISION; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "floors" ADD CONSTRAINT "floors_location_id_fkey" FOREIGN KEY ("location_id") REFERENCES "locations"("location_id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "rooms" ADD CONSTRAINT "rooms_floor_id_fkey" FOREIGN KEY ("floor_id") REFERENCES "floors"("floor_id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "equipments" ADD CONSTRAINT "equipments_room_id_fkey" FOREIGN KEY ("room_id") REFERENCES "rooms"("room_id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "ticket_images" ADD CONSTRAINT "ticket_images_ticket_id_fkey" FOREIGN KEY ("ticket_id") REFERENCES "tickets"("ticket_id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "tickets" ADD CONSTRAINT "tickets_location_id_fkey" FOREIGN KEY ("location_id") REFERENCES "locations"("location_id") ON DELETE CASCADE ON UPDATE CASCADE; | ||
|
|
||
| -- AddForeignKey | ||
| ALTER TABLE "tickets" ADD CONSTRAINT "tickets_room_id_fkey" FOREIGN KEY ("room_id") REFERENCES "rooms"("room_id") ON DELETE CASCADE ON UPDATE CASCADE; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Migration will block reads/writes on tickets, floors, rooms, equipments.
Squawk flags: the column type change takes an ACCESS EXCLUSIVE lock during table rewrite, and all 6 re-added FK constraints lack NOT VALID, each requiring a full table scan under a blocking lock. On tables with meaningful production traffic this can cause request timeouts/outages during deploy.
🔒 Safer migration pattern
--- ALTER TABLE "tickets" ALTER COLUMN "rating" SET DATA TYPE DOUBLE PRECISION;
+-- Note: INT -> DOUBLE PRECISION still requires a table rewrite; consider running
+-- this during a low-traffic window or via a shadow-column + backfill + swap.
--- ALTER TABLE "floors" ADD CONSTRAINT "floors_location_id_fkey" ...;
+ALTER TABLE "floors" ADD CONSTRAINT "floors_location_id_fkey" ... NOT VALID;
+-- run in a separate migration/transaction:
+-- ALTER TABLE "floors" VALIDATE CONSTRAINT "floors_location_id_fkey";(repeat NOT VALID + VALIDATE CONSTRAINT split for the other 5 FKs)
🧰 Tools
🪛 Squawk (2.59.0)
[warning] 20-20: Changing a column type requires an ACCESS EXCLUSIVE lock on the table which blocks reads and writes while the table is rewritten. Changing the type of the column may also break other clients reading from the table.
(changing-column-type)
[warning] 23-23: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 23-23: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
[warning] 26-26: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 26-26: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
[warning] 29-29: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 29-29: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
[warning] 32-32: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 32-32: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
[warning] 35-35: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 35-35: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
[warning] 38-38: By default new constraints require a table scan and block writes to the table while that scan occurs. Use NOT VALID with a later VALIDATE CONSTRAINT call.
(constraint-missing-not-valid)
[warning] 38-38: Adding a foreign key constraint requires a table scan and a SHARE ROW EXCLUSIVE lock on both tables, which blocks writes to each table. Add NOT VALID to the constraint in one transaction and then VALIDATE the constraint in a separate transaction.
(adding-foreign-key-constraint)
🤖 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 `@server/prisma/migrations/20260606071851_change_rating_to_float/migration.sql`
around lines 19 - 38, The migration in the tickets/floors/rooms/equipments
foreign key section is too blocking for production because the rating type
change and immediate FK creation can take exclusive locks. Update the migration
flow to avoid table-wide blocking by splitting each foreign key add into a NOT
VALID creation followed by a separate VALIDATE CONSTRAINT step, and keep the
tickets.rating type change in mind when ordering operations in this migration
file.
Source: Linters/SAST tools
1 participant
Add 2 commit: Edit folder name/path and Add Feature.
Summary by CodeRabbit
New Features
Bug Fixes