sentry icon indicating copy to clipboard operation
sentry copied to clipboard

feat(data-secrecy): Data Secrecy Settings UI

Open iamrajjoshi opened this issue 1 year ago • 9 comments

Here I add the settings for Data Secrecy in the frontend. There are two:

  1. Org settings to disable and enable superuser access
  2. Allow users to temporarily waive data secrecy so sentry employees and help

Updated Demo: https://github.com/getsentry/sentry/pull/75791#issuecomment-2309088811

Old Demo

https://github.com/user-attachments/assets/2e2cb5e1-d8b9-46a0-9b65-64228c99e03d

https://github.com/user-attachments/assets/a2541ccc-eae1-478a-bc99-216514e282fc

Rewatch Links for Demo: https://sentry.rewatch.com/video/ispnhixepw165zl9-screen-recording-2024-08-07-at-1-28-31-pm https://sentry.rewatch.com/video/2jucpfwxswxs0dv6-screen-recording-2024-08-07-at-2-45-14-pm

Update: I moved the waive setting so that its under the original toggle image

iamrajjoshi avatar Aug 07 '24 23:08 iamrajjoshi

Bundle Report

Changes will increase total bundle size by 6.22kB :arrow_up:

Bundle name Size Change
app-webpack-bundle-array-push 28.67MB 6.22kB :arrow_up:

codecov[bot] avatar Aug 07 '24 23:08 codecov[bot]

thoughts about the UI:

can we group the data secrecy toggles in one section? or move the waiver section right underneath the main toggle?

grouping is what i was going for, but the form component we have is very finicky where it only lets us add 1 endpoint for it to call to update values when we use onBlur to update different settings individually. i don't want to refactor the component cause it's used by majority of the UI and I don't have the context for it.

Let me look into moving it tho

iamrajjoshi avatar Aug 08 '24 00:08 iamrajjoshi

thoughts about the UI: can we group the data secrecy toggles in one section? or move the waiver section right underneath the main toggle?

i moved the section!

iamrajjoshi avatar Aug 08 '24 00:08 iamrajjoshi

Major pieces are in place. I can help to prettify this on Fri.

leedongwei avatar Aug 08 '24 23:08 leedongwei

Screenshot 2024-08-22 at 11 45 05 AM

Screenshot 2024-08-22 at 11 44 57 AM

Screenshot 2024-08-22 at 11 44 41 AM

  • Note the function allowTempAccessProps.onChange is still buggy. We are trying to display the datetime in the picker in localtime, but the picker assumes it is in UTC all the time.

leedongwei avatar Aug 22 '24 18:08 leedongwei

Codecov Report

Attention: Patch coverage is 47.91667% with 25 lines in your changes missing coverage. Please review.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...pp/views/settings/components/dataSecrecy/index.tsx 47.82% 24 Missing :warning:
.../settings/organizationSecurityAndPrivacy/index.tsx 50.00% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #75791      +/-   ##
==========================================
+ Coverage   78.15%   78.23%   +0.07%     
==========================================
  Files        6910     6899      -11     
  Lines      307183   306450     -733     
  Branches    50351    50233     -118     
==========================================
- Hits       240080   239749     -331     
+ Misses      60754    60309     -445     
- Partials     6349     6392      +43     

codecov[bot] avatar Aug 22 '24 18:08 codecov[bot]

Note the function allowTempAccessProps.onChange is still buggy. We are trying to display the datetime in the picker in localtime, but the picker assumes it is in UTC all the time.

fixed the issue

https://github.com/user-attachments/assets/4c29e9b7-daa9-43eb-bc2c-3b755d15b1b0

iamrajjoshi avatar Aug 26 '24 00:08 iamrajjoshi

what does it look like when you have an access period in the past and you open the page? should we render an info message for that and/or when you don't have access to toggle it?

cathteng avatar Aug 26 '24 21:08 cathteng

what does it look like when you have an access period in the past and you open the page? should we render an info message for that and/or when you don't have access to toggle it?

how it looks when the access period was in past and u open the page image

this is how it looks when u don't have access to toggle image

it doesn't say anything, but since all the settings on the page can't be edited if u don't have access, it should have image

iamrajjoshi avatar Aug 27 '24 04:08 iamrajjoshi

When the toggle is off and the time is in the past my suggestion is to clear the time field, showing basically there's no access going on at the moment.

sentaur-athena avatar Aug 28 '24 17:08 sentaur-athena

When the toggle is off and the time is in the past my suggestion is to clear the time field, showing basically there's no access going on at the moment.

i agree with when the toggle is off, but my concern with hiding when its in the past is we don't currently show when it was past easily from the UI, so i could see someone waiving then coming back and being confused why it disappeared and think its a bug.

iamrajjoshi avatar Aug 28 '24 18:08 iamrajjoshi

so i could see someone waiving then coming back and being confused why it disappeared

It's fine as is. I don't feel strongly for it.

sentaur-athena avatar Aug 28 '24 21:08 sentaur-athena