feat(citizen-pwa): 3-step report wizard wiring + GPS fixes + auto-fill#59
Merged
feat(citizen-pwa): 3-step report wizard wiring + GPS fixes + auto-fill#59
Conversation
Rewrites SubmitReportForm/index.tsx from a single-page form into a 3-step wizard (Step1Evidence → Step2WhoWhere → Step3Review). On step 3 submit, creates a draft via createDraft(), then hands off to SubmissionPanel which drives RevealSheet state via useSubmissionMachine. - Step state 1/2/3 with data accumulation across steps - createDraft called with hashed MSISDN, severity='medium', patientCount→description - RevealSheet shown for server_confirmed/queued/failed_retryable states - queued/failed onClose navigates to /; success onSuccess navigates to /reports/:ref - failed_terminal shows SmsFallbackButton + OfflineBanner Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…cy form - /report now renders the 3-step wizard directly (no CitizenShell wrapper) - /report/new route removed — wizard is canonical at /report - SubmitReportForm.tsx (old single-page form) deleted - App.routes.test.tsx: update mock path and drop nav-button assertion (wizard hides bottom nav, so aria-current check no longer applies) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two post-submit UX bugs fixed: 1. SubmissionPanel now auto-fires machine.submit() on mount via useEffect. The wizard already captured user consent in Step3 — showing a second raw 'Submit report' button was a double-confirm that violated the wizard flow. 2. failed_terminal now renders RevealSheet (failed_retryable variant) instead of raw OfflineBanner + SmsFallbackButton. This matches the styled sheet users see for failed_retryable, which already includes hotline/SMS fallback cards. Removes: SmsFallbackButton import (no longer needed), reporterMsisdn prop from SubmissionPanel (only consumer was SmsFallbackButton). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Guard navigator.geolocation for browsers without Geolocation API - Map GeolocationPositionError codes to specific user messages (PERMISSION_DENIED, TIMEOUT vs generic fallback) - Pre-fill reporter name and phone from bantayog.reporter.* localStorage keys on mount; show "Pre-filled from your last report" hint - Persist name/phone to localStorage on Continue so next report skips re-entry - Fix stale eslint-disable-next-line placement for exhaustive-deps in SubmissionPanel Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…m nav Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reviewer's GuideReplaces the legacy one-page report form with a 3-step wizard flow, updates submission handling to auto-start and always surface terminal states via RevealSheet, improves GPS error handling and reporter info persistence, and adjusts routing, navigation shell, tests, and docs to match the new flow. Sequence diagram for 3-step wizard submission flowsequenceDiagram
actor User
participant Step1Evidence
participant WizardContainer
participant Step2WhoWhere
participant Step3Review
participant DraftService as createDraft
participant SubmissionPanel
participant SubmissionMachine as useSubmissionMachine
participant RevealSheet
User->>Step1Evidence: fill reportType, photoFile
User->>Step1Evidence: click Continue
Step1Evidence->>WizardContainer: onNext(step1Data)
WizardContainer->>WizardContainer: save formData.step1
WizardContainer->>WizardContainer: setStep(2)
User->>Step2WhoWhere: fill location, reporterName, reporterMsisdn, patientCount
User->>Step2WhoWhere: click Continue
Step2WhoWhere->>WizardContainer: onNext(step2Data)
WizardContainer->>WizardContainer: save formData.step2
WizardContainer->>WizardContainer: setStep(3)
WizardContainer->>Step3Review: render with reportData
User->>Step3Review: click Submit
Step3Review->>WizardContainer: onSubmit()
WizardContainer->>WizardContainer: handleStep3Submit()
WizardContainer->>DraftService: createDraft(reportType, location, reporterName, reporterMsisdnHash, patientCount, barangayId, nearestLandmark, photo)
DraftService-->>WizardContainer: draft
WizardContainer->>WizardContainer: setDraft(draft)
WizardContainer->>SubmissionPanel: render with draft
SubmissionPanel->>SubmissionMachine: useSubmissionMachine(draft)
SubmissionPanel->>SubmissionMachine: submit() on mount
alt server_confirmed
SubmissionMachine-->>SubmissionPanel: state server_confirmed
SubmissionPanel->>RevealSheet: state success, referenceCode draft.id
else queued
SubmissionMachine-->>SubmissionPanel: state queued
SubmissionPanel->>RevealSheet: state queued, onClose nav('/')
else failed_retryable
SubmissionMachine-->>SubmissionPanel: state failed_retryable
SubmissionPanel->>RevealSheet: state failed_retryable, onClose nav('/')
else failed_terminal
SubmissionMachine-->>SubmissionPanel: state failed_terminal
SubmissionPanel->>RevealSheet: state failed_retryable, onClose nav('/')
end
Sequence diagram for GPS capture and reporter auto-fill in Step2WhoWheresequenceDiagram
actor User
participant Step2WhoWhere
participant Geolocation as navigator.geolocation
participant LocalStore as localStorage
Note over Step2WhoWhere: On mount
Step2WhoWhere->>LocalStore: getItem(bantayog.reporter.name)
LocalStore-->>Step2WhoWhere: savedName or null
Step2WhoWhere->>LocalStore: getItem(bantayog.reporter.msisdn)
LocalStore-->>Step2WhoWhere: savedMsisdn or null
alt savedName or savedMsisdn exists
Step2WhoWhere->>Step2WhoWhere: setReporterName, setReporterMsisdn
Step2WhoWhere->>Step2WhoWhere: setHasMemory(true)
end
Note over Step2WhoWhere: Auto-attempt GPS on mount
Step2WhoWhere->>Step2WhoWhere: attemptGps()
alt geolocation not supported
Step2WhoWhere->>Step2WhoWhere: setLocationError("GPS not supported on this device.")
Step2WhoWhere->>Step2WhoWhere: setLocationMethod('manual')
else geolocation supported
Step2WhoWhere->>Geolocation: getCurrentPosition(success, error, options)
alt success
Geolocation-->>Step2WhoWhere: position
Step2WhoWhere->>Step2WhoWhere: setLocation(position.coords)
Step2WhoWhere->>Step2WhoWhere: setLocationMethod('gps')
else error
Geolocation-->>Step2WhoWhere: error(code)
alt code == 1
Step2WhoWhere->>Step2WhoWhere: setLocationError("Location access denied. Choose municipality manually.")
else code == 3
Step2WhoWhere->>Step2WhoWhere: setLocationError("Location timed out. Choose municipality manually.")
else other error
Step2WhoWhere->>Step2WhoWhere: setLocationError("Could not get location. Choose municipality manually.")
end
Step2WhoWhere->>Step2WhoWhere: setLocationMethod('manual')
end
end
User->>Step2WhoWhere: fill or edit reporterName, reporterMsisdn
User->>Step2WhoWhere: click Continue
Step2WhoWhere->>Step2WhoWhere: validate fields
Step2WhoWhere->>LocalStore: setItem(bantayog.reporter.name, reporterName)
Step2WhoWhere->>LocalStore: setItem(bantayog.reporter.msisdn, reporterMsisdn)
Step2WhoWhere-->>User: proceed to Step3 via onNext(step2Data)
Class diagram for new wizard container and related data typesclassDiagram
class Step1Data {
string reportType
File photoFile
}
class Step2Data {
Location location
string reporterName
string reporterMsisdn
number patientCount
string locationMethod
string municipalityId
string municipalityLabel
string barangayId
string nearestLandmark
}
class Location {
number lat
number lng
}
class FormData {
Step1Data step1
Step2Data step2
}
class WizardContainer {
- number step
- FormData formData
- Draft draft
- boolean isCreatingDraft
- string draftError
+ handleStep1Next(data Step1Data) void
+ handleStep2Next(data Step2Data) void
+ handleStep1Back() void
+ handleStep2Back() void
+ handleStep3Back() void
+ handleStep3Submit() Promise~void~
}
class SubmitReportForm {
+ SubmitReportForm() WizardContainer
}
class SubmissionPanel {
- Draft draft
- number now
+ SubmissionPanel(draft Draft, onSuccess function) void
}
class Step1Evidence {
+ onNext(data Step1Data) void
+ onBack() void
+ isSubmitting boolean
}
class Step2WhoWhere {
+ onNext(data Step2Data) void
+ onBack() void
+ isSubmitting boolean
}
class Step3Review {
+ onBack() void
+ onSubmit() void
+ isSubmitting boolean
}
class useSubmissionMachine {
+ useSubmissionMachine(draft Draft, now number, onSuccess function, onError function) SubmissionMachine
}
class SubmissionMachine {
string state
+ submit() Promise~void~
}
class RevealSheet {
+ RevealSheet(state string, referenceCode string, onClose function) void
}
SubmitReportForm --> WizardContainer : returns
WizardContainer --> Step1Evidence : renders
WizardContainer --> Step2WhoWhere : renders
WizardContainer --> Step3Review : renders
WizardContainer --> FormData : manages
FormData --> Step1Data
FormData --> Step2Data
Step2Data --> Location
WizardContainer --> SubmissionPanel : renders when draft
SubmissionPanel --> useSubmissionMachine : uses
useSubmissionMachine --> SubmissionMachine : creates
SubmissionPanel --> SubmissionMachine : controls
SubmissionPanel --> RevealSheet : renders for all terminal states
Flow diagram for updated /report routing and shell behaviorflowchart LR
AppRouter["App router"]
CitizenShell["CitizenShell (tabs, bottom nav)"]
MapTab["MapTab route '/'"]
FeedTab["Feed route '/feed'"]
AlertsTab["Alerts route '/alerts'"]
ProfileTab["Profile route '/profile'"]
ReportRoute["Report route '/report'"]
Wizard["SubmitReportForm wizard (WizardContainer)"]
AppRouter --> CitizenShell
CitizenShell --> MapTab
CitizenShell --> FeedTab
CitizenShell --> AlertsTab
CitizenShell --> ProfileTab
AppRouter --> ReportRoute
ReportRoute --> Wizard
subgraph Bottom_nav_visible
CitizenShell
end
subgraph Bottom_nav_hidden
ReportRoute
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 5 minutes and 20 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughRefactors the single-page SubmitReportForm into a 3-step wizard with localStorage reporter persistence, replaces emoji nav icons with lucide-react icons, consolidates Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Wizard as WizardContainer
participant Step1 as Step1Evidence
participant Step2 as Step2WhoWhere
participant Step3 as Step3Review
participant Submission as SubmissionPanel
participant Reveal as RevealSheet
User->>Wizard: open /report
Wizard->>Step1: render evidence UI
User->>Step1: provide evidence, next
Step1->>Step2: pass step1 data
Step2->>Step2: load localStorage (name, msisdn)
User->>Step2: enter reporter info, handle GPS
Step2->>Step3: pass step2 data
User->>Step3: confirm submission
Step3->>Submission: finalize draft
Submission->>Submission: useEffect auto-submit()
Submission->>Reveal: update outcome (confirmed/queued/failed)
Reveal->>User: show result and redirect options
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
SubmissionPanel, thefailed_terminalstate rendersRevealSheetwithstate="failed_retryable", which is likely a copy‑paste error and should probably use a distinctfailed_terminal/failedstate to avoid misleading messaging. - When creating the draft you pass
barangay: formData.step2.municipalityLabel ?? '', which looks semantically mismatched; consider adding a dedicated municipality field to the payload or wiring the correct barangay label/id intobarangayto avoid confusing downstream consumers. - The new
localStorageusage inStep2WhoWhereruns unguarded in auseEffect; to make this more robust, wrap the gets/sets in a try/catch (and/or check fortypeof window !== 'undefined') to avoid runtime errors when storage is unavailable or in non‑browser environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `SubmissionPanel`, the `failed_terminal` state renders `RevealSheet` with `state="failed_retryable"`, which is likely a copy‑paste error and should probably use a distinct `failed_terminal`/`failed` state to avoid misleading messaging.
- When creating the draft you pass `barangay: formData.step2.municipalityLabel ?? ''`, which looks semantically mismatched; consider adding a dedicated municipality field to the payload or wiring the correct barangay label/id into `barangay` to avoid confusing downstream consumers.
- The new `localStorage` usage in `Step2WhoWhere` runs unguarded in a `useEffect`; to make this more robust, wrap the gets/sets in a try/catch (and/or check for `typeof window !== 'undefined'`) to avoid runtime errors when storage is unavailable or in non‑browser environments.
## Individual Comments
### Comment 1
<location path="apps/citizen-pwa/src/components/SubmitReportForm/index.tsx" line_range="75-84" />
<code_context>
+ const msisdnHash = await hashPhone(formData.step2.reporterMsisdn)
</code_context>
<issue_to_address>
**issue:** Guard `hashPhone` against empty or optional phone numbers to avoid unnecessary errors.
The old flow only set `reporterMsisdnHash` when a valid phone was present; now `hashPhone` runs unconditionally on `formData.step2.reporterMsisdn`. If `hashPhone` expects a non-empty MSISDN, an empty/optional value could throw and block submission. Either enforce `reporterMsisdn` as required and validated earlier, or guard the call as before:
```ts
const msisdnHash = formData.step2.reporterMsisdn
? await hashPhone(formData.step2.reporterMsisdn)
: undefined
// ...
...(msisdnHash ? { reporterMsisdnHash: msisdnHash } : {}),
```
</issue_to_address>
### Comment 2
<location path="apps/citizen-pwa/src/components/SubmitReportForm/index.tsx" line_range="83-84" />
<code_context>
- severity,
- location: { lat, lng },
- ...(phone && smsConsent ? { reporterMsisdnHash: await hashPhone(phone) } : {}),
+ reportType: formData.step1.reportType as ReportType,
+ barangay: formData.step2.municipalityLabel ?? '',
+ description:
+ formData.step2.patientCount > 0 ? `Patients: ${String(formData.step2.patientCount)}` : '',
</code_context>
<issue_to_address>
**question (bug_risk):** Double-check that `barangay` is meant to contain the municipality label rather than a barangay label.
The new draft sets `barangay` from `step2.municipalityLabel`, which inverts the usual hierarchy (barangay is more granular than municipality). Since `Step2Data` already distinguishes `barangayId` and `municipalityLabel`, this risks overloading the `barangay` field and degrading data quality if consumers expect an actual barangay name. Consider either adding a separate `municipality`/`municipalityLabel` field to the payload, or populating `barangay` with a barangay label and keeping municipality distinct.
</issue_to_address>
### Comment 3
<location path="apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx" line_range="414-415" />
<code_context>
}
}
+ localStorage.setItem('bantayog.reporter.name', reporterName)
+ localStorage.setItem('bantayog.reporter.msisdn', reporterMsisdn)
+
onNext({
</code_context>
<issue_to_address>
**🚨 question (security):** Persisting reporter name and phone in `localStorage` has privacy and security implications.
This PII will persist indefinitely in the browser and be readable by any script on this origin. That may be fine, but the tradeoff should be explicit. Depending on your privacy/threat model, consider instead:
- Storing a hashed or partially masked MSISDN,
- Making this a user opt‑in (“remember my details”), and/or
- Using shorter-lived storage (e.g. sessionStorage) if long-term persistence isn’t required.
If you keep this behavior, a clear UX indication and opt‑out would help avoid privacy surprises.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/citizen-pwa/src/App.routes.test.tsx`:
- Around line 44-47: The test "shows the report form at /report" currently only
asserts the form mounts; update it to also assert that the CitizenShell UI is
not present by adding a negative assertion after renderAppAt('/report') — e.g.,
use screen.queryByText or queryByRole to confirm elements rendered by
CitizenShell (banner title, bottom nav label, or the component name) are
null/absent; locate the test case in App.routes.test.tsx and modify the
it('shows the report form at /report', ...) block to include the negative
assertion so the route fails if CitizenShell is reintroduced.
In `@apps/citizen-pwa/src/components/SubmitReportForm/index.tsx`:
- Around line 190-194: The useEffect auto-submit calls machine.submit() on mount
which can run twice in React Strict Mode and bypass the machine's state !==
'idle' guard due to stale closure, causing duplicate Firebase writes; fix by
adding an idempotency guard in the component (e.g., a ref like
hasAutoSubmittedRef or checking draft.syncing/draft.id) and consult it inside
the useEffect before calling machine.submit(), or augment machine.submit() to be
idempotent by early-returning when the draft id is already syncing; reference
useEffect, machine.submit, state !== 'idle', and the draft/syncing flag when
implementing the guard.
In `@apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx`:
- Around line 414-415: Do not persist raw reporter PII indefinitely: remove the
long-lived localStorage write for reporterMsisdn (the 'bantayog.reporter.msisdn'
setItem) so the phone number is never stored in persistent browser storage, and
for reporterName either switch to sessionStorage or implement a retention TTL by
storing a companion timestamp key (e.g., 'bantayog.reporter.name.timestamp') and
clear the name when expired; update the Step2WhoWhere component to only use
ephemeral state/sessionStorage for PII and add logic to clear keys after the TTL
or on explicit user opt-out.
- Around line 369-378: The useEffect that reads localStorage (calling
localStorage.getItem) can throw in private/blocked modes; wrap those reads in a
try/catch and degrade gracefully by not setting persistent values (i.e., still
call setReporterName/setReporterMsisdn/setHasMemory only when reads succeed),
and similarly wrap any writes performed on the "Continue" path so they don't
throw (catch SecurityError/QuotaExceededError and skip persisting). Update the
effect that calls localStorage.getItem and the Continue-path code that writes to
localStorage so both use try/catch, avoid crashing, and fall back to
non-persistent behavior while still updating component state (functions:
setReporterName, setReporterMsisdn, setHasMemory).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d24b8675-7d14-4e4f-8442-fda1110c7397
📒 Files selected for processing (7)
apps/citizen-pwa/src/App.routes.test.tsxapps/citizen-pwa/src/components/CitizenShell.tsxapps/citizen-pwa/src/components/SubmitReportForm.tsxapps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsxapps/citizen-pwa/src/components/SubmitReportForm/index.tsxapps/citizen-pwa/src/routes.tsxdocs/progress.md
💤 Files with no reviewable changes (1)
- apps/citizen-pwa/src/components/SubmitReportForm.tsx
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx (2)
369-378:⚠️ Potential issue | 🟠 MajorGuard browser-storage access on both prefill and persist paths.
Line 370-Line 371 and Line 414-Line 415 call
localStoragedirectly; this can throw (SecurityError, quota errors) and crash Step 2 in restricted environments.Proposed defensive fix
useEffect(() => { - const savedName = localStorage.getItem('bantayog.reporter.name') - const savedMsisdn = localStorage.getItem('bantayog.reporter.msisdn') - if (savedName || savedMsisdn) { - // eslint-disable-next-line react-hooks/set-state-in-effect - if (savedName) setReporterName(savedName) - if (savedMsisdn) setReporterMsisdn(savedMsisdn) - setHasMemory(true) - } + try { + const savedName = localStorage.getItem('bantayog.reporter.name') + const savedMsisdn = localStorage.getItem('bantayog.reporter.msisdn') + if (savedName || savedMsisdn) { + // eslint-disable-next-line react-hooks/set-state-in-effect + if (savedName) setReporterName(savedName) + if (savedMsisdn) setReporterMsisdn(savedMsisdn) + setHasMemory(true) + } + } catch (err) { + console.warn('[Step2WhoWhere] localStorage read skipped:', err) + } }, []) @@ - localStorage.setItem('bantayog.reporter.name', reporterName) - localStorage.setItem('bantayog.reporter.msisdn', reporterMsisdn) + try { + localStorage.setItem('bantayog.reporter.name', reporterName) + localStorage.setItem('bantayog.reporter.msisdn', reporterMsisdn) + } catch (err) { + console.warn('[Step2WhoWhere] localStorage write skipped:', err) + }As per coding guidelines, "Defensive Programming: Assume external input is malicious/broken. Validate at the boundary. Never swallow errors with an empty catch block."
Also applies to: 414-416
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx` around lines 369 - 378, The component Step2WhoWhere directly accesses localStorage in the effect that prefills reporterName/reporterMsisdn (where setReporterName, setReporterMsisdn, setHasMemory are used) and in the persist path, which can throw in restricted environments; wrap both reads and writes to localStorage.getItem/setItem in try/catch, validate returned values (e.g., ensure they are strings/non-empty) before calling setReporterName/setReporterMsisdn, and on error fall back to not setting state and optionally log the error (do not use an empty catch). Remove the eslint-disable comment and ensure all localStorage usage in this component is guarded this way.
414-415:⚠️ Potential issue | 🟠 MajorAvoid indefinite persistence of raw reporter PII in
localStorage.Saving name + phone in long-lived browser storage is a privacy/compliance risk. At minimum, add retention/expiry and avoid persistent storage for raw MSISDN.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx` around lines 414 - 415, The code in Step2WhoWhere.tsx currently writes raw PII into localStorage via localStorage.setItem('bantayog.reporter.name', reporterName) and localStorage.setItem('bantayog.reporter.msisdn', reporterMsisdn); change this to avoid indefinite persistence by (1) not storing raw MSISDN—store a hashed value or only non-sensitive derivative (e.g., last 4 digits) under a different key, (2) include an expiry timestamp alongside any stored reporter data and ensure the component (Step2WhoWhere) clears/ignores the entry when expired, and (3) prefer sessionStorage or in-memory state for short-lived data; update code paths that read those keys to handle the new format (value+expiry or hashed/partial MSISDN) and to delete the storage when expired or after submission.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsx`:
- Around line 369-378: The component Step2WhoWhere directly accesses
localStorage in the effect that prefills reporterName/reporterMsisdn (where
setReporterName, setReporterMsisdn, setHasMemory are used) and in the persist
path, which can throw in restricted environments; wrap both reads and writes to
localStorage.getItem/setItem in try/catch, validate returned values (e.g.,
ensure they are strings/non-empty) before calling
setReporterName/setReporterMsisdn, and on error fall back to not setting state
and optionally log the error (do not use an empty catch). Remove the
eslint-disable comment and ensure all localStorage usage in this component is
guarded this way.
- Around line 414-415: The code in Step2WhoWhere.tsx currently writes raw PII
into localStorage via localStorage.setItem('bantayog.reporter.name',
reporterName) and localStorage.setItem('bantayog.reporter.msisdn',
reporterMsisdn); change this to avoid indefinite persistence by (1) not storing
raw MSISDN—store a hashed value or only non-sensitive derivative (e.g., last 4
digits) under a different key, (2) include an expiry timestamp alongside any
stored reporter data and ensure the component (Step2WhoWhere) clears/ignores the
entry when expired, and (3) prefer sessionStorage or in-memory state for
short-lived data; update code paths that read those keys to handle the new
format (value+expiry or hashed/partial MSISDN) and to delete the storage when
expired or after submission.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 96891b79-bd9f-47ef-bb45-e8f0038428f8
📒 Files selected for processing (2)
apps/citizen-pwa/src/components/SubmitReportForm/Step2WhoWhere.tsxapps/citizen-pwa/src/styles/globals.css
- barangay field: use barangayId (name) ?? municipalityLabel ?? '' — fixes semantic inversion where municipality was stored as barangay - hashPhone: guard against empty MSISDN (defensive; field is required in Step 2) - Strict Mode double-submit: add hasAutoSubmittedRef guard in SubmissionPanel - localStorage: wrap reads and writes in try/catch for restricted/private mode - MSISDN storage: move to sessionStorage to avoid long-lived PII in localStorage; name stays in localStorage (lower sensitivity) - Test: assert /report renders without CitizenShell banner/nav - Comment: clarify failed_terminal → RevealSheet state="failed_retryable" is intentional Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Apr 23, 2026
This was referenced May 1, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/reportto a 3-step wizard (Step1Evidence → Step2WhoWhere → Step3Review) with data accumulation across stepsSubmitReportForm.tsxand/report/newroute; consolidated to single/reportentry pointmachine.submit()on mount, RevealSheet for all terminal states)navigator.geolocation, specific error messages perGeolocationPositionErrorcode (permission denied, timed out)localStorage(bantayog.reporter.*) with "Pre-filled from your last report" hint; saves on ContinueChanges
SubmitReportForm/index.tsx— rewritten asWizardContainer(step state) +SubmissionPanel(submission machine); auto-starts submission; all failure states renderRevealSheetSubmitReportForm/Step2WhoWhere.tsx— GPS guard + specific error messages + localStorage pre-fill on mount + persist on Continueroutes.tsx—/report→ new wizard (noCitizenShellwrapper,hideBottomNav: true); removed/report/newApp.routes.test.tsx— updated mock path and test assertions for new route structureTest plan
pnpm --filter @bantayog/citizen-pwa typecheck— PASSpnpm --filter @bantayog/citizen-pwa lint— PASSpnpm --filter @bantayog/citizen-pwa exec vitest run— 101 passed, 2 todoSummary by Sourcery
Implement a new three-step report submission wizard with improved submission handling, GPS robustness, and reporter detail persistence.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Summary by CodeRabbit
New Features
Improvements
Style
Tests
Documentation