Page MenuHomeElementl

[dagit] Permissions
ClosedPublic

Authored by dish on May 10 2021, 7:20 PM.

Details

Summary

Introduce permissions in Dagit UI. By default, all permissions are true.

Throughout the app, use permission checks to disable or hide pieces of UI that should be unavailable to the viewer.

Test Plan

Force permissions to be false across the board.

For each location in Dagit where permissions are checked, verify that the relevant button/link/control is disabled or removed.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

dish requested review of this revision.May 10 2021, 7:29 PM
This revision is now accepted and ready to land.May 10 2021, 7:33 PM

This looks great! Exciting to see a permissions layer coming together. I think it'd be nice to try making a container component that applies the disabled + tooltip but it's pretty clean already with hook approach!

js_modules/dagit/packages/core/src/execute/LaunchRootExecutionButton.tsx
23

This is a really nice way to expose the permissions throughout, much better than misc if statements 👍

js_modules/dagit/packages/core/src/instance/InstanceBackfills.tsx
311

I wonder if we should try to standardize on disabling vs removing things you don't have permission to do? This seems like a case where it might be nice to do a "disabled + tooltip on menu item" so that folks know they're looking in the right place but don't have access.

I wonder if we could make a helper of some sort like:

<Permissioned requires={canCancelPartitionBackfill} disabledMessage={...}>
   <MenuItem Or Button>
</Permissioned>

That does a React.cloneElement of it's children to wrap in a tooltip and pass a disabled prop?

js_modules/dagit/packages/core/src/schedules/ScheduleDetails.tsx
79

Good call this cleans this up quite a bit 👍

js_modules/dagit/packages/core/src/instance/InstanceBackfills.tsx
311

Yeah, I think there may be some ways to dedupe this. I didn't want to get too into the weeds on that yet, especially since different controls need different behavior (tooltip, disabled, not-rendered, etc.)

This revision was automatically updated to reflect the committed changes.