feat: User preferences storage & admin: sorted billing plans by populality #3266
Conversation
WalkthroughAdds per-user JSON storage to UserPreferences with DB migration, service method, GET/PUT API endpoints and tests; webapp adds a hook to consume these endpoints and updates plan selectors to use and update user-preferred plan counts and internal loading. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Webapp Hook / UI
participant API as UserPreferencesController
participant SVC as UserPreferencesService
participant REPO as UserPreferencesRepository
participant DB as DB (user_preferences.storage_json)
rect rgba(240,248,255,1)
note left of UI: Read field
UI->>API: GET /v2/user-preferences/storage/{fieldName}
API->>SVC: findOrCreate(user)
SVC->>REPO: fetch prefs
REPO-->>SVC: UserPreferences(storageJson)
SVC-->>API: return value or null
API-->>UI: 200 UserStorageResponse{ data }
end
rect rgba(245,255,250,1)
note left of UI: Write field
UI->>API: PUT /v2/user-preferences/storage/{fieldName} { data }
API->>SVC: setStorageJsonField(fieldName, data, user)
SVC->>REPO: fetch or create prefs
SVC->>REPO: save updated storageJson (set/remove key)
REPO-->>SVC: persisted
SVC-->>API: done
API-->>UI: 200
end
sequenceDiagram
autonumber
participant User as Admin UI
participant GPS as GenericPlanSelector
participant Pref as useUserPreferenceStorage
participant API as /v2/user-preferences/storage/{fieldName}
participant Sort as useSortPlans
User->>GPS: Open selector
GPS->>Pref: GET preferred counts
Pref->>API: GET storage/billingAdminPreferredPlans
API-->>Pref: { data: { planId: count, ... } }
Pref-->>GPS: loadable with data
GPS->>Sort: sort(plans, counts)
Sort-->>GPS: sorted plans
User->>GPS: Select plan X
GPS->>Pref: update(increment count for X)
Pref->>API: PUT storage/billingAdminPreferredPlans { data: updatedCounts }
API-->>Pref: 200
GPS-->>User: onPlanChange(selected plan)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
webapp/src/service/apiSchema.generated.ts (1)
5975-5978: Consider makingUserStorageResponse.datarequired.200-success implies the field exists; optional
datablurs “missing” vs “explicit null”. Preferdata: unknownand use 404 for missing.Apply at the OpenAPI source so generation yields:
- UserStorageResponse: { - /** @description The data stored for the field */ - data?: unknown; - }; + UserStorageResponse: { + /** @description The data stored for the field */ + data: unknown; + };backend/data/src/main/kotlin/io/tolgee/model/UserPreferences.kt (1)
41-47: Make column mapping explicit and prefer null default.Avoid relying on naming strategies; map explicitly and default to null to prevent writing
{}unintentionally.- @Type(JsonBinaryType::class) - @Column(columnDefinition = "jsonb") - var storageJson: Map<String, Any>? = mutableMapOf() + @Type(JsonBinaryType::class) + @Column(name = "storage_json", columnDefinition = "jsonb") + var storageJson: Map<String, Any>? = nullbackend/data/src/main/kotlin/io/tolgee/dtos/request/UserStorageResponse.kt (1)
1-10: Move DTO to a response package for clarity.This is a response DTO but lives under dtos.request. Consider relocating.
-package io.tolgee.dtos.request +package io.tolgee.dtos.responseFollow-up: update imports (controller, any generated schema references).
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt (2)
65-71: Response semantics for missing field.Currently returns
{ data: null }when the key is absent. If desired, consider 404 or 204 to signal absence more explicitly; otherwise OK as-is.
73-84: Consider 204 No Content on update.PUT currently returns 200 with empty body. Returning 204 would better reflect no response content.
webapp/src/hooks/useUserPreferenceStorage.ts (1)
4-27: Generalize typing and auto-invalidate/refetch after update.Current cast fixes the type to Record<number, number>, which breaks reuse. Also, mutation doesn’t invalidate/refetch, so UI may stay stale.
-export function useUserPreferenceStorage(fieldName: string) { +export function useUserPreferenceStorage<T = unknown>(fieldName: string) { const loadable = useApiQuery({ url: '/v2/user-preferences/storage/{fieldName}', method: 'get', path: { fieldName }, - }) as UseQueryResult<{ data: Record<number, number> }>; + }) as UseQueryResult<{ data: T }>; - const mutation = useApiMutation({ + const mutation = useApiMutation({ url: '/v2/user-preferences/storage/{fieldName}', method: 'put', + invalidatePrefix: '/v2/user-preferences/storage', + options: { + onSuccess: () => loadable.refetch(), + }, }); return { loadable, - update: (value: Record<string, any>) => { + update: (value: T) => { mutation.mutate({ path: { fieldName }, content: { 'application/json': value, }, }); }, }; }Note: For your current use case, set
T = Record<string, number>.backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserPreferencesControllerTest.kt (1)
47-150: Good test coverage, but consider adding edge case tests.The test suite provides solid coverage for the storage JSON functionality, including basic operations, field isolation, overwrites, and different data types. However, consider adding tests for the following edge cases to improve robustness:
- Null value handling: Test storing and retrieving explicit null values vs. non-existent fields.
- Field name validation: Test special characters, very long field names, empty field names, or field names with path-like patterns (e.g.,
field/subfield).- Large payload handling: Test storing large values to ensure the JSON storage can handle them without performance degradation.
- Concurrent updates: Test race conditions when multiple requests update the same field simultaneously.
- Invalid data types: Test edge cases like extremely large numbers, deeply nested objects, or circular references.
Would you like me to generate sample tests for these edge cases?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt(3 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserPreferencesControllerTest.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/dtos/request/UserStorageResponse.kt(1 hunks)backend/data/src/main/kotlin/io/tolgee/model/UserPreferences.kt(3 hunks)backend/data/src/main/kotlin/io/tolgee/service/security/UserPreferencesService.kt(1 hunks)backend/data/src/main/resources/db/changelog/schema.xml(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx(2 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx(5 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx(2 hunks)webapp/src/hooks/useUserPreferenceStorage.ts(1 hunks)webapp/src/service/apiSchema.generated.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx (1)
webapp/src/hooks/useUserPreferenceStorage.ts (1)
useUserPreferenceStorage(4-27)
webapp/src/hooks/useUserPreferenceStorage.ts (1)
webapp/src/service/http/useQueryApi.ts (2)
useApiQuery(119-141)useApiMutation(224-265)
webapp/src/service/apiSchema.generated.ts (1)
webapp/src/service/billingApiSchema.generated.ts (2)
operations(1466-4390)components(218-1464)
🪛 Biome (2.1.2)
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx
[error] 51-51: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 52-52: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 86-86: This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.
This is the call path until the hook.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 88-88: This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.
This is the call path until the hook.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 101-101: This hook is being called indirectly and conditionally, but all hooks must be called in the exact same order in every component render.
This is the call path until the hook.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
🔇 Additional comments (4)
webapp/src/service/apiSchema.generated.ts (2)
1022-1025: New storage field endpoints wired correctly.Route and op bindings look consistent with existing patterns.
20267-20305: GET response typing looks good.Returns
UserStorageResponsewith standard error shapes.webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx (1)
8-16: LGTM — derive loading internally.Props surface is cleaner; passing plans and computed loading to GenericPlanSelector looks correct.
Also applies to: 25-30
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx (1)
11-25: LGTM! Clean refactor.The component now derives and manages its own loading state internally from the query hook, making it more self-contained and removing the need to propagate loading externally. This aligns with standard react-query patterns and improves the component's API surface.
backend/data/src/main/kotlin/io/tolgee/service/security/UserPreferencesService.kt
Show resolved
Hide resolved
...g/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx
Show resolved
Hide resolved
...g/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx
Show resolved
Hide resolved
JanCizmar
force-pushed
the
jancizmar/fe-storage-and-billing-plans-sorting
branch
from
ef736f8 to
7d1cb59
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt (1)
65-84: Clarify auth requirements and schema symmetry for storage endpointsThe basic flow (load/create preferences, read/update
storageJson, delegate to service) looks fine, but a few design points are worth tightening:
Auth / bypass behavior:
get()andsetLanguage()are annotated with@BypassEmailVerificationand@BypassForcedSsoAuthentication, while the new storage endpoints are not. If this storage is meant for general UI preferences similarly used before full verification/SSO, consider aligning the annotations to avoid surprising auth differences.Request vs response shape:
getStorageFieldwraps the value inUserStorageResponse, butsetStorageFieldaccepts a rawAny?body. For a clearer and more stable API (including generated OpenAPI), it may be better to accept a small request DTO mirroring the response (e.g.{ "value": ... }), or otherwise document the expected payload shape explicitly.Missing vs null: Right now a missing key and an explicitly stored
nullboth serialize the same way. If the frontend needs to distinguish these cases, you might extendUserStorageResponse(e.g. with anexistsflag) or use a sentinel convention.These are non-blocking, but clarifying them now will make the API easier to consume and evolve.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt(3 hunks)backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserPreferencesControllerTest.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserPreferencesControllerTest.kt
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx,kt,kts}
⚙️ CodeRabbit configuration file
As part of review, please check if the file follows 'The Stepdown Rule': The most important, high-level concepts should be at the top (like a newspaper headline and opening paragraph). Details should increase as you read downward. Functions should be ordered so that a caller appears before the functions it calls. When working with JavaScript components, we allow the main component to live at the bottom of the file as an exception to the rule.
Files:
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt
🧠 Learnings (1)
📚 Learning: 2025-10-07T14:36:39.331Z
Learnt from: Anty0
Repo: tolgee/tolgee-platform PR: 3263
File: backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt:279-290
Timestamp: 2025-10-07T14:36:39.331Z
Learning: In OrganizationRoleService in backend/data/src/main/kotlin/io/tolgee/service/organization/OrganizationRoleService.kt, the `removeOrDeactivateUser` method intentionally has two distinct paths: if a user can be removed, it calls `removeUserForReal` (which removes organization roles, project permissions, and evicts caches); if a user is managed by the organization and cannot be removed, it calls `userAccountService.disable(userId)` without any cleanup. This design is intentional—disabling should be reversible so the user stays unmodified (keeping their organization roles and project permissions intact), allowing them to be re-enabled if they return to the organization.
Applied to files:
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build backend 🏗️
- GitHub Check: Frontend static check 🪲
🔇 Additional comments (1)
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt (1)
9-21: Imports for storage DTO and@RequestBodylook correctThe new imports are minimal and match the new endpoint usages; no issues here.
| val preferences = findOrCreate(userAccount.id) | ||
| val currentStorage = preferences.storageJson?.toMutableMap() ?: mutableMapOf() | ||
| if (value != null) { | ||
| currentStorage[fieldName] = value | ||
| } else { | ||
| currentStorage.remove(fieldName) | ||
| } | ||
| preferences.storageJson = currentStorage | ||
| userPreferencesRepository.save(preferences) |
There was a problem hiding this comment.
This feels like it's going to race if two updates happen at the same time. But I suppose it's fine for non-essential frontend data.
Marketa hated that she had to scroll trough many billing plans to pick the one she uses all the time. It was time consuming for her.
I added the storage field which can be operated from frontend. We can store any data required for proper displaying of data on the frontend.
useUserPreferenceStorageso it's easy to get and update the data in the storage.Summary by CodeRabbit
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.