Christophe Vila
e2b2bf9611
Garmin auth/sync routes move under /api/garmin/*; sync.Service becomes garmin.Sync with garmin.SyncConfig/ClientConfig; applog becomes internal/log; the test mock moves into the garmin package as MockClient (breaking the test-only import cycle the merge created); stale test URLs and type names updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
143 lines
6.6 KiB
Markdown
143 lines
6.6 KiB
Markdown
# Profile deletion
|
|
|
|
Status: approved, not yet implemented.
|
|
|
|
## Problem
|
|
|
|
`docs/IDEAS.md` backlog item: "profile deletion: add a way to delete our
|
|
profile with a dedicated button on the profile page (which by extension will
|
|
also logout the user, to be consistent with the logic that login will trigger
|
|
the profile creation)".
|
|
|
|
Today there is no way for a user to remove their geniusrun account. The
|
|
closest existing feature, "Reset all" (`GarminConnection.tsx` /
|
|
`POST /api/sync/reset`), only wipes synced activity data and rewinds the
|
|
backfill watermark -- it leaves the `users` row, `profile`, Garmin
|
|
credentials, and custom workout-kind rules untouched. Profile deletion is a
|
|
strictly bigger, irreversible operation: the entire account for the signed-in
|
|
OIDC subject, gone, followed by a real logout.
|
|
|
|
## Design
|
|
|
|
### Data model: cascading deletes
|
|
|
|
The schema has FK columns pointing at `users(id)` (`profile.user_id`,
|
|
`workout_kinds.user_id`, `activities.user_id`, `sync_state.user_id`,
|
|
`sync_runs.user_id`) and at `workout_kinds(id)`
|
|
(`workout_type_paces.workout_kind_id`), none of which currently cascade.
|
|
`laps`/`activity_samples`/`kind_assignments` already cascade off `activities`
|
|
(`ON DELETE CASCADE`), which is what lets `ResetAllSyncedData` get away with
|
|
a single `DELETE FROM activities`. SQLite's `foreign_keys` pragma is already
|
|
enabled on every connection (`db.go`'s `_pragma=foreign_keys(1)`), so the same
|
|
mechanism works for the rest of the schema.
|
|
|
|
`schema.sql` is edited directly (no migration history, per project
|
|
convention) to add `ON DELETE CASCADE` to:
|
|
|
|
- `profile.user_id REFERENCES users(id)`
|
|
- `workout_kinds.user_id REFERENCES users(id)`
|
|
- `workout_type_paces.workout_kind_id REFERENCES workout_kinds(id)`
|
|
- `activities.user_id REFERENCES users(id)`
|
|
- `sync_state.user_id REFERENCES users(id)`
|
|
- `sync_runs.user_id REFERENCES users(id)`
|
|
|
|
With that in place, deleting a user becomes one statement:
|
|
|
|
```go
|
|
// store/users.go
|
|
func (db *DB) DeleteUser(ctx context.Context, userID int64) error {
|
|
_, err := db.ExecContext(ctx, `DELETE FROM users WHERE id = ?`, userID)
|
|
return err
|
|
}
|
|
```
|
|
|
|
After this change, regenerate `docs/DATABASE.md` (`go run ./cmd/dumpschema`).
|
|
|
|
### API: `DELETE /api/profile`
|
|
|
|
New handler alongside the existing `GET`/`PUT /api/profile` routes, inside
|
|
the `requireProvisionedUser` group (`server.go`). `userID` comes from
|
|
`userIDFromContext`, same as every other handler -- no client-supplied id.
|
|
|
|
1. If a sync is currently running for this user (`s.userSyncRunning[userID]`,
|
|
the same flag `backgroundSync` sets), respond `409 Conflict` ("a sync is
|
|
in progress for this account; wait for it to finish before deleting your
|
|
profile") rather than deleting rows out from under an in-flight write.
|
|
2. Call `s.DB.DeleteUser(ctx, userID)`.
|
|
3. Tear down this user's cached Garmin state (new `Server` method, e.g.
|
|
`removeGarminClient(userID)`):
|
|
- Under `s.mu`, pop `s.userGarmin[userID]` and delete the userID entry
|
|
from `userSync`, `userAuthStatus`, `userAuthMessage`, and
|
|
`userSyncRunning`.
|
|
- If a client existed, call `client.Close()` outside the lock to
|
|
terminate its subprocess (mirrors how `handleUpdateProfile` calls
|
|
`client.UpdateCredentials` outside `s.mu`).
|
|
- Best-effort `os.RemoveAll` on `filepath.Join(s.GarminBase.TokenStorePath,
|
|
strconv.FormatInt(userID, 10))` when `TokenStorePath` is configured --
|
|
the same path `clientFor` computes when building a client. Log on
|
|
error; this is cleanup of an already-orphaned directory, not something
|
|
that should fail the request that already deleted the DB row.
|
|
4. Respond `204 No Content`.
|
|
|
|
### Frontend
|
|
|
|
- `api/client.ts`: `deleteProfile: () => request<void>("/api/profile", {
|
|
method: "DELETE" })`.
|
|
- `Profile.tsx`: a final `<fieldset className="kind-editor danger-zone">`
|
|
("Danger zone") containing a `button-danger` "Delete profile" button
|
|
(same class `GarminConnection`'s "Reset all" uses). Clicking it reveals an
|
|
inline confirmation block (local component state, no separate modal
|
|
component -- same "reveal inline" shape `ClassifyControl` already uses for
|
|
its dropdown):
|
|
- Explanatory copy: this permanently deletes the account -- Garmin
|
|
credentials, every custom rule, every synced activity -- and logs the
|
|
user out. Cannot be undone.
|
|
- A text input; a "Permanently delete" button stays disabled until the
|
|
input's value is exactly `DELETE`.
|
|
- A "Cancel" button collapses the block back.
|
|
- On confirm: call `api.deleteProfile()`. On success, build a real
|
|
`<form method="post" action="{BASE_URL}/api/session/logout">` via the DOM
|
|
and submit it immediately -- the same POST-navigation the header's Log out
|
|
button already performs (`App.tsx`), required because logout must be a
|
|
real browser navigation through Keycloak's end-session redirect, not a
|
|
`fetch`. On failure, surface the error via the same `error` state /
|
|
`<p className="error">` pattern the rest of `Profile.tsx` already uses,
|
|
and leave the confirmation block open so the user can retry.
|
|
|
|
### Error handling
|
|
|
|
- Sync-in-progress: `409`, clear message, surfaced like every other API
|
|
error on this page.
|
|
- DB/teardown errors: `500` with `err.Error()`, same as the rest of this
|
|
codebase's handlers.
|
|
- The type-`DELETE`-to-confirm gate is UI-only -- same trust boundary as
|
|
every other destructive action in this app (e.g. Reset all's
|
|
`window.confirm`). The backend does not require a second confirmation
|
|
token.
|
|
|
|
### Testing
|
|
|
|
- `backend/internal/store`: `TestDeleteUser` -- provision a user, give them
|
|
activities (with laps/samples/kind_assignments), workout kinds (with
|
|
paces), sync_state, and sync_runs; delete; assert every row is gone.
|
|
Adversarial isolation check per this repo's convention
|
|
(`isolation_test.go`): provision two users, delete one, assert the other's
|
|
rows (profile, workout kinds, activities, etc.) are untouched.
|
|
- `backend/internal/api`: handler test for `DELETE /api/profile` against
|
|
`newTestServer` + `mock.Client` -- success path (204, user actually gone
|
|
from `GetUserBySub`), and the sync-in-progress 409 (set
|
|
`userSyncRunning[userID] = true` first).
|
|
- No frontend test suite exists yet (per `CLAUDE.md`) -- manually
|
|
smoke-tested in the browser: delete flow ends up back at the login/create
|
|
profile screen, a fresh login for that OIDC subject lands on "Create your
|
|
profile" again (proving the account is truly gone, not just logged out).
|
|
|
|
## Out of scope
|
|
|
|
- Any "export my data before deleting" flow -- not requested.
|
|
- Re-authentication (password/MFA re-entry) before deletion -- this app has
|
|
no such step anywhere else (e.g. Reset all), so it isn't introduced here
|
|
either.
|
|
- Admin-initiated deletion of another user's account -- there is no admin UI
|
|
in this app.
|