mikebridge commented on PR #45109:
URL: https://github.com/apache/superset/pull/45109#issuecomment-6075418311
Thanks @aminghadersohi, this was a careful read. Changes are in 77d82cd5ca.
1. **The marker is an assertion, not proof.** Agreed. UPDATING already calls
it "a compatibility contract, not an authorization credential". I've said the
same in the code comment at the gate and in the user docs: anyone can copy the
version from datasource metadata, it only prevents stale-title collisions, and
authorization is checked before it.
2. **Chart-level marker vs per-filter provenance.** Added a docs line: the
version is recorded per chart, so a filter column added by hand-editing saved
form data inherits it. Suggestions are a usability aid for compatible
selections, not a per-filter provenance guarantee.
3. **Remount drops typed text.** Intended. The key changes only on
reselection or a datasource change, and clearing the previous generation's
options matters more there than keeping half-typed text.
4. **Can loading stick?** In one narrow case, yes. An in-flight load aborted
by a change that keeps the values select visible but takes an early return
(switching to a datasource with `filter_select: false`) skipped its own reset.
I fixed it by resetting loading at the start of each effect run. The new test
fails without the fix.
5. **Test coverage.**
- *Cache key:* `test_unversioned_suggestion_cache_material_is_unchanged`
asserts that an unversioned view's key material keeps exactly the previous five
fields, even when a marker is sent. Together with the existing legacy-bytes
test of `_column_values_cache_key`, that pins the key end to end. Tables take
the same path with no selection version.
- *`isExtra`:* already covered. The `test.each` case
`['cube-member-id-v1', true]` in
`AdhocFilterEditPopoverSimpleTabContent.test.tsx` expects no marker for
dashboard extra filters.
6. **Silent `undefined`.** Agreed it's silent. It degrades to manual entry,
which is the safe direction, so I've left it as is.
--
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]
