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

   ### SUMMARY
   
   Fixes the semantic-view chart-save regression tracked in 
[SC-121388](https://app.shortcut.com/preset/story/121388).
   
   #43500 correctly rejected SQL Lab query objects, but its TABLE-only guard 
also rejected persistent semantic views. #43781 provides type-aware 
`Slice.resolved_datasource` resolution. This change allows only TABLE and 
SEMANTIC_VIEW, retaining the `query`/`saved_query` rejection.
   
   The Cube member-identity walkthrough stopped at step 4: Save as sent 
`datasource_type: "semantic_view"` and received HTTP 422, **"Datasource type is 
invalid"**. Create and overwrite can now resolve the semantic view, populate 
its name, and enforce datasource access.
   
   Independent Claude review confirmed namespace-exact lookup, manager-based 
grants, query-context type/id binding, and denial on both POST and PUT. The 
review's policy-consistency and type-only coverage findings have been folded in:
   
   - Explore form_data keys and permalinks use the same manager policy as chart 
saves; semantic-view keys work instead of returning 400 for an unsupported type.
   - POST datasource-access denial returns the documented 403 instead of an 
unhandled 500, including TABLE denials.
   - A type-only TABLE PUT is deliberately tightened from a no-op to validation 
of the retained ID in the requested namespace: allowed succeeds, missing/null 
ID yields 422, and denied access yields 403.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   Backend change; no UI layout change. Before: semantic-view Save as/overwrite 
returns 422. After: allowed saves return 201/200 and persist the semantic-view 
identity. Browser save/reopen on the integrated provider stack remains to be 
re-exercised after the core fix is consumed.
   
   ### TESTING INSTRUCTIONS
   
   1. As a user with access to a semantic view, save a new Explore chart and 
overwrite it. Verify the datasource remains `semantic_view`, including after 
reopening.
   2. Deny datasource access and verify 403; verify `query` and `saved_query` 
create requests still return 422.
   3. Exercise type-only updates for both TABLE and SEMANTIC_VIEW, including 
missing datasource and denied access.
   
   Local verification on the final tree:
   - Chart command and Explore unit suites: **99 passed**.
   - Semantic-save HTTP tests plus the complete chart API suite on isolated 
SQLite: **110 passed**.
   - Changed-file pre-commit hooks: **passed**, including MyPy.
   - Direct Pylint on all five production Python files: **10.00/10**.
   - Before production changes, regression tests reproduced the original 
semantic-save rejection; review-fold controls additionally failed for null IDs 
and the wrong Explore policy path before passing after the fold.
   
   Follow-ups (not claimed covered): align the shared `get_datasource_by_id` 
return annotation/consumers with SemanticView; add a real Gamma 
view/parent-layer grant HTTP case with provider-backed metadata and permission 
cleanup. Existing HTTP positives use admin and denial injection tests the 
route's error translation. No live-provider/browser validation or 
PostgreSQL/MySQL run is claimed here.
   
   ### ADDITIONAL INFORMATION
   
   - [x] Has associated issue: 
[SC-121388](https://app.shortcut.com/preset/story/121388); related #43500 and 
#43781.
   - [x] Required feature flags: semantic-layer UI requires `SEMANTIC_LAYERS`.
   - [ ] Changes UI
   - [ ] Includes DB Migration
     - [ ] 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
   
   Codex authored; Claude independently reviewed. Human review remains required.
   


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