From 23082730907e11d5fc54f3e0846175290a424751 Mon Sep 17 00:00:00 2001
From: Anton Kulyk <kuliks.anton@gmail.com>
Date: Wed, 16 Nov 2022 15:56:39 +0000
Subject: [PATCH] Clean up user selectors (#26477)
* Simplify null checks
* Use actual `User` type in plugins file
* Sort plugins file imports
* Add `login_attributes` type to `User`
* Convert user selectors to TypeScript
* Make `currentUser` in `State` nullable
* Simplify personal collection ID selectors
* Handle nullable current user
* Use `checkNotNull` to check nullable user object
* Remove explicit `User` type in `checkNotNull`
* Simplify `getIsSsoUser` selector
---
frontend/src/metabase-types/api/mocks/user.ts | 1 +
frontend/src/metabase-types/api/user.ts | 1 +
frontend/src/metabase-types/store/state.ts | 2 +-
.../UserPasswordApp/UserPasswordApp.tsx | 9 ++++++--
.../UserProfileApp/UserProfileApp.tsx | 9 ++++++--
.../src/metabase/account/profile/selectors.ts | 9 ++++++--
.../containers/HomeGreeting/HomeGreeting.tsx | 9 ++++++--
frontend/src/metabase/plugins/index.ts | 20 +++++++++-------
.../actions/core/initializeQB.ts | 2 +-
.../metabase/selectors/{user.js => user.ts} | 10 ++++----
.../src/metabase/selectors/user.unit.spec.js | 23 -------------------
.../src/metabase/selectors/user.unit.spec.ts | 21 +++++++++++++++++
12 files changed, 71 insertions(+), 45 deletions(-)
rename frontend/src/metabase/selectors/{user.js => user.ts} (73%)
delete mode 100644 frontend/src/metabase/selectors/user.unit.spec.js
create mode 100644 frontend/src/metabase/selectors/user.unit.spec.ts
diff --git a/frontend/src/metabase-types/api/mocks/user.ts b/frontend/src/metabase-types/api/mocks/user.ts
index a92fc89907a..674a681f7a9 100644
--- a/frontend/src/metabase-types/api/mocks/user.ts
+++ b/frontend/src/metabase-types/api/mocks/user.ts
@@ -8,6 +8,7 @@ export const createMockUser = (opts?: Partial<User>): User => ({
email: "user@metabase.test",
locale: null,
google_auth: false,
+ login_attributes: null,
is_active: true,
is_qbnewb: false,
is_superuser: false,
diff --git a/frontend/src/metabase-types/api/user.ts b/frontend/src/metabase-types/api/user.ts
index 6f216cd6ca4..5365546e938 100644
--- a/frontend/src/metabase-types/api/user.ts
+++ b/frontend/src/metabase-types/api/user.ts
@@ -19,6 +19,7 @@ export interface BaseUser {
export interface User extends BaseUser {
google_auth: boolean;
+ login_attributes: string[] | null;
is_installer: boolean;
has_invited_second_user: boolean;
has_question_and_dashboard: boolean;
diff --git a/frontend/src/metabase-types/store/state.ts b/frontend/src/metabase-types/store/state.ts
index 07408871414..c0d0ee77e44 100644
--- a/frontend/src/metabase-types/store/state.ts
+++ b/frontend/src/metabase-types/store/state.ts
@@ -11,7 +11,7 @@ import { SetupState } from "./setup";
export interface State {
admin: AdminState;
app: AppState;
- currentUser: User;
+ currentUser: User | null;
embed: EmbedState;
entities: EntitiesState;
form: FormState;
diff --git a/frontend/src/metabase/account/password/containers/UserPasswordApp/UserPasswordApp.tsx b/frontend/src/metabase/account/password/containers/UserPasswordApp/UserPasswordApp.tsx
index 4e5f026b815..842a3b5c7e8 100644
--- a/frontend/src/metabase/account/password/containers/UserPasswordApp/UserPasswordApp.tsx
+++ b/frontend/src/metabase/account/password/containers/UserPasswordApp/UserPasswordApp.tsx
@@ -1,11 +1,16 @@
import { connect } from "react-redux";
+
+import { checkNotNull } from "metabase/core/utils/types";
+
import { getUser } from "metabase/selectors/user";
-import { State } from "metabase-types/store";
+
+import type { State } from "metabase-types/store";
+
import { updatePassword, validatePassword } from "../../actions";
import UserPasswordForm from "../../components/UserPasswordForm";
const mapStateToProps = (state: State) => ({
- user: getUser(state),
+ user: checkNotNull(getUser(state)),
onValidatePassword: validatePassword,
onSubmit: updatePassword,
});
diff --git a/frontend/src/metabase/account/profile/containers/UserProfileApp/UserProfileApp.tsx b/frontend/src/metabase/account/profile/containers/UserProfileApp/UserProfileApp.tsx
index dbde8a6ed26..b060b1eadf5 100644
--- a/frontend/src/metabase/account/profile/containers/UserProfileApp/UserProfileApp.tsx
+++ b/frontend/src/metabase/account/profile/containers/UserProfileApp/UserProfileApp.tsx
@@ -1,12 +1,17 @@
import { connect } from "react-redux";
+
+import { checkNotNull } from "metabase/core/utils/types";
+
import { getUser } from "metabase/selectors/user";
-import { State } from "metabase-types/store";
+
+import type { State } from "metabase-types/store";
+
import UserProfileForm from "../../components/UserProfileForm";
import { updateUser } from "../../actions";
import { getIsSsoUser, getLocales } from "../../selectors";
const mapStateToProps = (state: State) => ({
- user: getUser(state),
+ user: checkNotNull(getUser(state)),
locales: getLocales(state),
isSsoUser: getIsSsoUser(state),
});
diff --git a/frontend/src/metabase/account/profile/selectors.ts b/frontend/src/metabase/account/profile/selectors.ts
index 46c12d573cc..bff59f25a30 100644
--- a/frontend/src/metabase/account/profile/selectors.ts
+++ b/frontend/src/metabase/account/profile/selectors.ts
@@ -1,9 +1,14 @@
import { createSelector } from "reselect";
+
import { getUser } from "metabase/selectors/user";
-import { PLUGIN_IS_PASSWORD_USER } from "metabase/plugins";
import { getSettings } from "metabase/selectors/settings";
-export const getIsSsoUser = createSelector([getUser], user => {
+import { PLUGIN_IS_PASSWORD_USER } from "metabase/plugins";
+
+export const getIsSsoUser = createSelector(getUser, user => {
+ if (!user) {
+ return false;
+ }
return !PLUGIN_IS_PASSWORD_USER.every(predicate => predicate(user));
});
diff --git a/frontend/src/metabase/home/homepage/containers/HomeGreeting/HomeGreeting.tsx b/frontend/src/metabase/home/homepage/containers/HomeGreeting/HomeGreeting.tsx
index e56cb1f9d2f..ad222fddaea 100644
--- a/frontend/src/metabase/home/homepage/containers/HomeGreeting/HomeGreeting.tsx
+++ b/frontend/src/metabase/home/homepage/containers/HomeGreeting/HomeGreeting.tsx
@@ -1,10 +1,15 @@
import { connect } from "react-redux";
+
+import { checkNotNull } from "metabase/core/utils/types";
+
import { getUser } from "metabase/selectors/user";
-import { State } from "metabase-types/store";
+
+import type { State } from "metabase-types/store";
+
import HomeGreeting from "../../components/HomeGreeting";
const mapStateToProps = (state: State) => ({
- user: getUser(state),
+ user: checkNotNull(getUser(state)),
showLogo: state.settings.values["show-metabot"],
});
diff --git a/frontend/src/metabase/plugins/index.ts b/frontend/src/metabase/plugins/index.ts
index 6ae32f1fdd1..8e5ab1964e3 100644
--- a/frontend/src/metabase/plugins/index.ts
+++ b/frontend/src/metabase/plugins/index.ts
@@ -1,20 +1,24 @@
-import { t } from "ttag";
import React from "react";
+import { t } from "ttag";
+
import PluginPlaceholder from "metabase/plugins/components/PluginPlaceholder";
-import {
+
+import type {
DatabaseEntityId,
PermissionSubject,
} from "metabase/admin/permissions/types";
-import {
- Collection,
+
+import type {
Bookmark,
- GroupsPermissions,
+ Collection,
Dataset,
Group,
+ GroupsPermissions,
+ User,
} from "metabase-types/api";
-import { AdminPathKey, State } from "metabase-types/store";
-import { User } from "metabase-types/types/User";
-import Question from "metabase-lib/Question";
+import type { AdminPathKey, State } from "metabase-types/store";
+import type Question from "metabase-lib/Question";
+
import { PluginGroupManagersType } from "./types";
// Plugin integration points. All exports must be objects or arrays so they can be mutated by plugins.
diff --git a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
index b6e08805b0d..41c7271d2c8 100644
--- a/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
+++ b/frontend/src/metabase/query_builder/actions/core/initializeQB.ts
@@ -308,7 +308,7 @@ async function handleQBInit(
question = question.lockDisplay();
const currentUser = getUser(getState());
- if (currentUser.is_qbnewb) {
+ if (currentUser?.is_qbnewb) {
uiControls.isShowingNewbModal = true;
MetabaseAnalytics.trackStructEvent("QueryBuilder", "Show Newb Modal");
}
diff --git a/frontend/src/metabase/selectors/user.js b/frontend/src/metabase/selectors/user.ts
similarity index 73%
rename from frontend/src/metabase/selectors/user.js
rename to frontend/src/metabase/selectors/user.ts
index f981584fbd9..0f46fdf6552 100644
--- a/frontend/src/metabase/selectors/user.js
+++ b/frontend/src/metabase/selectors/user.ts
@@ -1,13 +1,15 @@
import { createSelector } from "reselect";
import { PLUGIN_APPLICATION_PERMISSIONS } from "metabase/plugins";
-export const getUser = state => state.currentUser;
+import type { State } from "metabase-types/store";
+
+export const getUser = (state: State) => state.currentUser;
export const getUserId = createSelector([getUser], user => user?.id);
export const getUserIsAdmin = createSelector(
[getUser],
- user => (user && user.is_superuser) || false,
+ user => user?.is_superuser || false,
);
export const canManageSubscriptions = createSelector(
@@ -21,10 +23,10 @@ export const canManageSubscriptions = createSelector(
export const getUserAttributes = createSelector(
[getUser],
- user => (user && user.login_attributes) || [],
+ user => user?.login_attributes || [],
);
export const getUserPersonalCollectionId = createSelector(
[getUser],
- user => (user && user.personal_collection_id) || null,
+ user => user?.personal_collection_id,
);
diff --git a/frontend/src/metabase/selectors/user.unit.spec.js b/frontend/src/metabase/selectors/user.unit.spec.js
deleted file mode 100644
index 9bde3e13bdb..00000000000
--- a/frontend/src/metabase/selectors/user.unit.spec.js
+++ /dev/null
@@ -1,23 +0,0 @@
-import { getUserIsAdmin } from "./user";
-
-describe("metabase/selectors/user", () => {
- it("should return true if user is an admin", () => {
- const state = {
- currentUser: {
- is_superuser: true,
- },
- };
-
- expect(getUserIsAdmin(state)).toBe(true);
- });
-
- it("should return false if user is not an admin", () => {
- const state = {
- currentUser: {
- is_superuser: false,
- },
- };
-
- expect(getUserIsAdmin(state)).toBe(false);
- });
-});
diff --git a/frontend/src/metabase/selectors/user.unit.spec.ts b/frontend/src/metabase/selectors/user.unit.spec.ts
new file mode 100644
index 00000000000..07fb28a1a3d
--- /dev/null
+++ b/frontend/src/metabase/selectors/user.unit.spec.ts
@@ -0,0 +1,21 @@
+import { createMockUser } from "metabase-types/api/mocks";
+import { createMockState } from "metabase-types/store/mocks";
+import { getUserIsAdmin } from "./user";
+
+describe("metabase/selectors/user", () => {
+ it("should return true if user is an admin", () => {
+ const state = createMockState({
+ currentUser: createMockUser({ is_superuser: true }),
+ });
+
+ expect(getUserIsAdmin(state)).toBe(true);
+ });
+
+ it("should return false if user is not an admin", () => {
+ const state = createMockState({
+ currentUser: createMockUser({ is_superuser: false }),
+ });
+
+ expect(getUserIsAdmin(state)).toBe(false);
+ });
+});
--
GitLab