fix: integrate backend with android and rearrange signup and complete profile flows#174
Conversation
erinc00
left a comment
There was a problem hiding this comment.
Thanks for the update. I reviewed the latest Android changes in detail.
That said, I still see a few issues that should be addressed before merge:
-
AuthRepository.login() clears the session too aggressively after successful auth
In AuthRepository.login(), the access token is already obtained successfully, but if the subsequent profile fetch fails with a non-401 backend error, the token is cleared and the user is effectively logged out. Temporary /profiles/me failures should not necessarily invalidate a valid authenticated session. I think token clearing should be limited to actual auth-invalid cases such as 401. -
SignupScreen writes local profile state before signup succeeds
SignupScreen updates ProfileRepository before the signup request is confirmed successful. If the backend signup fails, local profile state can still be partially written and become inconsistent. This local save should happen only after successful signup, or be rolled back on failure. -
usesCleartextTraffic="true" is too broad as currently configured
I understand this may be needed for emulator/local backend development, but enabling cleartext traffic globally in the manifest is risky. This should ideally be restricted to debug-only behavior or handled through a more scoped network security setup. -
ProfileRepository.syncProfile() is very fragmented and can leave the backend partially updated
ProfileRepository.syncProfile() currently performs multiple sequential backend updates, so partial backend updates are possible if one of the later calls fails. The current backend contract may require this, but it is still worth noting as a consistency risk.
Additional notes / follow-up items:
- CompleteProfileScreen collects date of birth in the UI but syncs only age to the backend
The current profile flow collects date of birth in the UI but syncs only age to the backend. I do not think Android alone should fully solve this in this PR, so this can be treated as a backend follow-up. Still, backend support for storing a real birthDate field should be requested; otherwise, this data cannot round-trip cleanly across sessions/devices.
|
Thanks for the detailed review. Went through the points and updated the Android side accordingly.
Can you review again? @erinc00 |
gulcetahtasiz
left a comment
There was a problem hiding this comment.
Looks good now, thanks for addressing the feedback.
The issues raised in the previous review are properly resolved, especially around session handling, signup flow, and cleartext configuration. The syncProfile mitigation also makes sense given current backend constraints.
The remaining birthDate note can be handled on the backend side. approved
erinc00
left a comment
There was a problem hiding this comment.
Looks good to me overall. I reviewed the latest changes and the implementation seems consistent with the intended flow. I do not see any major blocker from my side at this point, so I’m approving. Ready to merge.
Summary
This PR integrates the backend with Android clients for the authentication and profile flows.
It also updates the signup flow so that users no longer enter profile details such as full name and phone number during signup. These fields are now collected later in the complete profile flow, making the authentication flow simpler and keeping profile completion as a separate step.
Changes
Notes
Can you review? @erinc00 @gulcetahtasiz
Closes #132