mikebridge commented on code in PR #44370:
URL: https://github.com/apache/superset/pull/44370#discussion_r4050457216
##########
superset/common/query_object.py:
##########
@@ -405,6 +406,38 @@ def validate(
) -> QueryObjectValidationError | None:
"""Validate query object"""
try:
+ if self.datasource and self.datasource.type == "semantic_view":
+ try:
+ cast(
+ "SemanticView", self.datasource
+ ).implementation.validate_selection_version(
+ self.extras.get("semantic_selection_version")
+ )
+ except ValueError as ex:
+ if self.extras.get("semantic_selection_version") == (
+ "unverified-external-selections"
+ ):
Review Comment:
This remains tracked in the PR description under **REVIEW FOLLOW-UPS**:
`unverified-external-selections` is a duplicated frontend/backend wire
sentinel. Kept that follow-up separate from this behavior fix; the
producer/consumer contract remains covered by the identity tests.
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FiltersConfigForm.tsx:
##########
@@ -940,6 +960,48 @@ const FiltersConfigForm = (
forceRender: true,
children: (
<>
+ <FormItem
+ hidden
+ name={['filters', filterId, 'semantic_selection_version']}
+ initialValue={
+ (filterToEdit ?? customizationToEdit)?.targets?.[0]
+ ?.semantic_selection_version
+ }
+ />
+ {datasetDetails?.semantic_selection_version &&
+ formFilter?.semantic_selection_version !==
+ datasetDetails.semantic_selection_version && (
+ <Alert
+ type="warning"
+ message={t('Choose current semantic filter fields')}
+ description={t(
+ 'Start field selection using current member IDs. This
clears any existing field selections, pre-filters, sorting, defaults and
dependencies. Saved display titles cannot be recovered automatically.',
+ )}
+ action={
+ <Button
+ onClick={() => {
+ setNativeFilterFieldValues(form, filterId, {
+ semantic_selection_version:
+ datasetDetails.semantic_selection_version,
+ column: undefined,
+ adhoc_filters: [],
+ granularity_sqla: undefined,
+ sortMetric: null,
+ defaultDataMask: {},
+ dependencies: [],
+ defaultValue: undefined,
+ controlValues: {},
+ });
Review Comment:
Fixed in
[9a1ee5f248](https://github.com/apache/superset/commit/9a1ee5f2483e868ba1fa47a35f08a5cadb2f1dae).
Reset clears `time_range`, `time_grains`, and both pre-filter controls,
suppresses saved-value fallbacks, and resets the collapsible UI state. The
save/reopen regression failed with the old pre-filter still checked and passes
with the pre-filters removed.
##########
superset/mcp_service/semantic_layer/tool/get_table.py:
##########
@@ -220,6 +220,7 @@ def _build_query_dict(
dimensions=request.dimensions,
filters=[{"col": f.col, "op": f.op, "val": f.val} for f in
request.filters],
time_range=request.time_range,
+ semantic_selection_version=request.semantic_selection_version,
Review Comment:
Fixed in
[9a1ee5f248](https://github.com/apache/superset/commit/9a1ee5f2483e868ba1fa47a35f08a5cadb2f1dae).
After the existing access check, the external-view resolver catches
`ValueError` specifically from selection-version validation and returns
`ValidationError` before execution. Missing/stale-version regression cases
pass; unrelated errors retain their existing classification.
##########
superset-frontend/src/dataMask/reducer.ts:
##########
@@ -162,6 +162,12 @@ function fillNativeFilters(
...getInitialDataMask(filter.id), // take initial data
...filter.defaultDataMask, // if something new came from BE - take it
...loaded,
+ // A restored value must not inherit the default's identity evidence.
+ ...(filter.targets?.[0]?.semantic_selection_version &&
+ loaded?.filterState &&
+ !loaded.extraFormData
+ ? { extraFormData: {} }
+ : {}),
Review Comment:
Folded in
[9a1ee5f248](https://github.com/apache/superset/commit/9a1ee5f2483e868ba1fa47a35f08a5cadb2f1dae)
into `getRestoredSelectionEvidence`, shared by native-filter and
chart-customization hydration. Existing restoration/default precedence is
preserved, and the hydration regression suite passes.
##########
superset-frontend/src/dashboard/util/charts/getFormDataWithExtraFilters.ts:
##########
@@ -568,6 +630,13 @@ export default function getFormDataWithExtraFilters({
? getExtraFormData(dataMask, customizationIds)
: {};
+ const appliedGroupByIds = new Set(
+ Object.keys(groupByFormData).length
+ ? getMatchingGroupByCustomizations(groupByCustomizations, chart)
+ .filter(item => groupByState[item.id]?.selectedValues.length)
Review Comment:
Confirmed and fixed in
[9a1ee5f248](https://github.com/apache/superset/commit/9a1ee5f2483e868ba1fa47a35f08a5cadb2f1dae).
Group-by processing returns applied IDs from actual contributed columns,
intersected with the final chart-specific output, rather than from raw selected
values. Three red-first cases cover non-string, empty, and conflicting
selections. The extracted provenance helper from ba79bfaa remains; actual
dynamic-groupby selections still retain the documented unsupported/uncertified
status.
--
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]
