From ba4e29fb821563dd51f1141f86545df155b70494 Mon Sep 17 00:00:00 2001 From: Ankita Kinger Date: Tue, 8 Apr 2025 00:41:02 +0530 Subject: [PATCH] fix: Updating admin settings logic to fix issues on EE (#40135) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Updating admin settings logic to fix issues on EE Fixes #`Issue Number` _or_ Fixes `Issue URL` > [!WARNING] > _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.Settings" ### :mag: Cypress test results > [!TIP] > 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 > Workflow run: > Commit: 1b0c2823290e6af5849b096d640ac856964a0d4c > Cypress dashboard. > Tags: `@tag.Settings` > Spec: >
Mon, 07 Apr 2025 17:54:10 UTC ## Communication Should the DevRel and Marketing teams inform users about this change? - [ ] Yes - [ ] No ## Summary by CodeRabbit ## Summary by CodeRabbit - **New Features** - Admin Settings now displays Profile and Organisation sections whenever available, independent of user privileges. - Updated category filtering ensures non-superusers see only the most relevant options. - Introduced a new locator for the sub-text link in Admin Settings to improve test coverage. - **Style** - Enhanced link presentation in the Admin Settings page for clearer navigation. - **Bug Fixes** - Simplified test logic for Admin settings, improving the reliability of test cases. - Ensured consistent boolean handling for user superuser status across various components. --- .../ExplorerTests/Admin_settings_2_spec.js | 16 +++---- app/client/cypress/locators/AdminsSettings.js | 1 + .../pages/AdminSettings/WithSuperUserHoc.tsx | 13 +++-- .../ce/pages/AdminSettings/config/index.ts | 31 +++++++----- .../BusinessFeatures/adminSettingsHelpers.tsx | 2 +- .../src/ce/utils/adminSettingsHelpers.ts | 48 +++++++++++++------ .../pages/AdminSettings/FormGroup/Common.tsx | 7 ++- .../src/pages/AdminSettings/LeftPane.tsx | 11 +++-- .../src/pages/AdminSettings/Profile/index.tsx | 21 ++++---- app/client/src/pages/common/MobileSidebar.tsx | 2 +- .../common/SearchBar/HomepageHeaderAction.tsx | 2 +- 11 files changed, 100 insertions(+), 54 deletions(-) diff --git a/app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Admin_settings_2_spec.js b/app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Admin_settings_2_spec.js index 2c93ab1893..db8d68f5b8 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Admin_settings_2_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/ExplorerTests/Admin_settings_2_spec.js @@ -8,20 +8,18 @@ const { describe( "Admin settings page", - { tags: ["@tag.IDE", "@tag.PropertyPane"] }, + { tags: ["@tag.IDE", "@tag.PropertyPane", "@tag.Settings"] }, function () { it("1. should test that configure link redirects to google maps setup doc", () => { cy.visit(adminSettingsHelper.routes.INSTANCE_SETTINGS, { timeout: 60000, }); - cy.get(adminsSettings.readMoreLink).within(() => { - cy.get("a") - .should("have.attr", "target", "_blank") - .invoke("removeAttr", "target") - .click() - .wait(3000); //for page to load fully; - cy.url().should("contain", GOOGLE_MAPS_SETUP_DOC); - }); + cy.get(adminsSettings.subTextLink) + .should("have.attr", "target", "_blank") + .invoke("removeAttr", "target") + .click() + .wait(3000); //for page to load fully; + cy.url().should("contain", GOOGLE_MAPS_SETUP_DOC); }); it( diff --git a/app/client/cypress/locators/AdminsSettings.js b/app/client/cypress/locators/AdminsSettings.js index a6e494786b..cdcfe8bc70 100644 --- a/app/client/cypress/locators/AdminsSettings.js +++ b/app/client/cypress/locators/AdminsSettings.js @@ -11,6 +11,7 @@ export default { githubButton: ".t--settings-sub-category-github-auth", formloginButton: ".t--settings-sub-category-form-login", readMoreLink: ".t--read-more-link", + subTextLink: ".t--sub-text-link", versionTab: ".t--settings-category-version", backButton: ".t--admin-settings-back-button", saveButton: ".t--admin-settings-save-button", diff --git a/app/client/src/ce/pages/AdminSettings/WithSuperUserHoc.tsx b/app/client/src/ce/pages/AdminSettings/WithSuperUserHoc.tsx index f9136d2301..f47557dc3d 100644 --- a/app/client/src/ce/pages/AdminSettings/WithSuperUserHoc.tsx +++ b/app/client/src/ce/pages/AdminSettings/WithSuperUserHoc.tsx @@ -1,23 +1,30 @@ -import { APPLICATIONS_URL } from "constants/routes"; import React from "react"; import { useSelector } from "react-redux"; import type { RouteComponentProps } from "react-router"; -import { Redirect } from "react-router"; +import { Redirect, useParams } from "react-router"; import { getCurrentUser } from "selectors/usersSelectors"; import { getShowAdminSettings } from "ee/utils/BusinessFeatures/adminSettingsHelpers"; import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; +import { APPLICATIONS_URL } from "constants/routes"; export default function WithSuperUserHOC( Component: React.ComponentType, ) { return function Wrapped(props: RouteComponentProps) { + // TODO: Fix this the next time the file is edited + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const params = useParams() as any; + const { category } = params; const user = useSelector(getCurrentUser); const isFeatureEnabled = useFeatureFlag(FEATURE_FLAG.license_gac_enabled); if (!user) return null; - if (!getShowAdminSettings(isFeatureEnabled, user)) { + if ( + ["profile"].indexOf(category) === -1 && + !getShowAdminSettings(isFeatureEnabled, user) + ) { return ; } diff --git a/app/client/src/ce/pages/AdminSettings/config/index.ts b/app/client/src/ce/pages/AdminSettings/config/index.ts index 6f9b28f334..da4ff2f8fa 100644 --- a/app/client/src/ce/pages/AdminSettings/config/index.ts +++ b/app/client/src/ce/pages/AdminSettings/config/index.ts @@ -16,32 +16,41 @@ import { config as AuditLogsConfig } from "ee/pages/AdminSettings/config/auditlo import { selectFeatureFlags } from "ee/selectors/featureFlagsSelectors"; import store from "store"; import { isMultiOrgFFEnabled } from "ee/utils/planHelpers"; +import { getCurrentUser } from "selectors/usersSelectors"; +import { getShowAdminSettings } from "ee/utils/BusinessFeatures/adminSettingsHelpers"; const featureFlags = selectFeatureFlags(store.getState()); const isMultiOrgEnabled = isMultiOrgFFEnabled(featureFlags); +const user = getCurrentUser(store.getState()); +const isFeatureEnabled = featureFlags.license_gac_enabled; +const isSuperUser = getShowAdminSettings(isFeatureEnabled, user); // Profile categories ConfigFactory.register(ProfileConfig); // Organisation categories -ConfigFactory.register(GeneralConfig); +if (isSuperUser) ConfigFactory.register(GeneralConfig); -if (!isMultiOrgEnabled) ConfigFactory.register(EmailConfig); +if (isSuperUser && !isMultiOrgEnabled) ConfigFactory.register(EmailConfig); -ConfigFactory.register(BrandingConfig); -ConfigFactory.register(AuditLogsConfig); +if (isSuperUser) ConfigFactory.register(BrandingConfig); + +if (isSuperUser) ConfigFactory.register(AuditLogsConfig); // User management categories -ConfigFactory.register(UserSettings); -ConfigFactory.register(Authentication); -ConfigFactory.register(ProvisioningConfig); -ConfigFactory.register(UserListing); +if (isSuperUser) ConfigFactory.register(UserSettings); + +if (isSuperUser) ConfigFactory.register(Authentication); + +if (isSuperUser) ConfigFactory.register(ProvisioningConfig); + +if (isSuperUser) ConfigFactory.register(UserListing); // Instance categories -if (!isMultiOrgEnabled) ConfigFactory.register(InstanceSettings); +if (isSuperUser && !isMultiOrgEnabled) ConfigFactory.register(InstanceSettings); -if (!isMultiOrgEnabled) ConfigFactory.register(Configuration); +if (isSuperUser && !isMultiOrgEnabled) ConfigFactory.register(Configuration); -if (!isMultiOrgEnabled) ConfigFactory.register(VersionConfig); +if (isSuperUser && !isMultiOrgEnabled) ConfigFactory.register(VersionConfig); export default ConfigFactory; diff --git a/app/client/src/ce/utils/BusinessFeatures/adminSettingsHelpers.tsx b/app/client/src/ce/utils/BusinessFeatures/adminSettingsHelpers.tsx index 69df995cf5..2014b5f490 100644 --- a/app/client/src/ce/utils/BusinessFeatures/adminSettingsHelpers.tsx +++ b/app/client/src/ce/utils/BusinessFeatures/adminSettingsHelpers.tsx @@ -19,7 +19,7 @@ export const getShowAdminSettings = ( }; export const getAdminSettingsPath = ( isEnabled: boolean, - isSuperUser: boolean | undefined, + isSuperUser: boolean, organizationPermissions: string[] = [], ) => { if (isEnabled) { diff --git a/app/client/src/ce/utils/adminSettingsHelpers.ts b/app/client/src/ce/utils/adminSettingsHelpers.ts index 616079c13d..4f5fc3c588 100644 --- a/app/client/src/ce/utils/adminSettingsHelpers.ts +++ b/app/client/src/ce/utils/adminSettingsHelpers.ts @@ -3,7 +3,10 @@ import type { AdminConfigType, Category, } from "ee/pages/AdminSettings/config/types"; -import { ADMIN_SETTINGS_CATEGORY_DEFAULT_PATH } from "constants/routes"; +import { + ADMIN_SETTINGS_CATEGORY_DEFAULT_PATH, + ADMIN_SETTINGS_CATEGORY_PROFILE_PATH, +} from "constants/routes"; import type { User } from "constants/userConstants"; /* settings is the updated & unsaved settings on Admin settings page */ @@ -33,12 +36,19 @@ export const saveAllowed = ( }; /* get default admin settings path */ -export const getDefaultAdminSettingsPath = ( - // TODO: Fix this the next time the file is edited - // eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-explicit-any - { isSuperUser, organizationPermissions: any = [] }: Record, -): string => { - return ADMIN_SETTINGS_CATEGORY_DEFAULT_PATH; +export const getDefaultAdminSettingsPath = ({ + isSuperUser = false, + // eslint-disable-next-line @typescript-eslint/no-unused-vars + organizationPermissions = [], +}: { + isSuperUser: boolean; + organizationPermissions: string[]; +}): string => { + if (isSuperUser) { + return ADMIN_SETTINGS_CATEGORY_DEFAULT_PATH; + } + + return ADMIN_SETTINGS_CATEGORY_PROFILE_PATH; }; export const showAdminSettings = (user?: User): boolean => { @@ -73,11 +83,11 @@ export const getFilteredOrgCategories = ( ) => { return categories ?.map((category: Category) => { - if (category.slug === "audit-logs" && !isAuditLogsEnabled) { - return null; + if (isSuperUser) { + return category; } - return category; + return null; }) .filter(Boolean) as Category[]; }; @@ -85,13 +95,19 @@ export const getFilteredOrgCategories = ( export const getFilteredUserManagementCategories = ( categories: Category[], // eslint-disable-next-line @typescript-eslint/no-unused-vars + showAdminSettings: boolean, + // eslint-disable-next-line @typescript-eslint/no-unused-vars isSuperUser?: boolean, ) => { return categories ?.map((category: Category) => { - return category; + if (isSuperUser) { + return category; + } + + return null; }) - .filter(Boolean); + .filter(Boolean) as Category[]; }; export const getFilteredInstanceCategories = ( @@ -101,7 +117,11 @@ export const getFilteredInstanceCategories = ( ) => { return categories ?.map((category: Category) => { - return category; + if (isSuperUser) { + return category; + } + + return null; }) - .filter(Boolean); + .filter(Boolean) as Category[]; }; diff --git a/app/client/src/pages/AdminSettings/FormGroup/Common.tsx b/app/client/src/pages/AdminSettings/FormGroup/Common.tsx index 3b23b4b11b..028252ddc2 100644 --- a/app/client/src/pages/AdminSettings/FormGroup/Common.tsx +++ b/app/client/src/pages/AdminSettings/FormGroup/Common.tsx @@ -95,7 +95,12 @@ export function FormGroup({ children, className, setting }: FieldHelperProps) { {children} {setting.subText && (setting.subTextLink ? ( - + {createMessage(() => setting.subText || "")} ) : ( diff --git a/app/client/src/pages/AdminSettings/LeftPane.tsx b/app/client/src/pages/AdminSettings/LeftPane.tsx index a4854bcc19..575c065621 100644 --- a/app/client/src/pages/AdminSettings/LeftPane.tsx +++ b/app/client/src/pages/AdminSettings/LeftPane.tsx @@ -23,6 +23,7 @@ import { import { useFeatureFlag } from "utils/hooks/useFeatureFlag"; import { FEATURE_FLAG } from "ee/entities/FeatureFlag"; import { getHasAuditLogsReadPermission } from "ee/utils/BusinessFeatures/permissionPageHelpers"; +import { getShowAdminSettings } from "ee/utils/BusinessFeatures/adminSettingsHelpers"; export const Wrapper = styled.div` flex-basis: ${(props) => props.theme.sidebarWidth}; @@ -209,6 +210,7 @@ export default function LeftPane() { isFeatureEnabled, organizationPermissions, ); + const showAdminSettings = getShowAdminSettings(isFeatureEnabled, user); const filteredOrgCategories = getFilteredOrgCategories( organizationCategories, @@ -218,6 +220,7 @@ export default function LeftPane() { const filteredUserManagmentCategories = getFilteredUserManagementCategories( userManagementCategories, + showAdminSettings, isSuperUser, ); @@ -228,7 +231,7 @@ export default function LeftPane() { return ( - {isSuperUser && profileCategories.length > 0 && ( + {profileCategories.length > 0 && ( Profile @@ -240,7 +243,7 @@ export default function LeftPane() { /> )} - {isSuperUser && organizationCategories.length > 0 && ( + {filteredOrgCategories.length > 0 && ( Organisation @@ -252,7 +255,7 @@ export default function LeftPane() { /> )} - {userManagementCategories.length > 0 && ( + {filteredUserManagmentCategories.length > 0 && ( User management @@ -264,7 +267,7 @@ export default function LeftPane() { /> )} - {instanceCategories.length > 0 && ( + {filteredInstanceCategories.length > 0 && ( Instance diff --git a/app/client/src/pages/AdminSettings/Profile/index.tsx b/app/client/src/pages/AdminSettings/Profile/index.tsx index c0734d5f65..cdf9db65b6 100644 --- a/app/client/src/pages/AdminSettings/Profile/index.tsx +++ b/app/client/src/pages/AdminSettings/Profile/index.tsx @@ -49,7 +49,6 @@ import { } from "actions/gitSyncActions"; import { Classes } from "@blueprintjs/core"; import { useGitModEnabled } from "pages/Editor/gitSync/hooks/modHooks"; -import { gitFetchGlobalProfile } from "git/store"; import useGlobalProfile from "git/hooks/useGlobalProfile"; const nameValidator = ( @@ -83,22 +82,31 @@ export const Profile = () => { const isFormLoginEnabled = useSelector(getIsFormLoginEnabled); const isFetching = useSelector(getIsFetchingGlobalGitConfig); const globalGitConfig = useSelector(getGlobalGitConfig); - const { globalProfile, updateGlobalProfile } = useGlobalProfile(); + const { fetchGlobalProfile, globalProfile, updateGlobalProfile } = + useGlobalProfile(); const [name, setName] = useState(user?.name || ""); const [isSaving, setIsSaving] = useState(false); const [areFormValuesUpdated, setAreFormValuesUpdated] = useState(false); const isGitModEnabled = useGitModEnabled(); const gitConfig = useMemo( () => (isGitModEnabled ? globalProfile : globalGitConfig), - [isGitModEnabled], + [isGitModEnabled, globalProfile, globalGitConfig], ); const [authorName, setAuthorNameInState] = useState(gitConfig?.authorName); const [authorEmail, setAuthorEmailInState] = useState(gitConfig?.authorEmail); + useEffect(() => { + setIsSaving(false); + setAreFormValuesUpdated(false); + setName(user?.name || ""); + setAuthorNameInState(gitConfig?.authorName); + setAuthorEmailInState(gitConfig?.authorEmail); + }, [gitConfig?.authorName, gitConfig?.authorEmail, user?.name]); + useEffect( function fetchGlobalGitConfigOnInitEffect() { if (isGitModEnabled) { - dispatch(gitFetchGlobalProfile()); + fetchGlobalProfile(); } else { dispatch(fetchGlobalGitConfigInit()); } @@ -106,11 +114,6 @@ export const Profile = () => { [dispatch, isGitModEnabled], ); - useEffect(() => { - setIsSaving(false); - setAreFormValuesUpdated(false); - }, [gitConfig, user?.name]); - useEffect(() => { if ( user?.name !== name || diff --git a/app/client/src/pages/common/MobileSidebar.tsx b/app/client/src/pages/common/MobileSidebar.tsx index 3d89938e26..25388097a4 100644 --- a/app/client/src/pages/common/MobileSidebar.tsx +++ b/app/client/src/pages/common/MobileSidebar.tsx @@ -115,7 +115,7 @@ export default function MobileSideBar(props: MobileSideBarProps) { getOnSelectAction(DropdownOnSelectActions.REDIRECT, { path: getAdminSettingsPath( isFeatureEnabled, - user?.isSuperUser, + user?.isSuperUser || false, organizationPermissions, ), }); diff --git a/app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx b/app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx index 50610c34a5..7d45975d08 100644 --- a/app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx +++ b/app/client/src/pages/common/SearchBar/HomepageHeaderAction.tsx @@ -86,7 +86,7 @@ const HomepageHeaderAction = ({ getOnSelectAction(DropdownOnSelectActions.REDIRECT, { path: getAdminSettingsPath( isFeatureEnabled, - user?.isSuperUser, + user?.isSuperUser || false, organizationPermissions, ), });