EnxDev opened a new pull request, #43392:
URL: https://github.com/apache/superset/pull/43392

   ### SUMMARY
   
   `extra_editors` — editorship a deployment grants indirectly via 
`EXTRA_EDITORS_RESOLVER` — was attached only to single-object `GET` responses 
(`DashboardRestApi.get` / `ChartRestApi.get`), never to list responses. 
`DashboardCard`, `ChartCard`, and the Dashboard/Chart list pages also checked 
`editors` only. Combined, a resolver-granted editor saw the Edit affordance 
disabled on cards/list rows even though the server's `is_editor` already let 
them save once the dashboard/chart was open — a UI-affordance gap, not an 
authorization bug (confirmed via `raise_for_editorship`/`is_editor`, which 
already union `editors` with the resolver output).
   
   - `superset/security/manager.py`: new `get_extra_editors_by_pk` 
batch-resolves
     extra editors for a page of rows in one query, re-querying by the primary
     keys FAB already selected (list responses only expose serialized rows to
     `pre_get_list`, not model instances).
   - `superset/dashboards/api.py` / `superset/charts/api.py`: a new
     `pre_get_list` attaches `extra_editors` per row, gated on
     `EXTRA_EDITORS_RESOLVER` being configured — zero extra cost when it isn't.
   - `permissionUtils.ts`'s `isUserEditorOrAdmin` now takes an optional
     `extraEditors` argument; `DashboardCard`, `ChartCard`, `DashboardList`, and
     `ChartList` pass `extra_editors` alongside `editors`.
   - `DatasetList` is untouched — datasets never got `EXTRA_EDITORS_RESOLVER`
     support, so its `editors`-only check is already correct.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   
https://github.com/user-attachments/assets/ec722c9d-7e11-43de-baba-aa04983de661
   
   ### TESTING INSTRUCTIONS
   - `pytest tests/integration_tests/dashboards/api_tests.py -k extra_editors`
     — 4 new tests: list responses omit/include `extra_editors` for
     dashboards and charts depending on whether `EXTRA_EDITORS_RESOLVER` is
     configured.
   - `npm run test -- permissionUtils.test.ts` — new cases for
     `isUserEditorOrAdmin`'s `extraEditors` union/precedence.
   - Manually: configure `EXTRA_EDITORS_RESOLVER`, grant a non-editor user
     editorship on a dashboard they can view but aren't listed as an editor
     of, then confirm the dashboard list/card Edit affordance is enabled for
     them.
     
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to