Skip to content

Commit 3b5ae71

Browse files
ref(js): Convert acl/access to a FC (#48031)
1 parent 31a7c1c commit 3b5ae71

File tree

3 files changed

+47
-142
lines changed

3 files changed

+47
-142
lines changed

static/app/components/acl/access.spec.jsx

Lines changed: 10 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,10 @@ describe('Access', function () {
1616
childrenMock.mockClear();
1717
});
1818

19-
it('has access when requireAll is false', function () {
20-
render(
21-
<Access access={['project:write', 'project:read', 'org:read']} requireAll={false}>
22-
{childrenMock}
23-
</Access>,
24-
{context: routerContext}
25-
);
26-
27-
expect(childrenMock).toHaveBeenCalledWith({
28-
hasAccess: true,
29-
hasSuperuser: false,
30-
});
31-
});
32-
3319
it('has access', function () {
3420
render(<Access access={['project:write', 'project:read']}>{childrenMock}</Access>, {
3521
context: routerContext,
22+
organization,
3623
});
3724

3825
expect(childrenMock).toHaveBeenCalledWith({
@@ -44,6 +31,7 @@ describe('Access', function () {
4431
it('has no access', function () {
4532
render(<Access access={['org:write']}>{childrenMock}</Access>, {
4633
context: routerContext,
34+
organization,
4735
});
4836

4937
expect(childrenMock).toHaveBeenCalledWith({
@@ -52,39 +40,10 @@ describe('Access', function () {
5240
});
5341
});
5442

55-
it('calls render function when no access', function () {
56-
const noAccessRenderer = jest.fn(() => null);
57-
render(
58-
<Access access={['org:write']} renderNoAccessMessage={noAccessRenderer}>
59-
{childrenMock}
60-
</Access>,
61-
{context: routerContext}
62-
);
63-
64-
expect(childrenMock).not.toHaveBeenCalled();
65-
expect(noAccessRenderer).toHaveBeenCalled();
66-
});
67-
68-
it('can specify org from props', function () {
69-
render(
70-
<Access
71-
organization={TestStubs.Organization({access: ['org:write']})}
72-
access={['org:write']}
73-
>
74-
{childrenMock}
75-
</Access>,
76-
{context: routerContext}
77-
);
78-
79-
expect(childrenMock).toHaveBeenCalledWith({
80-
hasAccess: true,
81-
hasSuperuser: false,
82-
});
83-
});
84-
8543
it('handles no org/project', function () {
8644
render(<Access access={['org:write']}>{childrenMock}</Access>, {
8745
context: routerContext,
46+
organization,
8847
});
8948

9049
expect(childrenMock).toHaveBeenCalledWith(
@@ -101,7 +60,7 @@ describe('Access', function () {
10160
user: null,
10261
};
10362

104-
render(<Access>{childrenMock}</Access>, {context: routerContext});
63+
render(<Access>{childrenMock}</Access>, {context: routerContext, organization});
10564

10665
expect(childrenMock).toHaveBeenCalledWith({
10766
hasAccess: true,
@@ -115,6 +74,7 @@ describe('Access', function () {
11574
};
11675
render(<Access isSuperuser>{childrenMock}</Access>, {
11776
context: routerContext,
77+
organization,
11878
});
11979

12080
expect(childrenMock).toHaveBeenCalledWith({
@@ -129,6 +89,7 @@ describe('Access', function () {
12989
};
13090
render(<Access isSuperuser>{childrenMock}</Access>, {
13191
context: routerContext,
92+
organization,
13293
});
13394

13495
expect(childrenMock).toHaveBeenCalledWith({
@@ -144,7 +105,7 @@ describe('Access', function () {
144105
<Access access={['project:write']}>
145106
<p>The Child</p>
146107
</Access>,
147-
{context: routerContext}
108+
{context: routerContext, organization}
148109
);
149110

150111
expect(screen.getByText('The Child')).toBeInTheDocument();
@@ -158,7 +119,7 @@ describe('Access', function () {
158119
<Access isSuperuser>
159120
<p>The Child</p>
160121
</Access>,
161-
{context: routerContext}
122+
{context: routerContext, organization}
162123
);
163124

164125
expect(screen.getByText('The Child')).toBeInTheDocument();
@@ -169,7 +130,7 @@ describe('Access', function () {
169130
<Access access={['org:write']}>
170131
<p>The Child</p>
171132
</Access>,
172-
{context: routerContext}
133+
{context: routerContext, organization}
173134
);
174135

175136
expect(screen.queryByText('The Child')).not.toBeInTheDocument();
@@ -183,7 +144,7 @@ describe('Access', function () {
183144
<Access isSuperuser>
184145
<p>The Child</p>
185146
</Access>,
186-
{context: routerContext}
147+
{context: routerContext, organization}
187148
);
188149
expect(screen.queryByRole('The Child')).not.toBeInTheDocument();
189150
});

static/app/components/acl/access.tsx

Lines changed: 27 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -1,116 +1,58 @@
1-
import {Component} from 'react';
1+
import {Fragment} from 'react';
22

3-
import {Alert} from 'sentry/components/alert';
4-
import {t} from 'sentry/locale';
5-
import {Config, Organization, Scope} from 'sentry/types';
3+
import ConfigStore from 'sentry/stores/configStore';
4+
import {useLegacyStore} from 'sentry/stores/useLegacyStore';
5+
import {Scope} from 'sentry/types';
66
import {isRenderFunc} from 'sentry/utils/isRenderFunc';
7-
import withConfig from 'sentry/utils/withConfig';
8-
import withOrganization from 'sentry/utils/withOrganization';
9-
10-
const DEFAULT_NO_ACCESS_MESSAGE = (
11-
<Alert type="error" showIcon>
12-
{t('You do not have sufficient permissions to access this.')}
13-
</Alert>
14-
);
7+
import useOrganization from 'sentry/utils/useOrganization';
158

169
// Props that function children will get.
17-
export type ChildRenderProps = {
10+
type ChildRenderProps = {
1811
hasAccess: boolean;
1912
hasSuperuser: boolean;
2013
};
2114

22-
type ChildFunction = (props: ChildRenderProps) => React.ReactNode;
15+
type ChildFunction = (props: ChildRenderProps) => JSX.Element;
2316

24-
type DefaultProps = {
17+
type Props = {
2518
/**
2619
* List of required access levels
2720
*/
28-
access: Scope[];
29-
21+
access?: Scope[];
3022
/**
31-
* Custom renderer function for "no access" message OR `true` to use
32-
* default message. `false` will suppress message.
23+
* Children can be a node or a function as child.
3324
*/
34-
renderNoAccessMessage: ChildFunction | boolean;
35-
25+
children?: React.ReactNode | ChildFunction;
3626
/**
3727
* Requires superuser
3828
*/
3929
isSuperuser?: boolean;
40-
41-
/**
42-
* Should the component require all access levels or just one or more.
43-
*/
44-
requireAll?: boolean;
45-
};
46-
47-
const defaultProps: DefaultProps = {
48-
renderNoAccessMessage: false,
49-
isSuperuser: false,
50-
requireAll: true,
51-
access: [],
5230
};
5331

54-
type Props = {
55-
/**
56-
* Configuration from ConfigStore
57-
*/
58-
config: Config;
59-
60-
/**
61-
* Current Organization
62-
*/
63-
organization: Organization;
64-
65-
/**
66-
* Children can be a node or a function as child.
67-
*/
68-
children?: React.ReactNode | ChildFunction;
69-
} & Partial<DefaultProps>;
70-
7132
/**
7233
* Component to handle access restrictions.
7334
*/
74-
class Access extends Component<Props> {
75-
static defaultProps = defaultProps;
76-
77-
render() {
78-
const {
79-
organization,
80-
config,
81-
access,
82-
requireAll,
83-
isSuperuser,
84-
renderNoAccessMessage,
85-
children,
86-
} = this.props;
35+
function Access({children, isSuperuser = false, access = []}: Props) {
36+
const config = useLegacyStore(ConfigStore);
37+
const organization = useOrganization();
8738

88-
const {access: orgAccess} = organization || {access: []};
89-
const method = requireAll ? 'every' : 'some';
39+
const {access: orgAccess} = organization || {access: []};
9040

91-
const hasAccess = !access || access[method](acc => orgAccess.includes(acc));
92-
const hasSuperuser = !!(config.user && config.user.isSuperuser);
41+
const hasAccess = !access || access.every(acc => orgAccess.includes(acc));
42+
const hasSuperuser = !!(config.user && config.user.isSuperuser);
9343

94-
const renderProps: ChildRenderProps = {
95-
hasAccess,
96-
hasSuperuser,
97-
};
44+
const renderProps: ChildRenderProps = {
45+
hasAccess,
46+
hasSuperuser,
47+
};
9848

99-
const render = hasAccess && (!isSuperuser || hasSuperuser);
49+
const render = hasAccess && (!isSuperuser || hasSuperuser);
10050

101-
if (!render && typeof renderNoAccessMessage === 'function') {
102-
return renderNoAccessMessage(renderProps);
103-
}
104-
if (!render && renderNoAccessMessage) {
105-
return DEFAULT_NO_ACCESS_MESSAGE;
106-
}
107-
108-
if (isRenderFunc<ChildFunction>(children)) {
109-
return children(renderProps);
110-
}
111-
112-
return render ? children : null;
51+
if (isRenderFunc<ChildFunction>(children)) {
52+
return children(renderProps);
11353
}
54+
55+
return <Fragment>{render ? children : null}</Fragment>;
11456
}
11557

116-
export default withOrganization(withConfig(Access));
58+
export default Access;

static/app/views/alerts/rules/issue/details/ruleDetails.spec.jsx

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ describe('AlertRuleDetails', () => {
1919
});
2020
const member = TestStubs.Member();
2121

22-
const createWrapper = (props = {}, newContext) => {
22+
const createWrapper = (props = {}, newContext, org = organization) => {
2323
const params = {
24-
orgId: organization.slug,
24+
orgId: org.slug,
2525
projectId: project.slug,
2626
ruleId: rule.id,
2727
};
@@ -38,7 +38,7 @@ describe('AlertRuleDetails', () => {
3838
{...props}
3939
/>
4040
</RuleDetailsContainer>,
41-
{context: routerContext, organization}
41+
{context: routerContext, organization: org}
4242
);
4343
};
4444

@@ -232,14 +232,16 @@ describe('AlertRuleDetails', () => {
232232
});
233233

234234
it('mute button is disabled if no alerts:write permission', async () => {
235+
const orgWithoutAccess = {
236+
features: ['issue-alert-incompatible-rules', 'mute-alerts'],
237+
access: [],
238+
};
239+
235240
const contextWithoutAccess = initializeOrg({
236-
organization: {
237-
features: ['issue-alert-incompatible-rules', 'mute-alerts'],
238-
access: [],
239-
},
241+
organization: orgWithoutAccess,
240242
});
241243

242-
createWrapper({}, contextWithoutAccess);
244+
createWrapper({}, contextWithoutAccess, orgWithoutAccess);
243245

244246
expect(await screen.findByRole('button', {name: 'Mute'})).toBeDisabled();
245247
});

0 commit comments

Comments
 (0)