-
-
Notifications
You must be signed in to change notification settings - Fork 302
feat: plans migration #3216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: plans migration #3216
Conversation
WalkthroughIntroduces plan migration functionality across backend and frontend: new scheduling/date utilities, messages/i18n, validation and routing, API schemas for migration CRUD and history, UI components/forms for creating/editing migrations (cloud and self-hosted), enhanced plan selectors and lists with migration indicators, and supporting test/e2e typings. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin
participant UI as Admin UI (Create Migration)
participant API as Billing API
Admin->>UI: Open "Create Migration"
UI->>API: GET Plans (filterHasMigration, type)
API-->>UI: Plans list
Admin->>UI: Fill form (source, target, offsets, enabled)
UI->>API: POST Create Migration
API-->>UI: 201 Created (migrationId)
UI-->>Admin: Show success, navigate to Plans
sequenceDiagram
autonumber
actor Admin
participant UI as Admin UI (Edit Migration)
participant API as Billing API
Admin->>UI: Open "Edit Migration"
UI->>API: GET Migration by id
API-->>UI: Migration details
UI->>API: GET Migration Subscriptions (paged)
API-->>UI: Page 1 data
alt Save changes
Admin->>UI: Submit form
UI->>API: PUT Update Migration
API-->>UI: 200 OK
UI-->>Admin: Success, navigate back
else Delete
Admin->>UI: Delete
UI->>API: DELETE Migration
API-->>UI: 204 No Content
UI-->>Admin: Success, navigate back
end
sequenceDiagram
autonumber
participant View as Plans View
participant Chip as PlanMigratingChip
participant API as Billing API
View->>Chip: Render with {migrationId,isEnabled,planType}
Chip-->>View: Show "Migrating"/"Pending" chip
Note over Chip: On tooltip open
Chip->>API: GET Migration (by id & type)
API-->>Chip: Migration detail
Chip-->>View: Tooltip with source/target, prices, offsets, "Configure" link
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
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
72ae86a
to
bf9215a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
🧹 Nitpick comments (8)
backend/data/src/main/kotlin/io/tolgee/component/CurrentDateProvider.kt (1)
89-92
: Simplify by reusing the existingdate
property.The expression
(forcedDate ?: date)
is redundant because thedate
property (lines 84-87) already performs the null checkforcedDate ?: Date()
. Directly usedate
to 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
CronTrigger
without validation. Invalid cron expressions will throwIllegalArgumentException
at runtime. Additionally, the null check for thefuture
is good defensive programming (Spring'sTaskScheduler.schedule
can return null if the scheduler is shut down or the trigger produces no next execution time), but this pattern isn't used in the existingscheduleWithFixedDelay
function (line 44), creating an inconsistency.Recommendations:
- Validate the cron expression before constructing the
CronTrigger
to 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
customButtons
array 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
id
orkey
property. 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
colors
object 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.tsx
contain 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
, butmigration
is 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
free
status 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
verifyTimesEmailSent
method 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
LocalDate
andZoneId.systemDefault
are standard Java time API classes needed for the newlocalDate
property.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_ID
parameter 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_ID
parameter.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_FOUND
andPLAN_HAS_MIGRATIONS
follow the existing naming conventions and are correctly placed. The frontend translation mapping forplan_has_migrations
is 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
filterHasMigration
prop 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_migrations
follows the established pattern and is logically placed near the relatedplan_has_subscribers
error.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
PrivateRoute
for authentication- Have
exact
matching 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
PlanType
union type is clean, minimal, and correctly typed for the two plan variants.webapp/src/constants/GlobalValidationSchema.tsx (1)
42-49
: LGTM!The
number.min
locale configuration correctly integrates the validation message with i18n translation, consistent with the existingstring.min
andstring.max
patterns.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
PlanMigrationForm
component (imported at line 3) uses the misspelledPLAN_MIRATION_FORM
validation schema. Once the typo inGlobalValidationSchema.tsx
is fixed (as noted in that file's review), ensure this component andPlanMigrationForm
are 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
/targetPlanId
toundefined
(updateCreatePlanMigrationFormData
tonumber | undefined
) and tighten your Yup schema to use.min(1)
forplanId
.webapp/src/component/common/FullWidthTooltip.tsx (1)
1-11
: LGTM!The
FullWidthTooltip
component correctly overrides the default Tooltip maxWidth using MUI's styled API andtooltipClasses
. The pattern of spreadingclassName
topopper
is 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
Settings01
icon andPlanMigratingChip
are correctly placed and used in the component.Also applies to: 29-29
148-151
: PlanMigratingChip props match CloudPlanModel schema
CloudPlanModel
inbillingApiSchema.generated.ts
defines bothmigrationId?: number
andactiveMigration?: 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
tableHead
prop are well-structured and maintain backward compatibility.Also applies to: 11-11, 22-24
49-63
: LGTM!The internal
PaginatedHateoasTableListComponent
cleanly handles the conditional rendering of the table header. The implementation is straightforward and correct.
35-46
: Approve listComponent function usage – ThelistComponent
prop 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
filterHasMigration
prop 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.ts
definesfilterHasMigration?: boolean
alongsidefilterAssignableToOrganization
for thegetPlans
operation 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.data
is 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.data
is 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
any
in 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
free
field toGenericPlanType
and the newPlansProps
type 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
free
status only when explicitly defined- Uses
satisfies
for type safetyThe logic properly handles both filter conditions and maintains the original mapping structure.
78-78
: LGTM! ThedataCy
prop enables better test flexibility.Passing the
dataCy
prop toSearchSelect
allows customization of data-cy attributes for different plan selector instances while maintaining backward compatibility through the default value.
18-18
: Ensure allGenericPlanSelector
usages handle the newonPlanChange(plan: T)
signature.
The callback now receives the fullplan
object instead of an ID—update every handler accordingly.
9-14
: LGTM! Type definitions are well-structured.The addition of
free
toGenericPlanType
and the newPlansProps
type for filtering configuration are clear and appropriately typed.
16-25
: API change improves developer experience.The change to
onPlanChange
receiving 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 propsplanProps
anddataCy
enhance 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
dataCy
prop is correctly passed through to theSearchSelect
component, 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
activeMigration
andmigrationId
fields to bothAdministrationCloudPlanModel
andSelfHostedEePlanAdministrationModel
enables 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
CloudPlanMigrationModel
andAdministrationSelfHostedEePlanMigrationModel
have 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
: includessourcePlanId
for creating new migrationsPlanMigrationRequest
: excludessourcePlanId
for updates (source plan is immutable)The
PlanMigrationHistoryModel
provides 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:
filterHasMigration
query parameter on plan listing endpoints enables filtering by migration status- Error codes
plan_migration_not_found
andplan_has_migrations
provide 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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 tsx
Length 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
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=ts
Length of output: 252
🏁 Script executed:
#!/bin/bash
# Search for correctly spelled constant
rg -n "PLAN_MIGRATION_FORM" --type=ts
Length 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
{info?.targetPlan.prices && ( | ||
<TooltipText> | ||
<PricePrimary | ||
prices={info?.sourcePlan.prices} | ||
period={'YEARLY'} | ||
highlightColor={''} | ||
sx={{ | ||
fontSize: 14, | ||
fontWeight: 500, | ||
}} | ||
noPeriodSwitch={true} | ||
/> | ||
</TooltipText> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify prices property existence before rendering.
Line 116 checks info?.targetPlan.prices
but this same pattern checks the source plan's prices. However, on line 119 it renders info?.sourcePlan.prices
. This appears to be a copy-paste error—the condition should check sourcePlan.prices
, not targetPlan.prices
.
Apply this diff:
- {info?.targetPlan.prices && (
+ {info?.sourcePlan.prices && (
<TooltipText>
<PricePrimary
prices={info?.sourcePlan.prices}
📝 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.
{info?.targetPlan.prices && ( | |
<TooltipText> | |
<PricePrimary | |
prices={info?.sourcePlan.prices} | |
period={'YEARLY'} | |
highlightColor={''} | |
sx={{ | |
fontSize: 14, | |
fontWeight: 500, | |
}} | |
noPeriodSwitch={true} | |
/> | |
</TooltipText> | |
)} | |
{info?.sourcePlan.prices && ( | |
<TooltipText> | |
<PricePrimary | |
prices={info?.sourcePlan.prices} | |
period={'YEARLY'} | |
highlightColor={''} | |
sx={{ | |
fontSize: 14, | |
fontWeight: 500, | |
}} | |
noPeriodSwitch={true} | |
/> | |
</TooltipText> | |
)} |
🤖 Prompt for AI Agents
In webapp/src/ee/billing/component/Plan/PlanMigratingChip.tsx around lines 116
to 129, the conditional is checking info?.targetPlan.prices but the rendered
PricePrimary uses info?.sourcePlan.prices; change the condition to verify
info?.sourcePlan.prices (or both source and target if intended) so the component
only renders when the source plan prices exist, using consistent optional
chaining (e.g., info?.sourcePlan?.prices).
| "unsupported_media_type" | ||
| "plan_migration_not_found" | ||
| "plan_has_migrations"; | ||
params?: unknown[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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_found
exists in your translation JSON (or adjust the mapping key) in en.json and all locale files. - Verify any other switch/if handling
ErrorResponseTyped.code
includes bothplan_migration_not_found
andplan_has_migrations
.
| "unsupported_media_type" | ||
| "plan_migration_not_found" | ||
| "plan_has_migrations"; | ||
params?: unknown[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 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' webapp
Length 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.tsx
Length of output: 844
Add missing translation mappings for new error codes
- Add cases for
"unsupported_media_type"
and"plan_migration_not_found"
inuseErrorTranslation.ts
sotranslateError
handles the new messages. - Add corresponding keys for
unsupported_media_type
andplan_migration_not_found
in 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot! ❤️
Would be nice if we kept the frontend components smaller.
|
||
val localDate: LocalDate | ||
get() { | ||
return (forcedDate ?: date).toInstant().atZone(systemDefault()).toLocalDate() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not simply date
instead of (forcedDate ?: date)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
{t('administration_plan_migration_configure_existing')} | ||
</Typography> | ||
</Box> | ||
<EditPlanMigrationForm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component is kind of long. I would extract this EditPlanMigrationForm to separate CloudPlanEditPlanMigrationForm
, this would enable us to move also the hooks from top saving many lines from this component. Also submit and onDelete methods will be closer to their usage.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
} | ||
`; | ||
|
||
export const PlanMigratingChip = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This component is very large and should be split into multiple smaller ones.
Also, I consider providing the plan type an anti-pattern. Instead, I recommend creating separate components for SelfHosted and Cloud and another component props for everything which differs to follow "open-closed principle" better.
Summary by CodeRabbit
New Features
Improvements
Internationalization