fix: login button does not render

apache · Apr 13, 2022 · a21ddaf · a21ddaf
1 parent 6e8e29c
commit a21ddaf
Show file tree

Hide file tree

Showing 3 changed files with 73 additions and 46 deletions.
diff --git a/superset-frontend/src/dashboard/util/findPermission.test.ts b/superset-frontend/src/dashboard/util/findPermission.test.ts
@@ -16,10 +16,54 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import {
+  UndefinedUser,
+  UserWithPermissionsAndRoles,
+} from 'src/types/bootstrapTypes';
 import Dashboard from 'src/types/Dashboard';
 import Owner from 'src/types/Owner';
-import findPermission, { canUserEditDashboard } from './findPermission';
+import findPermission, {
+  canUserEditDashboard,
+  isUserAdmin,
+} from './findPermission';
+
+const ownerUser: UserWithPermissionsAndRoles = {
+  createdOn: '2021-05-12T16:56:22.116839',
+  email: '[email protected]',
+  firstName: 'Test',
+  isActive: true,
+  isAnonymous: false,
+  lastName: 'User',
+  userId: 1,
+  username: 'owner',
+  permissions: {},
+  roles: { Alpha: [['can_write', 'Dashboard']] },
+};
+
+const adminUser: UserWithPermissionsAndRoles = {
+  ...ownerUser,
+  roles: {
+    ...(ownerUser?.roles || {}),
+    Admin: [['can_write', 'Dashboard']],
+  },
+  userId: 2,
+  username: 'admin',
+};
+
+const outsiderUser: UserWithPermissionsAndRoles = {
+  ...ownerUser,
+  userId: 3,
+  username: 'outsider',
+};
+
+const owner: Owner = {
+  first_name: 'Test',
+  id: ownerUser.userId,
+  last_name: 'User',
+  username: ownerUser.username,
+};
+
+const undefinedUser: UndefinedUser = {};
 
 describe('findPermission', () => {
   it('findPermission for single role', () => {
@@ -70,42 +114,6 @@ describe('findPermission', () => {
 });
 
 describe('canUserEditDashboard', () => {
-  const ownerUser: UserWithPermissionsAndRoles = {
-    createdOn: '2021-05-12T16:56:22.116839',
-    email: '[email protected]',
-    firstName: 'Test',
-    isActive: true,
-    isAnonymous: false,
-    lastName: 'User',
-    userId: 1,
-    username: 'owner',
-    permissions: {},
-    roles: { Alpha: [['can_write', 'Dashboard']] },
-  };
-
-  const adminUser: UserWithPermissionsAndRoles = {
-    ...ownerUser,
-    roles: {
-      ...ownerUser.roles,
-      Admin: [['can_write', 'Dashboard']],
-    },
-    userId: 2,
-    username: 'admin',
-  };
-
-  const outsiderUser: UserWithPermissionsAndRoles = {
-    ...ownerUser,
-    userId: 3,
-    username: 'outsider',
-  };
-
-  const owner: Owner = {
-    first_name: 'Test',
-    id: ownerUser.userId,
-    last_name: 'User',
-    username: ownerUser.username,
-  };
-
   const dashboard: Dashboard = {
     id: 1,
     dashboard_title: 'Test Dash',
@@ -136,9 +144,7 @@ describe('canUserEditDashboard', () => {
   it('rejects missing roles', () => {
     // in redux, when there is no user, the user is actually set to an empty object,
     // so we need to handle missing roles as well as a missing user.s
-    expect(
-      canUserEditDashboard(dashboard, {} as UserWithPermissionsAndRoles),
-    ).toEqual(false);
+    expect(canUserEditDashboard(dashboard, {})).toEqual(false);
   });
   it('rejects "admins" if the admin role does not have edit rights for some reason', () => {
     expect(
@@ -149,3 +155,15 @@ describe('canUserEditDashboard', () => {
     ).toEqual(false);
   });
 });
+
+test('isUserAdmin returns true for admin user', () => {
+  expect(isUserAdmin(adminUser)).toEqual(true);
+});
+
+test('isUserAdmin returns false for undefined user', () => {
+  expect(isUserAdmin(undefinedUser)).toEqual(false);
+});
+
+test('isUserAdmin returns false for non-admin user', () => {
+  expect(isUserAdmin(ownerUser)).toEqual(false);
+});
diff --git a/superset-frontend/src/dashboard/util/findPermission.ts b/superset-frontend/src/dashboard/util/findPermission.ts
@@ -17,7 +17,10 @@
  * under the License.
  */
 import memoizeOne from 'memoize-one';
-import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import {
+  UndefinedUser,
+  UserWithPermissionsAndRoles,
+} from 'src/types/bootstrapTypes';
 import Dashboard from 'src/types/Dashboard';
 
 type UserRoles = Record<string, [string, string][]>;
@@ -36,17 +39,21 @@ export default findPermission;
 // but is hardcoded in backend logic already, so...
 const ADMIN_ROLE_NAME = 'admin';
 
-export const isUserAdmin = (user: UserWithPermissionsAndRoles) =>
-  Object.keys(user.roles).some(role => role.toLowerCase() === ADMIN_ROLE_NAME);
+export const isUserAdmin = (
+  user: UserWithPermissionsAndRoles | UndefinedUser,
+) =>
+  Object.keys(user.roles || {}).some(
+    role => role.toLowerCase() === ADMIN_ROLE_NAME,
+  );
 
 const isUserDashboardOwner = (
   dashboard: Dashboard,
-  user: UserWithPermissionsAndRoles,
+  user: UserWithPermissionsAndRoles | UndefinedUser,
 ) => dashboard.owners.some(owner => owner.username === user.username);
 
 export const canUserEditDashboard = (
   dashboard: Dashboard,
-  user?: UserWithPermissionsAndRoles | null,
+  user?: UserWithPermissionsAndRoles | UndefinedUser | null,
 ) =>
   !!user?.roles &&
   (isUserAdmin(user) || isUserDashboardOwner(dashboard, user)) &&

diff --git a/superset-frontend/src/types/bootstrapTypes.ts b/superset-frontend/src/types/bootstrapTypes.ts
@@ -37,6 +37,8 @@ export interface UserWithPermissionsAndRoles extends User {
   roles: Record<string, [string, string][]>;
 }
 
+export type UndefinedUser = {};
+
 export type Dashboard = {
   dttm: number;
   id: number;