From 6d642fed355be8b35a2d4117906c85d89f25b71f Mon Sep 17 00:00:00 2001 From: crthpl Date: Mon, 13 Apr 2026 16:19:20 -0700 Subject: [PATCH] Fan out display-name rename to every cohort with per-cohort suffix collisions (ARB-513) Renaming a user's global display name now propagates to every non-read-only cohort they're in. Collisions with other accounts in a cohort are resolved by appending the smallest available `-N` suffix, surfaced to the user through a preflight/confirmation flow so they consent to any suffix before it's applied. Fixes ARB-513 --- DEPLOY.md | 122 --------- ...ead6335e6f34afa2f9a867b6d08e671feb0d4.json | 20 ++ ...ad09b7ca596147544f719c4b09d6020e8a01b.json | 20 -- ...35ecc50e5fbd111dcf036f88b636958974890.json | 12 + backend/src/db.rs | 123 +++++++-- backend/src/global_db.rs | 17 ++ backend/src/main.rs | 259 ++++++++++++++++-- frontend/src/lib/adminApi.ts | 81 +++++- frontend/src/lib/api.svelte.ts | 5 +- .../[cohort_name]/accounts/+page.svelte | 140 +++++++++- frontend/src/routes/admin/+page.svelte | 94 ++++++- 11 files changed, 674 insertions(+), 219 deletions(-) delete mode 100644 DEPLOY.md create mode 100644 backend/.sqlx/query-46a626a49c0e079a63e534937feead6335e6f34afa2f9a867b6d08e671feb0d4.json delete mode 100644 backend/.sqlx/query-c4f5b6e260992d4e2df494d9ba3ad09b7ca596147544f719c4b09d6020e8a01b.json create mode 100644 backend/.sqlx/query-de02634da7185857e9bbf779b3e35ecc50e5fbd111dcf036f88b636958974890.json diff --git a/DEPLOY.md b/DEPLOY.md deleted file mode 100644 index c15e0e90..00000000 --- a/DEPLOY.md +++ /dev/null @@ -1,122 +0,0 @@ -# Deployment - -## Architecture - -- **Frontend**: Static SPA on Vercel (SvelteKit with `adapter-static`) -- **Backend**: Rust binary on Fly.io (Docker, SQLite with persistent volume) -- Frontend and backend are on different domains, so HTTP API calls use cross-origin requests with CORS. - -## Staging - -- **Frontend**: https://platform-staging-five-gamma.vercel.app (Vercel project `platform-staging` under `trading-bootcamp` team) -- **Backend**: https://trading-bootcamp-staging.fly.dev (Fly app `trading-bootcamp-staging`) - -### Deploy staging backend - -```bash -fly deploy --config backend/fly.staging.toml -``` - -### Deploy staging frontend - -```bash -vercel --prod --scope trading-bootcamp -``` - -### Vercel env vars (Production environment) - -| Variable | Value | -|----------|-------| -| `PUBLIC_KINDE_CLIENT_ID` | `a9869bb1225848b9ad5bad2a04b72b5f` | -| `PUBLIC_KINDE_DOMAIN` | `https://account.trading.camp` | -| `PUBLIC_KINDE_REDIRECT_URI` | `https://platform-staging-five-gamma.vercel.app` | -| `PUBLIC_SERVER_URL` | `wss://trading-bootcamp-staging.fly.dev/api` | -| `PUBLIC_TEST_AUTH` | `false` | - -**Important**: When setting env vars via `vercel env add`, pipe with `printf` (not `echo`) to avoid embedding trailing newlines: -```bash -printf 'value' | vercel env add VAR_NAME production --scope trading-bootcamp -``` - -### Kinde setup - -Add the frontend URL to "Allowed callback URLs" in the Kinde application settings for client ID `a9869bb1225848b9ad5bad2a04b72b5f`. - -## Code changes required for deployment - -### 1. `.dockerignore` - -Excludes `backend/target`, `backend/data`, SQLite files, `node_modules`, `.git`, `.claude` etc. Without this, Docker context transfer is ~733MB instead of ~700KB. - -### 2. `backend/Dockerfile` modifications - -- **`ENV SQLX_OFFLINE=true`**: SQLx does compile-time query checking against a live database by default. In Docker there's no database, so offline mode uses the pre-generated `.sqlx/` cache instead. -- **`COPY ./backend/global_migrations /app/global_migrations`**: The multi-cohort feature added a `global_migrations/` directory that needs to be in the runtime image. - -### 3. `.vercelignore` - -Excludes the same heavy directories from Vercel uploads. - -### 4. CORS: `backend/Cargo.toml` + `backend/src/main.rs` - -Since frontend (Vercel) and backend (Fly.io) are on different domains, the browser blocks cross-origin API requests. The fix: - -- Added `"cors"` feature to `tower-http` in `Cargo.toml` -- Replaced the manual `SetResponseHeaderLayer` for `Access-Control-Allow-Origin: *` with `CorsLayer::permissive()`, which properly handles preflight OPTIONS requests and allows the `Authorization` header - -### 5. Cross-origin HTTP API calls: `frontend/src/lib/apiBase.ts` - -In development, the Vite dev server proxies `/api/*` to the backend (see `frontend/vite.config.ts`). In production, there's no proxy — the frontend and backend are on different domains. - -`apiBase.ts` derives the HTTP base URL from `PUBLIC_SERVER_URL`: -- `wss://host.fly.dev/api` → `https://host.fly.dev` -- `ws://localhost:8080` → `http://localhost:8080` - -`cohortApi.ts` and `adminApi.ts` use `API_BASE` to make absolute URL requests instead of relative `/api/...` paths. This works in both dev (Vite proxy still intercepts) and production (direct cross-origin requests). - -### 6. `frontend/src/routes/[cohort_name]/docs/[slug]/+page.svelte` - -Fixed markdown import paths — the file moved one level deeper into `[cohort_name]/` so the relative imports needed an extra `../` (e.g. `../../../../../docs/` → `../../../../../../docs/`). - -### 7. `backend/fly.staging.toml` - -Fly.io config for the staging app. Key differences from production (`fly.toml`): -- `app = 'trading-bootcamp-staging'` -- Separate persistent volume mount (`trading_bootcamp_staging`) -- Staging database path - -## Gotchas - -### Stale `.sqlx/` cache breaks Fly builds - -Because the Dockerfile sets `SQLX_OFFLINE=true`, the build relies entirely on the committed `.sqlx/` cache. If queries in `db.rs` change (e.g., new column like `account.color`) without regenerating the cache, Fly builds fail with cryptic errors like: - -``` -error: key must be a string at line 3 column 1 - --> src/db.rs:262:9 -``` - -This is *not* a syntax error in `db.rs` — it's SQLx failing to parse/find a matching cache file. Fix: - -```bash -cd backend -sqlx migrate run # ensure local DB matches migrations -cargo sqlx prepare -- --features dev-mode --tests # regenerate .sqlx/ -git add backend/.sqlx && git commit # commit the regenerated cache -``` - -Always include `--tests` so queries used only in tests are cached too. CLAUDE.md's "Required Checks" section mentions this but it's easy to miss — treat any SQL change in `db.rs` as requiring a cache regen before pushing. - -### Vercel deployment URL vs. production alias - -`vercel --prod` prints a line like: - -``` -Production: https://platform-staging-fgbm5rn8e-trading-bootcamp.vercel.app -``` - -That is the **immutable deployment URL** for this specific build, not a different project. The stable production alias (`https://platform-staging-five-gamma.vercel.app`) is updated to point to this deployment. Verify with `vercel project ls --scope trading-bootcamp` — the `Latest Production URL` column shows the alias. - -### Two `.vercel/` project links in the repo - -Both `/.vercel/` (repo root) and `/frontend/.vercel/` exist and point to projects named `platform-staging`, but with **different org IDs**. Running `vercel` from the repo root uses the root link, which is the correct one (the `trading-bootcamp` team's `platform-staging` project that aliases to `platform-staging-five-gamma.vercel.app`). Do not `cd frontend` before deploying. diff --git a/backend/.sqlx/query-46a626a49c0e079a63e534937feead6335e6f34afa2f9a867b6d08e671feb0d4.json b/backend/.sqlx/query-46a626a49c0e079a63e534937feead6335e6f34afa2f9a867b6d08e671feb0d4.json new file mode 100644 index 00000000..d95283ba --- /dev/null +++ b/backend/.sqlx/query-46a626a49c0e079a63e534937feead6335e6f34afa2f9a867b6d08e671feb0d4.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "\n SELECT id AS \"id!\"\n FROM account\n WHERE name = ?\n LIMIT 1\n ", + "describe": { + "columns": [ + { + "name": "id!", + "ordinal": 0, + "type_info": "Int64" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + true + ] + }, + "hash": "46a626a49c0e079a63e534937feead6335e6f34afa2f9a867b6d08e671feb0d4" +} diff --git a/backend/.sqlx/query-c4f5b6e260992d4e2df494d9ba3ad09b7ca596147544f719c4b09d6020e8a01b.json b/backend/.sqlx/query-c4f5b6e260992d4e2df494d9ba3ad09b7ca596147544f719c4b09d6020e8a01b.json deleted file mode 100644 index 055da3fd..00000000 --- a/backend/.sqlx/query-c4f5b6e260992d4e2df494d9ba3ad09b7ca596147544f719c4b09d6020e8a01b.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "db_name": "SQLite", - "query": "\n SELECT id\n FROM account\n WHERE name = ? AND (global_user_id != ? OR global_user_id IS NULL)\n ", - "describe": { - "columns": [ - { - "name": "id", - "ordinal": 0, - "type_info": "Int64" - } - ], - "parameters": { - "Right": 2 - }, - "nullable": [ - true - ] - }, - "hash": "c4f5b6e260992d4e2df494d9ba3ad09b7ca596147544f719c4b09d6020e8a01b" -} diff --git a/backend/.sqlx/query-de02634da7185857e9bbf779b3e35ecc50e5fbd111dcf036f88b636958974890.json b/backend/.sqlx/query-de02634da7185857e9bbf779b3e35ecc50e5fbd111dcf036f88b636958974890.json new file mode 100644 index 00000000..672d55f8 --- /dev/null +++ b/backend/.sqlx/query-de02634da7185857e9bbf779b3e35ecc50e5fbd111dcf036f88b636958974890.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "\n UPDATE account\n SET name = ?\n WHERE id = ?\n ", + "describe": { + "columns": [], + "parameters": { + "Right": 2 + }, + "nullable": [] + }, + "hash": "de02634da7185857e9bbf779b3e35ecc50e5fbd111dcf036f88b636958974890" +} diff --git a/backend/src/db.rs b/backend/src/db.rs index edce5571..cb9b25d0 100644 --- a/backend/src/db.rs +++ b/backend/src/db.rs @@ -944,6 +944,8 @@ impl DB { /// Ensure a user exists in this cohort DB by `global_user_id`. /// Used in multi-cohort mode where the global DB tracks the user identity. + /// On first creation, if `requested_name` is already taken in this cohort, the + /// smallest `-N` suffix (N >= 2) that's still available is appended. #[instrument(err, skip(self))] pub async fn ensure_user_created_by_global_id( &self, @@ -953,7 +955,6 @@ impl DB { ) -> SqlxResult> { let balance = Text(initial_balance); - // First try to find user by global_user_id let existing_user = sqlx::query!( r#" SELECT id AS "id!", name @@ -972,23 +973,12 @@ impl DB { })); } - // Check for name conflicts - let conflicting_account = sqlx::query!( - r#" - SELECT id - FROM account - WHERE name = ? AND (global_user_id != ? OR global_user_id IS NULL) - "#, - requested_name, - global_user_id - ) - .fetch_optional(&self.pool) - .await?; - - let final_name = if conflicting_account.is_some() { - format!("{requested_name}-g{global_user_id}") - } else { - requested_name.to_string() + let final_name = match self + .suggest_cohort_account_name(requested_name, None) + .await? + { + Ok(name) => name, + Err(failure) => return Ok(Err(failure)), }; let id = sqlx::query_scalar!( @@ -1009,6 +999,101 @@ impl DB { })) } + /// Probe `base`, then `base-2`, `base-3`, ..., up to `base-999`, returning the + /// first cohort-local `account.name` that is still available. When + /// `exclude_account_id` is `Some(id)`, rows with `account.id = id` do not count + /// as a conflict — use this so a user renaming themselves doesn't race their + /// own current row. + /// + /// # Errors + /// Returns a database error, or `NameSuffixExhausted` if `base-2..=base-999` + /// are all taken. + pub async fn suggest_cohort_account_name( + &self, + base: &str, + exclude_account_id: Option, + ) -> SqlxResult> { + for suffix in std::iter::once(0u32).chain(2..=999u32) { + let candidate = if suffix == 0 { + base.to_string() + } else { + format!("{base}-{suffix}") + }; + let row = sqlx::query_scalar!( + r#" + SELECT id AS "id!" + FROM account + WHERE name = ? + LIMIT 1 + "#, + candidate + ) + .fetch_optional(&self.pool) + .await?; + match (row, exclude_account_id) { + (None, _) => return Ok(Ok(candidate)), + (Some(id), Some(excluded)) if id == excluded => return Ok(Ok(candidate)), + _ => {} + } + } + Ok(Err(ValidationFailure::NameSuffixExhausted)) + } + + /// Look up a user account by `global_user_id`. Returns `(id, name)` of the + /// caller's cohort-local account, or `None` if they don't have one yet. + /// + /// # Errors + /// Returns a database error. + pub async fn get_user_account_by_global_user_id( + &self, + global_user_id: i64, + ) -> SqlxResult> { + let row = sqlx::query!( + r#" + SELECT id AS "id!", name + FROM account + WHERE global_user_id = ? + "#, + global_user_id + ) + .fetch_optional(&self.pool) + .await?; + Ok(row.map(|r| (r.id, r.name))) + } + + /// Rename an existing account. Returns `NameAlreadyExists` if the target name + /// would violate the cohort-local `account.name UNIQUE` constraint. + /// + /// # Errors + /// Returns a database error, or `NameAlreadyExists` via `ValidationResult`. + pub async fn rename_user_account( + &self, + account_id: i64, + new_name: &str, + ) -> SqlxResult> { + let result = sqlx::query!( + r#" + UPDATE account + SET name = ? + WHERE id = ? + "#, + new_name, + account_id, + ) + .execute(&self.pool) + .await; + + match result { + Ok(_) => Ok(Ok(())), + Err(sqlx::Error::Database(db_err)) + if db_err.message().contains("UNIQUE constraint failed") => + { + Ok(Err(ValidationFailure::NameAlreadyExists)) + } + Err(e) => Err(e), + } + } + /// # Errors /// Fails is there's a database error pub async fn get_portfolio(&self, account_id: i64) -> SqlxResult> { @@ -4544,6 +4629,7 @@ pub enum ValidationFailure { AlreadyOwner, EmptyName, NameAlreadyExists, + NameSuffixExhausted, InvalidAccountColor, InvalidOwner, OwnerInDifferentUniverse, @@ -4618,6 +4704,7 @@ impl ValidationFailure { Self::AlreadyOwner => "Already owner", Self::EmptyName => "Account name cannot be empty", Self::NameAlreadyExists => "Account name already exists", + Self::NameSuffixExhausted => "Could not find an available numeric suffix for this name", Self::InvalidAccountColor => "Account color must be a hex value like #aabbcc", Self::InvalidOwner => "Invalid owner", Self::OwnerInDifferentUniverse => "Owner must be in universe 0 or the same universe", diff --git a/backend/src/global_db.rs b/backend/src/global_db.rs index e8bca494..fe0db646 100644 --- a/backend/src/global_db.rs +++ b/backend/src/global_db.rs @@ -287,6 +287,23 @@ impl GlobalDB { .await } + /// Get a global user by primary key. + /// + /// # Errors + /// Returns an error on database failure. + pub async fn get_global_user_by_id( + &self, + id: i64, + ) -> Result, sqlx::Error> { + sqlx::query_as::<_, GlobalUser>( + r"SELECT id, kinde_id, display_name, is_admin, is_kinde_admin, admin_grant, email + FROM global_user WHERE id = ?", + ) + .bind(id) + .fetch_optional(&self.pool) + .await + } + /// Get all cohorts a user is a member of. /// /// # Errors diff --git a/backend/src/main.rs b/backend/src/main.rs index 035ed832..f671f8d2 100644 --- a/backend/src/main.rs +++ b/backend/src/main.rs @@ -8,9 +8,15 @@ use axum::{ routing::{delete, get, post, put}, Json, Router, }; -use backend::{auth::AccessClaims, global_db::CohortInfo, AppState}; +use backend::{ + auth::AccessClaims, + db::ValidationFailure, + global_db::CohortInfo, + websocket_api::{server_message::Message as SM, Accounts as WsAccounts, ServerMessage}, + AppState, CohortState, +}; use serde::{Deserialize, Serialize}; -use std::{env, path::Path, str::FromStr}; +use std::{collections::HashMap, env, path::Path, str::FromStr}; use tokio::{fs::create_dir_all, net::TcpListener}; use tower_http::{cors::CorsLayer, limit::RequestBodyLimitLayer, trace::TraceLayer}; use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; @@ -63,6 +69,7 @@ async fn main() -> anyhow::Result<()> { ) .route("/api/admin/users/:id", delete(delete_user_endpoint)) // Authenticated user endpoints + .route("/api/users/me", get(get_my_user)) .route("/api/users/me/display-name", put(update_my_display_name)) // Utility routes .route("/api/upload-image", post(upload_image)) @@ -706,9 +713,230 @@ async fn toggle_admin( } } +#[derive(Serialize)] +struct MyUserResponse { + id: i64, + display_name: String, + email: Option, + is_admin: bool, +} + +#[axum::debug_handler] +async fn get_my_user( + claims: AccessClaims, + State(state): State, +) -> Result, (StatusCode, String)> { + let user = state + .global_db + .get_global_user_by_kinde_id(&claims.sub) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .ok_or(( + StatusCode::NOT_FOUND, + "User not found; load the app first".to_string(), + ))?; + Ok(Json(MyUserResponse { + id: user.id, + display_name: user.display_name, + email: user.email, + is_admin: user.is_admin, + })) +} + #[derive(Deserialize)] struct UpdateDisplayNameRequest { display_name: String, + #[serde(default)] + confirmed_overrides: HashMap, +} + +#[derive(Serialize)] +#[allow(clippy::struct_field_names)] +struct RenameConflict { + cohort_name: String, + cohort_display_name: String, + suggested_name: String, +} + +#[derive(Serialize)] +#[serde(tag = "status", rename_all = "snake_case")] +enum RenameResponse { + Ok { display_name: String }, + NeedsConfirmation { conflicts: Vec }, +} + +struct PlannedRename { + cohort_display_name: String, + cohort_state: Arc, + account_id: i64, + target_name: String, +} + +/// Shared preflight-and-commit flow used by both the self-rename and admin-rename +/// endpoints. Updates `global_user.display_name` and fans out the new name to +/// every non-read-only cohort where the target user has an account, generating +/// `-2`, `-3`, … suffixes where a cohort's `account.name` is already taken. +/// +/// If any cohort would need a suffix that the caller hasn't confirmed yet, the +/// response is `(409, NeedsConfirmation { conflicts })` and nothing is written. +/// Caller should open a confirmation UI and re-submit with `confirmed_overrides` +/// populated from the conflicts list. +#[allow(clippy::too_many_lines)] +async fn apply_user_rename( + state: &AppState, + target_user_id: i64, + new_name: &str, + confirmed_overrides: &HashMap, +) -> Result<(StatusCode, Json), (StatusCode, String)> { + let target = state + .global_db + .get_global_user_by_id(target_user_id) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + .ok_or((StatusCode::NOT_FOUND, "User not found".to_string()))?; + + if target.display_name == new_name { + return Err(( + StatusCode::BAD_REQUEST, + "Display name unchanged".to_string(), + )); + } + + let cohorts = state + .global_db + .get_user_cohorts(target_user_id) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + + let mut conflicts: Vec = Vec::new(); + let mut planned: Vec = Vec::new(); + + for cohort_info in cohorts { + if cohort_info.is_read_only { + continue; + } + let Some(cohort_state) = state.cohorts.get(&cohort_info.name).map(|c| Arc::clone(&c)) + else { + continue; + }; + + let Some((account_id, _current_name)) = cohort_state + .db + .get_user_account_by_global_user_id(target_user_id) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + else { + continue; + }; + + let suggested = match cohort_state + .db + .suggest_cohort_account_name(new_name, Some(account_id)) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + { + Ok(name) => name, + Err(failure) => { + return Err((StatusCode::CONFLICT, failure.message().to_string())); + } + }; + + if suggested == new_name { + planned.push(PlannedRename { + cohort_display_name: cohort_info.display_name, + cohort_state, + account_id, + target_name: suggested, + }); + continue; + } + + match confirmed_overrides.get(&cohort_info.name) { + Some(confirmed) if confirmed == &suggested => { + planned.push(PlannedRename { + cohort_display_name: cohort_info.display_name, + cohort_state, + account_id, + target_name: confirmed.clone(), + }); + } + _ => { + conflicts.push(RenameConflict { + cohort_name: cohort_info.name, + cohort_display_name: cohort_info.display_name, + suggested_name: suggested, + }); + } + } + } + + if !conflicts.is_empty() { + return Ok(( + StatusCode::CONFLICT, + Json(RenameResponse::NeedsConfirmation { conflicts }), + )); + } + + // Commit: rename each cohort account first, then the global row. If a TOCTOU + // race causes a UNIQUE violation on a cohort rename, return 409 — the user + // retries and the preflight recomputes with the current state. Previously + // successful cohort renames stay in place; the next retry will see them as + // no-ops (we exclude the user's own row from the suggester). + for plan in &planned { + let result = plan + .cohort_state + .db + .rename_user_account(plan.account_id, &plan.target_name) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + match result { + Ok(()) => {} + Err(ValidationFailure::NameAlreadyExists) => { + return Err(( + StatusCode::CONFLICT, + format!( + "Race: '{}' was taken in cohort '{}' between preflight and commit. Please retry.", + plan.target_name, plan.cohort_display_name + ), + )); + } + Err(other) => { + return Err((StatusCode::INTERNAL_SERVER_ERROR, other.message().to_string())); + } + } + } + + state + .global_db + .update_user_display_name(target_user_id, new_name) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + + for plan in &planned { + let Some(account) = plan + .cohort_state + .db + .get_account(plan.account_id) + .await + .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))? + else { + continue; + }; + let msg = ServerMessage { + request_id: String::new(), + message: Some(SM::Accounts(WsAccounts { + accounts: vec![account.into()], + })), + }; + plan.cohort_state.subscriptions.send_public(msg); + } + + Ok(( + StatusCode::OK, + Json(RenameResponse::Ok { + display_name: new_name.to_string(), + }), + )) } #[axum::debug_handler] @@ -716,9 +944,9 @@ async fn update_my_display_name( claims: AccessClaims, State(state): State, Json(body): Json, -) -> Result { - let display_name = body.display_name.trim(); - if display_name.is_empty() { +) -> Result<(StatusCode, Json), (StatusCode, String)> { + let new_name = body.display_name.trim(); + if new_name.is_empty() { return Err(( StatusCode::BAD_REQUEST, "Display name cannot be empty".to_string(), @@ -738,13 +966,7 @@ async fn update_my_display_name( "User not found; load the app before updating your display name".to_string(), ))?; - state - .global_db - .update_user_display_name(global_user.id, display_name) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - - Ok(StatusCode::OK) + apply_user_rename(&state, global_user.id, new_name, &body.confirmed_overrides).await } #[axum::debug_handler] @@ -753,21 +975,16 @@ async fn admin_update_display_name( AxumPath(user_id): AxumPath, State(state): State, Json(body): Json, -) -> Result { +) -> Result<(StatusCode, Json), (StatusCode, String)> { check_admin(&state, &claims).await?; - let display_name = body.display_name.trim(); - if display_name.is_empty() { + let new_name = body.display_name.trim(); + if new_name.is_empty() { return Err(( StatusCode::BAD_REQUEST, "Display name cannot be empty".to_string(), )); } - state - .global_db - .update_user_display_name(user_id, display_name) - .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; - Ok(StatusCode::OK) + apply_user_rename(&state, user_id, new_name, &body.confirmed_overrides).await } #[axum::debug_handler] diff --git a/frontend/src/lib/adminApi.ts b/frontend/src/lib/adminApi.ts index 864a4384..a07bd658 100644 --- a/frontend/src/lib/adminApi.ts +++ b/frontend/src/lib/adminApi.ts @@ -162,16 +162,72 @@ export async function toggleAdmin(userId: number, isAdmin: boolean): Promise { - const res = await fetch(`${API_BASE}/api/admin/users/${userId}/display-name`, { +export interface RenameConflict { + cohort_name: string; + cohort_display_name: string; + suggested_name: string; +} + +export type RenameResult = + | { status: 'ok'; display_name: string } + | { status: 'needs_confirmation'; conflicts: RenameConflict[] }; + +export interface MyUser { + id: number; + display_name: string; + email: string | null; + is_admin: boolean; +} + +export async function fetchMyUser(): Promise { + const res = await fetch(`${API_BASE}/api/users/me`, { + headers: await authHeaders() + }); + if (!res.ok) { + const text = await res.text(); + throw new Error(text || res.statusText); + } + return res.json(); +} + +async function submitDisplayName( + url: string, + displayName: string, + confirmedOverrides: Record +): Promise { + const res = await fetch(url, { method: 'PUT', headers: await authHeaders(), - body: JSON.stringify({ display_name: displayName }) + body: JSON.stringify({ + display_name: displayName, + confirmed_overrides: confirmedOverrides + }) }); + if (res.status === 409) { + const body = await res.json(); + if (body?.status === 'needs_confirmation') { + return body as RenameResult; + } + // Non-confirmation 409 (e.g. TOCTOU race) — bubble up as an error. + throw new Error(typeof body === 'string' ? body : (body?.message ?? res.statusText)); + } if (!res.ok) { const text = await res.text(); throw new Error(text || res.statusText); } + return res.json(); +} + +export async function updateDisplayName( + userId: number, + displayName: string, + confirmedOverrides: Record = {} +): Promise { + return submitDisplayName( + `${API_BASE}/api/admin/users/${userId}/display-name`, + displayName, + confirmedOverrides + ); } export async function deleteUser(userId: number): Promise { @@ -185,16 +241,15 @@ export async function deleteUser(userId: number): Promise { } } -export async function updateMyDisplayName(displayName: string): Promise { - const res = await fetch(`${API_BASE}/api/users/me/display-name`, { - method: 'PUT', - headers: await authHeaders(), - body: JSON.stringify({ display_name: displayName }) - }); - if (!res.ok) { - const text = await res.text(); - throw new Error(text || res.statusText); - } +export async function updateMyDisplayName( + displayName: string, + confirmedOverrides: Record = {} +): Promise { + return submitDisplayName( + `${API_BASE}/api/users/me/display-name`, + displayName, + confirmedOverrides + ); } export async function updateMemberInitialBalance( diff --git a/frontend/src/lib/api.svelte.ts b/frontend/src/lib/api.svelte.ts index 0dd8e965..fb6a1dda 100644 --- a/frontend/src/lib/api.svelte.ts +++ b/frontend/src/lib/api.svelte.ts @@ -363,7 +363,10 @@ const handleMessage = (event: MessageEvent) => { } if (msg.accounts) { - serverState.accounts.clear(); + // Upsert, don't clear. The backend sends this message as an initial bulk + // snapshot on connect AND as a single-entry rename broadcast; both cases + // should merge into existing state. Full state resets happen through + // `resetServerState()` on disconnect. for (const account of msg.accounts.accounts || []) { serverState.accounts.set(account.id, account); if (account.name === 'Arbor Pixie') { diff --git a/frontend/src/routes/[cohort_name]/accounts/+page.svelte b/frontend/src/routes/[cohort_name]/accounts/+page.svelte index 475b58ee..c76a650f 100644 --- a/frontend/src/routes/[cohort_name]/accounts/+page.svelte +++ b/frontend/src/routes/[cohort_name]/accounts/+page.svelte @@ -11,10 +11,18 @@ import ShareOwnership from '$lib/components/forms/shareOwnership.svelte'; import { Button } from '$lib/components/ui/button'; import * as Select from '$lib/components/ui/select'; - import { updateMyDisplayName } from '$lib/adminApi'; + import * as AlertDialog from '$lib/components/ui/alert-dialog'; + import { + updateMyDisplayName, + fetchMyUser, + type RenameConflict, + type MyUser + } from '$lib/adminApi'; import { Check, Copy, Pencil, X } from '@lucide/svelte/icons'; import { toast } from 'svelte-sonner'; import { universeMode } from '$lib/universeMode.svelte'; + import { onMount } from 'svelte'; + import { page } from '$app/stores'; let token = $state(undefined); kinde.getToken().then((t) => (token = t)); @@ -74,26 +82,71 @@ document.getElementById('create-account-section')?.scrollIntoView({ behavior: 'smooth' }); } + let myUser = $state(null); let editingDisplayName = $state(false); let newDisplayName = $state(''); + let pendingConflicts = $state(null); + let pendingName = $state(''); + let submitting = $state(false); + + onMount(async () => { + try { + myUser = await fetchMyUser(); + } catch (e) { + console.warn('Failed to fetch current user', e); + } + }); + + let cohortAccountName = $derived(accountName(serverState.userId)); + let globalName = $derived(myUser?.display_name ?? cohortAccountName); + let nameDiffersInCohort = $derived(myUser !== null && cohortAccountName !== globalName); + let currentCohortLabel = $derived($page.params.cohort_name ?? 'this cohort'); function startEditingName() { editingDisplayName = true; - newDisplayName = accountName(serverState.userId) ?? ''; + newDisplayName = globalName ?? ''; } - async function saveDisplayName() { - if (!newDisplayName.trim()) return; + function cancelEditing() { + editingDisplayName = false; + pendingConflicts = null; + pendingName = ''; + } + + async function submitRename(name: string, overrides: Record) { + submitting = true; try { - await updateMyDisplayName(newDisplayName.trim()); - // Update the account name in local state - const account = serverState.accounts.get(serverState.userId ?? 0); - if (account) account.name = newDisplayName.trim(); - toast.success('Display name updated'); - editingDisplayName = false; + const result = await updateMyDisplayName(name, overrides); + if (result.status === 'ok') { + if (myUser) myUser = { ...myUser, display_name: result.display_name }; + toast.success('Display name updated'); + editingDisplayName = false; + pendingConflicts = null; + pendingName = ''; + } else { + pendingConflicts = result.conflicts; + pendingName = name; + } } catch (e) { toast.error('Failed to update: ' + (e instanceof Error ? e.message : String(e))); + } finally { + submitting = false; + } + } + + async function saveDisplayName() { + const trimmed = newDisplayName.trim(); + if (!trimmed) return; + await submitRename(trimmed, {}); + } + + async function confirmSuggestedRename() { + if (!pendingConflicts) return; + const overrides: Record = {}; + for (const c of pendingConflicts) { + overrides[c.cohort_name] = c.suggested_name; } + await submitRename(pendingName, overrides); } @@ -108,29 +161,42 @@

Accounts

+ {#if nameDiffersInCohort} +
+ Name for {currentCohortLabel}: + {cohortAccountName} +
+ {/if}
- Display name: + Name: {#if editingDisplayName} { if (e.key === 'Enter') saveDisplayName(); - if (e.key === 'Escape') editingDisplayName = false; + if (e.key === 'Escape') cancelEditing(); }} /> - {:else} - {accountName(serverState.userId)} + {globalName}
+ + { + if (!open) { + pendingConflicts = null; + pendingName = ''; + } + }} +> + + + Name already taken in some cohorts + + Your new name "{pendingName}" is already taken in some cohorts + you're a member of. You'll be known as: + + + {#if pendingConflicts} +
    + {#each pendingConflicts as conflict (conflict.cohort_name)} +
  • + {conflict.cohort_display_name}: + {conflict.suggested_name} +
    • + {/each} +
+ {/if} + + { + pendingConflicts = null; + pendingName = ''; + }} + disabled={submitting} + > + Cancel + + + Confirm + + + + diff --git a/frontend/src/routes/admin/+page.svelte b/frontend/src/routes/admin/+page.svelte index ee6424ed..e42ff313 100644 --- a/frontend/src/routes/admin/+page.svelte +++ b/frontend/src/routes/admin/+page.svelte @@ -7,6 +7,7 @@ updateConfig, toggleAdmin, updateDisplayName, + type RenameConflict, deleteUser, type CohortInfo, type GlobalConfig, @@ -14,6 +15,7 @@ } from '$lib/adminApi'; import { browser } from '$app/environment'; import { onMount } from 'svelte'; + import * as AlertDialog from '$lib/components/ui/alert-dialog'; import ChevronRight from '@lucide/svelte/icons/chevron-right'; import Pencil from '@lucide/svelte/icons/pencil'; import Trash2 from '@lucide/svelte/icons/trash-2'; @@ -59,6 +61,11 @@ // Editing state let editingUserId = $state(null); let editingName = $state(''); + let renameConflict = $state<{ + userId: number; + name: string; + conflicts: RenameConflict[]; + } | null>(null); // Confirm modal state let confirmModal = $state<{ @@ -126,6 +133,34 @@ editingName = user.display_name; } + async function submitAdminRename( + userId: number, + name: string, + overrides: Record + ) { + try { + const result = await updateDisplayName(userId, name, overrides); + if (result.status === 'ok') { + const user = allUsers.find((u) => u.id === userId); + if (user) user.display_name = result.display_name; + allUsers = [...allUsers]; + toast.success('Display name updated'); + renameConflict = null; + editingUserId = null; + } else { + renameConflict = { + userId, + name, + conflicts: result.conflicts + }; + } + } catch (e) { + toast.error('Failed to update: ' + (e instanceof Error ? e.message : String(e))); + renameConflict = null; + editingUserId = null; + } + } + async function saveEditingName() { if (editingUserId == null) return; const trimmed = editingName.trim(); @@ -133,16 +168,16 @@ toast.error('Display name cannot be empty'); return; } - try { - await updateDisplayName(editingUserId, trimmed); - const user = allUsers.find((u) => u.id === editingUserId); - if (user) user.display_name = trimmed; - allUsers = [...allUsers]; - toast.success('Display name updated'); - } catch (e) { - toast.error('Failed to update: ' + (e instanceof Error ? e.message : String(e))); + await submitAdminRename(editingUserId, trimmed, {}); + } + + async function confirmAdminRename() { + if (!renameConflict) return; + const overrides: Record = {}; + for (const c of renameConflict.conflicts) { + overrides[c.cohort_name] = c.suggested_name; } - editingUserId = null; + await submitAdminRename(renameConflict.userId, renameConflict.name, overrides); } function confirmToggleAdmin(user: UserWithCohorts) { @@ -481,3 +516,44 @@
{/if} + + { + if (!open) { + renameConflict = null; + editingUserId = null; + } + }} +> + + + Name already taken in some cohorts + + The new name "{renameConflict?.name}" is already taken in + some cohorts this user is a member of. They'll be known as: + + + {#if renameConflict} +
    + {#each renameConflict.conflicts as conflict (conflict.cohort_name)} +
  • + {conflict.cohort_display_name}: + {conflict.suggested_name} +
    • + {/each} +
+ {/if} + + { + renameConflict = null; + editingUserId = null; + }} + > + Cancel + + Confirm + + +