aminghadersohi commented on PR #42659:
URL: https://github.com/apache/superset/pull/42659#issuecomment-5332626347
Pushed `8a01ea6fdc` addressing @gabotorresruiz's two review comments —
details in the inline replies:
1. **Permalink `dashboardId` is a UUID string, not an int.** The `int(...)`
comparison dropped shared state for every permalink created since 3.1 and
warned "belongs to a different dashboard" for the *same* dashboard. The
permalink-resolved path no longer re-verifies at all
(`DashboardLookupResult.resolved_from_permalink`), and the explicit
`identifier` + `permalink_key` combination compares the reference against the
dashboard's id, uuid, and slug. Covered by new UUID-`dashboardId` tests for
both `get_dashboard_info` and `get_dashboard_layout`, a legacy-slug test, and
parametrized helper tests in `test_permalink.py`.
2. **Identifier typos no longer report the permalink message.** The original
not-found error is preserved whenever the lookup produced one; the "ask for a
fresh shared dashboard link" wording is reserved for requests that had to
resolve through a permalink. `{"identifier": "sales-dashbord"}` again returns
`DashboardInfo with identifier 'sales-dashbord' not found`, asserted for both
tools.
On the remaining bot suggestion (`mock_permalink.assert_not_called()` in
`test_get_dashboard_layout_identifier_takes_precedence_over_permalink`): that
one is incorrect and was not applied. When `identifier=10` *and*
`permalink_key` are both supplied, `lookup_dashboard_reference` deliberately
does resolve the permalink — the identifier picks the dashboard, and the
permalink still contributes state when it points at that same dashboard. The
assertion would fail. The precedence contract is already asserted by
`mock_find.assert_called_once_with(10, query_options=None)` and
`is_permalink_state is False`.
`pytest tests/unit_tests/mcp_service/dashboard/` — 393 passed.
--
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]
