Skip to content

Conversation

JanCizmar
Copy link
Contributor

@JanCizmar JanCizmar commented Oct 7, 2025

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.

  • I have also created a useUserPreferenceStorage so it's easy to get and update the data in the storage.

Summary by CodeRabbit

  • New Features

    • Introduced per-user preference storage to persist custom app data.
    • Plan selectors now remember and prioritize your most-used plans, with choices influencing future ordering.
    • Preferences persist across sessions for a personalized experience.
  • Improvements

    • Plan selection views show a loading state while data is fetched.
  • Tests

    • Added comprehensive tests covering storing, retrieving, updating, and removing user preference fields, including multiple data types and edge cases.

@JanCizmar JanCizmar requested a review from Anty0 October 7, 2025 10:16
Copy link
Contributor

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds per-user JSON storage to UserPreferences with GET/PUT endpoints, DTO, service logic, DB migration, and tests. Webapp adds a hook to consume these endpoints and updates plan selectors: GenericPlanSelector gains a loading prop, sorts by user-preferred counts, and increments preferences on selection; wrapper selectors now derive loading internally.

Changes

Cohort / File(s) Summary
Backend API: User storage endpoints
backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt
Adds GET /storage/{fieldName} returning UserStorageResponse and PUT /storage/{fieldName} to update/remove a field for the authenticated user.
Backend DTO
backend/data/src/main/kotlin/io/tolgee/dtos/request/UserStorageResponse.kt
Adds UserStorageResponse DTO with nullable data: Any? and JSON/swagger annotations.
Backend Model & Migration
backend/data/src/main/kotlin/io/tolgee/model/UserPreferences.kt, backend/data/src/main/resources/db/changelog/schema.xml
Adds storageJson mapped to jsonb column storage_json on user_preferences; Liquibase changeset adding the column.
Backend Service
backend/data/src/main/kotlin/io/tolgee/service/security/UserPreferencesService.kt
Adds setStorageJsonField(fieldName: String, value: Any?, userAccount: UserAccount) to set/remove entries in storageJson and persist preferences.
Backend Tests
backend/app/src/test/kotlin/io/tolgee/api/v2/controllers/UserPreferencesControllerTest.kt
Adds tests covering storing, overwriting, preserving, empty values, null for absent fields, type handling, and retrieval via new endpoints.
Webapp Hook
webapp/src/hooks/useUserPreferenceStorage.ts
New hook useUserPreferenceStorage(fieldName) performing GET /v2/user-preferences/storage/{fieldName} and PUT to update the field.
Webapp Generic Plan Selector
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx
Adds optional loading prop and loading UI; sorts plans using user-preferred counts (via useUserPreferenceStorage) and increments preference counts on selection.
Webapp Cloud/Self-hosted Selectors
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx, webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx
Update prop types to omit loading; components derive loading internally and forward it to GenericPlanSelector.
API Schema (generated)
webapp/src/service/apiSchema.generated.ts
Adds /v2/user-preferences/storage/{fieldName} GET/PUT operations, new UserStorageResponse schema, and operation signatures for getStorageField and setStorageField.

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 rgb(240,248,255)
    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: value = storageJson[fieldName] or null
    API-->>UI: 200 UserStorageResponse{ data }
  end

  rect rgb(245,255,250)
    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->>DB: update storage_json (set or remove key)
    DB-->>SVC: persisted
    SVC-->>API: done
    API-->>UI: 200
  end
Loading
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)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

I’m a rabbit who stores bits with care,
Hopping through JSON, nibbling keys in the air.
GET fetches my treats, PUT tucks them away,
Plans hop to the top where my favorites stay.
A tiny hop, a clever tweak — burrows brighter today. 🐰✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 42.86% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately summarizes the two primary changes in the pull request: adding user preference storage and sorting billing plans by popularity, matching the implemented endpoints, hook, and UI updates. Although it covers both aspects concisely, it directly reflects the intent and scope of the changeset.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jancizmar/fe-storage-and-billing-plans-sorting

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba1e0d and ef736f8.

📒 Files selected for processing (3)
  • backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt (3 hunks)
  • webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx (5 hunks)
  • webapp/src/hooks/useUserPreferenceStorage.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • backend/api/src/main/kotlin/io/tolgee/api/v2/controllers/UserPreferencesController.kt
🧰 Additional context used
🧬 Code graph analysis (2)
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx (1)
webapp/src/hooks/useUserPreferenceStorage.ts (1)
  • useUserPreferenceStorage (10-33)
webapp/src/hooks/useUserPreferenceStorage.ts (1)
webapp/src/service/http/useQueryApi.ts (2)
  • useApiQuery (119-141)
  • useApiMutation (224-265)
🪛 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] 103-103: 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). (11)
  • GitHub Check: E2E testing ‍🔎 (15, 11)
  • GitHub Check: E2E testing ‍🔎 (15, 5)
  • GitHub Check: E2E testing ‍🔎 (15, 9)
  • GitHub Check: E2E testing ‍🔎 (15, 10)
  • GitHub Check: E2E testing ‍🔎 (15, 4)
  • GitHub Check: E2E testing ‍🔎 (15, 7)
  • GitHub Check: E2E testing ‍🔎 (15, 6)
  • GitHub Check: E2E testing ‍🔎 (15, 8)
  • GitHub Check: E2E testing ‍🔎 (15, 0)
  • GitHub Check: BT ‍🔎 (server-app:runStandardTests)
  • GitHub Check: BT ‍🔎 (ee-test:test)

Comment @coderabbitai help to get the list of available commands and usage tips.

@JanCizmar JanCizmar marked this pull request as draft October 7, 2025 10:19
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (7)
webapp/src/service/apiSchema.generated.ts (1)

5975-5978: Consider making UserStorageResponse.data required.

200-success implies the field exists; optional data blurs “missing” vs “explicit null”. Prefer data: unknown and 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>? = null
backend/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.response

Follow-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:

  1. Null value handling: Test storing and retrieving explicit null values vs. non-existent fields.
  2. Field name validation: Test special characters, very long field names, empty field names, or field names with path-like patterns (e.g., field/subfield).
  3. Large payload handling: Test storing large values to ensure the JSON storage can handle them without performance degradation.
  4. Concurrent updates: Test race conditions when multiple requests update the same field simultaneously.
  5. 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

📥 Commits

Reviewing files that changed from the base of the PR and between 6b5315c and 1ba1e0d.

📒 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 UserStorageResponse with 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.

Comment on lines +45 to +59
fun setStorageJsonField(
fieldName: String,
value: Any?,
userAccount: UserAccount,
) {
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)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

Avoid lost updates on storageJson (concurrent writes overwrite each other).

This read-modify-write of the whole JSON map can drop parallel changes. Add optimistic locking (version) and wrap in one transaction; or patch the JSONB in-place.

Diff (transactional + no-op when unchanged):

+import org.springframework.transaction.annotation.Transactional
@@
-  fun setStorageJsonField(
+  @Transactional
+  fun setStorageJsonField(
     fieldName: String,
     value: Any?,
     userAccount: UserAccount,
   ) {
     val preferences = findOrCreate(userAccount.id)
     val currentStorage = preferences.storageJson?.toMutableMap() ?: mutableMapOf()
-    if (value != null) {
+    val previous = currentStorage[fieldName]
+    if (previous == value) return
+    if (value != null) {
       currentStorage[fieldName] = value
     } else {
       currentStorage.remove(fieldName)
     }
     preferences.storageJson = currentStorage
     userPreferencesRepository.save(preferences)
   }

Alternative (safer under concurrency): a repository UPDATE using jsonb_set to upsert/remove a single key atomically, avoiding map rewrites. I can draft this if you want.

To verify optimistic locking support:


🏁 Script executed:

#!/bin/bash
# Check for @Version on UserPreferences/AuditModel
rg -nP --type=kt -C3 $'@Version|\\bclass\\s+AuditModel\\b|\\bopen\\s+class\\s+AuditModel\\b|\\bclass\\s+UserPreferences\\b'

Length of output: 26


🏁 Script executed:

#!/bin/bash
# Check for @Version on UserPreferences or AuditModel
rg -nP -g '*.kt' -C3 '@Version|class\s+UserPreferences\b|open\s+class\s+AuditModel\b|class\s+AuditModel\b'

Length of output: 1357


Add optimistic locking or atomic JSONB updates to prevent lost updates

UserPreferences has no @Version field, so concurrent calls to setStorageJsonField will overwrite each other. Wrap the method in @transactional and add a version property (optimistic locking), or switch to an UPDATE using jsonb_set to patch a single key. Example refactor:

+ import org.springframework.transaction.annotation.Transactional
@@
- fun setStorageJsonField(
+ @Transactional
+ fun setStorageJsonField(
@@
+    val previous = currentStorage[fieldName]
+    if (previous == value) return
🤖 Prompt for AI Agents
In
backend/data/src/main/kotlin/io/tolgee/service/security/UserPreferencesService.kt
around lines 45 to 59, concurrent calls to setStorageJsonField can overwrite
each other because UserPreferences lacks optimistic locking and the method is
not transactional; either add an @Version Long field to the UserPreferences
entity and annotate this service method with @Transactional so save() will
fail/retry on concurrent updates, or implement an atomic repository-level update
that issues an UPDATE ... SET storage_json = jsonb_set(storage_json,
'{fieldName}', to_jsonb(:value)) WHERE user_account_id = :id (or equivalent) so
only the single key is patched without reading and writing the whole JSON.
Ensure to handle null removal via jsonb - operator and propagate/handle
OptimisticLockException if using @Version.

Comment on lines +31 to +34
if (loading) {
return <BoxLoading />;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix conditional hook usage (violates Rules of Hooks).

Hooks are called after early returns (loading/no plans) and indirectly via useSortPlans. Move hooks to top level.

 export const GenericPlanSelector = <T extends GenericPlanType>({
   onChange,
   value,
   selectProps,
   onPlanChange,
   plans,
   loading,
 }: GenericPlanSelector<T>) => {
-  if (loading) {
-    return <BoxLoading />;
-  }
+  const { incrementPlanWithId } = usePreferredPlans();
+  const sortedPlans = useSortPlans(plans || []);
+
+  if (loading) {
+    return <BoxLoading />;
+  }
 
   if (!plans) {
     return (
       <Box>
         <T keyName="administration-assign-plan-no-plans-to-assign" />
       </Box>
     );
   }
 
-  const { incrementPlanWithId } = usePreferredPlans();
-  const sortedPlans = useSortPlans(plans);

Also applies to: 51-53, 85-95, 100-118

Comment on lines +107 to +116
incrementPlanWithId: async (planId: number) => {
const refetched = await loadable.refetch();
const current = refetched.data?.data[planId] ?? 0;
const newValue = {
...refetched.data,
[planId]: current + 1,
};

update(newValue);
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Bug: newValue spreads the whole response, not the stored map.

You should spread the inner data map; otherwise you persist { data: ..., [planId]: ... }.

   return {
     preferredPlansLoadable: loadable,
     incrementPlanWithId: async (planId: number) => {
       const refetched = await loadable.refetch();
-      const current = refetched.data?.data[planId] ?? 0;
-      const newValue = {
-        ...refetched.data,
-        [planId]: current + 1,
-      };
+      const currentMap = (refetched.data?.data ?? {}) as Record<string, number>;
+      const newValue = {
+        ...currentMap,
+        [planId]: (currentMap[planId] ?? 0) + 1,
+      };
 
       update(newValue);
     },
   };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
incrementPlanWithId: async (planId: number) => {
const refetched = await loadable.refetch();
const current = refetched.data?.data[planId] ?? 0;
const newValue = {
...refetched.data,
[planId]: current + 1,
};
update(newValue);
},
return {
preferredPlansLoadable: loadable,
incrementPlanWithId: async (planId: number) => {
const refetched = await loadable.refetch();
const currentMap = (refetched.data?.data ?? {}) as Record<string, number>;
const newValue = {
...currentMap,
[planId]: (currentMap[planId] ?? 0) + 1,
};
update(newValue);
},
};
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx
around lines 107-116, the code spreads refetched.data (the whole response) into
newValue which produces an object like { data: ..., [planId]: ... }; instead,
spread the inner data map so the stored map is updated. Change newValue to
spread refetched.data.data (with safe null checks) and set [planId]: current + 1
so the resulting object is the updated map (e.g. { ...refetched.data.data,
[planId]: current + 1 }) before calling update.

Comment on lines +20306 to +20345
setStorageField: {
parameters: {
path: {
fieldName: string;
};
};
responses: {
/** OK */
200: unknown;
/** Bad Request */
400: {
content: {
"application/json": string;
};
};
/** Unauthorized */
401: {
content: {
"application/json": string;
};
};
/** Forbidden */
403: {
content: {
"application/json": string;
};
};
/** Not Found */
404: {
content: {
"application/json": string;
};
};
};
requestBody: {
content: {
"application/json": { [key: string]: unknown };
};
};
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

PUT request body is too restrictive; should accept any JSON, not only objects.

{ [key: string]: unknown } blocks primitives/arrays. Backend flow suggests setting a field to any JSON value. Use a permissive schema (e.g., JsonNode) or reuse the response DTO wrapper for symmetry.

Update the OpenAPI annotations and regenerate so this segment becomes one of:

Option A (most flexible):

-        "application/json": { [key: string]: unknown };
+        "application/json": components["schemas"]["JsonNode"];

Option B (symmetry with GET wrapper):

-        "application/json": { [key: string]: unknown };
+        "application/json": components["schemas"]["UserStorageResponse"];

Can you confirm the backend controller method parameter type? If it’s currently Map<String, Object>, switching to JsonNode (Jackson) would align with Option A and permit primitives/arrays. If it already expects the wrapper, Option B is correct.


🏁 Script executed:

#!/bin/bash
set -e
# Search for controller method named setStorageField
rg -n 'setStorageField' -g '*.java' -g '*.kt' || true
# Inspect PutMapping annotations for storage endpoints
rg -n '@PutMapping' -g '*.java' -g '*.kt' -C5 || true
# Find Map<String, Object> parameter usage
rg -nP 'Map<\s*String,\s*Object>' -g '*.java' -g '*.kt' || true

Length of output: 138304


🏁 Script executed:

#!/bin/bash
set -e
# Search for UserStorageResponse schema/type definition
rg -n 'UserStorageResponse' -- 'webapp/src/service/apiSchema.generated.ts'

# Search for JsonNode in schema components
rg -n 'JsonNode' -- 'webapp/src/service/apiSchema.generated.ts'

Length of output: 259


Use JsonNode for the PUT request body schema
Change the OpenAPI annotation so the requestBody becomes:

-        "application/json": { [key: string]: unknown };
+        "application/json": components["schemas"]["JsonNode"];

This allows primitives, arrays and objects. Regenerate apiSchema.generated.ts.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
setStorageField: {
parameters: {
path: {
fieldName: string;
};
};
responses: {
/** OK */
200: unknown;
/** Bad Request */
400: {
content: {
"application/json": string;
};
};
/** Unauthorized */
401: {
content: {
"application/json": string;
};
};
/** Forbidden */
403: {
content: {
"application/json": string;
};
};
/** Not Found */
404: {
content: {
"application/json": string;
};
};
};
requestBody: {
content: {
"application/json": { [key: string]: unknown };
};
};
};
setStorageField: {
parameters: {
path: {
fieldName: string;
};
};
responses: {
/** OK */
200: unknown;
/** Bad Request */
400: {
content: {
"application/json": string;
};
};
/** Unauthorized */
401: {
content: {
"application/json": string;
};
};
/** Forbidden */
403: {
content: {
"application/json": string;
};
};
/** Not Found */
404: {
content: {
"application/json": string;
};
};
};
requestBody: {
content: {
"application/json": components["schemas"]["JsonNode"];
};
};
};
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 20306 to 20345, the
PUT operation setStorageField currently types the requestBody content as { [key:
string]: unknown } which only models objects; update the OpenAPI annotation so
the requestBody content for "application/json" uses JsonNode (to allow
primitives, arrays, and objects), then regenerate apiSchema.generated.ts so the
generated TypeScript reflects JsonNode for that request body.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant