feat(replays): Granular permissions frontend #104671
Conversation
static/app/views/performance/transactionSummary/transactionReplays/transactionReplays.tsx
Show resolved
Hide resolved
| case 'sentry_member_selector': | ||
| return <SentryMemberSelectorField {...(componentProps as RenderFieldProps)} />; |
There was a problem hiding this comment.
Because of this name, at first, I thought this was just for Sentry employees 😁 I know you are just following the pattern, but why do we have this prefix?
| multiple: true, | ||
| visible: ({model, features}) => | ||
| features.has('granular-replay-permissions') && | ||
| model.getValue('hasGranularReplayPermissions'), |
There was a problem hiding this comment.
Won’t this cause the UI to jump? Wouldn’t it be better to show it as disabled with a tooltip, like we usually do?
There was a problem hiding this comment.
It is a very niche feature atm so I don't want it to take up too much space. I think it is fine to add this row directly underneath following the user interaction.
| await waitFor(() => { | ||
| expect( | ||
| screen.getByText("You don't have access to this feature") | ||
| ).toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
can't we use await screen.findByText(...) here?
There was a problem hiding this comment.
right, that would be easier! 😅
| } | ||
| ); | ||
|
|
||
| expect(screen.queryByText('Session Replay')).not.toBeInTheDocument(); |
There was a problem hiding this comment.
if the component is an empty dom element, we can assume that this is not rendered right? can't we drop this and only have the expect below?
| await waitFor(() => { | ||
| expect(screen.queryByTestId('replay-table')).not.toBeInTheDocument(); | ||
| }); |
There was a problem hiding this comment.
wouldn't help if we wait for all loading indicators to be gone first?
There was a problem hiding this comment.
yeah the problem is, we do not even render a loading indicator in this case...
I have no idea what in the document tree fires the request after ~45 minutes i resorted to this hacky solution :/
| expect( | ||
| screen.getByRole('checkbox', {name: 'Restrict Replay Access'}) | ||
| ).toBeInTheDocument(); |
There was a problem hiding this comment.
should we check - just in case - if the checkbox is enabled as well?
| export function ReplayAccessFallbackAlert() { | ||
| return ( | ||
| <Alert | ||
| type="warning" | ||
| showIcon | ||
| defaultExpanded | ||
| expand={t( | ||
| 'Replay access in this organization is limited to certain members. Please contact an organization admin to get access.' | ||
| )} |
There was a problem hiding this comment.
nit: the component name does not really indicate the reason for the fallback
Add org membership settings for allow-listing replay access.
Screen.Recording.2025-12-10.at.09.57.35.mov
Introduce a guard component
<ReplayAccess/>to check for the new permissions and wrap it around affected product areas: