aminghadersohi commented on code in PR #45504:
URL: https://github.com/apache/superset/pull/45504#discussion_r4331788526
##########
superset/mcp_service/app.py:
##########
@@ -329,7 +329,7 @@ def get_default_instructions(
- list_dashboards(request={{"created_by_me": true, "edited_by_me": true}})
- list_datasets(request={{"created_by_me": true, "edited_by_me": true}})
-Use created_by_me for authorship, edited_by_me for edit access, or both
+For dashboards, edited_by_me means direct USER editor (owner), excluding
role/group access. Use created_by_me for authorship, edited_by_me for direct
editor membership, or both
Review Comment:
Fixed in 69092bd509. I removed the dashboard-only qualifier and applied
strict unknown-field validation with suggestions to the chart, dataset, report
and database list schemas. One distinction: ListDatabasesRequest has
CreatedByMeMixin, not EditedByMeMixin, so its supported self filter remains
created_by_me.
Regression: test_editor_instructions_apply_to_all_editor_lists,
test_self_scoped_lists_reject_unknown_fields and
test_sibling_list_unknown_scope_never_reaches_dao failed against 3b40f8a29f and
passed in the scoped run (361 passed).
test_self_scoped_lists_preserve_filters_and_aliases passes on both versions to
preserve supported filters and aliases. Command: `pytest -q -m 'not manual and
not llm_compliance'
tests/unit_tests/mcp_service/common/test_list_scope_validation.py
tests/unit_tests/mcp_service/test_mcp_config.py`.
Siblings checked:
- superset/mcp_service/chart/schemas.py — fixed in 69092bd509
- superset/mcp_service/dataset/schemas.py — fixed in 69092bd509
- superset/mcp_service/report/schemas.py — fixed in 69092bd509
- superset/mcp_service/database/schemas.py — fixed in 69092bd509
- superset/mcp_service/dashboard/schemas.py — fixed in a11f4c1b43
- superset/daos/dashboard.py — not affected: editor and creator-or-editor
filters already constrain Subject.type to USER and match user_id.
- superset/daos/chart.py — not affected: the same direct-user subject
predicates already apply.
- superset/daos/dataset.py — not affected: the same direct-user subject
predicates already apply.
- superset/daos/report.py — not affected: the same direct-user subject
predicates already apply.
##########
superset/mcp_service/dashboard/tool/duplicate_dashboard.py:
##########
@@ -292,8 +292,9 @@ async def duplicate_dashboard(
"""
Duplicate an existing dashboard under a new title.
- By default the copy references the same charts as the source.
- Set duplicate_slices=true to also deep-copy every chart into new
+ By default the copy deep-copies charts so subsequent edits are isolated.
+ Set duplicate_slices=false only to deliberately share source charts.
+ With duplicate_slices=true, every chart is copied into new
chart objects owned by you, so edits to the copies never affect
the originals.
Review Comment:
Fixed in cb9e08b488455b0a5642f8421466493073c71acb with the suggested
wording: chart isolation is described as the default, and only deliberate
sharing is framed as an opt-in.
Regression: test_duplicate_description_explains_isolated_default fails
against 3b40f8a29f and passes in the scoped run (361 passed). It reads the
registered tool description and pairs it with the true default in
DuplicateDashboardRequest. Command: `pytest -q
tests/unit_tests/mcp_service/dashboard/tool/test_duplicate_dashboard.py::test_duplicate_description_explains_isolated_default`.
Siblings checked:
- superset/mcp_service/dashboard/schemas.py — not affected:
DuplicateDashboardRequest already advertises default=true and explicit false
sharing.
- docs/admin_docs/configuration/mcp-server.mdx — not affected: the copying
guide already describes isolated copies as the default.
- superset/dashboards/schemas.py — not affected: the REST copy contract has
a separate default; this is MCP guidance only.
- superset/daos/dashboard.py — not affected: both explicit copy modes are
retained; no copy semantics changed by this wording edit.
--
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]
