Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(data-secrecy): update fe state & address a couple bugs #82756

Merged
merged 2 commits into from
Dec 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions static/app/views/settings/components/dataSecrecy/index.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,4 +103,38 @@ describe('DataSecrecy', function () {
expect(accessMessage).toBeInTheDocument();
});
});

it('can update permanent access', async function () {
MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/data-secrecy/`,
body: {
accessStart: '2023-08-29T01:05:00+00:00',
accessEnd: '2024-08-29T01:05:00+00:00',
},
});

const mockUpdate = MockApiClient.addMockResponse({
url: `/organizations/${organization.slug}/`,
method: 'PUT',
});

organization.allowSuperuserAccess = false;
render(<DataSecrecy />, {organization});

// Toggle permanent access on
const allowAccessToggle = screen.getByRole('checkbox', {
name: /Sentry employees will not have access to your data unless granted permission/,
});
allowAccessToggle.click();

await waitFor(() => {
expect(mockUpdate).toHaveBeenCalledWith(
`/organizations/${organization.slug}/`,
expect.objectContaining({
method: 'PUT',
data: {allowSuperuserAccess: true},
})
);
});
});
});
68 changes: 49 additions & 19 deletions static/app/views/settings/components/dataSecrecy/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,13 @@ export default function DataSecrecy() {
const api = useApi();
const organization = useOrganization();

// state for the allowSuperuserAccess bit field
const [allowAccess, setAllowAccess] = useState(organization.allowSuperuserAccess);
const [allowDate, setAllowDate] = useState<WaiverData>();

// state of the data secrecy waiver
const [waiver, setWaiver] = useState<WaiverData>();

// state for the allowDateFormData field
const [allowDateFormData, setAllowDateFormData] = useState<string>('');

const {data, refetch} = useApiQuery<WaiverData>(
Expand All @@ -38,12 +43,9 @@ export default function DataSecrecy() {
}
);

const hasValidTempAccess =
allowDate?.accessEnd && moment().toISOString() < allowDate.accessEnd;

useEffect(() => {
if (data?.accessEnd) {
setAllowDate(data);
setWaiver(data);
// slice it to yyyy-MM-ddThh:mm format (be aware of the timezone)
const localDate = moment(data.accessEnd).local();
setAllowDateFormData(localDate.format('YYYY-MM-DDTHH:mm'));
Expand All @@ -57,10 +59,31 @@ export default function DataSecrecy() {
data: {allowSuperuserAccess: value},
});
setAllowAccess(value);
addSuccessMessage(t('Successfully updated access.'));

// if the user has allowed access, we need to remove the temporary access window
// only if there is an existing waiver
if (value && waiver) {
await api.requestPromise(`/organizations/${organization.slug}/data-secrecy/`, {
method: 'DELETE',
});
setAllowDateFormData('');
setWaiver(undefined);
}
addSuccessMessage(
value
? waiver
? t(
'Successfully removed temporary access window and allowed support access.'
)
: t('Successfully allowed support access.')
: t('Successfully removed support access.')
);
} catch (error) {
addErrorMessage(t('Unable to save changes.'));
}

// refetch to get the latest waiver data
refetch();
};

const updateTempAccessDate = async () => {
Expand All @@ -69,7 +92,7 @@ export default function DataSecrecy() {
await api.requestPromise(`/organizations/${organization.slug}/data-secrecy/`, {
method: 'DELETE',
});
setAllowDate({accessStart: '', accessEnd: ''});
setWaiver({accessStart: '', accessEnd: ''});
addSuccessMessage(t('Successfully removed temporary access window.'));
} catch (error) {
addErrorMessage(t('Unable to remove temporary access window.'));
Expand All @@ -86,14 +109,11 @@ export default function DataSecrecy() {
};

try {
await await api.requestPromise(
`/organizations/${organization.slug}/data-secrecy/`,
{
method: 'PUT',
data: nextData,
}
);
setAllowDate(nextData);
await api.requestPromise(`/organizations/${organization.slug}/data-secrecy/`, {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added 2 awaits 😵

method: 'PUT',
data: nextData,
});
setWaiver(nextData);
addSuccessMessage(t('Successfully updated temporary access window.'));
} catch (error) {
addErrorMessage(t('Unable to save changes.'));
Expand Down Expand Up @@ -123,10 +143,20 @@ export default function DataSecrecy() {
help: t(
'Open a temporary time window for Sentry employees to access your organization'
),
disabled: allowAccess && !organization.access.includes('org:write'),
value: allowAccess ? '' : allowDateFormData,
// disable the field if the user has allowed access or if the user does not have org:write access
disabled: allowAccess || !organization.access.includes('org:write'),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a bug, we should disable the date input if data secrecy is disabled OR if the user doesn't have enough permissions

disabledReason: allowAccess
? t('Disable permanent access first to set temporary access')
: !organization.access.includes('org:write')
? t('You do not have permission to modify access settings')
: undefined,
value: allowDateFormData,
onBlur: updateTempAccessDate,
onChange: v => {
// Don't allow the user to set the date if they have allowed access
if (allowAccess) {
return;
}
// the picker doesn't like having a datetime string with seconds+ and a timezone,
// so we remove it -- we will add it back when we save the date
const formattedDate = v ? moment(v).format('YYYY-MM-DDTHH:mm') : '';
Expand All @@ -140,9 +170,9 @@ export default function DataSecrecy() {
<PanelBody>
{!allowAccess && (
<PanelAlert>
{hasValidTempAccess
{waiver?.accessEnd && moment().isBefore(moment(waiver.accessEnd))
? tct(`Sentry employees has access to your organization until [date]`, {
date: formatDateTime(allowDate?.accessEnd as string),
date: formatDateTime(waiver?.accessEnd as string),
})
: t('Sentry employees do not have access to your organization')}
</PanelAlert>
Expand Down
Loading