aminghadersohi commented on PR #44096:
URL: https://github.com/apache/superset/pull/44096#issuecomment-5621515394
Follow-up covering every review item, including the three inline comments
added after the earlier reply:
- **Malformed permalink hydration:** guard `adhoc_filters` with
`Array.isArray` and tolerate null/undefined entries. Added eight
malformed-input regression cases verifying that a required select filter
restores its default instead of throwing.
- **Permalink deployment URLs:** use Superset's existing
`get_url_path("Superset.dashboard_permalink", user_friendly=True, key=...)`
rather than the MCP helper's hardcoded localhost fallback. Tests cover the
configured public origin, root and subdirectory deployments, request and
non-request contexts, and proxy `SCRIPT_NAME`. Documented the public URL and
mount-prefix configuration.
- **Concurrent channel initialization:** serialize cache initialization and
recheck the app cache inside the lock. A deterministic two-thread regression
test forces simultaneous initial cache misses and asserts that the callable
runs exactly once and both callers receive the same channel. This preserves the
once-per-app contract explained above; per-publish context-dependent deployment
channels would still violate the websocket consumer's fixed subscription.
- **Shared clear predicate and toast callback:** both earlier nits remain
addressed by the shared frontend/backend constants, hydration coverage, and
explicit non-serializability comment.
- **Translation CI:** synchronized only the seven flagged Simplified Chinese
entries with the confirmed corrections already on upstream master. Ran
extraction/update for both the base and PR source and the translation
regression comparison: **no translation regressions**.
Local validation: **86 focused backend/publisher tests, 41 TaskManager
tests, and 51 frontend tests passed**. The full PR-changed-file pre-commit gate
also passed.
**Live realtime + Undo screenshots are still outstanding:** there is no
running websocket-enabled stack in this environment, so neither screenshots nor
live browser verification are claimed. This remains a pre-merge validation
item, not something unit tests can replace.
--
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]
