EnxDev commented on PR #38584:
URL: https://github.com/apache/superset/pull/38584#issuecomment-5507637459

   ## EnxDev's Review Agent — apache/superset#38584 · HEAD a415661
   **lgtm** — no changes in substance since the prior pass; branch was just 
merged cleanly with master.
   
   Re-review superseding [my pass on 
`8915de0`](https://github.com/apache/superset/pull/38584#issuecomment-4952993024).
 The only commit since then is `Merge branch 'master' into 32961-fix` — no 
functional diff. Both previously-resolved concerns still hold at this HEAD:
   
   - **Info-leak (403 vs 422)** — `_validate_new_dashboard_access` 
(`superset/commands/chart/update.py:115-129`) still resolves 
`accessible_dashboards` via the base-filtered `DashboardDAO.find_by_ids` first, 
appends `DashboardsNotFoundValidationError` and returns for unauthorized IDs, 
and only then checks `is_managed_externally`/`is_editor` — inaccessible 
dashboards consistently read as not-found.
   - **Single ownership/editor check** — no duplicate loop; `update.py` and 
`create.py` both use the same merged `dash.is_managed_externally or not 
security_manager.is_editor(dash)` condition, matching each other.
   - **Filter validity** — `is_managed_externally` is in `search_columns` and 
`BASE_LIST_COLUMNS` (`superset/dashboards/api.py`), so the Save modal's `{col: 
'is_managed_externally', opr: 'eq', value: false}` filter is accepted and the 
on-mount guard reads a real value rather than `undefined`.
   - **Test hygiene** — the route-cleanup and `mockLoadDashboards` reset items 
from the prior review are both now handled by a guaranteed `afterEach` in 
`SaveModal.test.tsx`.
   
   Backend (create + update rejection) and frontend (no-preselect, 
filter-present) paths are covered, and CI is green.
   
   <!-- enxdev-review-agent:a415661 -->
   _Reviewed by EnxDev's Review Agent — @EnxDev · HEAD a415661._
   


-- 
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