fix: stop readReports rewriting the whole report list on every read (#994)

[MOHITKOURAV01]

Fixes #994.

The write that happened whether or not anything changed

const migrated = parsed.map((report) => { /* decode entities, rename statuses */ });

try {
  localStorage.setItem(STORAGE_KEY, JSON.stringify(migrated));   // unconditional
} catch { … }

The docstring above decodeStoredEntities says the migration "runs once, at the migration below, and the result is written back". It ran on every call and wrote on every one.

readReports() is called from the useState initialiser (line 143) and on mount, so a plain page load serialised the entire report list — base64 image data URIs, capped at 500 KB each — and wrote it straight back over the top of itself. Synchronously, on the path to first paint, into a store this component already treats as scarce: there is a 5 MB warning threshold and a pruning path fifty lines further down.

It was also a correctness problem across tabs. Two tabs open on the hub means each one rewriting storage from whatever snapshot it read, so a report submitted in tab A can be erased by tab B merely rendering.

The write now happens only when the entity decode or a status rename actually changed something.

The regression test was failing for a different reason than it looked

does not rewrite storage when nothing needs migrating was red on main, and I assumed it was catching the bug above. It was, but it would have stayed red after the fix: makeReport() defaults to status: 'Pending', which is itself a legacy value the migration renames to 'New'. The "nothing needs migrating" fixture needed migrating.

Gave that case a current status, and added one that reads three times over an already-migrated list and asserts total silence — which is the property that actually matters.

Second half: the form's controls had no names

<select value={form.category} …>   {/* accessible name: "" */}
<select value={form.severity} …>   {/* accessible name: "" */}
<select value={form.hashtag} …>    {/* accessible name: "" */}

A screen reader announced three unnamed combo boxes in a row. CommunityHub.test.jsx failed on it —

Found multiple elements with the role "combobox" and name ""

— and had worked around it by reaching past Testing Library to document.querySelectorAll('select')[0] and [1]. Index-based DOM access in a test is usually the markup telling you something.

Every control in the form now carries an aria-label, in all three locales. The form's severity control is named "Incident severity" rather than "Severity", because the filters block already has a properly-labelled select named "Severity" and two identical names is barely better than none — there is a test asserting the names within the form are distinct.

One thing I backed out while doing this: I initially added an aria-label to the location filter input too, then removed it. That input is already inside a wrapping <label> reading "Location (Search)", and an aria-label would have overridden the visible text — a worse outcome than leaving it alone.

Test repairs that fell out of this

Four separate cases of tests asserting against markup or copy that had moved on:

1. The form became collapsible behind a "Report Pollution" toggle, and the round-trip tests were never updated — they were querying a form that had not rendered.
2. Category became a required field, so a submit without one is blocked by constraint validation and onSubmit never runs.
3. The GPS tests looked for the wrong text. They queried /Use GPS for Location/i and /GPS Location attached/i, which are the inline defaults in the JSX. translation.json overrides both with "Use Current Location" and "Location attached" — what a reader actually sees. The tests were looking for strings that never reach the screen.
4. vi.stubGlobal('navigator', { ...globalThis.navigator, geolocation }) copies nothing. navigator's properties live on its prototype, so the spread produces {} and the component was handed an empty navigator. Replaced with a defineProperty stub that restores the original descriptor in a finally.

Checks

• npx vitest run src/components/CommunityHub.test.jsx src/components/CommunityHub.escaping.test.jsx — 22 passed (was 16, with 6 failing)
• npx eslint on all three files — no new problems (one pre-existing exhaustive-deps warning at line 306, deliberately annotated in the source)
• npm run build — passes

Three component suites remain red on main and are untouched by this PR: CityPollutionLeaderboard (uses jest.mock in a Vitest project), RouteForm, and SymptomReportButton. Each has its own cause and its own issue.

[vercel]

@MOHITKOURAV01 is attempting to deploy a commit to the Aditya Mahajan's projects Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

Thank You for Your Contribution! 🎉

Hi @MOHITKOURAV01,

Thank you for opening this Pull Request and contributing to our project. We truly appreciate your efforts.

Please make sure that:

• Your code follows the project's guidelines.
• You have linked the appropriate issue (if applicable).
• Screenshots are added for UI/UX changes.
• Your PR is ready for review.

The maintainer @Aditya8369 will review your PR shortly!

Happy Contributing! 🚀

[MOHITKOURAV01]

Note on the red checks.

CI is red on main itself — the latest run on 02e98f7 failed, with 18 unit tests down across 9 files plus 218 ESLint errors. So this PR opening red is inherited, not caused. To make that checkable rather than asserted:

• Fixed here: neither CommunityHub.test.jsx nor CommunityHub.escaping.test.jsx appears in the CI failure list any more (6 failing → 22 passing).
• Still red, untouched by this PR: RouteForm, SymptomReportButton, CityPollutionLeaderboard, verificationService, noiseTelemetryService, solarUvService, databaseService.
• CommunityHub.jsx keeps one pre-existing react-hooks/exhaustive-deps warning at line 306, which is deliberately annotated in the source (aqiCache is a stable ref).

[Aditya8369]

@MOHITKOURAV01 conflicts

[Aditya8369]

@MOHITKOURAV01 conflicts

[MOHITKOURAV01]

Merged main in to clear the conflict.

main restructured this file in 5dbb1a0 (community report moderation tools) — 236 insertions, and the report form was re-indented by two spaces along the way — so git could not line up either half of this branch against it. Rather than hand-editing conflict markers I took main's CommunityHub.jsx whole and re-applied the two changes onto it:

readReports — the conditional write-back is back, unchanged. main's copy still does

localStorage.setItem(STORAGE_KEY, JSON.stringify(migrated));

unconditionally, on every call, including the one in the useState initialiser at line 144.

The form — all seven controls carry their id and aria-label again, at main's indentation. The moderation controls added in 5dbb1a0 are untouched.

The three translation files merged cleanly on their own; I checked the seven communityHub.label* keys are present and the JSON parses in en, hi and bn.

$ npx vitest run src/components/CommunityHub.test.jsx src/components/CommunityHub.escaping.test.jsx
Tests  22 passed (22)

$ npx vitest run
Tests  11 failed | 1594 passed (1605)

versus 17 failed / 1584 passed on main — the six CommunityHub failures currently red on main are green on this branch. npm run lint reports one warning for this file (aqiCache in a dependency array), which is on main too and predates this branch.