From e2027cdbeb980d2dde8231fee5d230a21c2fc5d7 Mon Sep 17 00:00:00 2001 From: Jonas Date: Thu, 26 Sep 2024 16:12:57 -0400 Subject: [PATCH] ref(access): simplify access component (#77859) Simplify types and enforce passing of access type (even though) empty as it is better to explicitly require empty set of access level permissions for such components. --- static/app/components/acl/access.spec.tsx | 28 ++++--- static/app/components/acl/access.tsx | 80 ++++++++++--------- .../search/sources/commandSource.tsx | 2 +- .../settings/project/permissionAlert.tsx | 4 +- 4 files changed, 65 insertions(+), 49 deletions(-) diff --git a/static/app/components/acl/access.spec.tsx b/static/app/components/acl/access.spec.tsx index 999498fa8cd5a6..b2dd8de38911fc 100644 --- a/static/app/components/acl/access.spec.tsx +++ b/static/app/components/acl/access.spec.tsx @@ -134,7 +134,7 @@ describe('Access', function () { }) ); - render({childrenMock}, {organization}); + render({childrenMock}, {organization}); expect(childrenMock).toHaveBeenCalledWith({ hasAccess: true, @@ -149,9 +149,14 @@ describe('Access', function () { }) ); - render({childrenMock}, { - organization, - }); + render( + + {childrenMock} + , + { + organization, + } + ); expect(childrenMock).toHaveBeenCalledWith({ hasAccess: true, @@ -166,9 +171,14 @@ describe('Access', function () { }) ); - render({childrenMock}, { - organization, - }); + render( + + {childrenMock} + , + { + organization, + } + ); expect(childrenMock).toHaveBeenCalledWith({ hasAccess: true, @@ -208,7 +218,7 @@ describe('Access', function () { ); render( - +

The Child

, {organization} @@ -225,7 +235,7 @@ describe('Access', function () { ); render( - +

The Child

, {organization} diff --git a/static/app/components/acl/access.tsx b/static/app/components/acl/access.tsx index e4513063687526..8f12d61623a1f2 100644 --- a/static/app/components/acl/access.tsx +++ b/static/app/components/acl/access.tsx @@ -1,11 +1,9 @@ -import {Fragment} from 'react'; - import type {Scope} from 'sentry/types/core'; import type {Organization, Team} from 'sentry/types/organization'; import type {Project} from 'sentry/types/project'; import {isRenderFunc} from 'sentry/utils/isRenderFunc'; +import useOrganization from 'sentry/utils/useOrganization'; import {useUser} from 'sentry/utils/useUser'; -import withOrganization from 'sentry/utils/withOrganization'; // Props that function children will get. type ChildRenderProps = { @@ -17,20 +15,23 @@ type ChildRenderProps = { type ChildFunction = (props: ChildRenderProps) => any; type Props = { - organization: Organization; /** * List of required access levels */ - access?: Scope[]; + access: Scope[]; /** * Children can be a node or a function as child. */ - children?: React.ReactNode | ChildFunction; - + children: React.ReactNode | ChildFunction; /** * Requires superuser */ isSuperuser?: boolean; + /** + * Evaluate access against a defined organization. If this is not provided, + * the access is evaluated against the currently active organization. + */ + organization?: Organization; /** * Optional: To be used when you need to check for access to the Project @@ -39,7 +40,7 @@ type Props = { * An "org-member" does not have project:write but if they are "team-admin" for * of a parent team, they will have appropriate scopes. */ - project?: Project | null | undefined; + project?: Project; /** * Optional: To be used when you need to check for access to the Team * @@ -47,7 +48,7 @@ type Props = { * An "org-member" does not have team:write but if they are "team-admin" for * the team, they will have appropriate scopes. */ - team?: Team | null | undefined; + team?: Team; }; /** @@ -55,48 +56,53 @@ type Props = { */ function Access({ children, - isSuperuser = false, - access = [], + organization: overrideOrganization, + isSuperuser, + access, team, project, - organization, }: Props) { const user = useUser(); - team = team ?? undefined; - project = project ?? undefined; + const implicitOrganization = useOrganization(); + const organization = overrideOrganization || implicitOrganization; - const hasAccess = hasEveryAccess(access, {organization, team, project}); const hasSuperuser = Boolean(user?.isSuperuser); - - const renderProps: ChildRenderProps = { - hasAccess, - hasSuperuser, - }; - - const render = hasAccess && (!isSuperuser || hasSuperuser); + const hasAccess = hasEveryAccess(access, { + organization, + team, + project, + }); if (isRenderFunc(children)) { - return children(renderProps); + return children({ + hasAccess, + hasSuperuser, + }); } - return {render ? children : null}; + const render = hasAccess && (!isSuperuser || hasSuperuser); + return render ? children : null; } export function hasEveryAccess( access: Scope[], - props: {organization?: Organization; project?: Project; team?: Team} -) { - const {organization, team, project} = props; - const {access: orgAccess} = organization || {access: [] as Organization['access']}; - const {access: teamAccess} = team || {access: [] as Team['access']}; - const {access: projAccess} = project || {access: [] as Project['access']}; + entities: { + organization?: Organization | null; + project?: Project | null; + team?: Team | null; + } +): boolean { + const hasOrganizationAccess = entities.organization + ? access.every(acc => entities.organization?.access?.includes(acc)) + : false; + const hasTeamAccess = entities.team + ? access.every(acc => entities.team?.access?.includes(acc)) + : false; + const hasProjectAccess = entities.project + ? access.every(acc => entities.project?.access?.includes(acc)) + : false; - return ( - !access || - access.every(acc => orgAccess.includes(acc)) || - access.every(acc => teamAccess?.includes(acc)) || - access.every(acc => projAccess?.includes(acc)) - ); + return !access.length || hasOrganizationAccess || hasTeamAccess || hasProjectAccess; } -export default withOrganization(Access); +export default Access; diff --git a/static/app/components/search/sources/commandSource.tsx b/static/app/components/search/sources/commandSource.tsx index 2fdb7c996e263e..bde936dffce2fc 100644 --- a/static/app/components/search/sources/commandSource.tsx +++ b/static/app/components/search/sources/commandSource.tsx @@ -167,7 +167,7 @@ class CommandSource extends Component { function CommandSourceWithFeature(props: Omit) { return ( - + {({hasSuperuser}) => } ); diff --git a/static/app/views/settings/project/permissionAlert.tsx b/static/app/views/settings/project/permissionAlert.tsx index e5180c9edc865e..6517b244f180d4 100644 --- a/static/app/views/settings/project/permissionAlert.tsx +++ b/static/app/views/settings/project/permissionAlert.tsx @@ -7,8 +7,8 @@ import type {Project} from 'sentry/types/project'; interface Props extends React.ComponentPropsWithoutRef { access?: Scope[]; - project?: Project | null | undefined; - team?: Team | null | undefined; + project?: Project; + team?: Team; } export const permissionAlertText = t(