feat: implement live grievance map#466
Open
dharapandya85 wants to merge 1 commit intoRohanExploit:mainfrom
Open
feat: implement live grievance map#466dharapandya85 wants to merge 1 commit intoRohanExploit:mainfrom
dharapandya85 wants to merge 1 commit intoRohanExploit:mainfrom
Conversation
👷 Deploy request for fixmybharat pending review.Visit the deploys page to approve it
|
🙏 Thank you for your contribution, @dharapandya85!PR Details:
Quality Checklist:
Review Process:
Note: The maintainers will monitor code quality and ensure the overall project flow isn't broken. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
👋 Hello @dharapandya85! Thank you for your first Pull Request to VishwaGuru! 🎉
We appreciate your contribution to this open source platform empowering India's youth to engage with democracy.
What happens next?
- A maintainer will review your PR soon
- Please ensure all tests pass and the code follows our style guidelines
- Be ready to address any feedback or requested changes
Before your PR is merged:
- Ensure your code follows the project's coding standards
- All tests should pass
- Update documentation if needed
- Make sure your commits are clear and descriptive
Resources:
Thank you for contributing! 🚀
📝 WalkthroughWalkthroughThis PR implements a geo-tagged grievance tracking map feature. Backend validates coordinates and exposes a lightweight map endpoint returning only essential fields. Frontend renders an interactive Leaflet map with clustered markers color-coded by status, accessible via new routing and home view navigation. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant App as Frontend: App
participant Map as Frontend: GrievanceMap
participant API as Frontend: API Client
participant Backend as Backend: /api/grievances/map
participant DB as Database
User->>App: Navigate to /grievance-map
App->>Map: Render GrievanceMap
Map->>Map: Component Mount
Map->>API: grievancesApi.getMapData()
API->>Backend: GET /api/grievances/map
Backend->>DB: Query grievances with coordinates
DB-->>Backend: Return filtered grievance records
Backend-->>API: Return lightweight map data<br/>(id, category, status, severity,<br/>latitude, longitude, authority)
API-->>Map: JSON response
Map->>Map: Store in state, Parse coordinates
Map->>Map: Create status-based colored markers
Map->>Map: Render Leaflet Map with<br/>MarkerClusterGroup
Map-->>User: Display interactive map with<br/>clustered grievance markers
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues found across 9 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="frontend/src/views/ReportForm.jsx">
<violation number="1" location="frontend/src/views/ReportForm.jsx:289">
P2: Missing fallback when geolocation is unsupported leaves gettingLocation true and no error, which can permanently disable the Get Pin button on unsupported browsers.</violation>
</file>
<file name="backend/grievance_service.py">
<violation number="1" location="backend/grievance_service.py:99">
P2: Latitude range check negates latitude (`-90 <- latitude <=90`), so valid values like 90 are rejected. This should compare latitude directly.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+289
to
+292
| // else { | ||
| // setError("Geolocation is not supported by this browser."); | ||
| // setGettingLocation(false); | ||
| // } |
Contributor
There was a problem hiding this comment.
P2: Missing fallback when geolocation is unsupported leaves gettingLocation true and no error, which can permanently disable the Get Pin button on unsupported browsers.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At frontend/src/views/ReportForm.jsx, line 289:
<comment>Missing fallback when geolocation is unsupported leaves gettingLocation true and no error, which can permanently disable the Get Pin button on unsupported browsers.</comment>
<file context>
@@ -283,10 +285,11 @@ const ReportForm = ({ setView, setLoading, setError, setActionPlan, loading }) =
- setError("Geolocation is not supported by this browser.");
- setGettingLocation(false);
}
+ // else {
+ // setError("Geolocation is not supported by this browser.");
+ // setGettingLocation(false);
</file context>
Suggested change
| // else { | |
| // setError("Geolocation is not supported by this browser."); | |
| // setGettingLocation(false); | |
| // } | |
| else { | |
| setError("Geolocation is not supported by this browser."); | |
| setGettingLocation(false); | |
| } |
| latitude = float(latitude) | ||
| longitude = float(longitude) | ||
|
|
||
| if not (-90 <- latitude <=90): |
Contributor
There was a problem hiding this comment.
P2: Latitude range check negates latitude (-90 <- latitude <=90), so valid values like 90 are rejected. This should compare latitude directly.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At backend/grievance_service.py, line 99:
<comment>Latitude range check negates latitude (`-90 <- latitude <=90`), so valid values like 90 are rejected. This should compare latitude directly.</comment>
<file context>
@@ -90,6 +90,20 @@ def create_grievance(self, grievance_data: Dict[str, Any], db: Session = None) -
+ latitude = float(latitude)
+ longitude = float(longitude)
+
+ if not (-90 <- latitude <=90):
+ raise ValueError("Invalid latitude range")
+ if not(-180 <= longitude <=180):
</file context>
Suggested change
| if not (-90 <- latitude <=90): | |
| if not (-90 <= latitude <= 90): |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/views/ReportForm.jsx (1)
267-293:⚠️ Potential issue | 🔴 CriticalRemoving the
elsebranch leavesgettingLocationstuck astruewhen the Geolocation API is unavailable.When
navigator.geolocationis falsy,setGettingLocation(true)is called but never cleared — the "Get Pin" button stays permanently disabled with the loading spinner. This happens in private/restricted browser contexts and some mobile WebViews.🐛 Proposed fix — restore the else guard
} - // else { - // setError("Geolocation is not supported by this browser."); - // setGettingLocation(false); - // } + else { + setError("Geolocation is not supported by this browser."); + setGettingLocation(false); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/ReportForm.jsx` around lines 267 - 293, The getLocation function sets setGettingLocation(true) but omits the else branch when navigator.geolocation is falsy, leaving the loading state stuck; restore the missing else branch in getLocation to call setGettingLocation(false) and setError("Geolocation is not supported by this browser.") (or a similar user-facing message) so the UI exits the loading state when the Geolocation API is unavailable; update the navigator.geolocation check inside getLocation, and ensure the else executes the same cleanup (setGettingLocation(false) and setError) as the failure callback to prevent the "Get Pin" button from staying disabled.
🧹 Nitpick comments (5)
frontend/src/views/GrievanceMap.jsx (2)
11-13: Add anAbortControllercleanup to prevent state updates on unmounted component.React 19's
StrictModemounts/unmounts/remounts every component in development, causingloadMapDatato be called twice. Without a cleanup, both fetch calls complete andsetGrievancesmay be called after the component is gone.♻️ Proposed fix
useEffect(() => { - loadMapData(); - }, []); + const controller = new AbortController(); + loadMapData(controller.signal); + return () => controller.abort(); + }, []); + + const loadMapData = async (signal) => { try { - const response = await grievancesApi.getMapData(); + const response = await grievancesApi.getMapData(signal); const data = Array.isArray(response) ? response : response.data; setGrievances(data); } catch(err) { + if (err.name === "AbortError") return; console.error("Error loading map data:", err); } };Also update
getMapDatainfrontend/src/api/grievances.jsto forward the signal:- async getMapData() { - const response = await fetch(`${API_BASE}/api/grievances/map`); + async getMapData(signal) { + const response = await fetch(`${API_BASE}/api/grievances/map`, { signal });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/GrievanceMap.jsx` around lines 11 - 13, Add an AbortController to the useEffect in GrievanceMap.jsx to cancel in-flight loadMapData calls on unmount: create a controller, pass controller.signal into loadMapData (and ensure frontend/src/api/grievances.js's getMapData accepts/forwards that signal to fetch), and call controller.abort() in the effect cleanup so setGrievances isn't invoked after unmount; update loadMapData/getMapData signatures to accept an optional signal and early-return if the fetch was aborted.
26-48: MovegetColorandmarkerIconoutside the component; remove the unreachableg._idfallback.Both functions are pure (no state/prop dependencies) and recreated on every render. Extracting them avoids unnecessary allocations. On line 65,
g._idis MongoDB naming — the backend endpoint always returns a SQLid; the fallback is dead code.♻️ Proposed refactor
+const STATUS_COLORS = { + open: "red", + in_progress: "orange", + resolved: "green", + escalated: "purple", +}; + +const getColor = (status) => STATUS_COLORS[status?.toLowerCase()] ?? "blue"; + +const markerIcon = (status) => + L.divIcon({ + className: "custom-marker", + html: `<div style="background:${getColor(status)};width:14px;height:14px;border-radius:50%;border:2px solid white;box-shadow:0 0 4px rgba(0,0,0,0.4);"></div>`, + iconSize: [16, 16], + iconAnchor: [8, 8], + }); + const GrievanceMap = () => { - const getColor = (status) => { ... }; - const markerIcon = (status) => { ... }; ... <Marker - key={g.id || g._id} + key={g.id}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/views/GrievanceMap.jsx` around lines 26 - 48, Extract the pure helper functions getColor and markerIcon out of the React component so they're defined once at module scope (avoids recreating them on every render), and update any imports/usage accordingly; also remove the unreachable MongoDB fallback g._id in the component where IDs are used (ensure only the SQL id field is referenced, e.g., use g.id consistently). Ensure markerIcon still calls getColor(status) and that icon creation (L.divIcon) remains unchanged after moving it.backend/grievance_service.py (1)
103-106: Coordinate rejection is silent — considerlogger.warninginstead ofThe invalid-coordinate path calls
print("Invalid latitude/longitude provided")while the rest of the service uses structured logging (logger). This makes production debugging harder and is inconsistent with the surrounding code.♻️ Proposed fix
- except(ValueError, TypeError): - print("Invalid latitude/longitude provided") - latitude = None - longitude = None + except (ValueError, TypeError): + logger.warning("Invalid lat/lon provided; coordinates cleared.") + latitude = None + longitude = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/grievance_service.py` around lines 103 - 106, Replace the silent print in the invalid-coordinate except handler with a structured logger warning: inside the except(ValueError, TypeError) block that currently prints "Invalid latitude/longitude provided" and sets latitude/longitude to None, call logger.warning(...) instead and include contextual info (e.g., the grievance id or input values if available) so the rejection is recorded in the service logs consistently with the rest of grievance_service.py.backend/utils.py (1)
15-15:import uuidis unused in this file.No function in
utils.pyreferences theuuidmodule. Either remove it or, ifgenerate_reference_idis meant to use UUID-based IDs, replace therandom.choicessuffix withuuid.uuid4().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/utils.py` at line 15, The import uuid is unused; either remove the unused import or update the generate_reference_id function to use a UUID-based suffix instead of random.choices: if you remove it, delete the import uuid and leave generate_reference_id as-is; if you switch to UUIDs, replace the random suffix generation in generate_reference_id with uuid.uuid4() (or a shortened/hex form like uuid.uuid4().hex) and keep the import. Ensure generate_reference_id is the only referenced symbol changed so no other imports become unused.frontend/src/api/grievances.js (1)
7-20: Consolidate fetch + error handling to avoid drift across endpoints.create/getById/getMapData repeat the same fetch/ok/json pattern. A tiny helper reduces duplication and keeps behavior consistent (e.g., when adding headers, timeouts, or standardized errors later).
♻️ Example refactor
const API_BASE = import.meta.env.VITE_API_URL || ''; +const requestJson = async (url, options) => { + const response = await fetch(url, options); + if (!response.ok) { + throw new Error(`HTTP error! status: ${response.status}`); + } + return response.json(); +}; + export const grievancesApi = { // Create grievance async create(data) { - const response = await fetch(`${API_BASE}/api/grievances`,{ - method: 'POST', - headers :{ - 'Content-Type': 'application/json', - }, - body: JSON.stringify(data), - }); - if(!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - return response.json(); + return requestJson(`${API_BASE}/api/grievances`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(data), + }); }, //Get list of grievances async getAll(params = {}) { const queryParams = new URLSearchParams(); @@ //Get single grievance async getById(grievanceId) { - const response = await fetch(`${API_BASE}/api/grievances/${grievanceId}`); - if(!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - return response.json(); + return requestJson(`${API_BASE}/api/grievances/${grievanceId}`); }, //Get map data async getMapData() { - const response = await fetch(`${API_BASE}/api/grievances/map`); - if(!response.ok) { - throw new Error(`HTTP error! status: ${response.status}`); - } - return response.json(); + return requestJson(`${API_BASE}/api/grievances/map`); },Also applies to: 36-41, 44-49
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/api/grievances.js` around lines 7 - 20, Extract the repeated fetch+error/json pattern into a single helper (e.g., apiFetch or request) and refactor create, getById, and getMapData to call that helper; the helper should accept (url, options) perform fetch, check response.ok, throw a standardized Error with status and body when not ok, and return await response.json(), and update the existing methods (create, getById, getMapData) to pass their URL and options into this helper so headers/timeouts/auth can be changed in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/grievance_service.py`:
- Line 99: The latitude range check uses the incorrect operator sequence "-90 <-
latitude <=90" which is parsed as "-90 < -latitude" and wrongly rejects latitude
== 90; update the condition (wherever latitude is validated in
grievance_service.py, e.g., the block using the variable latitude) to use a
proper inclusive range check like "-90 <= latitude <= 90" (negated as needed,
e.g., if not (-90 <= latitude <= 90)) so latitude 90 is accepted and the
ValueError/coordinate stripping no longer occurs.
In `@backend/routers/grievances.py`:
- Around line 88-125: get_grievances_for_map currently calls query.all() with no
error handling or limit; wrap the DB work (the query, optional status filter,
.limit(...), and query.all() usage) in a try/except that catches SQL/DB errors,
logs the exception (use the module logger or raise a fastapi.HTTPException with
status_code=500 and a generic message) and returns an appropriate 500 response;
add a new optional parameter like limit: int = 100 (and enforce a sane max,
e.g., min(limit, 1000)) and apply it with query = query.limit(limit) before
.all(); keep the existing field mapping from results (g.id, g.category,
g.severity, g.status, g.latitude, g.longitude, g.assigned_authority) but ensure
you still access enum values safely (g.severity.value / g.status.value) inside
the try block so exceptions are handled.
- Around line 88-92: The get_grievances_for_map endpoint is missing auth; update
its signature to require an authenticated user by adding the dependency
parameter current_user: User = Depends(get_current_active_user) to the function
get_grievances_for_map, and ensure you import User and get_current_active_user
from backend.dependencies (or wherever they’re defined) so the endpoint uses the
same auth pattern as other routes.
In `@frontend/src/api/grievances.js`:
- Around line 24-27: The current truthy checks skip numeric 0 for params.limit
and params.offset; change the conditions to explicit null/undefined checks so 0
is accepted — e.g., replace "if(params.limit)" and "if(params.offset)" with
checks like "if (params.limit != null)" and "if (params.offset != null)" around
the queryParams.append calls in the grievances API (the
params.limit/params.offset branches that call queryParams.append).
In `@frontend/src/App.jsx`:
- Line 11: GrievanceMap is being eagerly imported which pulls heavy map libs
into the initial bundle; change the top-level import of GrievanceMap to a lazy
load by replacing the static import with React.lazy(() =>
import("./views/GrievanceMap")) and keep the existing <Suspense
fallback={<LoadingSpinner size="lg" />}> wrapper that already wraps the routes
so the lazy route is suspended correctly; update any references to the component
name GrievanceMap (used in your routes) to use the lazy-loaded symbol.
In `@frontend/src/main.jsx`:
- Line 7: Remove the duplicate global stylesheet import by deleting the line
importing "leaflet/dist/leaflet.css" from GrievanceMap.jsx so the single import
in main.jsx remains the authoritative global import; locate the import statement
in GrievanceMap.jsx (the `import "leaflet/dist/leaflet.css";` line) and remove
it, leaving any component-specific styles intact.
In `@frontend/src/views/GrievanceMap.jsx`:
- Line 19: Remove the unconditional console.log that prints sensitive grievance
data (the line console.log("Map data:", data) in the GrievanceMap.jsx component)
— either delete it completely or wrap it in a development-only guard (e.g.,
NODE_ENV check) and/or log a non-sensitive, sanitized summary instead; locate
the log in the GrievanceMap component where the variable data is available and
replace the console.log with a conditional/dev-only or sanitized logging
approach.
- Around line 49-54: The root div in the GrievanceMap component uses Tailwind's
h-screen which forces the map to fill the full viewport and push content below
the Navbar; update the container (the div that wraps MapContainer in
GrievanceMap.jsx) to not use h-screen but instead use a flexible height (e.g.,
replace h-screen with flex-1 or apply a calculated height like calc(100vh -
<navbarHeight>)) and ensure the parent layout uses a column flex so MapContainer
can grow to fill remaining space; target the GrievanceMap component's root div
and the MapContainer wrapper when making this change.
- Around line 15-25: The map currently swallows errors and provides no loading
feedback; update GrievanceMap.jsx by adding local state via useState for loading
and error (e.g., const [loading, setLoading] = useState(false); const [error,
setError] = useState(null)), then modify loadMapData to setLoading(true) and
setError(null) at start, assign setGrievances(data) on success and
setLoading(false), and in the catch setError(err) and setLoading(false); finally
render conditional UI inside the <div className="h-screen w-full"> wrapper to
show a spinner when loading and an error banner/message when error is non-null
before rendering the map.
---
Outside diff comments:
In `@frontend/src/views/ReportForm.jsx`:
- Around line 267-293: The getLocation function sets setGettingLocation(true)
but omits the else branch when navigator.geolocation is falsy, leaving the
loading state stuck; restore the missing else branch in getLocation to call
setGettingLocation(false) and setError("Geolocation is not supported by this
browser.") (or a similar user-facing message) so the UI exits the loading state
when the Geolocation API is unavailable; update the navigator.geolocation check
inside getLocation, and ensure the else executes the same cleanup
(setGettingLocation(false) and setError) as the failure callback to prevent the
"Get Pin" button from staying disabled.
---
Nitpick comments:
In `@backend/grievance_service.py`:
- Around line 103-106: Replace the silent print in the invalid-coordinate except
handler with a structured logger warning: inside the except(ValueError,
TypeError) block that currently prints "Invalid latitude/longitude provided" and
sets latitude/longitude to None, call logger.warning(...) instead and include
contextual info (e.g., the grievance id or input values if available) so the
rejection is recorded in the service logs consistently with the rest of
grievance_service.py.
In `@backend/utils.py`:
- Line 15: The import uuid is unused; either remove the unused import or update
the generate_reference_id function to use a UUID-based suffix instead of
random.choices: if you remove it, delete the import uuid and leave
generate_reference_id as-is; if you switch to UUIDs, replace the random suffix
generation in generate_reference_id with uuid.uuid4() (or a shortened/hex form
like uuid.uuid4().hex) and keep the import. Ensure generate_reference_id is the
only referenced symbol changed so no other imports become unused.
In `@frontend/src/api/grievances.js`:
- Around line 7-20: Extract the repeated fetch+error/json pattern into a single
helper (e.g., apiFetch or request) and refactor create, getById, and getMapData
to call that helper; the helper should accept (url, options) perform fetch,
check response.ok, throw a standardized Error with status and body when not ok,
and return await response.json(), and update the existing methods (create,
getById, getMapData) to pass their URL and options into this helper so
headers/timeouts/auth can be changed in one place.
In `@frontend/src/views/GrievanceMap.jsx`:
- Around line 11-13: Add an AbortController to the useEffect in GrievanceMap.jsx
to cancel in-flight loadMapData calls on unmount: create a controller, pass
controller.signal into loadMapData (and ensure frontend/src/api/grievances.js's
getMapData accepts/forwards that signal to fetch), and call controller.abort()
in the effect cleanup so setGrievances isn't invoked after unmount; update
loadMapData/getMapData signatures to accept an optional signal and early-return
if the fetch was aborted.
- Around line 26-48: Extract the pure helper functions getColor and markerIcon
out of the React component so they're defined once at module scope (avoids
recreating them on every render), and update any imports/usage accordingly; also
remove the unreachable MongoDB fallback g._id in the component where IDs are
used (ensure only the SQL id field is referenced, e.g., use g.id consistently).
Ensure markerIcon still calls getColor(status) and that icon creation
(L.divIcon) remains unchanged after moving it.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
backend/grievance_service.pybackend/routers/grievances.pybackend/utils.pyfrontend/src/App.jsxfrontend/src/api/grievances.jsfrontend/src/main.jsxfrontend/src/views/GrievanceMap.jsxfrontend/src/views/Home.jsxfrontend/src/views/ReportForm.jsx
| latitude = float(latitude) | ||
| longitude = float(longitude) | ||
|
|
||
| if not (-90 <- latitude <=90): |
There was a problem hiding this comment.
Typo <- silently rejects latitude 90 (North Pole).
-90 <- latitude is parsed by Python as -90 < -latitude, turning the range check into -90 ≤ latitude < 90 — latitude exactly 90 raises ValueError and strips coordinates.
🐛 Proposed fix
- if not (-90 <- latitude <=90):
+ if not (-90 <= latitude <= 90):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if not (-90 <- latitude <=90): | |
| if not (-90 <= latitude <= 90): |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/grievance_service.py` at line 99, The latitude range check uses the
incorrect operator sequence "-90 <- latitude <=90" which is parsed as "-90 <
-latitude" and wrongly rejects latitude == 90; update the condition (wherever
latitude is validated in grievance_service.py, e.g., the block using the
variable latitude) to use a proper inclusive range check like "-90 <= latitude
<= 90" (negated as needed, e.g., if not (-90 <= latitude <= 90)) so latitude 90
is accepted and the ValueError/coordinate stripping no longer occurs.
Comment on lines
+88
to
+125
| @router.get("/api/grievances/map") | ||
| def get_grievances_for_map( | ||
| status: Optional[str] = None, | ||
| db: Session = Depends(get_db) | ||
| ): | ||
| """ | ||
| Optimized endpoint for map rendering. | ||
| Returns only geo + minimal fields. | ||
| """ | ||
| query = db.query( | ||
| Grievance.id, | ||
| Grievance.category, | ||
| Grievance.severity, | ||
| Grievance.status, | ||
| Grievance.latitude, | ||
| Grievance.longitude, | ||
| Grievance.assigned_authority | ||
| ).filter( | ||
| Grievance.latitude.isnot(None), | ||
| Grievance.longitude.isnot(None) | ||
| ) | ||
| if status: | ||
| query = query.filter(Grievance.status == status) | ||
| results = query.all() | ||
|
|
||
| return [ | ||
| { | ||
| "id": g.id, | ||
| "category": g.category, | ||
| "severity": g.severity.value, | ||
| "status": g.status.value, | ||
| "latitude": g.latitude, | ||
| "longitude": g.longitude, | ||
| "authority": g.assigned_authority, | ||
|
|
||
| } | ||
| for g in results | ||
| ] |
There was a problem hiding this comment.
New map endpoint has no error handling and no result limit.
Unlike every other endpoint in this file, get_grievances_for_map has no try/except: a transient DB error returns an unlogged 500. More critically, query.all() fetches every geo-tagged grievance with no pagination — this will become a memory and latency issue as the dataset grows.
🛡️ Proposed fix — add try/except and a configurable limit
`@router.get`("/api/grievances/map")
def get_grievances_for_map(
status: Optional[str] = None,
+ limit: int = Query(500, ge=1, le=2000, description="Max markers to return"),
db: Session = Depends(get_db)
):
"""
Optimized endpoint for map rendering.
Returns only geo + minimal fields.
"""
+ try:
query = db.query(
Grievance.id,
Grievance.category,
Grievance.severity,
Grievance.status,
Grievance.latitude,
Grievance.longitude,
Grievance.assigned_authority
).filter(
Grievance.latitude.isnot(None),
Grievance.longitude.isnot(None)
)
if status:
query = query.filter(Grievance.status == status)
- results = query.all()
+ results = query.limit(limit).all()
return [
{
"id": g.id,
"category": g.category,
"severity": g.severity.value,
"status": g.status.value,
"latitude": g.latitude,
"longitude": g.longitude,
"authority": g.assigned_authority,
}
for g in results
]
+ except Exception as e:
+ logger.error(f"Error fetching map data: {e}", exc_info=True)
+ raise HTTPException(status_code=500, detail="Failed to retrieve map data")🧰 Tools
🪛 Ruff (0.15.2)
[warning] 91-91: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routers/grievances.py` around lines 88 - 125, get_grievances_for_map
currently calls query.all() with no error handling or limit; wrap the DB work
(the query, optional status filter, .limit(...), and query.all() usage) in a
try/except that catches SQL/DB errors, logs the exception (use the module logger
or raise a fastapi.HTTPException with status_code=500 and a generic message) and
returns an appropriate 500 response; add a new optional parameter like limit:
int = 100 (and enforce a sane max, e.g., min(limit, 1000)) and apply it with
query = query.limit(limit) before .all(); keep the existing field mapping from
results (g.id, g.category, g.severity, g.status, g.latitude, g.longitude,
g.assigned_authority) but ensure you still access enum values safely
(g.severity.value / g.status.value) inside the try block so exceptions are
handled.
Comment on lines
+88
to
+92
| @router.get("/api/grievances/map") | ||
| def get_grievances_for_map( | ||
| status: Optional[str] = None, | ||
| db: Session = Depends(get_db) | ||
| ): |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check app-level middleware, auth schemes, and dependencies
rg -n --type py -C3 "add_middleware|HTTPBearer|get_current_user|Depends.*auth|oauth2_scheme"Repository: RohanExploit/VishwaGuru
Length of output: 2683
🏁 Script executed:
#!/bin/bash
# Check how the grievances router is registered in the main app
rg -n --type py -C5 "grievances.*router|include_router.*grievance"Repository: RohanExploit/VishwaGuru
Length of output: 791
🏁 Script executed:
#!/bin/bash
# Check frontend getMapData for auth headers
rg -n --type js -C5 "getMapData|Authorization.*token|Bearer"Repository: RohanExploit/VishwaGuru
Length of output: 2906
🏁 Script executed:
#!/bin/bash
# Also check the grievances router file to see if other endpoints have explicit auth
fd -t f "grievances.py" | xargs cat -n | head -200Repository: RohanExploit/VishwaGuru
Length of output: 9107
🏁 Script executed:
#!/bin/bash
# Find the main app file to understand the setup
fd -t f -e py "main.py|app.py" | head -5Repository: RohanExploit/VishwaGuru
Length of output: 112
Add authentication to /api/grievances/map endpoint.
The endpoint currently lacks explicit auth dependency and is fully public. No app-level middleware guards the grievances router—only CORS and GZip are configured in main.py. This allows unauthenticated access to latitude, longitude, and assigned_authority for every grievance in the database. The frontend's getMapData() call in frontend/src/api/grievances.js confirms this pattern by using a bare fetch() without an Authorization header, unlike other API calls that use getHeaders() with Bearer tokens.
Add current_user: User = Depends(get_current_active_user) to the function signature to match the auth pattern already defined in backend/dependencies.py.
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 91-91: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/routers/grievances.py` around lines 88 - 92, The
get_grievances_for_map endpoint is missing auth; update its signature to require
an authenticated user by adding the dependency parameter current_user: User =
Depends(get_current_active_user) to the function get_grievances_for_map, and
ensure you import User and get_current_active_user from backend.dependencies (or
wherever they’re defined) so the endpoint uses the same auth pattern as other
routes.
Comment on lines
+24
to
+27
| if(params.status) queryParams.append('status', params.status); | ||
| if(params.category) queryParams.append('category', params.category); | ||
| if(params.limit) queryParams.append('limit', params.limit); | ||
| if(params.offset) queryParams.append('offset', params.offset); |
There was a problem hiding this comment.
Allow 0 values for limit/offset.
Truthy checks skip 0, which is often a valid value (especially for offset). Prefer explicit != null checks.
✅ Suggested fix
- if(params.limit) queryParams.append('limit', params.limit);
- if(params.offset) queryParams.append('offset', params.offset);
+ if (params.limit != null) queryParams.append('limit', params.limit);
+ if (params.offset != null) queryParams.append('offset', params.offset);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if(params.status) queryParams.append('status', params.status); | |
| if(params.category) queryParams.append('category', params.category); | |
| if(params.limit) queryParams.append('limit', params.limit); | |
| if(params.offset) queryParams.append('offset', params.offset); | |
| if(params.status) queryParams.append('status', params.status); | |
| if(params.category) queryParams.append('category', params.category); | |
| if (params.limit != null) queryParams.append('limit', params.limit); | |
| if (params.offset != null) queryParams.append('offset', params.offset); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/api/grievances.js` around lines 24 - 27, The current truthy
checks skip numeric 0 for params.limit and params.offset; change the conditions
to explicit null/undefined checks so 0 is accepted — e.g., replace
"if(params.limit)" and "if(params.offset)" with checks like "if (params.limit !=
null)" and "if (params.offset != null)" around the queryParams.append calls in
the grievances API (the params.limit/params.offset branches that call
queryParams.append).
| import FloatingButtonsManager from './components/FloatingButtonsManager'; | ||
| import LoadingSpinner from './components/LoadingSpinner'; | ||
| import { DarkModeProvider, useDarkMode } from './contexts/DarkModeContext'; | ||
| import GrievanceMap from "./views/GrievanceMap"; |
There was a problem hiding this comment.
GrievanceMap is eagerly imported, breaking code-splitting for the entire app.
Every other view component (lines 14–44) is loaded with React.lazy(). This eager import forces Leaflet, react-leaflet, and react-leaflet-cluster (~150 KB+ minified) into the initial bundle, increasing load time for all users regardless of whether they visit the map.
♻️ Proposed fix — convert to lazy import
-import GrievanceMap from "./views/GrievanceMap";
+const GrievanceMap = React.lazy(() => import('./views/GrievanceMap'));The existing <Suspense fallback={<LoadingSpinner size="lg" />}> wrapper at line 208 already covers all lazy routes, so no other changes are needed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import GrievanceMap from "./views/GrievanceMap"; | |
| const GrievanceMap = React.lazy(() => import('./views/GrievanceMap')); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/App.jsx` at line 11, GrievanceMap is being eagerly imported
which pulls heavy map libs into the initial bundle; change the top-level import
of GrievanceMap to a lazy load by replacing the static import with React.lazy(()
=> import("./views/GrievanceMap")) and keep the existing <Suspense
fallback={<LoadingSpinner size="lg" />}> wrapper that already wraps the routes
so the lazy route is suspended correctly; update any references to the component
name GrievanceMap (used in your routes) to use the lazy-loaded symbol.
| import App from './App.jsx' | ||
| import './i18n' // Initialize i18n | ||
| import './offlineQueue' // Initialize offline queue listeners | ||
| import "leaflet/dist/leaflet.css"; |
There was a problem hiding this comment.
Duplicate Leaflet CSS import — GrievanceMap.jsx also imports leaflet/dist/leaflet.css.
Global stylesheets are idempotent but importing from two places is redundant. Since main.jsx is the global entry point, the import in GrievanceMap.jsx (line 6) can be removed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/main.jsx` at line 7, Remove the duplicate global stylesheet
import by deleting the line importing "leaflet/dist/leaflet.css" from
GrievanceMap.jsx so the single import in main.jsx remains the authoritative
global import; locate the import statement in GrievanceMap.jsx (the `import
"leaflet/dist/leaflet.css";` line) and remove it, leaving any component-specific
styles intact.
Comment on lines
+15
to
+25
| const loadMapData = async () => { | ||
| try { | ||
| const response = await grievancesApi.getMapData(); | ||
| const data = Array.isArray(response)?response: response.data; | ||
| console.log("Map data:", data); | ||
|
|
||
| setGrievances(data); | ||
| } catch(err) { | ||
| console.error("Error loading map data:", err); | ||
| } | ||
| }; |
There was a problem hiding this comment.
No loading or error feedback — the map silently stays empty on slow networks or API failures.
loadMapData errors are swallowed into console.error. Add a loading / error state and render appropriate feedback in the map container.
♻️ Suggested pattern
const GrievanceMap = () => {
const [grievances, setGrievances] = useState([]);
+ const [loading, setLoading] = useState(true);
+ const [error, setError] = useState(null);
const loadMapData = async () => {
try {
const response = await grievancesApi.getMapData();
const data = Array.isArray(response) ? response : response.data;
setGrievances(data);
} catch(err) {
console.error("Error loading map data:", err);
+ setError("Failed to load grievance data.");
+ } finally {
+ setLoading(false);
}
};Then render a spinner or error banner inside the <div className="h-screen w-full"> wrapper before displaying the map.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/GrievanceMap.jsx` around lines 15 - 25, The map currently
swallows errors and provides no loading feedback; update GrievanceMap.jsx by
adding local state via useState for loading and error (e.g., const [loading,
setLoading] = useState(false); const [error, setError] = useState(null)), then
modify loadMapData to setLoading(true) and setError(null) at start, assign
setGrievances(data) on success and setLoading(false), and in the catch
setError(err) and setLoading(false); finally render conditional UI inside the
<div className="h-screen w-full"> wrapper to show a spinner when loading and an
error banner/message when error is non-null before rendering the map.
| try { | ||
| const response = await grievancesApi.getMapData(); | ||
| const data = Array.isArray(response)?response: response.data; | ||
| console.log("Map data:", data); |
There was a problem hiding this comment.
Remove console.log — it dumps sensitive grievance data (lat/lon, authority, status) to the browser console in production.
🐛 Proposed fix
- console.log("Map data:", data);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| console.log("Map data:", data); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/GrievanceMap.jsx` at line 19, Remove the unconditional
console.log that prints sensitive grievance data (the line console.log("Map
data:", data) in the GrievanceMap.jsx component) — either delete it completely
or wrap it in a development-only guard (e.g., NODE_ENV check) and/or log a
non-sensitive, sanitized summary instead; locate the log in the GrievanceMap
component where the variable data is available and replace the console.log with
a conditional/dev-only or sanitized logging approach.
Comment on lines
+49
to
+54
| return ( | ||
| <div className="h-screen w-full"> | ||
| <MapContainer | ||
| center={[20.5937, 78.9629]} | ||
| zoom={5} | ||
| style={{height: "100%", width:"100%"}} |
There was a problem hiding this comment.
h-screen causes the map to overflow below the Navbar.
AppContent renders a <Navbar /> above <main> (see App.jsx lines 205–206). With h-screen on the root div, the map's total height equals the full viewport, pushing content below the fold. Use a calculated height or CSS flex-grow to fill only the remaining viewport.
♻️ Suggested fix
- <div className="h-screen w-full">
+ <div className="h-[calc(100vh-64px)] w-full"> {/* adjust 64px to actual Navbar height */}or using Tailwind flex-1:
- <div className="h-screen w-full">
+ <div className="flex-1 w-full min-h-0">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| return ( | |
| <div className="h-screen w-full"> | |
| <MapContainer | |
| center={[20.5937, 78.9629]} | |
| zoom={5} | |
| style={{height: "100%", width:"100%"}} | |
| return ( | |
| <div className="h-[calc(100vh-64px)] w-full"> {/* adjust 64px to actual Navbar height */} | |
| <MapContainer | |
| center={[20.5937, 78.9629]} | |
| zoom={5} | |
| style={{height: "100%", width:"100%"}} |
Suggested change
| return ( | |
| <div className="h-screen w-full"> | |
| <MapContainer | |
| center={[20.5937, 78.9629]} | |
| zoom={5} | |
| style={{height: "100%", width:"100%"}} | |
| return ( | |
| <div className="flex-1 w-full min-h-0"> | |
| <MapContainer | |
| center={[20.5937, 78.9629]} | |
| zoom={5} | |
| style={{height: "100%", width:"100%"}} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/views/GrievanceMap.jsx` around lines 49 - 54, The root div in
the GrievanceMap component uses Tailwind's h-screen which forces the map to fill
the full viewport and push content below the Navbar; update the container (the
div that wraps MapContainer in GrievanceMap.jsx) to not use h-screen but instead
use a flexible height (e.g., replace h-screen with flex-1 or apply a calculated
height like calc(100vh - <navbarHeight>)) and ensure the parent layout uses a
column flex so MapContainer can grow to fill remaining space; target the
GrievanceMap component's root div and the MapContainer wrapper when making this
change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Live view of grievances in a map, with feature of status and authority in their locality.
When grievances created, live view occurs. (To be implemented).
Type of Change
Related Issue
Closes #283
Testing Done
Screenshots (if applicable)
Checklist
Co-Authors
Summary by cubic
Implemented a live grievance map with clustered, color‑coded markers showing status, severity, and responsible authority. Added an optimized API endpoint and a protected route for fast, location‑based viewing.
New Features
Bug Fixes
Written for commit 049294b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes