feat: plans migration #3216
feat: plans migration #3216
Conversation
WalkthroughAdds plan migration functionality across backend and webapp: new API endpoints and schemas, UI forms and routes for creating/editing migrations (cloud and self-hosted EE), list and status components, chips with detail tooltips, validation, links, and Cypress data-cy types. Backend includes date utility, cron scheduling, new messages/i18n, and an internal properties setter facade refactor. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Admin User
participant UI as Admin UI (Webapp)
participant API as Billing API
participant DB as Storage
rect rgb(240,248,255)
note over UI: Create Migration (Cloud/EE)
Admin->>UI: Open Create Migration
UI->>API: POST /plans/migration (payload)
API->>DB: Persist migration
DB-->>API: Migration id
API-->>UI: 201 Created + id
UI-->>Admin: Success message + navigate to plans
end
rect rgb(245,255,240)
note over UI: Edit Migration with Subscriptions
Admin->>UI: Open Edit Migration
UI->>API: GET /plans/migration/{id}
API-->>UI: Migration detail
UI->>API: GET /plans/migration/{id}/subscriptions?page=N
API-->>UI: Paged records
Admin->>UI: Save changes
UI->>API: PUT /plans/migration/{id} (payload)
API->>DB: Update migration
API-->>UI: 200 OK
UI-->>Admin: Updated + navigate to list
Admin->>UI: Delete migration
UI->>API: DELETE /plans/migration/{id}
API->>DB: Remove migration
API-->>UI: 204 No Content
UI-->>Admin: Deleted + navigate to list
end
sequenceDiagram
autonumber
participant List as Plans View
participant Chip as Migrating Chip
participant API as Billing API
List->>Chip: Render with migrationId
Chip-->>List: Hidden if no id
Chip->>Chip: onOpen => set opened=true
Chip->>API: GET /plans/migration/{id} (enabled by opened)
API-->>Chip: Migration detail
Chip-->>List: Show tooltip with detail (edit link)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dkrizan
force-pushed
the
danielkrizan/plan-migration
branch
from
d6dccb9 to
af75b99
Compare
|
This PR is stale because it has been open for 30 days with no activity. |
…dates when subscription scheduled, complete schedule on webhook event
dkrizan
force-pushed
the
danielkrizan/plan-migration
branch
from
72ae86a to
bf9215a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (8)
backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt (1)
89-92: Simplify by reusing the existingdateproperty.The expression
(forcedDate ?: date)is redundant because thedateproperty (lines 84-87) already performs the null checkforcedDate ?: Date(). Directly usedateto maintain consistency with the existing pattern.Apply this diff:
val localDate: LocalDate get() { - return (forcedDate ?: date).toInstant().atZone(systemDefault()).toLocalDate() + return date.toInstant().atZone(systemDefault()).toLocalDate() }backend/data/src/main/kotlin/io/tolgee/component/SchedulingManager.kt (1)
54-57: Consider validating the cron expression and document the null check.The cron string is passed directly to
CronTriggerwithout validation. Invalid cron expressions will throwIllegalArgumentExceptionat runtime. Additionally, the null check for thefutureis good defensive programming (Spring'sTaskScheduler.schedulecan return null if the scheduler is shut down or the trigger produces no next execution time), but this pattern isn't used in the existingscheduleWithFixedDelayfunction (line 44), creating an inconsistency.Recommendations:
- Validate the cron expression before constructing the
CronTriggerto provide clearer error messages:fun scheduleWithCron( runnable: Runnable, cron: String, ): String { + // Validate cron expression early for better error messages + try { + CronTrigger(cron).nextExecutionTime(org.springframework.scheduling.TriggerContext { + java.time.Instant.now() + }) + } catch (e: IllegalArgumentException) { + throw IllegalArgumentException("Invalid cron expression: $cron", e) + } + val future = taskScheduler.schedule(runnable, CronTrigger(cron)) if (future == null) { throw IllegalStateException("Future from scheduler was null") } val id = UUID.randomUUID().toString() scheduledTasks[id] = future return id }
- For consistency, consider adding the same null check to
scheduleWithFixedDelay:fun scheduleWithFixedDelay( runnable: Runnable, period: Duration, ): String { val future = taskScheduler.scheduleWithFixedDelay(runnable, period) + if (future == null) { + throw IllegalStateException("Future from scheduler was null") + } val id = UUID.randomUUID().toString() scheduledTasks[id] = future return id }backend/data/src/main/resources/I18n_en.properties (1)
117-117: Consider the appropriateness of the mouse emoji in the subject line.The 🐁 emoji in the subject line seems unusual for a professional plan migration email. Most email clients will render this emoji, but it may come across as unprofessional or confusing to users. Consider either:
- Removing the emoji entirely for a more professional tone
- Using a more business-appropriate emoji like 📋 or 📅 if an icon is desired
- Clarifying the intent if the mouse emoji has specific brand significance
</invite_end -->
webapp/src/component/layout/HeaderBar.tsx (1)
85-90: Consider a more stable key for custom buttons.Using array index as a React key can cause issues if the
customButtonsarray is dynamically updated, reordered, or filtered, leading to incorrect component reconciliation and potential state bugs.If the buttons have stable identifiers, pass objects with an
idorkeyproperty. Otherwise, consider wrapping each button in a Fragment with a unique key at the call site, or accept an array of{ key: string; element: ReactNode }objects:- customButtons?: ReactNode[]; + customButtons?: Array<{ key: string; element: ReactNode }>;Then render:
- {props.customButtons && - props.customButtons.map((button, index) => ( - <Box key={index} display="flex" alignItems="center"> - {button} - </Box> - ))} + {props.customButtons && + props.customButtons.map(({ key, element }) => ( + <Box key={key} display="flex" alignItems="center"> + {element} + </Box> + ))}Alternatively, if the array is truly static per render and never reordered, document that assumption in a comment.
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx (1)
13-15: Consider typing the colors mapping more strictly.The
colorsobject only maps'COMPLETED'to'success'. On Line 35,(colors as any)[status]uses a type assertion to access this mapping dynamically. While this works, a more type-safe approach would define a proper index signature or use a Record type.Apply this diff for better type safety:
-const colors = { - COMPLETED: 'success', -}; +const colors: Partial<Record<Status, 'success' | 'default'>> = { + COMPLETED: 'success', +};Then on Line 35:
- color={(colors as any)[status] || 'default'} + color={colors[status] || 'default'}webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx (1)
97-186: Consider extracting shared migration edit logic.This file and
AdministrationSelfHostedEePlanMigrationEdit.tsxcontain nearly identical logic (queries, mutations, submit handlers, table rendering). The only differences are API URLs and navigation links. Consider abstracting the shared logic into a reusable component or hook to reduce duplication and improve maintainability.webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
55-60: Simplify redundant null check.Line 57 checks
migration && migration.sourcePlan.id, butmigrationis already checked on line 55. The inner check is redundant.Apply this diff:
const defaultSourcePlan = migration ? { - id: migration && migration.sourcePlan.id, + id: migration.sourcePlan.id, free: migration.sourcePlan.free, } : undefined;webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx (1)
44-60: Filter logic is correct.The filtering implementation correctly:
- Excludes plans in
hiddenIds- Filters by
freestatus when specified- Includes all plans when no filters apply
The logic is sound and handles all edge cases properly.
For improved readability, consider extracting the filter predicate:
- const selectItems = plans - .filter((plan) => { - if (planProps?.hiddenIds?.includes(plan.id)) { - return false; - } - if (planProps?.free !== undefined) { - return planProps.free === plan.free; - } - return true; - }) - .map( + const shouldIncludePlan = (plan: T) => { + if (planProps?.hiddenIds?.includes(plan.id)) return false; + if (planProps?.free !== undefined) return planProps.free === plan.free; + return true; + }; + + const selectItems = plans + .filter(shouldIncludePlan) + .map( (plan) => ({ value: plan.id, name: plan.name, } satisfies SelectItem<number>) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/component/SchedulingManager.kt(2 hunks)backend/data/src/main/kotlin/io/tolgee/constants/Message.kt(1 hunks)backend/data/src/main/resources/I18n_en.properties(1 hunks)backend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt(1 hunks)e2e/cypress/support/dataCyType.d.ts(5 hunks)webapp/src/component/common/FullWidthTooltip.tsx(1 hunks)webapp/src/component/common/table/PaginatedHateoasTable.tsx(4 hunks)webapp/src/component/layout/HeaderBar.tsx(2 hunks)webapp/src/constants/GlobalValidationSchema.tsx(2 hunks)webapp/src/constants/links.tsx(2 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CreatePlanMigrationForm.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/fields/PlanSelectorField.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx(4 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationCreate.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationCreate.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx(4 hunks)webapp/src/ee/billing/administration/subscriptionPlans/viewsSelfHostedEe/AdministrationEePlansView.tsx(4 hunks)webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx(1 hunks)webapp/src/eeSetup/eeModule.ee.tsx(3 hunks)webapp/src/service/apiSchema.generated.ts(2 hunks)webapp/src/service/billingApiSchema.generated.ts(16 hunks)webapp/src/translationTools/useErrorTranslation.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-11T14:43:26.099Z
Learnt from: stepan662
PR: tolgee/tolgee-platform#3124
File: backend/data/src/main/kotlin/io/tolgee/batch/MtProviderCatching.kt:38-39
Timestamp: 2025-06-11T14:43:26.099Z
Learning: In Tolgee Platform, backend Message enum keys are not looked up via `messages*.properties`; localization is performed in the frontend, so adding new enum constants does not require backend properties entries.
Applied to files:
backend/data/src/main/kotlin/io/tolgee/constants/Message.kt
🧬 Code graph analysis (17)
webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx (10)
webapp/src/hooks/useSuccessMessage.tsx (1)
useMessage(6-9)webapp/src/service/http/useQueryApi.ts (2)
useBillingApiQuery(288-293)useBillingApiMutation(295-300)webapp/src/constants/links.tsx (2)
LINKS(69-455)Link(1-45)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
PlanMigrationFormData(29-32)webapp/src/component/layout/DashboardPage.tsx (1)
DashboardPage(43-90)webapp/src/views/administration/components/BaseAdministrationView.tsx (1)
BaseAdministrationView(13-60)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx (1)
EditPlanMigrationForm(17-32)webapp/src/component/common/table/PaginatedHateoasTable.tsx (1)
PaginatedHateoasTable(26-47)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx (1)
PlanMigrationStatus(17-52)webapp/src/component/common/EmptyState.tsx (1)
EmptyState(24-42)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CreatePlanMigrationForm.tsx (2)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (2)
CreatePlanMigrationFormData(34-35)PlanMigrationForm(42-193)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts (1)
PlanType(1-1)
webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationCreate.tsx (7)
webapp/src/hooks/useSuccessMessage.tsx (1)
useMessage(6-9)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (2)
CreatePlanMigrationFormData(34-35)PlanMigrationFormData(29-32)webapp/src/constants/links.tsx (1)
LINKS(69-455)webapp/src/service/http/useQueryApi.ts (1)
useBillingApiMutation(295-300)webapp/src/component/layout/DashboardPage.tsx (1)
DashboardPage(43-90)webapp/src/views/administration/components/BaseAdministrationView.tsx (1)
BaseAdministrationView(13-60)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CreatePlanMigrationForm.tsx (1)
CreatePlanMigrationForm(21-28)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (5)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts (1)
PlanType(1-1)webapp/src/constants/GlobalValidationSchema.tsx (1)
Validation(52-538)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/fields/PlanSelectorField.tsx (1)
PlanSelectorField(8-31)webapp/src/component/common/LabelHint.tsx (1)
LabelHint(17-26)webapp/src/hooks/confirmation.tsx (1)
confirmation(5-7)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx (2)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (2)
PlanMigrationFormData(29-32)PlanMigrationForm(42-193)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts (1)
PlanType(1-1)
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx (1)
webapp/src/service/http/useQueryApi.ts (1)
useBillingApiQuery(288-293)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/fields/PlanSelectorField.tsx (4)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts (1)
PlanType(1-1)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
PlanMigrationFormData(29-32)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx (1)
CloudPlanSelector(7-33)webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx (1)
SelfHostedEePlanSelector(6-29)
webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx (10)
webapp/src/hooks/useSuccessMessage.tsx (1)
useMessage(6-9)webapp/src/service/http/useQueryApi.ts (2)
useBillingApiQuery(288-293)useBillingApiMutation(295-300)webapp/src/constants/links.tsx (2)
LINKS(69-455)Link(1-45)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
PlanMigrationFormData(29-32)webapp/src/component/layout/DashboardPage.tsx (1)
DashboardPage(43-90)webapp/src/views/administration/components/BaseAdministrationView.tsx (1)
BaseAdministrationView(13-60)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx (1)
EditPlanMigrationForm(17-32)webapp/src/component/common/table/PaginatedHateoasTable.tsx (1)
PaginatedHateoasTable(26-47)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx (1)
PlanMigrationStatus(17-52)webapp/src/component/common/EmptyState.tsx (1)
EmptyState(24-42)
webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationCreate.tsx (7)
webapp/src/hooks/useSuccessMessage.tsx (1)
useMessage(6-9)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (2)
CreatePlanMigrationFormData(34-35)PlanMigrationFormData(29-32)webapp/src/constants/links.tsx (1)
LINKS(69-455)webapp/src/service/http/useQueryApi.ts (1)
useBillingApiMutation(295-300)webapp/src/component/layout/DashboardPage.tsx (1)
DashboardPage(43-90)webapp/src/views/administration/components/BaseAdministrationView.tsx (1)
BaseAdministrationView(13-60)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CreatePlanMigrationForm.tsx (1)
CreatePlanMigrationForm(21-28)
webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx (5)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts (1)
PlanType(1-1)webapp/src/service/http/useQueryApi.ts (1)
useBillingApiQuery(288-293)webapp/src/constants/links.tsx (2)
LINKS(69-455)Link(1-45)webapp/src/component/common/FullWidthTooltip.tsx (1)
FullWidthTooltip(3-11)webapp/src/ee/billing/component/Price/PricePrimary.tsx (1)
PricePrimary(28-66)
webapp/src/eeSetup/eeModule.ee.tsx (6)
webapp/src/component/common/PrivateRoute.tsx (1)
PrivateRoute(8-14)webapp/src/constants/links.tsx (1)
LINKS(69-455)webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationCreate.tsx (1)
AdministrationCloudPlanMigrationCreate(17-74)webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx (1)
AdministrationCloudPlanMigrationEdit(22-187)webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationCreate.tsx (1)
AdministrationSelfHostedEePlanMigrationCreate(17-75)webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx (1)
AdministrationSelfHostedEePlanMigrationEdit(22-188)
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx (1)
webapp/src/service/http/useQueryApi.ts (1)
useBillingApiQuery(288-293)
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx (1)
webapp/src/component/searchSelect/SearchSelect.tsx (2)
SearchSelect(41-131)SelectItem(8-11)
webapp/src/component/common/FullWidthTooltip.tsx (1)
webapp/src/colors.tsx (1)
Tooltip(37-40)
webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx (2)
webapp/src/constants/links.tsx (2)
Link(1-45)LINKS(69-455)webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx (1)
PlanMigratingChip(30-228)
webapp/src/service/billingApiSchema.generated.ts (1)
webapp/src/service/apiSchema.generated.ts (1)
components(1065-6030)
webapp/src/ee/billing/administration/subscriptionPlans/viewsSelfHostedEe/AdministrationEePlansView.tsx (2)
webapp/src/constants/links.tsx (2)
Link(1-45)LINKS(69-455)webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx (1)
PlanMigratingChip(30-228)
🪛 Biome (2.1.2)
webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx
[error] 42-42: 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] 43-43: 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] 71-71: 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)
🔇 Additional comments (58)
backend/testing/src/main/kotlin/io/tolgee/fixtures/EmailTestUtil.kt (1)
69-71: LGTM!The new
verifyTimesEmailSentmethod correctly uses Mockito'stimes()verification to check exact invocation count. It follows the same pattern as the existingverifyEmailSent()method and provides useful test utility functionality for count-based email verification.backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt (1)
19-20: LGTM! Imports are correct.The imports for
LocalDateandZoneId.systemDefaultare standard Java time API classes needed for the newlocalDateproperty.backend/data/src/main/resources/I18n_en.properties (1)
118-128: LGTM! Well-structured email body with clear communication.The email body is well-formatted with:
- Clear explanation of the migration with before/after plan details
- Actionable options for users (change plan or contact support)
- Proper HTML formatting and placeholder usage
- Professional yet friendly tone
The 7 placeholders ({0}-{6}) are used appropriately and the message structure follows the established notification pattern in this file.
</invite_end -->
webapp/src/constants/links.tsx (2)
66-66: LGTM! Consistent parameter naming.The new
PLAN_MIGRATION_IDparameter follows the existing naming conventions and integrates well with the routing structure.
250-268: LGTM! Well-structured migration routes.The four new link constants for plan migrations (cloud and self-hosted EE, create and edit) follow the established patterns and are properly nested under their respective parent links. The edit routes correctly use the
PLAN_MIGRATION_IDparameter.backend/data/src/main/kotlin/io/tolgee/constants/Message.kt (1)
311-312: LGTM! New error constants properly integrated.The two new enum constants
PLAN_MIGRATION_NOT_FOUNDandPLAN_HAS_MIGRATIONSfollow the existing naming conventions and are correctly placed. The frontend translation mapping forplan_has_migrationsis already in place inuseErrorTranslation.ts.Based on learnings
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/cloud/fields/CloudPlanSelector.tsx (1)
15-23: LGTM! Clean prop addition for migration filtering.The
filterHasMigrationprop is well-integrated following the same pattern asfilterPublic. The optional type ensures backward compatibility, and it's correctly passed through to the API query.webapp/src/translationTools/useErrorTranslation.ts (1)
110-111: LGTM! Consistent error translation mapping.The new error case for
plan_has_migrationsfollows the established pattern and is logically placed near the relatedplan_has_subscriberserror.webapp/src/eeSetup/eeModule.ee.tsx (2)
71-74: LGTM! Migration component imports are properly structured.The four new imports for cloud and self-hosted EE plan migration components follow the existing import patterns and are clearly named.
146-184: LGTM! Migration routes are well-integrated.The new routes for plan migrations (both cloud and self-hosted EE, create and edit) are properly configured:
- Use
PrivateRoutefor authentication- Have
exactmatching to prevent conflicts- Are logically placed near their related plan routes
- Reference the correct link templates and components
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/types.ts (1)
1-1: LGTM!The
PlanTypeunion type is clean, minimal, and correctly typed for the two plan variants.webapp/src/constants/GlobalValidationSchema.tsx (1)
42-49: LGTM!The
number.minlocale configuration correctly integrates the validation message with i18n translation, consistent with the existingstring.minandstring.maxpatterns.webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CreatePlanMigrationForm.tsx (2)
1-28: Note: This component depends on the PLAN_MIRATION_FORM typo fix.The
PlanMigrationFormcomponent (imported at line 3) uses the misspelledPLAN_MIRATION_FORMvalidation schema. Once the typo inGlobalValidationSchema.tsxis fixed (as noted in that file's review), ensure this component andPlanMigrationFormare updated accordingly.
7-13: Verify that 0 is a valid sentinel value for unselected plan IDs. If your Plan IDs are guaranteed to start at 1, 0 can safely represent “none selected”; otherwise, switchsourcePlanId/targetPlanIdtoundefined(updateCreatePlanMigrationFormDatatonumber | undefined) and tighten your Yup schema to use.min(1)forplanId.webapp/src/component/common/FullWidthTooltip.tsx (1)
1-11: LGTM!The
FullWidthTooltipcomponent correctly overrides the default Tooltip maxWidth using MUI's styled API andtooltipClasses. The pattern of spreadingclassNametopopperis the correct approach for styled Tooltip customization.webapp/src/ee/billing/administration/subscriptionPlans/viewsSelfHostedEe/AdministrationEePlansView.tsx (2)
13-13: LGTM! Icon import added correctly.The Settings01 icon import is used correctly in the migration action button below.
155-159: LGTM! Migration chip integrated correctly.The PlanMigratingChip is properly integrated with the correct
planType="self-hosted"and migration data from the plan model.webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx (3)
1-11: LGTM! Types and imports are well-structured.The Status type correctly references the API schema, and the Props interface is clear.
21-30: LGTM! Status label mapping is correct.The switch statement properly maps known statuses to translation keys with a sensible fallback.
32-51: LGTM! Conditional tooltip rendering is well-implemented.The component correctly wraps the chip in a tooltip when a date is provided, with proper UTC formatting.
webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationCreate.tsx (5)
1-15: LGTM! Imports are clean and correctly organized.All imports are used and follow the project's module path conventions.
17-20: LGTM! Component initialization follows standard patterns.Hooks are correctly initialized and used throughout the component.
22-38: LGTM! Submit handler is correctly implemented.The type casting and success flow (message + navigation) are appropriate for the create operation.
40-43: LGTM! Mutation configuration is correct.The billing API mutation is properly configured for creating cloud plan migrations.
45-73: LGTM! Component rendering is well-structured.The navigation breadcrumb, form integration, and loading state management are all correctly implemented.
webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationCreate.tsx (5)
1-15: LGTM! Imports are correctly organized.All necessary imports are present and follow project conventions.
17-20: LGTM! Hook initialization is standard.
22-38: LGTM! Submit handler correctly targets self-hosted endpoint.The endpoint and navigation target are appropriate for self-hosted EE plan migrations.
40-43: LGTM! Mutation is correctly configured for self-hosted migrations.
62-75: LGTM! Form configuration is correct for self-hosted.The
planType="self-hosted"prop ensures the form behaves correctly for self-hosted EE migrations.webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx (3)
1-8: LGTM! Type definitions and imports are clean.The type aliases correctly reference the API schema models for both cloud and self-hosted migrations.
9-15: LGTM! Props interface is well-defined.The Props type correctly supports both cloud and self-hosted migration models via a union type.
17-32: LGTM! Initial values mapping is correct.The component correctly maps migration data to the form's expected structure and forwards all props to PlanMigrationForm.
webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx (2)
12-12: LGTM!The new imports for
Settings01icon andPlanMigratingChipare correctly placed and used in the component.Also applies to: 29-29
148-151: PlanMigratingChip props match CloudPlanModel schema
CloudPlanModelinbillingApiSchema.generated.tsdefines bothmigrationId?: numberandactiveMigration?: boolean, so passing these props is correct.webapp/src/component/common/table/PaginatedHateoasTable.tsx (3)
1-1: LGTM!The import additions and type extension to support an optional
tableHeadprop are well-structured and maintain backward compatibility.Also applies to: 11-11, 22-24
49-63: LGTM!The internal
PaginatedHateoasTableListComponentcleanly handles the conditional rendering of the table header. The implementation is straightforward and correct.
35-46: Approve listComponent function usage – ThelistComponentprop is defined as aJSXElementConstructor<any>, so supplying a function that receives props and returns JSX correctly satisfies the expected component type; no changes needed.webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/selfHostedEe/fields/SelfHostedEePlanSelector.tsx (2)
12-13: LGTM!The addition of the optional
filterHasMigrationprop is clean and maintains backward compatibility. The prop follows the same pattern asorganizationId.
14-21: filterHasMigration is supported by the backend API schema
The generatedbillingApiSchema.generated.tsdefinesfilterHasMigration?: booleanalongsidefilterAssignableToOrganizationfor thegetPlansoperation on/v2/administration/billing/self-hosted-ee-plans, so no further changes are needed.webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx (1)
78-78: Non-null assertion is safe here.The non-null assertion is acceptable because the loading check on line 74 guarantees that
migrationLoadable.datais defined at this point.webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx (1)
78-78: Non-null assertion is safe here.The non-null assertion is acceptable because the loading check on line 74 guarantees that
migrationLoadable.datais defined at this point.webapp/src/ee/billing/administration/subscriptionPlans/components/migration/fields/PlanSelectorField.tsx (1)
8-31: LGTM!The component provides a clean abstraction over cloud and self-hosted plan selectors, correctly integrates with Formik, and forwards props appropriately. The use of
anyin the generic type (line 17) is acceptable here for flexibility, given that the actual plan types are determined by the underlying selector components.webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
42-192: Well-structured generic form component.The component properly abstracts create and edit modes, handles plan selection with interdependent constraints, integrates validation, and provides clear UX for delete confirmation. The generic type constraints appropriately restrict usage to the two supported form data types.
webapp/src/ee/billing/administration/subscriptionPlans/components/planForm/genericFields/GenericPlanSelector.tsx (8)
9-14: LGTM! Type extensions support the filtering feature.The addition of the
freefield toGenericPlanTypeand the newPlansPropstype cleanly support the plan filtering functionality introduced in this PR.
44-60: LGTM! Filtering logic is correct and well-structured.The plan filtering implementation correctly:
- Excludes plans whose IDs are in
hiddenIds- Filters by
freestatus only when explicitly defined- Uses
satisfiesfor type safetyThe logic properly handles both filter conditions and maintains the original mapping structure.
78-78: LGTM! ThedataCyprop enables better test flexibility.Passing the
dataCyprop toSearchSelectallows customization of data-cy attributes for different plan selector instances while maintaining backward compatibility through the default value.
18-18: Ensure allGenericPlanSelectorusages handle the newonPlanChange(plan: T)signature.
The callback now receives the fullplanobject instead of an ID—update every handler accordingly.
9-14: LGTM! Type definitions are well-structured.The addition of
freetoGenericPlanTypeand the newPlansPropstype for filtering configuration are clear and appropriately typed.
16-25: API change improves developer experience.The change to
onPlanChangereceiving the full plan object (line 18) instead of just the ID is a breaking change but provides better DX by reducing the need for consumers to perform lookups. The new optional propsplanPropsanddataCyenhance filtering capabilities and testability.
27-35: LGTM! Props destructuring is correct.The component signature correctly destructures the new props with an appropriate default for
dataCy.
76-85: LGTM! dataCy properly propagated for testing.The
dataCyprop is correctly passed through to theSearchSelectcomponent, enabling test targeting with appropriate defaults.webapp/src/service/billingApiSchema.generated.ts (6)
22-32: LGTM! Migration endpoints are well-structured and consistent.The new plan migration endpoints for both cloud and self-hosted plans follow REST conventions and maintain consistency across both plan types. The CRUD operations (create, get, update, delete) and subscription history retrieval are properly defined.
Also applies to: 58-68
243-243: LGTM! Plan model extensions support migration tracking.The addition of
activeMigrationandmigrationIdfields to bothAdministrationCloudPlanModelandSelfHostedEePlanAdministrationModelenables proper tracking of plan migrations. Both fields are correctly optional and consistently applied across plan types.Also applies to: 281-282, 1240-1240, 1278-1279
318-330: LGTM! Migration model schemas are complete and consistent.Both
CloudPlanMigrationModelandAdministrationSelfHostedEePlanMigrationModelhave well-structured schemas with:
- Proper fields for migration scheduling (offset days)
- Source and target plan references
- Enabled flag for controlling migration status
- Optional subscriptions count for tracking impact
The consistency between cloud and self-hosted variants is excellent.
Also applies to: 379-391
546-556: LGTM! Request and response schemas are well-designed.The migration request schemas correctly differentiate between:
CreatePlanMigrationRequest: includessourcePlanIdfor creating new migrationsPlanMigrationRequest: excludessourcePlanIdfor updates (source plan is immutable)The
PlanMigrationHistoryModelprovides comprehensive tracking with organization details, plan names, scheduling/finalization timestamps, and status. The paged wrapper follows standard patterns.Also applies to: 1153-1172, 1038-1043
680-680: LGTM! Query parameters and error codes properly support migrations.The addition of:
filterHasMigrationquery parameter on plan listing endpoints enables filtering by migration status- Error codes
plan_migration_not_foundandplan_has_migrationsprovide appropriate error handling for migration operationsThese additions are consistent with the migration feature requirements.
Also applies to: 2314-2314, 911-913
1755-1958: LGTM! Operation definitions are complete and follow API conventions.All plan migration operations (create, get, update, delete, list subscriptions) are properly defined for both cloud and self-hosted variants with:
- Correct request/response body types
- Proper path parameters (
migrationId)- Standard HTTP status codes
- Pagination support for subscription listings
The consistency between cloud and self-hosted operation definitions is excellent.
Also applies to: 2389-2592
| static readonly PLAN_MIRATION_FORM = () => | ||
| Yup.object().shape({ | ||
| monthlyOffsetDays: Yup.number().required().min(0), | ||
| yearlyOffsetDays: Yup.number().required().min(0), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
Fix typo in constant name: PLAN_MIRATION_FORM → PLAN_MIGRATION_FORM.
The constant is misspelled as PLAN_MIRATION_FORM (missing a "G" in "MIGRATION"). This typo will propagate to all consumers of this validation schema.
Apply this diff to fix the typo:
- static readonly PLAN_MIRATION_FORM = () =>
+ static readonly PLAN_MIGRATION_FORM = () =>
Yup.object().shape({
monthlyOffsetDays: Yup.number().required().min(0),
yearlyOffsetDays: Yup.number().required().min(0),
});Then update all references across the codebase:
🏁 Script executed:
#!/bin/bash
# Description: Find all references to the misspelled constant name
rg -n "PLAN_MIRATION_FORM" --type ts --type tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
# Search for references to the misspelled constant in all .ts and .tsx files
rg -n "PLAN_MIRATION_FORM" -g '*.ts*'Length of output: 252
Fix typo in constant name: PLAN_MIRATION_FORM → PLAN_MIGRATION_FORM
Rename the constant in webapp/src/constants/GlobalValidationSchema.tsx (line 533) and update its usage in webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (line 83).
--- a/webapp/src/constants/GlobalValidationSchema.tsx
@@ -533,7 +533,7 @@
- static readonly PLAN_MIRATION_FORM = () =>
+ static readonly PLAN_MIGRATION_FORM = () =>--- a/webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx
@@ -83,7 +83,7 @@
- validationSchema={Validation.PLAN_MIRATION_FORM}
+ validationSchema={Validation.PLAN_MIGRATION_FORM}Then verify no other references remain:
rg -n "PLAN_MIRATION_FORM" -g '*.ts*'🤖 Prompt for AI Agents
In webapp/src/constants/GlobalValidationSchema.tsx around lines 533–537 the
constant name PLAN_MIRATION_FORM is misspelled; rename it to PLAN_MIGRATION_FORM
and export it under the corrected name, then update its usage in
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx
(around line 83) to import/reference PLAN_MIGRATION_FORM instead of
PLAN_MIRATION_FORM; after changes run a project-wide search (e.g., rg -n
"PLAN_MIRATION_FORM" -g '*.ts*') and fix any remaining references to the
misspelled identifier.
| initialValues={initValues} | ||
| enableReinitialize | ||
| onSubmit={onSubmit} | ||
| validationSchema={Validation.PLAN_MIRATION_FORM} |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Verify validation schema name typo.
The validation schema is named PLAN_MIRATION_FORM (missing a 'G' in "MIGRATION"). This appears to be a typo that may also exist in GlobalValidationSchema.tsx. Verify and correct if needed for consistency.
🏁 Script executed:
#!/bin/bash
# Check if the validation schema name is consistently misspelled
rg -n "PLAN_MIRATION_FORM" --type=tsLength of output: 252
🏁 Script executed:
#!/bin/bash
# Search for correctly spelled constant
rg -n "PLAN_MIGRATION_FORM" --type=tsLength of output: 0
Rename the validation schema constant to fix the typo.
- In webapp/src/constants/GlobalValidationSchema.tsx (line 533), rename
PLAN_MIRATION_FORM→PLAN_MIGRATION_FORM. - In webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (line 83), update
Validation.PLAN_MIRATION_FORM→Validation.PLAN_MIGRATION_FORM.
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx
around line 83 and webapp/src/constants/GlobalValidationSchema.tsx around line
533, there is a typo in the validation schema constant name: rename the
exported/defined constant PLAN_MIRATION_FORM to PLAN_MIGRATION_FORM in
GlobalValidationSchema.tsx and update the reference in PlanMigrationForm.tsx
from Validation.PLAN_MIRATION_FORM to Validation.PLAN_MIGRATION_FORM so the
import and usage match the corrected name.
| const messaging = useMessage(); | ||
| const history = useHistory(); | ||
| const formatDate = useDateFormatter(); | ||
| const migrationId = match.params[PARAMS.PLAN_MIGRATION_ID] as number; |
There was a problem hiding this comment.
Fix type coercion for route parameter.
Route parameters from useRouteMatch().params are strings, not numbers. Casting as number doesn't perform conversion—it only suppresses TypeScript errors and will cause runtime type mismatches when the value is used in API calls.
Apply this diff to properly parse the parameter:
- const migrationId = match.params[PARAMS.PLAN_MIGRATION_ID] as number;
+ const migrationId = Number(match.params[PARAMS.PLAN_MIGRATION_ID]);📝 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.
| const migrationId = match.params[PARAMS.PLAN_MIGRATION_ID] as number; | |
| const migrationId = Number(match.params[PARAMS.PLAN_MIGRATION_ID]); |
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx
around line 28, the code is wrongly casting a route param to number using "as
number"; instead, parse the string param to a number (e.g., Number(...) or
parseInt(..., 10)), validate the result (check for NaN) and handle invalid
values (show an error, fall back, or navigate away) before using it in API calls
so runtime type mismatches are avoided.
| navigation={[ | ||
| [ | ||
| t('administration_cloud_plans'), | ||
| LINKS.ADMINISTRATION_BILLING_CLOUD_PLANS.build(), | ||
| ], | ||
| [ | ||
| t('administration_plan_migration_configure'), | ||
| LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build(), | ||
| ], | ||
| ]} |
There was a problem hiding this comment.
Fix the navigation breadcrumb for self-hosted plans.
Lines 53-54 incorrectly reference cloud plans in the breadcrumb for a self-hosted EE plan migration page. The first breadcrumb item should link to the EE plans page, not the cloud plans page.
Apply this diff to correct the navigation:
navigation={[
[
- t('administration_cloud_plans'),
- LINKS.ADMINISTRATION_BILLING_CLOUD_PLANS.build(),
+ t('administration_ee_plans'),
+ LINKS.ADMINISTRATION_BILLING_EE_PLANS.build(),
],
[
t('administration_plan_migration_configure'),
LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build(),
],
]}📝 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.
| navigation={[ | |
| [ | |
| t('administration_cloud_plans'), | |
| LINKS.ADMINISTRATION_BILLING_CLOUD_PLANS.build(), | |
| ], | |
| [ | |
| t('administration_plan_migration_configure'), | |
| LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build(), | |
| ], | |
| ]} | |
| navigation={[ | |
| [ | |
| t('administration_ee_plans'), | |
| LINKS.ADMINISTRATION_BILLING_EE_PLANS.build(), | |
| ], | |
| [ | |
| t('administration_plan_migration_configure'), | |
| LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build(), | |
| ], | |
| ]} |
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationCreate.tsx
around lines 51 to 60, the first breadcrumb item currently references the cloud
plans; update it to point to the EE plans page by replacing the cloud plans link
and label with the EE plans equivalents (use
LINKS.ADMINISTRATION_BILLING_EE_PLANS.build() and the corresponding translation
key like t('administration_ee_plans') instead of the cloud ones) so the
breadcrumb correctly navigates to the EE plans page.
| customButtons={[ | ||
| <Button | ||
| key="create-migration" | ||
| variant="contained" | ||
| size="medium" | ||
| startIcon={<Settings01 width={19} height={19} />} | ||
| component={Link} | ||
| color="warning" | ||
| to={LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build()} | ||
| data-cy="administration-plans-create-migration" | ||
| > | ||
| {t('administration_cloud_plan_create_migration')} | ||
| </Button>, | ||
| ]} |
There was a problem hiding this comment.
Fix the translation key for consistency.
Line 128 uses the translation key 'administration_cloud_plan_create_migration' in the self-hosted EE plans view. This should use a generic key or a self-hosted-specific key to avoid confusion.
Consider using a generic key like 'administration_plan_create_migration' or a self-hosted-specific key like 'administration_ee_plan_create_migration':
- {t('administration_cloud_plan_create_migration')}
+ {t('administration_plan_create_migration')}📝 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.
| customButtons={[ | |
| <Button | |
| key="create-migration" | |
| variant="contained" | |
| size="medium" | |
| startIcon={<Settings01 width={19} height={19} />} | |
| component={Link} | |
| color="warning" | |
| to={LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build()} | |
| data-cy="administration-plans-create-migration" | |
| > | |
| {t('administration_cloud_plan_create_migration')} | |
| </Button>, | |
| ]} | |
| customButtons={[ | |
| <Button | |
| key="create-migration" | |
| variant="contained" | |
| size="medium" | |
| startIcon={<Settings01 width={19} height={19} />} | |
| component={Link} | |
| color="warning" | |
| to={LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_CREATE.build()} | |
| data-cy="administration-plans-create-migration" | |
| > | |
| {t('administration_plan_create_migration')} | |
| </Button>, | |
| ]} |
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/viewsSelfHostedEe/AdministrationEePlansView.tsx
around lines 117-130, the translation key used for the create migration button
is the cloud-specific 'administration_cloud_plan_create_migration'; replace it
with a self-hosted appropriate key (e.g.
'administration_ee_plan_create_migration' or a generic
'administration_plan_create_migration') by changing the t(...) call accordingly,
and then add/update that key in the i18n translation files for all supported
locales so the new key resolves correctly (run i18n build/lint or tests to
verify).
| if (!migrationId) { | ||
| return null; | ||
| } | ||
| const [opened, setOpened] = useState(false); | ||
| const infoCloudLoadable = useBillingApiQuery({ | ||
| url: '/v2/administration/billing/cloud-plans/migration/{migrationId}', | ||
| method: 'get', | ||
| path: { migrationId: migrationId }, | ||
| options: { | ||
| enabled: planType == 'cloud' && !!migrationId && opened, | ||
| }, | ||
| }); | ||
|
|
||
| const infoSelfHostedEeLoadable = useBillingApiQuery({ | ||
| url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}', | ||
| method: 'get', | ||
| path: { migrationId: migrationId }, | ||
| options: { | ||
| enabled: planType == 'self-hosted' && !!migrationId && opened, | ||
| }, | ||
| }); | ||
|
|
||
| const loadable = | ||
| planType == 'cloud' ? infoCloudLoadable : infoSelfHostedEeLoadable; | ||
|
|
||
| const info = loadable.data; | ||
|
|
||
| const configureLink = | ||
| planType == 'cloud' | ||
| ? LINKS.ADMINISTRATION_BILLING_CLOUD_PLAN_MIGRATION_EDIT | ||
| : LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_EDIT; | ||
|
|
||
| const { t } = useTranslate(); |
There was a problem hiding this comment.
Fix React hooks rule violation.
Hooks (useState, useBillingApiQuery, useTranslate) are called after an early return (line 39-41), violating the Rules of Hooks. This will cause React to throw errors if the component re-renders with different migrationId values, as hooks must be called in the same order on every render.
Move the early return after all hook calls:
- if (!migrationId) {
- return null;
- }
const [opened, setOpened] = useState(false);
const infoCloudLoadable = useBillingApiQuery({
url: '/v2/administration/billing/cloud-plans/migration/{migrationId}',
method: 'get',
- path: { migrationId: migrationId },
+ path: { migrationId: migrationId! },
options: {
enabled: planType == 'cloud' && !!migrationId && opened,
},
});
const infoSelfHostedEeLoadable = useBillingApiQuery({
url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}',
method: 'get',
- path: { migrationId: migrationId },
+ path: { migrationId: migrationId! },
options: {
enabled: planType == 'self-hosted' && !!migrationId && opened,
},
});
const loadable =
planType == 'cloud' ? infoCloudLoadable : infoSelfHostedEeLoadable;
const info = loadable.data;
const configureLink =
planType == 'cloud'
? LINKS.ADMINISTRATION_BILLING_CLOUD_PLAN_MIGRATION_EDIT
: LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_EDIT;
const { t } = useTranslate();
+
+ if (!migrationId) {
+ return null;
+ }
+
return (The enabled option in the query configurations already prevents requests when migrationId is falsy, so moving the guard down is safe.
📝 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.
| if (!migrationId) { | |
| return null; | |
| } | |
| const [opened, setOpened] = useState(false); | |
| const infoCloudLoadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/cloud-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId }, | |
| options: { | |
| enabled: planType == 'cloud' && !!migrationId && opened, | |
| }, | |
| }); | |
| const infoSelfHostedEeLoadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId }, | |
| options: { | |
| enabled: planType == 'self-hosted' && !!migrationId && opened, | |
| }, | |
| }); | |
| const loadable = | |
| planType == 'cloud' ? infoCloudLoadable : infoSelfHostedEeLoadable; | |
| const info = loadable.data; | |
| const configureLink = | |
| planType == 'cloud' | |
| ? LINKS.ADMINISTRATION_BILLING_CLOUD_PLAN_MIGRATION_EDIT | |
| : LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_EDIT; | |
| const { t } = useTranslate(); | |
| // — remove the guard here (was originally above all hooks) | |
| const [opened, setOpened] = useState(false); | |
| const infoCloudLoadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/cloud-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId! }, | |
| options: { | |
| enabled: planType === 'cloud' && !!migrationId && opened, | |
| }, | |
| }); | |
| const infoSelfHostedEeLoadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId! }, | |
| options: { | |
| enabled: planType === 'self-hosted' && !!migrationId && opened, | |
| }, | |
| }); | |
| const loadable = | |
| planType === 'cloud' ? infoCloudLoadable : infoSelfHostedEeLoadable; | |
| const info = loadable.data; | |
| const configureLink = | |
| planType === 'cloud' | |
| ? LINKS.ADMINISTRATION_BILLING_CLOUD_PLAN_MIGRATION_EDIT | |
| : LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_EDIT; | |
| const { t } = useTranslate(); | |
| // — move the guard down below all hooks | |
| if (!migrationId) { | |
| return null; | |
| } | |
| return ( | |
| // ... | |
| ); |
🧰 Tools
🪛 Biome (2.1.2)
[error] 42-42: 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] 43-43: 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] 71-71: 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)
🤖 Prompt for AI Agents
In webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx around lines 39 to
71, hooks (useState, useBillingApiQuery, useTranslate) are being called after an
early return when migrationId is falsy, violating the Rules of Hooks; move the
early return so that all hooks are invoked unconditionally on every render
(i.e., remove the immediate return at the top and place the migrationId guard
after the hook calls and after deriving loadable/info/configureLink), relying on
the existing query options.enabled checks to prevent requests when migrationId
is falsy.
| | "unsupported_media_type" | ||
| | "plan_migration_not_found" | ||
| | "plan_has_migrations"; | ||
| params?: unknown[]; |
There was a problem hiding this comment.
🧩 Analysis chain
New plan-migration error codes added — make UI handling exhaustive
The added codes "plan_migration_not_found" and "plan_has_migrations" look correct. Please ensure:
- all error-to-translation mappings (e.g., useErrorTranslation) include these,
- any switch/if handling on ErrorResponseTyped.code is exhaustive to avoid falling through,
- i18n keys exist for both codes.
Run to verify mappings and i18n:
🏁 Script executed:
#!/bin/bash
rg -n --no-heading -C2 -S 'plan_migration_not_found|plan_has_migrations'
# sanity check common translation folders (adjust paths if different)
fd -t f -E node_modules 'en.*\.(json|ts|tsx|yml|yaml)' | xargs -I{} rg -n --no-heading -S 'plan_migration_not_found|plan_has_migrations' {}Length of output: 17850
Add missing mapping for plan_migration_not_found and sync i18n keys
- In webapp/src/translationTools/useErrorTranslation.ts add:
case 'plan_migration_not_found': return t('plan_migration_not_found'); // or use 'administration_plan_migration_not_found' to match existing keys
- Ensure
plan_migration_not_foundexists in your translation JSON (or adjust the mapping key) in en.json and all locale files. - Verify any other switch/if handling
ErrorResponseTyped.codeincludes bothplan_migration_not_foundandplan_has_migrations.
| | "unsupported_media_type" | ||
| | "plan_migration_not_found" | ||
| | "plan_has_migrations"; | ||
| params?: unknown[]; |
There was a problem hiding this comment.
🧩 Analysis chain
Keep StorageTestResult.message in sync with UI surfacing
Same new codes are present in StorageTestResult.message. Confirm any places that surface storage/test errors or generic error toasts also translate these new messages.
Suggested checks:
🏁 Script executed:
#!/bin/bash
rg -n --no-heading -C2 -S 'StorageTestResult|content_storage_test_failed|unsupported_media_type|plan_migration_not_found|plan_has_migrations' webappLength of output: 30670
🏁 Script executed:
#!/bin/bash
rg -n "plan_migration_not_found" webapp/src/i18n/en.json
rg -n "unsupported_media_type" webapp/src/i18n/en.json
rg -n "plan_has_migrations" webapp/src/i18n/en.json
rg -n "unsupported_media_type" -C2 webapp/src/translationTools/useErrorTranslation.ts
rg -n "plan_migration_not_found" -C2 webapp/src/translationTools/useErrorTranslation.ts
sed -n '120,150p' webapp/src/ee/developer/storage/StorageEditDialog.tsxLength of output: 844
Add missing translation mappings for new error codes
- Add cases for
"unsupported_media_type"and"plan_migration_not_found"inuseErrorTranslation.tssotranslateErrorhandles the new messages. - Add corresponding keys for
unsupported_media_typeandplan_migration_not_foundin the i18n JSONs (e.g.en.json) to surface user-friendly text.
🤖 Prompt for AI Agents
In webapp/src/service/apiSchema.generated.ts around lines 5519-5522, the union
of error code strings was extended with "unsupported_media_type" and
"plan_migration_not_found" but the translation layer and i18n resources were not
updated; add corresponding cases in useErrorTranslation.ts to map those two new
codes to appropriate translation keys (e.g., "errors.unsupported_media_type" and
"errors.plan_migration_not_found") and then add those keys with user-facing
messages in the i18n JSON files (e.g., locales/en.json and other locales) so
translateError returns readable messages for these errors.
backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt
Outdated
Show resolved
Hide resolved
| notifications.email.password-changed=Password has been changed for your account. | ||
|
|
||
| notifications.email.plan-migration-subject=Your 🐁 plan will be updated on {0} | ||
| notifications.email.plan-migration-body=Hello! 👋<br/><br/>\ |
There was a problem hiding this comment.
Oh, the encoding is somehow broken. I believe utf-8 is now supported for properties file. Can we fix it? In the meantime, I am asking marketa to provide proper e-mail.
...ng/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx
Outdated
Show resolved
Hide resolved
| import { useDateFormatter } from 'tg.hooks/useLocale'; | ||
| import { EditPlanMigrationForm } from 'tg.ee.module/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm'; | ||
|
|
||
| export const AdministrationSelfHostedEePlanMigrationEdit = () => { |
There was a problem hiding this comment.
I see that SelfHosted and Cloud edition are very similar, but most of the code is duplicated. You should find a way how to deduplicate it.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
backend/data/src/main/resources/I18n_en.properties (1)
118-132: Verify UTF‑8 loading and placeholder/URL handling.The body uses Unicode apostrophes (’), HTML, and placeholders
{0..4}(with{4}embedded in a URL). Please confirm:
- The MessageSource loads properties as UTF‑8 (to avoid mojibake). This echoes a previous review note.
- The parameter order matches the placeholders.
{4}values are URL‑encoded when interpolated.
🧹 Nitpick comments (2)
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx (1)
50-50: Remove trailing space in table cell.The trailing space
{' '}after the closing</Link>tag serves no purpose since there's no adjacent content in the cell.Apply this diff:
- </Link>{' '} + </Link>backend/data/src/main/resources/I18n_en.properties (1)
117-117: Align subject key with existing naming convention.Consider
notifications.email.subject.PLAN_MIGRATIONfor consistency with other subjects.-notifications.email.plan-migration-subject=Upcoming update to your Tolgee subscription plan +notifications.email.subject.PLAN_MIGRATION=Upcoming update to your Tolgee subscription planIf renamed, update all usages accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt(2 hunks)backend/data/src/main/resources/I18n_en.properties(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx
- backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt
- webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx (5)
webapp/src/service/response.types.ts (1)
HateoasListData(99-101)webapp/src/component/common/table/PaginatedHateoasTable.tsx (1)
PaginatedHateoasTable(26-47)webapp/src/constants/links.tsx (2)
Link(1-45)LINKS(69-455)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx (1)
PlanMigrationStatus(17-52)webapp/src/component/common/EmptyState.tsx (1)
EmptyState(24-42)
⏰ 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). (3)
- GitHub Check: Frontend static check 🪲
- GitHub Check: Build backend 🏗️
- GitHub Check: Build frontend 🏗️
| notifications.email.plan-migration-subject=Upcoming update to your Tolgee subscription plan | ||
| notifications.email.plan-migration-body=Dear {0},<br/>\ | ||
| <br/>\ | ||
| Earlier this year, we introduced a new pricing structure for new Tolgee customers while keeping existing users on their current plans. We’re now moving all subscriptions to the new plans to make things simpler and fair for everyone.<br/>\ | ||
| <br/>\ | ||
| In the new structure, translation strings have been replaced with keys and seats to better reflect how Tolgee is used. You can view the updated pricing here <a href="https://tolgee.io/pricing">https://tolgee.io/pricing</a>.<br/>\ | ||
| <br/>\ | ||
| Your current {1} plan will automatically switch to the new {2} plan at your next renewal, whether monthly or yearly, depending on your subscription. If you’d like to explore other subscriptions options, you can do so anytime in the <a href="{3}">Subscriptions</a> section within the Tolgee platform.<br/>\ | ||
| <br/>\ | ||
| Thank you for your continued trust and support!<br/>\ | ||
| <br/>\ | ||
| P.S. I understand these changes may cause discomfort and not accommodate everyone’s circumstances. If this materially impacts your business, I warmly invite you to contact me directly for a personal dialogue regarding your concerns. You can reach me at:<br/>\ | ||
| <ul>\ | ||
| <li>Email: jan@tolgee.io</li>\ | ||
| <li><a href="https://tolg.ee/slack">Tolgee Slack</a></li>\ | ||
| <li>Schedule a meeting via Calendly <a href="https://calendly.com/jancizmar/quick-chat-with-jan?month={4}">https://calendly.com/jancizmar/quick-chat-with-jan?month={4}</a></li>\ | ||
| </ul>\ | ||
| Warm regards,<br/>\ | ||
| Jan<br/>\ | ||
| CEO, Tolgee |
There was a problem hiding this comment.
Avoid double greeting/sign‑off with the global email template; fix minor wording.
If this body is injected into notifications.email.template, it will produce “Hello!” + “Dear {0},” and two sign‑offs (“Warm regards …” and “Regards, Tolgee”). Remove greeting/sign‑off from the body and fix “subscriptions options” → “subscription options”.
-notifications.email.plan-migration-body=Dear {0},<br/>\
-<br/>\
-Earlier this year, we introduced a new pricing structure for new Tolgee customers while keeping existing users on their current plans. We’re now moving all subscriptions to the new plans to make things simpler and fair for everyone.<br/>\
+notifications.email.plan-migration-body=Earlier this year, we introduced a new pricing structure for new Tolgee customers while keeping existing users on their current plans. We’re now moving all subscriptions to the new plans to make things simpler and fair for everyone.<br/>\
<br/>\
In the new structure, translation strings have been replaced with keys and seats to better reflect how Tolgee is used. You can view the updated pricing here <a href="https://tolgee.io/pricing">https://tolgee.io/pricing</a>.<br/>\
<br/>\
-Your current {1} plan will automatically switch to the new {2} plan at your next renewal, whether monthly or yearly, depending on your subscription. If you’d like to explore other subscriptions options, you can do so anytime in the <a href="{3}">Subscriptions</a> section within the Tolgee platform.<br/>\
+Your current {1} plan will automatically switch to the new {2} plan at your next renewal, whether monthly or yearly, depending on your subscription. If you’d like to explore other subscription options, you can do so anytime in the <a href="{3}">Subscriptions</a> section within the Tolgee platform.<br/>\
<br/>\
Thank you for your continued trust and support!<br/>\
<br/>\
P.S. I understand these changes may cause discomfort and not accommodate everyone’s circumstances. If this materially impacts your business, I warmly invite you to contact me directly for a personal dialogue regarding your concerns. You can reach me at:<br/>\
<ul>\
<li>Email: jan@tolgee.io</li>\
<li><a href="https://tolg.ee/slack">Tolgee Slack</a></li>\
<li>Schedule a meeting via Calendly <a href="https://calendly.com/jancizmar/quick-chat-with-jan?month={4}">https://calendly.com/jancizmar/quick-chat-with-jan?month={4}</a></li>\
</ul>\
-Warm regards,<br/>\
-Jan<br/>\
-CEO, Tolgee📝 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.
| notifications.email.plan-migration-subject=Upcoming update to your Tolgee subscription plan | |
| notifications.email.plan-migration-body=Dear {0},<br/>\ | |
| <br/>\ | |
| Earlier this year, we introduced a new pricing structure for new Tolgee customers while keeping existing users on their current plans. We’re now moving all subscriptions to the new plans to make things simpler and fair for everyone.<br/>\ | |
| <br/>\ | |
| In the new structure, translation strings have been replaced with keys and seats to better reflect how Tolgee is used. You can view the updated pricing here <a href="https://tolgee.io/pricing">https://tolgee.io/pricing</a>.<br/>\ | |
| <br/>\ | |
| Your current {1} plan will automatically switch to the new {2} plan at your next renewal, whether monthly or yearly, depending on your subscription. If you’d like to explore other subscriptions options, you can do so anytime in the <a href="{3}">Subscriptions</a> section within the Tolgee platform.<br/>\ | |
| <br/>\ | |
| Thank you for your continued trust and support!<br/>\ | |
| <br/>\ | |
| P.S. I understand these changes may cause discomfort and not accommodate everyone’s circumstances. If this materially impacts your business, I warmly invite you to contact me directly for a personal dialogue regarding your concerns. You can reach me at:<br/>\ | |
| <ul>\ | |
| <li>Email: jan@tolgee.io</li>\ | |
| <li><a href="https://tolg.ee/slack">Tolgee Slack</a></li>\ | |
| <li>Schedule a meeting via Calendly <a href="https://calendly.com/jancizmar/quick-chat-with-jan?month={4}">https://calendly.com/jancizmar/quick-chat-with-jan?month={4}</a></li>\ | |
| </ul>\ | |
| Warm regards,<br/>\ | |
| Jan<br/>\ | |
| CEO, Tolgee | |
| # backend/data/src/main/resources/I18n_en.properties | |
| notifications.email.plan-migration-body=Earlier this year, we introduced a new pricing structure for new Tolgee customers while keeping existing users on their current plans. We’re now moving all subscriptions to the new plans to make things simpler and fair for everyone.<br/>\ | |
| <br/>\ | |
| In the new structure, translation strings have been replaced with keys and seats to better reflect how Tolgee is used. You can view the updated pricing here <a href="https://tolgee.io/pricing">https://tolgee.io/pricing</a>.<br/>\ | |
| <br/>\ | |
| Your current {1} plan will automatically switch to the new {2} plan at your next renewal, whether monthly or yearly, depending on your subscription. If you’d like to explore other subscription options, you can do so anytime in the <a href="{3}">Subscriptions</a> section within the Tolgee platform.<br/>\ | |
| <br/>\ | |
| Thank you for your continued trust and support!<br/>\ | |
| <br/>\ | |
| P.S. I understand these changes may cause discomfort and not accommodate everyone’s circumstances. If this materially impacts your business, I warmly invite you to contact me directly for a personal dialogue regarding your concerns. You can reach me at:<br/>\ | |
| <ul>\ | |
| <li>Email: jan@tolgee.io</li>\ | |
| <li><a href="https://tolg.ee/slack">Tolgee Slack</a></li>\ | |
| <li>Schedule a meeting via Calendly <a href="https://calendly.com/jancizmar/quick-chat-with-jan?month={4}">https://calendly.com/jancizmar/quick-chat-with-jan?month={4}</a></li>\ | |
| </ul>\ |
🤖 Prompt for AI Agents
In backend/data/src/main/resources/I18n_en.properties around lines 117 to 136,
remove the inline greeting and sign‑off so the body can be injected into the
global email template: delete "Dear {0}," at the top and remove the closing
lines from "Warm regards,<br/> Jan<br/> CEO, Tolgee" (leave the P.S. content if
needed but without a personal sign‑off), and correct the phrase "subscriptions
options" to "subscription options"; keep all links and placeholders intact
(including the Calendly month placeholder).
| import { T, useTranslate } from '@tolgee/react'; | ||
| import { PaginatedHateoasTable } from 'tg.component/common/table/PaginatedHateoasTable'; | ||
| import React from 'react'; | ||
| import { UseQueryResult } from 'react-query'; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify react-query import source.
The import uses 'react-query', but modern versions of this library are published under '@tanstack/react-query'. Confirm this is consistent with the project's dependencies and migration strategy.
Based on learnings
Run the following script to check which package is used in package.json:
🏁 Script executed:
#!/bin/bash
# Check if the project uses react-query or @tanstack/react-query
cat package.json | jq '.dependencies["react-query"], .dependencies["@tanstack/react-query"]'Length of output: 9
🏁 Script executed:
#!/bin/bash
# Check devDependencies and peerDependencies for react-query and @tanstack/react-query
cat package.json | jq '.devDependencies["react-query"], .devDependencies["@tanstack/react-query"], .peerDependencies["react-query"], .peerDependencies["@tanstack/react-query"]'Length of output: 19
Import source for React Query
The import on line 8 references react-query, but neither react-query nor @tanstack/react-query is declared in package.json. Install the intended package—react-query (v3) or @tanstack/react-query (v4)—and update the import path accordingly.
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx
around line 8, the code imports UseQueryResult from 'react-query' but neither
react-query (v3) nor @tanstack/react-query (v4) is declared in package.json;
choose the intended library version, add it to package.json (yarn add or npm
install) and update the import path accordingly (keep 'react-query' for v3 or
change to '@tanstack/react-query' for v4), then run the installer and rebuild to
ensure types resolve.
| <TableCell> | ||
| {formatDate(item.scheduledAt, { | ||
| timeZone: 'UTC', | ||
| dateStyle: 'short', | ||
| timeStyle: 'short', | ||
| })} | ||
| </TableCell> |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the date field for the "Migrated At" column.
The column displays item.scheduledAt, but the header translation key administration_plan_migrated_at and the presence of item.finalizedAt (used on line 62) suggest this should show the actual migration completion date, not the scheduled date.
For completed migrations, users likely expect to see when the migration actually happened (finalizedAt), not when it was originally scheduled. Consider:
- Using
item.finalizedAtfor completed migrations anditem.scheduledAtfor scheduled ones, or - Using
item.finalizedAt || item.scheduledAtto show completion date when available, falling back to scheduled date
Run the following script to check the data model and confirm available fields:
🏁 Script executed:
#!/bin/bash
# Find the PlanMigrationHistoryModel schema definition
ast-grep --pattern 'PlanMigrationHistoryModel$$$'Length of output: 0
🏁 Script executed:
#!/bin/bash
rg -n --type=ts --type=tsx "scheduledAt" -C2
rg -n --type=ts --type=tsx "finalizedAt" -C2Length of output: 55
🏁 Script executed:
#!/bin/bash
rg -n "scheduledAt" -C2 --glob "*.ts*"
rg -n "finalizedAt" -C2 --glob "*.ts*"Length of output: 2214
Use migration completion date for the “Migrated At” column.
Replace formatDate(item.scheduledAt, …) with formatDate(item.finalizedAt ?? item.scheduledAt, …) so it shows the actual migration date when available, falling back to the scheduled date.
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx
around lines 54 to 60, the "Migrated At" column currently formats
item.scheduledAt; change it to format the migration completion date by using
item.finalizedAt when present and falling back to item.scheduledAt otherwise
(i.e., use finalizedAt ?? scheduledAt) while keeping the same formatDate options
and UTC timezone.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx (1)
116-129: customButtons prop not supported downstream.This issue was already identified in a previous review.
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx (2)
8-8: Import source for React Query.This issue was already identified in a previous review.
54-60: Use migration completion date.This issue was already identified in a previous review.
🧹 Nitpick comments (1)
webapp/src/ee/billing/component/Plan/migration/PlanMigrationDetail.tsx (1)
73-73: Remove redundant optional chaining.The optional chaining operators (
?.) onmigration.monthlyOffsetDays(line 73) andmigration.yearlyOffsetDays(line 79) are redundant. The early return at line 32 ensuresmigrationis truthy at this point.Apply this diff:
<TooltipText> <T keyName="administration_plan_migration_monthly_timing" - params={{ days: migration?.monthlyOffsetDays, b: <b /> }} + params={{ days: migration.monthlyOffsetDays, b: <b /> }} /> </TooltipText> <TooltipText> <T keyName="administration_plan_migration_yearly_timing" - params={{ days: migration?.yearlyOffsetDays, b: <b /> }} + params={{ days: migration.yearlyOffsetDays, b: <b /> }} /> </TooltipText>Also applies to: 79-79
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CloudPlanEditPlanMigrationForm.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/SelfHostedEePlanEditPlanMigrationForm.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/AdministrationPlanMigrationEditBase.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx(4 hunks)webapp/src/ee/billing/administration/subscriptionPlans/viewsSelfHostedEe/AdministrationEePlansView.tsx(4 hunks)webapp/src/ee/billing/component/Plan/migration/CloudPlanMigratingChip.tsx(1 hunks)webapp/src/ee/billing/component/Plan/migration/PlanMigrationChip.tsx(1 hunks)webapp/src/ee/billing/component/Plan/migration/PlanMigrationDetail.tsx(1 hunks)webapp/src/ee/billing/component/Plan/migration/PlanMigrationPlanPriceDetail.tsx(1 hunks)webapp/src/ee/billing/component/Plan/migration/SelfHostedEePlanMigratingChip.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- webapp/src/ee/billing/administration/subscriptionPlans/migration/selfhosted/AdministrationSelfHostedEePlanMigrationEdit.tsx
- webapp/src/ee/billing/administration/subscriptionPlans/migration/cloud/AdministrationCloudPlanMigrationEdit.tsx
- webapp/src/ee/billing/administration/subscriptionPlans/viewsSelfHostedEe/AdministrationEePlansView.tsx
🧰 Additional context used
🧬 Code graph analysis (10)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CloudPlanEditPlanMigrationForm.tsx (3)
webapp/src/service/http/useQueryApi.ts (1)
useBillingApiMutation(295-300)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
PlanMigrationFormData(29-32)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx (1)
EditPlanMigrationForm(17-32)
webapp/src/ee/billing/component/Plan/migration/SelfHostedEePlanMigratingChip.tsx (3)
webapp/src/service/http/useQueryApi.ts (1)
useBillingApiQuery(288-293)webapp/src/ee/billing/component/Plan/migration/PlanMigrationChip.tsx (1)
PlanMigrationChip(24-67)webapp/src/constants/links.tsx (1)
LINKS(69-455)
webapp/src/ee/billing/component/Plan/migration/PlanMigrationPlanPriceDetail.tsx (1)
webapp/src/ee/billing/component/Price/PricePrimary.tsx (1)
PricePrimary(28-66)
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/AdministrationPlanMigrationEditBase.tsx (6)
webapp/src/service/response.types.ts (1)
HateoasListData(99-101)webapp/src/component/navigation/Navigation.tsx (1)
NavigationItem(33-37)webapp/src/hooks/useSuccessMessage.tsx (1)
useMessage(6-9)webapp/src/component/layout/DashboardPage.tsx (1)
DashboardPage(43-90)webapp/src/views/administration/components/BaseAdministrationView.tsx (1)
BaseAdministrationView(13-60)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx (1)
PlanMigrationHistoryList(21-73)
webapp/src/ee/billing/component/Plan/migration/PlanMigrationChip.tsx (3)
webapp/src/constants/links.tsx (1)
Link(1-45)webapp/src/component/common/FullWidthTooltip.tsx (1)
FullWidthTooltip(3-11)webapp/src/ee/billing/component/Plan/migration/PlanMigrationDetail.tsx (1)
PlanMigrationDetail(30-103)
webapp/src/ee/billing/component/Plan/migration/CloudPlanMigratingChip.tsx (3)
webapp/src/service/http/useQueryApi.ts (1)
useBillingApiQuery(288-293)webapp/src/ee/billing/component/Plan/migration/PlanMigrationChip.tsx (1)
PlanMigrationChip(24-67)webapp/src/constants/links.tsx (1)
LINKS(69-455)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/SelfHostedEePlanEditPlanMigrationForm.tsx (3)
webapp/src/service/http/useQueryApi.ts (1)
useBillingApiMutation(295-300)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/PlanMigrationForm.tsx (1)
PlanMigrationFormData(29-32)webapp/src/ee/billing/administration/subscriptionPlans/components/migration/EditPlanMigrationForm.tsx (1)
EditPlanMigrationForm(17-32)
webapp/src/ee/billing/component/Plan/migration/PlanMigrationDetail.tsx (2)
webapp/src/constants/links.tsx (1)
Link(1-45)webapp/src/ee/billing/component/Plan/migration/PlanMigrationPlanPriceDetail.tsx (1)
PlanMigrationPlanPriceDetail(25-61)
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationHistoryList.tsx (3)
webapp/src/service/response.types.ts (1)
HateoasListData(99-101)webapp/src/component/common/table/PaginatedHateoasTable.tsx (1)
PaginatedHateoasTable(26-47)webapp/src/constants/links.tsx (2)
Link(1-45)LINKS(69-455)
webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx (2)
webapp/src/constants/links.tsx (2)
Link(1-45)LINKS(69-455)webapp/src/ee/billing/component/Plan/migration/CloudPlanMigratingChip.tsx (1)
CloudPlanMigratingChip(6-34)
🪛 Biome (2.1.2)
webapp/src/ee/billing/component/Plan/migration/SelfHostedEePlanMigratingChip.tsx
[error] 16-16: 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] 17-17: 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)
webapp/src/ee/billing/component/Plan/migration/CloudPlanMigratingChip.tsx
[error] 16-16: 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] 17-17: 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)
⏰ 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 (7)
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/CloudPlanEditPlanMigrationForm.tsx (1)
1-59: LGTM!The component correctly wires up API mutations for updating and deleting cloud plan migrations, with proper error handling and loading states passed to the child form.
webapp/src/ee/billing/administration/subscriptionPlans/viewsCloud/AdministrationCloudPlansView.tsx (1)
148-151: LGTM!The CloudPlanMigratingChip is properly integrated with the correct props to display migration status for each plan.
webapp/src/ee/billing/component/Plan/migration/PlanMigrationPlanPriceDetail.tsx (1)
1-61: LGTM!The component correctly renders plan price details with appropriate styling and translation support for migration contexts.
webapp/src/ee/billing/administration/subscriptionPlans/components/migration/SelfHostedEePlanEditPlanMigrationForm.tsx (1)
1-61: LGTM!The component correctly implements the self-hosted EE plan migration editing flow, properly wiring API mutations and passing the planType to distinguish it from cloud migrations.
webapp/src/ee/billing/component/Plan/migration/PlanMigrationChip.tsx (1)
1-67: LGTM!The component correctly implements a migration status chip with a detailed tooltip, properly handling loading, error, and success states with good UX.
webapp/src/ee/billing/component/Plan/migration/PlanMigrationDetail.tsx (1)
30-102: LGTM!The component is well-structured with proper separation of concerns, consistent use of translations, and appropriate Material-UI patterns. The conditional rendering for active/pending states and the graceful handling of missing migrations provide good user experience.
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/AdministrationPlanMigrationEditBase.tsx (1)
44-56: **No action needed: onSubmit/onDelete only fire on successful API calls.** Both form components pass callbacks viamutate(..., { onSuccess: … }), so errors won’t trigger success messages or navigation.
| if (migrations.isLoading) { | ||
| return <SpinnerProgress />; | ||
| } | ||
|
|
||
| const migration = migrations.data!; |
There was a problem hiding this comment.
Add error handling and null checks before accessing data.
The component only checks migrations.isLoading but doesn't handle error states or the case where data is undefined. The non-null assertion at line 62 will cause a runtime error if the query fails or returns no data.
Apply this diff to add proper guards:
if (migrations.isLoading) {
return <SpinnerProgress />;
}
+ if (migrations.isError || !migrations.data) {
+ return (
+ <DashboardPage>
+ <BaseAdministrationView
+ windowTitle={t('administration_plan_migration_configure_existing')}
+ allCentered
+ hideChildrenOnLoading={false}
+ navigation={navigation}
+ >
+ <Typography color="error">
+ <T keyName="administration_plan_migration_not_found" />
+ </Typography>
+ </BaseAdministrationView>
+ </DashboardPage>
+ );
+ }
+
- const migration = migrations.data!;
+ const migration = migrations.data;📝 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.
| if (migrations.isLoading) { | |
| return <SpinnerProgress />; | |
| } | |
| const migration = migrations.data!; | |
| if (migrations.isLoading) { | |
| return <SpinnerProgress />; | |
| } | |
| if (migrations.isError || !migrations.data) { | |
| return ( | |
| <DashboardPage> | |
| <BaseAdministrationView | |
| windowTitle={t('administration_plan_migration_configure_existing')} | |
| allCentered | |
| hideChildrenOnLoading={false} | |
| navigation={navigation} | |
| > | |
| <Typography color="error"> | |
| <T keyName="administration_plan_migration_not_found" /> | |
| </Typography> | |
| </BaseAdministrationView> | |
| </DashboardPage> | |
| ); | |
| } | |
| const migration = migrations.data; |
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/AdministrationPlanMigrationEditBase.tsx
around lines 58 to 62, the component only checks migrations.isLoading and then
uses migrations.data! which can be undefined on error; add guards to handle
migrations.isError (or migrations.error) and the case where migrations.data is
null/undefined: if migrations.isError render an error message/component (or log
and return null), and if !migrations.data return a fallback UI (e.g., a "not
found" message or null) instead of using the non‑null assertion; remove the
forced non‑null operator and use a local const assignment only after verifying
data exists so runtime errors are avoided.
| if (!migrationId) { | ||
| return null; | ||
| } | ||
| const [opened, setOpened] = useState(false); | ||
| const loadable = useBillingApiQuery({ | ||
| url: '/v2/administration/billing/cloud-plans/migration/{migrationId}', | ||
| method: 'get', | ||
| path: { migrationId: migrationId }, | ||
| options: { | ||
| enabled: !!migrationId && opened, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix Rules of Hooks violation.
Hooks are called after the early return, violating React's Rules of Hooks. This can cause state corruption and unpredictable behavior.
Apply this diff to move hooks before the early return:
export const CloudPlanMigratingChip = ({
migrationId,
isEnabled,
}: {
migrationId?: number;
isEnabled?: boolean;
}) => {
- if (!migrationId) {
- return null;
- }
const [opened, setOpened] = useState(false);
const loadable = useBillingApiQuery({
url: '/v2/administration/billing/cloud-plans/migration/{migrationId}',
method: 'get',
- path: { migrationId: migrationId },
+ path: { migrationId: migrationId! },
options: {
enabled: !!migrationId && opened,
},
});
+ if (!migrationId) {
+ return null;
+ }
+
return (
<PlanMigrationChip
loadable={loadable}
editLink={LINKS.ADMINISTRATION_BILLING_CLOUD_PLAN_MIGRATION_EDIT}
isEnabled={isEnabled}
onOpen={() => setOpened(true)}
/>
);
};📝 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.
| if (!migrationId) { | |
| return null; | |
| } | |
| const [opened, setOpened] = useState(false); | |
| const loadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/cloud-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId }, | |
| options: { | |
| enabled: !!migrationId && opened, | |
| }, | |
| }); | |
| export const CloudPlanMigratingChip = ({ | |
| migrationId, | |
| isEnabled, | |
| }: { | |
| migrationId?: number; | |
| isEnabled?: boolean; | |
| }) => { | |
| const [opened, setOpened] = useState(false); | |
| const loadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/cloud-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId! }, | |
| options: { | |
| enabled: !!migrationId && opened, | |
| }, | |
| }); | |
| if (!migrationId) { | |
| return null; | |
| } | |
| return ( | |
| <PlanMigrationChip | |
| loadable={loadable} | |
| editLink={LINKS.ADMINISTRATION_BILLING_CLOUD_PLAN_MIGRATION_EDIT} | |
| isEnabled={isEnabled} | |
| onOpen={() => setOpened(true)} | |
| /> | |
| ); | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 16-16: 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] 17-17: 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)
🤖 Prompt for AI Agents
In webapp/src/ee/billing/component/Plan/migration/CloudPlanMigratingChip.tsx
around lines 13 to 24, the useState and useBillingApiQuery hooks are called
after an early return which violates the Rules of Hooks; move the hook calls to
before the early return (call useState and useBillingApiQuery unconditionally),
keep the query's enabled option computed from opened && !!migrationId so it
won't run when migrationId is falsy, and then retain the existing early return
(if (!migrationId) return null) after the hooks so rendering still returns null
when appropriate.
| if (!migrationId) { | ||
| return null; | ||
| } | ||
| const [opened, setOpened] = useState(false); | ||
| const loadable = useBillingApiQuery({ | ||
| url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}', | ||
| method: 'get', | ||
| path: { migrationId: migrationId }, | ||
| options: { | ||
| enabled: !!migrationId && opened, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Fix Rules of Hooks violation.
Hooks are called after the early return, violating React's Rules of Hooks. This can cause state corruption and unpredictable behavior.
Apply this diff to move hooks before the early return:
export const SelfHostedEePlanMigratingChip = ({
migrationId,
isEnabled,
}: {
migrationId?: number;
isEnabled?: boolean;
}) => {
- if (!migrationId) {
- return null;
- }
const [opened, setOpened] = useState(false);
const loadable = useBillingApiQuery({
url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}',
method: 'get',
- path: { migrationId: migrationId },
+ path: { migrationId: migrationId! },
options: {
enabled: !!migrationId && opened,
},
});
+ if (!migrationId) {
+ return null;
+ }
+
return (
<PlanMigrationChip
loadable={loadable}
editLink={LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_EDIT}
isEnabled={isEnabled}
onOpen={() => setOpened(true)}
/>
);
};📝 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.
| if (!migrationId) { | |
| return null; | |
| } | |
| const [opened, setOpened] = useState(false); | |
| const loadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId }, | |
| options: { | |
| enabled: !!migrationId && opened, | |
| }, | |
| }); | |
| export const SelfHostedEePlanMigratingChip = ({ | |
| migrationId, | |
| isEnabled, | |
| }: { | |
| migrationId?: number; | |
| isEnabled?: boolean; | |
| }) => { | |
| const [opened, setOpened] = useState(false); | |
| const loadable = useBillingApiQuery({ | |
| url: '/v2/administration/billing/self-hosted-ee-plans/migration/{migrationId}', | |
| method: 'get', | |
| path: { migrationId: migrationId! }, | |
| options: { | |
| enabled: !!migrationId && opened, | |
| }, | |
| }); | |
| if (!migrationId) { | |
| return null; | |
| } | |
| return ( | |
| <PlanMigrationChip | |
| loadable={loadable} | |
| editLink={LINKS.ADMINISTRATION_BILLING_EE_PLAN_MIGRATION_EDIT} | |
| isEnabled={isEnabled} | |
| onOpen={() => setOpened(true)} | |
| /> | |
| ); | |
| }; |
🧰 Tools
🪛 Biome (2.1.2)
[error] 16-16: 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] 17-17: 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)
🤖 Prompt for AI Agents
In
webapp/src/ee/billing/component/Plan/migration/SelfHostedEePlanMigratingChip.tsx
around lines 13 to 24, hooks (useState and useBillingApiQuery) are currently
declared after an early return which breaks React's Rules of Hooks; move the
hooks (declare const [opened, setOpened] = useState(false) and the loadable =
useBillingApiQuery({...}) call) to before the if (!migrationId) { return null; }
guard so hooks are always called in the same order, then keep the early return
to bail out when migrationId is falsy.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationRecordList.tsx (2)
8-8: React Query import path likely incorrect or missing dependencyUse '@tanstack/react-query' (v4+) or ensure 'react-query' (v3) is installed. Update the import to match the installed package to avoid type resolution/build errors.
Run to verify package presence:
#!/bin/bash jq -r '.dependencies["react-query"], .dependencies["@tanstack/react-query"], .devDependencies["react-query"], .devDependencies["@tanstack/react-query"]' package.json
54-59: Show actual migration date when availableUse finalizedAt when present, falling back to scheduledAt, so “Migrated At” shows completion time for completed items.
Apply:
- {formatDate(item.scheduledAt, { + {formatDate(item.finalizedAt ?? item.scheduledAt, { timeZone: 'UTC', dateStyle: 'short', timeStyle: 'short', })}
🧹 Nitpick comments (2)
webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationRecordList.tsx (1)
43-49: Prefer client-side navigation to avoid full page reloadUse RouterLink (react-router) with
component={RouterLink}andto=...to keep SPA navigation smooth.Example:
-import { Link, TableCell, TableRow } from '@mui/material'; +import { Link as MuiLink, TableCell, TableRow } from '@mui/material'; +import { Link as RouterLink } from 'react-router-dom'; ... - <Link - href={LINKS.ORGANIZATION_PROFILE.build({ + <MuiLink + component={RouterLink} + to={LINKS.ORGANIZATION_PROFILE.build({ [PARAMS.ORGANIZATION_SLUG]: item.organizationSlug, })} > {item.organizationName} - </Link> + </MuiLink>backend/development/src/main/kotlin/io/tolgee/facade/InternalPropertiesSetterFacade.kt (1)
10-11: Make property resolution more robust and iteration clearer.Use
memberPropertiesto include inherited members and prefer a for‑loop to avoid non‑local returns.-import kotlin.reflect.full.declaredMemberProperties +import kotlin.reflect.full.memberProperties @@ - val props = instance::class.declaredMemberProperties + val props = instance::class.memberPropertiesOptionally refactor the loop for clarity:
- name.split(".").let { namePath -> - namePath.forEachIndexed { idx, property -> - val isLast = idx == namePath.size - 1 - val props = instance::class.memberProperties - val prop = props.find { it.name == property } ?: throw NotFoundException() - if (isLast) { - (prop as? KMutableProperty1<Any, Any?>)?.let { - if (!it.hasAnnotation<E2eRuntimeMutable>()) { - throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE) - } - it.set(instance, setPropertyDto.value) - onSet?.invoke() - return - } ?: throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE) - } - instance = (prop as KProperty1<Any, Any?>).get(instance) - ?: throw NotFoundException() - } - } + val namePath = name.split(".") + for ((idx, property) in namePath.withIndex()) { + val isLast = idx == namePath.lastIndex + val props = instance::class.memberProperties + val prop = props.find { it.name == property } ?: throw NotFoundException() + if (isLast) { + (prop as? KMutableProperty1<Any, Any?>)?.let { + if (!it.hasAnnotation<E2eRuntimeMutable>()) { + throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE) + } + it.set(instance, setPropertyDto.value) + onSet?.invoke() + return + } ?: throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE) + } + instance = (prop as KProperty1<Any, Any?>).get(instance) ?: throw NotFoundException() + }Note: Consider wrapping
it.set(...)to surface a clearer BadRequest on type mismatch.Also applies to: 25-25, 22-40
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/development/src/main/kotlin/io/tolgee/controllers/internal/PropertiesController.kt(1 hunks)backend/development/src/main/kotlin/io/tolgee/facade/InternalPropertiesSetterFacade.kt(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/AdministrationPlanMigrationEditBase.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationRecordList.tsx(1 hunks)webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx(1 hunks)webapp/src/service/billingApiSchema.generated.ts(16 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- webapp/src/ee/billing/administration/subscriptionPlans/migration/general/PlanMigrationStatus.tsx
- webapp/src/ee/billing/administration/subscriptionPlans/migration/general/AdministrationPlanMigrationEditBase.tsx
🔇 Additional comments (3)
webapp/src/service/billingApiSchema.generated.ts (2)
22-32: Migration endpoints look consistent across cloud and self‑hostedCreate/get/update/delete and subscriptions endpoints are coherent and mirror each other. No issues spotted.
Also applies to: 58-68
1040-1044: Confirm UI consumption of embedded list keyThe paged list embeds items under
_embedded.planMigrationRecordModelList. Ensure consumers (e.g., PaginatedHateoasTable/HateoasListData) handle this key, or add a mapper to normalize it.You can grep for handling of embedded keys:
Also applies to: 1914-1933, 2550-2566
backend/development/src/main/kotlin/io/tolgee/controllers/internal/PropertiesController.kt (1)
13-15: Clean delegation; please confirm internal-only exposure.Constructor DI and facade delegation look good. Confirm that
@InternalControllerensures this endpoint is dev/ops‑only and inaccessible in production-user contexts.Also applies to: 22-22
| (prop as? KMutableProperty1<Any, Any?>)?.let { | ||
| if (!it.hasAnnotation<E2eRuntimeMutable>()) { | ||
| io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE | ||
| } | ||
| it.set(instance, setPropertyDto.value) | ||
| onSet?.invoke() | ||
| return | ||
| } ?: throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE) |
There was a problem hiding this comment.
Bug: mutability guard is a no‑op (missing throw).
If the property lacks @E2eRuntimeMutable, it still gets set. This breaks the intended safety check.
Apply this fix:
- (prop as? KMutableProperty1<Any, Any?>)?.let {
- if (!it.hasAnnotation<E2eRuntimeMutable>()) {
- io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE
- }
- it.set(instance, setPropertyDto.value)
- onSet?.invoke()
- return
- } ?: throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE)
+ (prop as? KMutableProperty1<Any, Any?>)?.let {
+ if (!it.hasAnnotation<E2eRuntimeMutable>()) {
+ throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE)
+ }
+ it.set(instance, setPropertyDto.value)
+ onSet?.invoke()
+ return
+ } ?: throw BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE)🤖 Prompt for AI Agents
In
backend/development/src/main/kotlin/io/tolgee/facade/InternalPropertiesSetterFacade.kt
around lines 28 to 35, the mutability guard currently does nothing because the
code only references the error message when the property lacks
@E2eRuntimeMutable but continues to set the property; change the logic so that
if the property does not have @E2eRuntimeMutable you throw
BadRequestException(io.tolgee.constants.Message.PROPERTY_NOT_MUTABLE) and do not
call it.set or onSet; only call it.set(instance, setPropertyDto.value) and
onSet?.invoke() when the annotation check passes.
2 participants
Summary by CodeRabbit