Conversation
📝 WalkthroughWalkthroughThis PR enhances dashboard filtering capabilities across multiple features by introducing new filter state props and implementing saved filter management. Additionally, the Tenants module undergoes substantial refactoring to replace mock data with backend-driven queries, data transformations, and CRUD operations via React Query hooks. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
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/Leases/Leases.tsx (1)
303-338: Second table section bypasses filtering and pagination.Lines 306-337 render a static slice of
MOCK_LEASES.slice(2, 4)which ignores the search, filter, and pagination logic applied to the first table. This creates inconsistent behavior where the second "Grove Street" group always shows the same items regardless of filters.🔎 Consider applying consistent filtering
Either:
- Remove the hardcoded second table section
- Group leases by property and apply filters to all groups
- Add a comment explaining this is intentional demo/placeholder UI
🧹 Nitpick comments (22)
src/pages/Dashboard/features/Equipments/Equipments.tsx (1)
256-278: Consider extracting placeholder filter logic.The pattern of ignoring
__no_items__placeholder values is repeated across multiple filter checks. Consider extracting a helper function to reduce duplication.🔎 Suggested helper function
// Helper to check if filter should match (ignoring placeholder values) const hasValidFilterValues = (filterValues: string[] | undefined): string[] => { return (filterValues || []).filter(v => v !== '__no_items__'); }; // Usage in filtering: const validCategories = hasValidFilterValues(filters.category); const matchesCategory = validCategories.length === 0 || validCategories.includes(item.category);src/pages/Dashboard/components/DashboardFilter.tsx (1)
73-80: Potential unnecessary re-renders due tofilterOptionsin dependency array.The
filterOptionsobject reference may change on every parent render (if not memoized), causing this effect to run unnecessarily. Consider using a more stable comparison or memoizingfilterOptionsin parent components.🔎 Alternative approach using JSON comparison
useEffect(() => { const updated: Record<string, string[]> = {}; Object.keys(filterOptions).forEach(key => { updated[key] = initialFilters[key] || []; }); - setSelectedFilters(updated); -}, [initialFilters, filterOptions]); + setSelectedFilters(prev => { + // Only update if actually different + const prevKeys = Object.keys(prev).sort().join(','); + const newKeys = Object.keys(updated).sort().join(','); + if (prevKeys !== newKeys) return updated; + + const hasChanges = Object.keys(updated).some( + key => JSON.stringify(prev[key]) !== JSON.stringify(updated[key]) + ); + return hasChanges ? updated : prev; + }); +}, [initialFilters, filterOptions]);Alternatively, ensure parent components memoize
filterOptionswithuseMemo.src/pages/Dashboard/features/Tenants/Tenants.tsx (2)
38-48: Placeholder filter values will be selectable but have no effect.Using
__no_items__as a placeholder value creates a selectable option that does nothing. Consider disabling these filter categories entirely in theDashboardFiltercomponent when no real options are available, or use adisabledproperty on the options.🔎 Consider marking options as disabled
If
DashboardFiltersupports adisabledproperty on options:const filterOptions: Record<string, FilterOption[]> = { tenantType: [ - { value: '__no_items__', label: 'No tenant types available' } + { value: '__no_items__', label: 'No tenant types available', disabled: true } ], ... };Alternatively, conditionally exclude empty filter categories from
filterOptions.
107-116: Async delete handler invoked without awaiting in JSX.At line 203,
handleDeleteTenant(tenant.id).catch(console.error)is called, but this pattern can lead to unhandled promise issues in some React versions. The handler itself is well-structured with confirmation and error handling.🔎 Consider wrapping in a non-async callback
onDelete={() => { - handleDeleteTenant(tenant.id).catch(console.error); + void handleDeleteTenant(tenant.id); }}The
voidoperator explicitly marks the promise as intentionally not awaited.src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx (1)
37-41: AddedkeyType,propertyId, andunitIdto transformation.The
keyTypefield preserves the backend value for filtering, which is correct. However,propertyIdandunitIdare added but not currently used in any filter or display logic. If not needed, consider removing them to avoid confusion.src/pages/Dashboard/features/Leases/Leases.tsx (1)
7-49: Mock data in production code should be replaced.The
MOCK_LEASESconstant appears to be placeholder data. Consider migrating to React Query hooks (similar to Tenants.tsx) for backend integration.src/pages/Dashboard/features/Listing/Listing.tsx (2)
186-196: DuplicateddaysListedcalculation logic.The same date calculation appears twice: once for MULTI property units (lines 186-193) and once for SINGLE properties (lines 269-276). Consider extracting to a helper function.
🔎 Extract helper function
const calculateDaysListed = (listedAt: string | undefined): number | undefined => { if (!listedAt) return undefined; const listedDate = new Date(listedAt); const today = new Date(); const diffTime = today.getTime() - listedDate.getTime(); return Math.floor(diffTime / (1000 * 60 * 60 * 24)); };Also applies to: 269-279
442-442: Type assertionas anybypasses type safety.The
onFiltersChange={(newFilters) => setFilters(newFilters as any)}cast indicates a type mismatch betweenDashboardFilter's output and the component's specific filter state type.🔎 Use `Record` for filters state
Change the filters state type to match
DashboardFilter's output:-const [filters, setFilters] = useState<{ - status: string[]; - daysListed: string[]; - syndication: string[]; - bedrooms: string[]; - bathrooms: string[]; -}>({...}); +const [filters, setFilters] = useState<Record<string, string[]>>({ + status: [], + daysListed: [], + syndication: [], + bedrooms: [], + bathrooms: [] +});Then update
onFiltersChange:-onFiltersChange={(newFilters) => setFilters(newFilters as any)} +onFiltersChange={setFilters}src/pages/Dashboard/features/Units/Units.tsx (1)
255-322: Balance category calculation is complex and may have misleading naming.The "balance" filter actually categorizes by total monthly rent, not account balance. Consider renaming to
rentCategoryor similar for clarity.Additionally, the large
countryToCurrencyandcurrencyThresholdsmaps could be extracted to a shared utility.🔎 Consider renaming and extracting utilities
- Rename for clarity:
-balanceCategory: 'low' | 'medium' | 'high' = 'medium'; +rentCategory: 'low' | 'medium' | 'high' = 'medium';
- Extract currency mapping to a shared utility file for reuse across components.
src/pages/Dashboard/features/Tenants/components/TenantInsuranceSection.tsx (1)
7-8: UnusedtenantIdparameter.The
tenantIdprop is accepted but not used. This is acceptable for a placeholder component, but consider adding an ESLint disable comment or prefixing with underscore (_tenantId) to signal intentional non-use.🔎 Suggested fix
-const TenantInsuranceSection: React.FC<TenantInsuranceSectionProps> = ({ tenantId }) => { +const TenantInsuranceSection: React.FC<TenantInsuranceSectionProps> = ({ tenantId: _tenantId }) => {src/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsx (1)
23-26: Unused props and misleading empty state.Both
tenantIdandtenantprops are accepted but unused. Additionally, the empty state check on line 94 is unreachable becausetransactionsis hard-coded with data. Consider either using an empty array to test the empty state, or adding a comment clarifying this is intentional mock data.🔎 Suggested fix to make empty state testable
-const TenantTransactionsSection = ({ tenantId, tenant }: TenantTransactionsSectionProps) => { +const TenantTransactionsSection = ({ tenantId: _tenantId, tenant: _tenant }: TenantTransactionsSectionProps) => { // Note: There's no direct API for tenant transactions yet // This is a placeholder that shows empty state - const transactions: Transaction[] = [ - { - id: 1, - status: 'Paid', - ... - }, - ... - ]; + // TODO: Replace with actual API call using tenantId + const transactions: Transaction[] = [];src/pages/Dashboard/features/Tenants/components/TenantApplicationsSection.tsx (2)
55-63: MovestatusMapoutside the render loop.
statusMapis a static object but is recreated for each application in the.map()callback. Move it outside the component for better performance.🔎 Proposed fix
+const STATUS_MAP: Record<string, string> = { + 'APPROVED': 'Approved', + 'SUBMITTED': 'Pending', + 'UNDER_REVIEW': 'Pending', + 'DRAFT': 'Draft', + 'REJECTED': 'Rejected', + 'WITHDRAWN': 'Withdrawn', +}; + const TenantApplicationsSection = ({ tenantId, tenantUserId }: TenantApplicationsSectionProps) => { // ... inside map callback: - const statusMap: Record<string, string> = { - 'APPROVED': 'Approved', - 'SUBMITTED': 'Pending', - 'UNDER_REVIEW': 'Pending', - 'DRAFT': 'Draft', - 'REJECTED': 'Rejected', - 'WITHDRAWN': 'Withdrawn', - }; - const status = statusMap[app.status] || 'Pending'; + const status = STATUS_MAP[app.status] || 'Pending';
6-8: UnusedtenantIdprop.The
tenantIdprop is defined but onlytenantUserIdis used for filtering. Consider removingtenantIdfrom the interface if not needed, or prefix with underscore to indicate intentional non-use.src/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsx (2)
17-20: UnusedtenantIdprop.The
tenantIdprop is accepted but not used. Consider prefixing with underscore (_tenantId) to indicate intentional non-use until API integration is complete.
98-104: Empty click handler on View button.The View button has an empty
onClickhandler. Consider either implementing navigation to the request detail or removing the button until the functionality is ready.🔎 Proposed fix
<button className="bg-[#3A6D6C] text-white px-6 py-2 rounded-full text-xs font-medium hover:bg-[#2c5251] transition-colors shadow-[inset_0_4px_2px_rgba(0,0,0,0.1)] flex items-center gap-2" - onClick={() => { }} + onClick={() => console.log('TODO: Navigate to request detail', request.id)} > <Eye className="w-4 h-4" /> View </button>src/pages/Dashboard/features/Tenants/components/TenantLeasesSection.tsx (1)
19-32: Use React Query instead of direct service calls for consistency.This component uses
useState+useEffectfor data fetching whileTenantApplicationsSectionuses React Query (useGetAllApplications). Consider using a similar React Query hook for consistency, caching benefits, and better error/loading state management.🔎 Example refactor
-import { useMemo, useEffect, useState } from 'react'; -import { leasingService, type BackendLeasing } from '../../../../../services/leasing.service'; +import { useMemo } from 'react'; +import { useGetAllLeasings } from '../../../../../hooks/useLeasingQueries'; +import type { BackendLeasing } from '../../../../../services/leasing.service'; const TenantLeasesSection = ({ tenantId, tenant }: TenantLeasesSectionProps) => { const navigate = useNavigate(); - const [allLeases, setAllLeases] = useState<BackendLeasing[]>([]); - const [isLoading, setIsLoading] = useState(true); - - useEffect(() => { - const fetchLeases = async () => { - try { - setIsLoading(true); - const leases = await leasingService.getAll(); - setAllLeases(leases); - } catch (error) { - console.error('Failed to fetch leases:', error); - } finally { - setIsLoading(false); - } - }; - fetchLeases(); - }, []); + const { data: allLeases = [], isLoading, error } = useGetAllLeasings();src/pages/Dashboard/features/Tenants/components/TenantProfileSection.tsx (4)
73-87: File extension validation not enforced.While MIME type is validated, the file extension is not checked. MIME types can be spoofed. Consider adding extension validation for defense in depth.
🔎 Proposed fix
const handleFileSelect = (e: React.ChangeEvent<HTMLInputElement>) => { const file = e.target.files?.[0]; if (file) { // Validate file type if (!allowedFileTypes.includes(file.type)) { alert(`Invalid file type. Please select one of the following: ${allowedFileExtensions.join(', ')}`); if (fileInputRef.current) { fileInputRef.current.value = ''; } setSelectedFile(null); return; } + // Also validate file extension + const fileExtension = '.' + file.name.split('.').pop()?.toLowerCase(); + if (!allowedFileExtensions.includes(fileExtension)) { + alert(`Invalid file extension. Please select one of the following: ${allowedFileExtensions.join(', ')}`); + if (fileInputRef.current) { + fileInputRef.current.value = ''; + } + setSelectedFile(null); + return; + } setSelectedFile(file); } };
129-135: DuplicateformatDateutility.A
formatDateutility already exists atsrc/utils/formatDate.ts. Consider reusing it for consistency, or if the format differs, consider extending the existing utility.
164-164: Avoid usinganytype.The loop variables (
contact,pet,vehicle) are typed asany. The types are already defined inTenantProfileSectionPropsinterface - use those instead.🔎 Proposed fix
- {tenant.emergencyContacts.map((contact: any, index: number) => ( + {tenant.emergencyContacts.map((contact, index) => (TypeScript will infer the correct type from
tenant.emergencyContactswhich is already typed in the interface.
118-127: Add loading state during document deletion.The delete operation doesn't show a loading indicator while the mutation is in progress. Users might click multiple times or be confused about the state.
🔎 Proposed fix
+ const [deletingDocId, setDeletingDocId] = useState<string | null>(null); + const handleDeleteDocument = async (documentId: string) => { if (window.confirm('Are you sure you want to delete this document?')) { try { + setDeletingDocId(documentId); await deleteDocumentMutation.mutateAsync(documentId); } catch (error) { console.error('Failed to delete document:', error); alert(`Failed to delete document: ${error instanceof Error ? error.message : 'Unknown error'}`); + } finally { + setDeletingDocId(null); } } }; // In the delete button JSX: - <button - onClick={() => handleDeleteDocument(doc.id)} - className="p-2 text-red-500 hover:bg-red-50 rounded-full transition-colors" - title="Delete document" - > - <Trash2 className="w-4 h-4" /> - </button> + <button + onClick={() => handleDeleteDocument(doc.id)} + disabled={deletingDocId === doc.id} + className="p-2 text-red-500 hover:bg-red-50 rounded-full transition-colors disabled:opacity-50" + title="Delete document" + > + {deletingDocId === doc.id ? ( + <Loader2 className="w-4 h-4 animate-spin" /> + ) : ( + <Trash2 className="w-4 h-4" /> + )} + </button>src/pages/Dashboard/features/Tenants/TenantDetail.tsx (2)
29-29: Hardcoded external image URL for default avatar.Using an external Unsplash URL as default image could:
- Create privacy concerns (external requests)
- Break if the URL becomes unavailable
- Slow down page loads for users with slow connections
Consider using a local placeholder image or inline SVG.
250-259: Fragile scroll behavior usingsetTimeout.Using
setTimeoutwith a fixed 100ms delay to wait for render is fragile and may not work reliably across different devices/network conditions. Consider using a ref callback oruseLayoutEffectinstead.🔎 Alternative approach using ref
+ const profileSectionRef = useRef<HTMLDivElement>(null); + // In the onClick handler: - onClick={() => { - setActiveTab('profile'); - // Scroll to profile section after a brief delay to ensure tab is rendered - setTimeout(() => { - const profileSection = document.getElementById('profile-section'); - if (profileSection) { - profileSection.scrollIntoView({ behavior: 'smooth', block: 'start' }); - } - }, 100); - }} + onClick={() => { + setActiveTab('profile'); + profileSectionRef.current?.scrollIntoView({ behavior: 'smooth', block: 'start' }); + }} // In JSX: - <div id="profile-section"> + <div ref={profileSectionRef}>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
src/pages/Dashboard/components/DashboardFilter.tsxsrc/pages/Dashboard/features/Application/Application.tsxsrc/pages/Dashboard/features/Equipments/Equipments.tsxsrc/pages/Dashboard/features/KeysLocks/KeysLocks.tsxsrc/pages/Dashboard/features/Leads/leads.tsxsrc/pages/Dashboard/features/Leases/Leases.tsxsrc/pages/Dashboard/features/Listing/Listing.tsxsrc/pages/Dashboard/features/Tasks/Tasks.tsxsrc/pages/Dashboard/features/Tenants/TenantDetail.tsxsrc/pages/Dashboard/features/Tenants/Tenants.tsxsrc/pages/Dashboard/features/Tenants/components/TenantApplicationsSection.tsxsrc/pages/Dashboard/features/Tenants/components/TenantCard.tsxsrc/pages/Dashboard/features/Tenants/components/TenantInsuranceSection.tsxsrc/pages/Dashboard/features/Tenants/components/TenantLeasesSection.tsxsrc/pages/Dashboard/features/Tenants/components/TenantProfileSection.tsxsrc/pages/Dashboard/features/Tenants/components/TenantRequestsSection.tsxsrc/pages/Dashboard/features/Tenants/components/TenantTransactionsSection.tsxsrc/pages/Dashboard/features/Units/Units.tsxsrc/pages/Dashboard/features/Units/components/UnitGroupCard.tsxsrc/pages/Dashboard/features/Units/components/UnitItem.tsx
🧰 Additional context used
🧬 Code graph analysis (11)
src/pages/Dashboard/features/Tenants/TenantDetail.tsx (2)
src/services/tenant.service.ts (1)
BackendTenantProfile(4-32)src/hooks/useTenantQueries.ts (2)
useDeleteTenant(116-130)useGetTenant(36-45)
src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Tenants/components/TenantLeasesSection.tsx (1)
src/services/leasing.service.ts (2)
BackendLeasing(4-29)leasingService(298-298)
src/pages/Dashboard/features/Tenants/components/TenantApplicationsSection.tsx (2)
src/hooks/useApplicationQueries.ts (1)
useGetAllApplications(17-26)src/services/application.service.ts (1)
BackendApplication(5-62)
src/pages/Dashboard/features/Equipments/Equipments.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Tasks/Tasks.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
SavedFilter(10-13)
src/pages/Dashboard/features/Units/Units.tsx (1)
src/services/listing.service.ts (1)
BackendListing(3-75)
src/pages/Dashboard/features/Application/Application.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Tenants/Tenants.tsx (3)
src/hooks/useTenantQueries.ts (2)
useDeleteTenant(116-130)useGetAllTenants(22-31)src/services/tenant.service.ts (2)
Tenant(78-84)tenantService(484-484)src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Tenants/components/TenantProfileSection.tsx (2)
src/hooks/useTenantQueries.ts (3)
useGetTenantDocuments(179-188)useUploadTenantDocument(152-174)useDeleteTenantDocument(193-205)src/utils/formatDate.ts (1)
formatDate(1-10)
src/pages/Dashboard/features/Leases/Leases.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
🔇 Additional comments (45)
src/pages/Dashboard/features/Units/components/UnitItem.tsx (1)
16-16: LGTM!The new optional
hasDraftListingfield extends theUnitinterface to support draft listing tracking. This aligns with the broader filtering and listing-draft support being introduced across the dashboard.src/pages/Dashboard/features/Tasks/Tasks.tsx (3)
40-51: LGTM!Good implementation of saved filter tracking with
activeSavedFilterstate. The initial saved filter correctly usesdate: ['today']which aligns with the available filter options defined infilterOptions.date.
196-204: LGTM!The
handleSelectSavedFilterandhandleClearSavedFilterhandlers correctly manage the saved filter state. SettingactiveSavedFilteron selection and clearing both filters and the active name on clear provides proper synchronization.
410-423: LGTM!The DashboardFilter integration correctly passes
onClearSavedFilter,activeSavedFilter, andinitialFiltersprops, enabling bidirectional synchronization between the filter UI and the saved filter state.src/pages/Dashboard/features/Tenants/components/TenantCard.tsx (2)
12-12: LGTM!Good addition of the optional
onDeletecallback prop, enabling parent components to handle deletion with their own confirmation flow.Also applies to: 21-22
51-61: No action needed—parent component properly implements confirmation flow.The parent component (Tenants.tsx) correctly handles confirmation before triggering deletion. The
handleDeleteTenantfunction (line 107-116) callswindow.confirmand only proceeds with the deletion mutation if the user confirms. TheonDeletecallback (line 202-203) invokes this confirmation-protected function, ensuring users are always prompted before deletion occurs.src/pages/Dashboard/features/Units/components/UnitGroupCard.tsx (1)
15-16: LGTM!The new optional fields
propertyStatusandbalanceCategoryextend theUnitGroupinterface to support the expanded filtering dimensions being introduced across the Units feature.src/pages/Dashboard/features/Leads/leads.tsx (3)
91-110: LGTM!Good filtering of archived, removed, and inactive listings from the dropdown options. This ensures users only see relevant, active listings when filtering leads.
163-176: LGTM!The optional chaining (
?.length) provides safe handling when filter arrays are undefined. The filtering logic correctly matches leads against each selected filter criterion.
346-354: LGTM!The DashboardFilter integration with
initialFiltersandshowClearAllaligns with the broader pattern across dashboard features.src/pages/Dashboard/features/Equipments/Equipments.tsx (4)
101-113: LGTM!Good extension of filter state to include
subcategoryandunitdimensions, providing more granular filtering capabilities.
142-146: LGTM!Good practice to preserve the original
backendStatusfor filtering while using the mappedstatusfor display. This ensures accurate filtering against backend enum values.
174-209: LGTM!The memoized unique collections (
uniqueSubcategories,uniqueUnits) efficiently derive filter options from the transformed equipment data with proper type guards.
362-369: LGTM!The DashboardFilter integration correctly passes
initialFiltersandshowClearAll, aligning with the broader dashboard filtering pattern.src/pages/Dashboard/features/Application/Application.tsx (4)
37-62: LGTM!Good enrichment of the transformed card data with
propertyUnit,propertyId,unitId, andbackendStatus. The property/unit combination logic correctly handles both SINGLE and MULTI property types.
87-108: LGTM!The memoized
uniquePropertyUnitsefficiently derives filter options from applications with proper null filtering.
158-166: LGTM!The property/unit filter correctly ignores the
__no_items__placeholder value and only applies filtering when valid selections exist.
229-237: LGTM!The DashboardFilter integration correctly passes
initialFiltersandshowClearAll, maintaining consistency with other dashboard features.src/pages/Dashboard/components/DashboardFilter.tsx (4)
26-27: LGTM!Good extension of the public API with
onClearSavedFilterandactiveSavedFilterprops to support saved filter state management from parent components.
96-99: LGTM!Good UX to clear the active saved filter when the user manually changes filters, as the current selection no longer matches the saved preset.
163-190: LGTM!The active saved filter badge provides clear visual feedback and the inline clear button offers a convenient way to reset. Good accessibility with the
titleattribute.
207-224: LGTM!Good handling of the
__no_items__placeholder values by disabling the checkbox and updating styling to indicate non-interactivity.src/pages/Dashboard/features/Tenants/Tenants.tsx (5)
1-8: LGTM!Imports are correctly structured with React Query hooks (
useGetAllTenants,useDeleteTenant) and UI components. ThetenantServiceimport provides the transformation utility.
62-85: Filter logic correctly ignores placeholder values.The filtering implementation properly checks for and ignores
__no_items__placeholder values, ensuring they don't affect the filter results. The search matches against name, email, and phone fields appropriately.
174-188: Loading and error states are well-implemented.The loading spinner with
Loader2and the error display with proper error message extraction provide good UX. The mutual exclusivity check (error && !isLoading) prevents showing error during loading.
191-217: Tenant grid rendering logic is correct.The conditional rendering for empty states correctly distinguishes between "no tenants found" (no data) and "no tenants match your filters" (filtered out). Props are explicitly passed to
TenantCardmatching the expected interface.
14-26: Remove this comment. ThetenantServiceimport is actively used on line 25.The code calls
tenantService.transformTenant()and the method exists insrc/services/tenant.service.tswith proper edge case handling for null/undefined values (email, phone, and image all have fallback values).Likely an incorrect or invalid review comment.
src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx (3)
66-94: Comprehensive filtering logic implemented correctly.The filter logic properly handles all filter dimensions (search, status, keyType, property, assignee) with appropriate empty-state checks using optional chaining and length validation.
104-129: Filter options and labels are well-structured.Status and keyType options use consistent backend values matching the filtering logic. Dynamic options for property and assignee integrate properly.
195-196: DashboardFilter props added for consistency.Adding
initialFiltersandshowClearAllaligns with the pattern used across other dashboard features.src/pages/Dashboard/features/Leases/Leases.tsx (3)
57-69: Filter state and placeholder options align with the codebase pattern.Using
Record<string, string[]>provides flexibility, and__no_items__placeholders indicate filters without available data.
83-105: Search now includes lease number, filter logic correctly ignores placeholders.The search extension to include
lease.lease.toString()is good for numeric lease IDs. The placeholder ignore logic is consistent with other files.
200-202: DashboardFilter integration is consistent.Props
initialFiltersandshowClearAllare correctly passed.src/pages/Dashboard/features/Listing/Listing.tsx (5)
22-23: Interface extended withdaysListedandisSyndicated.Good additions to track listing age and syndication status for filtering.
39-46: Filter state expanded for bedrooms and bathrooms.Correctly initialized with empty arrays.
94-116: Filter options for bedrooms/bathrooms added.Options include "5+" and "4+" for upper bounds, which the filtering logic handles correctly.
362-400: Filter logic handles all new dimensions correctly.The bedrooms/bathrooms filtering properly handles the "5+"/"4+" cases with range checks. The combined filter predicate requires all conditions to match.
443-445: DashboardFilter props correctly added.
initialFiltersandshowClearAllare properly passed for filter state synchronization.src/pages/Dashboard/features/Units/Units.tsx (5)
45-90: Listings map restructured for comprehensive listing tracking.The refactor properly separates active and draft listings at both property and unit levels. The logic correctly prioritizes the most recent listing when duplicates exist.
125-132: Property status mapping with fallback.The status mapping defaults to
'archived'for unknown or missing statuses. This is a safe default, but consider if'inactive'or another status would be more appropriate fornull/undefinedbackend values.
339-377: Enhanced filtering logic is comprehensive.The filter implementation properly handles all new dimensions including status, occupancy, propertyType, marketingStatus, and balance. The marketing status logic correctly derives state from listing presence.
399-431: Filter options and labels updated consistently.The options align with the filtering logic and backend data structures.
470-471: DashboardFilter props correctly added.Consistent with other dashboard feature components.
src/pages/Dashboard/features/Tenants/TenantDetail.tsx (2)
14-67: LGTM - Well-structured data transformation.The
transformTenantForDetailfunction cleanly transforms backend data to the UI format with appropriate fallbacks for missing fields. The null coalescing and optional chaining are used correctly.
137-166: LGTM - Good loading and error state handling.The component properly handles loading and error states with appropriate UI feedback. The error state includes a useful action button to navigate back.
| // Get unique properties and assignees for filter options | ||
| const propertyOptions = useMemo(() => { | ||
| const uniqueProperties = Array.from(new Set(transformedKeys.map(k => k.property).filter(Boolean))); | ||
| return uniqueProperties.map(prop => ({ value: prop, label: prop })); | ||
| }, [transformedKeys]); | ||
|
|
||
| const assigneeOptions = useMemo(() => { | ||
| const uniqueAssignees = Array.from(new Set(transformedKeys.map(k => k.assignee).filter(Boolean))); | ||
| return uniqueAssignees.map(assignee => ({ value: assignee, label: assignee })); | ||
| }, [transformedKeys]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, search for "__no_items__" as suggested in the review
rg -n "__no_items__" --type=tsx --type=tsRepository: scalixity-dev/PMS_Frontend
Length of output: 97
🏁 Script executed:
# Also search for common empty state patterns in filter implementations
rg -n "no.*option|empty.*filter|placeholder" --type=tsx --type=ts -i | head -50Repository: scalixity-dev/PMS_Frontend
Length of output: 97
🏁 Script executed:
# Let's examine the actual KeysLocks.tsx file to understand the context
wc -l src/pages/Dashboard/features/KeysLocks/KeysLocks.tsxRepository: scalixity-dev/PMS_Frontend
Length of output: 126
🏁 Script executed:
# Get the full context around lines 55-64, including how filters are used
cat -n src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx | sed -n '40,80p'Repository: scalixity-dev/PMS_Frontend
Length of output: 2489
🏁 Script executed:
# Search for "__no_items__" without type filter
rg -n "__no_items__"Repository: scalixity-dev/PMS_Frontend
Length of output: 2757
🏁 Script executed:
# Search for how filters are rendered in KeysLocks.tsx - look for more context
cat -n src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx | sed -n '100,150p'Repository: scalixity-dev/PMS_Frontend
Length of output: 2163
🏁 Script executed:
# Look for Select or dropdown components using propertyOptions/assigneeOptions
cat -n src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx | sed -n '150,220p'Repository: scalixity-dev/PMS_Frontend
Length of output: 3886
🏁 Script executed:
# Search for other filter implementations that might show empty state handling
rg -n "propertyOptions|assigneeOptions" src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx -A 3 -B 1Repository: scalixity-dev/PMS_Frontend
Length of output: 851
🏁 Script executed:
# Look for common empty state patterns in Select/filter components
rg -n "no.*options|isMulti|isClearable" src/pages --type=js --type=jsx 2>/dev/null | head -20Repository: scalixity-dev/PMS_Frontend
Length of output: 52
Add empty state placeholder for dynamic filter options to match established pattern.
The propertyOptions and assigneeOptions will be empty arrays if transformedKeys is empty, making those filters unusable. Other dashboard features (Equipments, Tenants, Leases, Application) handle this by adding a placeholder option with { value: '__no_items__', label: 'No X available' } when filter arrays are empty, then filtering out the placeholder during filtering logic. Apply the same pattern to maintain consistency.
| <div className="bg-[#7BD747] text-white px-4 py-2 rounded-full text-xs font-medium shadow-[inset_0_4px_1px_rgba(0,0,0,0.1)]"> | ||
| {status} | ||
| </div> |
There was a problem hiding this comment.
Status badge always shows green regardless of actual status.
The status badge on line 73 uses a fixed green color (#7BD747) for all statuses, but rejected/withdrawn applications should display differently. This creates a misleading UI where rejected applications appear successful.
🔎 Proposed fix
+ const getStatusColor = (status: string) => {
+ switch (status) {
+ case 'Approved': return 'bg-[#7BD747]';
+ case 'Rejected': return 'bg-red-500';
+ case 'Withdrawn': return 'bg-gray-500';
+ case 'Draft': return 'bg-gray-400';
+ default: return 'bg-orange-500'; // Pending states
+ }
+ };
+
return (
<div key={app.id} className="bg-[#F6F6F8] rounded-[2rem] p-6 shadow-lg">
<div className="flex items-center justify-between">
<div>
<h3 className="text-lg font-bold text-gray-800 mb-2">{applicantName}</h3>
<p className="text-sm text-gray-600">Applied: {applicationDate}</p>
<p className="text-sm text-gray-600">Status: {status}</p>
</div>
- <div className="bg-[#7BD747] text-white px-4 py-2 rounded-full text-xs font-medium shadow-[inset_0_4px_1px_rgba(0,0,0,0.1)]">
+ <div className={`${getStatusColor(status)} text-white px-4 py-2 rounded-full text-xs font-medium shadow-[inset_0_4px_1px_rgba(0,0,0,0.1)]`}>
{status}
</div>
</div>
</div>
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="bg-[#7BD747] text-white px-4 py-2 rounded-full text-xs font-medium shadow-[inset_0_4px_1px_rgba(0,0,0,0.1)]"> | |
| {status} | |
| </div> | |
| const getStatusColor = (status: string) => { | |
| switch (status) { | |
| case 'Approved': return 'bg-[#7BD747]'; | |
| case 'Rejected': return 'bg-red-500'; | |
| case 'Withdrawn': return 'bg-gray-500'; | |
| case 'Draft': return 'bg-gray-400'; | |
| default: return 'bg-orange-500'; // Pending states | |
| } | |
| }; | |
| return ( | |
| <div key={app.id} className="bg-[#F6F6F8] rounded-[2rem] p-6 shadow-lg"> | |
| <div className="flex items-center justify-between"> | |
| <div> | |
| <h3 className="text-lg font-bold text-gray-800 mb-2">{applicantName}</h3> | |
| <p className="text-sm text-gray-600">Applied: {applicationDate}</p> | |
| <p className="text-sm text-gray-600">Status: {status}</p> | |
| </div> | |
| <div className={`${getStatusColor(status)} text-white px-4 py-2 rounded-full text-xs font-medium shadow-[inset_0_4px_1px_rgba(0,0,0,0.1)]`}> | |
| {status} | |
| </div> | |
| </div> | |
| </div> | |
| ); |
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/Tenants/components/TenantApplicationsSection.tsx
around lines 73 to 75, the status badge uses a hard-coded green background
(#7BD747) so all statuses appear successful; replace the fixed color with a
status-to-class mapping (normalize status string then map e.g. approved/accepted
-> green classes, pending/in-review -> yellow/amber classes, rejected/withdrawn
-> red classes, and a neutral/default color for unknown statuses) and use that
computed className for the badge while preserving existing
text/spacing/rounded/shadow styles so the badge color reflects the actual
status.
| } catch (error) { | ||
| console.error('Failed to fetch leases:', error); | ||
| } finally { |
There was a problem hiding this comment.
Silent error handling leaves users uninformed.
Errors are only logged to console. Users see an empty state without knowing a fetch failed. Add error state UI similar to the loading state.
🔎 Proposed fix
const TenantLeasesSection = ({ tenantId, tenant }: TenantLeasesSectionProps) => {
const navigate = useNavigate();
const [allLeases, setAllLeases] = useState<BackendLeasing[]>([]);
const [isLoading, setIsLoading] = useState(true);
+ const [error, setError] = useState<string | null>(null);
useEffect(() => {
const fetchLeases = async () => {
try {
setIsLoading(true);
+ setError(null);
const leases = await leasingService.getAll();
setAllLeases(leases);
} catch (error) {
console.error('Failed to fetch leases:', error);
+ setError('Failed to load leases. Please try again.');
} finally {
setIsLoading(false);
}
};
fetchLeases();
}, []);
+
+ if (error) {
+ return (
+ <div className="text-center py-12 bg-red-50 rounded-[2rem]">
+ <p className="text-red-600">{error}</p>
+ </div>
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (error) { | |
| console.error('Failed to fetch leases:', error); | |
| } finally { | |
| const TenantLeasesSection = ({ tenantId, tenant }: TenantLeasesSectionProps) => { | |
| const navigate = useNavigate(); | |
| const [allLeases, setAllLeases] = useState<BackendLeasing[]>([]); | |
| const [isLoading, setIsLoading] = useState(true); | |
| const [error, setError] = useState<string | null>(null); | |
| useEffect(() => { | |
| const fetchLeases = async () => { | |
| try { | |
| setIsLoading(true); | |
| setError(null); | |
| const leases = await leasingService.getAll(); | |
| setAllLeases(leases); | |
| } catch (error) { | |
| console.error('Failed to fetch leases:', error); | |
| setError('Failed to load leases. Please try again.'); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; | |
| fetchLeases(); | |
| }, []); | |
| if (error) { | |
| return ( | |
| <div className="text-center py-12 bg-red-50 rounded-[2rem]"> | |
| <p className="text-red-600">{error}</p> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/Tenants/components/TenantLeasesSection.tsx
around lines 25-27, the catch only logs errors to console leaving users with an
empty state; add an error state (e.g., useState<string | null> error) and set it
in the catch with a user-friendly message (and optionally the error.message),
ensure finally still clears loading as appropriate, and update the render to
show an error UI (reusing the loading state layout) when error is set — include
a retry action/button that re-invokes the fetch and clears the error when
retried.
| // Transform leases for display | ||
| // Note: BackendLeasing doesn't have tenantId, so we'll show all leases | ||
| // In a real scenario, leases would be linked to tenants through applications or a tenantId field | ||
| const leases = useMemo(() => { | ||
| return allLeases.map((lease: BackendLeasing) => { | ||
| const propertyName = lease.property?.propertyName || 'Unknown Property'; | ||
| const createdAt = new Date(lease.createdAt); | ||
| const date = createdAt.toLocaleDateString('en-GB', { day: '2-digit', month: 'short', year: 'numeric' }); | ||
| const time = createdAt.toLocaleTimeString('en-GB', { hour: '2-digit', minute: '2-digit', hour12: true }); | ||
|
|
||
| return { | ||
| id: lease.id, | ||
| tenantName: tenant.name, | ||
| tenantInitial: tenantInitial, | ||
| date: date, | ||
| time: time, | ||
| status: 'Active', // BackendLeasing doesn't have status, defaulting to Active | ||
| property: propertyName | ||
| }; | ||
| }); | ||
| }, [allLeases, tenant.name, tenantInitial]); |
There was a problem hiding this comment.
All leases displayed regardless of tenant - potential data exposure concern.
The comment on lines 38-39 acknowledges that BackendLeasing lacks tenantId, but the current implementation shows all leases to every tenant. This could expose unrelated lease information to users and is misleading since this is a "Tenant Leases Section".
Consider either:
- Filtering leases based on a relationship (e.g., through applications)
- Not rendering this section until proper tenant-lease linking is available
- Adding a clear UI indicator that these are "all leases" not "this tenant's leases"
| return { | ||
| id: Number(backendTenant.id) || 0, // Convert string ID to number for compatibility |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for tenant ID usage patterns and type definitions
rg -n "tenantId|tenant\.id" --type=ts --type=tsx -C 2 | head -100Repository: scalixity-dev/PMS_Frontend
Length of output: 97
🏁 Script executed:
# Look for Tenant interface/type definitions
rg -n "interface Tenant|type Tenant|backendTenant" --type=ts --type=tsx -A 5 | head -80Repository: scalixity-dev/PMS_Frontend
Length of output: 97
🏁 Script executed:
# Check the TenantDetail.tsx file directly
cat -n src/pages/Dashboard/features/Tenants/TenantDetail.tsx | head -60Repository: scalixity-dev/PMS_Frontend
Length of output: 3488
🏁 Script executed:
# Search for API/backend responses to understand data format
rg -n "id.*string|id.*number|uuid|UUID" --type=ts --type=tsx src/pages/Dashboard/features/Tenants/ -A 1 -B 1Repository: scalixity-dev/PMS_Frontend
Length of output: 97
🏁 Script executed:
# Find the tenant.service file to check the BackendTenantProfile type
find . -name "tenant.service*" -type fRepository: scalixity-dev/PMS_Frontend
Length of output: 102
🏁 Script executed:
# Search for tenant ID patterns more broadly
rg "tenantId|tenant\.id" -A 2 -B 2 | head -80Repository: scalixity-dev/PMS_Frontend
Length of output: 6475
🏁 Script executed:
# Look for BackendTenantProfile definition
rg "BackendTenantProfile|interface.*Tenant" -A 10 | head -100Repository: scalixity-dev/PMS_Frontend
Length of output: 5405
🏁 Script executed:
# Check if IDs are numeric or strings in responses
rg "\"id\".*:|id.*Number|id.*string" -A 1 -B 1 | head -80Repository: scalixity-dev/PMS_Frontend
Length of output: 5955
Remove unnecessary Number() conversion for tenant ID.
BackendTenantProfile.id is already a string type from the backend. Converting it with Number(backendTenant.id) || 0 will fail for non-numeric IDs (e.g., UUIDs), resulting in NaN || 0, which makes the ID become 0. This breaks all downstream operations since child components expect the actual string ID. Keep the ID as a string to match the backend type and the frontend Tenant interface definition.
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/Tenants/TenantDetail.tsx around lines 24-25,
remove the Number() conversion and preserve the backend string ID; replace "id:
Number(backendTenant.id) || 0" with a string-preserving expression such as "id:
backendTenant.id || ''" (or ensure the returned value matches the Tenant
interface, e.g., backendTenant.id ?? ''), so non-numeric IDs (UUIDs) are not
turned into 0 and downstream components receive the actual string ID.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/Dashboard/features/Tenants/Tenants.tsx (1)
216-229: Bug: Using wrong loading variable name.Line 216 uses
loadingbut the React Query hook returnsisLoading. This will cause the loading state to never show (sinceloadingwould be undefined/falsy after removing the duplicate declaration).🔎 Proposed fix
- {!loading && !error && ( + {!isLoading && !error && (Also update the
onDeleteSuccessprop on TenantCard (line 228) - if using React Query, the mutation'sonSuccesshandles cache invalidation automatically, sofetchTenantsreference is no longer needed:- onDeleteSuccess={fetchTenants} + onDelete={() => handleDeleteTenant(tenant.id)}src/pages/Dashboard/components/DashboardFilter.tsx (1)
182-263: Critical: Malformed JSX structure will cause parse/render errors.The static analyzer reports a parse error at line 263. Examining the code, there appears to be duplicate/misplaced JSX: lines 182-263 contain filter button and dropdown rendering code that appears outside the main desktop filter bar
<div>(which starts at line 214). This creates invalid JSX structure with orphaned elements.The active saved filter badge (lines 182-210) and filter dropdowns (lines 211-263) appear to be incorrectly placed between the mobile filter bar and desktop filter bar sections.
🔎 Analysis and suggested fix
The code structure should be:
- Mobile filter bar (lines 156-180) ✓
- Desktop filter bar starting at line 214
But lines 182-263 inject content that breaks this structure. The active saved filter badge (lines 184-210) should likely be inside the desktop filter bar's
<div className="flex items-center gap-2">section around line 265.Please verify the intended placement and restructure the JSX. The badge and dropdown code needs to be moved inside the appropriate container element.
{/* Mobile & Tablet Search & Filter Bar */} <div className="lg:hidden bg-white rounded-full flex items-center gap-2 p-2 mb-4 shadow-sm"> {/* ... mobile content ... */} </div> - <div className="flex items-center gap-2"> - {/* Display active saved filter badge */} - {activeSavedFilter && ( - {/* ... badge content ... */} - )} - {Object.keys(filterOptions).map((filterType) => ( - {/* ... misplaced dropdown code ... */} - ))} - </div> - {/* Desktop Filter Bar */} <div ref={dropdownRef} className="hidden lg:flex bg-[#3A6D6C] p-4 rounded-full items-center gap-4 mb-8 justify-between relative shadow-md"> + {/* Move badge and filter buttons inside here */}
♻️ Duplicate comments (1)
src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx (1)
59-68: Add empty state placeholder for dynamic filter options to match established pattern.The
propertyOptionsandassigneeOptionswill be empty arrays iftransformedKeysis empty, making those filters unusable. Other dashboard features handle this by adding a placeholder option with{ value: '__no_items__', label: 'No X available' }. Apply the same pattern for consistency.🔎 Proposed fix
const filterOptions: Record<string, FilterOption[]> = { status: [ { value: 'AVAILABLE', label: 'Available' }, { value: 'ISSUED', label: 'Issued' }, { value: 'LOST', label: 'Lost' }, { value: 'DAMAGED', label: 'Damaged' }, { value: 'INACTIVE', label: 'Inactive' }, ], keyType: [ { value: 'DOOR', label: 'Main Door' }, { value: 'MAILBOX', label: 'Mailbox' }, { value: 'GARAGE', label: 'Garage' }, { value: 'GATE', label: 'Gate' }, { value: 'STORAGE', label: 'Storage' }, { value: 'OTHER', label: 'Other' }, ], - property: propertyOptions, - assignee: assigneeOptions, + property: propertyOptions.length > 0 ? propertyOptions : [ + { value: '__no_items__', label: 'No properties available' } + ], + assignee: assigneeOptions.length > 0 ? assigneeOptions : [ + { value: '__no_items__', label: 'No assignees available' } + ], };Then update the filtering logic to ignore placeholders:
// Property filter - const matchesProperty = !filters.property?.length || - filters.property.includes(key.property); + const matchesProperty = !filters.property?.length || + filters.property.filter(v => v !== '__no_items__').length === 0 || + filters.property.includes(key.property); // Assignee filter - const matchesAssignee = !filters.assignee?.length || - filters.assignee.includes(key.assignee); + const matchesAssignee = !filters.assignee?.length || + filters.assignee.filter(v => v !== '__no_items__').length === 0 || + filters.assignee.includes(key.assignee);
🧹 Nitpick comments (5)
src/pages/Dashboard/features/Equipments/Equipments.tsx (1)
219-228: Consider consistency:propertyfilter uses empty array fallback while others use__no_items__placeholder.The
propertyfilter falls back to an empty array when no properties exist (Line 225), whilecategory,subcategory, andunituse the__no_items__placeholder pattern. This inconsistency means the property dropdown may appear empty rather than showing a helpful "No properties available" message.🔎 Suggested fix for consistency
- property: uniqueProperties.length > 0 ? uniqueProperties : [], + property: uniqueProperties.length > 0 ? uniqueProperties : [ + { value: '__no_items__', label: 'No properties available' } + ],src/pages/Dashboard/features/Tenants/Tenants.tsx (1)
132-141: Consider UX improvement: Replacewindow.confirmwith a styled modal.Using
window.confirmis functional but doesn't match the polished modal UI seen in other components (e.g.,DeleteConfirmationModalin Tasks, custom delete modal in Equipments). Consider using a consistent styled modal for better UX.src/pages/Dashboard/features/Listing/Listing.tsx (1)
369-374: Unlisted items are excluded when "Days Listed" filters are active.When any
daysListedfilter is selected, unlisted items (wherelisting.daysListedisundefined) will be filtered out. This may be intentional but could surprise users who expect unlisted items to appear alongside filtered listings.If unlisted items should be included regardless of the days filter, consider:
🔎 Proposed fix
- const matchesDaysListed = !filters.daysListed?.length || - (listing.daysListed !== undefined && ( + const matchesDaysListed = !filters.daysListed?.length || + listing.daysListed === undefined || // Include unlisted items + ( (filters.daysListed.includes('new') && listing.daysListed < 7) || (filters.daysListed.includes('recent') && listing.daysListed >= 7 && listing.daysListed <= 30) || (filters.daysListed.includes('old') && listing.daysListed > 30) - )); + );src/pages/Dashboard/features/Units/Units.tsx (2)
128-136: Consider handling 'INACTIVE' status explicitly.Per the
BackendListinginterface, property status can be'ACTIVE' | 'INACTIVE' | 'ARCHIVED' | null. ThestatusMapdoesn't include 'INACTIVE', so it falls back to 'archived'. If 'INACTIVE' should be treated differently, consider adding it to the map.🔎 Proposed fix
const statusMap: Record<string, 'active' | 'archived'> = { 'ACTIVE': 'active', 'ARCHIVED': 'archived', + 'INACTIVE': 'archived', // Explicitly map INACTIVE };
270-325: Some currencies lack dedicated thresholds.The
countryToCurrencymap includes currencies like NZD, CHF, ZAR, BRL, MXN that don't have entries incurrencyThresholds. These will fall back to USD thresholds, which may not be appropriate for their purchasing power.🔎 Proposed fix - add missing thresholds
const currencyThresholds: Record<string, { low: number; high: number }> = { 'USD': { low: 25000, high: 75000 }, 'CAD': { low: 33000, high: 100000 }, 'GBP': { low: 20000, high: 60000 }, 'EUR': { low: 23000, high: 69000 }, 'AUD': { low: 37000, high: 110000 }, + 'NZD': { low: 40000, high: 120000 }, 'INR': { low: 2000000, high: 6000000 }, 'CNY': { low: 180000, high: 540000 }, 'JPY': { low: 3500000, high: 10500000 }, + 'CHF': { low: 22000, high: 66000 }, 'SGD': { low: 33000, high: 100000 }, 'HKD': { low: 195000, high: 585000 }, 'AED': { low: 92000, high: 275000 }, 'SAR': { low: 94000, high: 280000 }, + 'ZAR': { low: 450000, high: 1350000 }, + 'BRL': { low: 125000, high: 375000 }, + 'MXN': { low: 430000, high: 1290000 }, };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
src/pages/Dashboard/components/DashboardFilter.tsxsrc/pages/Dashboard/features/Application/Application.tsxsrc/pages/Dashboard/features/Equipments/Equipments.tsxsrc/pages/Dashboard/features/KeysLocks/KeysLocks.tsxsrc/pages/Dashboard/features/Leads/leads.tsxsrc/pages/Dashboard/features/Leases/Leases.tsxsrc/pages/Dashboard/features/Listing/Listing.tsxsrc/pages/Dashboard/features/Tasks/Tasks.tsxsrc/pages/Dashboard/features/Tenants/Tenants.tsxsrc/pages/Dashboard/features/Units/Units.tsxsrc/pages/Dashboard/features/Units/components/UnitGroupCard.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- src/pages/Dashboard/features/Units/components/UnitGroupCard.tsx
- src/pages/Dashboard/features/Leads/leads.tsx
🧰 Additional context used
🧬 Code graph analysis (6)
src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Tenants/Tenants.tsx (3)
src/hooks/useTenantQueries.ts (2)
useDeleteTenant(116-130)useGetAllTenants(22-31)src/services/tenant.service.ts (2)
Tenant(78-84)tenantService(484-484)src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Leases/Leases.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Equipments/Equipments.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Application/Application.tsx (1)
src/pages/Dashboard/components/DashboardFilter.tsx (1)
FilterOption(5-8)
src/pages/Dashboard/features/Units/Units.tsx (1)
src/services/listing.service.ts (1)
BackendListing(3-75)
🪛 Biome (2.1.2)
src/pages/Dashboard/features/Tenants/Tenants.tsx
[error] 23-23: Shouldn't redeclare 'setFilters'. Consider to delete it or rename it.
'setFilters' is defined here:
(lint/suspicious/noRedeclare)
src/pages/Dashboard/components/DashboardFilter.tsx
[error] 263-263: Unexpected token. Did you mean {'}'} or }?
(parse)
🔇 Additional comments (33)
src/pages/Dashboard/features/Tasks/Tasks.tsx (3)
41-54: LGTM! Saved filter state management is well implemented.The
activeSavedFilterstate tracking and the updated saved filter default (date: ['today']instead offrequency: ['DAILY']) align with the DashboardFilter's new props. The initialization and state flow are correct.
209-217: LGTM! Clear and select handlers properly synchronize state.The
handleSelectSavedFiltercorrectly sets both the filters and the active filter name, whilehandleClearSavedFilterproperly resets both states.
418-432: LGTM! DashboardFilter integration is complete.All new props (
onClearSavedFilter,activeSavedFilter,initialFilters,showClearAll) are correctly wired to the component.src/pages/Dashboard/features/Equipments/Equipments.tsx (5)
102-114: LGTM! Extended filter state structure is correct.The filter state now properly supports the new subcategory and unit dimensions with proper initialization.
137-159: LGTM! Equipment transformation is well structured.The transformation correctly preserves
backendStatusfor filtering while mapping display status, and properly extractspropertyId,unit, andunitIdfields.
175-210: LGTM! Derived filter options are correctly computed.The
uniqueSubcategoriesanduniqueUnitsmemos properly filter out empty strings and create the expected{ value, label }structure.
244-281: LGTM! Filtering logic correctly handles backend status and placeholders.The filtering uses
backendStatusfor status matching and properly ignores__no_items__placeholder values across all filter dimensions.
363-370: LGTM! DashboardFilter integration is complete.The
initialFiltersandshowClearAllprops are properly passed to enable persistent filter state and the clear-all option.src/pages/Dashboard/features/Leases/Leases.tsx (3)
59-72: LGTM! Filter state and placeholder options are correctly implemented.The generic
Record<string, string[]>type aligns with the DashboardFilter API, and the__no_items__placeholders clearly communicate when no data is available for occupancy and property type filters.
85-107: LGTM! Filtering logic handles placeholders correctly.The search now includes tenant and lease fields, and the occupancy/propertyType filters properly ignore placeholder values when evaluating matches.
198-205: LGTM! DashboardFilter integration is complete.Props are correctly wired with
initialFiltersandshowClearAll.src/pages/Dashboard/features/Tenants/Tenants.tsx (1)
87-119: LGTM! Filtering and sorting logic is well implemented.The search covers name, email, and phone fields. The placeholder value handling correctly ignores
__no_items__values, and the sorting respects thesortOrderstate.src/pages/Dashboard/features/KeysLocks/KeysLocks.tsx (3)
36-57: LGTM! Extended key transformation is well structured.The transformation correctly preserves
keyTypefor filtering while displaying the mapped type, and includespropertyIdandunitIdfor potential future use.
70-98: LGTM! Filtering logic is comprehensive.The search includes assignee, and all filter dimensions (status, keyType, property, assignee) are correctly applied with AND logic.
220-227: LGTM! DashboardFilter integration is complete.Props are correctly wired with
initialFiltersandshowClearAll.src/pages/Dashboard/components/DashboardFilter.tsx (3)
26-27: LGTM! New props for saved filter management are well typed.The
onClearSavedFiltercallback andactiveSavedFilterstate prop enable parent components to manage saved filter state externally.Also applies to: 41-43
76-83: LGTM! Filter synchronization effect is correct.The effect properly syncs
selectedFilterswithinitialFilterswhen either changes, initializing missing keys to empty arrays.
110-113: LGTM! Saved filter clearing on manual changes is well implemented.Both
handleFilterToggleandhandleClearAllcorrectly clear the active saved filter when the user manually modifies filters, providing intuitive UX.Also applies to: 127-130
src/pages/Dashboard/features/Application/Application.tsx (4)
37-62: LGTM! Application transformation is well structured.The transformation correctly builds the
propertyUnitdisplay string (handling MULTI vs SINGLE property types), preservesbackendStatusfor potential future use, and includespropertyIdandunitId.
88-129: LGTM! Dynamic filter options and placeholders are correctly implemented.The
uniquePropertyUnitscomputation properly extracts and deduplicates property/unit combinations, and the fallback to__no_items__placeholder when empty maintains consistency with other dashboard features.
135-173: LGTM! Filtering logic correctly handles placeholders and status mapping.The search now includes
propertyUnit, thescreeningStatusfilter uses the correct mapped status, and thepropertyUnitsfilter properly ignores the__no_items__placeholder.
230-238: LGTM! DashboardFilter integration is complete.Props are correctly wired with
initialFiltersandshowClearAll.src/pages/Dashboard/features/Listing/Listing.tsx (6)
22-23: LGTM!Good addition of
daysListedandisSyndicatedfields to the interface. These are properly typed as optional, which correctly reflects that they're only populated for active listings.
40-47: LGTM!Filter state initialization with empty arrays for the new bedroom and bathroom filters is correct.
96-108: LGTM!Filter options for bedrooms and bathrooms are well-defined. The "5+" and "4+" special values align with the filtering logic in
filteredListings.
187-197: LGTM!Days listed calculation is correct. The logic properly handles cases where there's no active listing by leaving
daysListedas undefined.
377-379: LGTM!Syndication filter logic correctly treats unlisted items as "not syndicated" since
isSyndicatedis set tofalsefor them during transformation.
439-447: LGTM!The integration with
DashboardFilterproperly passesinitialFiltersand enablesshowClearAll, maintaining filter state consistency.src/pages/Dashboard/features/Units/Units.tsx (5)
48-93: LGTM!Good refactoring from a single
activeListingsMapto a structuredlistingsMapobject with four maps. This cleanly separates active vs draft and property vs unit listings while maintaining the "most recent by listedAt" logic.
364-373: LGTM!The marketing status filter logic correctly distinguishes between listed (active), unlisted (no listings), and draft states by checking unit-level listing flags.
402-426: LGTM!Filter options are well-defined and align with the filtering logic. The values match backend enums where applicable.
473-480: LGTM!Consistent integration with
DashboardFilter, matching the pattern used inListing.tsx.
327-340: No action required. TheUnitGroupinterface already includes bothpropertyStatus?: 'active' | 'archived'andbalanceCategory?: 'low' | 'medium' | 'high'fields (lines 15-16 of UnitGroupCard.tsx), which correctly align with the returned object.
| // Bathrooms filter | ||
| const matchesBathrooms = !filters.bathrooms?.length || | ||
| filters.bathrooms.some(filterValue => { | ||
| if (filterValue === '4+') { | ||
| return listing.bathrooms >= 4; | ||
| } | ||
| const filterBathrooms = parseInt(filterValue, 10); | ||
| return listing.bathrooms === filterBathrooms; | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full context of the Listing.tsx file around lines 391-399
wc -l src/pages/Dashboard/features/Listing/Listing.tsxRepository: scalixity-dev/PMS_Frontend
Length of output: 122
🏁 Script executed:
# Read the relevant section with context around lines 391-399
sed -n '370,420p' src/pages/Dashboard/features/Listing/Listing.tsxRepository: scalixity-dev/PMS_Frontend
Length of output: 2334
🏁 Script executed:
# Look for the bedrooms filter logic to compare (mentioned in the scratchpad)
sed -n '375,410p' src/pages/Dashboard/features/Listing/Listing.tsx | cat -nRepository: scalixity-dev/PMS_Frontend
Length of output: 1854
🏁 Script executed:
# Search for how listing data is structured and where bathrooms comes from
rg -n "bathrooms" src/pages/Dashboard/features/Listing/Listing.tsx -B 2 -A 2Repository: scalixity-dev/PMS_Frontend
Length of output: 2759
🏁 Script executed:
# Search for any data types or interfaces that define listing structure
rg -n "interface.*[Ll]isting|type.*[Ll]isting" --type ts -C 3 | head -50Repository: scalixity-dev/PMS_Frontend
Length of output: 4309
🏁 Script executed:
# Check if there are any tests for the filtering logic
find . -name "*.test.ts*" -o -name "*.spec.ts*" | xargs grep -l "bathrooms\|filter" 2>/dev/null | head -10Repository: scalixity-dev/PMS_Frontend
Length of output: 52
Fractional bathroom and bedroom values won't match filter options.
The filters use exact equality checks after parsing integer filter values, but listing data is loaded with parseFloat() which allows fractional values (e.g., 1.5, 2.5 bathrooms). A listing with 1.5 bathrooms won't match the "1" filter option. The same issue affects the bedrooms filter. Consider using Math.floor() on listing values before comparison so fractional values match their floor integer (e.g., 1.5 matches "1").
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/Listing/Listing.tsx around lines 391 to 399, the
bathroom (and similarly bedroom) filter compares parsed integer filter values to
listing.bathrooms which may be fractional, so 1.5 won't match the "1" filter;
change the comparisons to use Math.floor(listing.bathrooms) (and
Math.floor(listing.bedrooms) for the bedrooms filter) before comparing to parsed
integers, keep the existing '4+' branch checking >= 4, and ensure you still
parse filter strings with parseInt(filterValue, 10).
| const [searchQuery, setSearchQuery] = useState(''); | ||
| const [filters, setFilters] = useState<Record<string, string[]>>({}); | ||
| const [currentPage, setCurrentPage] = useState(1); | ||
| const itemsPerPage = 9; | ||
| const deleteTenantMutation = useDeleteTenant(); | ||
|
|
||
| // Fetch tenants using React Query | ||
| const { data: backendTenants = [], isLoading, error } = useGetAllTenants(); | ||
| const { sidebarCollapsed = false } = useOutletContext<{ sidebarCollapsed: boolean }>() ?? {}; | ||
| const [, setFilters] = useState<Record<string, string[]>>({}); | ||
|
|
||
| const handleSearchChange = (_search: string) => { | ||
| // console.log('Search:', search); | ||
| // Transform backend tenants to frontend format | ||
| const tenants: Tenant[] = useMemo(() => { | ||
| return backendTenants.map((tenant) => tenantService.transformTenant(tenant)); | ||
| }, [backendTenants]); |
There was a problem hiding this comment.
Critical: Duplicate state declarations will cause runtime errors.
The React Query integration adds new state (filters on line 15, tenants on line 26) but the old manual implementation still exists below (lines 58-79), causing duplicate declarations. The static analysis correctly flags line 23 redeclaring setFilters.
This will cause a runtime error: SyntaxError: Identifier 'setFilters' has already been declared.
🔎 Proposed fix: Remove duplicate declarations and old fetch logic
Remove lines 23, 58-79 (the duplicate state declarations and the old fetchTenants function):
const { sidebarCollapsed = false } = useOutletContext<{ sidebarCollapsed: boolean }>() ?? {};
- const [, setFilters] = useState<Record<string, string[]>>({});
// Transform backend tenants to frontend format
const tenants: Tenant[] = useMemo(() => {
return backendTenants.map((tenant) => tenantService.transformTenant(tenant));
}, [backendTenants]);
// ... other code ...
- const [tenants, setTenants] = useState<Tenant[]>([]);
- const [loading, setLoading] = useState(true);
- const [error, setError] = useState<string | null>(null);
-
- const fetchTenants = async () => {
- try {
- setLoading(true);
- setError(null);
- const backendTenants = await tenantService.getAll();
- const transformedTenants = backendTenants.map((tenant) => tenantService.transformTenant(tenant));
- setTenants(transformedTenants);
- } catch (err) {
- console.error('Error fetching tenants:', err);
- setError(err instanceof Error ? err.message : 'Failed to fetch tenants');
- } finally {
- setLoading(false);
- }
- };
-
- useEffect(() => {
- fetchTenants();
- }, []);Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (2.1.2)
[error] 23-23: Shouldn't redeclare 'setFilters'. Consider to delete it or rename it.
'setFilters' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In src/pages/Dashboard/features/Tenants/Tenants.tsx around lines 14 to 28, there
is a duplicate state declaration for filters (the second "const [, setFilters] =
useState..." on line ~23) that conflicts with the earlier "const [filters,
setFilters] = useState..." and the file still contains the old manual
fetchTenants logic further down (lines ~58-79); remove the duplicate state
declaration (the anonymous array destructure that redeclares setFilters) and
delete the obsolete fetchTenants implementation and any related manual state
handling (lines 58-79), then ensure the component uses the React Query data
(backendTenants -> tenants via tenantService.transformTenant) and the single
filters state declared at the top.
2 participants
Summary by CodeRabbit
Release Notes
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.