feat: implement CSV export utility, add debounce hook, and integrate … - #264
Merged
Conversation
…download functionality across report pages
Contributor
📝 WalkthroughWalkthroughThis PR implements form submission debouncing across the dashboard, introduces CSV export for multiple report pages, refactors the Provider Statement report to use live data with date filtering, adds file upload support to lead activities, converts lease recurring transactions to backend state management, and comprehensively rewrites the user-facing maintenance request form with improved validation and step-based navigation. ChangesMulti-Feature Enhancement: Debouncing, CSV Exports, Reports Refactor, and Form Improvements
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/pages/Dashboard/features/Leads/LeadDetail.tsx (2)
427-439:⚠️ Potential issue | 🟠 Major | ⚡ Quick winInclude
fileUrlwhen updating notes.Line 427 uploads the replacement file, but Line 438 still patches only
content. Edited notes can never change their attachment, and successful uploads become orphaned because nothing links them back to the note.🛠️ Suggested fix
if (editingItem && editingItem.item.originalData?.type === 'note') { // Update existing note await updateNoteMutation.mutateAsync({ leadId: id, noteId: editingItem.item.originalData.id, - noteData: { content: noteText } + noteData: { + content: noteText, + ...(fileUrl ? { fileUrl } : {}), + } }); } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/Leads/LeadDetail.tsx` around lines 427 - 439, The update branch currently uploads a replacement file to produce fileUrl but then calls updateNoteMutation.mutateAsync with only { content: noteText }, leaving uploads unlinked; change the update call in the editing branch (the block checking editingItem && editingItem.item.originalData?.type === 'note') to pass the full noteData (which already includes fileUrl when present) instead of the hard-coded { content: noteText }, so use noteData in the payload to update both content and attachment via updateNoteMutation.mutateAsync.
1176-1178:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't assume every uploaded file is an image.
The new feature uploads arbitrary files, but the timeline only renders
fileUrlthrough<img>. PDFs/docs will show as broken thumbnails and users have no way to open the attachment they just uploaded.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/Leads/LeadDetail.tsx` around lines 1176 - 1178, The timeline currently assumes every attachment is an image by using item.image in LeadDetail.tsx; change the rendering to detect whether the attachment is an image (e.g., check item.mimeType or file extension on item.fileUrl) and only render <img> for true image types, otherwise render a clickable link/button that opens or downloads the file (use the existing item.fileUrl), include target="_blank" and rel="noopener noreferrer" for external opens, and provide a fallback icon/filename so non-image uploads (PDFs/docs) are viewable from the timeline.src/pages/Dashboard/features/Leases/LeaseDetail.tsx (2)
1047-1049:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse the same enabled field you filtered on.
activeRecurringTransactionsonly keeps rows wherert.enabledis truthy, but the badge readstransaction.isEnabled. If the query shape is the one this file already assumes on Line 96, every row here will render as inactive.Suggested fix
-<span className={`px-2 py-0.5 rounded-full text-xs font-bold ${transaction.isEnabled ? 'bg-green-100 text-green-700' : 'bg-gray-100 text-gray-500'}`}> - {transaction.isEnabled ? 'Active' : 'Inactive'} +<span className={`px-2 py-0.5 rounded-full text-xs font-bold ${transaction.enabled ? 'bg-green-100 text-green-700' : 'bg-gray-100 text-gray-500'}`}> + {transaction.enabled ? 'Active' : 'Inactive'} </span>Also applies to: 94-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 `@src/pages/Dashboard/features/Leases/LeaseDetail.tsx` around lines 1047 - 1049, The badge uses transaction.isEnabled but the list was filtered from activeRecurringTransactions where the enabled flag is rt.enabled; update the rendering to use the same field the query/filter uses (e.g., replace transaction.isEnabled with transaction.enabled or transaction.rt.enabled to match the query shape used to build activeRecurringTransactions) and make the same change for the other occurrences noted (the nearby badge at the other index range) so the badge reflects the actual enabled flag.
423-428:⚠️ Potential issue | 🟡 MinorFix recurring delete refresh when mutation is skipped
useDeleteRecurringTransactionalready invalidates the recurring query (queryKey: [...transactionQueryKeys.all, 'recurring']), which matchesuseGetRecurringTransactions, so the list refreshes whendeleteRecurringTxnMutation.mutateAsync(txnId)runs.However,
LeaseDetail.handleDeleteTransactionskips the mutation whenlooksLikeBackendIdis false and then closes the modal/clearstransactionToDeletewithout invalidating or updatingactiveRecurringTransactions(derived fromuseGetRecurringTransactions()), so the deleted row can remain visible in that case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/Leases/LeaseDetail.tsx` around lines 423 - 428, handleDeleteTransaction currently skips calling the recurring delete mutation when looksLikeBackendId is false and then closes the modal without refreshing activeRecurringTransactions, leaving the deleted row visible; update handleDeleteTransaction to ensure the recurring list is refreshed when the mutation is skipped by either (a) calling the cache invalidation used by useDeleteRecurringTransaction (e.g. queryClient.invalidateQueries([...transactionQueryKeys.all, 'recurring'])) after you setIsDeleteTransactionModalOpen(false)/setTransactionToDelete(null), or (b) directly remove the deleted txnId from the activeRecurringTransactions source (the data returned by useGetRecurringTransactions) so the UI is updated; locate this logic in LeaseDetail.handleDeleteTransaction and apply one of these fixes so the recurring query is always refreshed even when mutateAsync is not invoked.src/pages/Dashboard/features/Application/NewApplication.tsx (1)
276-323:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffDebouncing blocks immediate retry after submission errors.
All three form handlers are wrapped with
useDebouncedCallbackusing the default 1-second delay. While this successfully prevents accidental double-submissions on success, it also blocks users from immediately retrying after errors (e.g., network failures, validation errors from the backend). Users who see an error modal and click submit again must wait the full debounce period, creating a frustrating experience.Consider one of these alternatives:
- Reset debounce on error: Clear the timer when an error occurs so users can retry immediately
- Shorter delay: Reduce the delay to 300-500ms for better UX balance
- Success-only debouncing: Only apply debounce logic after successful submissions, not on errors
Example: Reset debounce timer on error
This would require enhancing the
useDebouncedCallbackhook to expose areset()function:export function useDebouncedCallback<T extends (...args: any[]) => any>( callback: T, delay = 1000 ): [T, () => void] { const timer = useRef<ReturnType<typeof setTimeout> | null>(null); const callbackRef = useRef(callback); callbackRef.current = callback; const reset = useCallback(() => { if (timer.current) { clearTimeout(timer.current); timer.current = null; } }, []); const debouncedFn = useCallback((...args: Parameters<T>) => { if (timer.current) return; callbackRef.current(...args); timer.current = setTimeout(() => { timer.current = null; }, delay); }, [delay]) as T; return [debouncedFn, reset]; }Then in your handlers:
const [handleSubmitSuccess, resetDebounce] = useDebouncedCallback(async () => { // ... existing logic try { await submitApplication(leasingId); } catch (error) { resetDebounce(); // Allow immediate retry setErrorMessages([...]); setShowErrorModal(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 `@src/pages/Dashboard/features/Application/NewApplication.tsx` around lines 276 - 323, The debounced submit handler blocks immediate retries after errors; update useDebouncedCallback to expose a reset() and change the handler setup to const [handleSubmitSuccess, resetDebounce] = useDebouncedCallback(...), then call resetDebounce() inside the catch block of handleSubmitSuccess (before setErrorMessages/setShowErrorModal) so the timer is cleared on error and users can retry immediately; alternatively, reduce the debounce delay (e.g., 300–500ms) or apply debouncing only after a successful create in applicationService.create if you prefer not to expose reset().src/pages/Dashboard/features/MoveIn/MoveIn.tsx (1)
144-224:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd an in-flight guard before lease create/update.
At Line 144, debounce suppresses only within the debounce interval. A second trigger after that interval can still enter Line 190 while the first mutation is in progress, risking duplicate lease writes.
Suggested fix
const handleCompleteMoveIn = useDebouncedCallback(async () => { + if (createLeaseMutation.isPending || updateLeaseMutation.isPending) { + return; + } + // Validate required fields if (!formData.tenantId || !formData.propertyId) { alert('Please complete all required fields (Property and Tenant)'); return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/MoveIn/MoveIn.tsx` around lines 144 - 224, handleCompleteMoveIn can be invoked again after the debounce window while a mutation is still in flight, causing duplicate lease writes; add an in-flight guard boolean (e.g., isSubmitting / isProcessing) scoped alongside the component state and check it at the top of handleCompleteMoveIn to early-return if true, set it true immediately before calling createLeaseMutation.mutateAsync or updateLeaseMutation.mutateAsync, and clear it in a finally block after the try/catch so the flag is always reset; update related UI (disable submit button or rely on the flag) as needed to prevent concurrent calls while the createLeaseMutation.mutateAsync / updateLeaseMutation.mutateAsync operations are pending.
🧹 Nitpick comments (10)
src/hooks/useDebounce.ts (1)
8-8: 💤 Low valueConsider stricter typing for function parameters.
While
any[]is functional for a generic debounce hook, you could constrain the generic to require functions with known parameter types for better type safety. However, given this is a utility hook meant to wrap any function, the current approach is acceptable if you're comfortable with the ESLint suppression trade-off.Alternative: Acknowledge the linter with a focused suppression
+// eslint-disable-next-line `@typescript-eslint/no-explicit-any` export function useDebouncedCallback<T extends (...args: any[]) => any>( callback: T, delay = 1000 ): T {This makes it clear the
anyusage is intentional for maximum flexibility.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/hooks/useDebounce.ts` at line 8, The useDebouncedCallback<T extends (...args: any[]) => any> signature uses any[] for parameters which weakens type safety; either tighten the generic to accept a function type and derive its parameter types (e.g., make the generic represent the function type and use Parameters<T> for args) so callers get proper typings, or explicitly add a focused ESLint suppression comment above the declaration clarifying the intentional use of any for maximum flexibility; update the function signature in useDebouncedCallback and associated type references (the useDebouncedCallback declaration and any internal param typings) accordingly.src/pages/userdashboard/features/Leases/UserLeaseDetails.tsx (2)
1-8: ⚡ Quick winMove helper function after imports to follow ES module conventions.
The
fmtDatehelper is defined before the import statements, which is unconventional. ES module imports should appear at the top of the file before any code.📦 Suggested reordering
-import { useState, useMemo } from 'react'; - -const fmtDate = (iso: string) => { - if (!iso) return ''; - try { - return new Date(iso).toLocaleDateString('en-US', { month: 'short', day: '2-digit', year: 'numeric' }); - } catch { return iso; } -}; import { useParams, useNavigate } from 'react-router-dom'; +import { useState, useMemo } from 'react'; import { Calendar, Home, User, ArrowLeft } from 'lucide-react'; import type { Lease } from '../../utils/types'; import PrimaryActionButton from "../../../../components/common/buttons/PrimaryActionButton"; @@ -18,6 +13,13 @@ import { useGetTransactions } from "../../../../hooks/useTransactionQueries"; import { useGetRenderedDocuments } from "../../../../hooks/useDocumentsQueries"; +const fmtDate = (iso: string) => { + if (!iso) return ''; + try { + return new Date(iso).toLocaleDateString('en-US', { month: 'short', day: '2-digit', year: 'numeric' }); + } catch { return iso; } +}; + // Constants const DASHBOARD_PATH = "/userdashboard";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/userdashboard/features/Leases/UserLeaseDetails.tsx` around lines 1 - 8, The helper function fmtDate is declared before the ES module imports; move the fmtDate declaration so all import statements (e.g., the existing "import { useState, useMemo } from 'react';") appear first, then define fmtDate below them; ensure the function name (fmtDate) and its behavior remain unchanged and any usages still reference the same identifier.
3-8: ⚡ Quick winExtract the duplicated
fmtDatehelper to a shared utility module.The
fmtDatefunction is duplicated in bothUserLeaseDetails.tsxandLeaseCard.tsx(lines 5-10). This violates the DRY principle and makes maintenance harder.♻️ Suggested refactor
Create a shared utility file, e.g.,
src/utils/date.utils.ts:export const formatLeaseDate = (iso: string): string => { if (!iso) return ''; try { return new Date(iso).toLocaleDateString('en-US', { month: 'short', day: '2-digit', year: 'numeric' }); } catch { return iso; } };Then import and use it in both files:
import { formatLeaseDate } from '`@/utils/date.utils`'; // Usage label={`${formatLeaseDate(lease.startDate)} – ${lease.endDate ? formatLeaseDate(lease.endDate) : 'Present'}`}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/userdashboard/features/Leases/UserLeaseDetails.tsx` around lines 3 - 8, The helper fmtDate is duplicated; extract it into a shared utility function (e.g., formatLeaseDate) and replace local fmtDate usages in UserLeaseDetails.tsx and LeaseCard.tsx with an import from the new util. Create the new exported function (formatLeaseDate) that preserves current behavior and signature (accepts iso: string, returns string), then update imports in the components to use formatLeaseDate wherever fmtDate was used and remove the local fmtDate definitions.src/pages/Dashboard/features/Application/components/ApplicantForm.tsx (1)
258-263: ⚡ Quick winType the
overrideValueparameter instead of usingany.The
overrideValueparameter should be typed as the union of all possibleFormDatafield value types for better type safety.🔧 Suggested type-safe implementation
- const handleBlur = (key: keyof FormData, overrideValue?: any) => { + const handleBlur = (key: keyof FormData, overrideValue?: FormData[keyof FormData]) => { setTouched(prev => ({ ...prev, [key]: true })); const valueToValidate = overrideValue !== undefined ? overrideValue : data[key]; const error = validateField(key, valueToValidate); setErrors(prev => ({ ...prev, [key]: 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 `@src/pages/Dashboard/features/Application/components/ApplicantForm.tsx` around lines 258 - 263, The handleBlur function uses overrideValue: any — change this to a typed union matching all possible FormData field value types (e.g., string | number | boolean | Date | null or whatever your FormData fields use) so TypeScript can check calls to handleBlur; update the signature const handleBlur = (key: keyof FormData, overrideValue?: /* union-of-FormData-value-types */) => { ... } and ensure validateField accepts that union type as well (adjust validateField generic/parameter types if needed) to keep type-safety across handleBlur, data[key], and setErrors.src/pages/userdashboard/features/Leases/components/LeaseCard.tsx (1)
1-10: ⚡ Quick winMove helper function after imports to follow ES module conventions.
The
fmtDatehelper is defined before the import statements. ES module imports should appear at the top of the file before any code. Additionally, this helper is duplicated inUserLeaseDetails.tsx— consider extracting it to a shared utility module as suggested in that file's review.📦 Suggested reordering
import type { Lease } from "../../../utils/types"; import { StatusPill } from "./StatusPill"; import { useNavigate } from "react-router-dom"; const fmtDate = (iso: string) => { if (!iso) return ''; try { return new Date(iso).toLocaleDateString('en-US', { month: 'short', day: '2-digit', year: 'numeric' }); } catch { return iso; } }; -🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/userdashboard/features/Leases/components/LeaseCard.tsx` around lines 1 - 10, Move the fmtDate helper so all import statements appear at the top of LeaseCard.tsx (ensure fmtDate is defined after the imports), and remove the duplicate in UserLeaseDetails.tsx by extracting fmtDate into a shared utility (e.g., create and export a formatDate/ fmtDate function from a common utils/date helper) then import and use that function in both LeaseCard (where StatusPill and useNavigate are imported) and UserLeaseDetails; update references to the helper (fmtDate) accordingly to the new exported name and path.src/utils/downloadCsv.ts (2)
15-31: ⚡ Quick winConsider adding UTF-8 BOM for Excel compatibility.
Excel on Windows may misinterpret special characters (accented letters, currency symbols) without a UTF-8 Byte Order Mark. Prepending
'\uFEFF'tocsvContentensures correct encoding.♻️ Suggested enhancement
export function downloadCsv(filename: string, headers: string[], rows: unknown[][]): void { - const csvContent = [ + const csvContent = '\uFEFF' + [ headers.map(escapeCsv).join(','), ...rows.map(row => row.map(escapeCsv).join(',')), ].join('\n');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/downloadCsv.ts` around lines 15 - 31, The CSV exporter (downloadCsv) should prepend a UTF-8 BOM so Excel renders special characters correctly; modify how csvContent is passed to the Blob (used by URL.createObjectURL) by prefixing it with '\uFEFF' (e.g. create the Blob from ['\uFEFF', csvContent]) before creating the object URL and triggering link.click; update the logic around csvContent/Blob creation in the downloadCsv function accordingly.
23-29: 💤 Low valueAdd SSR safety guard.
The function assumes
documentis available and will crash in server-side rendering contexts. Add a guard or document this as browser-only.🛡️ Suggested guard
export function downloadCsv(filename: string, headers: string[], rows: unknown[][]): void { + if (typeof document === 'undefined') { + console.warn('downloadCsv requires a browser environment'); + return; + } const csvContent = [🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/downloadCsv.ts` around lines 23 - 29, The downloadCsv function currently assumes a browser DOM and directly uses document APIs (link creation, appendChild, click, removeChild), which will throw during SSR; add an SSR safety guard at the top of the function (e.g., check typeof document !== 'undefined' or typeof window !== 'undefined') and early-return or throw a clear browser-only error when running outside the browser; ensure the DOM-only code that references document (creating the anchor, setting href/download, appending, clicking, removing) is executed only after that guard so downloadCsv (or its exported name in src/utils/downloadCsv.ts) is safe to import/server-render.src/services/reports.service.ts (1)
197-204: 💤 Low valueFormat fallback could display raw ISO strings.
When date parsing fails,
formatDatereturns the originalisostring, which may not be user-friendly (e.g.,"2024-13-45T99:99:99Z"for malformed input). Consider returning'—'or'Invalid date'on error.♻️ Alternative fallback
function formatDate(iso: string | null | undefined): string { if (!iso) return '—'; try { return new Date(iso).toLocaleDateString('en-US', { year: 'numeric', month: 'short', day: '2-digit' }); } catch { - return iso; + return '—'; } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/reports.service.ts` around lines 197 - 204, The formatDate function currently returns the raw iso string on parse errors; update formatDate (the function named formatDate) so that if new Date(iso) parsing throws or results in an invalid Date, it returns a user-friendly fallback like '—' or 'Invalid date' instead of returning the original iso value; keep the existing null/undefined check and ensure both thrown errors and invalid Date instances (isNaN(date.getTime())) are handled to produce the chosen fallback.src/pages/Dashboard/features/Reports/Reports.tsx (2)
8-13: 💤 Low valueConsider logging localStorage errors.
The
getHiddenReportscatch block silently swallows all errors. IflocalStoragecontains malformed JSON or quota is exceeded, the silent failure could make debugging difficult.🐛 Enhanced error handling
function getHiddenReports(): string[] { - try { return JSON.parse(localStorage.getItem(SETTINGS_KEY) ?? '[]'); } catch { return []; } + try { + return JSON.parse(localStorage.getItem(SETTINGS_KEY) ?? '[]'); + } catch (err) { + console.warn('Failed to load hidden reports from localStorage:', err); + return []; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/Reports/Reports.tsx` around lines 8 - 13, The catch in getHiddenReports currently swallows all errors—update it to catch the error as e and log a descriptive message plus the error details (e.g., with console.error or the app logger) while still returning []; also wrap saveHiddenReports' localStorage.setItem call in a try/catch to log any exceptions (including quota or serialization errors) with context and the hidden value being saved so debugging is possible; ensure function names getHiddenReports and saveHiddenReports are used to locate and update these blocks.
118-118: 💤 Low valueRemove unused parameter.
The
toggleSectionfunction's_idparameter is unused (ESLint warning). If this is a placeholder for future functionality, consider adding a comment; otherwise remove it.♻️ Cleanup suggestion
-const toggleSection = (_id: string) => {}; +const toggleSection = () => {};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/Reports/Reports.tsx` at line 118, The toggleSection function currently declares an unused parameter _id which triggers an ESLint warning; either remove the parameter from the function signature (change const toggleSection = () => {}), or if it's a deliberate placeholder for future use, add a brief comment inside or above toggleSection (e.g., // _id reserved for future section toggling) and prefix it with /* eslint-disable-next-line `@typescript-eslint/no-unused-vars` */ or rename to _unused to indicate intentionality; update any callers if you remove the parameter and keep references to the function name toggleSection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/hooks/useDebounce.ts`:
- Around line 16-22: The debounce hook sets timer.current via setTimeout inside
the function returned by useCallback but never clears that timeout on unmount;
add a useEffect in the same module (importing useEffect if not present) that
returns a cleanup function which, if timer.current exists, calls
clearTimeout(timer.current) and sets timer.current = null to prevent the timeout
from firing after unmount; keep the cleanup effect’s dependency array minimal
(e.g., []) so it runs on unmount and reference the existing timer ref name
(timer.current) and existing symbols useDebounce, callbackRef, and the
useCallback-returned function to locate the code to update.
In `@src/pages/Dashboard/features/Leads/LeadDetail.tsx`:
- Around line 59-71: The uploadFile function currently returns undefined on
non-OK responses which lets callers proceed and silently drop attachments;
change uploadFile to throw an error when the fetch response is not ok (include
response status and any JSON error message or res.statusText) so upstream
callers (the create/update activity flows that call uploadFile) will receive the
rejection and can abort the save and surface an error to the user; keep the
success path returning the URL (data.url || data.fileUrl || data.path) but on
failure await res.text() or res.json() for details and throw a new Error with
that information.
In `@src/pages/Dashboard/features/Leases/LeaseDetail.tsx`:
- Around line 395-396: The form is writing the human-facing modal frequency
(data.frequency, e.g. "Monthly") directly into invoiceSchedule instead of the
backend enum; change the assignment to map data.frequency to the backend enum
(e.g. "Monthly" -> "MONTHLY", "Every two weeks" -> "EVERY_TWO_WEEKS") before
setting invoiceSchedule, keeping the fallback to
backendLease?.recurringRent?.startOn or new Date() for startOn; apply the same
normalization wherever the edit form is seeded or invoiceSchedule is set
(locations using data.frequency, invoiceSchedule, and
backendLease?.recurringRent) so the add/edit paths and display helpers all use
the backend enum values.
- Around line 382-401: The current edit branch (when recurringRentModalMode ===
'edit') wrongly always calls updateLeaseMutation.mutateAsync to update
lease.recurringRent even when recurringRentToEdit is an "other recurring
transaction" (a backend transaction row), which overwrites the lease's main
recurring rent; add a guard that detects when recurringRentToEdit refers to a
separate recurring-transaction (not the lease's main recurringRent) and do NOT
call updateLeaseMutation in that case — instead call the appropriate
updateRecurringTransaction mutation (e.g.,
updateRecurringTransaction.mutateAsync with the selected transaction id and
payload) or, until that mutation exists, early-return/log a warning and skip
updating lease.recurringRent; update the logic around recurringRentToEdit,
recurringRentModalMode and updateLeaseMutation.mutateAsync to implement this
guard.
In `@src/pages/Dashboard/features/Maintenance/components/PropertyTenantsStep.tsx`:
- Around line 49-50: PropertyTenantsStep currently allows advancing when
linkEquipment is true but no equipment is selected even if canCreateEquipment is
false; update the component to validate equipment selection before calling
onNext (or enabling the Next button): if linkEquipment === true and
canCreateEquipment === false then require selectedEquipment (or
selectedPropertyEquipment state) to be non-empty, set a validation error state
(e.g., equipmentError) and prevent calling onNext with an inconsistent payload;
apply the same guard/validation to the other flows referenced (the handler
around lines 117-132 and the submit/next logic around 397-410) so the form
cannot proceed without a chosen equipment when linking is requested.
In `@src/pages/Dashboard/features/Reports/Contacts.tsx`:
- Around line 87-91: handleDownload currently writes raw contact.phone values to
CSV (using activeColumns, filteredContacts and downloadCsv) but the UI displays
formatted numbers via formatPhoneNumber; update handleDownload so when mapping
columns for each contact it detects the phone column (col.id === 'phone' or
appropriate ContactItem key) and passes the contact.phone through
formatPhoneNumber (falling back to empty string for missing values) before
adding to rows, leaving other columns unchanged.
In `@src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx`:
- Around line 1-3: Add an import for the shared CSV export utility named
downloadCsv at the top of the file alongside the existing imports (with the
other utilities/imports such as useState, useMemo and lucide-react icons) so the
ProviderStatement component can call downloadCsv when exporting CSVs; locate the
import section in ProviderStatement.tsx and add an import referencing the shared
utility module and the downloadCsv symbol.
In `@src/pages/userdashboard/features/Requests/UserNewRequest.tsx`:
- Around line 119-123: The current loop always marks non-video files as IMAGE;
change the mapping in the uploaded.forEach to derive FileType from the file
MIME: use file.type.startsWith('image/') ? FileType.IMAGE :
file.type.startsWith('video/') ? FileType.OTHER : FileType.DOCUMENT (or the
appropriate DOCUMENT enum member) so documents are not misclassified; update the
code around uploaded.forEach and the attachmentDtos.push to use this conditional
mapping and reference the FileType enum.
- Around line 78-79: The code is casting lease.property.address to any when
formatting the address; instead, narrow the typed value first (e.g., const addr
= lease.property?.address) and then reference addr.streetAddress and addr.city
directly without using as any, handling the null case (addr ?
`${addr.streetAddress ?? ''}, ${addr.city ?? ''}` : '') so you preserve type
safety and avoid the unnecessary cast on lease.property.address.
In `@src/services/lead.service.ts`:
- Around line 109-120: The Backend response interfaces are missing the new
optional fields so TypeScript casts are used in LeadDetail; update the
interfaces BackendTask, BackendCall, and BackendMeeting inside lead.service.ts
to include assigneeLabel?: string and fileUrl?: string (matching
CreateTaskDto/UpdateTaskDto and the call/meeting DTOs) so the properties are
properly typed and the (… as any).assigneeLabel/fileUrl casts in LeadDetail.tsx
can be removed; ensure the types use the same optional string shape as the DTOs.
---
Outside diff comments:
In `@src/pages/Dashboard/features/Application/NewApplication.tsx`:
- Around line 276-323: The debounced submit handler blocks immediate retries
after errors; update useDebouncedCallback to expose a reset() and change the
handler setup to const [handleSubmitSuccess, resetDebounce] =
useDebouncedCallback(...), then call resetDebounce() inside the catch block of
handleSubmitSuccess (before setErrorMessages/setShowErrorModal) so the timer is
cleared on error and users can retry immediately; alternatively, reduce the
debounce delay (e.g., 300–500ms) or apply debouncing only after a successful
create in applicationService.create if you prefer not to expose reset().
In `@src/pages/Dashboard/features/Leads/LeadDetail.tsx`:
- Around line 427-439: The update branch currently uploads a replacement file to
produce fileUrl but then calls updateNoteMutation.mutateAsync with only {
content: noteText }, leaving uploads unlinked; change the update call in the
editing branch (the block checking editingItem &&
editingItem.item.originalData?.type === 'note') to pass the full noteData (which
already includes fileUrl when present) instead of the hard-coded { content:
noteText }, so use noteData in the payload to update both content and attachment
via updateNoteMutation.mutateAsync.
- Around line 1176-1178: The timeline currently assumes every attachment is an
image by using item.image in LeadDetail.tsx; change the rendering to detect
whether the attachment is an image (e.g., check item.mimeType or file extension
on item.fileUrl) and only render <img> for true image types, otherwise render a
clickable link/button that opens or downloads the file (use the existing
item.fileUrl), include target="_blank" and rel="noopener noreferrer" for
external opens, and provide a fallback icon/filename so non-image uploads
(PDFs/docs) are viewable from the timeline.
In `@src/pages/Dashboard/features/Leases/LeaseDetail.tsx`:
- Around line 1047-1049: The badge uses transaction.isEnabled but the list was
filtered from activeRecurringTransactions where the enabled flag is rt.enabled;
update the rendering to use the same field the query/filter uses (e.g., replace
transaction.isEnabled with transaction.enabled or transaction.rt.enabled to
match the query shape used to build activeRecurringTransactions) and make the
same change for the other occurrences noted (the nearby badge at the other index
range) so the badge reflects the actual enabled flag.
- Around line 423-428: handleDeleteTransaction currently skips calling the
recurring delete mutation when looksLikeBackendId is false and then closes the
modal without refreshing activeRecurringTransactions, leaving the deleted row
visible; update handleDeleteTransaction to ensure the recurring list is
refreshed when the mutation is skipped by either (a) calling the cache
invalidation used by useDeleteRecurringTransaction (e.g.
queryClient.invalidateQueries([...transactionQueryKeys.all, 'recurring'])) after
you setIsDeleteTransactionModalOpen(false)/setTransactionToDelete(null), or (b)
directly remove the deleted txnId from the activeRecurringTransactions source
(the data returned by useGetRecurringTransactions) so the UI is updated; locate
this logic in LeaseDetail.handleDeleteTransaction and apply one of these fixes
so the recurring query is always refreshed even when mutateAsync is not invoked.
In `@src/pages/Dashboard/features/MoveIn/MoveIn.tsx`:
- Around line 144-224: handleCompleteMoveIn can be invoked again after the
debounce window while a mutation is still in flight, causing duplicate lease
writes; add an in-flight guard boolean (e.g., isSubmitting / isProcessing)
scoped alongside the component state and check it at the top of
handleCompleteMoveIn to early-return if true, set it true immediately before
calling createLeaseMutation.mutateAsync or updateLeaseMutation.mutateAsync, and
clear it in a finally block after the try/catch so the flag is always reset;
update related UI (disable submit button or rely on the flag) as needed to
prevent concurrent calls while the createLeaseMutation.mutateAsync /
updateLeaseMutation.mutateAsync operations are pending.
---
Nitpick comments:
In `@src/hooks/useDebounce.ts`:
- Line 8: The useDebouncedCallback<T extends (...args: any[]) => any> signature
uses any[] for parameters which weakens type safety; either tighten the generic
to accept a function type and derive its parameter types (e.g., make the generic
represent the function type and use Parameters<T> for args) so callers get
proper typings, or explicitly add a focused ESLint suppression comment above the
declaration clarifying the intentional use of any for maximum flexibility;
update the function signature in useDebouncedCallback and associated type
references (the useDebouncedCallback declaration and any internal param typings)
accordingly.
In `@src/pages/Dashboard/features/Application/components/ApplicantForm.tsx`:
- Around line 258-263: The handleBlur function uses overrideValue: any — change
this to a typed union matching all possible FormData field value types (e.g.,
string | number | boolean | Date | null or whatever your FormData fields use) so
TypeScript can check calls to handleBlur; update the signature const handleBlur
= (key: keyof FormData, overrideValue?: /* union-of-FormData-value-types */) =>
{ ... } and ensure validateField accepts that union type as well (adjust
validateField generic/parameter types if needed) to keep type-safety across
handleBlur, data[key], and setErrors.
In `@src/pages/Dashboard/features/Reports/Reports.tsx`:
- Around line 8-13: The catch in getHiddenReports currently swallows all
errors—update it to catch the error as e and log a descriptive message plus the
error details (e.g., with console.error or the app logger) while still returning
[]; also wrap saveHiddenReports' localStorage.setItem call in a try/catch to log
any exceptions (including quota or serialization errors) with context and the
hidden value being saved so debugging is possible; ensure function names
getHiddenReports and saveHiddenReports are used to locate and update these
blocks.
- Line 118: The toggleSection function currently declares an unused parameter
_id which triggers an ESLint warning; either remove the parameter from the
function signature (change const toggleSection = () => {}), or if it's a
deliberate placeholder for future use, add a brief comment inside or above
toggleSection (e.g., // _id reserved for future section toggling) and prefix it
with /* eslint-disable-next-line `@typescript-eslint/no-unused-vars` */ or rename
to _unused to indicate intentionality; update any callers if you remove the
parameter and keep references to the function name toggleSection.
In `@src/pages/userdashboard/features/Leases/components/LeaseCard.tsx`:
- Around line 1-10: Move the fmtDate helper so all import statements appear at
the top of LeaseCard.tsx (ensure fmtDate is defined after the imports), and
remove the duplicate in UserLeaseDetails.tsx by extracting fmtDate into a shared
utility (e.g., create and export a formatDate/ fmtDate function from a common
utils/date helper) then import and use that function in both LeaseCard (where
StatusPill and useNavigate are imported) and UserLeaseDetails; update references
to the helper (fmtDate) accordingly to the new exported name and path.
In `@src/pages/userdashboard/features/Leases/UserLeaseDetails.tsx`:
- Around line 1-8: The helper function fmtDate is declared before the ES module
imports; move the fmtDate declaration so all import statements (e.g., the
existing "import { useState, useMemo } from 'react';") appear first, then define
fmtDate below them; ensure the function name (fmtDate) and its behavior remain
unchanged and any usages still reference the same identifier.
- Around line 3-8: The helper fmtDate is duplicated; extract it into a shared
utility function (e.g., formatLeaseDate) and replace local fmtDate usages in
UserLeaseDetails.tsx and LeaseCard.tsx with an import from the new util. Create
the new exported function (formatLeaseDate) that preserves current behavior and
signature (accepts iso: string, returns string), then update imports in the
components to use formatLeaseDate wherever fmtDate was used and remove the local
fmtDate definitions.
In `@src/services/reports.service.ts`:
- Around line 197-204: The formatDate function currently returns the raw iso
string on parse errors; update formatDate (the function named formatDate) so
that if new Date(iso) parsing throws or results in an invalid Date, it returns a
user-friendly fallback like '—' or 'Invalid date' instead of returning the
original iso value; keep the existing null/undefined check and ensure both
thrown errors and invalid Date instances (isNaN(date.getTime())) are handled to
produce the chosen fallback.
In `@src/utils/downloadCsv.ts`:
- Around line 15-31: The CSV exporter (downloadCsv) should prepend a UTF-8 BOM
so Excel renders special characters correctly; modify how csvContent is passed
to the Blob (used by URL.createObjectURL) by prefixing it with '\uFEFF' (e.g.
create the Blob from ['\uFEFF', csvContent]) before creating the object URL and
triggering link.click; update the logic around csvContent/Blob creation in the
downloadCsv function accordingly.
- Around line 23-29: The downloadCsv function currently assumes a browser DOM
and directly uses document APIs (link creation, appendChild, click,
removeChild), which will throw during SSR; add an SSR safety guard at the top of
the function (e.g., check typeof document !== 'undefined' or typeof window !==
'undefined') and early-return or throw a clear browser-only error when running
outside the browser; ensure the DOM-only code that references document (creating
the anchor, setting href/download, appending, clicking, removing) is executed
only after that guard so downloadCsv (or its exported name in
src/utils/downloadCsv.ts) is safe to import/server-render.
🪄 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: c1072f8d-2af4-4a24-ad0e-d88478c48adb
📒 Files selected for processing (38)
src/config/api.config.tssrc/hooks/useDebounce.tssrc/hooks/useReportsQueries.tssrc/pages/Dashboard/features/Application/NewApplication.tsxsrc/pages/Dashboard/features/Application/components/ApplicantForm.tsxsrc/pages/Dashboard/features/KeysLocks/AddKey.tsxsrc/pages/Dashboard/features/Leads/AddLead.tsxsrc/pages/Dashboard/features/Leads/LeadDetail.tsxsrc/pages/Dashboard/features/Leases/LeaseDetail.tsxsrc/pages/Dashboard/features/ListUnit/ListUnit.tsxsrc/pages/Dashboard/features/Maintenance/components/DueDateMaterialsStep.tsxsrc/pages/Dashboard/features/Maintenance/components/PropertyTenantsStep.tsxsrc/pages/Dashboard/features/MoveIn/MoveIn.tsxsrc/pages/Dashboard/features/Properties/AddProperty.tsxsrc/pages/Dashboard/features/Properties/EditProperty.tsxsrc/pages/Dashboard/features/Reports/Contacts.tsxsrc/pages/Dashboard/features/Reports/GeneralExpenses.tsxsrc/pages/Dashboard/features/Reports/GeneralIncome.tsxsrc/pages/Dashboard/features/Reports/MaintenanceRequestsReport.tsxsrc/pages/Dashboard/features/Reports/PropertyExpenses.tsxsrc/pages/Dashboard/features/Reports/PropertyStatement.tsxsrc/pages/Dashboard/features/Reports/RentRoll.tsxsrc/pages/Dashboard/features/Reports/RentersInsurance.tsxsrc/pages/Dashboard/features/Reports/Reports.tsxsrc/pages/Dashboard/features/Reports/TenantStatement.tsxsrc/pages/Dashboard/features/Reports/VacantRentals.tsxsrc/pages/Dashboard/features/ServicePros/ProviderStatement.tsxsrc/pages/Dashboard/settings/index.tsxsrc/pages/userdashboard/features/Applications/UserNewApplication.tsxsrc/pages/userdashboard/features/Leases/UserLeaseDetails.tsxsrc/pages/userdashboard/features/Leases/components/LeaseCard.tsxsrc/pages/userdashboard/features/Requests/UserNewRequest.tsxsrc/pages/userdashboard/features/Requests/components/UserStep1RequestForm.tsxsrc/pages/userdashboard/features/Requests/components/UserStep2PropertyTenants.tsxsrc/pages/userdashboard/features/Requests/hooks/useNewRequestForm.tssrc/services/lead.service.tssrc/services/reports.service.tssrc/utils/downloadCsv.ts
Comment on lines
+16
to
+22
| return useCallback((...args: Parameters<T>) => { | ||
| if (timer.current) return; | ||
| callbackRef.current(...args); | ||
| timer.current = setTimeout(() => { | ||
| timer.current = null; | ||
| }, delay); | ||
| }, [delay]) as T; |
Contributor
There was a problem hiding this comment.
Memory leak: timer not cleaned up on unmount.
The useCallback sets a timeout via timer.current = setTimeout(...) but never clears it when the component unmounts. If the component unmounts while the timer is active, the timeout will still fire and attempt to write to timer.current on an unmounted component.
🔧 Proposed fix
return useCallback((...args: Parameters<T>) => {
if (timer.current) return;
callbackRef.current(...args);
timer.current = setTimeout(() => {
timer.current = null;
}, delay);
+
+ return () => {
+ if (timer.current) {
+ clearTimeout(timer.current);
+ timer.current = null;
+ }
+ };
}, [delay]) as T;Actually, wait - useCallback doesn't support cleanup functions. We need to add a useEffect to handle cleanup:
export function useDebouncedCallback<T extends (...args: any[]) => any>(
callback: T,
delay = 1000
): T {
const timer = useRef<ReturnType<typeof setTimeout> | null>(null);
const callbackRef = useRef(callback);
callbackRef.current = callback;
+
+ useEffect(() => {
+ return () => {
+ if (timer.current) {
+ clearTimeout(timer.current);
+ timer.current = null;
+ }
+ };
+ }, []);
return useCallback((...args: Parameters<T>) => {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/hooks/useDebounce.ts` around lines 16 - 22, The debounce hook sets
timer.current via setTimeout inside the function returned by useCallback but
never clears that timeout on unmount; add a useEffect in the same module
(importing useEffect if not present) that returns a cleanup function which, if
timer.current exists, calls clearTimeout(timer.current) and sets timer.current =
null to prevent the timeout from firing after unmount; keep the cleanup effect’s
dependency array minimal (e.g., []) so it runs on unmount and reference the
existing timer ref name (timer.current) and existing symbols useDebounce,
callbackRef, and the useCallback-returned function to locate the code to update.
Comment on lines
+59
to
+71
| async function uploadFile(file: File): Promise<string | undefined> { | ||
| const formData = new FormData(); | ||
| formData.append('file', file); | ||
| const res = await fetch(`${import.meta.env.VITE_API_BASE_URL || 'http://localhost:3000'}/upload/file`, { | ||
| method: 'POST', | ||
| credentials: 'include', | ||
| body: formData, | ||
| }); | ||
| if (res.ok) { | ||
| const data = await res.json(); | ||
| return data.url || data.fileUrl || data.path; | ||
| } | ||
| return undefined; |
Contributor
There was a problem hiding this comment.
Fail the save when the attachment upload fails.
Line 71 returns undefined for any non-2xx upload, and every caller then creates/updates the activity without the selected file. That silently drops user data instead of telling them the attachment failed.
🛠️ Suggested fix
async function uploadFile(file: File): Promise<string | undefined> {
const formData = new FormData();
formData.append('file', file);
const res = await fetch(`${import.meta.env.VITE_API_BASE_URL || 'http://localhost:3000'}/upload/file`, {
method: 'POST',
credentials: 'include',
body: formData,
});
- if (res.ok) {
- const data = await res.json();
- return data.url || data.fileUrl || data.path;
- }
- return undefined;
+ if (!res.ok) {
+ throw new Error('File upload failed');
+ }
+
+ const data = await res.json();
+ const fileUrl = data.url || data.fileUrl || data.path;
+ if (!fileUrl) {
+ throw new Error('Upload succeeded but no file URL was returned');
+ }
+
+ return fileUrl;
}📝 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.
Suggested change
| async function uploadFile(file: File): Promise<string | undefined> { | |
| const formData = new FormData(); | |
| formData.append('file', file); | |
| const res = await fetch(`${import.meta.env.VITE_API_BASE_URL || 'http://localhost:3000'}/upload/file`, { | |
| method: 'POST', | |
| credentials: 'include', | |
| body: formData, | |
| }); | |
| if (res.ok) { | |
| const data = await res.json(); | |
| return data.url || data.fileUrl || data.path; | |
| } | |
| return undefined; | |
| async function uploadFile(file: File): Promise<string | undefined> { | |
| const formData = new FormData(); | |
| formData.append('file', file); | |
| const res = await fetch(`${import.meta.env.VITE_API_BASE_URL || 'http://localhost:3000'}/upload/file`, { | |
| method: 'POST', | |
| credentials: 'include', | |
| body: formData, | |
| }); | |
| if (!res.ok) { | |
| throw new Error('File upload failed'); | |
| } | |
| const data = await res.json(); | |
| const fileUrl = data.url || data.fileUrl || data.path; | |
| if (!fileUrl) { | |
| throw new Error('Upload succeeded but no file URL was returned'); | |
| } | |
| return fileUrl; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/Leads/LeadDetail.tsx` around lines 59 - 71, The
uploadFile function currently returns undefined on non-OK responses which lets
callers proceed and silently drop attachments; change uploadFile to throw an
error when the fetch response is not ok (include response status and any JSON
error message or res.statusText) so upstream callers (the create/update activity
flows that call uploadFile) will receive the rejection and can abort the save
and surface an error to the user; keep the success path returning the URL
(data.url || data.fileUrl || data.path) but on failure await res.text() or
res.json() for details and throw a new Error with that information.
Comment on lines
+382
to
+401
| } else if (recurringRentModalMode === 'edit') { | ||
| if (recurringRentToEdit && recurringRentToEdit.id && recurringRentToEdit.id.startsWith('trans-')) { | ||
| // In case it's a dummy transaction, we just skip it for now since there's no update endpoint. | ||
| // A real transaction from DB would not have an id starting with trans- | ||
| console.warn('Editing other recurring transactions is currently not supported via the API'); | ||
| } else { | ||
| // Update main recurring rent | ||
| await updateLeaseMutation.mutateAsync({ | ||
| id: id as string, | ||
| data: { | ||
| recurringRent: { | ||
| enabled: data.isEnabled, | ||
| amount: parseFloat(data.totalAmount || 0), | ||
| invoiceSchedule: data.frequency, // E.g. 'Monthly' | ||
| startOn: data.firstInvoiceDate ? data.firstInvoiceDate.toISOString() : (backendLease?.recurringRent?.startOn || new Date().toISOString()), | ||
| isMonthToMonth: backendLease?.recurringRent?.isMonthToMonth ?? false, | ||
| markPastPaid: backendLease?.recurringRent?.markPastPaid ?? false, | ||
| } | ||
| } as any | ||
| }); |
Contributor
There was a problem hiding this comment.
Don't send "other recurring transaction" edits through updateLeaseMutation.
The pencil action for rows in "Other recurring transactions" passes a backend transaction into this save path, but this branch updates lease.recurringRent instead of the selected recurring transaction. Editing a fee/income row here will leave that row unchanged and overwrite the lease's main recurring rent settings.
Suggested guard until a real recurring-transaction update mutation exists
} else if (recurringRentModalMode === 'edit') {
- if (recurringRentToEdit && recurringRentToEdit.id && recurringRentToEdit.id.startsWith('trans-')) {
- // In case it's a dummy transaction, we just skip it for now since there's no update endpoint.
- // A real transaction from DB would not have an id starting with trans-
- console.warn('Editing other recurring transactions is currently not supported via the API');
- } else {
+ const isMainRecurringRent = !recurringRentToEdit?.id;
+ if (!isMainRecurringRent) {
+ throw new Error('Editing other recurring transactions is not supported yet.');
+ } else {
// Update main recurring rent
await updateLeaseMutation.mutateAsync({Also applies to: 1058-1063
🧰 Tools
🪛 ESLint
[error] 400-400: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/Leases/LeaseDetail.tsx` around lines 382 - 401,
The current edit branch (when recurringRentModalMode === 'edit') wrongly always
calls updateLeaseMutation.mutateAsync to update lease.recurringRent even when
recurringRentToEdit is an "other recurring transaction" (a backend transaction
row), which overwrites the lease's main recurring rent; add a guard that detects
when recurringRentToEdit refers to a separate recurring-transaction (not the
lease's main recurringRent) and do NOT call updateLeaseMutation in that case —
instead call the appropriate updateRecurringTransaction mutation (e.g.,
updateRecurringTransaction.mutateAsync with the selected transaction id and
payload) or, until that mutation exists, early-return/log a warning and skip
updating lease.recurringRent; update the logic around recurringRentToEdit,
recurringRentModalMode and updateLeaseMutation.mutateAsync to implement this
guard.
Comment on lines
+395
to
+396
| invoiceSchedule: data.frequency, // E.g. 'Monthly' | ||
| startOn: data.firstInvoiceDate ? data.firstInvoiceDate.toISOString() : (backendLease?.recurringRent?.startOn || new Date().toISOString()), |
Contributor
There was a problem hiding this comment.
Map the modal frequency back to the backend enum before saving.
Line 937 seeds the edit form with "Monthly", but this branch writes data.frequency directly into invoiceSchedule. The rest of this file expects enum values like MONTHLY/EVERY_TWO_WEEKS, so the edit path now speaks a different contract than both the add path and the display helpers.
Suggested normalization
+const mapFrequencyToInvoiceSchedule = (frequency: string) => {
+ const normalized = frequency.trim().toUpperCase().replace(/\s+/g, '_');
+ return normalized;
+};
+
...
enabled: data.isEnabled,
amount: parseFloat(data.totalAmount || 0),
- invoiceSchedule: data.frequency, // E.g. 'Monthly'
+ invoiceSchedule: mapFrequencyToInvoiceSchedule(data.frequency),
startOn: data.firstInvoiceDate ? data.firstInvoiceDate.toISOString() : (backendLease?.recurringRent?.startOn || new Date().toISOString()),Also applies to: 937-939, 219-230
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/Leases/LeaseDetail.tsx` around lines 395 - 396,
The form is writing the human-facing modal frequency (data.frequency, e.g.
"Monthly") directly into invoiceSchedule instead of the backend enum; change the
assignment to map data.frequency to the backend enum (e.g. "Monthly" ->
"MONTHLY", "Every two weeks" -> "EVERY_TWO_WEEKS") before setting
invoiceSchedule, keeping the fallback to backendLease?.recurringRent?.startOn or
new Date() for startOn; apply the same normalization wherever the edit form is
seeded or invoiceSchedule is set (locations using data.frequency,
invoiceSchedule, and backendLease?.recurringRent) so the add/edit paths and
display helpers all use the backend enum values.
Comment on lines
+49
to
50
| const PropertyTenantsStep: React.FC<PropertyTenantsStepProps> = ({ onNext, onBack, properties, initialData, canCreateEquipment = true }) => { | ||
| const [selectedProperty, setSelectedProperty] = useState(initialData?.selectedProperty || ''); |
Contributor
There was a problem hiding this comment.
Validate equipment selection when linkEquipment is enabled.
When creation is gated by canCreateEquipment={false}, users can still continue with linkEquipment enabled and no selected equipment, which propagates an inconsistent payload to parent/store.
Suggested fix
const handleContinue = () => {
if (!selectedProperty) {
setValidationError('Please select a property before continuing.');
return;
}
+ if (linkEquipment && !selectedEquipment) {
+ setValidationError('Please select equipment or disable "Link equipment".');
+ return;
+ }
if (petsInResidence === 'yes' && selectedPets.length === 0) {
setValidationError('Please select at least one pet type.');
return;
}Also applies to: 117-132, 397-410
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/Maintenance/components/PropertyTenantsStep.tsx`
around lines 49 - 50, PropertyTenantsStep currently allows advancing when
linkEquipment is true but no equipment is selected even if canCreateEquipment is
false; update the component to validate equipment selection before calling
onNext (or enabling the Next button): if linkEquipment === true and
canCreateEquipment === false then require selectedEquipment (or
selectedPropertyEquipment state) to be non-empty, set a validation error state
(e.g., equipmentError) and prevent calling onNext with an inconsistent payload;
apply the same guard/validation to the other flows referenced (the handler
around lines 117-132 and the submit/next logic around 397-410) so the form
cannot proceed without a chosen equipment when linking is requested.
Comment on lines
+87
to
+91
| const handleDownload = () => { | ||
| const headers = activeColumns.map(col => col.label); | ||
| const rows = filteredContacts.map(contact => activeColumns.map(col => contact[col.id as keyof ContactItem] ?? '')); | ||
| downloadCsv('contacts', headers, rows); | ||
| }; |
Contributor
There was a problem hiding this comment.
CSV export omits phone number formatting.
The handleDownload function exports raw contact.phone values, but the UI display (line 101) applies formatPhoneNumber. This inconsistency means CSV downloads will contain unformatted phone numbers while the screen shows formatted ones.
📞 Suggested fix to format phone in CSV
const handleDownload = () => {
const headers = activeColumns.map(col => col.label);
- const rows = filteredContacts.map(contact => activeColumns.map(col => contact[col.id as keyof ContactItem] ?? ''));
+ const rows = filteredContacts.map(contact => activeColumns.map(col => {
+ const val = contact[col.id as keyof ContactItem] ?? '';
+ return col.id === 'phone' ? formatPhoneNumber(String(val)) : val;
+ }));
downloadCsv('contacts', headers, rows);
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/Reports/Contacts.tsx` around lines 87 - 91,
handleDownload currently writes raw contact.phone values to CSV (using
activeColumns, filteredContacts and downloadCsv) but the UI displays formatted
numbers via formatPhoneNumber; update handleDownload so when mapping columns for
each contact it detects the phone column (col.id === 'phone' or appropriate
ContactItem key) and passes the contact.phone through formatPhoneNumber (falling
back to empty string for missing values) before adding to rows, leaving other
columns unchanged.
Comment on lines
+1
to
+3
| import { useState, useMemo } from 'react'; | ||
| import { useNavigate } from 'react-router-dom'; | ||
| import { ChevronLeft, ChevronUp, X, Check, Loader2 } from 'lucide-react'; | ||
| import { ChevronLeft, ChevronUp, X, Check, Loader2, Download, LayoutTemplate } from 'lucide-react'; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Import downloadCsv utility at the top of the file.
To use the shared CSV export utility (as suggested in the refactor above), add the import alongside other utilities.
➕ Add the missing import
import { useState, useMemo } from 'react';
import { useNavigate } from 'react-router-dom';
import { ChevronLeft, ChevronUp, X, Check, Loader2, Download, LayoutTemplate } from 'lucide-react';
+import { downloadCsv } from '../../../../utils/downloadCsv';
import DashboardFilter from '../../components/DashboardFilter';📝 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.
Suggested change
| import { useState, useMemo } from 'react'; | |
| import { useNavigate } from 'react-router-dom'; | |
| import { ChevronLeft, ChevronUp, X, Check, Loader2 } from 'lucide-react'; | |
| import { ChevronLeft, ChevronUp, X, Check, Loader2, Download, LayoutTemplate } from 'lucide-react'; | |
| import { useState, useMemo } from 'react'; | |
| import { useNavigate } from 'react-router-dom'; | |
| import { ChevronLeft, ChevronUp, X, Check, Loader2, Download, LayoutTemplate } from 'lucide-react'; | |
| import { downloadCsv } from '../../../../utils/downloadCsv'; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/ServicePros/ProviderStatement.tsx` around lines
1 - 3, Add an import for the shared CSV export utility named downloadCsv at the
top of the file alongside the existing imports (with the other utilities/imports
such as useState, useMemo and lucide-react icons) so the ProviderStatement
component can call downloadCsv when exporting CSVs; locate the import section in
ProviderStatement.tsx and add an import referencing the shared utility module
and the downloadCsv symbol.
Comment on lines
+78
to
+79
| ? `${(lease.property.address as any).streetAddress ?? ''}, ${(lease.property.address as any).city ?? ''}` | ||
| : '', |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
LEASE_FILE="$(fd -i 'lease.service.ts' | head -n1 || true)"
if [ -n "${LEASE_FILE}" ]; then
echo "Inspecting lease DTO in: ${LEASE_FILE}"
rg -n -C3 'address|streetAddress|city|property' "${LEASE_FILE}"
else
echo "lease.service.ts not found via fd"
fi
echo
echo "Current explicit any usage in NewRequest:"
rg -n -C2 'as any' src/pages/userdashboard/features/Requests/UserNewRequest.tsxRepository: scalixity-dev/PMS_Frontend
Length of output: 1683
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/userdashboard/features/Requests/UserNewRequest.tsx"
echo "---- $FILE (around 70-90) ----"
nl -ba "$FILE" | sed -n '60,95p'
echo
echo "---- Imports / lease typing in $FILE ----"
rg -n "BackendLease|lease\.property|lease\b|interface|type" "$FILE"
echo
echo "---- lease.service.ts: BackendLease property.address typing ----"
nl -ba "src/services/lease.service.ts" | sed -n '1,120p'Repository: scalixity-dev/PMS_Frontend
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/userdashboard/features/Requests/UserNewRequest.tsx"
echo "---- $FILE (around 70-90) ----"
sed -n '60,95p' "$FILE" | cat -n -w3 -s' '
echo
echo "---- lease typing usage context (in $FILE) ----"
rg -n "lease\.property|lease\W|BackendLease|address" "$FILE"
echo
echo "---- src/services/lease.service.ts: BackendLease property.address typing ----"
# show the address type block (top of file)
sed -n '1,170p' src/services/lease.service.ts | cat -n -w3 -s' 'Repository: scalixity-dev/PMS_Frontend
Length of output: 240
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file (avoid assuming exact path exists)
FILE="$(fd -i "UserNewRequest\.tsx" src/pages/userdashboard/features/Requests 2>/dev/null | head -n1 || true)"
if [ -z "$FILE" ]; then
FILE="$(fd -i "UserNewRequest\.tsx" src 2>/dev/null | head -n1 || true)"
fi
echo "---- Located file: $FILE ----"
if [ -n "$FILE" ]; then
echo "---- $FILE lines 70-90 ----"
awk 'NR>=70 && NR<=90 {printf "%4d: %s\n", NR, $0}' "$FILE"
echo
echo "---- Explicit any usage around address shaping ----"
rg -n -C2 'as any' "$FILE"
else
echo "File not found in repo via fd."
fi
echo
echo "---- lease.service.ts: find address type/BackendLease ----"
LEASE_FILE="$(fd -i 'lease\.service\.ts' src/services 2>/dev/null | head -n1 || true)"
if [ -z "$LEASE_FILE" ]; then
LEASE_FILE="$(fd -i 'lease\.service\.ts' src 2>/dev/null | head -n1 || true)"
fi"
if [ -n "$LEASE_FILE" ]; then
echo "Inspecting: $LEASE_FILE"
rg -n -C3 'interface BackendLease|property\?:|address\?:|streetAddress|city' "$LEASE_FILE"
else
echo "lease.service.ts not found."
fiRepository: scalixity-dev/PMS_Frontend
Length of output: 1763
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate lease service
LEASE_FILE="$(fd -i 'lease\.service\.ts' src 2>/dev/null | head -n1 || true)"
echo "---- lease.service.ts: $LEASE_FILE ----"
if [ -n "$LEASE_FILE" ]; then
echo
echo "---- BackendLease / property / address related type lines ----"
rg -n -C3 'interface BackendLease|property\?:|address\?:|streetAddress|city|stateRegion|zipCode|country' "$LEASE_FILE" | head -n 200
echo
echo "---- Show BackendLease interface block start (approx) ----"
# print from BackendLease line until CreateLeaseDto to capture address typing
START="$(rg -n 'interface BackendLease' "$LEASE_FILE" | head -n1 | cut -d: -f1)"
END="$(rg -n 'interface CreateLeaseDto' "$LEASE_FILE" | head -n1 | cut -d: -f1)"
if [ -n "$START" ] && [ -n "$END" ] && [ "$START" -lt "$END" ]; then
sed -n "${START},$((END-1))p" "$LEASE_FILE"
else
sed -n '1,260p' "$LEASE_FILE"
fi
else
echo "lease.service.ts not found under src."
fiRepository: scalixity-dev/PMS_Frontend
Length of output: 4043
Remove unnecessary as any when formatting lease.property.address (lines 78-79).
BackendLease.property.addressis already typed as{ streetAddress: string; city: string; ... } | null, so the(lease.property.address as any)casts bypass type safety; format using the typed value instead (e.g., narrow to a localaddrfromlease.property?.addressand referenceaddr.streetAddress/addr.citydirectly).
🧰 Tools
🪛 ESLint
[error] 78-78: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 78-78: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/userdashboard/features/Requests/UserNewRequest.tsx` around lines 78
- 79, The code is casting lease.property.address to any when formatting the
address; instead, narrow the typed value first (e.g., const addr =
lease.property?.address) and then reference addr.streetAddress and addr.city
directly without using as any, handling the null case (addr ?
`${addr.streetAddress ?? ''}, ${addr.city ?? ''}` : '') so you preserve type
safety and avoid the unnecessary cast on lease.property.address.
Source: Linters/SAST tools
Comment on lines
+119
to
+123
| uploaded.forEach((result, i) => { | ||
| const file = filesToUpload[i]; | ||
| const fileType: FileType = file.type.startsWith('video/') ? 'OTHER' : 'IMAGE'; | ||
| attachmentDtos.push({ fileUrl: result.url, fileType }); | ||
| }); |
Contributor
There was a problem hiding this comment.
Preserve attachment type mapping for non-image uploads.
At Line 121, every non-video file is labeled IMAGE. This mislabels document uploads and can break downstream attachment handling/rendering contracts.
Suggested fix
-const fileType: FileType = file.type.startsWith('video/') ? 'OTHER' : 'IMAGE';
+const fileType: FileType = file.type.startsWith('image/') ? 'IMAGE' : 'OTHER';📝 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.
Suggested change
| uploaded.forEach((result, i) => { | |
| const file = filesToUpload[i]; | |
| const fileType: FileType = file.type.startsWith('video/') ? 'OTHER' : 'IMAGE'; | |
| attachmentDtos.push({ fileUrl: result.url, fileType }); | |
| }); | |
| uploaded.forEach((result, i) => { | |
| const file = filesToUpload[i]; | |
| const fileType: FileType = file.type.startsWith('image/') ? 'IMAGE' : 'OTHER'; | |
| attachmentDtos.push({ fileUrl: result.url, fileType }); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/userdashboard/features/Requests/UserNewRequest.tsx` around lines
119 - 123, The current loop always marks non-video files as IMAGE; change the
mapping in the uploaded.forEach to derive FileType from the file MIME: use
file.type.startsWith('image/') ? FileType.IMAGE : file.type.startsWith('video/')
? FileType.OTHER : FileType.DOCUMENT (or the appropriate DOCUMENT enum member)
so documents are not misclassified; update the code around uploaded.forEach and
the attachmentDtos.push to use this conditional mapping and reference the
FileType enum.
Comment on lines
109
to
+120
| export interface CreateTaskDto { | ||
| description: string; // Required | ||
| dueDate: string; // Required, must be a valid date | ||
| assigneeLabel?: string; | ||
| fileUrl?: string; | ||
| } | ||
|
|
||
| export interface UpdateTaskDto { | ||
| description?: string; | ||
| dueDate?: string; | ||
| assigneeLabel?: string; | ||
| fileUrl?: string; |
Contributor
There was a problem hiding this comment.
Sync backend response types with the new task/call/meeting DTO fields.
CreateTaskDto/UpdateTaskDto add assigneeLabel? and fileUrl? (and the call/meeting DTOs add fileUrl?), but BackendTask, BackendCall, and BackendMeeting in src/services/lead.service.ts don’t include those optional properties—so src/pages/Dashboard/features/Leads/LeadDetail.tsx has to use (… as any).assigneeLabel/fileUrl at lines 246, 250, 258, 313, and 357. Add the same optional fields to the backend response interfaces and remove the casts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/lead.service.ts` around lines 109 - 120, The Backend response
interfaces are missing the new optional fields so TypeScript casts are used in
LeadDetail; update the interfaces BackendTask, BackendCall, and BackendMeeting
inside lead.service.ts to include assigneeLabel?: string and fileUrl?: string
(matching CreateTaskDto/UpdateTaskDto and the call/meeting DTOs) so the
properties are properly typed and the (… as any).assigneeLabel/fileUrl casts in
LeadDetail.tsx can be removed; ensure the types use the same optional string
shape as the DTOs.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx (1)
344-346:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUI description contradicts validation requirements.
The description states "Only the main category is required, but you can select a sub-category, issue and sub-issue to narrow down the request," but the validation logic at lines 548-550 actually requires
category,subCategory, andissue. The phrasing "you can select" implies these fields are optional, yet users will encounter validation errors when they leave them empty. This creates a confusing and frustrating user experience.📝 Suggested fix
Update the description to accurately reflect which fields are required:
- Search or select the issue category. Only the main category is required, but you can select a sub-category, issue and sub-issue to narrow down the request. + Search or select the issue category. Category, subcategory, and issue are required. You may optionally select a sub-issue to further narrow down the request. Select 'other' option if the category you are looking for isn't here.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx` around lines 344 - 346, The paragraph text in AdvancedRequestForm.tsx currently says only the main category is required but validation (in AdvancedRequestForm's validation logic that checks category, subCategory, and issue) requires category, subCategory and issue; update the paragraph copy to accurately state that category, sub-category and issue are required (and keep the note about selecting sub-options to narrow results and the 'other' option) so the UI matches the validation behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx`:
- Line 395: The Tailwind "important" prefixes use the old leading-! form in this
component; update all class strings that use leading exclamation marks (e.g.,
the buttonClassName prop and other className usages in AdvancedRequestForm) to
the v4 style trailing-! form — replace instances like "!bg-white !border-none
!rounded-md !py-3" with "bg-white! border-none! rounded-md! py-3!" and
conditional "!ring-2 !ring-red-400" with "ring-2! ring-red-400!"; search for and
update the occurrences referenced around the buttonClassName usage and the
similar class strings in this file (including the ones noted near the
errors.issue conditional and other lines mentioned) so all important utilities
use the postfix "!" syntax.
---
Outside diff comments:
In `@src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx`:
- Around line 344-346: The paragraph text in AdvancedRequestForm.tsx currently
says only the main category is required but validation (in AdvancedRequestForm's
validation logic that checks category, subCategory, and issue) requires
category, subCategory and issue; update the paragraph copy to accurately state
that category, sub-category and issue are required (and keep the note about
selecting sub-options to narrow results and the 'other' option) so the UI
matches the validation behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82d7b493-55c2-4a2a-bde2-e1a7835aae09
📒 Files selected for processing (1)
src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx
| options={getIssueOptions()} | ||
| placeholder={formData.subCategory ? "Select Issue" : "Select Subcategory First"} | ||
| required | ||
| buttonClassName={`!bg-white !border-none !rounded-md !py-3 ${errors.issue ? '!ring-2 !ring-red-400' : ''}`} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for Tailwind classes using old !-prefix syntax in className attributes
rg -n "className.*![a-z]" --type tsx --type ts --type jsx --type js -C2Repository: scalixity-dev/PMS_Frontend
Length of output: 98
🌐 Web query:
Tailwind CSS v4 important modifier syntax breaking changes
💡 Result:
In Tailwind CSS v4, the primary breaking change regarding the "important" modifier syntax is the shift in its required placement. While Tailwind v3 allowed you to mark a utility as important by placing the! prefix at the start of the utility (e.g.,!flex), v4 mandates that the! modifier be placed at the end of the utility class name (e.g., flex!) [1][2]. Key details regarding this change include: 1. Backwards Compatibility: The legacy prefix syntax (!utility) remains supported for compatibility purposes but is considered deprecated [1][2]. 2. Code Migration: The official Tailwind CSS upgrade tooling automatically handles the migration of this syntax, moving the! modifier from the start to the end of your classes [2][3]. 3. @apply behavior: When using @apply within your CSS, you should append the! to each individual utility class that you wish to mark as!important [4]. 4. Global Important Flag: Tailwind v4 supports a global important flag (equivalent to setting important: true in the old tailwind.config.js) by adding the important flag after the main @import statement in your CSS (e.g., @import "tailwindcss" important;) [5][6]. Note that this global flag behavior has seen refinements in v4, as it can sometimes affect declarations within @apply; if you encounter issues where @apply unintentionally becomes!important, it is recommended to ensure you are using the latest version of Tailwind, as adjustments have been made to address this specific behavior [7][6]. It is recommended to use the official Tailwind CSS upgrade tool, which performs these syntax transformations automatically during the migration process [2][3].
Citations:
- 1: https://tailwindcss.com/docs/upgrade-guide
- 2: https://combray.prose.sh/2025-11-30-tailwind-best-practices
- 3: https://wowhow.cloud/blogs/tailwind-css-v4-everything-changed-migration-guide-2026
- 4: Add initial codemod tooling tailwindlabs/tailwindcss#14434
- 5: Add support for
importantin v4 tailwindlabs/tailwindcss#14448 - 6: Using important automatically applies it to utilites set via @apply in v4 tailwindlabs/tailwindcss#18400
- 7: [v4] Breaking change: When using the important flag, @apply now also make all of the declarations !important tailwindlabs/tailwindcss#17649
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx"
echo "== Inspect around targeted line =="
sed -n '360,410p' "$FILE" | nl -ba | sed -n '350,430p' || true
echo
echo "== Find legacy important prefix used with ring utilities in this file =="
rg -n --glob='*.{ts,tsx,js,jsx}' "!ring-" "$FILE" || true
echo
echo "== Find other Tailwind-style important prefix usages in this file (className / template literals) =="
rg -n --glob='*.{ts,tsx,js,jsx}' "class(Name)?=.*![a-z]" "$FILE" || trueRepository: scalixity-dev/PMS_Frontend
Length of output: 757
🌐 Web query:
Tailwind CSS v4 upgrade guide important modifier placement !utility vs utility!
💡 Result:
In Tailwind CSS v4, the syntax for the important modifier has been updated to improve consistency and readability [1][2]. You must now place the exclamation mark (!) at the end of the utility class name (e.g., utility!) instead of at the beginning (!utility) [3][4][5]. Key details regarding this change: 1. Standard Syntax: The new convention is to append! to the end of the class, such as flex! or bg-red-500! [3][2]. 2. Compatibility: While the new syntax is the recommended standard, the legacy v3 syntax (placing! at the beginning) is still supported for backward compatibility, though it is considered deprecated [3][4][5]. 3. Reasoning: Moving the modifier to the end makes the utility easier to reason about, especially when using variants [1]. It also removes ambiguity regarding where the modifier belongs relative to variants (e.g., hover:bg-red-500! is the v4 pattern, compared to the v3 hover:!bg-red-500) [4][1]. 4. Automation: The official Tailwind CSS upgrade tool (npx @tailwindcss/upgrade) can automatically handle the migration of these modifiers across your codebase, along with other breaking changes [5][6]. If you are writing CSS using the @apply directive, you should also update your syntax to append the! to each utility class rather than using the!important flag at the end of the line [1]. Example: v3:
...
v4: ...
Citations:
- 1: Add initial codemod tooling tailwindlabs/tailwindcss#14434
- 2: https://dev.to/dataformathub/tailwind-css-v4-deep-dive-why-the-oxide-engine-changes-everything-in-2026-2595
- 3: https://tailwindcss.com/docs/upgrade-guide
- 4: https://gist.github.com/jumploops/fcc3c4b5130d5a672904f302d641ce43
- 5: https://combray.prose.sh/2025-11-30-tailwind-best-practices
- 6: https://blog.elunari.uk/tailwind-css-v4-migration-guide
Update Tailwind v4 “important” syntax to append ! to utilities
AdvancedRequestForm.tsx uses v3-style important prefixes in the conditional classes (and also for the base classes): !ring-2 !ring-red-400 (plus !bg-white !border-none !rounded-md !py-3) in lines 366, 378, and 395. Tailwind v4’s recommended syntax places ! at the end (e.g., ring-2! ring-red-400!, and bg-white! border-none! ...); the old !utility form is deprecated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/Dashboard/features/Maintenance/components/AdvancedRequestForm.tsx`
at line 395, The Tailwind "important" prefixes use the old leading-! form in
this component; update all class strings that use leading exclamation marks
(e.g., the buttonClassName prop and other className usages in
AdvancedRequestForm) to the v4 style trailing-! form — replace instances like
"!bg-white !border-none !rounded-md !py-3" with "bg-white! border-none!
rounded-md! py-3!" and conditional "!ring-2 !ring-red-400" with "ring-2!
ring-red-400!"; search for and update the occurrences referenced around the
buttonClassName usage and the similar class strings in this file (including the
ones noted near the errors.issue conditional and other lines mentioned) so all
important utilities use the postfix "!" syntax.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…download functionality across report pages
Summary by CodeRabbit
New Features
Bug Fixes
Chores