chore: redirect ai agent user to app with carbon modal open#39999
chore: redirect ai agent user to app with carbon modal open#39999
Conversation
WalkthroughThe changes update the user redirection functionality. The Changes
Sequence Diagram(s)sequenceDiagram
participant SS as SignupSuccess
participant RU as redirectUserAfterSignup
participant D as Dispatch
participant H as History
SS->>RU: Call redirectUserAfterSignup({redirectUrl, shouldEnableFirstTimeUserOnboarding, validLicense, dispatch, isAiAgentFlowEnabled})
RU->>D: dispatch(setCurrentApplicationIdForCreateNewApp) [if base IDs provided]
RU->>H: history.replace(APPLICATIONS_URL)
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:
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
|
github-actions
bot
added
the
skip-changelog
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/client/src/ce/utils/signupHelpers.ts (1)
60-66: Update comment to reflect current implementationThe comment still references
firstTimeUserOnboardingInitbut that import and functionality has been removed./** ! Dev Note: - * setCurrentApplicationIdForCreateNewApp & firstTimeUserOnboardingInit + * setCurrentApplicationIdForCreateNewApp * in the following block support only applicationId * but since baseId and id are same for applications created outside git context * and since these redux actions are only called during onboarding, * passing baseApplicationId as applicationId should be fine * **/
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/client/src/ce/utils/signupHelpers.ts(3 hunks)app/client/src/pages/setup/SignupSuccess.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (1)
app/client/src/pages/setup/SignupSuccess.tsx (1)
app/client/src/ce/utils/signupHelpers.ts (1)
redirectUserAfterSignup(29-94)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-lint / client-lint
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (6)
app/client/src/pages/setup/SignupSuccess.tsx (2)
15-15: New selector import for AI agent flowThis import adds the capability to detect if the AI agent flow is enabled, which is now used in the redirection logic.
31-31: Added state for AI agent flow detectionGood addition to capture whether the AI agent flow is enabled via the selector.
app/client/src/ce/utils/signupHelpers.ts (4)
1-1: Removed unused importRemoved unused import for
firstTimeUserOnboardingInitwhile keepingsetCurrentApplicationIdForCreateNewApp.
19-20: Added type import for Redux dispatchGood practice to properly type the dispatch function using the Redux Dispatch type.
21-27: Added interface for function parametersGood refactoring to create a typed interface for the function parameters, which improves maintainability and type safety.
68-71: Simplified redirection logicThe conditional logic checking for
isEnabledForCreateNewhas been removed, and the code now directly dispatches the action and redirects to Applications URL. This aligns with the PR objective to streamline navigation.
| redirectUserAfterSignup({ | ||
| redirectUrl, | ||
| shouldEnableFirstTimeUserOnboarding, | ||
| validLicense, | ||
| dispatch, | ||
| isNonInvitedUser, | ||
| ), | ||
| isAiAgentFlowEnabled, | ||
| }), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Updated function call to use object parameter
The function call has been updated to use an object parameter which improves maintainability. However, the dependency array is empty which could lead to stale values if the dependencies change.
Consider adding the dependencies to the array:
const redirectUsingQueryParam = useCallback(
() =>
redirectUserAfterSignup({
redirectUrl,
shouldEnableFirstTimeUserOnboarding,
validLicense,
dispatch,
isAiAgentFlowEnabled,
}),
- [],
+ [redirectUrl, shouldEnableFirstTimeUserOnboarding, validLicense, dispatch, isAiAgentFlowEnabled],
);📝 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.
| redirectUserAfterSignup({ | |
| redirectUrl, | |
| shouldEnableFirstTimeUserOnboarding, | |
| validLicense, | |
| dispatch, | |
| isNonInvitedUser, | |
| ), | |
| isAiAgentFlowEnabled, | |
| }), | |
| const redirectUsingQueryParam = useCallback( | |
| () => | |
| redirectUserAfterSignup({ | |
| redirectUrl, | |
| shouldEnableFirstTimeUserOnboarding, | |
| validLicense, | |
| dispatch, | |
| isAiAgentFlowEnabled, | |
| }), | |
| [redirectUrl, shouldEnableFirstTimeUserOnboarding, validLicense, dispatch, isAiAgentFlowEnabled], | |
| ); |
| export const redirectUserAfterSignup = ( | ||
| redirectUrl: string, | ||
| shouldEnableFirstTimeUserOnboarding: string | null, | ||
| _validLicense?: boolean, | ||
| // TODO: Fix this the next time the file is edited | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| dispatch?: any, | ||
| isEnabledForCreateNew?: boolean, // is Enabled for only non-invited users | ||
| // TODO: Fix this the next time the file is edited | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| ): any => { | ||
| props: RedirectUserAfterSignupProps, | ||
| ) => { | ||
| const { dispatch, redirectUrl, shouldEnableFirstTimeUserOnboarding } = props; | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Updated function signature to use interface
The function now accepts a single object parameter with a defined interface. However, isAiAgentFlowEnabled is defined in the interface but not used in the function body.
🏁 Script executed:
#!/bin/bash
# Check if isAiAgentFlowEnabled is being used elsewhere in the codebase
echo "Searching for isAiAgentFlowEnabled usage..."
rg "isAiAgentFlowEnabled" --type tsLength of output: 1301
Action Required: Address the Unused Parameter in Signup Helpers
In the updated function signature for redirectUserAfterSignup, the interface now includes the isAiAgentFlowEnabled property. However, this property isn’t being utilized within the function body. Please either remove isAiAgentFlowEnabled from the RedirectUserAfterSignupProps interface if it isn’t needed in this context, or update the function logic to incorporate it appropriately if it's intended to affect the redirection behavior.
- File:
app/client/src/ce/utils/signupHelpers.ts(Lines 29-33) - Suggestion: Remove the unused parameter from the interface or integrate it into the function's logic.
1 participant
/ok-to-test tags="@tag.Sanity"
This is the corresponding CE PR for redirecting ai agent new user to app page directly.
Summary by CodeRabbit
New Features
Refactor
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14189262440
Commit: 03c954c
Cypress dashboard.
Tags:
@tag.SanitySpec:
Tue, 01 Apr 2025 07:46:41 UTC