feat: implement help flows (request, availability, assigned, my requests)#195
Conversation
…requests features with backend integration
kckagancan
left a comment
There was a problem hiding this comment.
NEPH Android Review — Major Issues Only
android/app/src/main/java/com/neph/features/assignedrequest/data/AssignedRequestRepository.kt
fetchAssignedRequests()effectively assumes a single assignment by reading oneassignmentobject and wrapping it inlistOf(...). If the product/backend can return multiple assigned items, this is the wrong data model.helpTypeis stored as a single string (need_type). This does not align well with the newer multi-help-type request model.locationLabelis too weak: it only showsLat/Lonor"Location unavailable". For an assigned-response flow, this is not operationally strong enough if better location fields exist. (Location may use the same things as in profile)- The data shape here is notably different from the help-request mapping elsewhere (
snake_case, flat lat/lon, etc.), which raises a contract consistency risk.
android/app/src/main/java/com/neph/features/assignedrequest/presentation/AssignedRequestScreen.kt
- The screen is visually built like a list, but the repository currently behaves like single-assignment logic. That mismatch is risky.
- The screen is too passive for an assigned-workflow page:
- no detail action
- no responder/requester coordination action
- no stronger operational UI
- Status is shown as a raw backend string, which may surface ugly/internal enum values directly to users.
- Location display is still too weak because it depends on the repository’s lat/lon-only label.
android/app/src/main/java/com/neph/features/availability/data/AvailabilityRepository.kt
- Availability state is primarily driven by local cached state (
SharedPreferences) with no visible “fetch real backend state on app start/open” flow here. This can drift from server truth. - The model is probably too thin for the feature’s importance: only
isAvailableandassignmentIdare tracked, which may become fragile for a richer volunteer assignment flow. - The request body uses
isAvailable; if backend expects a different naming convention, this is brittle. Combined with other files, the backend contract handling in this feature feels inconsistent.
android/app/src/main/java/com/neph/features/home/presentation/HomeScreen.kt
Request Helpis opened directly with no active-request gate. This conflicts with the rule that a user should not proceed into a new request flow when they already have an active one.- Availability state is loaded from the local repository state, which may not reflect the real backend state.
- If turning availability on results in an assignment, the UI only shows an info message; it does not guide the user into the assigned-request flow.
Available to Helpis shown to all authenticated users with no visible role/capability check.
android/app/src/main/java/com/neph/features/myhelprequests/data/MyHelpRequestsRepository.kt
- The model still assumes a single
helpTypestring, while the request creation UI allows multiple help types. - Location display is too weak again: only lat/lon or
"Location unavailable", despite the request form collecting more meaningful location structure. - This repository is shaped like a generic history list, not an active-request-status-focused model.
- Its backend field names/shapes differ quite a lot from the assigned-request side, which creates a broader API contract consistency risk.
android/app/src/main/java/com/neph/features/myhelprequests/presentation/MyHelpRequestsScreen.kt
- This screen does not match the intended product role. It behaves like a passive list/history page instead of:
- showing the user’s current active request
- surfacing its status clearly
- showing assigned responder details
- allowing cancel
- showing “no active request” when none exists
- There is no detail flow, no cancel action, and no assigned-person info.
- Status is shown raw and location is weak, so even the visible request summary is not very strong.
- Structurally this page is still closer to “request history list” than “active request tracking page”.
android/app/src/main/java/com/neph/features/requesthelp/data/RequestHelpRepository.kt
- This is one of the biggest issues in the whole set.
- The submission model only contains:
needTypedescriptionisSavedLocally
- That means the real structured form data is not being sent structurally:
- affected people count
- risk flags
- vulnerable groups
- blood type
- location fields
- contact info
- confirmation
needTypeis still a single string, even though the UI supports multi-select help types.- The repository requires a token for creation, so the guest flow is not real at submission time.
- The active-request check is only implemented for authenticated users, which reinforces that the guest version of the flow is incomplete.
android/app/src/main/java/com/neph/features/requesthelp/presentation/RequestHelpScreen.kt
- The form UI is close to the intended design, but the actual submission is still old-contract based:
- most structured fields are flattened into the
description - help types are joined into one string
- most structured fields are flattened into the
- This makes the feature look rich in UI while remaining structurally weak in data.
- The guest flow is only cosmetic right now:
- guests can fill the form
- but cannot actually submit
- The “only one active request” rule is enforced too late:
- the user can fill the whole form
- only at submit time they are blocked
- instead of being redirected to their current active request early
- Success handling is weak:
- it sets a success message
- then immediately navigates back
- so the success state may never be meaningfully seen
Canceluses back navigation rather than a guaranteed return-to-home behavior.
android/app/src/main/java/com/neph/navigation/AppNavGraph.kt
- The active-request rule is not enforced at navigation level:
- Home always navigates directly to
RequestHelp - there is no guard that redirects a user with an active request to the active-request/status page
- Home always navigates directly to
- Request Help route is guest-accessible, but the real submit flow requires auth. That makes the guest flow navigation-open but functionally incomplete.
- This file confirms that the active-request gating problem is real and not being handled elsewhere.
Highest-Priority Problems Across These Files
- Request Help UI is rich, but the backend submission contract is still minimal and old-style.
- Multi-select help type exists in UI but is collapsed into a single string at submission/display layers.
- Guest Request Help flow is not truly supported.
- Single active-request rule is enforced too late and not at navigation/entry level.
- My Help Requests does not yet behave like the intended active-request status tracking page.
- Availability / Assigned Request flow still feels too shallow for the operational workflow it is supposed to support.
…mprove request/assignment flows
|
Updated Android to match the current backend and workflow expectations:
Still intentionally unchanged:
Could you recheck @kckagancan ? |
erinc00
left a comment
There was a problem hiding this comment.
Thanks for the update. The latest revision addresses a large part of the earlier review: the request submission payload is now structured, multi-help-type handling is no longer collapsed into a single string at the repository layer, current-request/history handling is much stronger, and the assigned-request flow now has more meaningful UI and actions.
That said, I still do not think this is fully ready to merge yet. I still see two important issues:
-
AvailabilityRepository.refreshAssignmentState() still does not fully refresh availability from backend truth
Right now the refresh logic only checks whether there is a current assignment. If there is an assignment, isAvailable is forced to true, but if there is no assignment, the code keeps the previous cached isAvailable value and only clears assignmentId. That means stale local availability can still survive across app restarts or screen reloads. I think this does not fully resolve the earlier concern about availability being primarily driven by cached local state rather than real backend state. -
The active-request gate in HomeScreen is still fail-open
The request-help guard is earlier than before, which is good, but if hasActiveHelpRequest() throws for any reason, the current implementation still falls through to onRequestHelp(). That means the “single active request” rule can still be bypassed on network/backend/auth-related failures. If this rule is important product logic, I do not think the app should open a fresh request flow when the active-request check could not be completed reliably.
Because of these two points, I would still keep this as request changes rather than approve.
…date' into feature/assigned-requests-son
erinc00
left a comment
There was a problem hiding this comment.
Looks much better now. The two main blockers from my previous review appear to be addressed:
• AvailabilityRepository.refreshAssignmentState() now reads the real backend availability state instead of relying on stale cached isAvailable.
• The active-request guard in HomeScreen is no longer fail-open; if the check cannot be completed, the user is shown an error instead of being sent directly into a new request flow.
More broadly, the latest revision also improves the assigned-request and my-help-requests flows in the right direction: richer backend mapping, better location/status handling, stronger request details, and guest-compatible request tracking/status updates.
I do not currently see a remaining major blocker in the surfaced changes, so I’m okay with approving this.
Small follow-up notes only:
• fetchGuestHelpRequests() may still be brittle if one stored guest-tracked request becomes invalid or stale; it may be worth handling those entries more defensively later.
• guest access token handling in guest status-update paths could be made a bit more robust/consistent as a follow-up.
This PR implements and integrates the core help-related flows on the Android side, replacing placeholder screens with backend-connected features.
Implemented:
Key details:
Technical notes:
Limitations:
Closes #193