Browse Source

ref(role): refactor to functional component (#31345)

* ref(role): refactor to functional component

* fix(import): use named import

* fix(role): allow null return from render fn
Jonas 3 years ago
parent
commit
c5c2b107cc

+ 41 - 44
static/app/components/acl/role.tsx

@@ -1,7 +1,7 @@
 import * as React from 'react';
 
 import ConfigStore from 'sentry/stores/configStore';
-import {Organization} from 'sentry/types';
+import {Organization, User} from 'sentry/types';
 import {isActiveSuperuser} from 'sentry/utils/isActiveSuperuser';
 import {isRenderFunc} from 'sentry/utils/isRenderFunc';
 import withOrganization from 'sentry/utils/withOrganization';
@@ -10,9 +10,33 @@ type RoleRenderProps = {
   hasRole: boolean;
 };
 
-type ChildrenRenderFn = (props: RoleRenderProps) => React.ReactNode;
+type ChildrenRenderFn = (props: RoleRenderProps) => React.ReactElement | null;
 
-type Props = {
+function checkUserRole(user: User, organization: Organization, role: RoleProps['role']) {
+  if (!user) {
+    return false;
+  }
+
+  if (isActiveSuperuser()) {
+    return true;
+  }
+
+  if (!Array.isArray(organization.availableRoles)) {
+    return false;
+  }
+
+  const roleIds = organization.availableRoles.map(r => r.id);
+
+  if (!roleIds.includes(role) || !roleIds.includes(organization.role ?? '')) {
+    return false;
+  }
+
+  const requiredIndex = roleIds.indexOf(role);
+  const currentIndex = roleIds.indexOf(organization.role ?? '');
+  return currentIndex >= requiredIndex;
+}
+
+interface RoleProps {
   /**
    * Minimum required role
    */
@@ -28,50 +52,23 @@ type Props = {
    * The other interface is more simple, only show `children` if user has
    * the minimum required role.
    */
-  children: React.ReactNode | ChildrenRenderFn;
-};
-
-class Role extends React.Component<Props> {
-  hasRole() {
-    const user = ConfigStore.get('user');
-    const {organization, role} = this.props;
-    const {availableRoles} = organization;
-
-    const currentRole = organization.role ?? '';
-
-    if (!user) {
-      return false;
-    }
-
-    if (isActiveSuperuser()) {
-      return true;
-    }
-
-    if (!Array.isArray(availableRoles)) {
-      return false;
-    }
-
-    const roleIds = availableRoles.map(r => r.id);
+  children: React.ReactElement | ChildrenRenderFn;
+}
 
-    if (!roleIds.includes(role) || !roleIds.includes(currentRole)) {
-      return false;
-    }
+function Role({role, organization, children}: RoleProps): React.ReactElement | null {
+  const hasRole = React.useMemo(
+    () => checkUserRole(ConfigStore.get('user'), organization, role),
+    // It seems that this returns a stable reference, but
+    [organization, role, ConfigStore.get('user')]
+  );
 
-    const requiredIndex = roleIds.indexOf(role);
-    const currentIndex = roleIds.indexOf(currentRole);
-    return currentIndex >= requiredIndex;
+  if (isRenderFunc<ChildrenRenderFn>(children)) {
+    return children({hasRole});
   }
 
-  render() {
-    const {children} = this.props;
-    const hasRole = this.hasRole();
-
-    if (isRenderFunc<ChildrenRenderFn>(children)) {
-      return children({hasRole});
-    }
-
-    return hasRole && children ? children : null;
-  }
+  return hasRole ? children : null;
 }
 
-export default withOrganization(Role);
+const withOrganizationRole = withOrganization(Role);
+
+export {withOrganizationRole as Role};

+ 1 - 1
static/app/components/events/eventTagsAndScreenshot/screenshot/index.tsx

@@ -2,7 +2,7 @@ import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import {openModal} from 'sentry/actionCreators/modal';
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import MenuItemActionLink from 'sentry/components/actions/menuItemActionLink';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';

+ 1 - 1
static/app/components/events/interfaces/debugMeta-v2/debugImageDetails/candidate/actions.tsx

@@ -2,7 +2,7 @@ import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import Access from 'sentry/components/acl/access';
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import ActionButton from 'sentry/components/actions/button';
 import MenuItemActionLink from 'sentry/components/actions/menuItemActionLink';
 import Button from 'sentry/components/button';

+ 2 - 2
static/app/components/eventsTable/eventsTableRow.tsx

@@ -37,12 +37,12 @@ class EventsTableRow extends React.Component<Props> {
         attachment={event.crashFile}
       >
         {url =>
-          url && (
+          url ? (
             <small>
               {crashFileType}: <a href={`${url}?download=1`}>{event.crashFile?.name}</a> (
               <FileSize bytes={event.crashFile?.size || 0} />)
             </small>
-          )
+          ) : null
         }
       </AttachmentUrl>
     );

+ 2 - 2
static/app/utils/attachmentUrl.tsx

@@ -1,6 +1,6 @@
 import {memo} from 'react';
 
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import {IssueAttachment, Organization} from 'sentry/types';
 import withOrganization from 'sentry/utils/withOrganization';
 
@@ -9,7 +9,7 @@ type Props = {
   projectId: string;
   eventId: string;
   attachment: IssueAttachment;
-  children: (downloadUrl: string | null) => React.ReactNode;
+  children: (downloadUrl: string | null) => React.ReactElement | null;
 };
 
 function AttachmentUrl({attachment, organization, eventId, projectId, children}: Props) {

+ 2 - 2
static/app/views/organizationGroupDetails/groupEventAttachments/groupEventAttachmentsTableRow.tsx

@@ -56,13 +56,13 @@ const GroupEventAttachmentsTableRow = ({
           attachment={attachment}
         >
           {url =>
-            !isDeleted && (
+            !isDeleted ? (
               <EventAttachmentActions
                 url={url}
                 onDelete={onDelete}
                 attachmentId={attachment.id}
               />
-            )
+            ) : null
           }
         </AttachmentUrl>
       </ActionsWrapper>

+ 1 - 1
static/app/views/settings/projectDebugFiles/debugFileRow.tsx

@@ -2,7 +2,7 @@ import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import Access from 'sentry/components/acl/access';
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import Confirm from 'sentry/components/confirm';

+ 1 - 1
static/app/views/settings/projectProguard/projectProguardRow.tsx

@@ -2,7 +2,7 @@ import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import Access from 'sentry/components/acl/access';
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import Confirm from 'sentry/components/confirm';

+ 1 - 1
static/app/views/settings/projectSourceMaps/detail/sourceMapsArtifactRow.tsx

@@ -2,7 +2,7 @@ import {Fragment} from 'react';
 import styled from '@emotion/styled';
 
 import Access from 'sentry/components/acl/access';
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import Button from 'sentry/components/button';
 import ButtonBar from 'sentry/components/buttonBar';
 import Confirm from 'sentry/components/confirm';

+ 19 - 1
tests/js/spec/components/acl/role.spec.jsx

@@ -2,7 +2,7 @@ import Cookies from 'js-cookie';
 
 import {mountWithTheme, screen} from 'sentry-test/reactTestingLibrary';
 
-import Role from 'sentry/components/acl/role';
+import {Role} from 'sentry/components/acl/role';
 import ConfigStore from 'sentry/stores/configStore';
 
 describe('Role', function () {
@@ -91,6 +91,24 @@ describe('Role', function () {
       ConfigStore.config.user = user;
     });
 
+    it('updates if user changes', function () {
+      const user = {...ConfigStore.config.user};
+      ConfigStore.config.user = undefined;
+      const {rerender} = mountWithTheme(<Role role="member">{childrenMock}</Role>, {
+        context: routerContext,
+      });
+
+      expect(childrenMock).toHaveBeenCalledWith({
+        hasRole: false,
+      });
+      ConfigStore.config.user = user;
+
+      rerender(<Role role="member">{childrenMock}</Role>);
+      expect(childrenMock).toHaveBeenCalledWith({
+        hasRole: true,
+      });
+    });
+
     it('handles no availableRoles', function () {
       mountWithTheme(
         <Role role="member" organization={{...organization, availableRoles: undefined}}>