feature/addmin/addlocation addCategories - #10
Conversation
|
Caution Review failedFailed to post review comments 📝 WalkthroughWalkthroughThis PR adds a comprehensive admin management system for ticket-tracking, including four independent admin pages (Equipment, Categories, Locations/Rooms, Users) with CRUD operations, file upload capability for bulk equipment import, shared admin UI components, ticket display loading states with skeleton placeholders, and database cascade delete integrity. All changes are accompanied by theme variable updates and backend route/controller implementations. ChangesAdmin Routing, Navigation & Shared Components
Equipment & Asset Management
Category Management
Location, Floor & Room Hierarchy Management
User Management
Ticket Display Loading States & Skeleton UI
Issue Creation Form
Database Cascade Delete Integrity
Backend Infrastructure & Dependencies
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 15
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
client/src/pages/Dashboard.css (1)
31-31:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix CSS variable syntax error.
The variable reference
var(---text-color-main)has three leading dashes instead of two. CSS custom properties use exactly two dashes (--), so this will fail to resolve and likely fall back to a default color or break the styling.🐛 Proposed fix
- color: var(---text-color-main); + color: var(--text-color-main);🤖 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/Dashboard.css` at line 31, The CSS uses an invalid custom property name in Dashboard.css where the declaration color: var(---text-color-main); has three leading hyphens; update the variable reference to use the correct two-dash custom property syntax by changing var(---text-color-main) to var(--text-color-main) so the --text-color-main custom property resolves correctly.client/src/pages/AddIssue.jsx (1)
175-222:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win
isSubmittingis never reset on error, blocking retry attempts.If the API call fails,
isSubmittingremainstrueand the user cannot attempt to submit again. The confirm button stays disabled permanently until the page is refreshed.🐛 Proposed fix using finally block
if (response.data.success) { alert("แจ้งปัญหาสำเร็จเรียบร้อยแล้ว!"); navigate('/tracking'); } } catch (error) { console.error("Error submitting ticket:", error); alert(error.response?.data?.message || "เกิดข้อผิดพลาดในการบันทึกข้อมูล"); + } finally { + setIsSubmitting(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/AddIssue.jsx` around lines 175 - 222, The submit flow leaves isSubmitting true on errors; update the submit handler (where isSubmitting and setIsSubmitting are used) to always reset isSubmitting by adding a finally block (or calling setIsSubmitting(false) in both success and catch paths) after the try/catch so retries are possible; reference the existing isSubmitting, setIsSubmitting, and the try/catch around the axios.post call to locate where to add the finally reset.
♻️ Duplicate comments (1)
client/src/pages/adminPage/LocationManagement.jsx (1)
9-9:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winImport path casing will break case-sensitive builds.
Same issue as
Categories.jsx: the file isPopupAlert.jsx, sopopupAlertwill fail to resolve on Linux CI/containers.🐛 Proposed fix
-import { PopupAlert } from '../../components/componentsAdmin/popupAlert'; +import { PopupAlert } from '../../components/componentsAdmin/PopupAlert';🤖 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/LocationManagement.jsx` at line 9, The import in LocationManagement.jsx uses incorrect casing and will fail on case-sensitive filesystems; update the import statement that currently references '../../components/componentsAdmin/popupAlert' to match the actual filename 'PopupAlert.jsx' (i.e., import PopupAlert from '../../components/componentsAdmin/PopupAlert'), mirroring the same fix applied in Categories.jsx so the PopupAlert symbol resolves correctly.
🟡 Minor comments (11)
client/src/components/CardPendingProblem.jsx-51-51 (1)
51-51:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against undefined values for ticketId and admin.
Both
data.ticketIdanddata.adminare rendered without null/undefined checks. If these fields are missing from the API response, the UI will display "undefined" to users.🛡️ Proposed fix to add fallback values
- <p style={{ color: 'gray' }}>{data.ticketId}</p> + <p style={{ color: 'gray' }}>{data.ticketId || 'N/A'}</p> <h3>เรื่อง : {data.title}</h3><p>รายละเอียด : {data.description}</p> - <p>ผู้ดำเนินการ : {data.admin}</p> + <p>ผู้ดำเนินการ : {data.admin || 'ยังไม่มีผู้รับผิดชอบ'}</p>Also applies to: 66-66
🤖 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/CardPendingProblem.jsx` at line 51, In CardPendingProblem.jsx, guard against undefined values when rendering data.ticketId and data.admin by using optional chaining and a fallback (e.g., data?.ticketId || '—' and data?.admin || 'Unknown') in the JSX where ticketId and admin are displayed; update the render expressions that reference data.ticketId and data.admin so they won't render the literal "undefined" when the API omits those fields.client/src/pages/Dashboard.css-54-56 (1)
54-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove or populate the empty CSS rule.
The
.filter-containerselector has an empty block with no declarations. Empty rules add no value and clutter the stylesheet.♻️ Proposed fix
If no styling is needed, remove the rule entirely:
-.filter-container { - -}Otherwise, add the intended declarations.
🤖 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/Dashboard.css` around lines 54 - 56, The .filter-container rule is empty and should be removed or given its intended styles; locate the .filter-container selector in Dashboard.css and either delete the empty block to clean up the stylesheet or populate it with the required declarations (e.g., layout, spacing, or visibility properties) so it serves a purpose.client/src/pages/AddIssue.jsx-57-57 (1)
57-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove debug console.log statement.
This debug logging should be removed before merging.
🧹 Proposed fix
setEquipments(equipRes.data); - console.log(equipRes) } catch (error) {🤖 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/AddIssue.jsx` at line 57, Remove the debug console.log(equipRes) statement from the AddIssue.jsx component (where equipRes is logged after the equipment response) — either delete the console.log call or replace it with a proper error/operation handler (e.g., update component state or call a logger function) so no debug-only console output remains in production code; locate the console.log in the AddIssue component's function that handles the equipment response and remove or refactor it accordingly.client/src/components/CardPendingProblem.css-1-2 (1)
1-2:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove empty CSS rule.
The empty
p, span {}rule on lines 1-2 serves no purpose and should be removed.🗑️ Proposed fix
-p, -span {} - .container-pending-card { width: 100%;🤖 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/CardPendingProblem.css` around lines 1 - 2, Remove the no-op CSS rule in CardPendingProblem.css by deleting the empty selector block "p, span {}" so the file contains only meaningful styles; locate the empty rule (the "p, span" selector) and remove those lines to clean up the stylesheet.client/src/components/componentsAdmin/PopupAlert.jsx-5-12 (1)
5-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAuto-close timer may reset on parent re-renders.
The
useEffectincludesonClosein its dependency array. If the parent component doesn't memoizeonClose(viauseCallback), every parent re-render will create a new function reference, causing this effect to re-run and restart the 3-second timer. This can prevent the popup from auto-closing as expected.🔧 Recommended fix options
Option 1: Remove
onClosefrom dependencies and use ESLint disable commentuseEffect(() => { if (isOpen) { const timer = setTimeout(() => { onClose(); }, 3000); return () => clearTimeout(timer); } + // eslint-disable-next-line react-hooks/exhaustive-deps - }, [isOpen, onClose]); + }, [isOpen]);Option 2: Ask parent components to memoize
onCloseEnsure all consumers wrap their
onClosehandlers withuseCallback:const handleClose = useCallback(() => { setIsOpen(false); }, []); <PopupAlert onClose={handleClose} ... />🤖 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/PopupAlert.jsx` around lines 5 - 12, The auto-close useEffect in PopupAlert resets whenever onClose reference changes; fix by stabilizing the callback via a ref: create a ref (e.g., onCloseRef) and update it in a small effect when onClose changes (onCloseRef.current = onClose), then change the auto-close effect to depend only on isOpen and use onCloseRef.current() inside the timer callback, keeping the existing clearTimeout cleanup so the timer isn't restarted by parent re-renders.client/src/components/componentsAdmin/PopupAlert.css-3-8 (1)
3-8:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComment contradicts actual positioning logic.
The Thai comment on line 3 claims the popup appears in the "upper right corner" (มุมขวาบน), but the CSS uses
left: 50%withtransform: translate(-50%, -50%)which centers the element horizontally. Update the comment to reflect that the popup is top-center positioned.📝 Proposed comment correction
-/* ส่วนคลุมให้อยู่มุมขวาบนและทำแอนิเมชันตอนเลื่อนเข้ามา */ +/* ส่วนคลุมให้อยู่ตรงกลางด้านบนและทำแอนิเมชันตอนเลื่อนเข้ามา */ .popup-alert-overlay { position: fixed; top: 15%; - left: 50%; /* ดันมาขวาครึ่งจอ */ - transform: translate(-50%, -50%); /* ดึงกลับให้จุดศูนย์กลางของกล่องอยู่ตรงกลางเป๊ะ */ + left: 50%; /* วางตำแหน่งกึ่งกลางแนวนอน */ + transform: translate(-50%, -50%); /* จัดให้อยู่ตรงกลางพอดี */🤖 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/PopupAlert.css` around lines 3 - 8, Update the misleading Thai comment for the .popup-alert-overlay CSS: replace the phrase stating it appears in the "upper right corner" (มุมขวาบน) with a comment that accurately describes the positioning as top-center (e.g., "อยู่ตรงกลางด้านบน" or equivalent), since left: 50% with transform: translate(-50%, -50%) centers the element horizontally at the top. Ensure the comment still mentions the animation on entry if relevant.client/src/pages/adminPage/LocationManagement.css-122-124 (1)
122-124:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove blank lines before declarations (stylelint failures).
Stylelint flags
declaration-empty-line-beforeat Lines 124 and 158. Drop the blank line preceding each declaration.🎨 Fix
justify-content: space-between; - transition: all 0.2s ease-in-out;.manage-location-container select { - width: 100%;🤖 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/LocationManagement.css` around lines 122 - 124, Stylelint reports declaration-empty-line-before violations in LocationManagement.css; remove the blank line immediately before the declarations (e.g., the lines declaring "justify-content: space-between;" and "transition: all 0.2s ease-in-out;") so there is no empty line preceding each property, ensuring the CSS rule block has properties listed consecutively with no blank lines (this fixes the declaration-empty-line-before errors).client/src/components/componentsAdmin/ImportEquipments.jsx-41-68 (1)
41-68:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNo visible loading state while a selected file is uploading.
When a file is selected, the import button is replaced by the file badge, and the confirm button is hidden during submit (
file && !isSubmitting). So whileisSubmittingis true there is no spinner or disabled affordance — the UI looks frozen. Consider keeping the confirm button mounted with a disabled/“กำลังส่ง...” state instead of unmounting it.🤖 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/ImportEquipments.jsx` around lines 41 - 68, The selected file confirm button is unmounted while uploading so there’s no visible loading state; keep the confirm upload button (confirm-upload-file) mounted and show a disabled/loading state by rendering it whenever file is present and binding its disabled prop to isSubmitting, change its label to 'กำลังส่ง...' (or show a spinner) when isSubmitting, and ensure the original import button (btn-import-custom) is also disabled while isSubmitting; update handlers uploadFile, handleButtonClick and the buttons' disabled logic to rely on isSubmitting so the UI shows a clear in-progress affordance.client/src/hooks/useImportEquipments.js-27-31 (1)
27-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
window.location.reload()runs before the success feedback, making it unreliable.Calling
window.location.reload()at Line 28 starts navigation, so the subsequentalert(response.data.message),setFile(null), andreturn truemay not run reliably — the user often won't see the success message. Reload after acknowledging success (or drop the alert and just refetch state instead of a full reload).🛠 Proposed reorder
const response = await axios.post(url, formData); - window.location.reload(); alert(response.data.message); setFile(null); + window.location.reload(); return 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 `@client/src/hooks/useImportEquipments.js` around lines 27 - 31, The success reload is happening before user feedback so alert(response.data.message), setFile(null), and return true may not execute; update the useImportEquipments hook to either (A) move window.location.reload() to after the alert and setFile calls so axios.post(...), then alert(response.data.message), setFile(null), and finally window.location.reload(), or (B) remove window.location.reload() entirely and instead update local state or trigger a refetch (e.g., call the parent refresh handler) after receiving response from axios.post to reflect the new data without forcing a full page reload; ensure you reference the axios.post call, response.data.message, and setFile when making the change.client/src/pages/adminPage/Categories.jsx-17-21 (1)
17-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInitial
formDatakeys don't match the keys used everywhere else.The initial state declares
ticketCtgIdName/ticketCtgIdStatus, buthandleEditClick,handleSaveCategory, and the inputs all useticketCtgName/ticketCtgStatus. The declared properties are dead, and the real fields beginundefined.🐛 Proposed fix
const [formData, setFormData] = useState({ ticketCtgId: '', - ticketCtgIdName: '', - ticketCtgIdStatus: '' + ticketCtgName: '', + ticketCtgStatus: '' });🤖 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/Categories.jsx` around lines 17 - 21, The initial state for formData uses incorrect keys (ticketCtgIdName, ticketCtgIdStatus) causing undefined values elsewhere; update the useState initializer in Categories.jsx so formData contains the correct keys (ticketCtgId, ticketCtgName, ticketCtgStatus) to match how handleEditClick, handleSaveCategory, the input components and setFormData expect and use those properties.client/src/pages/adminPage/LocationManagement.jsx-179-179 (1)
179-179:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConfirmation text says "add floor" but this adds a location.
handleAddNewLocationreuses the floor confirmation message ("ยืนยันการเพิ่มชั้นนี้?"). Update it to reflect adding a location.🐛 Proposed fix
- if (window.confirm("ยืนยันการเพิ่มชั้นนี้?")) { + if (window.confirm("ยืนยันการเพิ่มสถานที่นี้?")) {🤖 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/LocationManagement.jsx` at line 179, The confirmation prompt in handleAddNewLocation uses the floor-specific text ("ยืนยันการเพิ่มชั้นนี้?") but this function adds a location; update the window.confirm call inside handleAddNewLocation to use a location-appropriate message (e.g., "ยืนยันการเพิ่มสถานที่นี้?") so the confirmation accurately reflects the action.
🧹 Nitpick comments (14)
client/src/components/ConfirmButton.jsx (1)
11-14: ⚡ Quick winRemove unused props
typeandform.The
typeandformprops are destructured but never referenced in the component JSX. This unnecessarily expands the component API without providing functionality.🧹 Proposed fix
export const ConfirmButton = ({ isOpen, title, message, onConfirm, onCancel, confirmText = "ยืนยัน", cancelText = "ยกเลิก", - type = "button", - form, disabled }) => {🤖 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/ConfirmButton.jsx` around lines 11 - 14, The ConfirmButton component currently destructures unused props type and form (in the props list in ConfirmButton.jsx); remove type and form from the component's parameter destructuring and drop any corresponding defaultProps/propTypes or JSDoc entries for type and form (if present) so the component API no longer advertises unused props; scan for and remove any dead references to type/form inside ConfirmButton-related code to keep the surface area minimal.client/src/components/componentsAdmin/DropdownAdd.jsx (1)
50-92: 💤 Low valueConsider extracting inline styles to a CSS file.
The modal overlay and content use extensive inline styles (lines 51–72, 81, 86–87), making the component harder to maintain and theme consistently. Moving these to a dedicated CSS file would improve readability and reusability.
🤖 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/DropdownAdd.jsx` around lines 50 - 92, The modal JSX in DropdownAdd.jsx uses extensive inline styles for the overlay and content when isAdding is true (the fixed overlay div and inner white box surrounding addLabel, input bound to newValue, and buttons that call handleSave/handleCancel); extract these style blocks into a CSS (or CSS module) class set (e.g., .dropdown-add-overlay, .dropdown-add-modal, .dropdown-add-input, .dropdown-add-actions) and replace the inline style props with className references, moving paddings, positioning, z-index, colors, shadows, flex layout and spacing into the stylesheet to improve maintainability and theming.client/src/components/componentsAdmin/PopupAlert.css (2)
70-70: 💤 Low valueConsider using kebab-case for keyframe name.
The keyframe name
popInuses camelCase. CSS convention and the stylelint rule prefer kebab-case (pop-in).♻️ Optional refactor
-@keyframes popIn { +@keyframes pop-in { from { transform: translate(-50%, -50%) scale(0.8); opacity: 0; } to { transform: translate(-50%, -50%) scale(1); opacity: 1; } }And update the reference in
.popup-alert-overlay:.popup-alert-overlay { position: fixed; top: 15%; left: 50%; transform: translate(-50%, -50%); z-index: 50; - animation: popIn 0.4s cubic-bezier(0.16, 1, 0.3, 1); + animation: pop-in 0.4s cubic-bezier(0.16, 1, 0.3, 1); background-color: rgba(0, 0, 0, 0.4); }🤖 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/PopupAlert.css` at line 70, Rename the `@keyframes` identifier from popIn to kebab-case pop-in and update any references to it (e.g., the animation-name or animation shorthand used in the .popup-alert-overlay selector) so the animation continues to work and conforms to the stylelint convention.
27-39: ⚡ Quick winUse CSS variables for theme colors.
The success and error themes use hardcoded color values. For consistency with the theme system established in
index.css, consider referencing CSS variables instead.♻️ Proposed refactor to use CSS variables
/* ธีมสำหรับ "สำเร็จ" (Success) */ .popup-alert-box.success { - background-color: `#f0fdf4`; /* สีพื้นหลังเขียวอ่อน */ - border-left-color: `#22c55e`; /* สีเส้นขอบเขียว */ - color: `#166534`; /* สีตัวอักษรเขียวเข้ม */ + background-color: var(--tbs-green-light); + border-left-color: var(--tbs-green); + color: var(--text-color-main); } /* ธีมสำหรับ "ผิดพลาด" (Error) */ .popup-alert-box.error { - background-color: `#fef2f2`; /* สีพื้นหลังแดงอ่อน */ - border-left-color: `#ef4444`; /* สีเส้นขอบแดง */ - color: `#991b1b`; /* สีตัวอักษรแดงเข้ม */ + background-color: `#fef2f2`; + border-left-color: var(--button-red-bg); + color: `#991b1b`; }🤖 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/PopupAlert.css` around lines 27 - 39, Replace the hardcoded colors in .popup-alert-box.success and .popup-alert-box.error with the theme CSS variables used in index.css (e.g., --color-success-bg, --color-success-border, --color-success-text and --color-error-bg, --color-error-border, --color-error-text); update the properties background-color, border-left-color and color in those classes to reference the corresponding variables so the popup alert follows the global theme system.client/src/components/CardPendingProblem.css (1)
70-70: ⚡ Quick winConsider using CSS variables for skeleton colors.
The skeleton loading styles use hardcoded colors (
#e5e7eb,#d1d5db,#fef08a,#ffffff) while the rest of the file now uses CSS variables for theming. For consistency, consider defining skeleton-specific variables inindex.cssor reusing existing variables.♻️ Example refactor
Add to
index.css:--skeleton-bg: `#e5e7eb`; --skeleton-bg-dark: `#d1d5db`; --skeleton-status: `#fef08a`;Then update this file:
.skleton-card-pending { width: 100%; border-radius: 10px; padding: 1rem; - background-color: var(--card-bg, `#ffffff`); + background-color: var(--card-bg); box-shadow: var(--shadow-md); } .skleton-img { - background-color: `#e5e7eb`; + background-color: var(--skeleton-bg); width: 150px; height: 150px; border-radius: 8px; } .skleton-text { - background-color: `#e5e7eb`; + background-color: var(--skeleton-bg); border-radius: 4px; height: 16px; margin-bottom: 10px; width: 80%; }Also applies to: 81-82, 88-88, 99-99, 103-103
🤖 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/CardPendingProblem.css` at line 70, Replace hardcoded skeleton colors in CardPendingProblem.css with CSS variables (e.g., use --skeleton-bg, --skeleton-bg-dark, --skeleton-status and fallback values) instead of literal hex values like `#e5e7eb`, `#d1d5db`, `#fef08a`, `#ffffff`; add those variables to your global stylesheet (index.css) so the skeleton selectors in CardPendingProblem.css reference the new variables while retaining sensible fallbacks, and update any occurrences that match the diff comment (including usages alongside --card-bg).client/src/pages/adminPage/Categories.css (1)
1-12: ⚖️ Poor tradeoffGeneric class names risk cross-page style collisions.
.main-container,.manage-container,.header-container,.layout-table, and.btn*are also defined inLocationManagement.css(and used inAssetManagement.jsx). Since these stylesheets are globally scoped, whichever loads last wins and styles can bleed between admin pages (e.g.,.manage-containerwidth/.layout-tablediffer per page). Consider page-scoped prefixes or CSS Modules.🤖 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/Categories.css` around lines 1 - 12, The CSS uses generic global class names (.main-container, .manage-container, .header-container, .layout-table, .btn*) that collide with other pages; fix by scoping these styles—either rename each selector in Categories.css to a page-prefixed variant (e.g., .categories-main-container, .categories-manage-container, .categories-header-container, .categories-layout-table, .categories-btn-*) and update the corresponding JSX/HTML to match, or convert Categories.css to a CSS Module (e.g., Categories.module.css) and update the component to import and reference the module class names (e.g., styles.manageContainer) so styles are locally scoped and won't bleed into LocationManagement/AssetManagement.server/controllers/managementControllers.js (1)
201-225: 💤 Low valueRemove debug
console.logstatements.Lines 202 and 205 log request params/roomId on every delete. Other handlers in this file don't do this; drop them before merge.
♻️ Cleanup
export const deleteRoom = async (req, res) => { - console.log("พารามิเตอร์ที่ส่งมาคือ:", req.params); try { const { id } = req.params; - console.log(" roomId ที่ส่งมาคือ:", id); if (!id) {🤖 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/managementControllers.js` around lines 201 - 225, In the deleteRoom controller, remove the debug console.log statements that print request parameters and the roomId (the two console.log calls inside the deleteRoom function) so the handler no longer logs sensitive request data on every call; keep the rest of the logic (param check, prisma.room.delete, and error handling) unchanged.server/routes/managementRoutes.js (1)
15-15: ⚡ Quick winConsider adding multer limits for the in-memory upload.
multer.memoryStorage()buffers the entire file in RAM with nolimits. Without afileSizecap (and ideally a MIME/extension filter), a large or malicious upload to/uploadEquipmentscan exhaust server memory.🛡️ Suggested hardening
-const upload = multer({ storage: multer.memoryStorage() }); +const upload = multer({ + storage: multer.memoryStorage(), + limits: { fileSize: 5 * 1024 * 1024 }, // 5 MB +});🤖 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/routes/managementRoutes.js` at line 15, The multer setup currently uses multer.memoryStorage() with no limits; update the upload configuration (the const upload = multer(...) declaration) to add sensible limits (e.g., fileSize) and a fileFilter that validates MIME types/extensions for the /uploadEquipments route, so large or malicious files are rejected before they hit RAM; keep memoryStorage but set limits.fileSize and implement fileFilter to check allowed mime types (and optionally fieldSize/parts) in the multer options for upload.client/src/components/componentsAdmin/ImportEquipments.css (1)
129-139: 💤 Low value
fadeInkeyframe is defined but never referenced. Noanimationproperty uses it, so it's dead CSS. Stylelint also flags the name as non-kebab-case (Line 129). Either wire it up to the.file-display-badgereveal or remove it.♻️ Option: apply the animation
.file-display-badge { position: relative; display: inline-flex; align-items: center; background-color: `#f1f5f9`; padding: 6px 14px; padding-right: 35px; border-radius: 20px; border: 1px solid `#e2e8f0`; + animation: fade-in 0.2s ease; }And rename the keyframe to
fade-in.🤖 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/ImportEquipments.css` around lines 129 - 139, The `@keyframes` rule named fadeIn is unused and flagged for non-kebab-case; either remove it or rename and wire it up to the reveal element — rename the keyframe to fade-in and add an animation declaration on .file-display-badge (e.g., animation: fade-in 150ms ease-out both) so the badge uses the animation when shown; update any references to the keyframe name accordingly to avoid the Stylelint error.server/prisma/schema.prisma (1)
44-44: 💤 Low valueMissing space after comma in relation attributes.
references: [...],onDelete: Cascadeis missing a space (Lines 44, 100, 140, 143). It still parses, butprisma formatwill rewrite these; add the space for consistency.Also applies to: 100-100, 140-140, 143-143
🤖 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/schema.prisma` at line 44, Fix the missing space after the comma in relation attributes so Prisma formatting is stable: locate the relation declarations such as the one using "location Location `@relation`(fields: [locationId], references: [locationId],onDelete: Cascade)" and similar relation lines elsewhere and insert a space after the comma so they read "references: [locationId], onDelete: Cascade" (apply the same fix for the other occurrences with references/onDelete).client/src/pages/adminPage/UserManagement.css (1)
38-54: ⚡ Quick winOverly generic global class names risk cross-page collisions.
.form-selectedis also defined inAssetManagement.css, and.btnis a very common selector. Since these stylesheets are global, definitions can leak/override each other across pages. Consider scoping under.manage-user-containeror prefixing (e.g..user-form-selected,.user-btns).🤖 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/UserManagement.css` around lines 38 - 54, The CSS uses overly-generic global selectors `.form-selected` and `.btn` which collide with other pages (e.g. `AssetManagement.css`); update the rules in UserManagement.css to scope or prefix them (e.g. rename `.form-selected` -> `.user-form-selected` and `.btn` -> `.user-btns` or nest them under a parent `.manage-user-container`) and then update any corresponding JSX/HTML to use those new class names so styles no longer leak across pages.client/src/pages/adminPage/UserManagement.jsx (1)
98-101: ⚡ Quick winSave/Delete buttons have no handlers — edits can't be persisted.
Neither button has an
onClick, so the management form is read-only in practice. If user CRUD is in scope for this page, the persistence logic still needs to be wired.Want me to scaffold the save/delete handlers (service calls +
useUsersrefetch) for this page?🤖 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/UserManagement.jsx` around lines 98 - 101, In UserManagement.jsx the save and delete buttons (.btn-confirm and .btn-cancel) lack onClick handlers so edits aren't persisted; add two handlers (e.g., handleSave and handleDelete) that call the appropriate service methods (e.g., usersService.updateUser(userId, payload) and usersService.deleteUser(userId)) and then trigger the useUsers hook refetch() to refresh state; wire these handlers to the buttons' onClick, handle loading/disabled state during the async calls, and surface success/error feedback (and ensure you reference the component state/props that hold the current userId and edited fields).client/src/pages/adminPage/LocationManagement.jsx (1)
159-163: 💤 Low valueRedundant feedback:
triggerAlertandalert()both fire.Success paths use
triggerAlert(PopupAlert) but the catch blocks fire bothtriggerAlertand a nativealert(), producing two error popups. Pick one mechanism for consistency. Several debugconsole.logs (Lines 118, 130, 150, 181, 233–235) are also left in and should be removed before release.Also applies to: 190-194
🤖 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/LocationManagement.jsx` around lines 159 - 163, The catch blocks in LocationManagement.jsx currently call both triggerAlert and a native alert (e.g., the catch after the add-floor API call), causing duplicate error popups; remove the native alert() calls and rely on triggerAlert('error', ...) for user feedback (update the catch blocks that reference alert(`เกิดข้อผิดพลาด: ...`) to only call triggerAlert). Also remove leftover debug console.log statements referenced around lines 118, 130, 150, 181, and 233–235 to clean up the file before release. Ensure you update both occurrences noted (including the block at 190–194) so all error handling is consistent and non-duplicative.client/src/hooks/useCategories.js (1)
10-15: 💤 Low valueRemove debug log and correct the error label.
console.logon line 11 is a debug artifact, and the catch label on line 14 says "fetching tickets" instead of categories.♻️ Proposed cleanup
const response = await axios.get('/api/manage/getTicketCategories'); - console.log("ข้อมูลจาก Backend:", response.data); setCategories(response.data); } catch (error) { - console.error('Error fetching tickets:', error); + console.error('Error fetching categories:', error); }🤖 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/hooks/useCategories.js` around lines 10 - 15, Remove the debug console.log and correct the catch message: in useCategories.js, inside the async fetch that calls axios.get('/api/manage/getTicketCategories') and calls setCategories(response.data), delete the console.log("ข้อมูลจาก Backend:", response.data) and change console.error('Error fetching tickets:', error) to a more accurate label like console.error('Error fetching categories:', error) so the log reflects the actual operation.
🤖 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/DropdownAdd.jsx`:
- Around line 50-92: The modal opened when isAdding is true lacks an Escape key
handler; add a useEffect in the DropdownAdd component that attaches a window
'keydown' listener when isAdding is true, checks for event.key === 'Escape' (or
event.keyCode === 27 for broader support) and calls the existing handleCancel to
close the modal, and ensure the listener is removed in the cleanup or when
isAdding becomes false to avoid leaks; reference symbols: isAdding,
handleCancel, handleSave, setNewValue, newValue, addLabel.
- Around line 17-22: The handler currently checks e.target.value ===
'CREATE_NEW' which can collide with real option ids; replace the literal
sentinel with a unique sentinel (e.g., a JS Symbol or a clearly namespaced const
like '__CREATE_NEW__') and update DropdownAdd.jsx so the special "create new"
option uses that sentinel as its value; then change the comparison in the event
handler (the branch that calls setIsAdding(true) vs onChange(e)) to test against
the new sentinel so real options from the options array cannot be misdetected.
In `@client/src/hooks/useUsers.js`:
- Around line 9-11: Remove the debug console logging of user data to avoid
exposing PII: in useUsers.js where you call axios.get('/api/manage/getUsers'),
delete or comment out the console.log("ข้อมูลจาก Backend:", response.data) line
and keep setting state via setUsers(response.data); ensure no other logs print
response.data or individual user fields (emails) anywhere in the fetch path.
In `@client/src/index.css`:
- Line 24: Remove the duplicated CSS custom property --tbs-green-active by
deleting the earlier declaration (the one currently at the top of the file) and
keep the single, logically grouped definition in the status color section (the
later declaration currently set to `#dcfce3`); ensure only one --tbs-green-active
exists to avoid dead code and unintended overrides.
- Line 73: Duplicate CSS custom property --text-color-main is defined twice;
remove the unintended duplicate and leave a single consistent definition (keep
the intended value, e.g., `#333`) or rename the green variant to a distinct name
such as --text-color-success and update any usages that expect the green color;
ensure only one --text-color-main remains and refactor CSS references
accordingly.
- Line 83: The CSS variable --card-hover is set to pure red (`#ff0000`) which
likely is a debug placeholder and conflicts with the green theme; confirm intent
and if it's meant for card hover use replace the value with the project's green
palette variable (e.g., use the appropriate --green-... token or a subtle green
tint) and update any references to --card-hover accordingly so hover states
match the theme.
In `@client/src/pages/AddIssue.jsx`:
- Line 243: The form's native onSubmit("add-issue") calls handleSubmit directly
when Enter is pressed, bypassing the confirmation modal; change the form submit
handler to preventDefault and open the confirmation modal instead, then only
invoke handleSubmit when the user confirms in the modal (move submission logic
out of the direct onSubmit path). Specifically: update the form's onSubmit to
call a new or existing showConfirmation/openConfirmModal function that does
event.preventDefault(), and wire the modal's confirm button to call handleSubmit
(or a wrapper submitConfirmed) so keyboard submission cannot bypass the modal.
In `@client/src/pages/adminPage/AssetManagement.jsx`:
- Around line 36-47: The handler handleEditClick reads selectedItem.room.roomId
directly which will throw if room is null; update the setFormData call to guard
against missing room by using optional chaining for roomId (e.g.,
selectedItem.room?.roomId) and keep the existing optional chaining for
floorId/locationId, providing sensible defaults (empty string) when values are
absent; ensure setSelectedId(selectedItem.equipmentId) remains and preserve the
console.log if desired.
- Around line 62-72: The handleUpdate handler never calls the update action and
also leaves the UI permanently disabled because isSubmitting is set true and
never reset; fix by importing/destructuring updateEquipment from useEquipment
(or the correct hook) and invoking it with the mapped payload for selectedId
inside handleUpdate, await the call, handle errors with try/catch, and always
call setIsSubmitting(false) and setIsUpdateConfirmOpen(false) in finally so
buttons (ConfirmButton disabled={isSubmitting}) are re-enabled; ensure you
reference selectedId to build the payload and use setIsSubmitting(true) only
just before awaiting updateEquipment and reset it in finally.
In `@client/src/pages/adminPage/Categories.jsx`:
- Line 11: Fix two issues: (1) Update the import of the popup component to match
the actual file casing by importing PopupAlert (capital P) where PopupAlert.jsx
is exported (replace imports of '../../components/componentsAdmin/popupAlert'
with '../../components/componentsAdmin/PopupAlert' in Categories.jsx and
LocationManagement.jsx); (2) Align the category form state keys so the inputs,
validation and save logic use the same names: change the initial formData in
Categories.jsx to use ticketCtgName and ticketCtgStatus (or update the
save/validation code to use ticketCtgIdName/ticketCtgIdStatus) so the field
names referenced by your validation/save handlers match the keys stored in
formData.
In `@client/src/pages/Dashboard.jsx`:
- Line 7: Update the import in Dashboard.jsx to match the exact filename case
for the exported components: change the module specifier that currently
references 'CardpendingProblem.jsx' to the correct 'CardPendingProblem.jsx' so
the imported symbols CardPendingProblem and CardPendingSkeleton resolve on
case-sensitive filesystems; ensure any other imports referencing this file use
the same exact capitalization.
In `@server/controllers/EquipmentControllers.js`:
- Around line 105-122: The bug is a typo: the variable declared as filename is
later referenced as fileName causing CSV uploads to crash; update the CSV branch
to use the same identifier (filename) everywhere (or consistently rename both to
fileName) so the conditional if (filename.endsWith('.xlsx')) / else if
(filename.endsWith('.csv')) and any other uses in EquipmentControllers.js
reference the same symbol, ensuring CSV parsing executes instead of throwing a
ReferenceError.
- Around line 87-96: Move the prisma.equipment.findMany call and the
construction of existingEquipments/existingSet inside the try block so any
rejection is caught by the handler; also perform the early guard check for
!req.file (the file upload check) before running the DB query to avoid querying
when no file was uploaded. Update the code around prisma.equipment.findMany,
existingEquipments, existingSet and seenInFile so you declare seenInFile once
and remove duplicate declarations, ensuring the DB query and set creation happen
after the !req.file guard and within the try/catch that wraps the upload
processing.
In `@server/controllers/managementControllers.js`:
- Around line 354-362: The getUsers controller currently returns all User
columns via prisma.user.findMany(), exposing unnecessary PII; update the call in
getUsers to explicitly select only the fields the UI requires (use
prisma.user.findMany({ select: { ... } }) instead of returning all fields) and
exclude sensitive fields like email, fullName, googleId, avatarUrl or any other
PII unless the UI needs them—keep only minimal identifiers and role/metadata
(e.g., id, username, role, createdAt) as required by the frontend and adjust the
JSON response accordingly.
In `@server/prisma/schema.prisma`:
- Around line 140-143: Current schema uses onDelete: Cascade on Ticket.location,
Ticket.room, TicketImage.ticket, Floor.location, Room.floor and Equipment.room
which will hard-delete historical tickets and images; change foreign key delete
behavior to either Restrict or SetNull for Ticket relations (e.g., modify
Ticket.location and Ticket.room relations and TicketImage.ticket) and update
fields to be nullable when using SetNull (or enforce Restrict to prevent
deletion), and confirm admin decommission semantics or implement soft-delete
flags (e.g., Location.status/Room.status) instead of cascades; also fix
formatting by adding a space before each onDelete: occurrences (Floor.location,
TicketImage.ticket, Ticket.location, Ticket.room).
---
Outside diff comments:
In `@client/src/pages/AddIssue.jsx`:
- Around line 175-222: The submit flow leaves isSubmitting true on errors;
update the submit handler (where isSubmitting and setIsSubmitting are used) to
always reset isSubmitting by adding a finally block (or calling
setIsSubmitting(false) in both success and catch paths) after the try/catch so
retries are possible; reference the existing isSubmitting, setIsSubmitting, and
the try/catch around the axios.post call to locate where to add the finally
reset.
In `@client/src/pages/Dashboard.css`:
- Line 31: The CSS uses an invalid custom property name in Dashboard.css where
the declaration color: var(---text-color-main); has three leading hyphens;
update the variable reference to use the correct two-dash custom property syntax
by changing var(---text-color-main) to var(--text-color-main) so the
--text-color-main custom property resolves correctly.
---
Minor comments:
In `@client/src/components/CardPendingProblem.css`:
- Around line 1-2: Remove the no-op CSS rule in CardPendingProblem.css by
deleting the empty selector block "p, span {}" so the file contains only
meaningful styles; locate the empty rule (the "p, span" selector) and remove
those lines to clean up the stylesheet.
In `@client/src/components/CardPendingProblem.jsx`:
- Line 51: In CardPendingProblem.jsx, guard against undefined values when
rendering data.ticketId and data.admin by using optional chaining and a fallback
(e.g., data?.ticketId || '—' and data?.admin || 'Unknown') in the JSX where
ticketId and admin are displayed; update the render expressions that reference
data.ticketId and data.admin so they won't render the literal "undefined" when
the API omits those fields.
In `@client/src/components/componentsAdmin/ImportEquipments.jsx`:
- Around line 41-68: The selected file confirm button is unmounted while
uploading so there’s no visible loading state; keep the confirm upload button
(confirm-upload-file) mounted and show a disabled/loading state by rendering it
whenever file is present and binding its disabled prop to isSubmitting, change
its label to 'กำลังส่ง...' (or show a spinner) when isSubmitting, and ensure the
original import button (btn-import-custom) is also disabled while isSubmitting;
update handlers uploadFile, handleButtonClick and the buttons' disabled logic to
rely on isSubmitting so the UI shows a clear in-progress affordance.
In `@client/src/components/componentsAdmin/PopupAlert.css`:
- Around line 3-8: Update the misleading Thai comment for the
.popup-alert-overlay CSS: replace the phrase stating it appears in the "upper
right corner" (มุมขวาบน) with a comment that accurately describes the
positioning as top-center (e.g., "อยู่ตรงกลางด้านบน" or equivalent), since left:
50% with transform: translate(-50%, -50%) centers the element horizontally at
the top. Ensure the comment still mentions the animation on entry if relevant.
In `@client/src/components/componentsAdmin/PopupAlert.jsx`:
- Around line 5-12: The auto-close useEffect in PopupAlert resets whenever
onClose reference changes; fix by stabilizing the callback via a ref: create a
ref (e.g., onCloseRef) and update it in a small effect when onClose changes
(onCloseRef.current = onClose), then change the auto-close effect to depend only
on isOpen and use onCloseRef.current() inside the timer callback, keeping the
existing clearTimeout cleanup so the timer isn't restarted by parent re-renders.
In `@client/src/hooks/useImportEquipments.js`:
- Around line 27-31: The success reload is happening before user feedback so
alert(response.data.message), setFile(null), and return true may not execute;
update the useImportEquipments hook to either (A) move window.location.reload()
to after the alert and setFile calls so axios.post(...), then
alert(response.data.message), setFile(null), and finally
window.location.reload(), or (B) remove window.location.reload() entirely and
instead update local state or trigger a refetch (e.g., call the parent refresh
handler) after receiving response from axios.post to reflect the new data
without forcing a full page reload; ensure you reference the axios.post call,
response.data.message, and setFile when making the change.
In `@client/src/pages/AddIssue.jsx`:
- Line 57: Remove the debug console.log(equipRes) statement from the
AddIssue.jsx component (where equipRes is logged after the equipment response) —
either delete the console.log call or replace it with a proper error/operation
handler (e.g., update component state or call a logger function) so no
debug-only console output remains in production code; locate the console.log in
the AddIssue component's function that handles the equipment response and remove
or refactor it accordingly.
In `@client/src/pages/adminPage/Categories.jsx`:
- Around line 17-21: The initial state for formData uses incorrect keys
(ticketCtgIdName, ticketCtgIdStatus) causing undefined values elsewhere; update
the useState initializer in Categories.jsx so formData contains the correct keys
(ticketCtgId, ticketCtgName, ticketCtgStatus) to match how handleEditClick,
handleSaveCategory, the input components and setFormData expect and use those
properties.
In `@client/src/pages/adminPage/LocationManagement.css`:
- Around line 122-124: Stylelint reports declaration-empty-line-before
violations in LocationManagement.css; remove the blank line immediately before
the declarations (e.g., the lines declaring "justify-content: space-between;"
and "transition: all 0.2s ease-in-out;") so there is no empty line preceding
each property, ensuring the CSS rule block has properties listed consecutively
with no blank lines (this fixes the declaration-empty-line-before errors).
In `@client/src/pages/adminPage/LocationManagement.jsx`:
- Line 179: The confirmation prompt in handleAddNewLocation uses the
floor-specific text ("ยืนยันการเพิ่มชั้นนี้?") but this function adds a
location; update the window.confirm call inside handleAddNewLocation to use a
location-appropriate message (e.g., "ยืนยันการเพิ่มสถานที่นี้?") so the
confirmation accurately reflects the action.
In `@client/src/pages/Dashboard.css`:
- Around line 54-56: The .filter-container rule is empty and should be removed
or given its intended styles; locate the .filter-container selector in
Dashboard.css and either delete the empty block to clean up the stylesheet or
populate it with the required declarations (e.g., layout, spacing, or visibility
properties) so it serves a purpose.
---
Duplicate comments:
In `@client/src/pages/adminPage/LocationManagement.jsx`:
- Line 9: The import in LocationManagement.jsx uses incorrect casing and will
fail on case-sensitive filesystems; update the import statement that currently
references '../../components/componentsAdmin/popupAlert' to match the actual
filename 'PopupAlert.jsx' (i.e., import PopupAlert from
'../../components/componentsAdmin/PopupAlert'), mirroring the same fix applied
in Categories.jsx so the PopupAlert symbol resolves correctly.
---
Nitpick comments:
In `@client/src/components/CardPendingProblem.css`:
- Line 70: Replace hardcoded skeleton colors in CardPendingProblem.css with CSS
variables (e.g., use --skeleton-bg, --skeleton-bg-dark, --skeleton-status and
fallback values) instead of literal hex values like `#e5e7eb`, `#d1d5db`, `#fef08a`,
`#ffffff`; add those variables to your global stylesheet (index.css) so the
skeleton selectors in CardPendingProblem.css reference the new variables while
retaining sensible fallbacks, and update any occurrences that match the diff
comment (including usages alongside --card-bg).
In `@client/src/components/componentsAdmin/DropdownAdd.jsx`:
- Around line 50-92: The modal JSX in DropdownAdd.jsx uses extensive inline
styles for the overlay and content when isAdding is true (the fixed overlay div
and inner white box surrounding addLabel, input bound to newValue, and buttons
that call handleSave/handleCancel); extract these style blocks into a CSS (or
CSS module) class set (e.g., .dropdown-add-overlay, .dropdown-add-modal,
.dropdown-add-input, .dropdown-add-actions) and replace the inline style props
with className references, moving paddings, positioning, z-index, colors,
shadows, flex layout and spacing into the stylesheet to improve maintainability
and theming.
In `@client/src/components/componentsAdmin/ImportEquipments.css`:
- Around line 129-139: The `@keyframes` rule named fadeIn is unused and flagged
for non-kebab-case; either remove it or rename and wire it up to the reveal
element — rename the keyframe to fade-in and add an animation declaration on
.file-display-badge (e.g., animation: fade-in 150ms ease-out both) so the badge
uses the animation when shown; update any references to the keyframe name
accordingly to avoid the Stylelint error.
In `@client/src/components/componentsAdmin/PopupAlert.css`:
- Line 70: Rename the `@keyframes` identifier from popIn to kebab-case pop-in and
update any references to it (e.g., the animation-name or animation shorthand
used in the .popup-alert-overlay selector) so the animation continues to work
and conforms to the stylelint convention.
- Around line 27-39: Replace the hardcoded colors in .popup-alert-box.success
and .popup-alert-box.error with the theme CSS variables used in index.css (e.g.,
--color-success-bg, --color-success-border, --color-success-text and
--color-error-bg, --color-error-border, --color-error-text); update the
properties background-color, border-left-color and color in those classes to
reference the corresponding variables so the popup alert follows the global
theme system.
In `@client/src/components/ConfirmButton.jsx`:
- Around line 11-14: The ConfirmButton component currently destructures unused
props type and form (in the props list in ConfirmButton.jsx); remove type and
form from the component's parameter destructuring and drop any corresponding
defaultProps/propTypes or JSDoc entries for type and form (if present) so the
component API no longer advertises unused props; scan for and remove any dead
references to type/form inside ConfirmButton-related code to keep the surface
area minimal.
In `@client/src/hooks/useCategories.js`:
- Around line 10-15: Remove the debug console.log and correct the catch message:
in useCategories.js, inside the async fetch that calls
axios.get('/api/manage/getTicketCategories') and calls
setCategories(response.data), delete the console.log("ข้อมูลจาก Backend:",
response.data) and change console.error('Error fetching tickets:', error) to a
more accurate label like console.error('Error fetching categories:', error) so
the log reflects the actual operation.
In `@client/src/pages/adminPage/Categories.css`:
- Around line 1-12: The CSS uses generic global class names (.main-container,
.manage-container, .header-container, .layout-table, .btn*) that collide with
other pages; fix by scoping these styles—either rename each selector in
Categories.css to a page-prefixed variant (e.g., .categories-main-container,
.categories-manage-container, .categories-header-container,
.categories-layout-table, .categories-btn-*) and update the corresponding
JSX/HTML to match, or convert Categories.css to a CSS Module (e.g.,
Categories.module.css) and update the component to import and reference the
module class names (e.g., styles.manageContainer) so styles are locally scoped
and won't bleed into LocationManagement/AssetManagement.
In `@client/src/pages/adminPage/LocationManagement.jsx`:
- Around line 159-163: The catch blocks in LocationManagement.jsx currently call
both triggerAlert and a native alert (e.g., the catch after the add-floor API
call), causing duplicate error popups; remove the native alert() calls and rely
on triggerAlert('error', ...) for user feedback (update the catch blocks that
reference alert(`เกิดข้อผิดพลาด: ...`) to only call triggerAlert). Also remove
leftover debug console.log statements referenced around lines 118, 130, 150,
181, and 233–235 to clean up the file before release. Ensure you update both
occurrences noted (including the block at 190–194) so all error handling is
consistent and non-duplicative.
In `@client/src/pages/adminPage/UserManagement.css`:
- Around line 38-54: The CSS uses overly-generic global selectors
`.form-selected` and `.btn` which collide with other pages (e.g.
`AssetManagement.css`); update the rules in UserManagement.css to scope or
prefix them (e.g. rename `.form-selected` -> `.user-form-selected` and `.btn` ->
`.user-btns` or nest them under a parent `.manage-user-container`) and then
update any corresponding JSX/HTML to use those new class names so styles no
longer leak across pages.
In `@client/src/pages/adminPage/UserManagement.jsx`:
- Around line 98-101: In UserManagement.jsx the save and delete buttons
(.btn-confirm and .btn-cancel) lack onClick handlers so edits aren't persisted;
add two handlers (e.g., handleSave and handleDelete) that call the appropriate
service methods (e.g., usersService.updateUser(userId, payload) and
usersService.deleteUser(userId)) and then trigger the useUsers hook refetch() to
refresh state; wire these handlers to the buttons' onClick, handle
loading/disabled state during the async calls, and surface success/error
feedback (and ensure you reference the component state/props that hold the
current userId and edited fields).
In `@server/controllers/managementControllers.js`:
- Around line 201-225: In the deleteRoom controller, remove the debug
console.log statements that print request parameters and the roomId (the two
console.log calls inside the deleteRoom function) so the handler no longer logs
sensitive request data on every call; keep the rest of the logic (param check,
prisma.room.delete, and error handling) unchanged.
In `@server/prisma/schema.prisma`:
- Line 44: Fix the missing space after the comma in relation attributes so
Prisma formatting is stable: locate the relation declarations such as the one
using "location Location `@relation`(fields: [locationId], references:
[locationId],onDelete: Cascade)" and similar relation lines elsewhere and insert
a space after the comma so they read "references: [locationId], onDelete:
Cascade" (apply the same fix for the other occurrences with
references/onDelete).
In `@server/routes/managementRoutes.js`:
- Line 15: The multer setup currently uses multer.memoryStorage() with no
limits; update the upload configuration (the const upload = multer(...)
declaration) to add sensible limits (e.g., fileSize) and a fileFilter that
validates MIME types/extensions for the /uploadEquipments route, so large or
malicious files are rejected before they hit RAM; keep memoryStorage but set
limits.fileSize and implement fileFilter to check allowed mime types (and
optionally fieldSize/parts) in the multer options for upload.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| if (e.target.value === 'CREATE_NEW') { | ||
| setIsAdding(true); | ||
| } else { | ||
| onChange(e); // ส่งค่ากลับไปให้ฟอร์มหลัก | ||
| } | ||
| }; |
There was a problem hiding this comment.
Risk of ID collision with sentinel value 'CREATE_NEW'.
The component uses the string literal 'CREATE_NEW' as a sentinel to trigger the modal. If any actual option in the options array has id: 'CREATE_NEW', the user will be unable to select that option—it will always open the modal instead.
🛡️ Proposed fix using Symbol
+const CREATE_NEW = Symbol('CREATE_NEW');
+
export const DropdownWithAdd = ({
name,
value,
options,
onChange,
onSaveNew,
placeholder,
addLabel
}) => {
const [isAdding, setIsAdding] = useState(false);
const [newValue, setNewValue] = useState('');
const handleSelectChange = (e) => {
- if (e.target.value === 'CREATE_NEW') {
+ if (e.target.value === '__CREATE_NEW__') {
setIsAdding(true);
} else {
onChange(e);
}
};
// ... rest of component
- <option value="CREATE_NEW">➕ {addLabel}</option>
+ <option value="__CREATE_NEW__">➕ {addLabel}</option>Alternative: Use a prefixed constant like '__CREATE_NEW__' that is unlikely to collide with real IDs.
🤖 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/DropdownAdd.jsx` around lines 17 - 22,
The handler currently checks e.target.value === 'CREATE_NEW' which can collide
with real option ids; replace the literal sentinel with a unique sentinel (e.g.,
a JS Symbol or a clearly namespaced const like '__CREATE_NEW__') and update
DropdownAdd.jsx so the special "create new" option uses that sentinel as its
value; then change the comparison in the event handler (the branch that calls
setIsAdding(true) vs onChange(e)) to test against the new sentinel so real
options from the options array cannot be misdetected.
| {isAdding && ( | ||
| <div style={{ | ||
| position: 'fixed', // ยึดติดกับหน้าจอ | ||
| top: 0, | ||
| left: 0, | ||
| width: '100vw', | ||
| height: '100vh', | ||
| backgroundColor: 'rgba(0, 0, 0, 0.5)', // พื้นหลังสีดำโปร่งแสง (จางๆ) | ||
| display: 'flex', | ||
| justifyContent: 'center', // จัดให้อยู่ตรงกลางแนวนอน | ||
| alignItems: 'center', // จัดให้อยู่ตรงกลางแนวตั้ง | ||
| zIndex: 1000 // ดันให้อยู่ชั้นบนสุด ไม่ให้โดนอันอื่นบัง | ||
| }}> | ||
|
|
||
| {/* กล่องสีขาวที่อยู่ตรงกลาง */} | ||
| <div style={{ | ||
| backgroundColor: '#fff', | ||
| padding: '25px', | ||
| borderRadius: '10px', | ||
| width: '350px', | ||
| boxShadow: '0 4px 15px rgba(0,0,0,0.3)', | ||
| textAlign: 'center' | ||
| }}> | ||
| <h4 style={{ marginTop: 0, marginBottom: '20px' }}>{addLabel}</h4> | ||
|
|
||
| <input | ||
| type="text" | ||
| value={newValue} | ||
| onChange={(e) => setNewValue(e.target.value)} | ||
| placeholder={`พิมพ์${addLabel}...`} | ||
| className="form-control" | ||
| style={{ width: '100%', marginBottom: '20px', padding: '10px' }} | ||
| autoFocus | ||
| /> | ||
|
|
||
| <div style={{ display: 'flex', justifyContent: 'center', gap: '10px' }}> | ||
| <button type="button" onClick={handleSave} className="btn-confirm" style={{ padding: '8px 20px' }}>บันทึก</button> | ||
| <button type="button" onClick={handleCancel} className="btn-cancel" style={{ padding: '8px 20px' }}>ยกเลิก</button> | ||
| </div> | ||
| </div> | ||
|
|
||
| </div> | ||
| )} |
There was a problem hiding this comment.
Add keyboard escape handler to close modal.
The modal does not respond to the Escape key, forcing users to click "ยกเลิก" to cancel. This degrades keyboard accessibility.
⌨️ Proposed fix
+import { useMemo, useState, useEffect } from 'react'
export const DropdownWithAdd = ({
// ... props
}) => {
const [isAdding, setIsAdding] = useState(false);
const [newValue, setNewValue] = useState('');
+
+ useEffect(() => {
+ const handleEscape = (e) => {
+ if (e.key === 'Escape' && isAdding) {
+ handleCancel();
+ }
+ };
+
+ if (isAdding) {
+ document.addEventListener('keydown', handleEscape);
+ return () => document.removeEventListener('keydown', handleEscape);
+ }
+ }, [isAdding]);
const handleSelectChange = (e) => {🤖 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/DropdownAdd.jsx` around lines 50 - 92,
The modal opened when isAdding is true lacks an Escape key handler; add a
useEffect in the DropdownAdd component that attaches a window 'keydown' listener
when isAdding is true, checks for event.key === 'Escape' (or event.keyCode ===
27 for broader support) and calls the existing handleCancel to close the modal,
and ensure the listener is removed in the cleanup or when isAdding becomes false
to avoid leaks; reference symbols: isAdding, handleCancel, handleSave,
setNewValue, newValue, addLabel.
| const response = await axios.get('/api/manage/getUsers'); | ||
| console.log("ข้อมูลจาก Backend:", response.data); | ||
| setUsers(response.data); |
There was a problem hiding this comment.
Avoid logging user data (PII) to the console.
response.data here contains user records including emails. Logging this to the browser console leaks PII into client logs and is a privacy/compliance risk. Remove the debug log.
🛡️ Proposed fix
const response = await axios.get('/api/manage/getUsers');
- console.log("ข้อมูลจาก Backend:", response.data);
setUsers(response.data);📝 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 response = await axios.get('/api/manage/getUsers'); | |
| console.log("ข้อมูลจาก Backend:", response.data); | |
| setUsers(response.data); | |
| const response = await axios.get('/api/manage/getUsers'); | |
| setUsers(response.data); |
🤖 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/hooks/useUsers.js` around lines 9 - 11, Remove the debug console
logging of user data to avoid exposing PII: in useUsers.js where you call
axios.get('/api/manage/getUsers'), delete or comment out the
console.log("ข้อมูลจาก Backend:", response.data) line and keep setting state via
setUsers(response.data); ensure no other logs print response.data or individual
user fields (emails) anywhere in the fetch path.
| --tbs-green-dark: #27ae60; /* สีเขียวเข้ม (สำหรับ Text) */ | ||
| --tbs-green-accent: #27ae60; /* สีเขียวที่เน้นหนักขึ้น */ | ||
| --tbs-green-accent: #229b09; /* สีเขียวที่เน้นหนักขึ้น */ | ||
| --tbs-green-active: #d7ffd4; |
There was a problem hiding this comment.
Duplicate custom property --tbs-green-active.
The variable --tbs-green-active is defined twice: once on line 24 (#d7ffd4) and again on line 71 (#dcfce3). CSS will use the last declaration, making the first one dead code. Remove one of the duplicate definitions.
🐛 Proposed fix to remove duplicate
--tbs-green-dark: `#27ae60`;
--tbs-green-accent: `#229b09`;
- --tbs-green-active: `#d7ffd4`;
/* =========================================
2. สีของ Navbar (Navbar Color)Keep only the definition on line 71 within the status color section where it logically belongs.
Also applies to: 71-71
🤖 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` at line 24, Remove the duplicated CSS custom property
--tbs-green-active by deleting the earlier declaration (the one currently at the
top of the file) and keep the single, logically grouped definition in the status
color section (the later declaration currently set to `#dcfce3`); ensure only one
--tbs-green-active exists to avoid dead code and unintended overrides.
| --status-resolved-bg: #37A70B; | ||
| --tbs-green-active: #dcfce3; | ||
| --status-resolved-text:#ffffff; | ||
| --text-color-main: #166534; |
There was a problem hiding this comment.
Duplicate custom property --text-color-main.
The variable --text-color-main is defined twice: line 73 sets it to #166534 (dark green) and line 97 sets it to #333 (dark gray). The second definition overrides the first. Remove the duplicate and use a single consistent value, or rename one of them if they serve different purposes.
🐛 Proposed fix to resolve duplicate
--status-resolved-bg: `#37A70B`;
--tbs-green-active: `#dcfce3`;
--status-resolved-text:`#ffffff`;
- --text-color-main: `#166534`;
/* ยกเลิก/ด่วนมาก (Emergency/Cancel) */
--status-red: `#e74c3c`;Keep only the definition on line 97, or if the green text color is intentional for specific use cases, rename it to something like --text-color-success.
Also applies to: 97-97
🤖 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` at line 73, Duplicate CSS custom property
--text-color-main is defined twice; remove the unintended duplicate and leave a
single consistent definition (keep the intended value, e.g., `#333`) or rename the
green variant to a distinct name such as --text-color-success and update any
usages that expect the green color; ensure only one --text-color-main remains
and refactor CSS references accordingly.
| const existingEquipments = await prisma.equipment.findMany({ | ||
| select: { equipmentCode: true } | ||
| }); | ||
|
|
||
| const existingSet = new Set( | ||
| existingEquipments.map(e => e.equipmentCode?.trim()) | ||
| ); | ||
|
|
||
| const seenInFile = new Set(); | ||
| try { |
There was a problem hiding this comment.
Move the pre-upload Prisma query inside the try block.
existingEquipments/existingSet are computed before the try (Line 96). If findMany rejects, it becomes an unhandled rejection and escapes the handler's 500 fallback, potentially crashing the request. Also, the !req.file check on Line 97 runs after this DB query, so it queries even when no file was uploaded.
🛡️ Suggested restructure
- const existingEquipments = await prisma.equipment.findMany({
- select: { equipmentCode: true }
- });
-
- const existingSet = new Set(
- existingEquipments.map(e => e.equipmentCode?.trim())
- );
-
- const seenInFile = new Set();
- try {
- if (!req.file) {
- return res.status(400).json({ error: 'No file uploaded' });
- }
+ const seenInFile = new Set();
+ try {
+ if (!req.file) {
+ return res.status(400).json({ error: 'No file uploaded' });
+ }
+ const existingEquipments = await prisma.equipment.findMany({
+ select: { equipmentCode: true }
+ });
+ const existingSet = new Set(
+ existingEquipments.map(e => e.equipmentCode?.trim())
+ );(Then drop the duplicate declarations.)
📝 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 existingEquipments = await prisma.equipment.findMany({ | |
| select: { equipmentCode: true } | |
| }); | |
| const existingSet = new Set( | |
| existingEquipments.map(e => e.equipmentCode?.trim()) | |
| ); | |
| const seenInFile = new Set(); | |
| try { | |
| const seenInFile = new Set(); | |
| try { | |
| if (!req.file) { | |
| return res.status(400).json({ error: 'No file uploaded' }); | |
| } | |
| const existingEquipments = await prisma.equipment.findMany({ | |
| select: { equipmentCode: true } | |
| }); | |
| const existingSet = new Set( | |
| existingEquipments.map(e => e.equipmentCode?.trim()) | |
| ); |
🤖 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/EquipmentControllers.js` around lines 87 - 96, Move the
prisma.equipment.findMany call and the construction of
existingEquipments/existingSet inside the try block so any rejection is caught
by the handler; also perform the early guard check for !req.file (the file
upload check) before running the DB query to avoid querying when no file was
uploaded. Update the code around prisma.equipment.findMany, existingEquipments,
existingSet and seenInFile so you declare seenInFile once and remove duplicate
declarations, ensuring the DB query and set creation happen after the !req.file
guard and within the try/catch that wraps the upload processing.
| if (filename.endsWith('.xlsx')) { | ||
| const workbook = new ExcelJS.Workbook(); | ||
| await workbook.xlsx.load(req.file.buffer); | ||
| const worksheet = workbook.worksheets[0]; | ||
|
|
||
| worksheet.eachRow((row, rowNumber) => { | ||
| if (rowNumber === 1) return; | ||
| const rowData = { | ||
| rowNumber, | ||
| equipmentCode: row.getCell(1).value, | ||
| equipmentName: row.getCell(2).value, | ||
| equipmentCtgId: row.getCell(3).value, | ||
| roomId: row.getCell(4).value, | ||
| locationName: row.getCell(5).value, | ||
| }; | ||
| rawData.push(rowData); | ||
| }); | ||
| } else if (fileName.endsWith('.csv')) { |
There was a problem hiding this comment.
Critical: fileName is undefined — CSV uploads always fail.
The variable declared on Line 101 is filename (lowercase n), but Line 122 references fileName. This is a ReferenceError, so any .csv upload throws and returns HTTP 500 instead of being parsed.
🐛 Proposed fix
- } else if (fileName.endsWith('.csv')) {
+ } else if (filename.endsWith('.csv')) {📝 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.
| if (filename.endsWith('.xlsx')) { | |
| const workbook = new ExcelJS.Workbook(); | |
| await workbook.xlsx.load(req.file.buffer); | |
| const worksheet = workbook.worksheets[0]; | |
| worksheet.eachRow((row, rowNumber) => { | |
| if (rowNumber === 1) return; | |
| const rowData = { | |
| rowNumber, | |
| equipmentCode: row.getCell(1).value, | |
| equipmentName: row.getCell(2).value, | |
| equipmentCtgId: row.getCell(3).value, | |
| roomId: row.getCell(4).value, | |
| locationName: row.getCell(5).value, | |
| }; | |
| rawData.push(rowData); | |
| }); | |
| } else if (fileName.endsWith('.csv')) { | |
| if (filename.endsWith('.xlsx')) { | |
| const workbook = new ExcelJS.Workbook(); | |
| await workbook.xlsx.load(req.file.buffer); | |
| const worksheet = workbook.worksheets[0]; | |
| worksheet.eachRow((row, rowNumber) => { | |
| if (rowNumber === 1) return; | |
| const rowData = { | |
| rowNumber, | |
| equipmentCode: row.getCell(1).value, | |
| equipmentName: row.getCell(2).value, | |
| equipmentCtgId: row.getCell(3).value, | |
| roomId: row.getCell(4).value, | |
| locationName: row.getCell(5).value, | |
| }; | |
| rawData.push(rowData); | |
| }); | |
| } else if (filename.endsWith('.csv')) { |
🤖 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/EquipmentControllers.js` around lines 105 - 122, The bug
is a typo: the variable declared as filename is later referenced as fileName
causing CSV uploads to crash; update the CSV branch to use the same identifier
(filename) everywhere (or consistently rename both to fileName) so the
conditional if (filename.endsWith('.xlsx')) / else if
(filename.endsWith('.csv')) and any other uses in EquipmentControllers.js
reference the same symbol, ensuring CSV parsing executes instead of throwing a
ReferenceError.
| export const getUsers = async (req, res) => { | ||
| try { | ||
| const users = await prisma.user.findMany(); | ||
| res.status(200).json(users); | ||
| } catch (error) { | ||
| console.error('Error fetching users:', error); | ||
| res.status(500).json({ error: 'Failed to fetch users' }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm sensitive fields on the User model
fd -t f 'schema.prisma' --exec rg -nP -A25 '^model [Uu]ser\b' {}Repository: CNPJB/trouble-tracking-system-project
Length of output: 1292
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Controller snippet =="
sed -n '330,390p' server/controllers/managementControllers.js | nl -ba
echo
echo "== Search for password fields in Prisma schema =="
fd -t f 'schema.prisma' | head -n 1 | xargs -I{} bash -lc 'rg -n --hidden -S "password|passcode|hash|credential" {} -n || true'Repository: CNPJB/trouble-tracking-system-project
Length of output: 167
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Controller snippet (managementControllers.js around getUsers) =="
awk 'NR>=330 && NR<=390 {print NR ":" $0}' server/controllers/managementControllers.js
echo
echo "== Search for credential/password/hash-like fields in Prisma schema =="
SCHEMA="$(fd -t f 'schema.prisma' -a . | head -n 1 || true)"
if [ -z "${SCHEMA}" ]; then
echo "No schema.prisma found"
exit 0
fi
echo "Using schema: ${SCHEMA}"
rg -n --hidden -S 'password|passcode|credential|hash|hashed|jwt|api[_-]?key|secret' "${SCHEMA}" || trueRepository: CNPJB/trouble-tracking-system-project
Length of output: 1411
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find where getUsers is used =="
rg -n "getUsers" server || true
echo
echo "== Look for route wiring that mentions managementControllers/getUsers =="
rg -n "managementControllers|controllers/managementControllers|getUsers" server || trueRepository: CNPJB/trouble-tracking-system-project
Length of output: 770
🏁 Script executed:
#!/bin/bash
set -euo pipefail
awk 'NR>=1 && NR<=80 {print NR ":" $0}' server/routes/managementRoutes.jsRepository: CNPJB/trouble-tracking-system-project
Length of output: 2741
Restrict fields returned by getUsers to avoid unnecessary PII exposure (server/controllers/managementControllers.js L354-362)
getUsers returns prisma.user.findMany() with no projection. This endpoint is admin-only, but it still exposes all User columns (e.g., email, fullName, googleId, avatarUrl, role). Use select/omit to return only what the UI needs (no password-hash field exists in User, so the risk here is PII minimization rather than credential leakage).
🤖 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/managementControllers.js` around lines 354 - 362, The
getUsers controller currently returns all User columns via
prisma.user.findMany(), exposing unnecessary PII; update the call in getUsers to
explicitly select only the fields the UI requires (use prisma.user.findMany({
select: { ... } }) instead of returning all fields) and exclude sensitive fields
like email, fullName, googleId, avatarUrl or any other PII unless the UI needs
them—keep only minimal identifiers and role/metadata (e.g., id, username, role,
createdAt) as required by the frontend and adjust the JSON response accordingly.
| location Location @relation(fields: [locationId], references: [locationId],onDelete: Cascade) | ||
| parentTicket Ticket? @relation("SubTickets", fields: [parentTicketId], references: [ticketId]) | ||
| subTickets Ticket[] @relation("SubTickets") | ||
| room Room? @relation(fields: [roomId], references: [roomId]) | ||
| room Room? @relation(fields: [roomId], references: [roomId],onDelete: Cascade) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="server/prisma/schema.prisma"
echo "== File exists? =="
ls -la "$FILE"
echo "== onDelete occurrences (Cascade/Restrict/SetNull) =="
rg -n "onDelete\s*:" "$FILE" || true
echo "== Ticket model relations (near Ticket) =="
# show a reasonable window around the Ticket model
rg -n "model\s+Ticket\b" "$FILE" || true
python3 - <<'PY'
from pathlib import Path
import re
p=Path("server/prisma/schema.prisma")
lines=p.read_text().splitlines()
# find model Ticket line
for i,l in enumerate(lines):
if re.search(r"\bmodel\s+Ticket\b", l):
start=max(0,i-20); end=min(len(lines), i+80)
for j in range(start,end):
print(f"{j+1:4d}:{lines[j]}")
break
else:
print("model Ticket not found")
PY
echo "== Location/Room models + LocationStatus definition (near Location/Room/LocationStatus) =="
python3 - <<'PY'
from pathlib import Path
import re
p=Path("server/prisma/schema.prisma")
lines=p.read_text().splitlines()
targets=["model Location","model Room","enum LocationStatus","enum LocationStatus "]
def find_block(needle):
for i,l in enumerate(lines):
if needle in l:
start=max(0,i-20); end=min(len(lines), i+80)
print(f"\n--- Block around {needle} (line {i+1}) ---")
for j in range(start,end):
print(f"{j+1:4d}:{lines[j]}")
return
print(f"\n--- No block found for: {needle} ---")
for t in ["model Location","model Room","enum LocationStatus"]:
find_block(t)
PY
echo "== Equipment model (for Equipment.room onDelete) =="
rg -n "model\s+Equipment\b" "$FILE" || true
python3 - <<'PY'
from pathlib import Path
import re
p=Path("server/prisma/schema.prisma")
lines=p.read_text().splitlines()
for i,l in enumerate(lines):
if re.search(r"\bmodel\s+Equipment\b", l):
start=max(0,i-20); end=min(len(lines), i+90)
for j in range(start,end):
print(f"{j+1:4d}:{lines[j]}")
break
else:
print("model Equipment not found")
PY
echo "== Check spacing issue near the claimed lines (140-143, 44, 100) =="
python3 - <<'PY'
from pathlib import Path
p=Path("server/prisma/schema.prisma")
lines=p.read_text().splitlines()
for ln in [44,100,140,141,142,143]:
if 1<=ln<=len(lines):
print(f"{ln:4d}:{lines[ln-1]}")
PYRepository: CNPJB/trouble-tracking-system-project
Length of output: 23052
Major: onDelete: Cascade on Ticket/Room/Equipment can hard-delete historical data (and images).
server/prisma/schema.prisma configures cascade deletes that wipe tickets when a Location/Room is removed:
Ticket.location→LocationhasonDelete: Cascade(line 140).Ticket.room→RoomhasonDelete: Cascade(line 143).TicketImage.ticket→TickethasonDelete: Cascade(line 100), so ticket images are deleted too.- Deleting
Locationcan reach tickets viaFloorandRoom:Floor.location(line 44) cascades andRoom.floor(line 59) cascades. - Deleting a
Roomalso deletes equipment history viaEquipment.roomonDelete: Cascade(line 87).
Since TicketStatus includes resolved, this would remove closed/historical tickets (and their images) rather than preserving them. Consider Restrict/SetNull for ticket FKs (adjusting nullability as needed) or soft-delete via LocationStatus/room status instead of hard-deleting rows. Also confirm the intended deletion semantics for admin decommissioning.
Formatting nit: missing space before onDelete: at lines 44, 100, 140, 143.
🤖 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/schema.prisma` around lines 140 - 143, Current schema uses
onDelete: Cascade on Ticket.location, Ticket.room, TicketImage.ticket,
Floor.location, Room.floor and Equipment.room which will hard-delete historical
tickets and images; change foreign key delete behavior to either Restrict or
SetNull for Ticket relations (e.g., modify Ticket.location and Ticket.room
relations and TicketImage.ticket) and update fields to be nullable when using
SetNull (or enforce Restrict to prevent deletion), and confirm admin
decommission semantics or implement soft-delete flags (e.g.,
Location.status/Room.status) instead of cascades; also fix formatting by adding
a space before each onDelete: occurrences (Floor.location, TicketImage.ticket,
Ticket.location, Ticket.room).
addlocation addCategories
Summary by CodeRabbit
Release Notes
New Features
UI Improvements
Style