fix(condo): DOMA-13033 fix tooltip for miniapps#7330
Conversation
Alexander-Turkin
temporarily deployed
to
internal
GitHub Actions
Inactive
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
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.
📝 WalkthroughWalkthroughIntroduces B2B miniapp tariff gating: subscription hook now exposes Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant UI as Miniapp UI (TopCard / Menu)
participant Guard as SubscriptionGuardWithTooltip
participant Hook as useOrganizationSubscription
participant Tooltip as NoSubscriptionTooltip
participant AppAction as Miniapp Action
rect rgba(200,230,255,0.5)
User->>UI: clicks action / hovers menu
UI->>Guard: render/check access (feature?, b2bAppId?)
end
rect rgba(200,255,200,0.5)
Guard->>Hook: isFeatureAvailable(feature) / isB2BAppEnabled(b2bAppId)
Hook-->>Guard: availability result
Guard-->Guard: decide allowed vs fallback
end
alt allowed
Guard->>AppAction: enable / perform action
AppAction-->>User: open/connect/start
else not allowed
Guard->>Tooltip: render fallback (passes b2bAppId)
Tooltip-->>User: show upgrade/trial message or disabled button
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
![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: ac88595f35
ℹ️ 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".
| : isFeatureAvailable(feature) | ||
|
|
||
| if (hasFeature) { | ||
| if (hasFeature || isAppAvailableForTariff) { |
There was a problem hiding this comment.
This condition now lets SubscriptionGuardWithTooltip render children for all feature-only usages because isAppAvailableForTariff defaults to true when b2bAppId is missing, so hasFeature || isAppAvailableForTariff is always truthy. As a result, existing callers that pass only feature (there are many across incident/comment/ticket flows) no longer show the fallback/tooltip and effectively lose UI subscription gating.
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: 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/miniapp/components/AppDescription/TopCard.tsx (1)
161-161:⚠️ Potential issue | 🟡 MinorRemove unused dependencies from
useMemo.
isAppAvailableForTariffandUnavailableForTariffMessageare listed in the dependency array but are not used inside theuseMemocallback. This causes unnecessary re-computations ofbuttonProps.🧹 Proposed fix
- }, [id, appUrl, contextStatus, connectAction, intl, router, canManageB2BApps, accessible, isAppAvailableForTariff, UnavailableForTariffMessage]) + }, [id, appUrl, contextStatus, connectAction, intl, router, canManageB2BApps, accessible])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx` at line 161, In TopCard.tsx, remove the unused dependencies isAppAvailableForTariff and UnavailableForTariffMessage from the useMemo dependency array that computes buttonProps (the useMemo callback does not reference them), leaving only the actual used variables (id, appUrl, contextStatus, connectAction, intl, router, canManageB2BApps, accessible) so buttonProps won't recompute unnecessarily.
🧹 Nitpick comments (3)
apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx (1)
6-11: Consider using a type alias instead of an interface.Based on learnings from this repository, type aliases are preferred over interface declarations for object shapes.
♻️ Suggested refactor
-interface SubscriptionFeatureGuardProps extends Omit<NoSubscriptionTooltipProps, 'children'> { +type SubscriptionFeatureGuardProps = Omit<NoSubscriptionTooltipProps, 'children'> & { children: React.ReactElement feature?: NoSubscriptionTooltipProps['feature'] appId?: string fallback: React.ReactElement }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/components/SubscriptionGuardWithTooltip.tsx` around lines 6 - 11, Replace the interface SubscriptionFeatureGuardProps with a type alias while keeping the exact shape and generics (retain Omit<NoSubscriptionTooltipProps, 'children'> and properties children, feature, appId, fallback) so consumers of SubscriptionGuardWithTooltip continue to work unchanged; update any imports/exports that reference SubscriptionFeatureGuardProps to use the new type alias name.apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx (1)
16-23: Consider using a type alias instead of an interface.Based on learnings from this repository, type aliases are preferred over interface declarations for object shapes. Interfaces should only be used when declaration merging or
implements/extendsis needed.♻️ Suggested refactor
-export interface NoSubscriptionTooltipProps { +export type NoSubscriptionTooltipProps = { children: React.ReactElement placement?: 'top' | 'bottom' | 'left' | 'right' feature?: AvailableFeature | AvailableFeature[] path?: string skipTooltip?: boolean b2bAppId?: string }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/components/NoSubscriptionTooltip.tsx` around lines 16 - 23, Replace the interface declaration NoSubscriptionTooltipProps with a type alias that defines the same object shape (e.g., type NoSubscriptionTooltipProps = { children: React.ReactElement; placement?: 'top'|'bottom'|'left'|'right'; feature?: AvailableFeature|AvailableFeature[]; path?: string; skipTooltip?: boolean; b2bAppId?: string }). Update any references to NoSubscriptionTooltipProps (component prop typing in NoSubscriptionTooltip) to use the new type alias; do not change semantics or optionality of fields.apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx (1)
129-130: Unused variableisAppAvailableForTariff.The variable
isAppAvailableForTariffis computed on line 130 but is never used in the component logic (only incorrectly listed in useMemo deps). SinceSubscriptionGuardWithTooltipnow handles the availability check internally viab2bAppId, this variable can be removed along with theisB2BAppEnabledimport fromuseOrganizationSubscription.♻️ Proposed cleanup
- const { isB2BAppEnabled } = useOrganizationSubscription() - const isAppAvailableForTariff = isB2BAppEnabled(id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx` around lines 129 - 130, Remove the unused availability check: delete the isAppAvailableForTariff constant and the isB2BAppEnabled import from useOrganizationSubscription in TopCard.tsx (the variable isAppAvailableForTariff and the hook import are unused), and ensure any useMemo or effect deps no longer reference isAppAvailableForTariff; rely on SubscriptionGuardWithTooltip via its b2bAppId prop which now handles availability internally.
🤖 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/miniapp/components/AppDescription/TopCard.tsx`:
- Around line 226-230: The fallback passed into SubscriptionGuardWithTooltip
uses the children prop on Button (children={UnavailableForTariffMessage}) which
static analysis flags; change the fallback to use JSX children syntax instead by
rendering Button with the disabled prop and placing UnavailableForTariffMessage
between its opening and closing tags (keep the surrounding span if needed), and
ensure the primary Button inside SubscriptionGuardWithTooltip remains unchanged;
update references to SubscriptionGuardWithTooltip, Button,
UnavailableForTariffMessage, buttonProps and id accordingly.
In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx`:
- Line 9: The prop name in the SubscriptionGuardWithTooltip component is
inconsistent: the Props interface declares appId?: string but the component
destructures b2bAppId (and JSDoc mentions b2bAppId). Update the Props interface
and all references to use the same name (choose either appId or b2bAppId) —
e.g., rename the interface field to b2bAppId?: string (or change the
destructuring and JSDoc to appId) so SubscriptionGuardWithTooltip’s parameter
destructuring, JSDoc, and any usages align with the chosen symbol
(SubscriptionGuardWithTooltip, Props, appId/b2bAppId).
In `@apps/condo/pages/_app.tsx`:
- Around line 408-410: The miniappTooltip arrow function currently passes
children via the children prop to NoSubscriptionTooltip; change it to use JSX
children syntax instead: return <NoSubscriptionTooltip b2bAppId={app.id}
placement={placement}>{element}</NoSubscriptionTooltip> from the miniappTooltip
function so NoSubscriptionTooltip receives its child element as standard JSX
children (keep the same props b2bAppId and placement and the parameter names
element/placement).
---
Outside diff comments:
In `@apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx`:
- Line 161: In TopCard.tsx, remove the unused dependencies
isAppAvailableForTariff and UnavailableForTariffMessage from the useMemo
dependency array that computes buttonProps (the useMemo callback does not
reference them), leaving only the actual used variables (id, appUrl,
contextStatus, connectAction, intl, router, canManageB2BApps, accessible) so
buttonProps won't recompute unnecessarily.
---
Nitpick comments:
In `@apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx`:
- Around line 129-130: Remove the unused availability check: delete the
isAppAvailableForTariff constant and the isB2BAppEnabled import from
useOrganizationSubscription in TopCard.tsx (the variable isAppAvailableForTariff
and the hook import are unused), and ensure any useMemo or effect deps no longer
reference isAppAvailableForTariff; rely on SubscriptionGuardWithTooltip via its
b2bAppId prop which now handles availability internally.
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 16-23: Replace the interface declaration
NoSubscriptionTooltipProps with a type alias that defines the same object shape
(e.g., type NoSubscriptionTooltipProps = { children: React.ReactElement;
placement?: 'top'|'bottom'|'left'|'right'; feature?:
AvailableFeature|AvailableFeature[]; path?: string; skipTooltip?: boolean;
b2bAppId?: string }). Update any references to NoSubscriptionTooltipProps
(component prop typing in NoSubscriptionTooltip) to use the new type alias; do
not change semantics or optionality of fields.
In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx`:
- Around line 6-11: Replace the interface SubscriptionFeatureGuardProps with a
type alias while keeping the exact shape and generics (retain
Omit<NoSubscriptionTooltipProps, 'children'> and properties children, feature,
appId, fallback) so consumers of SubscriptionGuardWithTooltip continue to work
unchanged; update any imports/exports that reference
SubscriptionFeatureGuardProps to use the new type alias name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: df44de9d-c311-407c-aa5e-50dcd8a2b422
📒 Files selected for processing (4)
apps/condo/domains/miniapp/components/AppDescription/TopCard.tsxapps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsxapps/condo/pages/_app.tsx
| <SubscriptionGuardWithTooltip b2bAppId={id} fallback={ | ||
| <span><Button {...buttonProps} disabled children={UnavailableForTariffMessage}/></span> | ||
| }> | ||
| <Button {...buttonProps}/> | ||
| )} | ||
| </SubscriptionGuardWithTooltip> |
There was a problem hiding this comment.
Avoid passing children using the children prop; use JSX children syntax instead.
The static analysis tool flagged this as an error. Using the children prop directly is discouraged—prefer the standard JSX children pattern for clarity and to avoid potential issues.
🐛 Proposed fix
<SubscriptionGuardWithTooltip b2bAppId={id} fallback={
- <span><Button {...buttonProps} disabled children={UnavailableForTariffMessage}/></span>
+ <span><Button {...buttonProps} disabled>{UnavailableForTariffMessage}</Button></span>
}>
<Button {...buttonProps}/>
</SubscriptionGuardWithTooltip>📝 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.
| <SubscriptionGuardWithTooltip b2bAppId={id} fallback={ | |
| <span><Button {...buttonProps} disabled children={UnavailableForTariffMessage}/></span> | |
| }> | |
| <Button {...buttonProps}/> | |
| )} | |
| </SubscriptionGuardWithTooltip> | |
| <SubscriptionGuardWithTooltip b2bAppId={id} fallback={ | |
| <span><Button {...buttonProps} disabled>{UnavailableForTariffMessage}</Button></span> | |
| }> | |
| <Button {...buttonProps}/> | |
| </SubscriptionGuardWithTooltip> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx` around
lines 226 - 230, The fallback passed into SubscriptionGuardWithTooltip uses the
children prop on Button (children={UnavailableForTariffMessage}) which static
analysis flags; change the fallback to use JSX children syntax instead by
rendering Button with the disabled prop and placing UnavailableForTariffMessage
between its opening and closing tags (keep the surrounding span if needed), and
ensure the primary Button inside SubscriptionGuardWithTooltip remains unchanged;
update references to SubscriptionGuardWithTooltip, Button,
UnavailableForTariffMessage, buttonProps and id accordingly.
apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx
Outdated
Show resolved
Hide resolved
| const miniappTooltip = ({ element, placement }) => ( | ||
| <NoSubscriptionTooltip b2bAppId={app.id} children={element} placement={placement} /> | ||
| ) |
There was a problem hiding this comment.
Avoid passing children using the children prop; use JSX children syntax instead.
The static analysis tool flagged this pattern. Use standard JSX children syntax for clarity.
🐛 Proposed fix
const miniappTooltip = ({ element, placement }) => (
- <NoSubscriptionTooltip b2bAppId={app.id} children={element} placement={placement} />
+ <NoSubscriptionTooltip b2bAppId={app.id} placement={placement}>
+ {element}
+ </NoSubscriptionTooltip>
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const miniappTooltip = ({ element, placement }) => ( | |
| <NoSubscriptionTooltip b2bAppId={app.id} children={element} placement={placement} /> | |
| ) | |
| const miniappTooltip = ({ element, placement }) => ( | |
| <NoSubscriptionTooltip b2bAppId={app.id} placement={placement}> | |
| {element} | |
| </NoSubscriptionTooltip> | |
| ) |
🧰 Tools
🪛 Biome (2.4.6)
[error] 409-409: 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/_app.tsx` around lines 408 - 410, The miniappTooltip arrow
function currently passes children via the children prop to
NoSubscriptionTooltip; change it to use JSX children syntax instead: return
<NoSubscriptionTooltip b2bAppId={app.id}
placement={placement}>{element}</NoSubscriptionTooltip> from the miniappTooltip
function so NoSubscriptionTooltip receives its child element as standard JSX
children (keep the same props b2bAppId and placement and the parameter names
element/placement).
| ? feature.every(f => isFeatureAvailable(f)) | ||
| : isFeatureAvailable(feature) | ||
| : false | ||
| const isAvailable = b2bAppId |
There was a problem hiding this comment.
Please don't use nested ternary operator. Use if else inside useMemo instead
| ? isAppAvailableForTariff | ||
| : feature | ||
| ? Array.isArray(feature) | ||
| ? feature.every(f => isFeatureAvailable(f)) |
There was a problem hiding this comment.
| ? feature.every(f => isFeatureAvailable(f)) | |
| ? feature.every(isFeatureAvailable) |
apps/condo/pages/_app.tsx
Outdated
| disabled={disabled} | ||
| isCollapsed={isCollapsed} | ||
| toolTipDecorator={disabled ? wrapElementIntoNoOrganizationToolTip : null} | ||
| toolTipDecorator={disabled ? wrapElementIntoNoOrganizationToolTip : !isAppAvailable ? miniappTooltip : null} |
There was a problem hiding this comment.
Calculate it in useMemo and without nested ternary operator
toplenboren
left a comment
toplenboren
left a comment
There was a problem hiding this comment.
Please address @nomerdvadcatpyat 's feedback
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.
Actionable comments posted: 2
🧹 Nitpick comments (3)
apps/condo/pages/_app.tsx (1)
384-386: Samechildrenprop pattern issue exists here.Line 385 also uses the
childrenprop instead of JSX children syntax. For consistency, both tooltip wrapper functions should use the same pattern.♻️ Proposed fix
const featureTooltip = ({ element, placement }) => ( - <NoSubscriptionTooltip path={`/${item.path}`} children={element} placement={placement} /> + <NoSubscriptionTooltip path={`/${item.path}`} placement={placement}> + {element} + </NoSubscriptionTooltip> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/pages/_app.tsx` around lines 384 - 386, The featureTooltip function is passing the wrapped node via a children prop to NoSubscriptionTooltip instead of using JSX children syntax; update featureTooltip ({ element, placement }) => (<NoSubscriptionTooltip path={`/${item.path}`}>{element}</NoSubscriptionTooltip>) to match the other tooltip wrapper and ensure consistent usage of JSX children with the NoSubscriptionTooltip component (refer to featureTooltip, NoSubscriptionTooltip and item.path).apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx (1)
16-23: Consider using a type alias instead of interface.Based on learnings, the repository prefers type aliases over interfaces for object shapes unless declaration merging or
extends/implementsis needed. SinceNoSubscriptionTooltipPropsis a standalone type, consider:♻️ Suggested change
-export interface NoSubscriptionTooltipProps { +export type NoSubscriptionTooltipProps = { children: React.ReactElement placement?: 'top' | 'bottom' | 'left' | 'right' feature?: AvailableFeature | AvailableFeature[] path?: string skipTooltip?: boolean b2bAppId?: string }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/components/NoSubscriptionTooltip.tsx` around lines 16 - 23, Replace the interface NoSubscriptionTooltipProps with a type alias: convert the declaration "interface NoSubscriptionTooltipProps { ... }" to "type NoSubscriptionTooltipProps = { ... }" keeping all property names and types (children: React.ReactElement, placement?: 'top'|'bottom'|'left'|'right', feature?: AvailableFeature | AvailableFeature[], path?: string, skipTooltip?: boolean, b2bAppId?: string) exactly the same so callers of NoSubscriptionTooltipProps and the component using it are unaffected; update any imports/exports if needed to use the new type name.apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx (1)
53-55: Potential runtime issue whenfeatureisundefined.When
featureisundefinedand not an array, line 55 callsisFeatureAvailable(feature)which passesundefinedto the hook. Based on the relevant code snippet, this constructs the key'undefinedEndAt'and returnsfalse.While this "works" (returns
false), it relies on implicit behavior. Consider adding an explicit guard:♻️ Proposed improvement for clarity
const hasFeature = Array.isArray(feature) ? feature.every(isFeatureAvailable) - : isFeatureAvailable(feature) + : feature ? isFeatureAvailable(feature) : false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx` around lines 53 - 55, The current hasFeature computation calls isFeatureAvailable(feature) when feature is not an array, which passes undefined into the hook and relies on implicit behavior; change the guard so you never call isFeatureAvailable with undefined — update the hasFeature expression in SubscriptionGuardWithTooltip.tsx (the hasFeature/feature logic) to explicitly check for undefined (e.g., return false if feature is null/undefined) or provide a default before calling isFeatureAvailable; alternatively, add an early return or default param to ensure isFeatureAvailable is only invoked with a valid feature identifier.
🤖 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 56-68: NoSubscriptionTooltip and SubscriptionGuardWithTooltip have
mismatched defaults when neither b2bAppId nor feature is provided; make their
availability logic consistent. Update NoSubscriptionTooltip's isAvailable (in
the useMemo) to treat the "neither provided" case the same as
SubscriptionGuardWithTooltip (i.e., return isAppAvailableForTariff/default true)
or alternatively change SubscriptionGuardWithTooltip's isAppAvailableForTariff
default to false so both components behave the same; pick one approach and apply
it to both places (referencing NoSubscriptionTooltip::isAvailable,
SubscriptionGuardWithTooltip::isAppAvailableForTariff and the b2bAppId/feature
checks) so the same input state yields the same availability result across
components.
In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx`:
- Around line 51-59: SubscriptionGuardWithTooltip currently treats missing
b2bAppId as app-available (isAppAvailableForTariff defaults to true) causing the
guard to pass when both props are omitted; update the component to require at
least one of the props and fail the guard otherwise: change
isAppAvailableForTariff from "b2bAppId ? isB2BAppEnabled(b2bAppId) : true" to
either a strict boolean (e.g. "b2bAppId ? isB2BAppEnabled(b2bAppId) : false")
and add a runtime check at the top of SubscriptionGuardWithTooltip that throws
or returns a clear error/log when both feature and b2bAppId are undefined, or
alternatively assert the required prop presence and throw a descriptive Error
mentioning SubscriptionGuardWithTooltip, feature, and b2bAppId so callers must
provide at least one.
---
Nitpick comments:
In `@apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsx`:
- Around line 16-23: Replace the interface NoSubscriptionTooltipProps with a
type alias: convert the declaration "interface NoSubscriptionTooltipProps { ...
}" to "type NoSubscriptionTooltipProps = { ... }" keeping all property names and
types (children: React.ReactElement, placement?: 'top'|'bottom'|'left'|'right',
feature?: AvailableFeature | AvailableFeature[], path?: string, skipTooltip?:
boolean, b2bAppId?: string) exactly the same so callers of
NoSubscriptionTooltipProps and the component using it are unaffected; update any
imports/exports if needed to use the new type name.
In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx`:
- Around line 53-55: The current hasFeature computation calls
isFeatureAvailable(feature) when feature is not an array, which passes undefined
into the hook and relies on implicit behavior; change the guard so you never
call isFeatureAvailable with undefined — update the hasFeature expression in
SubscriptionGuardWithTooltip.tsx (the hasFeature/feature logic) to explicitly
check for undefined (e.g., return false if feature is null/undefined) or provide
a default before calling isFeatureAvailable; alternatively, add an early return
or default param to ensure isFeatureAvailable is only invoked with a valid
feature identifier.
In `@apps/condo/pages/_app.tsx`:
- Around line 384-386: The featureTooltip function is passing the wrapped node
via a children prop to NoSubscriptionTooltip instead of using JSX children
syntax; update featureTooltip ({ element, placement }) =>
(<NoSubscriptionTooltip
path={`/${item.path}`}>{element}</NoSubscriptionTooltip>) to match the other
tooltip wrapper and ensure consistent usage of JSX children with the
NoSubscriptionTooltip component (refer to featureTooltip, NoSubscriptionTooltip
and item.path).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d2a9eeec-af64-4729-8c2a-e151b24c4d38
📒 Files selected for processing (3)
apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsxapps/condo/pages/_app.tsx
| const isAvailable = useMemo(() => { | ||
| if (b2bAppId) { | ||
| return isAppAvailableForTariff | ||
| } else if (feature) { | ||
| if (Array.isArray(feature)) { | ||
| return feature.every(isFeatureAvailable) | ||
| } else { | ||
| return isFeatureAvailable(feature) | ||
| } | ||
| } else { | ||
| return false | ||
| } | ||
| }, [b2bAppId, isAppAvailableForTariff, feature, isFeatureAvailable]) |
There was a problem hiding this comment.
Inconsistent behavior when neither feature nor b2bAppId is provided.
In NoSubscriptionTooltip, when neither is provided, isAvailable returns false (line 66), so the tooltip is shown.
However, in SubscriptionGuardWithTooltip, when neither is provided, isAppAvailableForTariff defaults to true (line 51), so children are rendered.
This inconsistency could lead to confusing behavior where the same scenario produces different outcomes depending on which component is used directly.
Also applies to: 51-59
🤖 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 56 - 68, NoSubscriptionTooltip and SubscriptionGuardWithTooltip have
mismatched defaults when neither b2bAppId nor feature is provided; make their
availability logic consistent. Update NoSubscriptionTooltip's isAvailable (in
the useMemo) to treat the "neither provided" case the same as
SubscriptionGuardWithTooltip (i.e., return isAppAvailableForTariff/default true)
or alternatively change SubscriptionGuardWithTooltip's isAppAvailableForTariff
default to false so both components behave the same; pick one approach and apply
it to both places (referencing NoSubscriptionTooltip::isAvailable,
SubscriptionGuardWithTooltip::isAppAvailableForTariff and the b2bAppId/feature
checks) so the same input state yields the same availability result across
components.
apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx
Show resolved
Hide resolved
| if (disabled) { | ||
| tooltipDecorator = wrapElementIntoNoOrganizationToolTip | ||
| } else if (!isAppAvailable) { | ||
| tooltipDecorator = miniappTooltip |
There was a problem hiding this comment.
This tooltip only for miniapps?
There was a problem hiding this comment.
Yes, because we use it in appsByCategories
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.
Actionable comments posted: 2
🤖 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 149-153: The tooltip currently chooses the trial CTA based only on
hasActivatedAnyTrial, which can be false while bestPlanWithFeature is null after
b2bAppId filtering; update the decision logic for both buttonAction and button
text so they also consider bestPlanWithFeature. Concretely, change the condition
that assigns buttonAction (and the logic that selects the CTA label used in
tooltipTitle) to fall back to handleViewPlans whenever bestPlanWithFeature is
null (i.e., use handleActivateTrial only when bestPlanWithFeature exists and
hasActivatedAnyTrial is false), referencing buttonAction, bestPlanWithFeature,
hasActivatedAnyTrial, handleViewPlans and handleActivateTrial to locate and
update the expressions.
In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx`:
- Around line 53-55: The current hasFeature computation calls
isFeatureAvailable(undefined) when the prop feature is omitted; change it to
explicitly short-circuit when feature is null/undefined so we don't call
isFeatureAvailable with invalid input. Update the hasFeature line in
SubscriptionGuardWithTooltip (the hasFeature constant) to first check feature
=== undefined || feature === null and return true (i.e., do not block) otherwise
evaluate Array.isArray(feature) ? feature.every(f => isFeatureAvailable(f)) :
isFeatureAvailable(feature); keep references to hasFeature and
isFeatureAvailable intact so callers like TopCard (which passes only b2bAppId)
won't trigger isFeatureAvailable(undefined).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4c12faea-9c88-467e-933a-2cdf5f712101
📒 Files selected for processing (2)
apps/condo/domains/subscription/components/NoSubscriptionTooltip.tsxapps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx
| const buttonAction = (!feature && !b2bAppId) || hasActivatedAnyTrial ? handleViewPlans : handleActivateTrial | ||
|
|
||
| const tooltipTitle = ( | ||
| <Space size={12} direction='vertical'> | ||
| <Typography.Text size='small'>{hasActivatedAnyTrial ? UpgradePlanMessage : FeatureLockedMessage}</Typography.Text> | ||
| <Typography.Text size='small'>{hasActivatedAnyTrial && !isLoading ? UpgradePlanMessage : FeatureLockedMessage}</Typography.Text> |
There was a problem hiding this comment.
Use the "view plans" CTA when no matching trial exists.
After the new b2bAppId filtering, bestPlanWithFeature can be null even when hasActivatedAnyTrial is false (for example, no trial-enabled plan includes this miniapp, or all matching plans are already activated). In that state the tooltip still renders the trial CTA and only redirects to settings inside handleActivateTrial, so the copy promises a free trial that does not exist. Please derive the button text/action from bestPlanWithFeature as well.
💡 Proposed fix
- const buttonText = hasActivatedAnyTrial ? ViewPlansButton : TryTrialButton
-
- const buttonAction = (!feature && !b2bAppId) || hasActivatedAnyTrial ? handleViewPlans : handleActivateTrial
+ const canStartTrial = Boolean(bestPlanWithFeature) && !hasActivatedAnyTrial
+ const buttonText = canStartTrial ? TryTrialButton : ViewPlansButton
+
+ const buttonAction = canStartTrial ? handleActivateTrial : handleViewPlans🤖 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 149 - 153, The tooltip currently chooses the trial CTA based only on
hasActivatedAnyTrial, which can be false while bestPlanWithFeature is null after
b2bAppId filtering; update the decision logic for both buttonAction and button
text so they also consider bestPlanWithFeature. Concretely, change the condition
that assigns buttonAction (and the logic that selects the CTA label used in
tooltipTitle) to fall back to handleViewPlans whenever bestPlanWithFeature is
null (i.e., use handleActivateTrial only when bestPlanWithFeature exists and
hasActivatedAnyTrial is false), referencing buttonAction, bestPlanWithFeature,
hasActivatedAnyTrial, handleViewPlans and handleActivateTrial to locate and
update the expressions.
| const hasFeature = Array.isArray(feature) | ||
| ? feature.every(f => isFeatureAvailable(f)) | ||
| : isFeatureAvailable(feature) |
There was a problem hiding this comment.
Don't call isFeatureAvailable with undefined.
apps/condo/domains/miniapp/components/AppDescription/TopCard.tsx:226-230 already uses this component with only b2bAppId, so Line 55 currently evaluates isFeatureAvailable(undefined) on every render. useOrganizationSubscription.ts:66-79 happens to turn that into false, but this guard is relying on an invalid input shape instead of explicitly short-circuiting the feature branch.
💡 Proposed fix
- const hasFeature = Array.isArray(feature)
- ? feature.every(f => isFeatureAvailable(f))
- : isFeatureAvailable(feature)
+ const hasFeature = feature
+ ? Array.isArray(feature)
+ ? feature.every(f => isFeatureAvailable(f))
+ : isFeatureAvailable(feature)
+ : false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const hasFeature = Array.isArray(feature) | |
| ? feature.every(f => isFeatureAvailable(f)) | |
| : isFeatureAvailable(feature) | |
| const hasFeature = feature | |
| ? Array.isArray(feature) | |
| ? feature.every(f => isFeatureAvailable(f)) | |
| : isFeatureAvailable(feature) | |
| : false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/condo/domains/subscription/components/SubscriptionGuardWithTooltip.tsx`
around lines 53 - 55, The current hasFeature computation calls
isFeatureAvailable(undefined) when the prop feature is omitted; change it to
explicitly short-circuit when feature is null/undefined so we don't call
isFeatureAvailable with invalid input. Update the hasFeature line in
SubscriptionGuardWithTooltip (the hasFeature constant) to first check feature
=== undefined || feature === null and return true (i.e., do not block) otherwise
evaluate Array.isArray(feature) ? feature.every(f => isFeatureAvailable(f)) :
isFeatureAvailable(feature); keep references to hasFeature and
isFeatureAvailable intact so callers like TopCard (which passes only b2bAppId)
won't trigger isFeatureAvailable(undefined).
|
Alexander-Turkin
deleted the
fix/condo/DOMA-13033/fit-tooltip-for-miniapps
branch
* feat(condo): DOMA-13033 fix tooltip for miniapps * feat(condo): DOMA-13033 review fix * feat(condo): DOMA-13033 review fix (cherry picked from commit c3cfeb8)
4 participants
Summary by CodeRabbit
New Features
Improvements