Create the user profile document on signup; implement updateProvider - #26
Merged
Conversation
Two related gaps on the write path, both of which made a Firebase-backed build unusable and one of which was already broken in debug. **Signup created no profile document.** `IAuthService.signup` only creates the credential — it accepts `phoneNumber` and `role` and has nowhere to put them, so a signed-up user had an auth account and a session but no `clients`/`providers` document. Every later profile read resolved to nothing. `AuthRepository.signup` now writes the matching DTO (keyed on the auth uid) before persisting the session, so it applies to both `BackendServiceStub` and `FirebaseBackendService` rather than only the Firebase path the audit called out. The credential and the profile are two non-atomic writes; only a server-side transaction could fix that. The ordering is deliberate — the profile write happens first, so a failure leaves no session rather than a session pointing at a missing profile, and it is surfaced as an error the user can read instead of being swallowed into a silent success. `AuthViewModel` renders `exception.message` verbatim, so the message is written as user-facing text. The residual orphaned-account case is now recorded in ROADMAP.md. New provider profiles set `verificationStatus` explicitly: `ProviderDto`'s default is `""` (PR #24 gave every DTO param a default so Firestore's mapper can construct them), and `ProviderDto.toDomain()` runs that field through an unguarded `VerificationStatus.valueOf()`, so a defaulted value would throw on the first read back. **`ProviderRepository.updateProvider` was a hard stub** returning `Result.Error(Exception("Not implemented in backend service"))` for every input, behind a `// TODO: Add updateProvider to IBackendService`. That method is what `CleanerProfileViewModel.saveProfile` calls, so cleaner profile edits always failed, in debug as well as release. `updateProvider` now exists on `IBackendService` (upsert, mirroring `updateClient`), is implemented on both backends, and the repository delegates to it and caches the response. A new `Provider.toDto()` mapper is the inverse of the existing `ProviderDto.toDomain()`. `BackendServiceStub` gained a mutable `providerStore` (the convention `reviewStore` already uses) so profiles created or edited in-session are visible to later reads; `searchProviders` returns a snapshot of it rather than the backing list. Tests: 6 new `AuthRepositoryTest` cases covering both profile shapes, the verification-status guard, the ordering of the two writes, and the failure path leaving no session; a new `ProviderRepositoryTest`; stub upsert and snapshot tests; mapper round-trip tests. Docs reconciled: AUDIT_REPORT, README, ROADMAP, TECH_LEAD_REVIEW and IMPLEMENTATION_SUMMARY all cited these as open. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G6LvbheYzc1mN9toTuuPR5
Self-review catch: I gave `BackendServiceStub` a mutable `providerStore` so provider profiles created at signup survive to the next read, but left `updateClient` as a pure echo that stores nothing. The asymmetry bites in debug. `getClient` falls through to a synthetic placeholder for any unknown id — `name = "Client $clientId"`, a fabricated `$clientId@example.com` email, and an **empty** phone number. So a customer who had just signed up would see the name, email and phone they typed replaced by placeholder text on the very next profile read, and `ClientRepository.updateClient` (customer profile editing) would appear to save and then silently revert. `clientStore` mirrors `providerStore`: seeded from `seedClients`, read by `getClient`, upserted by `updateClient`. The synthetic fallback stays for genuinely unknown ids — existing tests cover it — but it no longer shadows real data. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G6LvbheYzc1mN9toTuuPR5
There was a problem hiding this comment.
Pull request overview
This PR closes two write-path gaps in the :data layer that prevented Firebase-backed usage: (1) signup now creates the corresponding clients/providers profile document, and (2) provider profile edits now work end-to-end via a new IBackendService.updateProvider upsert implemented for both the stub and Firebase backends.
Changes:
- Create the user profile document on signup in
AuthRepository(before persisting the session), including explicitProviderDto.verificationStatusinitialization to avoid mapper crashes. - Add and implement
IBackendService.updateProvideracross backends and wireProviderRepository.updateProviderto delegate + cache like the client path. - Add/extend unit tests around signup/profile creation, provider upsert/caching, backend stub behavior, and mapper round-trips; reconcile docs to mark the gaps resolved (while calling out remaining non-atomicity).
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
data/src/main/java/com/example/diamonds/data/repository/AuthRepository.kt |
Writes the matching profile doc on successful signup (client/provider) before saving the session; wraps profile-write failures into a user-facing error. |
data/src/main/java/com/example/diamonds/data/remote/backend/IBackendService.kt |
Adds updateProvider(provider: ProviderDto) contract (upsert) mirroring updateClient. |
data/src/main/java/com/example/diamonds/data/remote/backend/FirebaseBackendService.kt |
Implements updateProvider via Firestore set(...) on providers collection. |
data/src/main/java/com/example/diamonds/data/remote/backend/BackendServiceStub.kt |
Adds mutable providerStore and implements provider upsert + snapshot searchProviders. |
data/src/main/java/com/example/diamonds/data/repository/ProviderRepository.kt |
Replaces hard-stubbed updateProvider with real backend delegation + local cache upsert. |
data/src/main/java/com/example/diamonds/data/mapper/Mappers.kt |
Adds Provider.toDto() mapper (inverse of ProviderDto.toDomain()). |
data/src/main/java/com/example/diamonds/data/remote/auth/FirebaseAuthService.kt |
Documents that signup only creates the auth credential; profile persistence happens in AuthRepository. |
app/src/main/java/com/example/diamonds/di/Modules.kt |
Wires IBackendService into AuthRepository via Hilt. |
data/src/test/java/com/example/diamonds/data/repository/AuthRepositoryTest.kt |
Adds signup tests covering client/provider doc creation, verification status, ordering, and error reporting. |
data/src/test/java/com/example/diamonds/data/repository/ProviderRepositoryTest.kt |
New tests for offline handling, field fan-out, caching, and error propagation/wrapping in updateProvider. |
data/src/test/java/com/example/diamonds/data/remote/backend/BackendServiceStubTest.kt |
Adds tests for provider upsert visibility, overwrite semantics, and snapshot search behavior. |
data/src/test/java/com/example/diamonds/data/mapper/MappersTest.kt |
Adds Provider.toDto() round-trip and enum-serialization tests. |
TECH_LEAD_REVIEW.md |
Updates findings/status to reflect updateProvider and signup profile-doc creation as addressed. |
ROADMAP.md |
Marks fixed items and explicitly records remaining non-atomic signup-write gap. |
README.md |
Marks updateProvider as resolved in the known-issues table. |
IMPLEMENTATION_SUMMARY.md |
Reconciles “signup creates no profile doc” to resolved (with non-atomicity caveat). |
AUDIT_REPORT.md |
Marks provider update and signup profile-doc issues as resolved (with remaining non-atomicity noted). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Aug 2, 2026
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
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.
Two related write-path gaps. Both make a Firebase-backed build unusable; one is already broken in debug today.
1. Signup created no profile document
IAuthService.signuponly creates the credential. It acceptsphoneNumberandroleand has nowhere to put either, so a signed-up user ended up with an auth account and a session but noclients/providersdocument — every later profile read resolved to nothing.AuthRepository.signupnow writes the matching DTO, keyed on the auth uid, before persisting the session. Doing it in the repository rather than inFirebaseAuthServicemeans it covers both backends, not just the Firebase path the audit called out.On the non-atomicity. The credential and the profile are two separate writes and only a server-side transaction could make them one. The ordering is deliberate: the profile write happens first, so a failure leaves no session at all rather than a session pointing at a profile that doesn't exist — the latter would surface later as unexplained "not found" errors on every screen. The failure is returned as an error the user can act on rather than swallowed into a silent success;
AuthViewModelrendersexception.messageverbatim, so the message is written as user-facing text. The residual case (auth account exists, profile doesn't, email is now taken so signup can't be retried) is recorded inROADMAP.mdas needing a server-side callable function.New provider profiles set
verificationStatusexplicitly.ProviderDto's default is""— PR #24 gave every DTO param a default so Firestore's object mapper can construct them — andProviderDto.toDomain()runs that field through an unguardedVerificationStatus.valueOf(). A defaulted value would throw on the first read back. There's a test pinning this.2.
ProviderRepository.updateProviderwas a hard stubIt returned that for every input.
CleanerProfileViewModel.saveProfilecalls it, so cleaner profile edits always failed — in debug as well as release.IBackendService.updateProvideradded (an upsert, mirroringupdateClient), implemented onBackendServiceStubandFirebaseBackendService.ProviderRepository.updateProviderdelegates to it and caches the response, matchingClientRepository.updateClient.Provider.toDto()mapper — the inverse of the existingProviderDto.toDomain(), with enums serialised as theirnameso the two round-trip.BackendServiceStubgained a mutableproviderStore(the conventionreviewStorealready uses at:477) so profiles created or edited in-session are visible to later reads;searchProvidersreturns a snapshot of it rather than the backing list.Tests
AuthRepositoryTest— 6 new cases: client profile shape, provider profile shape, theverificationStatusguard, the ordering of the two writes, no session saved when the profile write fails, and the error message reaching the user. Existing cases updated for the new constructor param.ProviderRepositoryTest— new file: offline, full field fan-out to the backend, caching, error propagation without caching, thrown-exception wrapping.BackendServiceStubTest— upsert inserts and is visible togetProvider; upsert overwrites rather than duplicating;searchProvidersreturns a snapshot.MappersTest—Provider.toDto()round-trip and enum-name serialisation.Docs
AUDIT_REPORT.md,README.md,ROADMAP.md,TECH_LEAD_REVIEW.mdandIMPLEMENTATION_SUMMARY.mdall listed these as open; each is reconciled, with the remaining atomicity gap stated rather than marked done.Verification
No Android SDK in this environment, so CI (
assembleDebug,allUnitTests,assembleDebugAndroidTest) is the compile signal — please treat a red run as blocking.🤖 Generated with Claude Code
https://claude.ai/code/session_01G6LvbheYzc1mN9toTuuPR5
Generated by Claude Code