diff --git a/enterprise/frontend/src/metabase-enterprise/auth/index.js b/enterprise/frontend/src/metabase-enterprise/auth/index.js index 01569f21404c3c7c2d2e52af852d263cedc152dd..8cae8c1327cbab20401e1723bfb483c305a33b54 100644 --- a/enterprise/frontend/src/metabase-enterprise/auth/index.js +++ b/enterprise/frontend/src/metabase-enterprise/auth/index.js @@ -7,7 +7,7 @@ import { hasPremiumFeature } from "metabase-enterprise/settings"; import MetabaseSettings from "metabase/lib/settings"; import { PLUGIN_AUTH_PROVIDERS, - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS, + PLUGIN_IS_PASSWORD_USER, PLUGIN_ADMIN_SETTINGS_UPDATES, } from "metabase/plugins"; import { UtilApi } from "metabase/services"; @@ -256,7 +256,7 @@ PLUGIN_AUTH_PROVIDERS.push(providers => { return providers; }); -PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.push( +PLUGIN_IS_PASSWORD_USER.push( user => !user.google_auth && !user.ldap_auth && diff --git a/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.jsx b/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.jsx index d87facb471650a4169b42c0726276ce96287d29d..740476a295c6ca4deafb5fbe8d28b6dbd3b907c2 100644 --- a/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.jsx +++ b/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.jsx @@ -2,12 +2,14 @@ import React, { useMemo } from "react"; import PropTypes from "prop-types"; import { t } from "ttag"; import Radio from "metabase/core/components/Radio"; -import { PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS } from "metabase/plugins"; +import { getFullName } from "metabase/lib/user"; +import { PLUGIN_IS_PASSWORD_USER } from "metabase/plugins"; import { AccountHeaderRoot, HeaderAvatar, HeaderSection, HeaderTitle, + HeaderSubtitle, } from "./AccountHeader.styled"; const propTypes = { @@ -18,7 +20,7 @@ const propTypes = { const AccountHeader = ({ user, path, onChangeLocation }) => { const hasPasswordChange = useMemo( - () => PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.every(f => f(user)), + () => PLUGIN_IS_PASSWORD_USER.every(predicate => predicate(user)), [user], ); @@ -34,11 +36,14 @@ const AccountHeader = ({ user, path, onChangeLocation }) => { [hasPasswordChange], ); + const userFullName = getFullName(user); + return ( - <AccountHeaderRoot> + <AccountHeaderRoot data-testid="account-header"> <HeaderSection> <HeaderAvatar user={user} /> - <HeaderTitle>{t`Account settings`}</HeaderTitle> + {userFullName && <HeaderTitle>{userFullName}</HeaderTitle>} + <HeaderSubtitle>{user.email}</HeaderSubtitle> </HeaderSection> <Radio value={path} diff --git a/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.styled.jsx b/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.styled.jsx index 284a3a2ee0c4d6f3136165cdab4c0e41c34aae6a..03825ede8a245c14a355ae0afe4e41d89b0a7886 100644 --- a/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.styled.jsx +++ b/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.styled.jsx @@ -34,7 +34,14 @@ export const HeaderSection = styled.div` `; export const HeaderTitle = styled.h2` + font-size: 1rem; text-align: center; + margin-bottom: ${space(0)}; +`; + +export const HeaderSubtitle = styled.h3` + text-align: center; + color: ${color("text-medium")}; `; export const HeaderAvatar = styled(UserAvatar)` diff --git a/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.unit.spec.js b/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.unit.spec.js index 8ac3be18980b10d1dc808673e49d44dbab8ca167..87c29bac183dadc0780a9c6624c037078d65576b 100644 --- a/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.unit.spec.js +++ b/frontend/src/metabase/account/app/components/AccountHeader/AccountHeader.unit.spec.js @@ -1,7 +1,7 @@ import React from "react"; import { fireEvent, render, screen } from "@testing-library/react"; import AccountHeader from "./AccountHeader"; -import { PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS } from "metabase/plugins"; +import { PLUGIN_IS_PASSWORD_USER } from "metabase/plugins"; const getUser = () => ({ id: 1, @@ -12,19 +12,17 @@ const getUser = () => ({ }); describe("AccountHeader", () => { - const ORIGINAL_SHOW_CHANGE_PASSWORD_CONDITIONS = [ - ...PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS, - ]; + const ORIGINAL_PLUGIN_IS_PASSWORD_USER = [...PLUGIN_IS_PASSWORD_USER]; beforeEach(() => { - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.splice(0); + PLUGIN_IS_PASSWORD_USER.splice(0); }); afterEach(() => { - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.splice( + PLUGIN_IS_PASSWORD_USER.splice( 0, - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.length, - ...ORIGINAL_SHOW_CHANGE_PASSWORD_CONDITIONS, + PLUGIN_IS_PASSWORD_USER.length, + ...ORIGINAL_PLUGIN_IS_PASSWORD_USER, ); }); @@ -41,7 +39,7 @@ describe("AccountHeader", () => { it("should show the password tab if it is enabled by a plugin", () => { const user = getUser(); - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.push(user => user.google_auth); + PLUGIN_IS_PASSWORD_USER.push(user => user.google_auth); render(<AccountHeader user={user} />); @@ -50,7 +48,7 @@ describe("AccountHeader", () => { it("should hide the password tab if it is disabled by a plugin", () => { const user = getUser(); - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.push(user => !user.google_auth); + PLUGIN_IS_PASSWORD_USER.push(user => !user.google_auth); render(<AccountHeader user={user} />); diff --git a/frontend/src/metabase/admin/people/components/GroupMembersTable/GroupMembersTable.tsx b/frontend/src/metabase/admin/people/components/GroupMembersTable/GroupMembersTable.tsx index c3ee651887804c7960cc7ee84d8f692d3b899281..83de3f0323d992ec26da11255b01ffdb43aa4c69 100644 --- a/frontend/src/metabase/admin/people/components/GroupMembersTable/GroupMembersTable.tsx +++ b/frontend/src/metabase/admin/people/components/GroupMembersTable/GroupMembersTable.tsx @@ -4,6 +4,7 @@ import PropTypes from "prop-types"; import { t } from "ttag"; import { isAdminGroup, isDefaultGroup } from "metabase/lib/groups"; +import { getFullName } from "metabase/lib/user"; import Icon from "metabase/components/Icon"; import AdminEmptyText from "metabase/components/AdminEmptyText"; import AdminContentTable from "metabase/components/AdminContentTable"; @@ -198,7 +199,7 @@ const UserRow = ({ }; function getName(user: IUser): string { - const name = [user.first_name, user.last_name].join(" ").trim(); + const name = getFullName(user); if (!name) { return "-"; diff --git a/frontend/src/metabase/admin/people/components/PeopleListRow.jsx b/frontend/src/metabase/admin/people/components/PeopleListRow.jsx index d8525552c5ac57487f13e0c3380d1aa96b9aa790..1b55dcc1fa4efe7d6be265f0d29c7b2460ee7fb5 100644 --- a/frontend/src/metabase/admin/people/components/PeopleListRow.jsx +++ b/frontend/src/metabase/admin/people/components/PeopleListRow.jsx @@ -5,6 +5,7 @@ import moment from "moment"; import _ from "underscore"; import { color } from "metabase/lib/colors"; +import { getFullName } from "metabase/lib/user"; import * as Urls from "metabase/lib/urls"; import EntityMenu from "metabase/components/EntityMenu"; @@ -133,7 +134,7 @@ const PeopleListRow = ({ * @returns {string} */ function getName(user) { - const name = [user.first_name, user.last_name].join(" ").trim(); + const name = getFullName(user); if (!name) { return "-"; diff --git a/frontend/src/metabase/collections/components/BaseTableItem.jsx b/frontend/src/metabase/collections/components/BaseTableItem.jsx index c7992e740fc4ae3b878412f1ea2419bee04529c4..7a6368da2f5ca4c479d5234c8e5b9bbb9cc0467c 100644 --- a/frontend/src/metabase/collections/components/BaseTableItem.jsx +++ b/frontend/src/metabase/collections/components/BaseTableItem.jsx @@ -13,6 +13,7 @@ import Tooltip from "metabase/components/Tooltip"; import ActionMenu from "metabase/collections/components/ActionMenu"; import { color } from "metabase/lib/colors"; +import { getFullName } from "metabase/lib/user"; import { ItemCell, @@ -190,9 +191,7 @@ function getLastEditedBy(lastEditInfo) { return ""; } - const name = [lastEditInfo.first_name, lastEditInfo.last_name] - .join(" ") - .trim(); + const name = getFullName(lastEditInfo); return name || lastEditInfo.email; } diff --git a/frontend/src/metabase/components/LastEditInfoLabel.js b/frontend/src/metabase/components/LastEditInfoLabel.js index e1642330d924bb259a179b0cfc526ecb9244fdbc..233fa727449bc3dde788f72f67ef5060ae5fd87b 100644 --- a/frontend/src/metabase/components/LastEditInfoLabel.js +++ b/frontend/src/metabase/components/LastEditInfoLabel.js @@ -5,6 +5,7 @@ import { t } from "ttag"; import moment from "moment"; import { getUser } from "metabase/selectors/user"; +import { getFullName } from "metabase/lib/user"; import { TextButton } from "metabase/components/Button.styled"; import Tooltip from "metabase/components/Tooltip"; import DateTime from "metabase/components/DateTime"; @@ -33,9 +34,7 @@ LastEditInfoLabel.propTypes = { }; function formatEditorName(lastEditInfo) { - const name = [lastEditInfo.first_name, lastEditInfo.last_name] - .join(" ") - .trim(); + const name = getFullName(lastEditInfo); return name || lastEditInfo.email; } diff --git a/frontend/src/metabase/entities/users/forms.js b/frontend/src/metabase/entities/users/forms.js index 47b30b04742f3219ac978086e1608e2ee6c4e5fe..5737b428972b20ba8b983089035424c6b0f00540 100644 --- a/frontend/src/metabase/entities/users/forms.js +++ b/frontend/src/metabase/entities/users/forms.js @@ -3,7 +3,10 @@ import _ from "underscore"; import { t } from "ttag"; import MetabaseSettings from "metabase/lib/settings"; import MetabaseUtils from "metabase/lib/utils"; -import { PLUGIN_ADMIN_USER_FORM_FIELDS } from "metabase/plugins"; +import { + PLUGIN_ADMIN_USER_FORM_FIELDS, + PLUGIN_IS_PASSWORD_USER, +} from "metabase/plugins"; import validate from "metabase/lib/validate"; import FormGroupsWidget from "metabase/components/form/widgets/FormGroupsWidget"; @@ -82,7 +85,18 @@ export default { ], }, user: { - fields: [...getNameFields(), getEmailField(), getLocaleField()], + fields: user => { + const isSsoUser = !PLUGIN_IS_PASSWORD_USER.every(predicate => + predicate(user), + ); + + if (isSsoUser) { + return [getLocaleField()]; + } + + // password user + return [...getNameFields(), getEmailField(), getLocaleField()]; + }, disablePristineSubmit: true, }, setup: () => ({ diff --git a/frontend/src/metabase/lib/user.ts b/frontend/src/metabase/lib/user.ts new file mode 100644 index 0000000000000000000000000000000000000000..289695d0288072e44342e209d93fe9634a3ef3e6 --- /dev/null +++ b/frontend/src/metabase/lib/user.ts @@ -0,0 +1,5 @@ +import { User } from "metabase-types/api"; + +export function getFullName(user: User): string | null { + return [user.first_name, user.last_name].join(" ").trim() || null; +} diff --git a/frontend/src/metabase/plugins/builtin/auth/google.js b/frontend/src/metabase/plugins/builtin/auth/google.js index b0f312ec78016bb7992b8a04797acb333b66d709..d6118b1c946c89592036b2de19f1a57c5f1a69dc 100644 --- a/frontend/src/metabase/plugins/builtin/auth/google.js +++ b/frontend/src/metabase/plugins/builtin/auth/google.js @@ -4,7 +4,7 @@ import { updateIn } from "icepick"; import { PLUGIN_AUTH_PROVIDERS, PLUGIN_ADMIN_SETTINGS_UPDATES, - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS, + PLUGIN_IS_PASSWORD_USER, } from "metabase/plugins"; import MetabaseSettings from "metabase/lib/settings"; @@ -56,4 +56,4 @@ PLUGIN_ADMIN_SETTINGS_UPDATES.push(sections => ({ }, })); -PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.push(user => !user.google_auth); +PLUGIN_IS_PASSWORD_USER.push(user => !user.google_auth); diff --git a/frontend/src/metabase/plugins/builtin/auth/ldap.js b/frontend/src/metabase/plugins/builtin/auth/ldap.js index 96efd3117790a85ced0f55e1f47bfbc306be18f4..315ffb09542ff06f8c7c24afd2bc6667c895101c 100644 --- a/frontend/src/metabase/plugins/builtin/auth/ldap.js +++ b/frontend/src/metabase/plugins/builtin/auth/ldap.js @@ -3,7 +3,7 @@ import { updateIn } from "icepick"; import { PLUGIN_ADMIN_SETTINGS_UPDATES, - PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS, + PLUGIN_IS_PASSWORD_USER, } from "metabase/plugins"; import SettingsLdapForm from "metabase/admin/settings/components/SettingsLdapForm"; @@ -124,4 +124,4 @@ PLUGIN_ADMIN_SETTINGS_UPDATES.push( }), ); -PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS.push(user => !user.ldap_auth); +PLUGIN_IS_PASSWORD_USER.push(user => !user.ldap_auth); diff --git a/frontend/src/metabase/plugins/index.ts b/frontend/src/metabase/plugins/index.ts index 87cbdb4d95474531834713b14e854cc61b71a70c..3716aa15f9b735c48d84cf3b51b09e9f710b3ec1 100644 --- a/frontend/src/metabase/plugins/index.ts +++ b/frontend/src/metabase/plugins/index.ts @@ -13,6 +13,7 @@ import { Group, } from "metabase-types/api"; import { AdminPathKey, State } from "metabase-types/store"; +import { User } from "metabase-types/types/User"; import { PluginGroupManagersType } from "./types"; // Plugin integration points. All exports must be objects or arrays so they can be mutated by plugins. @@ -68,8 +69,9 @@ export const PLUGIN_ADMIN_USER_MENU_ROUTES = []; // authentication providers export const PLUGIN_AUTH_PROVIDERS = [] as any; -// Only show the password tab in account settings if these functions all return true -export const PLUGIN_SHOW_CHANGE_PASSWORD_CONDITIONS = []; +// Only show the password tab in account settings if these functions all return true. +// Otherwise, the user is logged in via SSO and should hide first name, last name, and email field in profile settings metabase#23298. +export const PLUGIN_IS_PASSWORD_USER: ((user: User) => boolean)[] = []; // selectors that customize behavior between app versions export const PLUGIN_SELECTORS = { diff --git a/frontend/src/metabase/redux/user.js b/frontend/src/metabase/redux/user.js index 1e75503630f1c0e2ab5e27165f4f5924c8146a71..0166f746ebbf0c68745ecbb3c4bc15dc5b29a862 100644 --- a/frontend/src/metabase/redux/user.js +++ b/frontend/src/metabase/redux/user.js @@ -5,6 +5,7 @@ import { } from "metabase/lib/redux"; import { UserApi } from "metabase/services"; import { CLOSE_QB_NEWB_MODAL } from "metabase/query_builder/actions"; +import Users from "metabase/entities/users"; export const REFRESH_CURRENT_USER = "metabase/user/REFRESH_CURRENT_USER"; export const refreshCurrentUser = createAction(REFRESH_CURRENT_USER, () => { @@ -35,6 +36,18 @@ export const currentUser = handleActions( [CLOSE_QB_NEWB_MODAL]: { next: (state, { payload }) => ({ ...state, is_qbnewb: false }), }, + [Users.actionTypes.UPDATE]: { + next: (state, { payload }) => { + const isCurrentUserUpdated = state.id === payload.user.id; + if (isCurrentUserUpdated) { + return { + ...state, + ...payload.user, + }; + } + return state; + }, + }, }, null, ); diff --git a/frontend/test/metabase/lib/user.unit.spec.ts b/frontend/test/metabase/lib/user.unit.spec.ts new file mode 100644 index 0000000000000000000000000000000000000000..bd97d1cc881904b588dfcb0c3854114747b6820d --- /dev/null +++ b/frontend/test/metabase/lib/user.unit.spec.ts @@ -0,0 +1,38 @@ +import { createMockUser } from "metabase-types/api/mocks"; +import { getFullName } from "metabase/lib/user"; + +describe("lib/user", () => { + describe("getFullName", () => { + test("has both first_name and last_name", () => { + const user = { + first_name: "Testy", + last_name: "Tableton", + }; + expect(getFullName(createMockUser(user))).toEqual("Testy Tableton"); + }); + + test("has only first_name", () => { + const user = { + first_name: "Testy", + last_name: null, + }; + expect(getFullName(createMockUser(user))).toEqual("Testy"); + }); + + test("has only last_name", () => { + const user = { + first_name: null, + last_name: "Tableton", + }; + expect(getFullName(createMockUser(user))).toEqual("Tableton"); + }); + + test("has no name", () => { + const user = { + first_name: null, + last_name: null, + }; + expect(getFullName(createMockUser(user))).toEqual(null); + }); + }); +}); diff --git a/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js b/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js index 33bdeb6726018b6fc2a8d93e9e7d5d5da83f58b2..e4bc318f280d9dc99d1593bebbeee4f6bf0f9393 100644 --- a/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js +++ b/frontend/test/metabase/scenarios/onboarding/setup/user_settings.cy.spec.js @@ -31,6 +31,8 @@ const CURRENT_USER = { const requestsCount = alias => cy.state("requests").filter(a => a.alias === alias); describe("user > settings", () => { + const fullName = `${first_name} ${last_name}`; + beforeEach(() => { restore(); cy.signInAsNormalUser(); @@ -38,7 +40,7 @@ describe("user > settings", () => { it("should be able to remove first name and last name (metabase#22754)", () => { cy.visit("/account/profile"); - cy.findByText("Account settings"); + cy.findByText(fullName); cy.findByLabelText("First name").clear(); cy.findByLabelText("Last name").clear(); cy.button("Update").click(); @@ -51,7 +53,10 @@ describe("user > settings", () => { it("should show user details with disabled submit button", () => { cy.visit("/account/profile"); - cy.findByText("Account settings"); + cy.findByTestId("account-header").within(() => { + cy.findByText(fullName); + cy.findByText(email); + }); cy.findByDisplayValue(first_name); cy.findByDisplayValue(last_name); cy.findByDisplayValue(email); @@ -202,5 +207,11 @@ describe("user > settings", () => { it("should hide change password tab", () => { cy.findByText("Password").should("not.exist"); }); + + it("should hide first name, last name, and email input (metabase#23298)", () => { + cy.findByLabelText("First name").should("not.exist"); + cy.findByLabelText("Last name").should("not.exist"); + cy.findByLabelText("Email").should("not.exist"); + }); }); });