[ DIGIT-362 ] Property Tax Mobile UI #3
Conversation
…ct application designed for efficient property tax management by agents and citizens. Key features include map-based property selection, multi-step property forms, document uploads, application tracking, and integrated notifications. Agents can verify and manage property applications, while citizens can view, update, and submit their property details. The business purpose is to streamline property tax workflows, enhance transparency, and empower users with self-service capabilities, improving accuracy and service delivery for municipal property tax operations.
WalkthroughIntroduces a comprehensive mobile-first Property Tax frontend application built with React, TypeScript, and Vite. The PR establishes configuration, core app structure with routing and providers, shared components, feature modules for Agent and Citizen workflows, API integration via RTK Query, mock data for development, and extensive TypeScript models for type safety. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes This PR introduces over 100 new files spanning configuration, components, API integration, models, and utilities. While many follow repetitive patterns (similar API slice structure, comparable component templates), several files contain substantial business logic requiring careful attention:
Additionally, the large scope, heterogeneous file types (config, types, components, hooks, utilities), and interplay between Agent and Citizen features demand comprehensive review to ensure consistency and correctness.
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (58)
mobile-ui/src/app/features/Agent/components/BottomNavigation.tsx-27-30 (1)
27-30: Addtype="button"to prevent accidental form submits.
Line 89: default button type is"submit"; if this component is ever rendered inside a<form>, clicking a tab can trigger form submission.<button key={item.id} + type="button" className={`nav-item ${activeTab === item.id ? 'active' : ''}`} onMouseDown={(e) => (e.currentTarget as HTMLButtonElement).blur()} onClick={() => handleTabClick(item.id)} aria-label={item.label} >Also applies to: 89-100
mobile-ui/src/app/components/common/ConfirmationDialog.tsx-41-48 (1)
41-48: Handle MUIonClosereasons (backdrop vs escape) instead of always rejecting
Right nowonClose={onClose || onReject}treats backdrop-click / ESC the same as “Reject”. If “Reject” is a domain action (not just “cancel”), you likely want to close without triggering reject, or selectively ignore backdrop clicks.- <Dialog + <Dialog open={open} - onClose={onClose || onReject} // Close dialog on backdrop click or close button + onClose={(event, reason) => { + if (reason === 'backdropClick') return; // optional: prevent accidental dismiss + (onClose ?? onReject)(); + }} PaperProps={{ sx: confirmDialogPaperStyle }} maxWidth="xs" fullWidth hideBackdrop={false} >Also applies to: 50-56
mobile-ui/src/app/components/common/ConfirmationDialog.tsx-57-67 (1)
57-67: Addaria-labelledby/aria-describedbyfor dialog accessibility
The dialog content is visually clear, but screen readers won’t reliably associate the title/description without ARIA wiring.- <Dialog + <Dialog open={open} onClose={(event, reason) => { if (reason === 'backdropClick') return; // optional: prevent accidental dismiss (onClose ?? onReject)(); }} + aria-labelledby="confirmation-dialog-title" + aria-describedby="confirmation-dialog-description" PaperProps={{ sx: confirmDialogPaperStyle }} maxWidth="xs" fullWidth hideBackdrop={false} > ... - <DialogContent sx={{ p: 0, pt: 3.5 }}> + <DialogContent sx={{ p: 0, pt: 3.5 }}> {/* Dialog title */} - <Typography sx={confirmTitleStyle}>Confirm Comment</Typography> + <Typography id="confirmation-dialog-title" sx={confirmTitleStyle}> + Confirm Comment + </Typography> {/* Confirmation message */} - <Typography sx={confirmDescStyle}> + <Typography id="confirmation-dialog-description" sx={confirmDescStyle}> Are you sure you want to Confirm this request? </Typography>Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/features/Citizen/api/Property.slice.ts-1-26 (1)
1-26: Remove dead code or complete the Redux property state management implementation.The entire Redux Toolkit slice in
Property.slice.ts(lines 1–23) is commented out, and the companion hookuseProperty.tsis also completely commented out. Additionally, thepropertySliceis not registered in the Redux store configuration (store.ts), so even if the code were uncommented, it would not function.This appears to be incomplete work-in-progress migration from
PropertyContextto Redux/RTK Query. The file ends with a stray///marker on line 26 and lacks a trailing newline.Resolution:
- If completing the implementation: Uncomment all code in
Property.slice.ts(lines 1–23) anduseProperty.ts, register thepropertySliceinstore.ts, verify thatPropertyandPropertyStatemodel imports resolve correctly, remove the stray///marker, and add a trailing newline.- If not needed in this PR: Remove both
Property.slice.tsand the dead code fromuseProperty.ts, and open a separate issue or branch to complete the property state management refactoring.mobile-ui/src/app/components/NotificationCard.tsx-29-32 (1)
29-32: Guard localization lookups (avoid crash when messages/locale aren’t ready).
Right nowgetMessagesFromSession(...)!+messages['profile'][lang][...]will throw if session messages orlangkey is missing/late-loaded.- const lang = getAppLocale(); - const messages = getMessagesFromSession(isCitizenProfile ? 'CITIZEN' : 'AGENT')!; + const lang = getAppLocale(); + const role = isCitizenProfile ? 'CITIZEN' : 'AGENT'; + const messages = getMessagesFromSession(role); + const profileMsgs = + messages?.profile?.[lang] ?? + messages?.profile?.en ?? + {};Then replace usages like
messages['profile'][lang]['notification-settings']withprofileMsgs['notification-settings'] ?? 'Notification Settings'(same for the other labels).Also applies to: 49-66, 110-113, 162-165, 183-184, 203-204, 223-224
mobile-ui/src/app/components/NotificationCard.tsx-45-79 (1)
45-79: Agent categories aren’t localized (UI will show raw keys).
Citizen categories usemessages[...]; Agent categories currently hardcode'application-updates', etc.const notificationCategories = isCitizenProfile ? [ { key: 'bill-reminders', - title: messages['profile'][lang]['bill-reminders'], - desc: messages['profile'][lang]['bill-reminders-desc'], + title: profileMsgs['bill-reminders'], + desc: profileMsgs['bill-reminders-desc'], }, ... ] : [ { key: 'application-updates', - title: 'application-updates', - desc: 'application-updates-desc', + title: profileMsgs['application-updates'], + desc: profileMsgs['application-updates-desc'], }, { key: 'appointment-scheduling', - title: 'appointment-scheduling', - desc: 'appointment-scheduling-desc', + title: profileMsgs['appointment-scheduling'], + desc: profileMsgs['appointment-scheduling-desc'], }, ];Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/components/ProfileCard.tsx-215-237 (1)
215-237: Potential bug:wardmight be a string;.map(...)would iterate characters. Normalize tostring[].- {(ward || ['Ward Nos. 24 && 38 of BBMP', 'Ward Nos. 46 of BBMP']).map( + {((Array.isArray(ward) ? ward : ward ? [ward] : [])).map( (item: string, index: number) => ( <Typography key={index} fontWeight={300} fontSize={14} color={COLORS.text} > {item} </Typography> ) )}Also consider removing the hardcoded fallback ward strings (same issue as the hardcoded address).
mobile-ui/src/app/components/ProfileCard.tsx-43-47 (1)
43-47: HardensessionStorageparsing (try/catch + shape check) to prevent UI crash on malformed data.useEffect(() => { - const userString = sessionStorage.getItem('user'); - const user = userString ? JSON.parse(userString) : null; - setUserProfile(user); + const userString = sessionStorage.getItem('user'); + if (!userString) return setUserProfile(null); + try { + const parsed = JSON.parse(userString); + setUserProfile(parsed ?? null); + } catch { + setUserProfile(null); + } }, [isCitizenProfile, sessionUser]);Also:
isCitizenProfileisn’t used in the effect body—unless you expect the stored “user” payload to change when toggling profile type, you can likely drop it from the dependency array to avoid redundant parses.mobile-ui/src/app/components/ProfileCard.tsx-165-173 (1)
165-173: PII risk: showing Aadhaar (even partially) is likely non-compliant; avoid rendering or fully redact; don’t treat it asnumber.
- Aadhaar should generally not be displayed in UI unless there’s a strong requirement + policy; even then, store/handle as string (leading zeros) and fully mask.
- Current masking leaks digits and uses
number(drops leading zeros).- const aadharNo = getProfileValue('adhaarNo') as number | undefined; + const aadharNo = getProfileValue('adhaarNo') as string | undefined; {isCitizenProfile && ( <Typography fontSize={15} color="#666"> - {aadharNo - ? `${aadharNo.toString().slice(0, 4)} **** ***${aadharNo - .toString() - .slice(-1)}` - : ''} + {/* Consider removing entirely, or display only last 4 */} + {aadharNo ? `**** **** ${aadharNo.slice(-4)}` : ''} </Typography> )}Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/components/ProfileCard.tsx-27-41 (1)
27-41: Guard i18n lookups; remove!to avoid runtime crashes when messages/lang keys are missing.
getMessagesFromSession(role)!plus deep indexing can throw if session is empty, role mismatches, orlangkey isn’t present.- const messages = getMessagesFromSession(role)!; + const messages = getMessagesFromSession(role); + const profileMsgs = messages?.profile?.[lang] ?? messages?.profile?.en ?? {}; - const personalDetailsLabel = messages['profile'][lang]['personal-details']; + const personalDetailsLabel = profileMsgs['personal-details'] ?? 'Personal details'; // ...same pattern for the restCommittable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/components/ProfileCard.tsx-79-89 (1)
79-89: Remove hardcoded sample address; use localized “not provided” fallback.Returning
"Plot 567, 27th Main Road, HSR Layout"risks showing incorrect data as if it’s real.const formatAddress = (address: Address | undefined): string => { - if (!address) return 'Plot 567, 27th Main Road, HSR Layout'; + if (!address) return profileMsgs['address-not-provided'] ?? 'Address not provided'; const parts = [ address.addressLine1, address.addressLine2, address.city, address.state, address.pinCode, ].filter(Boolean); - return parts.length > 0 ? parts.join(', ') : 'Address not provided'; + return parts.length > 0 ? parts.join(', ') : (profileMsgs['address-not-provided'] ?? 'Address not provided'); };Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/components/Popup/NotificationPopup.tsx-10-43 (1)
10-43: Default branch rendersSuccessPopupbut doesn’t forcetype="success".If
typeis undefined/unknown, you’ll render the Success component while still passing the originaltype, leading to missing/incorrect icon/styles downstream.// Default: render SuccessPopup : <SuccessPopup - type={type} + type="success" title={title} message={message} open={open} onClose={onClose} />mobile-ui/src/app/features/Agent/components/DetailsCard.tsx-31-36 (1)
31-36: Use stable keys instead of array indices.Using array index as the React
keyprop can cause rendering bugs when the list is reordered, items are added, or items are removed. React may incorrectly reuse component instances, leading to stale data or incorrect rendering.Since the
KeyValuetype doesn't include a unique identifier, consider one of these solutions:Solution 1 (Recommended): Use the key property if it's guaranteed to be unique:
{items.map(({ key, value }, i) => ( - <div className="details-card-row" key={i}> + <div className="details-card-row" key={key}> <span className="details-card-key">{key} :</span> <span className="details-card-value">{value}</span> </div> ))}Solution 2: If keys may not be unique, add an
idfield toKeyValue:type KeyValue = { + id: string; key: string; value: React.ReactNode; };- {items.map(({ key, value }, i) => ( - <div className="details-card-row" key={i}> + {items.map(({ id, key, value }) => ( + <div className="details-card-row" key={id}> <span className="details-card-key">{key} :</span> <span className="details-card-value">{value}</span> </div> ))}mobile-ui/src/app/features/Agent/components/ChromeTabs.tsx-23-58 (1)
23-58: Missing ARIA attributes and keyboard navigation for tabs pattern.The component implements a tab switcher but lacks proper accessibility support:
- Missing
role="tablist"on the container androle="tab"on buttons- Missing
aria-selectedattributes to indicate active tab- No keyboard navigation (Left/Right arrow keys) to switch between tabs
- These omissions violate WCAG 1.3.1 (Info and Relationships) and 4.1.2 (Name, Role, Value)
Add ARIA attributes and keyboard support. Here's a reference implementation pattern:
const ChromeTabs: React.FC<ChromeTabsProps> = ({ selected, onTabChange, height = 50 }) => { const { mapTabText, landuseTabText } = useHomePageLocalization(); const handleKeyDown = (event: React.KeyboardEvent, currentIndex: 0 | 1) => { if (event.key === 'ArrowLeft') { onTabChange(0); } else if (event.key === 'ArrowRight') { onTabChange(1); } }; return ( <TabsContainer role="tablist"> <TabButton role="tab" aria-selected={selected === 0} tabIndex={selected === 0 ? 0 : -1} $isSelected={selected === 0} $height={height} onClick={() => onTabChange(0)} onKeyDown={(e) => handleKeyDown(e, 0)} // ... styles > {mapTabText} </TabButton> <TabButton role="tab" aria-selected={selected === 1} tabIndex={selected === 1 ? 0 : -1} $isSelected={selected === 1} $height={height} onClick={() => onTabChange(1)} onKeyDown={(e) => handleKeyDown(e, 1)} // ... styles > {landuseTabText} </TabButton> </TabsContainer> ); };mobile-ui/src/app/features/Agent/components/ChromeTabs.tsx-101-103 (1)
101-103: Removing focus indicators creates an accessibility barrier.Removing the
focus-visibleoutline prevents keyboard users from seeing which tab is currently focused, violating WCAG 2.4.7 (Focus Visible). Keyboard navigation requires visible focus indicators.Remove the focus-visible override or provide an alternative focus indicator:
transition: 'background 0.2s', '&:hover': { background: $isSelected ? '#ededed' : '#f5f5f5', }, - // Remove focus ring - '&:focus-visible': { - outline: 'none', - }, }));Or provide a custom focus style:
'&:focus-visible': { outline: '2px solid #0066cc', outlineOffset: '2px', },mobile-ui/src/app/features/PropertyForm/components/CountIncrementors.tsx-37-40 (1)
37-40: Remove unreachable and incorrect decrement logic.The special case on line 39 (
value === 0 && min < 0) is unreachable and logically flawed:
- If
value === 0andmin < 0(e.g.,min = -5), thenvalue > ministrue, so line 38 executes first and decrements to-1. Line 39 never runs.- Line 39 would only execute if
value <= min, but the conditionmin < 0doesn't align with that scenario.- Even if it were reachable, jumping directly to
min(e.g.,0→-5) violates expected single-step decrement behavior.Apply this diff to simplify:
- const dec = () => { - if (typeof value === "number" && value > min) setValue(value - 1); - else if (value === 0 && min < 0) setValue(min); - }; + const dec = () => { + if (value > min) setValue(value - 1); + };mobile-ui/src/app/features/PropertyForm/components/CountIncrementors.tsx-32-35 (1)
32-35: Simplify and fix increment logic.The special case on line 33 (
value === 0 && min <= 1) is redundant and creates confusing behavior. The condition is overly broad—it triggers whenminis any value ≤ 1, which includes scenarios likemin = -5. In such cases, the increment jumps to1rather than incrementing naturally from0to1.The standard increment logic on line 34 (
value < max ? value + 1) already handles incrementing from0to1correctly.Apply this diff to simplify:
- const inc = () => { - if (value === 0 && min <= 1) setValue(1); - else if (typeof value === "number" && value < max) setValue(value + 1); - }; + const inc = () => { + if (value < max) setValue(value + 1); + };mobile-ui/src/app/features/PropertyForm/components/CountIncrementors.tsx-26-29 (1)
26-29: Fix input clearing behavior to allow proper data entry.The immediate conversion of empty string to
0(line 28) prevents users from clearing the field to type a new value. This is particularly problematic whenminis negative—users cannot clear the field and type a negative number because it immediately resets to0.Apply this diff to allow the field to remain empty temporarily:
- const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { - const val = e.target.value === "" ? "" : Number(e.target.value); - if (val === "" || (val >= min && val <= max)) setValue(val === "" ? 0 : val); - }; + const handleChange = (e: React.ChangeEvent<HTMLInputElement>) => { + const val = e.target.value; + if (val === "") { + setValue(min > 0 ? min : 0); + return; + } + const numVal = Number(val); + if (!isNaN(numVal) && numVal >= min && numVal <= max) { + setValue(numVal); + } + };Alternatively, consider allowing the value to be temporarily invalid (empty or out of bounds) during editing, and only validate on blur.
Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/components/NavHeader.tsx-14-14 (1)
14-14: Strengthen type safety with a union type forrole.The
roleprop is typed asstring, but only two values are valid:"CITIZEN"and"AGENT". This weakens type safety and allows invalid values to be passed at compile time.Apply this diff to use a union type:
-const NavHeader: FC<{role : string}> = ({role}) => { +const NavHeader: FC<{role: 'CITIZEN' | 'AGENT'}> = ({role}) => {mobile-ui/src/app/features/PropertyForm/components/IGRSDetail/IGRSFormTextFiled.tsx-8-22 (1)
8-22: Replaceanytype with proper MUI SxProps type.The
sxprop is typed asany, which defeats TypeScript's type safety. MUI provides a specific type for style props.Apply this diff to fix the type:
+import { SxProps, Theme } from '@mui/material'; + interface FormTextFieldProps { label: string; // allow undefined values safely value?: string; onChange: (value: string) => void; onBlur?: () => void; onKeyPress?: (e: React.KeyboardEvent<HTMLInputElement>) => void; placeholder?: string; type?: 'text' | 'number'; required?: boolean; error?: string; touched?: boolean; - sx?: any; + sx?: SxProps<Theme>; inputProps?: React.InputHTMLAttributes<HTMLInputElement>; }mobile-ui/src/app/features/Agent/components/Layout.tsx-94-101 (1)
94-101: Log parsing errors for better debuggability.The try-catch block silently swallows JSON parsing errors, which makes debugging difficult if sessionStorage contains corrupted data.
Apply this diff to log errors:
const agentUser: AgentUser | null = (() => { try { const userStr = sessionStorage.getItem('user'); return userStr ? JSON.parse(userStr) : null; - } catch { + } catch (error) { + console.error('Failed to parse user from sessionStorage:', error); return null; } })();mobile-ui/src/app/features/Agent/components/Layout.tsx-185-188 (1)
185-188: Persist the zone/ward selection or clarify the intent.The
handleJurisdictionConfirmfunction closes the popup but doesn't persist the user's zone/ward selection. The selected values remain in local component state and will be lost on unmount. If the selection should be remembered across sessions, consider saving it to sessionStorage or passing it to a parent component/context.If persistence is intended, apply this approach:
const handleJurisdictionConfirm = () => { setShowLocationPopup(false); - // Here you can save selected zone/ward if needed elsewhere + // Persist selection to sessionStorage + sessionStorage.setItem( + 'selectedJurisdiction', + JSON.stringify({ + zoneIndex: selectedZoneIndex, + ward: selectedWard, + }) + ); };If persistence is not needed, remove the TODO-style comment to avoid confusion.
mobile-ui/LICENSE-3-4 (1)
3-4: MIT license should name the copyright holderMIT templates typically include the owner name/org; leaving it blank can create ambiguity.
-Copyright (c) 2025 +Copyright (c) 2025 <Company/Organization Name>Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/features/Agent/api/propertyData.hooks.ts-94-113 (1)
94-113: Clarify and simplify coordinate transformation logic.The coordinate salvage logic (lines 98-110) is confusing. It checks if values are numbers, then attempts to swap based on magnitude (assuming longitude > 90 means it's in the wrong position), but the fallback in the else branch (lines 106-108) swaps them identically regardless. This logic appears either redundant or incorrect.
The current logic:
if (Math.abs(a) > 90 && Math.abs(b) <= 90) { lat = b; lng = a; // Swap if a is likely longitude } else { lat = b; lng = a; // Always swap anyway? }Both branches produce the same result. Please clarify the intended behavior:
- If coordinates are always stored as
[lng, lat](per line 94 comment), why is there salvage logic?- If the format is uncertain, document the expected formats and validate them properly.
Consider:
const [initialLng, initialLat] = coord; // Validate coordinate ranges if (typeof initialLat !== 'number' || typeof initialLng !== 'number') { console.error('Invalid coordinate format:', coord); return null; // Skip invalid coordinates } if (Math.abs(initialLat) > 90 || Math.abs(initialLng) > 180) { console.error('Coordinate out of valid range:', { lat: initialLat, lng: initialLng }); return null; } return { latitude: initialLat, longitude: initialLng, gisDataId };Then filter out nulls:
.map(...).filter(Boolean)mobile-ui/src/app/features/Agent/api/propertyData.hooks.ts-29-52 (1)
29-52: Add validation for required form data before API calls.The function uses non-null assertions on
formData.propertyAddress!(line 37) without validating that the data exists. If these fields are undefined, the API call will fail or send invalid data.Apply this diff to add validation:
const savePropertyBasics = async (propertyID?: string) => { + if (!formData.categoryOfOwnership || !formData.propertyType) { + throw new Error('Missing required property basic information'); + } + if (propertyID) { + if (!formData.propertyAddress) { + throw new Error('Property address is required for update'); + } const result = await updateProperty({ propertyId: propertyID, ownershipType: formData.categoryOfOwnership, propertyType: formData.propertyType, complexName: formData.apartmentName, - address: formData.propertyAddress!, + address: formData.propertyAddress, propertyNo: formData.propertyNo || '', }).unwrap();mobile-ui/src/app/components/common/RequestDialog.tsx-114-124 (1)
114-124: Replacedocument.getElementByIdwithuseReffor better React patterns.Using
document.getElementByIdbypasses React's ref system and is not idiomatic. Consider usinguseRefinstead.Apply this diff:
+import React, { useState, useRef } from 'react'; -import React, { useState } from 'react'; const RequestDialog: React.FC<RequestDialogProps> = ({ open, onClose, onSubmit, }) => { const [comment, setComment] = useState(''); const [selectedFile, setSelectedFile] = useState<File | undefined>(undefined); + const fileInputRef = useRef<HTMLInputElement>(null); // ... other handlers ... return ( // ... dialog content ... <Box sx={fileUploadBoxStyle} - onClick={() => document.getElementById('file-input')?.click()} + onClick={() => fileInputRef.current?.click()} > <input - id="file-input" + ref={fileInputRef} type="file" hidden onChange={handleFileChange} accept=".pdf,.doc,.docx,.jpg,.jpeg,.png" />mobile-ui/src/app/components/SupportCard.tsx-22-22 (1)
22-22: Remove non-null assertion without validation.The non-null assertion
getMessagesFromSession("CITIZEN")!assumes messages will always be available. If the session doesn't contain citizen messages or localization hasn't loaded, this will cause a runtime error.Apply this diff to add proper null handling:
const lang = useAppSelector(state => state.lang.citizenLang); - const messages = getMessagesFromSession("CITIZEN")!; + const messages = getMessagesFromSession("CITIZEN"); + + // Return null if messages aren't loaded yet + if (!messages) { + return null; + }mobile-ui/src/app/features/Citizen/components/OverviewStack.tsx-32-32 (1)
32-32: Remove non-null assertion without validation.The non-null assertion
getMessagesFromSession('CITIZEN')!assumes messages will always be available. If the session doesn't contain citizen messages or the localization hasn't loaded yet, this will cause a runtime error.Apply this diff to add proper null handling:
- const messages = getMessagesFromSession('CITIZEN')!; // Localized messages + const messages = getMessagesFromSession('CITIZEN'); + + // Show loader if messages aren't available yet + if (!messages) { + return <LoadingPage />; + }mobile-ui/src/app/features/Agent/api/propertyData.hooks.ts-57-78 (1)
57-78: Fix inconsistent null handling in GIS data submission.Lines 71-72 use optional chaining (
coordinates?.lat) followed by non-null assertion (!), which defeats the purpose of optional chaining. Ifcoordinatesis undefined, the optional chaining will returnundefined, and the non-null assertion will pass it through, causing the API to receiveundefinedvalues.Apply this diff:
const saveGISData = async (propertyID: string, gisDataId?: string) => { - const { coordinates } = formData.locationData!; + const locationData = formData.locationData; + if (!locationData?.coordinates) { + throw new Error('Location coordinates are required for GIS data'); + } + const { coordinates } = locationData; if (gisDataId) { await updateGISData({ gisDataId, propertyId: propertyID, source: 'GPS', type: 'POLYGON', }).unwrap(); return gisDataId; } else { const result = await submitGISData({ propertyId: propertyID, - latitude: coordinates?.lat!, - longitude: coordinates?.lng!, + latitude: coordinates.lat, + longitude: coordinates.lng, source: 'GPS', type: 'POLYGON', }).unwrap(); return result.data.ID; } };mobile-ui/src/app/features/Citizen/components/PropertyComponent.tsx-59-65 (1)
59-65: Typo in status key: "ASSIGNIED" should be "ASSIGNED".This typo will cause the status mapping to fail for assigned properties, incorrectly showing 20% progress instead of 40%.
const STATUS_PROGRESS_MAP: Record<string, number> = { INITIATED: 20, - ASSIGNIED: 40, + ASSIGNED: 40, VERIFIED: 60, AUDIT_VERIFIED: 80, APPROVED: 100, };mobile-ui/src/app/features/Agent/components/SearchPropertyResultCard.tsx-6-7 (1)
6-7: Missing Leaflet CSS import.The component uses
react-leafletbut doesn't import Leaflet's CSS. This will cause the map to render incorrectly (missing tiles, broken layout).Add the CSS import:
import { MapContainer, TileLayer, Marker } from 'react-leaflet'; import L from 'leaflet'; +import 'leaflet/dist/leaflet.css';mobile-ui/src/app/features/Citizen/components/MapView.tsx-92-102 (1)
92-102: Inconsistent coordinate source inpropertyToMarkerInfo.When
p?.locationData?.coordinatesexists, the function checks that path but then returns coordinates fromp.Property?.GISDatainstead. This appears to be a bug - the function should use the coordinates from the path it validated.if (p?.locationData?.coordinates) { return { - lat: p.Property?.GISData?.Latitude, - lng: p.Property?.GISData?.Longitude, + lat: p.locationData.coordinates.lat, + lng: p.locationData.coordinates.lng, address: p.locationData.address, blockNo: p?.Property?.Address?.BlockNo, pinCode: p?.Property?.Address?.PinCode, id: p.id, enumerationProgress, }; }mobile-ui/src/app/features/PropertyForm/api/documentUpload.api.ts-32-39 (1)
32-39: MissinginvalidatesTagson update mutation.The
updateDocumentUploadDetailsmutation doesn't invalidate the cache, unlikecreateDocumentUploadDetailsanddeleteDocumentUploadDetails. This will cause stale data to be displayed after an update.updateDocumentUploadDetails: builder.mutation<DocumentUploadDetailsResponse, { document_id: string; body: DocumentUploadDetailsRequest }>({ query: ({ document_id, body }) => ({ url: `/v1/documents/${document_id}`, method: 'PUT', body, }), + invalidatesTags: [TAG_TYPES.DOCUMENT], }),mobile-ui/src/app/features/Citizen/components/MapView.tsx-376-401 (1)
376-401: Dead code: document click handler for commented-out button.The
handleClickfunction listens for clicks on.marker-card-btn-2, but this button is commented out in the marker HTML (lines 182-190). The handler body is also mostly commented out. Either complete the feature or remove the dead code.- // Handle "View Details" button click (same document-level listener as before) - useEffect(() => { - function handleClick(e: MouseEvent) { - const target = e.target as HTMLElement; - const btn = target.closest - ? (target.closest('.marker-card-btn-2') as HTMLElement | null) - : null; - if (!btn) return; - const idAttr = btn.getAttribute('data-prop-id'); - const markerData = markersData.find((m) => String(m.id) === String(idAttr)); - - if (markerData) { - // if (onViewDetails) { - // onViewDetails(markerData.original); - // } else { - // navigate(`/citizen/properties/${markerData.id}`, { - // state: { - // applicationId: markerData.id, - // }, - // }); - // } - } - } - document.addEventListener('click', handleClick); - return () => document.removeEventListener('click', handleClick); - }, [markersData, navigate, onViewDetails]);Also remove the unused
navigateimport if this handler is removed.mobile-ui/src/app/features/Citizen/components/MapView.tsx-262-263 (1)
262-263: Hardcoded API key exposed in client-side code.The MapTiler API key is hardcoded in the style URL. This key will be visible to anyone inspecting network requests and could be abused. Consider moving it to an environment variable.
const styleUrl = - 'https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc'; + `https://api.maptiler.com/maps/base-v4/style.json?key=${import.meta.env.VITE_MAPTILER_API_KEY}`;Ensure the API key is configured with appropriate restrictions (HTTP referrer, usage limits) in the MapTiler dashboard.
mobile-ui/src/app/features/Citizen/models/IGRSAdditionalDetails.model.ts-2-2 (1)
2-2: Typo in type name:ISGRAdditionalDetailsshould beIGRSAdditionalDetails.The type name has the letters swapped ("ISGR" vs "IGRS"), inconsistent with the filename and the comment which correctly reference "IGRS". This type is defined and used across multiple files:
mobile-ui/src/app/features/Citizen/models/IGRSAdditionalDetails.model.ts:2(primary definition)mobile-ui/src/app/features/Citizen/models/Property.model.ts:7(import)mobile-ui/src/redux/apis/propertyApi.ts:32(duplicate interface definition)mobile-ui/src/context/PropertyFormContext.tsx:64(duplicate interface definition)Update the type name in all locations and ensure consistent naming across the codebase.
mobile-ui/src/app/features/Citizen/components/ErrorMessage.tsx-12-60 (1)
12-60: Make ErrorMessage component more flexible with props.The component currently displays a static "Something went wrong" message with no customization options. This limits its reusability and makes debugging difficult.
Refactor to accept optional props:
+interface ErrorMessageProps { + message?: string; + title?: string; +} + -const ErrorMessage: FC = () => ( +const ErrorMessage: FC<ErrorMessageProps> = ({ + message = "Something went wrong", + title = "Error" +}) => ( <Box sx={{ my: 4, mx: "auto", maxWidth: 360, textAlign: "center" }}> <Paper elevation={2} sx={{ py: 4, width: "300px", px: 2, bgcolor: "#fff", borderRadius: 3, boxShadow: "0 2px 12px rgba(200,0,0,0.06)", border: "2px solid #f8bbbd", display: "flex", flexDirection: "column", alignItems: "center", }} > <ErrorOutlineIcon sx={{ fontSize: 60, color: "#d32f2f", mb: 2, }} /> <Typography variant="h6" sx={{ color: "#d32f2f", fontWeight: 700, mb: 1, }} > - Error + {title} </Typography> <Typography sx={{ color: "#a94442", fontSize: 15, fontWeight: 500, }} > - Something went wrong + {message} </Typography> </Paper> </Box> );mobile-ui/src/app/components/NavButton.tsx-10-38 (1)
10-38: Add button semantics + keyboard support (currentBoxis not accessible).
A clickabledivmisses keyboard navigation and proper semantics; preferButtonBase/IconButtonor renderBoxascomponent="button"with focus styles.-import { Box, Typography } from "@mui/material"; +import { Box, ButtonBase, Typography } from "@mui/material"; import type { FC } from "react"; import type { NavButtonProps } from "../models/NavButton.model"; const NavButton: FC<NavButtonProps> = ({ icon, message, onClick }) => { return ( - <Box + <ButtonBase + onClick={onClick} + focusRipple + aria-label={typeof message === "string" ? message : "Navigation button"} + sx={{ + borderRadius: "20px", + }} + > + <Box sx={{ display: "flex", alignItems: "center", cursor: "pointer", height: "50px", justifyContent: "center", bgcolor: "rgba(255, 255, 255, 0.5)", borderRadius: "20px", px: 2.5, width: "160px", boxShadow: "none", transition: "background 0.2s", gap: 1, '&:hover': { bgcolor: "#E5D3C6" }, }} - onClick= {onClick} > {icon} <Typography sx={{ color: "#D88336", fontSize: 16, fontWeight: 400 }}> {message} </Typography> </Box> + </ButtonBase> ); }mobile-ui/src/app/features/Citizen/api/CitizenHomePageApi/CitizenHomePageApi.ts-7-22 (1)
7-22: RemoveisDraft=truedefault; conditionally omit the parameter when undefined.The type signature marks
isDraftas optional, but the implementation defaults it totrue, causing callers who omitisDraftto always fetch only draft applications. This silently changes query semantics. Use conditional spread to omit the parameter when undefined, letting RTK Query'sfetchBaseQueryhandle the omission automatically—aligning with the codebase pattern used in similar endpoints (e.g.,floorApi.ts).- query: ({ assesseeId, isDraft=true }) => ({ + query: ({ assesseeId, isDraft }) => ({ url: `/v1/applications/search`, params: { assesseeId, - isDraft, + ...(typeof isDraft === "boolean" ? { isDraft } : {}), }, method: 'GET', }),mobile-ui/src/app/features/Citizen/components/ui/PropertyList.tsx-16-30 (1)
16-30: Ensure React keys are always stable/non-null; remove commented-out JSX.- {properties.map((prop: CitizenPropertySummary) => ( + {properties.map((prop: CitizenPropertySummary, idx) => ( <Box - key={prop.ID} + key={prop.ID ?? prop.applicationID ?? String(idx)} sx={{ display: 'block', justifyContent: 'center', }} > - {/* <PropertyCard property={prop} appId={prop.applicationID} /> */} <PropertyCard property={prop} propertyId={prop.ID ?? ''} /> </Box> ))}mobile-ui/src/app/features/Agent/api/agentApiSlice.ts-14-34 (1)
14-34: Fail fast ifVITE_ENUMERATION_HOSTis missing; confirm asyncprepareHeaderssupport.export const agentApiSlice = createApi({ reducerPath: 'agentApi', baseQuery: fetchBaseQuery({ - baseUrl: import.meta.env.VITE_ENUMERATION_HOST, + baseUrl: (() => { + const url = import.meta.env.VITE_ENUMERATION_HOST; + if (!url) throw new Error('Missing VITE_ENUMERATION_HOST'); + return url; + })(), prepareHeaders: async (headers, { getState: _getState }) => { const token = await authService.getValidToken(); if (token) { headers.set('authorization', `Bearer ${token}`) } - headers.set('content-type', 'application/json') + // Only set content-type when sending a JSON body (prefer per-endpoint) return headers }, }), endpoints: () => ({}), })mobile-ui/public/manifest.json-2-18 (1)
2-18: Update manifest.json with proper PWA icons and relative asset paths; add missing screenshot file.The manifest references
/vite.svg(a Vite placeholder), missing proper 192x512 PNG icons required for PWA install prompts and home screen icons. Additionally,start_url,scope, andicons.srcuse absolute paths that will break if the app is deployed under a non-root base path, andscreenshots[0].srcreferences/screenshot-mobile.pngwhich does not exist. Replace the placeholder icon with a 192×192 and 512×512 PNG icon set (including maskable variants), update paths to use relative URLs (e.g.,./vite.svgor remove leading/), provide the missing screenshot file, and configurevite.config.tswith abasesetting if subpath deployment is needed.mobile-ui/src/app/features/Citizen/api/CitizenPropertiesPageApi/CitizenPropertiesPageApi.ts-7-23 (1)
7-23: Fix query arg contract: unusedStatusand type mismatch forisDraft.The
Statusfield is required in the endpoint args but never used in the query; alsoisDraftshould be optional since it has a default value. This pattern is inconsistent with the similarCitizenHomePageApiendpoint which correctly typesisDraftas optional.export const citizenPropertiesPageApi = apiSlice.injectEndpoints({ endpoints: (builder) => ({ // Fetch applications for 'homepage' by assesseeId & isDraft getCitizenProperties: builder.query< CitizenApplicationSearchResponse, - { assesseeId: string; Status: string; isDraft: boolean } + { assesseeId: string; isDraft?: boolean } >({ - query: ({ assesseeId, isDraft=true }) => ({ + query: ({ assesseeId, isDraft = true }) => ({ url: `/v1/applications/search`, params: { assesseeId, isDraft, }, method: 'GET', }), }), }), });mobile-ui/src/app/features/Citizen/components/EnumeratedInfo.tsx-66-131 (1)
66-131: Complete localization for all user-facing strings.Several user-facing strings are hardcoded instead of using the localization system that's already in place. The commented-out localization calls suggest this is incomplete work.
Uncomment and use the localized messages:
- <Typography fontSize={13} color="#888"> - {/* {messages['citizen.my-properties'][lang]['total-amount']}: */} - Total Amount : - </Typography> + <Typography fontSize={13} color="#888"> + {messages['citizen.my-properties'][lang]['total-amount']}: + </Typography>- {/* {messages['citizen.my-properties'][lang]['view-bills']} */} - View Bills + {messages['citizen.my-properties'][lang]['view-bills']}<Typography fontSize={14} color="#222" fontWeight={500}> - Issued Licenses: + {messages['citizen.my-properties'][lang]['issued-licenses']}: </Typography>- {/* {messages['citizen.my-properties'][lang]['view-licenses']} */} - View Licences + {messages['citizen.my-properties'][lang]['view-licenses']}Ensure all corresponding keys exist in the localization message files.
mobile-ui/src/app/features/Agent/api/fetchProperty.hooks.ts-12-19 (1)
12-19: Add proper TypeScript types instead ofany.The function parameters use
anytypes, which defeats the purpose of TypeScript and makes the code less maintainable and more error-prone. RTK Query hooks and the form update callback should have proper types.Define proper types for the parameters:
+import type { UseQueryHookResult } from '@reduxjs/toolkit/query/react'; +import type { ApplicationData, OwnerData, PropertyFormData } from '../models/types'; export const fetchPropertyDetails = async ( propID: string, applicationId: string, - getApplicationById: any, - getOwnerByPropertyId: any, - updateForm: (data: any) => void, + getApplicationById: (id: string) => UseQueryHookResult<ApplicationData, any>, + getOwnerByPropertyId: (id: string) => UseQueryHookResult<OwnerData, any>, + updateForm: (data: PropertyFormData) => void, showErrorPopup: (message: string) => void ) => {Adjust the type imports based on your actual type definitions.
Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/src/app/features/Agent/api/fetchProperty.hooks.ts-25-57 (1)
25-57: Add defensive null checks for nested property access.The code performs deep nested property access (e.g.,
applicationData.data.Property.GISData.Coordinates) without validation. If the API response structure changes or data is missing, this will throw runtime errors. Additionally, the type assertions toanymask underlying type issues.Add optional chaining and proper type guards:
updateForm({ id: propID, categoryOfOwnership: applicationData.data.Property?.OwnershipType || '', propertyType: applicationData.data.Property?.PropertyType || '', apartmentName: applicationData.data.Property?.ComplexName || '', propertyNo: applicationData.data.Property?.PropertyNo || '', - propertyAddress: (applicationData.data.Property.Address as any) || undefined, + propertyAddress: applicationData.data.Property?.Address || undefined, locationData: applicationData.data.Property?.GISData ? { gisDataId: applicationData.data.Property.GISData.ID, - address: applicationData.data.Property.Address?.Locality || '', + address: applicationData.data.Property?.Address?.Locality || '', coordinates: { lat: applicationData.data.Property.GISData.Latitude || 0, lng: applicationData.data.Property.GISData.Longitude || 0, }, drawnShapes: - applicationData.data.Property.GISData.Coordinates?.length > 0 + (applicationData.data.Property.GISData.Coordinates?.length ?? 0) > 0 ? [ { type: 'polygon', coordinates: applicationData.data.Property.GISData?.Coordinates.map( (coord: any) => [coord.Longitude, coord.Latitude] ), }, ] : [], } : undefined, owners: ownersData.data || [], - isgrDetails: (applicationData.data.Property.IGRS as any) || undefined, - assessmentDetails: (applicationData.data.Property.AssessmentDetails as any) || undefined, + isgrDetails: applicationData.data.Property?.IGRS || undefined, + assessmentDetails: applicationData.data.Property?.AssessmentDetails || undefined, importantNotes: applicationData.data.ImportantNote || '', });This makes the code more resilient to API changes and missing data.
Committable suggestion skipped: line range outside the PR's diff.
mobile-ui/index.html-7-7 (1)
7-7: Accessibility concern:user-scalable=noprevents users from zooming.The
user-scalable=nometa tag prevents users from pinching to zoom, which creates accessibility barriers for users with visual impairments. This violates WCAG 2.1 Success Criterion 1.4.4 (Resize text).Remove the
user-scalable=norestriction:- <meta name="viewport" content="width=device-width, initial-scale=1.0, maximum-scale=1.0, user-scalable=no, viewport-fit=cover" /> + <meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />If you need to prevent zoom on specific form inputs (to prevent iOS auto-zoom), use CSS instead:
input, select, textarea { font-size: 16px; /* iOS won't zoom if font-size >= 16px */ }mobile-ui/src/app/features/Agent/models/ReviewPage.model.ts-21-21 (1)
21-21: Replaceanytypes with proper type definitions.The
IGRS(line 21) andApplicationLogs(line 38) fields are typed asany, which defeats TypeScript's type safety. This can lead to runtime errors and makes the code harder to maintain.If the structure is unknown or variable, use
unknownand add type guards. If the structure is known, define proper interfaces:+// Define proper interfaces for IGRS and logs +export interface IGRSData { + // Add IGRS fields based on actual structure + registrationNumber?: string; + date?: string; + // ... other fields +} + +export interface ApplicationLog { + id: string; + timestamp: string; + action: string; + performedBy: string; + // ... other fields +} export interface PropertyDetails { // ... other fields - IGRS: any; + IGRS: IGRSData | null; } export interface ApplicationItem { // ... other fields - ApplicationLogs: any; + ApplicationLogs: ApplicationLog[]; // ... other fields }If the structure is truly unknown, use
unknowninstead ofany:- IGRS: any; + IGRS: unknown; - ApplicationLogs: any; + ApplicationLogs: unknown;Also applies to: 38-38
mobile-ui/db.json-1-892 (1)
1-892: Document the purpose and scope of this data file.This large JSON file contains sample property data, lookup lists, and configuration. However, several concerns need to be addressed:
Data Privacy: The file contains phone numbers (e.g., lines 595, 610, 625) that appear to be sample data. Ensure these are not real phone numbers.
Development vs. Production: It's unclear whether this data is:
- For local development/testing only
- Seeded into a database
- Served as a mock API response
Data Consistency: Some property records have inconsistent field usage (e.g.,
typefield contains both "Survey" and property type values like "Commercial", "Residential").Add documentation at the top of the file:
{ "_comment": "This file contains sample data for local development and testing only. DO NOT use in production. Phone numbers and personal data are fictional.", "documentTypes": [Consider these architectural improvements:
- Separate concerns: Split into multiple files (e.g.,
lookups.json,sample-properties.json,sample-messages.json)- Schema validation: Add JSON Schema validation to ensure data consistency
- Environment-specific: Use environment variables to control whether this data is loaded
- Backend integration: Document the transition path from this static data to real API endpoints
If this data is for json-server or similar mock API tools, document the usage in the README.
mobile-ui/src/app/features/Citizen/components/MapComponent.tsx-34-102 (1)
34-102: Performance issue: Map re-initializes on every prop change.The
useEffectdependency array includes all props (latLng,tileUrl,customIcon), causing the entire map to be destroyed and recreated whenever any prop changes. This is inefficient and causes a jarring user experience when only the marker position or icon needs to update.Split the effect into multiple effects for better performance:
// Effect 1: Initialize map only when container or tileUrl changes useEffect(() => { if (!containerRef.current || mapRef.current) return; mapRef.current = new maplibregl.Map({ container: containerRef.current, style: tileUrl, center: [latLng.lng, latLng.lat], zoom: 15, attributionControl: false, scrollZoom: false, doubleClickZoom: false, }); mapRef.current.addControl(new maplibregl.NavigationControl(), 'top-right'); return () => { if (mapRef.current) { mapRef.current.remove(); mapRef.current = null; } }; }, [tileUrl]); // Only recreate when style URL changes // Effect 2: Update marker when position or icon changes useEffect(() => { if (!mapRef.current) return; // Remove old marker if (markerRef.current) { markerRef.current.remove(); } // Create marker element const el = document.createElement('div'); // ... (marker creation logic) markerRef.current = new maplibregl.Marker({ element: el, anchor: 'bottom' }) .setLngLat([latLng.lng, latLng.lat]) .addTo(mapRef.current); // Update map center mapRef.current.setCenter([latLng.lng, latLng.lat]); return () => { markerRef.current?.remove(); markerRef.current = null; }; }, [latLng.lat, latLng.lng, customIcon]);mobile-ui/index.html-43-49 (1)
43-49: Performance concern:passive: falseon touchmove can degrade scroll performance.Using
passive: falseon thetouchmoveevent prevents the browser from optimizing scroll performance because it must wait to see ifpreventDefault()will be called. This can cause scroll jank on mobile devices.Consider these alternatives:
- Use CSS to control scrolling behavior instead:
+ <style> + body { + overscroll-behavior: none; /* Prevent bounce on body */ + } + .property-form-container { + overscroll-behavior: auto; /* Allow scroll in forms */ + } + </style> - <script> - // Prevent bounce scrolling on iOS - document.addEventListener('touchmove', function(e) { - if(e.target.closest('.property-form-container')) { - return; - } - e.preventDefault(); - }, { passive: false }); - </script>
- If you must use JavaScript, make it passive and use CSS for the prevention:
// Add class-based control instead document.body.style.touchAction = 'none'; document.querySelectorAll('.property-form-container').forEach(el => { el.style.touchAction = 'auto'; });mobile-ui/src/app/features/Citizen/components/PropertyList.tsx-14-20 (1)
14-20: Invalid fallback coordinates may cause confusion.The fallback coordinates
{lat: 0, lng: 0}point to "Null Island" in the Atlantic Ocean off the coast of Africa. This is not a meaningful default for properties that should be in India (based on the context).Consider one of these approaches:
- Use
nullorundefinedto indicate missing coordinates and handle this explicitly in the UI:- const coordinates = (Array.isArray(coordsArr) && coordsArr.length > 0) - ? { - lat: coordsArr[0]?.Latitude ?? 0, - lng: coordsArr[0]?.Longitude ?? 0, - } - : { lat: 0, lng: 0 }; + const coordinates = (Array.isArray(coordsArr) && coordsArr.length > 0) + ? { + lat: coordsArr[0]?.Latitude, + lng: coordsArr[0]?.Longitude, + } + : null;
- Use a sensible default for your region (e.g., center of Guntur, Andhra Pradesh):
- : { lat: 0, lng: 0 }; + : { lat: 16.3067, lng: 80.4365 }; // Default to Guntur city centermobile-ui/src/app/features/Citizen/components/ui/HistoryCard.tsx-22-36 (1)
22-36: Avoid runtime crashes in i18n lookup; align prop typing with null-guard
messages['citizen.my-properties'][lang]['no-history']can throw if any segment is missing (the||fallback won’t help). Alsopropertyis required but guarded as optional.interface HistoryProps { - property: CitizenPropertyData; + property?: CitizenPropertyData; } const History: FC<HistoryProps> = ({ property }) => { const lang = useAppSelector((state) => state.lang.citizenLang); const { loading } = useLocalization(); - const messages = getMessagesFromSession("CITIZEN")!; + const messages = getMessagesFromSession("CITIZEN"); if (loading) { return <LoadingPage />; } if (!property) return null; @@ return ( <Paper ...> - <Typography fontWeight={700} mb={1} fontSize={16}> - </Typography> {hasHistory ? ( <Stack gap={2}> @@ ) : ( <Typography color="#888" fontSize={14} mt={1}> - {messages['citizen.my-properties'][lang]['no-history'] || 'No ownership history available.'} + {messages?.['citizen.my-properties']?.[lang]?.['no-history'] ?? 'No ownership history available.'} </Typography> )} </Paper> ); };Also applies to: 55-72
mobile-ui/src/app/features/Citizen/components/ui/PlotInfoCard.tsx-23-50 (1)
23-50: Guard against undefined fields + avoid i18n deep-index crashes (and localize labels)
property.ComplexName.toString()can throw; andmessages['citizen.my-properties'][lang][...]can throw if any segment is missing. Also several labels are hard-coded English while the rest is localized.- const messages = getMessagesFromSession("CITIZEN")!; // Localized messages + const messages = getMessagesFromSession("CITIZEN"); // Localized messages @@ - const complexName = property.ComplexName.toString() || '-'; + const complexName = property.ComplexName?.toString?.() || '-'; @@ <Typography fontWeight={700} fontSize={16}> - {messages['citizen.my-properties'][lang]['plt-info']} + {messages?.['citizen.my-properties']?.[lang]?.['plt-info'] ?? 'Plot Info'} </Typography> @@ - Apartment Name + {messages?.['citizen.my-properties']?.[lang]?.['apartment-name'] ?? 'Apartment Name'} @@ - Locality + {messages?.['citizen.my-properties']?.[lang]?.['locality'] ?? 'Locality'}Also applies to: 78-106, 110-142
mobile-ui/src/app/features/Citizen/components/PropertyCard.tsx-74-92 (1)
74-92: Prevent i18n lookup crashes; reconcile Draft click behavior
Deep indexing intomessages[...]can throw, and Draft handling inhandleCardClick()is currently unreachable becauseonClickis unset whenstatus === 'Draft'.- const messages = getMessagesFromSession('CITIZEN')!; + const messages = getMessagesFromSession('CITIZEN'); @@ - {messages['citizen.my-properties'][lang]['address']} + {messages?.['citizen.my-properties']?.[lang]?.['address'] ?? 'Address'} @@ - onClick={status !== 'Draft' ? handleCardClick : undefined} + onClick={handleCardClick}Also applies to: 101-117, 171-172, 236-278
mobile-ui/src/app/features/Citizen/components/ui/DocumentsTab.tsx-100-158 (1)
100-158: useEffect dependency on.lengthmay cause missed updates or infinite loops.Using
documentDisplayList.lengthas a dependency instead of the full array reference can miss updates when documents change but the count stays the same. Additionally,fetchFileBlobshould be excluded or the effect should have proper guards to prevent re-fetching.useEffect(() => { if (documentDisplayList.length === 0) return; + + // Track if effect is still active for cleanup + let isCancelled = false; const fetchFilesSequentially = async () => { const blobs: Record<string, Blob> = {}; const errors: Record<string, string> = {}; const loading: Record<string, boolean> = {}; for (const doc of documentDisplayList) { // Skip if no fileStoreId or is upload needed - if (!doc.fileStoreId || doc.isUpload) continue; + if (!doc.fileStoreId || doc.isUpload || isCancelled) continue; try { // ... fetch logic } catch (error) { // ... error handling } finally { // ... cleanup } } }; fetchFilesSequentially(); + + return () => { + isCancelled = true; + }; - }, [documentDisplayList.length, fetchFileBlob]); + }, [documentDisplayList]); // fetchFileBlob is stable from RTK QueryConsider using a ref to track already-fetched file IDs to avoid re-fetching on re-renders.
mobile-ui/src/app/features/Citizen/models/CitizenPropertiesPageModel/CitizenPropertyPageModel.ts-82-102 (1)
82-102: Property casing inFloorDetailis inconsistent and mismatches the API contract.The
FloorDetailinterface mixes PascalCase (FloorNo,Classification,NatureOfUsage) with camelCase (constructionDate,effectiveFromDate). This inconsistency appears across multiple type definitions (HomePageData.model.ts,ApplicationModals.ts,propertyApi.ts) and requires workarounds in the code—e.g.,FloorDetailsCards.tsxandFloorDetailsPage.tsxuse OR logic (apiData.ConstructionDate || apiData.constructionDate) to handle both casings. The actual API contract (defined infloorApi.ts) uses consistent camelCase for all properties. Standardize allFloorDetailtype definitions to use camelCase throughout to match the backend API and eliminate the need for transformation logic.mobile-ui/src/app/features/Citizen/components/EnumOptComponent.tsx-31-62 (1)
31-62: Fix potential crash: don’t non-null assert session messages before the loading guard.
IfgetMessagesFromSession('CITIZEN')isnull/undefined, this component can throw before it ever returns<LoadingPage />.const EnumOptComponent: FC<EnumOptComponentProps> = ({ property }) => { // Localization, state, and hooks const lang = useAppSelector((state) => state.lang.citizenLang); // Current language const { loading } = useLocalization(); // Global loading state - const messages = getMessagesFromSession('CITIZEN')!; // Localized messages // No property: render nothing if (!property) return null; + // Show loader if localization is loading + if (loading) { + return <LoadingPage />; + } + + const messages = getMessagesFromSession('CITIZEN'); // Localized messages + if (!messages) { + return <LoadingPage />; // or null / fallback UI + } - - // Show loader if localization is loading - if (loading) { - return <LoadingPage />; - }
| "dependencies": { | ||
| "@emotion/react": "^11.14.0", | ||
| "@emotion/styled": "^11.14.1", | ||
| "@mapbox/mapbox-gl-draw": "^1.5.1", | ||
| "@mui/icons-material": "^7.3.5", | ||
| "@mui/lab": "^7.0.1-beta.18", | ||
| "@mui/material": "^7.3.6", | ||
| "@mui/x-date-pickers": "^8.16.0", | ||
| "@reduxjs/toolkit": "^2.10.1", | ||
| "@types/leaflet": "^1.9.21", | ||
| "@types/react-redux": "^7.1.34", | ||
| "axios": "^1.12.2", | ||
| "clsx": "^2.1.1", | ||
| "date-fns": "^4.1.0", | ||
| "dayjs": "^1.11.19", | ||
| "dotenv": "^17.2.3", | ||
| "leaflet": "^1.9.4", | ||
| "leaflet-draw": "^1.0.4", | ||
| "maplibre-gl": "^5.14.0", | ||
| "react": "^19.2.1", | ||
| "react-datepicker": "^8.8.0", | ||
| "react-dom": "^19.2.1", | ||
| "react-icons": "^5.5.0", | ||
| "react-leaflet": "^5.0.0", | ||
| "react-map-gl": "^8.1.0", | ||
| "react-redux": "^9.2.0", | ||
| "react-router-dom": "^7.9.4", | ||
| "react-select": "^5.10.2", | ||
| "react-toastify": "^11.0.5", | ||
| "zod": "^4.1.12" | ||
| }, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.39.1", | ||
| "@types/leaflet-draw": "^1.0.13", | ||
| "@types/mapbox__mapbox-gl-draw": "^1.4.9", | ||
| "@types/node": "^24.6.0", | ||
| "@types/react": "^19.1.16", | ||
| "@types/react-dom": "^19.1.9", | ||
| "@vitejs/plugin-react": "^5.0.4", | ||
| "eslint": "^9.39.1", | ||
| "eslint-plugin-react-hooks": "^5.2.0", | ||
| "eslint-plugin-react-refresh": "^0.4.22", | ||
| "globals": "^16.4.0", | ||
| "json-server": "^1.0.0-beta.3", | ||
| "typescript": "~5.9.3", | ||
| "typescript-eslint": "^8.45.0", | ||
| "vite": "^7.2.4" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
React 19.2 eslint-plugin-react-hooks version compatibility requirements
💡 Result:
Short answer: React 19.2 expects you to use the newer eslint-plugin-react-hooks (the blog calls out v6+). Upgrade the plugin to the latest v6+ (or later) so the linter understands new React 19.2 features like useEffectEvent and the merged "react-compiler" rules; older plugin versions may lack those rules and can have ESLint peer-dependency incompatibilities (ESLint v9 support was added in the newer plugin releases). [1][2][3]
Sources
- React 19.2 release notes (calls out eslint-plugin-react-hooks v6 and useEffectEvent). [1]
- eslint-plugin-react-hooks npm page (current docs and legacy vs recommended notes). [2]
- React/GitHub issues discussing ESLint v9 / plugin compatibility and the plugin releases. [3]
🌐 Web query:
eslint-plugin-react-hooks v6 release notes React 19 support
💡 Result:
eslint-plugin-react-hooks 6.x: the first official 6.x release is eslint-plugin-react-hooks@6.1.0. Key notes from that release: require Node.js ≥18; flat config is the default recommended (legacy moved to recommended-legacy); new rule violations (e.g. disallow calling use in try/catch, disallow calling useEffectEvent in arbitrary closures); handle React.useEffect in rules-of-hooks; added a settings option for additionalEffectHooks. (6.0.0 was mistakenly published/ deprecated; 6.1.0 is the first official 6.x.) [1]
If you need React 19 / React Compiler–related behavior: the React team merged the React Compiler rules into eslint-plugin-react-hooks and there have been related releases/canaries and follow-up issues (see linked release and compiler/compatibility issues). Check the 6.1.0+ changelog and the React repo issues for ongoing fixes around ESLint/React Compiler & React 19 support. [1][2][3]
Sources:
- Release notes (eslint-plugin-react-hooks@6.1.0). [1]
- React repo issues discussing ESLint/React Compiler / ESLint v9 / 6.x release confusion. [2][3]
🌐 Web query:
React 19.2.1 breaking changes hook semantics
💡 Result:
Short summary — React 19.2.1 did not change the core hook runtime, but the release cycle introduced stricter hook-related linting/rules and a few behavioral/ID tweaks you should watch for:
-
ESLint/react-hooks v6: stronger rules that can surface breaking patterns — notably disallowing calling use(...) inside try/catch blocks and disallowing calling useEffectEvent return-values from arbitrary closures. Also requires Node 18+ and changes recommended config defaults. [1][2]
-
useId ID prefix changed in the 19.x line (affects any code/CSS that depended on previous ID formats). [3]
-
Security/patch note: 19.2.1 was published as a patch in this cycle to address a Server Components RSC security issue (upgrade if you use RSC packages). This is not a hook-semantics change but important to apply. [4]
If you want, I can: (a) show concrete code examples of patterns that will now lint/error and how to fix them, or (b) list exact package versions / npm install commands to update.
Sources: React release/changelog and eslint-plugin-react-hooks release notes. [1] react.dev changelog/blog; [2] eslint-plugin-react-hooks v6 notes (npm/newreleases summaries); [3] community writeups about useId change; [4] React security bulletin about 19.2.1.
🌐 Web query:
eslint-plugin-react-hooks latest version 2025
💡 Result:
As of Dec 12, 2025:
- The latest stable release on npm is 5.2.0 (npm package page). [1]
- The React repo/discussion shows later 6.x/7.x releases and release-tag confusion (issues about 6.0/6.1 and tagging). [2]
- Maven/npm mirror records indicate newer artifacts (e.g., 6.1.1 and 7.0.1 published in 2025). If you need the absolutely newest published artifact (6.x/7.x), I can check the npm dist-tags or the GitHub releases/tags for the exact version and publish date. [3]
Sources: npm package listing for eslint-plugin-react-hooks. [1] GitHub issues in the facebook/react repository about plugin releases. [2] Maven/npm mirror showing 6.1.1 / 7.0.1. [3]
🏁 Script executed:
#!/bin/bash
# Query npm registry for eslint-plugin-react-hooks to get latest version info
npm view eslint-plugin-react-hooks versions --json 2>/dev/null | tail -20
echo "---"
npm view eslint-plugin-react-hooks dist-tags --json 2>/dev/nullRepository: egovernments/Infosys
Length of output: 990
Upgrade eslint-plugin-react-hooks to v7.0.1+ for React 19.2.1 compatibility.
React 19.2.1 requires eslint-plugin-react-hooks v6+ to support new hook linting rules (e.g., disallowing use() in try/catch blocks and useEffectEvent closure restrictions). The current version ^5.2.0 lacks these rules and is incompatible with ESLint v9. The latest stable release is v7.0.1.
- "eslint-plugin-react-hooks": "^5.2.0",
+ "eslint-plugin-react-hooks": "^7.0.1",📝 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.
| "dependencies": { | |
| "@emotion/react": "^11.14.0", | |
| "@emotion/styled": "^11.14.1", | |
| "@mapbox/mapbox-gl-draw": "^1.5.1", | |
| "@mui/icons-material": "^7.3.5", | |
| "@mui/lab": "^7.0.1-beta.18", | |
| "@mui/material": "^7.3.6", | |
| "@mui/x-date-pickers": "^8.16.0", | |
| "@reduxjs/toolkit": "^2.10.1", | |
| "@types/leaflet": "^1.9.21", | |
| "@types/react-redux": "^7.1.34", | |
| "axios": "^1.12.2", | |
| "clsx": "^2.1.1", | |
| "date-fns": "^4.1.0", | |
| "dayjs": "^1.11.19", | |
| "dotenv": "^17.2.3", | |
| "leaflet": "^1.9.4", | |
| "leaflet-draw": "^1.0.4", | |
| "maplibre-gl": "^5.14.0", | |
| "react": "^19.2.1", | |
| "react-datepicker": "^8.8.0", | |
| "react-dom": "^19.2.1", | |
| "react-icons": "^5.5.0", | |
| "react-leaflet": "^5.0.0", | |
| "react-map-gl": "^8.1.0", | |
| "react-redux": "^9.2.0", | |
| "react-router-dom": "^7.9.4", | |
| "react-select": "^5.10.2", | |
| "react-toastify": "^11.0.5", | |
| "zod": "^4.1.12" | |
| }, | |
| "devDependencies": { | |
| "@eslint/js": "^9.39.1", | |
| "@types/leaflet-draw": "^1.0.13", | |
| "@types/mapbox__mapbox-gl-draw": "^1.4.9", | |
| "@types/node": "^24.6.0", | |
| "@types/react": "^19.1.16", | |
| "@types/react-dom": "^19.1.9", | |
| "@vitejs/plugin-react": "^5.0.4", | |
| "eslint": "^9.39.1", | |
| "eslint-plugin-react-hooks": "^5.2.0", | |
| "eslint-plugin-react-refresh": "^0.4.22", | |
| "globals": "^16.4.0", | |
| "json-server": "^1.0.0-beta.3", | |
| "typescript": "~5.9.3", | |
| "typescript-eslint": "^8.45.0", | |
| "vite": "^7.2.4" | |
| }, | |
| "dependencies": { | |
| "@emotion/react": "^11.14.0", | |
| "@emotion/styled": "^11.14.1", | |
| "@mapbox/mapbox-gl-draw": "^1.5.1", | |
| "@mui/icons-material": "^7.3.5", | |
| "@mui/lab": "^7.0.1-beta.18", | |
| "@mui/material": "^7.3.6", | |
| "@mui/x-date-pickers": "^8.16.0", | |
| "@reduxjs/toolkit": "^2.10.1", | |
| "@types/leaflet": "^1.9.21", | |
| "@types/react-redux": "^7.1.34", | |
| "axios": "^1.12.2", | |
| "clsx": "^2.1.1", | |
| "date-fns": "^4.1.0", | |
| "dayjs": "^1.11.19", | |
| "dotenv": "^17.2.3", | |
| "leaflet": "^1.9.4", | |
| "leaflet-draw": "^1.0.4", | |
| "maplibre-gl": "^5.14.0", | |
| "react": "^19.2.1", | |
| "react-datepicker": "^8.8.0", | |
| "react-dom": "^19.2.1", | |
| "react-icons": "^5.5.0", | |
| "react-leaflet": "^5.0.0", | |
| "react-map-gl": "^8.1.0", | |
| "react-redux": "^9.2.0", | |
| "react-router-dom": "^7.9.4", | |
| "react-select": "^5.10.2", | |
| "react-toastify": "^11.0.5", | |
| "zod": "^4.1.12" | |
| }, | |
| "devDependencies": { | |
| "@eslint/js": "^9.39.1", | |
| "@types/leaflet-draw": "^1.0.13", | |
| "@types/mapbox__mapbox-gl-draw": "^1.4.9", | |
| "@types/node": "^24.6.0", | |
| "@types/react": "^19.1.16", | |
| "@types/react-dom": "^19.1.9", | |
| "@vitejs/plugin-react": "^5.0.4", | |
| "eslint": "^9.39.1", | |
| "eslint-plugin-react-hooks": "^7.0.1", | |
| "eslint-plugin-react-refresh": "^0.4.22", | |
| "globals": "^16.4.0", | |
| "json-server": "^1.0.0-beta.3", | |
| "typescript": "~5.9.3", | |
| "typescript-eslint": "^8.45.0", | |
| "vite": "^7.2.4" | |
| }, |
🤖 Prompt for AI Agents
In mobile-ui/package.json around lines 12 to 59 the devDependency
"eslint-plugin-react-hooks" is pinned to "^5.2.0" which is incompatible with
React 19.2.1 and ESLint v9; update the package to "eslint-plugin-react-hooks":
"^7.0.1" in devDependencies, run your package manager to install and update the
lockfile, and re-run lint to confirm the new rules are applied (update any
ESLint config if necessary).
| export function BottomBar() { | ||
| // Get the current language for citizen from Redux store | ||
| const lang = useAppSelector((state) => state.lang.citizenLang); | ||
| // Get localization loading state | ||
| const { loading } = useLocalization(); | ||
| // Get localized messages for the citizen module | ||
| const messages = getMessagesFromSession("CITIZEN")!; | ||
|
|
||
| // Show loader while localization messages are loading | ||
| if (loading) { | ||
| return <LoadingPage />; | ||
| } | ||
|
|
||
| // Navigation configuration for the bottom bar (desired order) | ||
| const navConfig = [ | ||
| { | ||
| label: messages['citizen.commons'][lang]['home-btn'], | ||
| icon: <HomeIcon />, | ||
| iconFilled: <HomeIconFilled />, | ||
| path: '/citizen', | ||
| }, | ||
| { | ||
| label: messages['citizen.commons'][lang]['my-city-btn'], | ||
| icon: <ApartmentOutlinedIcon />, | ||
| iconFilled: <ApartmentIcon />, | ||
| path: '/under-construction', | ||
| }, | ||
| { | ||
| label: messages['citizen.commons'][lang]['properties-btn'], | ||
| icon: <CottageOutlinedIcon />, | ||
| iconFilled: <CottageIcon />, | ||
| path: '/citizen/properties', | ||
| }, | ||
| { | ||
| label: messages['citizen.commons'][lang]['utility-btn'], | ||
| icon: <OfflineBoltOutlinedIcon />, | ||
| iconFilled: <OfflineBoltIcon />, | ||
| path: '/under-construction', | ||
| }, | ||
| ]; | ||
|
|
||
| const location = useLocation(); | ||
| const navigate = useNavigate(); | ||
|
|
||
| // Find the current nav index by matching the most specific path first | ||
| const matchIndex = [...navConfig] | ||
| .map((cfg, idx) => ({ idx, len: cfg.path.length, match: location.pathname.startsWith(cfg.path) })) | ||
| .filter(x => x.match) | ||
| .sort((a, b) => b.len - a.len)[0]?.idx ?? 0; | ||
|
|
There was a problem hiding this comment.
Fix hooks ordering violation.
The useLocation and useNavigate hooks are called after an early return, which violates React's Rules of Hooks. All hooks must be called at the top level in the same order on every render, before any conditional returns.
Move the hooks before the early return:
export function BottomBar() {
const lang = useAppSelector((state) => state.lang.citizenLang);
const { loading } = useLocalization();
const messages = getMessagesFromSession("CITIZEN")!;
+ const location = useLocation();
+ const navigate = useNavigate();
if (loading) {
return <LoadingPage />;
}
const navConfig = [
// ... config
];
- const location = useLocation();
- const navigate = useNavigate();Based on static analysis hints.
📝 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.
| export function BottomBar() { | |
| // Get the current language for citizen from Redux store | |
| const lang = useAppSelector((state) => state.lang.citizenLang); | |
| // Get localization loading state | |
| const { loading } = useLocalization(); | |
| // Get localized messages for the citizen module | |
| const messages = getMessagesFromSession("CITIZEN")!; | |
| // Show loader while localization messages are loading | |
| if (loading) { | |
| return <LoadingPage />; | |
| } | |
| // Navigation configuration for the bottom bar (desired order) | |
| const navConfig = [ | |
| { | |
| label: messages['citizen.commons'][lang]['home-btn'], | |
| icon: <HomeIcon />, | |
| iconFilled: <HomeIconFilled />, | |
| path: '/citizen', | |
| }, | |
| { | |
| label: messages['citizen.commons'][lang]['my-city-btn'], | |
| icon: <ApartmentOutlinedIcon />, | |
| iconFilled: <ApartmentIcon />, | |
| path: '/under-construction', | |
| }, | |
| { | |
| label: messages['citizen.commons'][lang]['properties-btn'], | |
| icon: <CottageOutlinedIcon />, | |
| iconFilled: <CottageIcon />, | |
| path: '/citizen/properties', | |
| }, | |
| { | |
| label: messages['citizen.commons'][lang]['utility-btn'], | |
| icon: <OfflineBoltOutlinedIcon />, | |
| iconFilled: <OfflineBoltIcon />, | |
| path: '/under-construction', | |
| }, | |
| ]; | |
| const location = useLocation(); | |
| const navigate = useNavigate(); | |
| // Find the current nav index by matching the most specific path first | |
| const matchIndex = [...navConfig] | |
| .map((cfg, idx) => ({ idx, len: cfg.path.length, match: location.pathname.startsWith(cfg.path) })) | |
| .filter(x => x.match) | |
| .sort((a, b) => b.len - a.len)[0]?.idx ?? 0; | |
| export function BottomBar() { | |
| // Get the current language for citizen from Redux store | |
| const lang = useAppSelector((state) => state.lang.citizenLang); | |
| // Get localization loading state | |
| const { loading } = useLocalization(); | |
| // Get localized messages for the citizen module | |
| const messages = getMessagesFromSession("CITIZEN")!; | |
| const location = useLocation(); | |
| const navigate = useNavigate(); | |
| // Show loader while localization messages are loading | |
| if (loading) { | |
| return <LoadingPage />; | |
| } | |
| // Navigation configuration for the bottom bar (desired order) | |
| const navConfig = [ | |
| { | |
| label: messages['citizen.commons'][lang]['home-btn'], | |
| icon: <HomeIcon />, | |
| iconFilled: <HomeIconFilled />, | |
| path: '/citizen', | |
| }, | |
| { | |
| label: messages['citizen.commons'][lang]['my-city-btn'], | |
| icon: <ApartmentOutlinedIcon />, | |
| iconFilled: <ApartmentIcon />, | |
| path: '/under-construction', | |
| }, | |
| { | |
| label: messages['citizen.commons'][lang]['properties-btn'], | |
| icon: <CottageOutlinedIcon />, | |
| iconFilled: <CottageIcon />, | |
| path: '/citizen/properties', | |
| }, | |
| { | |
| label: messages['citizen.commons'][lang]['utility-btn'], | |
| icon: <OfflineBoltOutlinedIcon />, | |
| iconFilled: <OfflineBoltIcon />, | |
| path: '/under-construction', | |
| }, | |
| ]; | |
| // Find the current nav index by matching the most specific path first | |
| const matchIndex = [...navConfig] | |
| .map((cfg, idx) => ({ idx, len: cfg.path.length, match: location.pathname.startsWith(cfg.path) })) | |
| .filter(x => x.match) | |
| .sort((a, b) => b.len - a.len)[0]?.idx ?? 0; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 62-62: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 63-63: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In mobile-ui/src/app/components/BottomBar.tsx around lines 21 to 70, the
useLocation and useNavigate hooks are called after an early return (when
loading), violating the Rules of Hooks; move the useLocation and useNavigate
calls above the loading conditional so all hooks (useAppSelector,
useLocalization, useLocation, useNavigate) are invoked unconditionally and in
the same order on every render, then keep the loading check and early return
afterwards and leave the rest of the navConfig and matchIndex logic unchanged.
| const NavBar: FC = () => { | ||
| const navigate = useNavigate(); // React Router navigation hook | ||
| const lang = useAppSelector((state) => state.lang.citizenLang); // Get current citizen language | ||
| const { loading } = useLocalization(); // Localization loading state | ||
| const messages = getMessagesFromSession('CITIZEN')!; // Localized messages for citizen module | ||
|
|
||
| // Show loader while localization messages are loading | ||
| if (loading) { | ||
| return <LoadingPage />; | ||
| } |
There was a problem hiding this comment.
Avoid potential crash: don’t non-null assert messages before the loading/availability guard.
Line 24 (getMessagesFromSession('CITIZEN')!) executes before the loading early-return, so initial load / empty session can throw.
const NavBar: FC = () => {
const navigate = useNavigate(); // React Router navigation hook
const lang = useAppSelector((state) => state.lang.citizenLang); // Get current citizen language
const { loading } = useLocalization(); // Localization loading state
- const messages = getMessagesFromSession('CITIZEN')!; // Localized messages for citizen module
// Show loader while localization messages are loading
if (loading) {
return <LoadingPage />;
}
+
+ const messages = getMessagesFromSession('CITIZEN');
+ if (!messages) return <LoadingPage message="Loading translations…" />;🤖 Prompt for AI Agents
In mobile-ui/src/app/components/NavBar.tsx around lines 20 to 29, the code
non-null asserts messages with getMessagesFromSession('CITIZEN')! before the
loading early-return which can throw on initial load; remove the non-null
assertion and either (a) move the getMessagesFromSession call below the if
(loading) return so it only runs after localization is ready, or (b) keep it
where it is but defensively check for null/undefined and handle that case (e.g.,
return LoadingPage or a safe fallback) before using messages.
| const lang = getAppLocale(); | ||
| // Get localized messages for the given role (CITIZEN or AGENT) | ||
| const messages = getMessagesFromSession(role === "CITIZEN" ? "CITIZEN" : "AGENT")!; |
There was a problem hiding this comment.
Remove non-null assertion and handle potential null messages.
Line 18 uses the non-null assertion operator (!) on getMessagesFromSession, which assumes it always returns a value. If it returns null or undefined, this will cause a runtime error when accessing nested properties on line 71.
Additionally, the ternary operator role === "CITIZEN" ? "CITIZEN" : "AGENT" is redundant once the role prop is typed as a union.
Apply this diff to add defensive null handling and remove redundancy:
const lang = getAppLocale();
- const messages = getMessagesFromSession(role === "CITIZEN" ? "CITIZEN" : "AGENT")!;
+ const messages = getMessagesFromSession(role);
const navigate = useNavigate();
+
+ // Handle missing messages gracefully
+ if (!messages) {
+ console.warn(`NavHeader: No messages found for role ${role}`);
+ }Then update line 71 to handle the null case:
- {(role === "CITIZEN" ? messages['citizen.commons'][lang]['prev-btn'] : messages['common'][lang]['previous.btn']) ?? "Previous"}
+ {(messages && role === "CITIZEN" ? messages['citizen.commons']?.[lang]?.['prev-btn'] : messages?.['common']?.[lang]?.['previous.btn']) ?? "Previous"}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In mobile-ui/src/app/components/NavHeader.tsx around lines 16 to 18, replace the
non-null assertion on getMessagesFromSession and the redundant ternary by
calling getMessagesFromSession(role) without "!" and guard its return value; if
messages is null/undefined, provide a safe fallback (e.g., defaultMessages or an
empty object) and propagate that fallback so later accesses are safe. Update the
code where messages is used around line 71 to check for messages before
accessing nested properties (or use optional chaining with a default), and
ensure the role prop is treated as the union type directly rather than using the
ternary.
| export const Popup: React.FC<PopupProps> = ({ | ||
| type = 'information', // Type of popup (alert, information, warning, success) | ||
| title, | ||
| message, | ||
| open, | ||
| duration = 3000, // Duration in ms before auto-close | ||
| }) => { | ||
| // Internal visibility state for animation and auto-close | ||
| const [visible, setVisible] = useState(open); | ||
|
|
||
| // Sync visibility with open prop | ||
| useEffect(() => { | ||
| setVisible(open); | ||
| }, [open]); | ||
|
|
||
| // Auto-close after duration if visible | ||
| useEffect(() => { | ||
| if (!visible) return; | ||
| if (duration > 0) { | ||
| const timer = window.setTimeout(() => { | ||
| setVisible(false); | ||
| }, duration); | ||
| return () => clearTimeout(timer); | ||
| } | ||
| }, [visible, duration]); | ||
|
|
There was a problem hiding this comment.
onClose is ignored; auto-close and Modal close won’t update parent state.
PopupProps is passed onClose, but this component never calls it (and doesn’t even accept it in props). That can leave the parent open stuck true, and the toast may not reopen reliably.
export const Popup: React.FC<PopupProps> = ({
type = 'information', // Type of popup (alert, information, warning, success)
title,
message,
- open,
+ open = false,
duration = 3000, // Duration in ms before auto-close
+ onClose,
}) => {
// Internal visibility state for animation and auto-close
const [visible, setVisible] = useState(open);
+ const handleClose = () => {
+ setVisible(false);
+ onClose?.();
+ };
+
// Sync visibility with open prop
useEffect(() => {
setVisible(open);
}, [open]);
// Auto-close after duration if visible
useEffect(() => {
if (!visible) return;
if (duration > 0) {
const timer = window.setTimeout(() => {
- setVisible(false);
+ handleClose();
}, duration);
return () => clearTimeout(timer);
}
- }, [visible, duration]);
+ }, [visible, duration, onClose]);
// Render MUI Modal with Fade transition and custom styling
return (
<Modal
open={visible}
- onClose={() => {
- setVisible(false);
- }}
+ onClose={handleClose}
closeAfterTransition
slots={{ backdrop: Backdrop }}
slotProps={{
backdrop: {
timeout: 200,
sx: { backgroundColor: 'transparent' },
},
}}Also applies to: 68-83, 91-145
🤖 Prompt for AI Agents
In mobile-ui/src/app/components/Popup/Popup.tsx around lines 37-62 (also apply
same fix to 68-83 and 91-145), the component never accepts or calls the onClose
callback so parent open state is never updated when the popup auto-closes or is
dismissed; update the component props to include onClose: () => void, call
onClose whenever you set visible to false (both in the auto-close timeout
cleanup and in any manual/Modal close handlers), and ensure you still manage
internal visible state for animations but always call props.onClose after
starting the closing animation so the parent can set open=false; also clear
timers on unmount to avoid calling onClose after unmount.
| const NoProperties: React.FC = () => { | ||
| const lang = useAppSelector((state) => state.lang.citizenLang); // Current language | ||
| const { loading: localizationLoading } = useLocalization(); // Global loading state | ||
| const messages = getMessagesFromSession('CITIZEN')!; // Localized messages | ||
|
|
||
| // Show loader if localization is loading | ||
| if (localizationLoading) { | ||
| return <LoadingPage />; | ||
| } |
There was a problem hiding this comment.
Fix potential crash: don’t read session messages before localization is ready.
getMessagesFromSession('CITIZEN')! runs before the loading guard; if session messages aren’t populated yet, this can throw even while localizationLoading is true.
const NoProperties: React.FC = () => {
const lang = useAppSelector((state) => state.lang.citizenLang); // Current language
const { loading: localizationLoading } = useLocalization(); // Global loading state
- const messages = getMessagesFromSession('CITIZEN')!; // Localized messages
// Show loader if localization is loading
if (localizationLoading) {
return <LoadingPage />;
}
+ const messages = getMessagesFromSession('CITIZEN');
+ if (!messages?.['citizen.no-property']?.[lang]) return <LoadingPage />;🤖 Prompt for AI Agents
In mobile-ui/src/app/features/Citizen/components/NoProperties.tsx around lines
19 to 27, the code calls getMessagesFromSession('CITIZEN')! before checking
localizationLoading which can throw if session messages are not yet populated;
move the call (or otherwise defer reading session messages) so it happens after
the localizationLoading guard, and add a safe null/undefined check (avoid using
the non-null assertion) — e.g., return the LoadingPage while localizationLoading
is true, then call getMessagesFromSession and handle a missing value gracefully
(fallback or early return) before using messages.
| import { type FC } from 'react'; | ||
| import { Paper, Typography, Button, Box, Chip } from '@mui/material'; | ||
| import { useNavigate } from 'react-router-dom'; | ||
| import markerSvg from '../../../assets/Citizen/property_page/location.svg'; | ||
| import { Edit } from '@mui/icons-material'; | ||
| import { useAppSelector } from '../../../../redux/Hooks'; | ||
| import { | ||
| getMessagesFromSession, | ||
| useLocalization, | ||
| } from '../../../../services/Citizen/Localization/LocalizationContext'; | ||
| import LoadingPage from '../../../components/Loader'; | ||
| import MapComponent from './MapComponent'; | ||
| import { useFormMode } from '../../../../context/FormModeContext'; | ||
|
|
There was a problem hiding this comment.
Fix TS/JSX typing + remove hard-coded MapTiler key from client
React.MouseEvent is referenced but React isn’t imported (likely TS error with the modern JSX transform). Also the MapTiler key in tileUrl will be shipped to all clients; move to env/config (and restrict it on the provider side).
-import { type FC } from 'react';
+import type { FC, MouseEvent as ReactMouseEvent } from 'react';
@@
- const stopPropagation = (e: React.MouseEvent) => e.stopPropagation();
+ const stopPropagation = (e: ReactMouseEvent) => e.stopPropagation();
@@
- const handleCompleteForm = (e?: React.MouseEvent) => {
+ const handleCompleteForm = (e?: ReactMouseEvent) => {
@@
<MapComponent
latLng={{ lat: latLng.lat, lng: latLng.lng }}
- tileUrl="https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc"
+ tileUrl={import.meta.env.VITE_MAPTILER_STYLE_URL}
customIcon={customIcon}
/>Also applies to: 98-117, 149-296
🤖 Prompt for AI Agents
In mobile-ui/src/app/features/Citizen/components/PropertyCard.tsx around lines
4-17 (and also apply to 98-117 and 149-296): fix the missing React type import
and remove the hard-coded MapTiler key. Add a type import from React (for
example: import type { MouseEvent } from 'react';) and update any
React.MouseEvent usages to use that imported MouseEvent (or import React as
needed) so TS no longer complains under the modern JSX transform. Replace the
literal MapTiler key in tileUrl with a config/env value (e.g.
process.env.REACT_APP_MAPTILER_KEY or import from a config module), and ensure
the provider-side key restrictions are used; apply both changes to all other
occurrences in the specified line ranges.
| const MAP_URL = | ||
| 'https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc'; | ||
| const LANDUSE_URL = | ||
| 'https://api.maptiler.com/maps/streets-v2/style.json?key=YguiTF06mLtcpSVKIQyc'; |
There was a problem hiding this comment.
Move API keys to environment variables.
The MapTiler API key is hardcoded in the source code, which is a critical security issue. Anyone with access to the client-side code can extract and misuse this key, potentially leading to unauthorized usage, unexpected charges, or service disruption.
Move the API key to environment variables:
-const MAP_URL =
- 'https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc';
-const LANDUSE_URL =
- 'https://api.maptiler.com/maps/streets-v2/style.json?key=YguiTF06mLtcpSVKIQyc';
+const MAP_URL =
+ `https://api.maptiler.com/maps/base-v4/style.json?key=${import.meta.env.VITE_MAPTILER_API_KEY}`;
+const LANDUSE_URL =
+ `https://api.maptiler.com/maps/streets-v2/style.json?key=${import.meta.env.VITE_MAPTILER_API_KEY}`;Then add the key to your .env file and ensure .env is in .gitignore.
🤖 Prompt for AI Agents
In mobile-ui/src/app/features/Citizen/components/PropertyMapSection.tsx around
lines 20 to 23, the MapTiler API key is hardcoded in the MAP_URL and LANDUSE_URL
constants; replace that by reading the key from an environment variable (e.g.
process.env.REACT_APP_MAPTILER_KEY or import.meta.env.VITE_MAPTILER_KEY
depending on build system) and construct the URLs using that value, add a
runtime check that throws or logs a clear error if the env var is missing, add
the key to .env (e.g. REACT_APP_MAPTILER_KEY=...) and ensure .env is listed in
.gitignore so the key is not committed.
| import type { UrgentAttention } from "../models/UrgentAttention.model"; | ||
| import type { Property } from "../models/Property.model"; |
There was a problem hiding this comment.
Incorrect relative import paths.
The imports use ../models/ which attempts to go up one directory from models and then into models again. Since all three files (CitizenHome.model.ts, UrgentAttention.model.ts, and Property.model.ts) are in the same mobile-ui/src/app/features/Citizen/models/ directory, the imports should use ./ instead of ../models/.
Apply this diff to fix the imports:
-import type { UrgentAttention } from "../models/UrgentAttention.model";
-import type { Property } from "../models/Property.model";
+import type { UrgentAttention } from "./UrgentAttention.model";
+import type { Property } from "./Property.model";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { UrgentAttention } from "../models/UrgentAttention.model"; | |
| import type { Property } from "../models/Property.model"; | |
| import type { UrgentAttention } from "./UrgentAttention.model"; | |
| import type { Property } from "./Property.model"; |
🤖 Prompt for AI Agents
In mobile-ui/src/app/features/Citizen/models/CitizenHome.model.ts around lines 1
to 2, the import paths incorrectly use "../models/..." which navigates out of
the current models directory; update the imports to use relative local paths
(./UrgentAttention.model and ./Property.model) so they reference the files in
the same directory.
| export type ISGRDetails = { | ||
| doorNoTo: string; // Door number (to) | ||
| igrsWard: string; // IGRS ward | ||
| igrsBlock: string; // IGRS block | ||
| doorNoFrom: string; // Door number (from) | ||
| habitation: string; // Habitation name | ||
| igrsLocality: string; // IGRS locality | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n --type=ts --type=tsx -C2 'ISGRDetails|IGRSDetails|isgrDetails|igrsDetails'Repository: egovernments/Infosys
Length of output: 91
🏁 Script executed:
rg -n --type=ts -C2 'ISGRDetails|IGRSDetails|isgrDetails|igrsDetails'Repository: egovernments/Infosys
Length of output: 31949
🏁 Script executed:
cat -n mobile-ui/src/app/features/Citizen/models/IGRSDetails.model.tsRepository: egovernments/Infosys
Length of output: 466
Fix typo in exported type name and add missing semicolon
The exported type ISGRDetails has swapped letters—should be IGRSDetails to match the filename and domain acronym. Also add the trailing semicolon.
-export type ISGRDetails = {
+export type IGRSDetails = {
doorNoTo: string; // Door number (to)
igrsWard: string; // IGRS ward
igrsBlock: string; // IGRS block
doorNoFrom: string; // Door number (from)
habitation: string; // Habitation name
igrsLocality: string; // IGRS locality
-}
+};This is a breaking change affecting imports across the codebase (Property.model.ts, propertyApi.ts, PropertyFormContext.tsx, formValidations.ts, and others).
📝 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.
| export type ISGRDetails = { | |
| doorNoTo: string; // Door number (to) | |
| igrsWard: string; // IGRS ward | |
| igrsBlock: string; // IGRS block | |
| doorNoFrom: string; // Door number (from) | |
| habitation: string; // Habitation name | |
| igrsLocality: string; // IGRS locality | |
| } | |
| export type IGRSDetails = { | |
| doorNoTo: string; // Door number (to) | |
| igrsWard: string; // IGRS ward | |
| igrsBlock: string; // IGRS block | |
| doorNoFrom: string; // Door number (from) | |
| habitation: string; // Habitation name | |
| igrsLocality: string; // IGRS locality | |
| }; |
🤖 Prompt for AI Agents
In mobile-ui/src/app/features/Citizen/models/IGRSDetails.model.ts around lines 2
to 9, the exported type name is misspelled as ISGRDetails and is missing a
trailing semicolon; rename the type to IGRSDetails and add the semicolon after
the type declaration. Update all affected imports/usages across the codebase
(e.g., Property.model.ts, propertyApi.ts, PropertyFormContext.tsx,
formValidations.ts and any other files) to import and reference IGRSDetails
instead of ISGRDetails to avoid breaking changes.
…ment in E-gov repository
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
♻️ Duplicate comments (16)
frontend/mobile-ui/src/app/components/NavHeader.tsx (2)
16-18: The non-null assertion and redundant ternary issues remain unresolved.This code segment still contains the issues flagged in the previous review:
- Line 18 uses the non-null assertion operator (
!) which will cause runtime errors ifgetMessagesFromSessionreturnsnullorundefined- The ternary
role === "CITIZEN" ? "CITIZEN" : "AGENT"is redundant onceroleis properly typedPlease address the previous review comment to add null handling and remove the redundancy.
14-14: Type theroleprop as a union instead ofstring.The
roleprop accepts only"CITIZEN"or"AGENT", but it's typed asstring. This weakens type safety and allows invalid values to be passed.🔎 Proposed fix
-const NavHeader: FC<{role : string}> = ({role}) => { +const NavHeader: FC<{role: "CITIZEN" | "AGENT"}> = ({role}) => {frontend/mobile-ui/src/app/features/Citizen/models/IGRSDetails.model.ts (1)
2-9: Acknowledge previous review: Type name typo already flagged.The past review correctly identified that
ISGRDetailsshould beIGRSDetailsand the missing semicolon. As noted, this is a breaking change affecting multiple files across the codebase.Note: Similar to the acronym concern in
IGRSAdditionalDetails.model.ts, verify whether "Integrated Grievance Redressal System" is the correct expansion for IGRS in this property tax context, as the fields (doorNoTo, igrsWard, igrsBlock, habitation, igrsLocality) suggest property location/registration data rather than grievance-related information.frontend/mobile-ui/src/app/features/Citizen/api/CitizenDocumentPageApi/fileStoreApi.ts (2)
16-24:responseHandlerand Content-Type issues previously flagged remain unaddressed.The previous review identified that
responseHandleris not a standard RTK QueryfetchBaseQueryfeature and the parent slice'sContent-Type: application/jsonheader will interfere with blob responses. These issues have not been resolved.
18-19: URL parameters should be encoded to prevent malformed URLs.If
fileStoreIdortenantIdcontain special characters (e.g., spaces,&,?), the URL will be malformed, causing request failures.🔎 Proposed fix
- query: ({ fileStoreId, tenantId = "pg" }) => ({ - url: `/filestore/v1/files/${fileStoreId}?tenantId=${tenantId}`, + query: ({ fileStoreId, tenantId = "pg" }) => ({ + url: `/filestore/v1/files/${encodeURIComponent(fileStoreId)}?tenantId=${encodeURIComponent(tenantId)}`,frontend/mobile-ui/src/app/components/Popup/Popup.tsx (1)
37-62:onClosecallback is still missing; parent state won't update on auto-close or modal dismissal.This issue was previously flagged and remains unresolved. The component never accepts or calls the
onClosecallback, leaving the parent'sopenstate stuck attruewhen the popup auto-closes or is dismissed via backdrop click.🔎 Proposed fix (as suggested in previous review)
export const Popup: React.FC<PopupProps> = ({ type = 'information', title, message, - open, + open = false, duration = 3000, + onClose, }) => { const [visible, setVisible] = useState(open); + const handleClose = () => { + setVisible(false); + onClose?.(); + }; + useEffect(() => { setVisible(open); }, [open]); useEffect(() => { if (!visible) return; if (duration > 0) { const timer = window.setTimeout(() => { - setVisible(false); + handleClose(); }, duration); return () => clearTimeout(timer); } - }, [visible, duration]); + }, [visible, duration, onClose]); return ( <Modal open={visible} - onClose={() => { - setVisible(false); - }} + onClose={handleClose} closeAfterTransition ...Also applies to: 68-83
frontend/mobile-ui/src/app/features/Citizen/models/CitizenHome.model.ts (1)
1-2: Incorrect relative import paths.The imports use
../models/which navigates out of the current models directory and then back into models. Since all three files are in the samemobile-ui/src/app/features/Citizen/models/directory, use./instead.🔎 Proposed fix
-import type { UrgentAttention } from "../models/UrgentAttention.model"; -import type { Property } from "../models/Property.model"; +import type { UrgentAttention } from "./UrgentAttention.model"; +import type { Property } from "./Property.model";frontend/mobile-ui/src/app/features/Agent/components/ChromeTabs.tsx (1)
65-65: Incorrect CSS property for flexbox layout.
justifyItemsis a CSS Grid property and has no effect on flexbox layouts. Since the container usesdisplay: 'flex', you should usejustifyContentinstead to center items horizontally.🔎 Proposed fix
const TabsContainer = styled(Box)({ display: 'flex', alignItems: 'center', - justifyItems:'center', + justifyContent: 'center', width: '338px', border: '1px solid #000',frontend/mobile-ui/src/app/features/Citizen/components/NoProperties.tsx (1)
19-27: Fix potential crash: don't read session messages before localization is ready.
getMessagesFromSession('CITIZEN')!runs before the loading guard on line 22. If session messages aren't populated yet, this can throw or return undefined even whilelocalizationLoadingis true, causing crashes when accessing message properties on lines 54, 59, and 62.🔎 Proposed fix
const NoProperties: React.FC = () => { const lang = useAppSelector((state) => state.lang.citizenLang); const { loading: localizationLoading } = useLocalization(); - const messages = getMessagesFromSession('CITIZEN')!; if (localizationLoading) { return <LoadingPage />; } + + const messages = getMessagesFromSession('CITIZEN'); + if (!messages?.['citizen.no-property']?.[lang]) { + return <LoadingPage />; + }frontend/mobile-ui/src/app/features/Agent/utils/propertyMapper.ts (1)
7-36: Fix incorrectstatusmapping and guard against missingapplication.Property(runtime crash).Two critical issues:
- Line 8:
application.Propertyis accessed without a null check. IfPropertyis missing, lines 15-35 will crash.- Line 24:
statusis mapped fromapplication.Priority, butisNew(line 27) andisVerified(line 29) checkapplication.Status. This inconsistency suggestsstatusshould useStatusnotPriority.🔎 Proposed fix
export const mapApplicationToPropertyItem = (application: Application): PropertyItem => { const property = application.Property; + + if (!property) { + return { + id: application.ID, + pId: application.ApplicationNo, + propertyNo: "N/A", + type: "RESIDENTIAL", + description: "Property details unavailable", + address: "Address not available", + phoneNumber: "", + area: "N/A", + propertyType: "RESIDENTIAL", + status: application.Status ?? "UNKNOWN", + dueDate: application.DueDate ?? new Date().toISOString(), + isNew: application.Status === "ASSIGNED", + isDraft: application.IsDraft === true, + isVerified: + application.Status === "VERIFIED" || + application.Status === "APPROVED" || + application.Status === "AUDIT_VERIFIED", + createdDate: application.CreatedAt, + gisData: undefined, + }; + } return { id: application.ID, pId: application.ApplicationNo, propertyNo: property.PropertyNo || 'N/A', type: property.PropertyType || 'RESIDENTIAL', description: `${property.PropertyType} - ${property.OwnershipType}`, address: property.Address - ? `${property.Address.Street}, ${property.Address.Locality}` + ? [property.Address.Street, property.Address.Locality].filter(Boolean).join(", ") || "Address not available" : 'Address not available', phoneNumber: '', area: property.AssessmentDetails?.ExtendOfSite || 'N/A', propertyType: property.PropertyType || 'RESIDENTIAL', - status: application.Priority || 'MEDIUM', + status: application.Status ?? 'UNKNOWN', dueDate: application.DueDate || new Date().toISOString(), isNew: (application.Status === 'ASSIGNED'), isDraft: application.IsDraft === true, isVerified: application.Status === 'VERIFIED' || application.Status === 'APPROVED' || application.Status === 'AUDIT_VERIFIED', createdDate: application.CreatedAt, gisData: property.GISData ? { latitude: property.GISData.Latitude, longitude: property.GISData.Longitude, coordinates: property.GISData.Coordinates } : undefined, }; };frontend/mobile-ui/src/app/components/NavBar.tsx (1)
20-29: Non-null assertion before loading guard can cause runtime errors.The
getMessagesFromSession('CITIZEN')!call on line 24 executes before the loading early-return on line 27-29. If the session is empty during initial load, this will throw.This issue was already raised in a previous review. Move the
getMessagesFromSessioncall after the loading check and add a defensive null check.frontend/mobile-ui/src/app/features/Citizen/components/PropertyCard.tsx (2)
99-99: TypeScript error:React.MouseEventreferenced without React namespace import.
React.MouseEventis used butReactisn't imported as a namespace. This will cause a TypeScript error with the modern JSX transform.This was flagged in a previous review. Import
MouseEventfrom React or useReact.MouseEventwith a proper namespace import.
291-295: Hardcoded MapTiler API key will be exposed to all clients.The API key in the
tileUrlis shipped in the client bundle. This is a security risk as the key can be extracted and abused.This was flagged in a previous review. Move the key to an environment variable (e.g.,
import.meta.env.VITE_MAPTILER_STYLE_URL) and configure domain restrictions on the MapTiler provider side.frontend/mobile-ui/src/app/components/BottomBar.tsx (1)
62-63: Hooks called after early return violates React's Rules of Hooks.
useLocationanduseNavigateare called after the early return on lines 30-32. This breaks React's requirement that hooks must be called unconditionally in the same order on every render.This was flagged in a previous review and by static analysis. Move these hooks before the loading check.
frontend/mobile-ui/src/app/features/Citizen/components/PropertyMapSection.tsx (1)
20-23: Hardcoded API key - already flagged in previous review.This issue was previously identified. Move the MapTiler API key to environment variables as suggested in the past review.
frontend/mobile-ui/src/app/features/Agent/components/Layout.tsx (1)
34-34: Usestringfor Aadhaar numbers to prevent precision issues.Aadhaar numbers are 12-digit identifiers that should be stored as strings. JavaScript's number type can represent 12-digit integers within
MAX_SAFE_INTEGER, but treating identification numbers as strings prevents inappropriate arithmetic operations and serialization issues.🔎 Proposed fix
-adhaarNo: number; +adhaarNo: string;Based on past review comments, this issue was previously identified.
| <input | ||
| id="file-input" | ||
| type="file" | ||
| hidden | ||
| onChange={handleFileChange} | ||
| accept=".pdf,.doc,.docx,.jpg,.jpeg,.png" | ||
| /> |
There was a problem hiding this comment.
Add server-side file validation.
The file type validation only occurs on the client side via the accept attribute. Malicious users can bypass this and upload arbitrary files, creating a security risk.
Ensure server-side validation checks:
- File type/MIME type
- File size limits
- Content scanning for malicious files
Additionally, add client-side file size validation for better UX:
🔎 Proposed client-side size validation
const handleFileChange = (event: React.ChangeEvent<HTMLInputElement>) => {
const file = event.target.files?.[0];
+ if (file) {
+ const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB
+ if (file.size > MAX_FILE_SIZE) {
+ alert('File size must be less than 10MB');
+ return;
+ }
+ }
setSelectedFile(file);
};Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/mobile-ui/src/app/components/common/RequestDialog.tsx around lines
118 to 124, the file input only uses the client-side accept attribute which can
be bypassed; add client-side file size validation to provide immediate UX
feedback (e.g., reject files over the configured maxBytes and show an error) and
update handleFileChange to enforce that before setting state. Then implement
server-side validation in the upload endpoint: verify the multipart file's MIME
type against an allowed list, validate the actual content (use server-side
MIME/sniffing, not just the client-provided type), enforce maximum file size
limits, perform content scanning (antivirus/malware scanning or a
safe-file-parser) and reject any disallowed files with appropriate status codes
and error messages; ensure logs record validation failures and user-facing
errors are sanitized.
| : [ | ||
| { | ||
| key: 'application-updates', | ||
| title: 'application-updates', | ||
| desc: 'application-updates-desc', | ||
| }, | ||
| { | ||
| key: 'appointment-scheduling', | ||
| title: 'appointment-scheduling', | ||
| desc: 'appointment-scheduling-desc', | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
Agent notification categories bypass localization, breaking UX.
Lines 71–72 and 76–77 use hardcoded string literals ('application-updates', 'application-updates-desc', etc.) instead of retrieving localized messages like the citizen flow does. When rendered on lines 130 and 133, agent users will see raw internal keys instead of translated, human-readable text.
🔎 Proposed fix to apply localization pattern consistently
: [
{
key: 'application-updates',
- title: 'application-updates',
- desc: 'application-updates-desc',
+ title: messages['profile'][lang]['application-updates'],
+ desc: messages['profile'][lang]['application-updates-desc'],
},
{
key: 'appointment-scheduling',
- title: 'appointment-scheduling',
- desc: 'appointment-scheduling-desc',
+ title: messages['profile'][lang]['appointment-scheduling'],
+ desc: messages['profile'][lang]['appointment-scheduling-desc'],
},
];🤖 Prompt for AI Agents
In frontend/mobile-ui/src/app/components/NotificationCard.tsx around lines 68 to
79, the notification category entries use hardcoded key strings
('application-updates', 'application-updates-desc', 'appointment-scheduling',
'appointment-scheduling-desc') so agents see raw keys instead of translated
labels; replace those literal strings with calls to the app's localization
function (the same t(...) or useTranslation pattern used in the citizen flow)
for both title and desc values, ensuring you pass the correct translation keys
and preserve the object shape so the render on lines ~130 and ~133 displays
human-readable, localized text.
| export const SupportHelp: React.FC = () => { | ||
| // Get current language and localized messages for Citizen | ||
| const lang = useAppSelector(state => state.lang.citizenLang); | ||
| const messages = getMessagesFromSession("CITIZEN")!; |
There was a problem hiding this comment.
Remove non-null assertion and add defensive handling.
The non-null assertion operator (!) suppresses null/undefined checks. If getMessagesFromSession returns null or undefined, the component will throw a runtime error when accessing messages["profile"].
🔎 Proposed fix with defensive handling
- const messages = getMessagesFromSession("CITIZEN")!;
+ const messages = getMessagesFromSession("CITIZEN");
+
+ if (!messages) {
+ return <Typography>Loading support information...</Typography>;
+ }Alternatively, provide a fallback or use optional chaining throughout the component.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const messages = getMessagesFromSession("CITIZEN")!; | |
| const messages = getMessagesFromSession("CITIZEN"); | |
| if (!messages) { | |
| return <Typography>Loading support information...</Typography>; | |
| } |
🤖 Prompt for AI Agents
In frontend/mobile-ui/src/app/components/SupportCard.tsx around line 22, remove
the non-null assertion on getMessagesFromSession("CITIZEN") and add defensive
handling: call getMessagesFromSession and store the result in a const without
'!', then if the result may be null/undefined either (a) provide a safe default
(e.g., const messages = result ?? { profile: {}, ... } ) or (b) short-circuit
render (return null or a placeholder) when no messages exist; also update any
property access to use optional chaining (e.g., messages?.profile) or guarded
access so the component never dereferences undefined.
| }) => { | ||
| const lang = useAppSelector(state => state.lang.citizenLang); // Current language | ||
| const { loading } = useLocalization(); // Global loading state | ||
| const messages = getMessagesFromSession("CITIZEN")!; // Localized messages |
There was a problem hiding this comment.
Non-null assertion could cause runtime error.
The non-null assertion getMessagesFromSession("CITIZEN")! will throw if the function returns null or undefined, causing the component to crash.
🔎 Proposed fix with safe fallback
- const messages = getMessagesFromSession("CITIZEN")!;
+ const messages = getMessagesFromSession("CITIZEN");
+
+ // Show loader if messages are not yet available
+ if (!messages) {
+ return <LoadingPage />;
+ }Committable suggestion skipped: line range outside the PR's diff.
| const styleUrl = | ||
| 'https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc'; |
There was a problem hiding this comment.
Hardcoded API key exposes credentials in source control.
The MapTiler API key is hardcoded in line 263. This is a security risk as the key will be visible in version control and client-side code.
🔎 Proposed fix
+ // Use environment variable for API key
- const styleUrl =
- 'https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc';
+ const apiKey = import.meta.env.VITE_MAPTILER_API_KEY || '';
+ const styleUrl = `https://api.maptiler.com/maps/base-v4/style.json?key=${apiKey}`;Add to .env file:
VITE_MAPTILER_API_KEY=YguiTF06mLtcpSVKIQyc
Add .env to .gitignore if not already present.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const styleUrl = | |
| 'https://api.maptiler.com/maps/base-v4/style.json?key=YguiTF06mLtcpSVKIQyc'; | |
| // Use environment variable for API key | |
| const apiKey = import.meta.env.VITE_MAPTILER_API_KEY || ''; | |
| const styleUrl = `https://api.maptiler.com/maps/base-v4/style.json?key=${apiKey}`; |
🤖 Prompt for AI Agents
In frontend/mobile-ui/src/app/features/Citizen/components/MapView.tsx around
lines 262-263 the MapTiler API key is hardcoded into the styleUrl which exposes
credentials; remove the literal key and read it from an environment variable
(e.g. VITE_MAPTILER_API_KEY) via import.meta.env, construct the styleUrl using
that variable, and add a runtime check to fail fast or provide a clear error if
the env var is missing; add VITE_MAPTILER_API_KEY to the project .env (not
committed) and ensure .env is listed in .gitignore so the key is not checked
into source control.
|
|
||
| // Map API data to display fields | ||
| // const surveyNumber = property.AdditionalDetails?.fieldValue?.serialNo?.toString() || '-'; | ||
| const complexName = property.ComplexName.toString() || '-'; |
There was a problem hiding this comment.
Unsafe .toString() call on ComplexName without null check.
Line 39 calls .toString() on property.ComplexName without checking if it's null or undefined, which will throw a runtime error. Other fields (lines 40-50) properly use optional chaining with fallbacks.
🔎 Proposed fix
- const complexName = property.ComplexName.toString() || '-';
+ const complexName = property.ComplexName?.toString() || '-';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const complexName = property.ComplexName.toString() || '-'; | |
| const complexName = property.ComplexName?.toString() || '-'; |
🤖 Prompt for AI Agents
In frontend/mobile-ui/src/app/features/Citizen/components/ui/PlotInfoCard.tsx
around line 39, property.ComplexName is being invoked with .toString() without a
null/undefined check which can throw at runtime; replace the unsafe call with a
safe conversion like using optional chaining and a fallback (e.g.,
property.ComplexName?.toString() ?? '-') or coerce with
String(property.ComplexName ?? '-') so the value is always defined and matches
the pattern used for the other fields.
| // Represents assessment details for a property | ||
| export type AssessmentDetails = { | ||
| reason: string; // Reason for assessment creation | ||
| extentOfSite: number; // Area/extent of the site | ||
| landUnderBuilding: number; // Area of land under the building | ||
| isUnspecifiedShare: boolean; // Whether the property has an unspecified share | ||
| occupancyCertificateDate: string; // Date of occupancy certificate | ||
| occupancyCertificateNumber: string; // Occupancy certificate number | ||
| } No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for AssessmentDetails type definitions across the codebase
echo "=== Searching for AssessmentDetails type definitions ==="
rg -n 'type AssessmentDetails|interface AssessmentDetails' --type=ts
echo -e "\n=== Searching for all AssessmentDetails references ==="
rg -n 'AssessmentDetails' --type=ts --type=tsx | head -30Repository: egovernments/Infosys
Length of output: 1588
Consolidate duplicate AssessmentDetails type definitions across the codebase.
At least 8 files currently export AssessmentDetails type with the same name but different definitions:
redux/apis/propertyApi.tscontext/PropertyFormContext.tsxredux/apis/ApplicationLog/Modal/ApplicationModals.tsapp/features/Agent/models/SearchPropertyData.model.tsapp/features/Agent/models/PropertyInfoSubmittedData.model.tsapp/features/Agent/models/HomePageData.model.tsapp/features/Citizen/models/CitizenPropertiesPageModel/CitizenPropertyPageModel.tsapp/features/Citizen/models/AssessmentDetails.model.ts(this file)
This creates type safety risks and import ambiguity. Consider consolidating into a single, canonical definition in a shared models directory or centralizing through a barrel export with renamed imports to distinguish each variant (e.g., AssessmentDetailsAPI, AssessmentDetailsForm).
Additionally, review whether all fields should remain required—properties like occupancy certificate details may not always be available depending on the use case.
| updateDocumentUploadDetails: builder.mutation<DocumentUploadDetailsResponse, { document_id: string; body: DocumentUploadDetailsRequest }>({ | ||
| query: ({ document_id, body }) => ({ | ||
| url: `/v1/documents/${document_id}`, | ||
| method: 'PUT', | ||
| body, | ||
| }), | ||
| }), |
There was a problem hiding this comment.
Missing cache invalidation on update mutation.
The updateDocumentUploadDetails mutation lacks an invalidatesTags property, which means the cache won't be refreshed after a successful update. This will cause the UI to display stale data until a manual refetch or page reload occurs.
🔎 Proposed fix
updateDocumentUploadDetails: builder.mutation<DocumentUploadDetailsResponse, { document_id: string; body: DocumentUploadDetailsRequest }>({
query: ({ document_id, body }) => ({
url: `/v1/documents/${document_id}`,
method: 'PUT',
body,
}),
+ invalidatesTags: [TAG_TYPES.DOCUMENT],
}),📝 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.
| updateDocumentUploadDetails: builder.mutation<DocumentUploadDetailsResponse, { document_id: string; body: DocumentUploadDetailsRequest }>({ | |
| query: ({ document_id, body }) => ({ | |
| url: `/v1/documents/${document_id}`, | |
| method: 'PUT', | |
| body, | |
| }), | |
| }), | |
| updateDocumentUploadDetails: builder.mutation<DocumentUploadDetailsResponse, { document_id: string; body: DocumentUploadDetailsRequest }>({ | |
| query: ({ document_id, body }) => ({ | |
| url: `/v1/documents/${document_id}`, | |
| method: 'PUT', | |
| body, | |
| }), | |
| invalidatesTags: [TAG_TYPES.DOCUMENT], | |
| }), |
🤖 Prompt for AI Agents
In frontend/mobile-ui/src/app/features/PropertyForm/api/documentUpload.api.ts
around lines 33 to 39, the updateDocumentUploadDetails mutation does not
invalidate cache tags after a successful PUT, so UI can show stale data; add an
invalidatesTags field to the mutation that returns the appropriate tag(s) (for
example invalidate the specific document by id using the incoming document_id:
invalidatesTags: (result, error, { document_id }) => [{ type: 'DocumentUpload',
id: document_id }] or, if you need to refresh lists, include the list tag as
well), ensuring the cache is refreshed after updates.
| const Providers: React.FC<ProvidersProps> = ({ children }) => { | ||
| // Get user role from Auth context (used for localization) | ||
| const { role } = useAuth(); | ||
| return ( | ||
| <LanguageProvider> | ||
| <SignUpFormProvider> | ||
| <PropertyApplicationsProvider> | ||
| <PropertyFormProvider> | ||
| <FormModeProvider> | ||
| {/* MUI date localization and app language localization */} | ||
| <LocalizationProvider dateAdapter={AdapterDayjs}> | ||
| <LangProvider role={role!}>{children}</LangProvider> |
There was a problem hiding this comment.
Remove unsafe non-null assertion on role.
The non-null assertion role! on line 31 will crash if role is null or undefined. This can occur during initial authentication load, logout, or when auth state is being determined.
🔎 Proposed fix to handle undefined role safely
const Providers: React.FC<ProvidersProps> = ({ children }) => {
const { role } = useAuth();
+
+ // Handle case where role might be undefined during auth initialization
+ if (!role) {
+ return null; // or return a loading state
+ }
+
return (
<LanguageProvider>
<SignUpFormProvider>
<PropertyApplicationsProvider>
<PropertyFormProvider>
<FormModeProvider>
<LocalizationProvider dateAdapter={AdapterDayjs}>
- <LangProvider role={role!}>{children}</LangProvider>
+ <LangProvider role={role}>{children}</LangProvider>
</LocalizationProvider>
</FormModeProvider>
</PropertyFormProvider>
</PropertyApplicationsProvider>
</SignUpFormProvider>
</LanguageProvider>
);
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const Providers: React.FC<ProvidersProps> = ({ children }) => { | |
| // Get user role from Auth context (used for localization) | |
| const { role } = useAuth(); | |
| return ( | |
| <LanguageProvider> | |
| <SignUpFormProvider> | |
| <PropertyApplicationsProvider> | |
| <PropertyFormProvider> | |
| <FormModeProvider> | |
| {/* MUI date localization and app language localization */} | |
| <LocalizationProvider dateAdapter={AdapterDayjs}> | |
| <LangProvider role={role!}>{children}</LangProvider> | |
| const Providers: React.FC<ProvidersProps> = ({ children }) => { | |
| // Get user role from Auth context (used for localization) | |
| const { role } = useAuth(); | |
| // Handle case where role might be undefined during auth initialization | |
| if (!role) { | |
| return null; // or return a loading state | |
| } | |
| return ( | |
| <LanguageProvider> | |
| <SignUpFormProvider> | |
| <PropertyApplicationsProvider> | |
| <PropertyFormProvider> | |
| <FormModeProvider> | |
| {/* MUI date localization and app language localization */} | |
| <LocalizationProvider dateAdapter={AdapterDayjs}> | |
| <LangProvider role={role}>{children}</LangProvider> |
🤖 Prompt for AI Agents
In frontend/mobile-ui/src/Providers.tsx around lines 20 to 31, the code uses a
non-null assertion role! when passing role to LangProvider which can crash if
role is null/undefined; change it to handle undefined safely by either (1)
passing a safe default like "guest" or the string your localization expects, (2)
making LangProvider accept an optional role and handle undefined, or (3) delay
rendering of LangProvider until auth state is resolved (e.g., render a
loading/fallback while role is undefined). Update the code to remove the "!"
operator and implement one of these safe handling approaches so null/undefined
role values are not asserted away.
2 participants
The Property Tax Frontend is a modern, mobile-first React application designed for efficient property tax management by agents and citizens. Key features include map-based property selection, multi-step property forms, document uploads, application tracking, and integrated notifications. Agents can verify and manage property applications, while citizens can view, update, and submit their property details. The business purpose is to streamline property tax workflows, enhance transparency, and empower users with self-service capabilities, improving accuracy and service delivery for municipal property tax operations.
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.