feat(condo): DOMA-12907 update subscription tooltip content#7311
feat(condo): DOMA-12907 update subscription tooltip content#7311Alexander-Turkin merged 9 commits intomainfrom
Conversation
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughIntroduces SubscriptionFeatureGuard and useActivateSubscriptions; replaces many NoSubscriptionTooltip usages with the guard; enhances NoSubscriptionTooltip with dynamic plan/trial activation logic; consolidates subscription activation flows and propagates activation loading to subscription plan UI. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Component (e.g., PlanCard / Button)
participant Guard as SubscriptionFeatureGuard
participant Tooltip as NoSubscriptionTooltip (fallback)
participant Hook as useActivateSubscriptions
participant API as GraphQL API
User->>UI: click "Activate trial" / AI action
UI->>Guard: render with feature prop
alt feature available
Guard->>UI: render children (interactive)
UI->>Hook: handleActivatePlan(params)
Hook->>API: activateSubscriptionPlan mutation
API-->>Hook: activation result
Hook-->>UI: success / error, refetches
else feature unavailable
Guard->>Tooltip: render fallback UI (disabled + CTA)
Tooltip->>User: show upgrade/try trial actions
User->>Tooltip: click "Try trial" or "View plans"
Tooltip->>Hook: (optional) handleActivatePlan or navigate
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Confidence Score: 2/5
Last reviewed commit: 6d9cad4 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| console.log(1, featureProp) | ||
| return children | ||
| } | ||
|
|
||
| if (path && !requiredFeature) { | ||
| console.log(2, featureProp) | ||
|
|
||
| return children | ||
| } | ||
|
|
||
| if (path && !hasSubscription) { | ||
| console.log(3, featureProp) | ||
|
|
||
| return children | ||
| } | ||
|
|
||
| if (recentlyActivatedTrialPlan) { | ||
| console.log(4, featureProp) | ||
|
|
||
| return ( | ||
| <Tour.Provider> | ||
| <TrialActivatedTooltip | ||
| planName={recentlyActivatedTrialPlan} | ||
| placement={placement} | ||
| onClose={handleClearRecentlyActivatedTrial} | ||
| getPopupContainer={getPopupContainer} | ||
| > | ||
| {children} | ||
| </TrialActivatedTooltip> | ||
| </Tour.Provider> | ||
| ) | ||
| } | ||
|
|
||
| if (isAvailable) { | ||
| console.log(5, featureProp) | ||
| return children | ||
| } | ||
| console.log(6, featureProp) |
There was a problem hiding this comment.
Debug console.log statements left in production code
Six console.log calls (numbered 1–6) were left in the component's render path. These will flood the browser console in production on every render of any NoSubscriptionTooltip or FeatureGate instance, which now covers a large portion of the application's UI. They should be removed before merging.
| console.log(1, featureProp) | |
| return children | |
| } | |
| if (path && !requiredFeature) { | |
| console.log(2, featureProp) | |
| return children | |
| } | |
| if (path && !hasSubscription) { | |
| console.log(3, featureProp) | |
| return children | |
| } | |
| if (recentlyActivatedTrialPlan) { | |
| console.log(4, featureProp) | |
| return ( | |
| <Tour.Provider> | |
| <TrialActivatedTooltip | |
| planName={recentlyActivatedTrialPlan} | |
| placement={placement} | |
| onClose={handleClearRecentlyActivatedTrial} | |
| getPopupContainer={getPopupContainer} | |
| > | |
| {children} | |
| </TrialActivatedTooltip> | |
| </Tour.Provider> | |
| ) | |
| } | |
| if (isAvailable) { | |
| console.log(5, featureProp) | |
| return children | |
| } | |
| console.log(6, featureProp) | |
| if (skipTooltip) { | |
| return children | |
| } | |
| if (path && !requiredFeature) { | |
| return children | |
| } | |
| if (path && !hasSubscription) { | |
| return children | |
| } | |
| if (recentlyActivatedTrialPlan) { | |
| return ( |
All six occurrences must be removed: lines 179, 184, 190, 196, 213, and 216.
| await handleActivatePlan({ | ||
| priceId: price.id, | ||
| isTrial: true, | ||
| planName, | ||
| trialDays: bestPlanWithFeature.plan?.trialDays || 0, | ||
| isCustomPrice: false, | ||
| }) | ||
|
|
||
| if (id) { | ||
| sessionStorage.setItem(TRIAL_POPUP_PLAN_KEY, planName) | ||
| sessionStorage.setItem(TRIAL_POPUP_INSTANCE_KEY, id) | ||
| } | ||
| setRecentlyActivatedTrialPlan(planName) | ||
| } finally { | ||
| setIsActivating(false) | ||
| } |
There was a problem hiding this comment.
Activation failure silently treated as success
handleActivatePlan (from useActivateSubscriptions) catches all errors internally and never re-throws them. This means whether the server call succeeds or fails, execution continues to lines 136–140 where sessionStorage is written and recentlyActivatedTrialPlan is set to a non-null value.
As a consequence, if the API call fails:
- The "trial activated" welcome tooltip will still be shown to the user.
- The
sessionStoragekeys will be set, so the tooltip will also re-appear on the next page load for that sameid.
The fix is for handleActivatePlan to return a boolean (or re-throw) so that the caller can distinguish success from failure, and only set the success state when the activation truly succeeded.
| const hasFeature = Array.isArray(feature) | ||
| ? feature.every(f => isFeatureAvailable(f)) | ||
| : isFeatureAvailable(feature) |
There was a problem hiding this comment.
isFeatureAvailable called with undefined returns false
When feature prop is undefined (e.g. TourStepCard passes feature={requiredFeature} and requiredFeature may be undefined), calling isFeatureAvailable(undefined) inside useOrganizationSubscription constructs the key "undefinedEndAt", which is absent from subscriptionFeatures, causing the function to return false.
With subscriptions enabled this means hasFeature is false for any FeatureGate that omits the feature prop, so the fallback (disabled/locked content) is rendered instead of children. In TourStepCard both render identical content, masking the issue — but this is coincidental, and any future FeatureGate omitting feature without skipTooltip will silently display the locked state.
A short guard at the top of the hasFeature evaluation would fix this:
const hasFeature = !feature
? true
: Array.isArray(feature)
? feature.every(f => isFeatureAvailable(f))
: isFeatureAvailable(feature)![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 595531f2a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (id) { | ||
| sessionStorage.setItem(TRIAL_POPUP_PLAN_KEY, planName) | ||
| sessionStorage.setItem(TRIAL_POPUP_INSTANCE_KEY, id) | ||
| } | ||
| setRecentlyActivatedTrialPlan(planName) |
There was a problem hiding this comment.
This block unconditionally persists trial popup state and shows the welcome tooltip after handleActivatePlan, but useActivateSubscriptions catches activation errors and does not rethrow. In failure scenarios (network error, backend rejection), users still see a successful trial-activation UI even though activation did not happen, which is misleading and can mask real failures.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/common/components/Comments/CommentForm.tsx`:
- Line 412: Guard the getPopupContainer callback against a null ref: currently
getPopupContainer={() => commentsContainerRef.current} can throw if
commentsContainerRef is null; update the callback used in CommentForm (the
getPopupContainer prop) to check commentsContainerRef and
commentsContainerRef.current (e.g., use optional chaining or a conditional) and
return a safe fallback (like document.body or null) when the ref is not set so
the function never dereferences a null ref.
In `@apps/condo/domains/subscription/components/FeatureGate.tsx`:
- Around line 6-10: Change the FeatureGateProps so that the feature prop is
required: update the interface FeatureGateProps (currently extends
Omit<NoSubscriptionTooltipProps, 'children'>) to explicitly declare feature:
AvailableFeature (non-optional) and ensure any usages respect the new type;
locate where FeatureGate calls isFeatureAvailable(feature) (the FeatureGate
component) and ensure feature cannot be undefined at call-site. Then update
callers like TourStepCard.tsx to only render FeatureGate when requiredFeature is
defined (or otherwise provide a valid AvailableFeature) instead of relying on
skipTooltip workarounds.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 178-216: Remove the debugging console.log statements from the
NoSubscriptionTooltip component render branches (the console.log calls near the
checks for skipTooltip, path && !requiredFeature, path && !hasSubscription,
recentlyActivatedTrialPlan, isAvailable, and the final fallback). Locate the
checks using the symbols skipTooltip, path, requiredFeature, hasSubscription,
recentlyActivatedTrialPlan, isAvailable, children, and
handleClearRecentlyActivatedTrial and delete the console.log(...) lines so the
component returns only the JSX/children without spamming the console in normal
usage.
- Around line 34-35: NoSubscriptionTooltip currently mounts
useActivateSubscriptions (and also calls useOrganizationSubscription) for every
tooltip instance which creates many unnecessary observers; remove the direct
calls to useActivateSubscriptions/useOrganizationSubscription from
NoSubscriptionTooltip and instead accept the needed values and callbacks as
props (trialSubscriptions, activatedSubscriptions, handleActivatePlan,
activateLoading, isFeatureAvailable, hasSubscription) or wire them through a
shared provider/context created in a higher-level component so the subscription
queries are executed once; alternatively, lazily initialize the activation hook
only when the tooltip is actually opened (e.g., on first open) by moving hook
usage into a parent that conditionally renders the activation UI or into an
onOpen handler.
- Around line 219-223: The default Tooltip render in the NoSubscriptionTooltip
component is ignoring the getPopupContainer prop; update the JSX that renders
<Tooltip title={tooltipTitle} placement={placement} zIndex={1110}> to also pass
getPopupContainer={getPopupContainer} (using the getPopupContainer prop declared
on the NoSubscriptionTooltip component) so the tooltip respects custom container
behavior in constrained layouts/modals.
In `@apps/condo/lang/es/es.json`:
- Around line 3160-3161: Change the copy for the keys
"subscription.trial.welcome.title" and "subscription.trial.welcome.description"
to use a neutral/informal, consistent tone: replace the gendered "Bienvenido"
with a neutral phrasing (e.g., "Te damos la bienvenida al plan «{planName}»")
and switch the description from formal "usted" to informal "tú" (e.g., "Junto
con tu período de prueba del plan «{planName}», hemos desbloqueado esta función
y más para ti. Prueba cómo simplifica tus tareas.") so both strings match the
surrounding UI tone.
In `@apps/condo/pages/_app.tsx`:
- Around line 384-386: The inline featureTooltip render currently passes the
tooltip content via the explicit children prop to NoSubscriptionTooltip; change
it to use JSX children syntax instead: in the featureTooltip function (symbols:
featureTooltip, element, placement, item.path, NoSubscriptionTooltip) remove
children={element} and place element between the opening and closing
NoSubscriptionTooltip tags, keeping the path and placement props as-is.
In `@apps/condo/pages/incident/`[id]/index.tsx:
- Around line 638-654: The Button components are passing their content via the
children prop which violates the lint rule; update both Button instances (the
disabled one and the interactive one that uses generateNewsLoading,
incidentClassifierIncidentLoading, incidentPropertiesLoading and
onClick={handleGenerateNews}) to remove the children prop and instead place
GenerateNewsLabel as the button's inner JSX content while preserving all other
props (id='generateNews', type='secondary', disabled, loading, onClick).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8325b66f-d52f-4d63-9290-bbd9555044f2
📒 Files selected for processing (20)
apps/condo/domains/common/components/Comments/CommentForm.tsxapps/condo/domains/common/components/Comments/CommentsTabContent.tsxapps/condo/domains/onboarding/components/TourPage/TourStepCard.tsxapps/condo/domains/subscription/components/FeatureGate.tsxapps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionPlanCard/SubscriptionPlanCard.tsxapps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionSettingsContent.tsxapps/condo/domains/subscription/components/TrialActivatedTooltip.tsxapps/condo/domains/subscription/components/index.tsapps/condo/domains/subscription/hooks/index.tsapps/condo/domains/subscription/hooks/useActivateSubscriptions.tsxapps/condo/domains/ticket/components/BaseTicketForm/index.tsxapps/condo/domains/ticket/components/IncidentForm/BaseIncidentForm.tsxapps/condo/domains/ticket/components/IncidentForm/CreateIncidentForm.tsxapps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsxapps/condo/lang/en/en.jsonapps/condo/lang/es/es.jsonapps/condo/lang/ru/ru.jsonapps/condo/pages/_app.tsxapps/condo/pages/incident/[id]/index.tsx
| const { trialSubscriptions, activatedSubscriptions, handleActivatePlan, activateLoading } = useActivateSubscriptions() | ||
| const { isFeatureAvailable, hasSubscription } = useOrganizationSubscription() |
There was a problem hiding this comment.
Avoid mounting activation-data hooks in every tooltip instance.
Line 34 runs useActivateSubscriptions() inside NoSubscriptionTooltip. Since this component is used per menu item in apps/condo/pages/_app.tsx (Lines 379-399), this multiplies subscription-related query observers and refetch pressure on every page render. Move this data flow to a shared parent/provider or lazily enable it only when trial activation UI is actually needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around
lines 34 - 35, NoSubscriptionTooltip currently mounts useActivateSubscriptions
(and also calls useOrganizationSubscription) for every tooltip instance which
creates many unnecessary observers; remove the direct calls to
useActivateSubscriptions/useOrganizationSubscription from NoSubscriptionTooltip
and instead accept the needed values and callbacks as props (trialSubscriptions,
activatedSubscriptions, handleActivatePlan, activateLoading, isFeatureAvailable,
hasSubscription) or wire them through a shared provider/context created in a
higher-level component so the subscription queries are executed once;
alternatively, lazily initialize the activation hook only when the tooltip is
actually opened (e.g., on first open) by moving hook usage into a parent that
conditionally renders the activation UI or into an onOpen handler.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx
Outdated
Show resolved
Hide resolved
| <Tooltip | ||
| title={tooltipTitle} | ||
| placement={placement} | ||
| zIndex={1110} | ||
| > |
There was a problem hiding this comment.
Forward getPopupContainer in the default tooltip path.
getPopupContainer is accepted in props (Line 26) but ignored in the normal <Tooltip /> render (Line 219), which can break expected container behavior in constrained layouts/modals.
💡 Suggested fix
return (
<Tooltip
title={tooltipTitle}
placement={placement}
+ getPopupContainer={getPopupContainer}
zIndex={1110}
>
{children}
</Tooltip>
)📝 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.
| <Tooltip | |
| title={tooltipTitle} | |
| placement={placement} | |
| zIndex={1110} | |
| > | |
| return ( | |
| <Tooltip | |
| title={tooltipTitle} | |
| placement={placement} | |
| getPopupContainer={getPopupContainer} | |
| zIndex={1110} | |
| > | |
| {children} | |
| </Tooltip> | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around
lines 219 - 223, The default Tooltip render in the NoSubscriptionTooltip
component is ignoring the getPopupContainer prop; update the JSX that renders
<Tooltip title={tooltipTitle} placement={placement} zIndex={1110}> to also pass
getPopupContainer={getPopupContainer} (using the getPopupContainer prop declared
on the NoSubscriptionTooltip component) so the tooltip respects custom container
behavior in constrained layouts/modals.
apps/condo/lang/es/es.json
Outdated
| "subscription.trial.welcome.title": "Bienvenido al plan «{planName}»", | ||
| "subscription.trial.welcome.description": "Junto con el período de prueba del plan «{planName}», hemos desbloqueado esta función y más para usted. Pruebe cómo simplifica sus tareas.", |
There was a problem hiding this comment.
Consider neutral + consistent tone in Spanish copy.
Line 3160 uses gendered “Bienvenido” and Line 3161 switches to formal “usted”, while nearby UI text mostly uses informal “tú”. Consider neutral/informal wording for consistency.
✍️ Suggested copy tweak
- "subscription.trial.welcome.title": "Bienvenido al plan «{planName}»",
- "subscription.trial.welcome.description": "Junto con el período de prueba del plan «{planName}», hemos desbloqueado esta función y más para usted. Pruebe cómo simplifica sus tareas.",
+ "subscription.trial.welcome.title": "Te damos la bienvenida al plan «{planName}»",
+ "subscription.trial.welcome.description": "Con el período de prueba del plan «{planName}», hemos desbloqueado esta función y más para ti. Prueba cómo simplifica tus tareas.",📝 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.
| "subscription.trial.welcome.title": "Bienvenido al plan «{planName}»", | |
| "subscription.trial.welcome.description": "Junto con el período de prueba del plan «{planName}», hemos desbloqueado esta función y más para usted. Pruebe cómo simplifica sus tareas.", | |
| "subscription.trial.welcome.title": "Te damos la bienvenida al plan «{planName}»", | |
| "subscription.trial.welcome.description": "Con el período de prueba del plan «{planName}», hemos desbloqueado esta función y más para ti. Prueba cómo simplifica tus tareas.", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/lang/es/es.json` around lines 3160 - 3161, Change the copy for the
keys "subscription.trial.welcome.title" and
"subscription.trial.welcome.description" to use a neutral/informal, consistent
tone: replace the gendered "Bienvenido" with a neutral phrasing (e.g., "Te damos
la bienvenida al plan «{planName}»") and switch the description from formal
"usted" to informal "tú" (e.g., "Junto con tu período de prueba del plan
«{planName}», hemos desbloqueado esta función y más para ti. Prueba cómo
simplifica tus tareas.") so both strings match the surrounding UI tone.
| <Button | ||
| disabled | ||
| type='secondary' | ||
| children={GenerateNewsLabel} | ||
| id='generateNews' | ||
| /> | ||
| </div> | ||
| } | ||
| > | ||
| <Button | ||
| disabled={generateNewsLoading || incidentClassifierIncidentLoading || incidentPropertiesLoading} | ||
| loading={generateNewsLoading} | ||
| type='secondary' | ||
| children={GenerateNewsLabel} | ||
| onClick={handleGenerateNews} | ||
| id='generateNews' | ||
| /> |
There was a problem hiding this comment.
Replace children prop usage on Button to satisfy lint rule
Line 641 and Line 651 pass button content through children prop, which violates the active Biome rule and will fail checks.
Suggested fix
<Button
disabled
type='secondary'
- children={GenerateNewsLabel}
id='generateNews'
- />
+ >
+ {GenerateNewsLabel}
+ </Button>
@@
<Button
disabled={generateNewsLoading || incidentClassifierIncidentLoading || incidentPropertiesLoading}
loading={generateNewsLoading}
type='secondary'
- children={GenerateNewsLabel}
onClick={handleGenerateNews}
id='generateNews'
- />
+ >
+ {GenerateNewsLabel}
+ </Button>🧰 Tools
🪛 Biome (2.4.4)
[error] 641-641: Avoid passing children using a prop
(lint/correctness/noChildrenProp)
[error] 651-651: Avoid passing children using a prop
(lint/correctness/noChildrenProp)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/pages/incident/`[id]/index.tsx around lines 638 - 654, The Button
components are passing their content via the children prop which violates the
lint rule; update both Button instances (the disabled one and the interactive
one that uses generateNewsLoading, incidentClassifierIncidentLoading,
incidentPropertiesLoading and onClick={handleGenerateNews}) to remove the
children prop and instead place GenerateNewsLabel as the button's inner JSX
content while preserving all other props (id='generateNews', type='secondary',
disabled, loading, onClick).
| id={`tour-step-${step.type}`} | ||
| skipTooltip={requiredFeature === undefined} | ||
| fallback={ | ||
| <div style={{ width: '100%' }}> |
| </div> | ||
| } | ||
| > | ||
| <div style={{ width: '100%' }}> |
| }, []) | ||
|
|
||
| if (skipTooltip) { | ||
| console.log(1, featureProp) |
|
|
||
| setActivateLoading(true) | ||
| try { | ||
| await activateSubscriptionPlan({ |
There was a problem hiding this comment.
too complex conditions in try block
better to simplify logic
| placement='left' | ||
| fallback={ | ||
| <div> | ||
| <Col style={{ cursor: 'not-allowed' }}> |
| </div> | ||
| } | ||
| > | ||
| <Col style={{ cursor: 'pointer' }} onClick={() => setCreateInvoiceModalOpen(true)}> |
There was a problem hiding this comment.
same here
better to move all styles to the file
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
| const TryTrialButton = useMemo(() => { | ||
| const currencyCode = bestPlanWithFeature?.prices?.[0]?.currencyCode | ||
| return intl.formatMessage({ | ||
| id: 'subscription.warns.tryTrialButton', | ||
| }, { currency: CURRENCY_SYMBOLS[currencyCode] || '' }) | ||
| }, [bestPlanWithFeature, intl]) | ||
|
|
||
| const handleActivateTrial = useCallback(async () => { | ||
| if (!bestPlanWithFeature) { | ||
| await router.push(`/settings?tab=${SETTINGS_TAB_SUBSCRIPTION}`) | ||
| return | ||
| } | ||
|
|
||
| const price = bestPlanWithFeature.prices?.[0] | ||
| if (!price?.id) return | ||
|
|
||
| setIsActivating(true) | ||
| try { | ||
| const planName = bestPlanWithFeature.plan?.name || '' | ||
|
|
||
| await handleActivatePlan({ | ||
| priceId: price.id, | ||
| isTrial: true, | ||
| planName, | ||
| trialDays: bestPlanWithFeature.plan?.trialDays || 0, | ||
| isCustomPrice: false, | ||
| }) | ||
|
|
||
| if (id) { | ||
| sessionStorage.setItem(TRIAL_POPUP_PLAN_KEY, planName) | ||
| sessionStorage.setItem(TRIAL_POPUP_INSTANCE_KEY, id) | ||
| } | ||
| setRecentlyActivatedTrialPlan(planName) | ||
| } finally { | ||
| setIsActivating(false) | ||
| } | ||
| }, [bestPlanWithFeature, handleActivatePlan, router, id]) | ||
|
|
||
| const handleViewPlans = useCallback(async () => { | ||
| await router.push(`/settings?tab=${SETTINGS_TAB_SUBSCRIPTION}`) | ||
| }, [router]) | ||
|
|
||
| const isLoading = activateLoading || isActivating | ||
|
|
||
| const buttonText = hasActivatedAnyTrial ? ViewPlansButton : TryTrialButton | ||
|
|
||
| const buttonAction = !feature || hasActivatedAnyTrial ? handleViewPlans : handleActivateTrial |
There was a problem hiding this comment.
Button text shows "Try for 0" but action navigates to plans page
When bestPlanWithFeature is null (no trial plan is available for the required feature) and the user hasn't activated any trial yet, the button text is computed as the TryTrialButton message (e.g. "Try for 0" with an empty currency because bestPlanWithFeature?.prices?.[0]?.currencyCode is undefined), but buttonAction is set to handleActivateTrial. When handleActivateTrial is invoked it immediately falls into the !bestPlanWithFeature branch and navigates to /settings?tab=subscription.
The result is a user-visible inconsistency: the button says "Try for 0" but actually redirects to the plans page — identical to the "View Plans" / "Upgrade" flow.
The button text (and action) should also account for whether bestPlanWithFeature is available:
const buttonText = hasActivatedAnyTrial || !bestPlanWithFeature
? ViewPlansButton
: TryTrialButton
const buttonAction = !feature || hasActivatedAnyTrial || !bestPlanWithFeature
? handleViewPlans
: handleActivateTrial| const { trialSubscriptions, activatedSubscriptions, handleActivatePlan, activateLoading } = useActivateSubscriptions() | ||
| const { isFeatureAvailable, hasSubscription } = useOrganizationSubscription() | ||
| const [isActivating, setIsActivating] = useState(false) | ||
| const [recentlyActivatedTrialPlan, setRecentlyActivatedTrialPlan] = useState<string | null>(null) | ||
|
|
||
| useEffect(() => { | ||
| if (!id) return | ||
|
|
||
| const storedPlan = sessionStorage.getItem(TRIAL_POPUP_PLAN_KEY) | ||
| const storedInstance = sessionStorage.getItem(TRIAL_POPUP_INSTANCE_KEY) | ||
|
|
||
| if (storedPlan && storedInstance === id) { | ||
| setRecentlyActivatedTrialPlan(storedPlan) | ||
| } | ||
| }, [id]) | ||
|
|
||
| const requiredFeature = path ? getRequiredFeature(path) : null | ||
| const feature = (featureProp || requiredFeature) as AvailableFeature | undefined | null | ||
|
|
||
| const NoSubscriptionWarning = intl.formatMessage({ | ||
| const FeatureLockedMessage = intl.formatMessage({ | ||
| id: 'subscription.warns.noActiveSubscription', | ||
| }) | ||
| const ActivateSubscriptionButton = intl.formatMessage({ | ||
| const UpgradePlanMessage = intl.formatMessage({ | ||
| id: 'subscription.warns.upgradePlan', | ||
| }) | ||
| const ViewPlansButton = intl.formatMessage({ | ||
| id: 'subscription.warns.activateSubscriptionButton', | ||
| }) | ||
|
|
||
| const handleActivateClick = useCallback(async () => { | ||
| const { data: plansData } = useGetAvailableSubscriptionPlansQuery({ | ||
| variables: { | ||
| organization: { id: organization?.id }, | ||
| }, | ||
| skip: !organization?.id, | ||
| }) |
There was a problem hiding this comment.
Duplicate GraphQL queries fired per component instance
NoSubscriptionTooltip (and by extension every FeatureGate) independently calls both useActivateSubscriptions() (which issues three GraphQL queries — trial subscriptions, pending requests, activated subscriptions) and useGetAvailableSubscriptionPlansQuery. With NoSubscriptionTooltip now wrapping every menu item in _app.tsx (10+ simultaneous instances) and multiple FeatureGate instances rendered per page, the same set of queries is watched N times for the same organization ID.
Apollo Client will deduplicate in-flight network requests, but it still creates N separate query subscriptions in the cache. This is wasteful and becomes increasingly expensive as more FeatureGate usages are added.
Consider lifting the shared data (trialSubscriptions, activatedSubscriptions, pendingRequests, and available plans) into a React context (e.g. SubscriptionContext) that is fetched once near the root, and consuming that context inside NoSubscriptionTooltip and FeatureGate instead of calling useActivateSubscriptions() and useGetAvailableSubscriptionPlansQuery on every instance.
| const featureTooltip = ({ element, placement }) => ( | ||
| <NoSubscriptionTooltip path={`/${item.path}`} children={element} placement={placement} /> | ||
| ) |
There was a problem hiding this comment.
featureTooltip decorator recreated on every render
featureTooltip is defined inside the category.items.map() callback, so a new function reference is produced on every render of MenuItems. If MenuItem performs a reference-equality check on toolTipDecorator (e.g. via React.memo), all menu items will re-render on every parent render even when nothing relevant changed.
Consider memoizing the decorator outside of the map, or using useCallback:
const featureTooltip = useCallback(
(path: string) => ({ element, placement }: { element: React.ReactElement, placement: string }) => (
<NoSubscriptionTooltip path={path} placement={placement}>{element}</NoSubscriptionTooltip>
),
[]
)
// then inside the map:
toolTipDecorator={featureTooltip(`/${item.path}`)}| const isAvailable = feature | ||
| ? Array.isArray(feature) | ||
| ? feature.every(f => isFeatureAvailable(f)) | ||
| : isFeatureAvailable(feature) | ||
| : false |
There was a problem hiding this comment.
isAvailable defaults to false when feature is null/undefined
The fallback branch false means that if NoSubscriptionTooltip is ever rendered without either a feature prop or a path that resolves to a feature, isAvailable will be false and the upgrade tooltip will always be shown (assuming neither skipTooltip nor the path-guarded early returns fire). This is a footgun for any future caller that wraps content without a feature prop and without skipTooltip.
A safer default would be true (treat absence of a feature requirement as "available"):
const isAvailable = feature
? Array.isArray(feature)
? feature.every(f => isFeatureAvailable(f))
: isFeatureAvailable(feature)
: true
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx (2)
210-214:⚠️ Potential issue | 🟡 MinorForward
getPopupContainerin the default<Tooltip />branch.The prop is accepted on Line 26 but dropped in the normal tooltip render path.
Suggested fix
return ( <Tooltip title={tooltipTitle} placement={placement} + getPopupContainer={getPopupContainer} zIndex={1110} > {children} </Tooltip> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around lines 210 - 214, The default Tooltip render branch in NoSubscriptionTooltip drops the getPopupContainer prop even though it’s accepted earlier; update the Tooltip invocation (the <Tooltip ...> that uses title={tooltipTitle}, placement={placement}, zIndex={1110}) to also pass getPopupContainer={getPopupContainer} (preserving the existing prop name) so the component forwards the popup container when rendering the normal tooltip path.
34-35:⚠️ Potential issue | 🟠 MajorAvoid mounting activation/subscription hooks in each tooltip instance.
Line 34 initializes
useActivateSubscriptions()insideNoSubscriptionTooltip, which can multiply query observers/refetch pressure when many tooltip instances are rendered (e.g., menu trees). Lift this state to a shared parent/provider and pass down derived props.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around lines 34 - 35, NoSubscriptionTooltip currently calls useActivateSubscriptions() and useOrganizationSubscription() directly (providing trialSubscriptions, activatedSubscriptions, handleActivatePlan, activateLoading, isFeatureAvailable, hasSubscription) which causes many hook instances; lift these hooks up into a shared parent or provider, derive the minimal props needed (e.g., trialSubscriptions, activatedSubscriptions flags, a single handleActivatePlan callback, activateLoading boolean, and feature/subscription booleans) and pass them into NoSubscriptionTooltip as props, then remove the useActivateSubscriptions/useOrganizationSubscription calls from NoSubscriptionTooltip so tooltip instances only consume the passed props instead of mounting query observers themselves.
🧹 Nitpick comments (1)
apps/condo/domains/subscription/hooks/useActivateSubscriptions.tsx (1)
10-16: Prefer atypealias for this params shape.
ActivatePlanParamsis a plain object contract, so a type alias is preferred in this codebase.Suggested refactor
-interface ActivatePlanParams { +type ActivatePlanParams = { priceId: string isTrial?: boolean planName?: string trialDays?: number isCustomPrice?: boolean -} +}Based on learnings: "In TypeScript files across the repository, prefer using type aliases (type X = { ... }) over interface declarations for object shapes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/hooks/useActivateSubscriptions.tsx` around lines 10 - 16, The ActivatePlanParams declaration is an interface used as a plain object contract; replace it with a type alias to match the repository convention. Locate the ActivatePlanParams symbol in useActivateSubscriptions.tsx and change the declaration from "interface ActivatePlanParams { ... }" to "type ActivatePlanParams = { ... }", preserving all property names and optional markers (priceId, isTrial?, planName?, trialDays?, isCustomPrice?). Ensure any imports/uses of ActivatePlanParams elsewhere remain valid after the swap.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 1-13: Reorder the imports in NoSubscriptionTooltip to follow the
repository grouping and alphabetical rules: place external imports first (e.g.,
React and next/router: React, useRouter), then `@open-condo` imports (e.g.,
useIntl, useOrganization, Button, Space, Tooltip, Tour, Typography), then
internal imports (move useGetAvailableSubscriptionPlansQuery and other
`@app/`@condo domain imports like CURRENCY_SYMBOLS, SETTINGS_TAB_SUBSCRIPTION,
TrialActivatedTooltip, getRequiredFeature, useActivateSubscriptions,
useOrganizationSubscription). Within each group alphabetize case-insensitively
(so React before useRouter, Button before Space, CURRENCY_SYMBOLS before
SETTINGS_TAB_SUBSCRIPTION, etc.) and ensure blank lines separate the groups.
- Around line 22-23: The placement prop on NoSubscriptionTooltip is too narrow;
update NoSubscriptionTooltipProps (and the placement prop in the
NoSubscriptionTooltip component) to accept Ant Design's TooltipPlacement type so
it matches INoOrganizationToolTipWrapper and callers like featureTooltip that
may pass 'topLeft'/'bottomRight' etc.; locate the placement declarations in
NoSubscriptionTooltipProps and the component signature and replace the union
'top'|'bottom'|'left'|'right' with TooltipPlacement (importing it from 'antd' if
missing) so the types align across INoOrganizationToolTipWrapper,
NoSubscriptionTooltipProps, and featureTooltip.
In `@apps/condo/domains/subscription/hooks/useActivateSubscriptions.tsx`:
- Around line 1-9: Reorder the imports in useActivateSubscriptions.tsx so they
follow the project's group ordering (builtin → external → `@open-condo` → internal
→ sibling → parent) with blank lines between groups and alphabetized
case-insensitively within each group; specifically move the internal import of
useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery,
useGetPendingSubscriptionRequestsQuery,
useGetOrganizationActivatedSubscriptionsQuery (from '@app/condo/gql') below the
`@open-condo` imports (getClientSideSenderInfo, useIntl, useOrganization,
Typography) and ensure external imports (notification from 'antd',
useCallback/useState from 'react') are placed before the `@open-condo` group, all
alphabetized.
In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx`:
- Around line 237-242: The Add Invoice trigger is only a clickable Col (the
element with class styles.cursorPointer and onClick={() =>
setCreateInvoiceModalOpen(true)}), so keyboard users cannot activate it; change
it to be keyboard-accessible by replacing or enhancing that Col: either render a
semantic interactive element (e.g., a button or antd Button/Link) or add
role="button", tabIndex={0} and an onKeyDown handler that triggers
setCreateInvoiceModalOpen(true) on Enter/Space, and ensure the PlusCircle and
Typography.Text (AddInvoiceMessage) remain visually the same and that aria-label
or accessible text is present for screen readers.
---
Duplicate comments:
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 210-214: The default Tooltip render branch in
NoSubscriptionTooltip drops the getPopupContainer prop even though it’s accepted
earlier; update the Tooltip invocation (the <Tooltip ...> that uses
title={tooltipTitle}, placement={placement}, zIndex={1110}) to also pass
getPopupContainer={getPopupContainer} (preserving the existing prop name) so the
component forwards the popup container when rendering the normal tooltip path.
- Around line 34-35: NoSubscriptionTooltip currently calls
useActivateSubscriptions() and useOrganizationSubscription() directly (providing
trialSubscriptions, activatedSubscriptions, handleActivatePlan, activateLoading,
isFeatureAvailable, hasSubscription) which causes many hook instances; lift
these hooks up into a shared parent or provider, derive the minimal props needed
(e.g., trialSubscriptions, activatedSubscriptions flags, a single
handleActivatePlan callback, activateLoading boolean, and feature/subscription
booleans) and pass them into NoSubscriptionTooltip as props, then remove the
useActivateSubscriptions/useOrganizationSubscription calls from
NoSubscriptionTooltip so tooltip instances only consume the passed props instead
of mounting query observers themselves.
---
Nitpick comments:
In `@apps/condo/domains/subscription/hooks/useActivateSubscriptions.tsx`:
- Around line 10-16: The ActivatePlanParams declaration is an interface used as
a plain object contract; replace it with a type alias to match the repository
convention. Locate the ActivatePlanParams symbol in useActivateSubscriptions.tsx
and change the declaration from "interface ActivatePlanParams { ... }" to "type
ActivatePlanParams = { ... }", preserving all property names and optional
markers (priceId, isTrial?, planName?, trialDays?, isCustomPrice?). Ensure any
imports/uses of ActivatePlanParams elsewhere remain valid after the swap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f7999b3f-ab7b-4acb-ad3d-80d3921a2b66
📒 Files selected for processing (7)
apps/condo/domains/onboarding/components/TourPage/TourStepCard.module.cssapps/condo/domains/onboarding/components/TourPage/TourStepCard.tsxapps/condo/domains/subscription/components/FeatureGate.tsxapps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/subscription/hooks/useActivateSubscriptions.tsxapps/condo/domains/ticket/components/BaseTicketForm/BaseTicketForm.module.cssapps/condo/domains/ticket/components/BaseTicketForm/index.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/condo/domains/onboarding/components/TourPage/TourStepCard.module.css
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/condo/domains/subscription/components/FeatureGate.tsx
| import { useGetAvailableSubscriptionPlansQuery } from '@app/condo/gql' | ||
| import { useRouter } from 'next/router' | ||
| import React, { useCallback } from 'react' | ||
| import React, { useCallback, useEffect, useMemo, useState } from 'react' | ||
|
|
||
| import { useIntl } from '@open-condo/next/intl' | ||
| import { Button, Space, Tooltip, Typography } from '@open-condo/ui' | ||
| import type { TooltipProps } from '@open-condo/ui' | ||
| import { useOrganization } from '@open-condo/next/organization' | ||
| import { Button, Space, Tooltip, Tour, Typography } from '@open-condo/ui' | ||
|
|
||
| import { CURRENCY_SYMBOLS } from '@condo/domains/common/constants/currencies' | ||
| import { SETTINGS_TAB_SUBSCRIPTION } from '@condo/domains/common/constants/settingsTabs' | ||
| import { TrialActivatedTooltip } from '@condo/domains/subscription/components/TrialActivatedTooltip' | ||
| import { getRequiredFeature } from '@condo/domains/subscription/constants/routeFeatureMapping' | ||
| import { useActivateSubscriptions, useOrganizationSubscription } from '@condo/domains/subscription/hooks' |
There was a problem hiding this comment.
Reorder imports to match repository grouping rules.
Internal @app/condo/gql should not precede external and @open-condo imports.
As per coding guidelines: "Enforce import order groups: builtin → external → @open-condo → internal → sibling → parent with newlines between groups" and "Alphabetize imports case-insensitively within each import group".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around
lines 1 - 13, Reorder the imports in NoSubscriptionTooltip to follow the
repository grouping and alphabetical rules: place external imports first (e.g.,
React and next/router: React, useRouter), then `@open-condo` imports (e.g.,
useIntl, useOrganization, Button, Space, Tooltip, Tour, Typography), then
internal imports (move useGetAvailableSubscriptionPlansQuery and other
`@app/`@condo domain imports like CURRENCY_SYMBOLS, SETTINGS_TAB_SUBSCRIPTION,
TrialActivatedTooltip, getRequiredFeature, useActivateSubscriptions,
useOrganizationSubscription). Within each group alphabetize case-insensitively
(so React before useRouter, Button before Space, CURRENCY_SYMBOLS before
SETTINGS_TAB_SUBSCRIPTION, etc.) and ensure blank lines separate the groups.
| placement?: 'top' | 'bottom' | 'left' | 'right' | ||
| feature?: AvailableFeature | AvailableFeature[] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's check the NoSubscriptionTooltip component definition
rg -n "placement\??:" apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx -A 2 -B 2Repository: open-condo-software/condo
Length of output: 282
🏁 Script executed:
#!/bin/bash
# Check for toolbar/decorator placement types that might be broader
rg -n "toolTipDecorator|INoOrganizationToolTipWrapper" apps/condo/domains/common/components/MenuItem.tsx -A 5 -B 5Repository: open-condo-software/condo
Length of output: 1534
🏁 Script executed:
#!/bin/bash
# Look for NoSubscriptionTooltip usage in _app.tsx
rg -n "NoSubscriptionTooltip|placement" apps/condo/pages/_app.tsx -A 3 -B 3Repository: open-condo-software/condo
Length of output: 1262
🏁 Script executed:
#!/bin/bash
# Find the INoOrganizationToolTipWrapper interface definition
rg -n "interface INoOrganizationToolTipWrapper|type INoOrganizationToolTipWrapper" -A 5 apps/condo/domains/onboarding/hooks/useNoOrganizationToolTip.tsRepository: open-condo-software/condo
Length of output: 172
🏁 Script executed:
#!/bin/bash
# Check what `@open-condo/ui` Tooltip accepts for placement
fd -e d.ts -e ts -e tsx node_modules/@open-condo/ui | xargs rg "placement" -B 3 -A 3 | head -100Repository: open-condo-software/condo
Length of output: 7303
🏁 Script executed:
#!/bin/bash
# Find the useNoOrganizationToolTip file
find apps/condo -name "*useNoOrganizationToolTip*" -o -name "*useNo*ToolTip*"Repository: open-condo-software/condo
Length of output: 133
🏁 Script executed:
#!/bin/bash
# Check the actual `@open-condo/ui` Tooltip component type definition
rg -n "type.*Placement|interface.*Tooltip.*Props" packages/ui/src/components/Tooltip/ -A 10 | head -150Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Look for broader placement type usage in Tooltip
rg -n "topLeft|bottomLeft|topRight|bottomRight" apps/condo --include="*.tsx" --include="*.ts" | head -20Repository: open-condo-software/condo
Length of output: 510
🏁 Script executed:
#!/bin/bash
# Read the useNoOrganizationToolTip.tsx file
cat -n apps/condo/domains/onboarding/hooks/useNoOrganizationToolTip.tsxRepository: open-condo-software/condo
Length of output: 1420
🏁 Script executed:
#!/bin/bash
# Check Tooltip component type definition more broadly
rg -n "placement" packages/ui/src/components/Tooltip/tooltip.tsx -B 5 -A 2 | head -100Repository: open-condo-software/condo
Length of output: 239
🏁 Script executed:
#!/bin/bash
# Check if there are broader placement type usages in the codebase
rg "topLeft|bottomLeft|topRight|bottomRight" apps/condo -t ts -t tsx | head -20Repository: open-condo-software/condo
Length of output: 96
🏁 Script executed:
#!/bin/bash
# Check what TooltipPlacement from antd includes
grep -r "TooltipPlacement" node_modules/antd/es/tooltip --include="*.d.ts" | head -20Repository: open-condo-software/condo
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Also check the `@open-condo/ui` Tooltip export to see full type
cat packages/ui/src/components/Tooltip/tooltip.tsx | head -50Repository: open-condo-software/condo
Length of output: 840
🏁 Script executed:
#!/bin/bash
# Check if we can find the antd TooltipPlacement type definition in the codebase
find . -name "*.d.ts" -o -name "*.ts" | xargs grep -l "TooltipPlacement" 2>/dev/null | grep -v node_modules | head -10Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Check what values `@open-condo/ui` Tooltip stories use for placement
cat packages/ui/src/stories/Tooltip.stories.tsx | grep -A 20 "placement.*options"Repository: open-condo-software/condo
Length of output: 51
🏁 Script executed:
#!/bin/bash
# Search for Tooltip.stories.tsx more carefully
find . -name "*Tooltip.stories*" -type f 2>/dev/null | head -5Repository: open-condo-software/condo
Length of output: 114
🏁 Script executed:
#!/bin/bash
# Check if antd's TooltipPlacement is used elsewhere and what values it might have
rg "TooltipPlacement" --type ts | head -20Repository: open-condo-software/condo
Length of output: 281
🏁 Script executed:
#!/bin/bash
# Let's examine how NoSubscriptionTooltip is actually used and if there's type mismatch
rg -n "NoSubscriptionTooltip" apps/condo --type ts -A 3 -B 2 | head -80Repository: open-condo-software/condo
Length of output: 6772
🏁 Script executed:
#!/bin/bash
# Check the fuller context around how featureTooltip is used
sed -n '380,395p' apps/condo/pages/_app.tsxRepository: open-condo-software/condo
Length of output: 937
🏁 Script executed:
#!/bin/bash
# Check if there are other implementations of toolTipDecorator besides featureTooltip
rg "toolTipDecorator\s*=" apps/condo -A 5 -B 2Repository: open-condo-software/condo
Length of output: 1867
🏁 Script executed:
#!/bin/bash
# Check the MenuItem component to see how toolTipDecorator is called
rg -n "toolTipDecorator\(" apps/condo/domains/common/components/MenuItem.tsx -B 5 -A 5Repository: open-condo-software/condo
Length of output: 314
🏁 Script executed:
#!/bin/bash
# Check the exact type signature of toolTipDecorator parameter in MenuItem
sed -n '20,45p' apps/condo/domains/common/components/MenuItem.tsxRepository: open-condo-software/condo
Length of output: 732
🏁 Script executed:
#!/bin/bash
# Look for any type definitions or annotations for the decorator function
rg -n "toolTipDecorator.*:" apps/condo/domains/common/components/MenuItem.tsx -B 2 -A 2Repository: open-condo-software/condo
Length of output: 410
🏁 Script executed:
#!/bin/bash
# Verify what TooltipPlacement from antd actually includes
python3 << 'EOF'
import json
import subprocess
# Search for TooltipPlacement type definition in the actual antd types
result = subprocess.run(['find', 'node_modules/antd', '-name', '*.d.ts', '-type', 'f'],
capture_output=True, text=True, timeout=5)
files = [f for f in result.stdout.split('\n') if 'tooltip' in f.lower()]
print("Tooltip-related type files:", files[:5])
EOFRepository: open-condo-software/condo
Length of output: 99
🏁 Script executed:
#!/bin/bash
# Let's check if there's a type error in the current code setup
# by looking at how placement flows through the decorator
cat -n apps/condo/pages/_app.tsx | sed -n '384,390p'Repository: open-condo-software/condo
Length of output: 455
🏁 Script executed:
#!/bin/bash
# Check if NoSubscriptionTooltip's placement prop actually causes type errors
# by searching for any type assertions or casts around placement
rg -n "placement.*as\s|@ts-|// `@ts`" apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxRepository: open-condo-software/condo
Length of output: 51
NoSubscriptionTooltip's placement prop type narrows the decorator contract.
INoOrganizationToolTipWrapper specifies placement: TooltipPlacement (Ant Design's broader type including 'topLeft', 'bottomRight', etc.), but NoSubscriptionTooltipProps['placement'] accepts only 'top' | 'bottom' | 'left' | 'right'. When featureTooltip passes a TooltipPlacement value to NoSubscriptionTooltip, it violates the type contract. While currently safe because MenuItem only passes 'right', this breaks type safety for future refactoring or alternative decorator implementations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around
lines 22 - 23, The placement prop on NoSubscriptionTooltip is too narrow; update
NoSubscriptionTooltipProps (and the placement prop in the NoSubscriptionTooltip
component) to accept Ant Design's TooltipPlacement type so it matches
INoOrganizationToolTipWrapper and callers like featureTooltip that may pass
'topLeft'/'bottomRight' etc.; locate the placement declarations in
NoSubscriptionTooltipProps and the component signature and replace the union
'top'|'bottom'|'left'|'right' with TooltipPlacement (importing it from 'antd' if
missing) so the types align across INoOrganizationToolTipWrapper,
NoSubscriptionTooltipProps, and featureTooltip.
| import { useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery, useGetPendingSubscriptionRequestsQuery, useGetOrganizationActivatedSubscriptionsQuery } from '@app/condo/gql' | ||
| import { notification } from 'antd' | ||
| import { useCallback, useState } from 'react' | ||
|
|
||
| import { getClientSideSenderInfo } from '@open-condo/miniapp-utils/helpers/sender' | ||
| import { useIntl } from '@open-condo/next/intl' | ||
| import { useOrganization } from '@open-condo/next/organization' | ||
| import { Typography } from '@open-condo/ui' | ||
|
|
There was a problem hiding this comment.
Fix import group ordering in this new hook.
The internal @app/condo/gql import is currently above external and @open-condo groups.
Suggested import reorder
-import { useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery, useGetPendingSubscriptionRequestsQuery, useGetOrganizationActivatedSubscriptionsQuery } from '@app/condo/gql'
import { notification } from 'antd'
import { useCallback, useState } from 'react'
import { getClientSideSenderInfo } from '@open-condo/miniapp-utils/helpers/sender'
import { useIntl } from '@open-condo/next/intl'
import { useOrganization } from '@open-condo/next/organization'
import { Typography } from '@open-condo/ui'
+
+import { useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery, useGetPendingSubscriptionRequestsQuery, useGetOrganizationActivatedSubscriptionsQuery } from '@app/condo/gql'As per coding guidelines: "Enforce import order groups: builtin → external → @open-condo → internal → sibling → parent with newlines between groups" and "Alphabetize imports case-insensitively within each import group".
📝 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.
| import { useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery, useGetPendingSubscriptionRequestsQuery, useGetOrganizationActivatedSubscriptionsQuery } from '@app/condo/gql' | |
| import { notification } from 'antd' | |
| import { useCallback, useState } from 'react' | |
| import { getClientSideSenderInfo } from '@open-condo/miniapp-utils/helpers/sender' | |
| import { useIntl } from '@open-condo/next/intl' | |
| import { useOrganization } from '@open-condo/next/organization' | |
| import { Typography } from '@open-condo/ui' | |
| import { notification } from 'antd' | |
| import { useCallback, useState } from 'react' | |
| import { getClientSideSenderInfo } from '@open-condo/miniapp-utils/helpers/sender' | |
| import { useIntl } from '@open-condo/next/intl' | |
| import { useOrganization } from '@open-condo/next/organization' | |
| import { Typography } from '@open-condo/ui' | |
| import { useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery, useGetPendingSubscriptionRequestsQuery, useGetOrganizationActivatedSubscriptionsQuery } from '@app/condo/gql' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/subscription/hooks/useActivateSubscriptions.tsx` around
lines 1 - 9, Reorder the imports in useActivateSubscriptions.tsx so they follow
the project's group ordering (builtin → external → `@open-condo` → internal →
sibling → parent) with blank lines between groups and alphabetized
case-insensitively within each group; specifically move the internal import of
useActivateSubscriptionPlanMutation, useGetOrganizationTrialSubscriptionsQuery,
useGetPendingSubscriptionRequestsQuery,
useGetOrganizationActivatedSubscriptionsQuery (from '@app/condo/gql') below the
`@open-condo` imports (getClientSideSenderInfo, useIntl, useOrganization,
Typography) and ensure external imports (notification from 'antd',
useCallback/useState from 'react') are placed before the `@open-condo` group, all
alphabetized.
| <Col className={styles.cursorPointer} onClick={() => setCreateInvoiceModalOpen(true)}> | ||
| <Space size={4} direction='horizontal'> | ||
| <PlusCircle /> | ||
| <Typography.Text size='medium' strong>{AddInvoiceMessage}</Typography.Text> | ||
| </Space> | ||
| </Col> |
There was a problem hiding this comment.
Make the Add Invoice trigger keyboard-accessible.
On Line 237, the action is bound to a clickable Col only. Keyboard-only users can’t activate it, which blocks invoice creation in this path.
Proposed fix
@@
- <Col className={styles.cursorPointer} onClick={() => setCreateInvoiceModalOpen(true)}>
+ <Col
+ className={styles.cursorPointer}
+ onClick={() => setCreateInvoiceModalOpen(true)}
+ role='button'
+ tabIndex={0}
+ onKeyDown={(event) => {
+ if (event.key === 'Enter' || event.key === ' ') {
+ event.preventDefault()
+ setCreateInvoiceModalOpen(true)
+ }
+ }}
+ >
<Space size={4} direction='horizontal'>
<PlusCircle />
<Typography.Text size='medium' strong>{AddInvoiceMessage}</Typography.Text>
</Space>
</Col>📝 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.
| <Col className={styles.cursorPointer} onClick={() => setCreateInvoiceModalOpen(true)}> | |
| <Space size={4} direction='horizontal'> | |
| <PlusCircle /> | |
| <Typography.Text size='medium' strong>{AddInvoiceMessage}</Typography.Text> | |
| </Space> | |
| </Col> | |
| <Col | |
| className={styles.cursorPointer} | |
| onClick={() => setCreateInvoiceModalOpen(true)} | |
| role='button' | |
| tabIndex={0} | |
| onKeyDown={(event) => { | |
| if (event.key === 'Enter' || event.key === ' ') { | |
| event.preventDefault() | |
| setCreateInvoiceModalOpen(true) | |
| } | |
| }} | |
| > | |
| <Space size={4} direction='horizontal'> | |
| <PlusCircle /> | |
| <Typography.Text size='medium' strong>{AddInvoiceMessage}</Typography.Text> | |
| </Space> | |
| </Col> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx` around lines
237 - 242, The Add Invoice trigger is only a clickable Col (the element with
class styles.cursorPointer and onClick={() => setCreateInvoiceModalOpen(true)}),
so keyboard users cannot activate it; change it to be keyboard-accessible by
replacing or enhancing that Col: either render a semantic interactive element
(e.g., a button or antd Button/Link) or add role="button", tabIndex={0} and an
onKeyDown handler that triggers setCreateInvoiceModalOpen(true) on Enter/Space,
and ensure the PlusCircle and Typography.Text (AddInvoiceMessage) remain
visually the same and that aria-label or accessible text is present for screen
readers.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/condo/domains/ticket/components/BaseTicketForm/index.tsx (1)
236-241:⚠️ Potential issue | 🟠 MajorMake the Add Invoice trigger keyboard-accessible.
Line 236 uses a clickable
Colwithout keyboard activation support, so Enter/Space users can’t open the modal.💡 Suggested fix
- <Col className={styles.cursorPointer} onClick={() => setCreateInvoiceModalOpen(true)}> + <Col + className={styles.cursorPointer} + onClick={() => setCreateInvoiceModalOpen(true)} + role='button' + tabIndex={0} + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + setCreateInvoiceModalOpen(true) + } + }} + > <Space size={4} direction='horizontal'> <PlusCircle /> <Typography.Text size='medium' strong>{AddInvoiceMessage}</Typography.Text> </Space> </Col>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx` around lines 236 - 241, The clickable Col with class styles.cursorPointer that opens the modal via onClick(() => setCreateInvoiceModalOpen(true)) is not keyboard-accessible; update the element (the Col wrapping the PlusCircle and AddInvoiceMessage) to be focusable and activate on Enter/Space by adding tabIndex={0}, role="button", and an onKeyDown handler that calls setCreateInvoiceModalOpen(true) when event.key is 'Enter' or ' ' (Space); alternatively replace the Col with an actual Button for semantics while preserving the Space/PlusCircle children and visual styling.apps/condo/pages/incident/[id]/index.tsx (1)
637-652:⚠️ Potential issue | 🟠 MajorReplace
children={...}onButton(Biome blocker).Line 640 and Line 650 still pass content via the
childrenprop, which violateslint/correctness/noChildrenProp.💡 Suggested fix
<Button disabled type='secondary' - children={GenerateNewsLabel} id='generateNews' - /> + > + {GenerateNewsLabel} + </Button> @@ <Button disabled={generateNewsLoading || incidentClassifierIncidentLoading || incidentPropertiesLoading} loading={generateNewsLoading} type='secondary' - children={GenerateNewsLabel} onClick={handleGenerateNews} id='generateNews' - /> + > + {GenerateNewsLabel} + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/incident/`[id]/index.tsx around lines 637 - 652, The Button components are passing content via the children prop which violates lint/correctness/noChildrenProp; update both Button usages (the disabled Button inside the dropdown and the clickable Button with onClick/handleGenerateNews) to pass the label as the Button's child element instead of using children={GenerateNewsLabel} — i.e., remove the children prop and place {GenerateNewsLabel} between the opening and closing <Button> tags (keeping existing props like disabled, loading, type, onClick, id intact).apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx (1)
27-50:⚠️ Potential issue | 🟠 MajorAvoid mounting activation/plan data hooks in every tooltip instance.
Line 28 and Line 45 run subscription activation/plan data flow for each
NoSubscriptionTooltipmount, even when tooltip content is never needed. With widespreadFeatureGate/menu usage, this can create unnecessary query observers and refetch pressure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around lines 27 - 50, The tooltip currently mounts heavy subscription hooks (useActivateSubscriptions, useOrganizationSubscription, useGetAvailableSubscriptionPlansQuery) every time NoSubscriptionTooltip mounts, causing unnecessary queries; fix this by extracting those hooks into a new inner component (e.g., SubscriptionTooltipContent) that contains useActivateSubscriptions, useOrganizationSubscription, and useGetAvailableSubscriptionPlansQuery and then render that component only when the tooltip is actually shown/open (or when featureProp/requiredFeature indicates content is needed), so NoSubscriptionTooltip itself remains lightweight and hooks run only on demand.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/onboarding/components/TourPage/TourStepCard.tsx`:
- Around line 161-175: If requiredFeature is undefined, avoid rendering
FeatureGate and directly return the card wrapper to skip subscription work: in
TourStepCard use the requiredFeature value to short-circuit (e.g., if
requiredFeature === undefined) and return the <div
className={styles.fullWidth}>{cardContent}</div> immediately; otherwise render
FeatureGate with feature={requiredFeature}, skipTooltip={false/unchanged} and
the same fallback/content. This change targets the FeatureGate usage and the
requiredFeature variable so steps without a feature don't mount FeatureGate.
---
Duplicate comments:
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 27-50: The tooltip currently mounts heavy subscription hooks
(useActivateSubscriptions, useOrganizationSubscription,
useGetAvailableSubscriptionPlansQuery) every time NoSubscriptionTooltip mounts,
causing unnecessary queries; fix this by extracting those hooks into a new inner
component (e.g., SubscriptionTooltipContent) that contains
useActivateSubscriptions, useOrganizationSubscription, and
useGetAvailableSubscriptionPlansQuery and then render that component only when
the tooltip is actually shown/open (or when featureProp/requiredFeature
indicates content is needed), so NoSubscriptionTooltip itself remains
lightweight and hooks run only on demand.
In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx`:
- Around line 236-241: The clickable Col with class styles.cursorPointer that
opens the modal via onClick(() => setCreateInvoiceModalOpen(true)) is not
keyboard-accessible; update the element (the Col wrapping the PlusCircle and
AddInvoiceMessage) to be focusable and activate on Enter/Space by adding
tabIndex={0}, role="button", and an onKeyDown handler that calls
setCreateInvoiceModalOpen(true) when event.key is 'Enter' or ' ' (Space);
alternatively replace the Col with an actual Button for semantics while
preserving the Space/PlusCircle children and visual styling.
In `@apps/condo/pages/incident/`[id]/index.tsx:
- Around line 637-652: The Button components are passing content via the
children prop which violates lint/correctness/noChildrenProp; update both Button
usages (the disabled Button inside the dropdown and the clickable Button with
onClick/handleGenerateNews) to pass the label as the Button's child element
instead of using children={GenerateNewsLabel} — i.e., remove the children prop
and place {GenerateNewsLabel} between the opening and closing <Button> tags
(keeping existing props like disabled, loading, type, onClick, id intact).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8aea26da-503c-4ff6-b7c0-166cff1dbc0e
📒 Files selected for processing (10)
apps/condo/domains/common/components/Comments/CommentForm.tsxapps/condo/domains/common/components/Comments/CommentsTabContent.tsxapps/condo/domains/onboarding/components/TourPage/TourStepCard.tsxapps/condo/domains/subscription/components/FeatureGate.tsxapps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/ticket/components/BaseTicketForm/index.tsxapps/condo/domains/ticket/components/IncidentForm/BaseIncidentForm.tsxapps/condo/domains/ticket/components/IncidentForm/CreateIncidentForm.tsxapps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsxapps/condo/pages/incident/[id]/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsx
- apps/condo/domains/ticket/components/IncidentForm/CreateIncidentForm.tsx
- apps/condo/domains/subscription/components/FeatureGate.tsx
apps/condo/domains/onboarding/components/TourPage/TourStepCard.tsx
Outdated
Show resolved
Hide resolved
tolmachev21
left a comment
tolmachev21
left a comment
There was a problem hiding this comment.
I like idea with feature gate!
apps/condo/lang/ru/ru.json
Outdated
| "subscription.trial.welcome.title": "Добро пожаловать в тариф «{planName}»", | ||
| "subscription.trial.welcome.description": "Вместе с пробным периодом тарифа «{planName}» мы открыли для вас эту функцию и не только. Попробуйте, как это упрощает задачи.", |
There was a problem hiding this comment.
I can't find this text in the Figma. Has it been updated by any chance?
| "subscription.warns.noActiveSubscription": "Чтобы получить доступ к разделу, прокачайте свой тариф или воспользуйтесь пробным доступом", | ||
| "subscription.warns.activateSubscriptionButton": "Перейти в тарифы", | ||
| "subscription.warns.upgradePlan": "Расширить тариф", | ||
| "subscription.warns.activateSubscriptionButton": "Подробнее о тарифах", | ||
| "subscription.warns.tryTrialButton": "Попробовать за 0{currency}", |
There was a problem hiding this comment.
Maybe we use following rule:
"subscription.warns.beforeTrial.title": "...",
"subscription.warns.beforeTrial.button": "..."
"subscription.warns.afterTrial.title": "..."
"subscription.warns.afterTrial.button": "..."
Or something similar?
| const featureTooltip = ({ element, placement }) => ( | ||
| <NoSubscriptionTooltip path={`/${item.path}`} children={element} placement={placement} /> | ||
| ) |
| const { trialSubscriptions, activatedSubscriptions, handleActivatePlan, activateLoading } = useActivateSubscriptions() | ||
| const { isFeatureAvailable, hasSubscription } = useOrganizationSubscription() |
| key='10' | ||
| feature='ai' | ||
| fallback={<div> | ||
| <Button |
There was a problem hiding this comment.
wouldnt it just add one more button?
There was a problem hiding this comment.
No, we check if we have a feature in the subscription and show children or fallback
| import { NoSubscriptionTooltip, NoSubscriptionTooltipProps } from '@condo/domains/subscription/components/NoSubscriptionTooltip' | ||
| import { useOrganizationSubscription } from '@condo/domains/subscription/hooks' | ||
|
|
||
| interface FeatureGateProps extends Omit<NoSubscriptionTooltipProps, 'children'> { |
There was a problem hiding this comment.
You have to really read the code to get what FeatureGate does – it is not clear from the name
Maybe rename the component?
There was a problem hiding this comment.
It is also really ambigious – you have feature flags and you also have this component that is called feature gate, but doesnt check feature flags! it checks subscriptions
There was a problem hiding this comment.
So the result code would look like this:
isFeatureEnabledByFeatureFlag ?? <FeatureGate ...>
I would have expected smth like
isFeatureEnabled || isFeatureEnabledBySubscription
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Alexander-Turkin
force-pushed
the
feat/condo/DOMA-12907/update-tooltip-content
branch
from
09997b9 to
32c4d28
Compare
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
apps/condo/pages/incident/[id]/index.tsx (1)
637-654:⚠️ Potential issue | 🟡 MinorReplace
childrenprop usage onButtonto satisfy lint rule.Lines 640 and 650 pass button content through the
childrenprop, which violates the Biome lint rulenoChildrenProp. Use JSX children syntax instead.Suggested fix
<Button disabled type='secondary' - children={GenerateNewsLabel} id='generateNews' - /> + > + {GenerateNewsLabel} + </Button> @@ <Button disabled={generateNewsLoading || incidentClassifierIncidentLoading || incidentPropertiesLoading} loading={generateNewsLoading} type='secondary' - children={GenerateNewsLabel} onClick={handleGenerateNews} id='generateNews' - /> + > + {GenerateNewsLabel} + </Button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/incident/`[id]/index.tsx around lines 637 - 654, The Button usages inside the SubscriptionFeatureGuard are passing content via the children prop (children={GenerateNewsLabel}), violating the noChildrenProp lint rule; update both Button elements to use normal JSX children instead (place GenerateNewsLabel between the <Button>...</Button> tags) while keeping existing props like disabled, loading, type, id and the onClick handler (handleGenerateNews) unchanged so the behavior stays identical.apps/condo/domains/ticket/components/BaseTicketForm/index.tsx (1)
236-241:⚠️ Potential issue | 🟠 MajorMake the Add Invoice trigger keyboard-accessible.
The action is bound to a clickable
Colonly. Keyboard-only users cannot activate it, which blocks invoice creation in this path.Proposed fix
- <Col className={styles.cursorPointer} onClick={() => setCreateInvoiceModalOpen(true)}> + <Col + className={styles.cursorPointer} + onClick={() => setCreateInvoiceModalOpen(true)} + role='button' + tabIndex={0} + onKeyDown={(event) => { + if (event.key === 'Enter' || event.key === ' ') { + event.preventDefault() + setCreateInvoiceModalOpen(true) + } + }} + > <Space size={4} direction='horizontal'> <PlusCircle /> <Typography.Text size='medium' strong>{AddInvoiceMessage}</Typography.Text> </Space> </Col>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx` around lines 236 - 241, The clickable Col used to open the invoice modal (the element that renders PlusCircle and Typography.Text with AddInvoiceMessage and uses styles.cursorPointer and onClick={() => setCreateInvoiceModalOpen(true)}) is not keyboard-accessible; replace it with a native interactive element (e.g., a button) or add accessibility behavior: ensure it is focusable (tabIndex=0), has an appropriate role/aria-label, and handle key events (activate on Enter and Space in an onKeyDown handler) to call setCreateInvoiceModalOpen(true); prefer using a semantic button instead of a plain Col to keep styles and focus behavior consistent and remove duplicate click/keyboard divergence.
🧹 Nitpick comments (4)
apps/condo/domains/ticket/components/BaseTicketForm/index.tsx (1)
187-189: Remove unusedhasMarketplaceFeaturevariable.The
hasMarketplaceFeaturevariable is declared but never used after the refactor toSubscriptionFeatureGuard, which handles the feature check internally.♻️ Suggested fix
- // Subscription check for marketplace feature - const { isFeatureAvailable } = useOrganizationSubscription() - const hasMarketplaceFeature = isFeatureAvailable('marketplace') - const [createInvoiceModalOpen, setCreateInvoiceModalOpen] = useState<boolean>(false)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx` around lines 187 - 189, Remove the unused hasMarketplaceFeature variable introduced by useOrganizationSubscription(); locate the declaration "const { isFeatureAvailable } = useOrganizationSubscription()" and the subsequent "const hasMarketplaceFeature = isFeatureAvailable('marketplace')" and delete the hasMarketplaceFeature line (and remove isFeatureAvailable import/destructure if it becomes unused) since SubscriptionFeatureGuard now handles the marketplace check internally.apps/condo/domains/common/components/Comments/CommentForm.tsx (2)
162-163: Remove unusedhasAiFeaturevariable.After the refactor to
SubscriptionFeatureGuard,hasAiFeatureis no longer used. The feature check is now handled internally by the guard component.♻️ Suggested fix
- const { isFeatureAvailable } = useOrganizationSubscription() - const hasAiFeature = isFeatureAvailable('ai') - const editableCommentFiles = editableComment?.filesAlso remove the import if
useOrganizationSubscriptionis no longer needed:-import { useOrganizationSubscription } from '@condo/domains/subscription/hooks'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/common/components/Comments/CommentForm.tsx` around lines 162 - 163, Remove the now-unused hasAiFeature variable and the associated call to useOrganizationSubscription in CommentForm (the feature check is handled by SubscriptionFeatureGuard); specifically delete the line defining hasAiFeature and the const { isFeatureAvailable } = useOrganizationSubscription() call, and if useOrganizationSubscription is not used elsewhere in CommentForm, remove its import as well.
408-433: Use a descriptive key instead of a magic string.The
key='10'on line 409 is a magic string. Use a descriptive key like'rewriteButton'for clarity and maintainability.♻️ Suggested fix
...(rewriteCommentEnabled ? [ <SubscriptionFeatureGuard - key='10' + key='rewriteButton' feature='ai'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/common/components/Comments/CommentForm.tsx` around lines 408 - 433, Replace the magic key '10' on the SubscriptionFeatureGuard with a descriptive identifier (e.g. 'rewriteButton') to improve clarity and maintainability; locate the SubscriptionFeatureGuard wrapping RewriteTextButton (component names: SubscriptionFeatureGuard and RewriteTextButton) and change key='10' to key='rewriteButton' (or another unique, descriptive key) ensuring it doesn't conflict with existing sibling keys.apps/condo/domains/subscription/components/SubscriptionFeatureGuard.tsx (1)
6-10: Consider using a type alias instead of an interface.Per repository conventions, prefer type aliases over interfaces for object shapes unless declaration merging or
extends/implementsis required. This interface extends another type but could still be expressed as a type alias.♻️ Suggested refactor
-interface SubscriptionFeatureGuardProps extends Omit<NoSubscriptionTooltipProps, 'children'> { - children: React.ReactElement - feature: NoSubscriptionTooltipProps['feature'] - fallback: React.ReactElement -} +type SubscriptionFeatureGuardProps = Omit<NoSubscriptionTooltipProps, 'children'> & { + children: React.ReactElement + feature: NoSubscriptionTooltipProps['feature'] + fallback: React.ReactElement +}Based on learnings: "prefer using type aliases (type X = { ... }) over interface declarations for object shapes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/SubscriptionFeatureGuard.tsx` around lines 6 - 10, Replace the interface SubscriptionFeatureGuardProps with a type alias that composes Omit<NoSubscriptionTooltipProps, 'children'> and the explicit props; specifically, define SubscriptionFeatureGuardProps as a type using intersection (or &), keeping the same property names (children, feature, fallback) and referencing NoSubscriptionTooltipProps['feature'] for feature so behavior and types remain identical to the original interface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/lang/es/es.json`:
- Line 3159: The translation currently hardcodes "0{currency}" causing wrong
symbol/order; change the ES JSON key "subscription.warns.tryTrialButton" to use
a "{price}" placeholder (e.g., "Probar por {price}"), and update
NoSubscriptionTooltip.tsx to pass a fully formatted price string (use your
existing formatter or Intl.NumberFormat to format the numeric amount with
currency and locale) instead of injecting only the currency symbol; ensure the
component calls the i18n message with { price: formattedPrice } when rendering.
---
Duplicate comments:
In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx`:
- Around line 236-241: The clickable Col used to open the invoice modal (the
element that renders PlusCircle and Typography.Text with AddInvoiceMessage and
uses styles.cursorPointer and onClick={() => setCreateInvoiceModalOpen(true)})
is not keyboard-accessible; replace it with a native interactive element (e.g.,
a button) or add accessibility behavior: ensure it is focusable (tabIndex=0),
has an appropriate role/aria-label, and handle key events (activate on Enter and
Space in an onKeyDown handler) to call setCreateInvoiceModalOpen(true); prefer
using a semantic button instead of a plain Col to keep styles and focus behavior
consistent and remove duplicate click/keyboard divergence.
In `@apps/condo/pages/incident/`[id]/index.tsx:
- Around line 637-654: The Button usages inside the SubscriptionFeatureGuard are
passing content via the children prop (children={GenerateNewsLabel}), violating
the noChildrenProp lint rule; update both Button elements to use normal JSX
children instead (place GenerateNewsLabel between the <Button>...</Button> tags)
while keeping existing props like disabled, loading, type, id and the onClick
handler (handleGenerateNews) unchanged so the behavior stays identical.
---
Nitpick comments:
In `@apps/condo/domains/common/components/Comments/CommentForm.tsx`:
- Around line 162-163: Remove the now-unused hasAiFeature variable and the
associated call to useOrganizationSubscription in CommentForm (the feature check
is handled by SubscriptionFeatureGuard); specifically delete the line defining
hasAiFeature and the const { isFeatureAvailable } =
useOrganizationSubscription() call, and if useOrganizationSubscription is not
used elsewhere in CommentForm, remove its import as well.
- Around line 408-433: Replace the magic key '10' on the
SubscriptionFeatureGuard with a descriptive identifier (e.g. 'rewriteButton') to
improve clarity and maintainability; locate the SubscriptionFeatureGuard
wrapping RewriteTextButton (component names: SubscriptionFeatureGuard and
RewriteTextButton) and change key='10' to key='rewriteButton' (or another
unique, descriptive key) ensuring it doesn't conflict with existing sibling
keys.
In `@apps/condo/domains/subscription/components/SubscriptionFeatureGuard.tsx`:
- Around line 6-10: Replace the interface SubscriptionFeatureGuardProps with a
type alias that composes Omit<NoSubscriptionTooltipProps, 'children'> and the
explicit props; specifically, define SubscriptionFeatureGuardProps as a type
using intersection (or &), keeping the same property names (children, feature,
fallback) and referencing NoSubscriptionTooltipProps['feature'] for feature so
behavior and types remain identical to the original interface.
In `@apps/condo/domains/ticket/components/BaseTicketForm/index.tsx`:
- Around line 187-189: Remove the unused hasMarketplaceFeature variable
introduced by useOrganizationSubscription(); locate the declaration "const {
isFeatureAvailable } = useOrganizationSubscription()" and the subsequent "const
hasMarketplaceFeature = isFeatureAvailable('marketplace')" and delete the
hasMarketplaceFeature line (and remove isFeatureAvailable import/destructure if
it becomes unused) since SubscriptionFeatureGuard now handles the marketplace
check internally.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f49b3dce-4018-40e6-b310-5cb1e157e2e5
📒 Files selected for processing (13)
apps/condo/domains/common/components/Comments/CommentForm.tsxapps/condo/domains/common/components/Comments/CommentsTabContent.tsxapps/condo/domains/onboarding/components/TourPage/TourStepCard.tsxapps/condo/domains/subscription/components/SubscriptionFeatureGuard.tsxapps/condo/domains/subscription/components/index.tsapps/condo/domains/ticket/components/BaseTicketForm/index.tsxapps/condo/domains/ticket/components/IncidentForm/BaseIncidentForm.tsxapps/condo/domains/ticket/components/IncidentForm/CreateIncidentForm.tsxapps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsxapps/condo/lang/en/en.jsonapps/condo/lang/es/es.jsonapps/condo/lang/ru/ru.jsonapps/condo/pages/incident/[id]/index.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsx
- apps/condo/domains/subscription/components/index.ts
- apps/condo/lang/en/en.json
- apps/condo/domains/ticket/components/IncidentForm/BaseIncidentForm.tsx
- apps/condo/lang/ru/ru.json
| "subscription.warns.activateSubscriptionButton": "Ir a tarifas", | ||
| "subscription.warns.upgradePlan": "Actualizar plan", | ||
| "subscription.warns.activateSubscriptionButton": "Más información sobre los planes", | ||
| "subscription.warns.tryTrialButton": "Probar por 0{currency}", |
There was a problem hiding this comment.
Use a formatted price placeholder instead of 0{currency}.
Line 3159 hardcodes the symbol order, but NoSubscriptionTooltip.tsx:90-95 only injects a bare currency symbol. That makes some locales/currencies render awkwardly here (0$, 0¥). Please switch this message to a {price} placeholder and pass a fully formatted amount from the component.
💡 Suggested change
- "subscription.warns.tryTrialButton": "Probar por 0{currency}",
+ "subscription.warns.tryTrialButton": "Probar por {price}",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/lang/es/es.json` at line 3159, The translation currently hardcodes
"0{currency}" causing wrong symbol/order; change the ES JSON key
"subscription.warns.tryTrialButton" to use a "{price}" placeholder (e.g.,
"Probar por {price}"), and update NoSubscriptionTooltip.tsx to pass a fully
formatted price string (use your existing formatter or Intl.NumberFormat to
format the numeric amount with currency and locale) instead of injecting only
the currency symbol; ensure the component calls the i18n message with { price:
formattedPrice } when rendering.
Alexander-Turkin
force-pushed
the
feat/condo/DOMA-12907/update-tooltip-content
branch
from
32c4d28 to
3295640
Compare
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsx (1)
254-254:⚠️ Potential issue | 🟡 MinorMissing dependencies in
useMemohook.The newly added
GenerateNewsSwitchHintandGenerateNewsSwitchLabelintl messages are used inside thisuseMemobut are not included in the dependency array. This could cause stale content if the locale changes. Other intl messages likeSaveLabelandToActualStatusTitleare already included, so these should follow the same pattern.Additionally,
canManageNewsItems,withNewsGeneration,aiEnabled, andgenerateNewsByIncidentEnabledare used in the condition at line 176 but also appear to be missing.Proposed fix
- }, [SaveLabel, ToActualStatusTitle, ToNotActualStatusTitle, WorkFinishFieldMessage, descriptionText, finishWorkRules, handleClose, handleUpdate, initialState, isActual, open]) + }, [SaveLabel, ToActualStatusTitle, ToNotActualStatusTitle, WorkFinishFieldMessage, descriptionText, finishWorkRules, handleClose, handleUpdate, initialState, isActual, open, GenerateNewsSwitchHint, GenerateNewsSwitchLabel, canManageNewsItems, withNewsGeneration, aiEnabled, generateNewsByIncidentEnabled])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsx` at line 254, The useMemo that builds the modal text is missing several dependencies which can lead to stale translations and state: add GenerateNewsSwitchHint and GenerateNewsSwitchLabel to the dependency array, and also include canManageNewsItems, withNewsGeneration, aiEnabled, and generateNewsByIncidentEnabled since they are referenced in the conditional logic (the useMemo that currently lists SaveLabel, ToActualStatusTitle, ToNotActualStatusTitle, WorkFinishFieldMessage, descriptionText, finishWorkRules, handleClose, handleUpdate, initialState, isActual, open should be updated to include these symbols so the memo recalculates when locale or feature flags change).
♻️ Duplicate comments (1)
apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx (1)
27-29:⚠️ Potential issue | 🟠 MajorStill mounting subscription queries in every tooltip instance.
These hooks run before every early return, so
apps/condo/pages/_app.tsxnow pays for the same subscription observers once per sidebar item even whenskipTooltip,path && !requiredFeature, orisAvailableimmediately returnchildren. Move this data loading to a shared parent/provider, or split the heavy branch into a child component that renders only when the tooltip is actually needed.Also applies to: 45-50, 148-162
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx` around lines 27 - 29, The subscription hooks (useOrganization, useActivateSubscriptions, useOrganizationSubscription) are being invoked on every NoSubscriptionTooltip render even when early-return branches (skipTooltip, path && !requiredFeature, isAvailable) immediately return children; lift these data-heavy hooks out of the top-level component by either moving them into a shared parent/provider or by extracting the heavy logic into a lazily-rendered child component (e.g., NoSubscriptionTooltipContent or TooltipBody) and only render that child when the tooltip will actually be shown (after evaluating skipTooltip, path, requiredFeature, and isAvailable). Ensure NoSubscriptionTooltip itself only performs the cheap checks and renders children or conditionally mounts the child that calls useOrganization, useActivateSubscriptions, and useOrganizationSubscription.
🧹 Nitpick comments (1)
apps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionSettingsContent.tsx (1)
71-78: Scope trial loading to the activating card.
activateLoadingis a single hook-wide boolean, but it is passed unchanged into everySubscriptionPlanCard. Starting one trial will put every visible "Try free" button into the loading state. Prefer tracking the activeplanId/priceIdso only the initiating card spins.Also applies to: 106-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionSettingsContent.tsx` around lines 71 - 78, The hook-wide boolean activateLoading causes every SubscriptionPlanCard to show loading; modify useActivateSubscriptions to track which plan/price is currently activating (e.g., activatingPlanId or activatingPriceId) and expose that instead of a single boolean, update handleActivatePlan to set and clear that id around the async call, and pass a per-card boolean (plan.id === activatingPlanId) into SubscriptionPlanCard (and do the same for the pending/activated sections where activateLoading is currently used).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/condo/domains/common/components/Comments/CommentForm.tsx`:
- Around line 408-423: The fallback for SubscriptionFeatureGuard only renders
the full-text disabled button, which omits the compact icon-only variant used by
RewriteTextButton; update the fallback to mirror RewriteTextButton's two-variant
pattern by including both the full-text disabled Button (using
UpdateTextMessage, classNames(styles.rewriteTextButton,
styles.rewriteButtonWithText), compact/minimal/type/size/icon as currently
shown) and the compact icon-only disabled Button wrapped in the same
Tooltip/visibility wrapper used by RewriteTextButton (using the Sparkles icon
and the same disabled/compact/minimal props) so users without the 'ai' feature
see the same upsell UI on narrow screens.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 52-57: Replace the global check using trialSubscriptions.length >
0 (used in hasActivatedAnyTrial) with the per-plan trial eligibility logic used
by SubscriptionPlanCard/SubscriptionPlanBadge: compute a flag using
bestPlanWithFeature and the same eligibility predicate you use for that plan
(reusing isFeatureAvailable or the plan-specific trial check) and base both
buttonText and buttonAction on that per-plan-eligibility flag instead of
trialSubscriptions; update references in the NoSubscriptionTooltip component
(where hasActivatedAnyTrial, buttonText, buttonAction, and isAvailable are
computed) so the CTA reflects bestPlanWithFeature's actual trial eligibility
rather than any historical trial in trialSubscriptions.
- Around line 73-74: The code is incorrectly using prices[0] to determine CTA
text and handleActivatePlan, making billing period selection dependent on
backend ordering; update NoSubscriptionTooltip to never read prices[0]
directly—instead derive a deterministic period (e.g., prefer a planPeriod prop
if present, else prefer "monthly" if a matching price exists, else "annual",
else fallback to the first price) and use that selectedPeriod variable for CTA
copy and handleActivatePlan; update all usages in this file (references around
planInfo, prices, hasPrice, and the handleActivatePlan invocation—also similar
logic in the blocks you noted at 91-95 and 103-116) so the UI either chooses
that explicit default or routes the CTA to the full plans page when no clear
default exists.
---
Outside diff comments:
In `@apps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsx`:
- Line 254: The useMemo that builds the modal text is missing several
dependencies which can lead to stale translations and state: add
GenerateNewsSwitchHint and GenerateNewsSwitchLabel to the dependency array, and
also include canManageNewsItems, withNewsGeneration, aiEnabled, and
generateNewsByIncidentEnabled since they are referenced in the conditional logic
(the useMemo that currently lists SaveLabel, ToActualStatusTitle,
ToNotActualStatusTitle, WorkFinishFieldMessage, descriptionText,
finishWorkRules, handleClose, handleUpdate, initialState, isActual, open should
be updated to include these symbols so the memo recalculates when locale or
feature flags change).
---
Duplicate comments:
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 27-29: The subscription hooks (useOrganization,
useActivateSubscriptions, useOrganizationSubscription) are being invoked on
every NoSubscriptionTooltip render even when early-return branches (skipTooltip,
path && !requiredFeature, isAvailable) immediately return children; lift these
data-heavy hooks out of the top-level component by either moving them into a
shared parent/provider or by extracting the heavy logic into a lazily-rendered
child component (e.g., NoSubscriptionTooltipContent or TooltipBody) and only
render that child when the tooltip will actually be shown (after evaluating
skipTooltip, path, requiredFeature, and isAvailable). Ensure
NoSubscriptionTooltip itself only performs the cheap checks and renders children
or conditionally mounts the child that calls useOrganization,
useActivateSubscriptions, and useOrganizationSubscription.
---
Nitpick comments:
In
`@apps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionSettingsContent.tsx`:
- Around line 71-78: The hook-wide boolean activateLoading causes every
SubscriptionPlanCard to show loading; modify useActivateSubscriptions to track
which plan/price is currently activating (e.g., activatingPlanId or
activatingPriceId) and expose that instead of a single boolean, update
handleActivatePlan to set and clear that id around the async call, and pass a
per-card boolean (plan.id === activatingPlanId) into SubscriptionPlanCard (and
do the same for the pending/activated sections where activateLoading is
currently used).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 268f4d5c-7203-48fb-ac73-6d4dc362e76c
📒 Files selected for processing (21)
apps/condo/domains/common/components/Comments/CommentForm.tsxapps/condo/domains/common/components/Comments/CommentsTabContent.tsxapps/condo/domains/onboarding/components/TourPage/TourStepCard.module.cssapps/condo/domains/onboarding/components/TourPage/TourStepCard.tsxapps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/subscription/components/SubscriptionFeatureGuard.tsxapps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionPlanCard/SubscriptionPlanCard.tsxapps/condo/domains/subscription/components/SubscriptionSettingsContent/SubscriptionSettingsContent.tsxapps/condo/domains/subscription/components/index.tsapps/condo/domains/subscription/hooks/index.tsapps/condo/domains/subscription/hooks/useActivateSubscriptions.tsxapps/condo/domains/ticket/components/BaseTicketForm/BaseTicketForm.module.cssapps/condo/domains/ticket/components/BaseTicketForm/index.tsxapps/condo/domains/ticket/components/IncidentForm/BaseIncidentForm.tsxapps/condo/domains/ticket/components/IncidentForm/CreateIncidentForm.tsxapps/condo/domains/ticket/hooks/useIncidentUpdateStatusModal.tsxapps/condo/lang/en/en.jsonapps/condo/lang/es/es.jsonapps/condo/lang/ru/ru.jsonapps/condo/pages/_app.tsxapps/condo/pages/incident/[id]/index.tsx
🚧 Files skipped from review as they are similar to previous changes (11)
- apps/condo/domains/ticket/components/IncidentForm/BaseIncidentForm.tsx
- apps/condo/domains/common/components/Comments/CommentsTabContent.tsx
- apps/condo/domains/ticket/components/IncidentForm/CreateIncidentForm.tsx
- apps/condo/lang/ru/ru.json
- apps/condo/lang/en/en.json
- apps/condo/domains/ticket/components/BaseTicketForm/BaseTicketForm.module.css
- apps/condo/domains/ticket/components/BaseTicketForm/index.tsx
- apps/condo/lang/es/es.json
- apps/condo/domains/subscription/hooks/useActivateSubscriptions.tsx
- apps/condo/domains/subscription/components/SubscriptionFeatureGuard.tsx
- apps/condo/domains/onboarding/components/TourPage/TourStepCard.module.css
| </div> | ||
| </NoSubscriptionTooltip> | ||
| ), | ||
| </FeatureGate>, |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
|
Alexander-Turkin
deleted the
feat/condo/DOMA-12907/update-tooltip-content
branch
4 participants
2026-03-05.17.32.46.mov
Summary by CodeRabbit
New Features
Improvements
Documentation