feat(user): persist per-user UI preferences on the User entity (starting with appMode) - #31030
feat(user): persist per-user UI preferences on the User entity (starting with appMode)#31030chirag-madlani wants to merge 10 commits into
Conversation
Adds a `preferences` property to the User JSON schema as an opaque,
server-side Map<String, Object> bag (existingJavaType pins it to
java.util.Map<java.lang.String, java.lang.Object> instead of letting
jsonschema2pojo generate a wrapper POJO). Default is {} so downstream
code can iterate without null checks.
Adds UserResourceIT#patch_preferences_add_appMode_returnsIt, which
PATCHes preferences.appMode via JSON-Patch and reads it back with
?fields=preferences.
No repository/resource-level validation or field-gating is added here
(opaque by design) — that is scoped to a follow-up task.
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
✅ TypeScript Types Auto-UpdatedThe generated TypeScript types have been automatically updated based on JSON schema changes in this PR. |
|
| Count | Rule |
|---|---|
| 9 | react-hooks/exhaustive-deps |
| 7 | @typescript-eslint/no-explicit-any |
| 3 | sonarjs/no-nested-functions |
| 2 | sonarjs/cyclomatic-complexity |
| 1 | sonarjs/no-duplicate-string |
All findings
| Location | Rule | Message | |
|---|---|---|---|
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:188:9 |
react-hooks/exhaustive-deps |
The 'onLoginHandler' function makes the dependencies of useMemo Hook (at line 842) change on every render. Move it inside the useMemo callback. Alternatively, w |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:267:6 |
react-hooks/exhaustive-deps |
React Hook useCallback has missing dependencies: 'navigate', 'setApplicationLoading', 'setCurrentUser', and 'setIsAuthenticated'. Either include them or remove |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:325:9 |
react-hooks/exhaustive-deps |
The 'resetUserDetails' function makes the dependencies of useMemo Hook (at line 842) change on every render. To fix this, wrap the definition of 'resetUserDetai |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:411:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'startTokenExpiryTimer'. Either include it or remove the dependency array. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:454:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has a missing dependency: 'startTokenExpiryTimer'. Either include it or remove the dependency array. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:464:9 |
react-hooks/exhaustive-deps |
The 'handleFailedLogin' function makes the dependencies of useMemo Hook (at line 842) change on every render. Move it inside the useMemo callback. Alternatively |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:526:5 |
react-hooks/exhaustive-deps |
React Hook useCallback has missing dependencies: 'authConfig?.provider', 'handledVerifiedUser', 'navigate', 'resetUserDetails', and 'startTokenExpiryTimer'. Eit |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:571:9 |
react-hooks/exhaustive-deps |
The 'initializeAxiosInterceptors' function makes the dependencies of useMemo Hook (at line 842) change on every render. To fix this, wrap the definition of 'ini |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:647:65 |
sonarjs/no-nested-functions |
Refactor this code to not nest functions more than 4 levels deep. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:657:39 |
sonarjs/no-nested-functions |
Refactor this code to not nest functions more than 4 levels deep. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:672:41 |
sonarjs/no-nested-functions |
Refactor this code to not nest functions more than 4 levels deep. |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:742:30 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 17 which is greater than 10 authorized.","cost":7,"secondaryLocations":[{"line":742,"column":29,"endLine":742,"endColum |
| 🟡 | src/components/Auth/AuthProviders/AuthProvider.tsx:831:6 |
react-hooks/exhaustive-deps |
React Hook useEffect has missing dependencies: 'cleanup', 'fetchAuthConfig', 'initializeAxiosInterceptors', and 'startTokenExpiryTimer'. Either include them or |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:63:10 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:95:12 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:138:14 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:164:14 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:177:16 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:201:14 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:245:14 |
@typescript-eslint/no-explicit-any |
Unexpected any. Specify a different type. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.test.ts:314:28 |
sonarjs/no-duplicate-string |
Define a constant instead of duplicating this literal 5 times. |
| 🟡 | src/hooks/currentUserStore/useCurrentUserStore.ts:204:17 |
sonarjs/cyclomatic-complexity |
{"message":"Function has a complexity of 11 which is greater than 10 authorized.","cost":1,"secondaryLocations":[{"line":204,"column":16,"endLine":204,"endColum |
Fix locally (fast - only checks files changed in this branch):
make ui-checkstyle-changed…users can PATCH The prepare() init runs only on create and *after* JSON-Patch is applied during updates. For users that already exist in the DB before this field ships, their stored JSON has no `preferences` key, so the fetched entity serializes without it — and the UI's first PATCH `add /preferences/appMode` throws "contains no mapping for the name 'preferences'" because the parent object is missing. Move the null-init onto setFields (the fetch path) so the container is always present before any patch is applied; the prepare() init stays as belt-and-suspenders for freshly created users. Reported by Gitar review on PR #31030. 3/3 patch_preferences_* IT tests still pass locally. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
|
mohityadav766
left a comment
There was a problem hiding this comment.
This needs more work, we should have another table user_preferences.
Where each user can have various types of preferences.
|
Closing in favor of the v2 architecture per reviewer feedback (Pablo + Mohit). Per-user preference storage will move from a field on the User entity to a dedicated New combined PR forthcoming — will link when opened. Branch |
Pull request was closed
Code Review ✅ Approved 4 resolved / 4 findingsAdds backend persistence for per-user UI preferences like appMode with a 300ms debounced sync, addressing logout state leaks, PATCH response handling, and preference initialization issues. ✅ 4 resolved✅ Bug: resetBackendSyncState never called on logout — module state leaks across sessions
✅ Bug: serverKnown not updated after field-gated PATCH response omits preferences
✅ Edge Case: In-flight PATCH failure can roll back a newer optimistic write
✅ Bug: preferences default-init in prepare() runs too late for existing users
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |



Summary
Persist the user's app-mode preference (AI vs Classic) — and any future opaque per-user UI preference — on the backend
Userentity, so the choice follows the user across devices, browsers, and profiles instead of only living in localStorage.What changed
Schema —
openmetadata-spec/.../teams/user.jsonpreferences: Map<String, Object>field, opaque to the server, stored inside the existinguser_entity.jsonJSON blob. No DB migration.existingJavaTypedirective ensures the Java shape isMap<String, Object>rather than a generated wrapper POJO.Backend —
openmetadata-serviceUserRepository.clearFieldsgatespreferencesbehindfields=preferences. The field is serialised only when explicitly requested — never in list responses, and never in other users' GETs.UserResource.getCurrentLoggedInUserforcespreferencesinto its fetched field set viaEntityUtil.addField, so the UI can hydrate on bootstrap without every caller needing to remember the query param.FIELDSexample updated.allowedFieldsis auto-derived from@JsonPropertyOrderon generatedUser.java, so no manual allow-list edit was needed.UI —
openmetadata-ui/.../hooks/currentUserStore/useCurrentUserStore.tsBACKEND_SYNCED_KEYS = new Set(['appMode'])— the whitelist of preference keys that also live on the backend. All otherUserPreferencesfields remain purely local (persisted by the existingpersistmiddleware, unchanged).hydrateBackendSyncedPreferences(user)is called fromAuthProviderright aftergetLoggedInUser()resolves (both the returning-session bootstrap and the interactive-login paths). It:addup to the backend.setPreference(patch)— the consumer-facing hook — is unchanged. Under the hood, writes to whitelisted keys additionally enqueue a 300ms-debouncedupdateUserDetail(userId, ops)JSON-Patch:null→remove, key not previously on server →add, otherwise →replace. Coalesces rapid writes into one PATCH.showErrorToastfires. No silent divergence between client and server.beforeunloadflushes any pending debounced patch best-effort.persistmiddleware is not removed — sidebar collapse state, recently-viewed, etc. still live on localStorage as before.Types
openmetadata-ui/.../generated/entity/teams/user.tsregenerated with the newpreferences?field.Testing
Java IT tests (
openmetadata-integration-tests/.../UserResourceIT.java) — 4 tests added:patch_preferences_add_appMode_returnsIt— JSON-Patchaddround-trip.get_user_without_fields_omitsPreferences— field-gating works both directions (withoutfields=preferences, the response omits it; with it, the response carries it).patch_preferences_replaceOp_persists—replaceop semantics.patch_preferences_removeOp_clearsKey—removeop semantics.list_users_omitsPreferencesForAllEntries— list-response omission across a page.These tests were drafted but not executed locally — they need Docker/MySQL/ES containers that weren't available in the authoring environment. CI on this PR is the first real run.
UI Jest tests (
useCurrentUserStore.test.ts) — 9 new tests, all passing locally:appModewhen server has a value (server wins).setPreference({ appMode })fires a debounced PATCH.appMode: nullemits aremoveop.appModemigrates to backend when the server has none.Full store test suite: 15/15 passing. Pre-existing
AuthProvider.test.tsx(12 tests) still passing after wiring the bootstrap call.Design notes / decisions
preferencesis opaque server-side. No validation of individual keys or values — the schema/enum lives in the UI. This keeps future preferences (likeconnectionsViewMode, or Collate-only keys) landable without schema changes.appModeis backend-synced in this PR. OtherUserPreferencesfields (sidebar collapse state, recently viewed, marketplace history, etc.) intentionally stay per-device. A future PR can categorize each and expandBACKEND_SYNCED_KEYS.fields=preferences,GET /users/{id}and list responses omit the field entirely. Only the loggedInUser endpoint opts in by default.existingJavaTypeon the schema preventsjsonschema2pojofrom generating a wrapperPreferencesPOJO with@JsonAnyGetter/@JsonAnySetter, which would have made the Java handling inconsistent with the "opaque bag" design.Follow-ups (out of scope for this PR)
UserPreferencesfields and expand the backend-synced whitelist (Collate'sAppModeSwitchershippedappModefirst, other fields will follow if they cross device).preferencesversioning if the shape ever needs breaking changes on the wire.Test plan for reviewers
/v1/users/{id}fires within ~300ms withop: add|replace|removeon/preferences/appMode.getLoggedInUserreturnspreferences.appMode; AppModeSwitcher reflects the choice.GET /v1/users/{id}(withoutfields=preferences) returnspreferences: null.GET /v1/userslist returns every user withpreferences: null.🤖 Generated with Claude Code