feat: added global logout feature#39942
Conversation
WalkthroughThis pull request integrates a new logout mechanism into the application’s execution flow. A new action type ( Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 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
|
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14091998430. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
app/client/src/workers/Evaluation/fns/logout.ts (1)
5-5: Consider using a constant for the action type.Using string literals directly in code can lead to issues if the action type needs to be referenced elsewhere in the codebase.
+import { LogoutActionTypes } from "../constants/actionTypes"; + function logoutFnDescriptor(redirectURL: string) { return { - type: "LOGOUT_USER_INIT" as const, + type: LogoutActionTypes.LOGOUT_USER_INIT, payload: { redirectURL, }, }; }app/client/src/ce/utils/autocomplete/EntityDefinitions.ts (2)
282-282: Enhance the function documentation.The current description is very brief compared to other function descriptions in the file. A more comprehensive description would be helpful for developers.
logoutUser: { "!url": "https://docs.appsmith.com/reference/appsmith-framework/widget-actions/store-value", - "!doc": "Logout user", + "!doc": "Logs out the current user and redirects to the specified URL. Useful for implementing custom logout buttons within the application.", "!type": "fn(redirectURL: string) -> void", },
339-339: Improve the example argument for logoutUser.The current example
["url"]is not clear enough. It would be more helpful to provide a clearer example with string literals to show actual usage.logoutUser: { - exampleArgs: ["url"], + exampleArgs: ["'/login'", "'/auth/login'"], },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts(2 hunks)app/client/src/ce/utils/autocomplete/EntityDefinitions.ts(2 hunks)app/client/src/components/editorComponents/ActionCreator/constants.ts(1 hunks)app/client/src/components/editorComponents/ActionCreator/utils.ts(1 hunks)app/client/src/workers/Evaluation/fns/index.ts(4 hunks)app/client/src/workers/Evaluation/fns/logout.ts(1 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
app/client/src/workers/Evaluation/fns/index.ts (1)
app/client/src/workers/Evaluation/fns/logout.ts (2)
logoutUser(14-18)TSLogoutActionType(12-12)
app/client/src/components/editorComponents/ActionCreator/utils.ts (1)
app/client/src/components/editorComponents/ActionCreator/constants.ts (1)
AppsmithFunction(32-37)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-prettier / prettier-check
- GitHub Check: client-build / client-build
🔇 Additional comments (9)
app/client/src/components/editorComponents/ActionCreator/constants.ts (1)
17-17: Addition of logoutUser to APPSMITH_GLOBAL_FUNCTIONS enum looks good.The addition of
logoutUserto the APPSMITH_GLOBAL_FUNCTIONS enum is clean and follows the existing pattern. This makes the function available as a global function throughout the application.app/client/src/components/editorComponents/ActionCreator/utils.ts (1)
118-121: logoutUser added to chainableFns array correctly.The
logoutUserfunction is properly added to thechainableFnsarray, making it available as a chainable function in the action creator. This follows the established pattern for other platform functions.app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts (2)
48-48: Import of logoutSaga correctly added.The
logoutSagais properly imported from "../userSagas", which will be used to handle the logout action.
135-137: Correctly implemented handler for LOGOUT_USER_INIT action type.The new case for "LOGOUT_USER_INIT" in the switch statement correctly calls the
logoutSagawith thetriggerparameter. This follows the same pattern as other action types in the function.app/client/src/workers/Evaluation/fns/index.ts (4)
63-63: Import of logoutUser function and type looks good.The import statement correctly brings in both the
logoutUserfunction and theTSLogoutActionTypetype from the "./logout" module.
118-121: logoutUser added to platformFns array correctly.The
logoutUserfunction is properly added to theplatformFnsarray with the correct name and function reference, following the same pattern as other platform functions.
223-223: LOGOUT_USER_INIT added to ActionTriggerFunctionNames.The "LOGOUT_USER_INIT" action type is properly mapped to the "logoutUser" function name in the ActionTriggerFunctionNames record.
243-244: TSLogoutActionType correctly added to ActionDescription union type.The
TSLogoutActionTypeis properly added to theActionDescriptionunion type, ensuring type safety when handling logout actions.app/client/src/workers/Evaluation/fns/logout.ts (1)
1-18: Implementation looks good.The implementation follows TypeScript best practices with proper type definitions and reuse of parameter types. The function structure is clean and well-organized.
|
Deploy-Preview-URL: https://ce-39942.dp.appsmith.com |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/client/src/ce/sagas/userSagas.tsx (2)
727-739: Function implementation looks good, but URL handling could be improved.The new
globalFunctionLogoutUserfunction properly handles the logout flow with redirect URL support, but there are opportunities to enhance security and robustness.export function* globalFunctionLogoutUser( action: ReduxAction<{ redirectURL: string }>, ) { - const redirectURL = `${AUTH_LOGIN_URL}${action.payload?.redirectURL ? "?redirectUrl=" + action.payload?.redirectURL : ""}`; + const redirectURL = `${AUTH_LOGIN_URL}${action.payload?.redirectURL ? "?redirectUrl=" + encodeURIComponent(action.payload.redirectURL) : ""}`; yield call(logoutSaga, { type: ReduxActionTypes.LOGOUT_USER_INIT, payload: { redirectURL, }, }); }
727-739: Consider adding JSDoc documentation for this global function.Since this function is intended to be used as a global logout mechanism, adding documentation would help other developers understand its purpose and usage.
+/** + * Global function to logout user with optional redirect URL + * @param action Redux action containing redirectURL in payload + */ export function* globalFunctionLogoutUser( action: ReduxAction<{ redirectURL: string }>, ) { const redirectURL = `${AUTH_LOGIN_URL}${action.payload?.redirectURL ? "?redirectUrl=" + action.payload?.redirectURL : ""}`; yield call(logoutSaga, { type: ReduxActionTypes.LOGOUT_USER_INIT, payload: { redirectURL, }, }); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (2)
app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts(2 hunks)app/client/src/ce/sagas/userSagas.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/client/src/ce/sagas/ActionExecution/ActionExecutionSagas.ts
⏰ 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 (1)
app/client/src/ce/sagas/userSagas.tsx (1)
731-731:Details
✅ Verification successful
Verify the queryParam name for consistency.
Ensure that the queryParam name
redirectUrlmatches what the login page expects. I notice you're using camelCase here, but check if the receiving end expects this exact casing.
🏁 Script executed:
#!/bin/bash # Check for references to redirectUrl parameter in login page handling rg -i "redirectUrl|redirect_url|REDIRECT_URL" --type ts --type tsx app/client/src | grep -i loginLength of output: 127
Action Required:
The previous search produced an error due to an unrecognized file type for tsx files. Please run the above revised script to confirm whether the login page (or related authentication logic) is referencing the query parameter "redirectUrl" (camelCase) or a different variant. Once you verify which casing the login page expects, update the code inapp/client/src/ce/sagas/userSagas.tsx(line 731) accordingly to ensure consistency.
🏁 Script executed:
#!/bin/bash # Corrected search for 'redirectUrl' references in login-related TypeScript files. # This command uses glob patterns for both .tsx and .ts files. rg -i -g "*.tsx" -g "*.ts" "redirectUrl|redirect_url|REDIRECT_URL" app/client/src | grep -i loginLength of output: 1907
Query Parameter Naming Verified
- Verified that the login page and other related modules consistently expect the query parameter to be
redirectUrl(camelCase).- The code in
app/client/src/ce/sagas/userSagas.tsxcorrectly appends?redirectUrl=...based on the payload, which aligns with what the login page expects.No changes are needed here.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (5)
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/ActionSelector_JsToNonJSMode_4_spec.ts(2 hunks)app/client/src/ce/constants/messages.ts(1 hunks)app/client/src/components/editorComponents/ActionCreator/FieldGroup/FieldGroupConfig.test.ts(1 hunks)app/client/src/components/editorComponents/ActionCreator/FieldGroup/FieldGroupConfig.ts(2 hunks)app/client/src/components/editorComponents/ActionCreator/viewComponents/ActionBlockTree/utils.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`app/client/cypress/**/**.*`: Review the following e2e test code written using the Cypress test library. Ensure that: - Follow best practices for Cypress code and e2e automation. ...
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/ActionSelector_JsToNonJSMode_4_spec.ts
🧬 Code Definitions (1)
app/client/src/components/editorComponents/ActionCreator/FieldGroup/FieldGroupConfig.ts (1)
app/client/src/ce/constants/messages.ts (1)
LOGOUT_USER(632-632)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: client-lint / client-lint
- GitHub Check: client-build / client-build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (6)
app/client/src/ce/constants/messages.ts (1)
632-632: New logout message constant added correctly.This addition creates a new message constant for the logout user functionality, maintaining consistency with the application's internationalization pattern.
app/client/src/components/editorComponents/ActionCreator/viewComponents/ActionBlockTree/utils.tsx (2)
92-94: Logout icon handler implemented properly.The new case for
AppsmithFunction.logoutUsercorrectly returns the appropriate icon component.
212-215: Proper action heading for logout functionality.The implementation correctly retrieves the URL field or defaults to a prompt for adding a redirect URL when the field is empty.
app/client/src/components/editorComponents/ActionCreator/FieldGroup/FieldGroupConfig.ts (2)
12-12: Message import added for logout functionality.The LOGOUT_USER constant is properly imported to be used in the field group configuration.
171-176: Logout user field group configuration implemented correctly.The configuration properly defines the label, fields, default parameters, and icon for the logout functionality.
app/client/cypress/e2e/Regression/ClientSide/ActionExecution/ActionSelector_JsToNonJSMode_4_spec.ts (1)
18-18: Test case numbering updatedThe test case has been renumbered to "1", which helps maintain sequential ordering in the test suite.
app/client/src/components/editorComponents/ActionCreator/FieldGroup/FieldGroupConfig.test.ts
Show resolved
Hide resolved
...ent/cypress/e2e/Regression/ClientSide/ActionExecution/ActionSelector_JsToNonJSMode_4_spec.ts
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14187453341. |
|
Deploy-Preview-URL: https://ce-39942.dp.appsmith.com |
rahulbarwal
left a comment
rahulbarwal
left a comment
There was a problem hiding this comment.
Overall this looks good. It's great to see this functionality coming in so quickly. Few points:
- Can you please update the PR description with: What is the current method for accomplishing this? What were its shortcomings? What is the motivation behind this change?
- Would setting the default redirect URL to the current URL, rather than directing to the
/applicationsroute, be more beneficial? Can we obtain some product insights on this?
...ent/cypress/e2e/Regression/ClientSide/ActionExecution/ActionSelector_JsToNonJSMode_4_spec.ts
Outdated
Show resolved
Hide resolved
app/client/src/components/editorComponents/ActionCreator/FieldGroup/FieldGroupConfig.test.ts
Show resolved
Hide resolved
Updated the thread for 1 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
app/client/cypress/support/commands.js (1)
174-194: Updated function signature to enhance flexibilityThe
LoginFromAPIcommand now accepts an optionalredirectUrlparameter to enable custom redirection after login. This is a good enhancement that supports the global logout feature implementation.Consider adding JSDoc documentation for the new parameter:
+/** + * Logs in a user via API + * @param {string} uname - The username + * @param {string} pword - The password + * @param {string} [redirectUrl] - Optional URL to redirect to after login + */ Cypress.Commands.add("LoginFromAPI", (uname, pword, redirectUrl) => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
app/client/cypress/support/commands.js(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`app/client/cypress/**/**.*`: Review the following e2e test code written using the Cypress test library. Ensure that: - Follow best practices for Cypress code and e2e automation. ...
app/client/cypress/**/**.*: Review the following e2e test code written using the Cypress test library. Ensure that:
- Follow best practices for Cypress code and e2e automation.
- Avoid using cy.wait in code.
- Avoid using cy.pause in code.
- Avoid using agHelper.sleep().
- Use locator variables for locators and do not use plain strings.
- Use data-* attributes for selectors.
- Avoid Xpaths, Attributes and CSS path.
- Avoid selectors like .btn.submit or button[type=submit].
- Perform logins via API with LoginFromAPI.
- Perform logout via API with LogOutviaAPI.
- Perform signup via API with SignupFromAPI.
- Avoid using it.only.
- Avoid using after and aftereach in test cases.
- Use multiple assertions for expect statements.
- Avoid using strings for assertions.
- Do not use duplicate filenames even with different paths.
- Avoid using agHelper.Sleep, this.Sleep in any file in code.
app/client/cypress/support/commands.js
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: client-lint / client-lint
- GitHub Check: perform-test / rts-build / build
- GitHub Check: client-unit-tests / client-unit-tests
- GitHub Check: perform-test / client-build / client-build
- GitHub Check: client-check-cyclic-deps / check-cyclic-dependencies
- GitHub Check: perform-test / server-build / server-unit-tests
- GitHub Check: client-build / client-build
- GitHub Check: client-prettier / prettier-check
🔇 Additional comments (2)
app/client/cypress/support/commands.js (2)
211-217: Redirect logic properly implementedThe conditional check ensures the location assertion validates against the provided redirectUrl if present, otherwise falls back to the default "/applications" path.
222-225:Details
✅ Verification successful
Network request assertion conditionally applied
Good implementation that only checks for the "getAllWorkspaces" network request when using the default redirect path.
Verify that all test cases using the
LoginFromAPIcommand without the redirectUrl parameter continue to work as expected:
🏁 Script executed:
#!/bin/bash # Find all cases where LoginFromAPI is called in tests rg "cy\.LoginFromAPI\(" --type js app/client/cypress/Length of output: 3308
Network Request Assertion and Test Usage Verified
- The code in
app/client/cypress/support/commands.js(lines 222-225) correctly applies thegetAllWorkspacesnetwork status assertion only when theredirectUrlparameter is not provided.- A grep search across the test files confirmed that all calls to
cy.LoginFromAPIpass only the username and password (i.e., without aredirectUrl), ensuring they rely on the default behavior.- The current implementation appears sound; please ensure that any future tests that might introduce a custom redirect path handle the network assertions appropriately.
3 participants
Description
Added a global
logoutUserfunction to expose the logout functionality for the users to implement their custom logout buttons.Some examples how to use it :
logoutUser()-> redirects to/user/loginwith redirection url to the current page urllogoutUser(appsmith.URL.pathname)-> redirects to/user/login?redirectUrl=${redirectUrl}with redirectUrl is the value ofappsmith.URL.pathnamelogoutUser("/app/abc")-> redirects to/user/login?redirectUrl=/app/abcWhy we are doing it ?
This was the feature asked by one of our enterprise users because they were implementing the custom header and in that custom header they had a logout feature which they were doing via API. We changed the signature for the logout API and their logout functionality broke. Hence we thought of introducing a way to expose the logout function for the users to implement the logout functionality by themselves. They also wanted the redirect url functionality whenever the users logs out , on login they should be redirected to the application from where they logged out.
Slack thread : https://theappsmith.slack.com/archives/C08F9NSTJM9/p1742871711072179
Fixes https://github.com/appsmithorg/appsmith-ee/issues/6748
or
Fixes
Issue URLWarning
If no issue exists, please create an issue first, and check with the maintainers if the issue is valid.
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14195567720
Commit: 3f0ebad
Cypress dashboard.
Tags:
@tag.AllSpec:
Tue, 01 Apr 2025 13:44:17 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes