Skip to content

feat(APP-1003): Iterate on permissions graph view from feedback - #1238

Closed
thekidnamedkd wants to merge 47 commits into
mainfrom
app-942-permissions-graph-finish
Closed

feat(APP-1003): Iterate on permissions graph view from feedback#1238
thekidnamedkd wants to merge 47 commits into
mainfrom
app-942-permissions-graph-finish

Conversation

@thekidnamedkd

@thekidnamedkd thekidnamedkd commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Description

This PR delivers the APP-1003 refinement pass on top of the finished APP-942 graph view.

The underlying APP-942 permissions graph work is already checked in; this summary only covers the APP-1003 refinement delta.

Related base graph work: APP-942 / superseded graph PR #1202.

APP-1003 refinement (permissions feedback)

  • Consolidate the view into a single graph screen driven by toggles; remove the Granted/Other tabs
  • Treat each permission as a distinct node instead of merging under a shared "anyone" header
  • Standardize resource-link display and show the permission ID above the name
  • Replace explicit "no condition"/"no description" labels with a dash placeholder
  • Add graph full-screen mode with Escape-to-exit
  • Rename the secondary switch to Show supporting permissions and add concise supporting-permissions help copy
  • Add informational tooltips to the Who / Where permissions-list headers
  • Prefer backend-enriched who / where / conditionEntity permission metadata and keep local plugin/account matching as a fallback
  • Move DAO self-permission stacks south of the DAO node and align stack handles
  • Hide stale/supporting entities from the default graph and explicitly label historical/uninstalled plugin nodes when shown

Type of Change

  • Minor: Feature (non-breaking change which adds new functionality)

Developer Checklist:

  • Manually smoke tested the functionality locally against the Citrea DAO permissions path above
  • Confirmed there are no new warnings on automated tests
  • Selected the correct base branch
  • Followed the code style guidelines of this project
  • Reviewed that the Files Changed in Github's UI reflect my intended changes
  • Confirmed the pipeline checks are not failing — pending CI

Review Checklist:

  • Tested in a preview or locally that the graph and list permissions views satisfy the APP-942 / APP-1003 UX paths
  • Confirmed that changes follow the code style guidelines of this project

@thekidnamedkd
thekidnamedkd requested a review from a team as a code owner July 15, 2026 09:06
@linear-code

linear-code Bot commented Jul 15, 2026

Copy link
Copy Markdown

APP-942

APP-1003

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

🚀 Preview Deployment: View Here

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown

E2E results (preview)

Smoke

Base URL https://app-next-5mzs5m7yr-aragon-app.vercel.app
Suite smoke
Playwright result ✅ passed
Summary 82 passed
HTML report View report
GitHub job ✅ completed

View run #4736

@milosh86 milosh86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great, left some housekeeping comments.

Comment thread apps/app/src/modules/settings/components/permissionsGraph/permissionsGraph.tsx Outdated
milosh86
milosh86 previously approved these changes Jul 16, 2026

@milosh86 milosh86 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great job 🙌 I've just tested it and it looks great!

One usage feedback, not related to implementation: Unknown address is not really helpful. You need to click to see which address is that, but it could be handy to have truncated address as a label instead.

@thekidnamedkd

Copy link
Copy Markdown
Contributor Author

Great job 🙌 I've just tested it and it looks great!

One usage feedback, not related to implementation: Unknown address is not really helpful. You need to click to see which address is that, but it could be handy to have truncated address as a label instead.

Yes great feedback. I had it the other way briefly as well Jordi said the two felt kind of redundant want and he preferred the parity of a detail inspect pop out, but let's see what consensus is. This is still kind of discovery mode when it comes to the "feel" stuff.

@thekidnamedkd
thekidnamedkd force-pushed the app-942-permissions-graph-finish branch from ede6757 to 089da6a Compare July 30, 2026 08:58
Render CREATE_PROPOSAL_PERMISSION on governing bodies as per-target creator nodes so who/where contracts no longer share a handle. Anyone uses the members icon, multisig shows 'Members of X', Safe and plugin creators keep body styling. Keep these rows visible under both filters, show their condition labels, and add the Safe logo to the list view.
When a governing body has an open Anyone (ANY_ADDR) create-proposal
grant, the graph builder now drops the more-specific create-proposal
creators (Safe, multisig, internal stage bodies) on that same body.
Keys off the presence of an Anyone grant rather than the backend
processInternal label, so it is robust to that classification. Reverts
the earlier processInternal structural exclusions in builder and filter;
the list view keeps every row as a raw audit.
@thekidnamedkd

Copy link
Copy Markdown
Contributor Author

Superseded by #1276, which contains the full audit remediation plus this branch's unique contribution (governing-body actor nodes keyed per (who, where)) ported as 684de8572. Closing in favor of that PR.

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.

3 participants