Skip to content

refactor(ui): share asset wipe permission logic - #34041

Open
sidsri14 wants to merge 1 commit into
dagster-io:masterfrom
sidsri14:refactor-asset-wipe-permission
Open

refactor(ui): share asset wipe permission logic#34041
sidsri14 wants to merge 1 commit into
dagster-io:masterfrom
sidsri14:refactor-asset-wipe-permission

Conversation

@sidsri14

Copy link
Copy Markdown

Summary

  • extract the deployment, location, and per-asset wipe permission cascade into a shared predicate
  • use the same predicate for single-asset and multi-asset wipe actions
  • add focused coverage for every permission level and denied/mismatched-location cases

Why

The two wipe hooks implemented the same three-level permission rule independently. Keeping the rule in one predicate prevents the single-asset and multi-asset paths from diverging when permission behavior changes.

Fixes #34001

Validation

  • yarn workspace @dagster-io/ui-core eslint src/assets/assetWipePermissions.ts src/assets/__tests__/assetWipePermissions.test.ts src/assets/useWipeDialog.tsx src/assets/useWipeMaterializations.tsx
  • yarn workspace @dagster-io/ui-core ts
  • yarn workspace @dagster-io/ui-core jest src/assets/__tests__/useAssetsPermissions.test.tsx src/assets/__tests__/assetWipePermissions.test.ts --runInBand (10 passed)
  • yarn workspace @dagster-io/ui-core jest-all-silent (160 suites, 1,314 passed, 4 skipped)

@sidsri14
sidsri14 marked this pull request as ready for review July 24, 2026 09:38
@greptile-apps

greptile-apps Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

Centralizes the asset-wipe permission cascade and reuses it across single-asset and multi-asset wipe actions.

  • Adds a shared predicate covering deployment-wide, location-scoped, and per-asset permissions.
  • Replaces duplicated permission checks in both wipe hooks.
  • Adds focused unit coverage for each grant level, denial, and mismatched locations.

Confidence Score: 5/5

The PR appears safe to merge with no actionable regressions identified.

The shared predicate preserves the prior permission ordering and both current wipe paths supply the inputs needed to evaluate deployment-wide, location-scoped, and per-asset grants.

Important Files Changed

Filename Overview
js_modules/ui-core/src/assets/assetWipePermissions.ts Introduces a shared predicate that preserves the existing three-level wipe-permission cascade.
js_modules/ui-core/src/assets/useWipeDialog.tsx Delegates single-asset wipe authorization to the shared predicate without changing behavior for current callers.
js_modules/ui-core/src/assets/useWipeMaterializations.tsx Delegates each selected asset's authorization check to the shared predicate while retaining all-assets-required semantics.
js_modules/ui-core/src/assets/tests/assetWipePermissions.test.ts Covers every permission level, complete denial, and rejection of permission from an unrelated location.

Reviews (1): Last reviewed commit: "refactor(ui): share asset wipe permissio..." | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[ui] Asset wipe permission cascade is duplicated between useWipeDialog and useWipeMaterializations

1 participant