aminghadersohi commented on PR #43567:
URL: https://github.com/apache/superset/pull/43567#issuecomment-5574561328

   Rechecked `de8ae20193` against the refreshed base and the requests in 
https://github.com/apache/superset/pull/43567#issuecomment-5497039076. The 
earlier aggregate-dimension rejection and shared-builder metric ordering remain 
correct, but the completeness gaps remain:
   
   - **Frontend/native contract:** `FunnelChartConfig` still accepts only one 
`ColumnRef`. Actual `groupby: ["stage"]` with saved/SIMPLE/SQL native metrics 
fails validation, as do multiple dimensions and presentation controls such as 
`show_labels`/`percent_calculation_type`.
   - **Updates:** reproducing a metric-config update on a saved funnel with 
`sort_by_metric=false`, `row_limit=4`, and `color_scheme=lyftColors` resets 
these to `true`, `10`, and `supersetColors` in both immediate and preview-first 
payloads. Immediate updates also lose omitted filters/presentation. 
Preview-first preserves those via its shallow merge; cached updates preserve 
filters but not general presentation/query state. Explicit query values and 
empty-filter clears work in the checked immediate/preview-first paths.
   - **Queries/previews:** captured compile and unsaved-preview queries still 
have `orderby=[]` despite `sort_by_metric=true` (`compile.py:145`, 
`preview_utils.py:108`). For stage/value rows, unsaved Vega has only a color 
encoding—no value axis—and ASCII is a table. Saved Funnel Vega still selects 
fields positionally and unconditionally sorts descending 
(`get_chart_preview.py:894–925`). A mocked query result containing 
`error="query failed"` returns ordinary ASCII/table/Vega previews instead of an 
error.
   - **Validation:** Tier 1 accepts text-valued `MAX(stage)`, a VARCHAR saved 
metric, SQL `MAX(stage)`, and a metric labeled `stage` colliding with the 
dimension. Canonicalization chooses the first of `Stage`/`stage` and 
`Total`/`total`, including filter references, rather than failing on ambiguity.
   - **Discovery/tests:** schema discovery lists Funnel, but primary 
`generate_chart` guidance and progression recommendations remain absent. The 
Funnel tests are unchanged and do not exercise 
generate/update/cached-update/get-data and all preview formats end to end.
   
   Verification: 18 focused Funnel tests and the full MCP chart suite (**1,405 
passed**), plus the reproductions above, using an isolated export of this exact 
head; no contributor code edits. Please address the remaining product paths 
before treating Funnel support as complete.
   
   The sole failing CI job, 
[python-dependency-liccheck](https://github.com/apache/superset/actions/runs/34076974504/job/101604917780),
 failed downloading `setuptools<81` after repeated `pypi.org` DNS-resolution 
failures, **before license checking**. This is an infrastructure failure, not 
evidence of a Funnel regression or license violation.
   


-- 
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]

Reply via email to