-
Notifications
You must be signed in to change notification settings - Fork 4.1k
feat: skip license page for cloud billing users #41102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… redirection logic
WalkthroughThe SignupSuccess component was refactored to introduce asynchronous control over user redirection, prevent concurrent redirects, and incorporate multi-organization billing status into the redirection logic. State management for redirection was improved, and auto-redirect conditions were consolidated into a single computed flag, with UI feedback provided during redirection. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignupSuccess Component
participant RedirectHelper
User->>SignupSuccess Component: Loads page
SignupSuccess Component->>SignupSuccess Component: Compute shouldAutoRedirect
alt shouldAutoRedirect is true and not isRedirecting
SignupSuccess Component->>SignupSuccess Component: Set isRedirecting = true
SignupSuccess Component->>RedirectHelper: redirectUsingQueryParam(isMultiOrgEnabled)
RedirectHelper-->>SignupSuccess Component: Redirect or error
SignupSuccess Component->>SignupSuccess Component: Set isRedirecting = false (on error)
else User clicks "Get Started"
User->>SignupSuccess Component: Clicks button
SignupSuccess Component->>SignupSuccess Component: Set isRedirecting = true
SignupSuccess Component->>RedirectHelper: redirectUsingQueryParam(isMultiOrgEnabled)
RedirectHelper-->>SignupSuccess Component: Redirect or error
SignupSuccess Component->>SignupSuccess Component: Set isRedirecting = false (on error)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/client/src/pages/setup/SignupSuccess.tsx (1)
29-29
: Consider aligning variable name with hook semanticsThe variable name
isMultiOrgEnabled
doesn't clearly convey that it's checking cloud billing status. Consider renaming toisCloudBillingEnabled
for clarity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/client/src/pages/setup/SignupSuccess.tsx
(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
app/client/src/pages/setup/SignupSuccess.tsx (7)
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-10-08T15:32:39.374Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: saiprabhu-dandanayak
PR: appsmithorg/appsmith#33911
File: app/client/src/api/TemplatesApi.ts:0-0
Timestamp: 2024-07-26T21:12:57.228Z
Learning: User saiprabhu-dandanayak has confirmed the removal of the `any` type in the TypeScript code for better type safety in the `TemplatesApi.ts` file.
Learnt from: sneha122
PR: appsmithorg/appsmith#30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-07-26T21:12:57.228Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Learnt from: sneha122
PR: appsmithorg/appsmith#30012
File: app/client/src/pages/Editor/DataSourceEditor/RestAPIDatasourceForm.tsx:679-682
Timestamp: 2024-10-08T15:32:34.115Z
Learning: The user `sneha122` has confirmed the resolution of the feedback regarding the redundancy of `|| false` in the `_.get` expression within the `RestAPIDatasourceForm.tsx` file.
Learnt from: alex-golovanov
PR: appsmithorg/appsmith#39379
File: app/client/src/pages/AppIDE/components/LibrariesList/JSLibrariesSection.tsx:0-0
Timestamp: 2025-02-20T13:36:10.743Z
Learning: When reviewing React components, ensure that useMemo and useEffect hooks include all variables used within their callbacks in their dependency arrays to prevent stale values and potential bugs.
Learnt from: brayn003
PR: appsmithorg/appsmith#38088
File: app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts:40-43
Timestamp: 2024-12-11T08:25:39.197Z
Learning: In `app/client/src/git/components/GitContextProvider/hooks/useGitBranches.ts`, the `useMemo` hook includes dependencies `artifactType` and `baseArtifactId` in its dependency array.
Learnt from: brayn003
PR: appsmithorg/appsmith#36989
File: app/client/packages/git/src/actions/helpers/createSingleArtifactAction.ts:52-68
Timestamp: 2024-10-21T13:17:59.251Z
Learning: In this codebase, when using Redux Toolkit's createSlice from Redux Toolkit, we can directly mutate the state within reducers since Immer handles immutability internally.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: perform-test / rts-build / build
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (3)
app/client/src/pages/setup/SignupSuccess.tsx (3)
89-94
: Good consolidation of redirect conditionsThe
shouldAutoRedirect
variable improves code readability by clearly expressing when automatic redirection should occur.
96-100
: Well-implemented auto-redirect logicThe useEffect properly handles automatic redirection with appropriate guard conditions to prevent race conditions.
110-116
: Correct implementation of loading stateGood change from imperative redirect call to declarative condition check. The loading spinner provides appropriate user feedback during redirection.
}, [ | ||
dispatch, | ||
isNonInvitedUser, | ||
isOnLoginPage, | ||
redirectUrl, | ||
shouldEnableFirstTimeUserOnboarding, | ||
validLicense, | ||
isAiAgentInstanceEnabled, | ||
isMultiOrgEnabled, | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix useCallback dependency array
The dependency array includes isNonInvitedUser
which isn't used in the callback. Also missing isRedirecting
which is used in the function.
}, [
dispatch,
- isNonInvitedUser,
+ isRedirecting,
isOnLoginPage,
redirectUrl,
shouldEnableFirstTimeUserOnboarding,
validLicense,
isAiAgentInstanceEnabled,
isMultiOrgEnabled,
]);
📝 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.
}, [ | |
dispatch, | |
isNonInvitedUser, | |
isOnLoginPage, | |
redirectUrl, | |
shouldEnableFirstTimeUserOnboarding, | |
validLicense, | |
isAiAgentInstanceEnabled, | |
isMultiOrgEnabled, | |
]); | |
}, [ | |
dispatch, | |
isRedirecting, | |
isOnLoginPage, | |
redirectUrl, | |
shouldEnableFirstTimeUserOnboarding, | |
validLicense, | |
isAiAgentInstanceEnabled, | |
isMultiOrgEnabled, | |
]); |
🤖 Prompt for AI Agents
In app/client/src/pages/setup/SignupSuccess.tsx around lines 60 to 69, the
useCallback dependency array incorrectly includes isNonInvitedUser which is not
used inside the callback, and it is missing isRedirecting which is used. Remove
isNonInvitedUser from the dependency array and add isRedirecting to ensure the
dependencies accurately reflect the variables used in the callback function.
}); | ||
await redirectUsingQueryParam(); | ||
}, | ||
[redirectUsingQueryParam], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add missing dependency to useCallback
The dispatch
function is used within the callback but missing from the dependency array.
- [redirectUsingQueryParam],
+ [redirectUsingQueryParam, dispatch],
📝 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.
[redirectUsingQueryParam], | |
[redirectUsingQueryParam, dispatch], |
🤖 Prompt for AI Agents
In app/client/src/pages/setup/SignupSuccess.tsx at line 86, the useCallback
hook's dependency array is missing the 'dispatch' function, which is used inside
the callback. Add 'dispatch' to the dependency array to ensure the callback
updates correctly when 'dispatch' changes.
Description
This PR enhances the signup success flow to automatically skip the license page when cloud billing is enabled, providing a smoother onboarding experience for cloud users.
Key Changes
🚀 New Features
useIsCloudBillingEnabled()
hook🔧 Improvements
shouldAutoRedirect
variable for better readability🛠️ Technical Details
isMultiOrgEnabled
flag to signup redirect parametersredirectUsingQueryParam
andonGetStarted
functions asyncisRedirecting
state to track redirect statusImpact
Automation
/ok-to-test tags="@tag.Sanity, @tag.Authentication, @tag.LicenseAndBilling"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/16167808138
Commit: ab4247c
Cypress dashboard.
Tags:
@tag.Sanity, @tag.Authentication, @tag.LicenseAndBilling
Spec:
Wed, 09 Jul 2025 12:00:44 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Bug Fixes