Skip to content

Commit 6125454

Browse files
author
Brian Donovan
committed
fix(chart): add independent sort_by_series control for word cloud
Make sort_by_metric and sort_by_series independent controls instead of the previous behavior where series sort was unconditionally appended alongside the metric sort. This allows users to choose metric-only sorting (enabling Druid TopN optimization) while optionally adding a secondary series sort for tie-breaking. Changes: - buildQuery.ts: sort_by_metric and sort_by_series are now independent - controlPanel.tsx: add sort_by_series checkbox (default: false) - buildQuery.test.ts: cover all four sort combinations - DB migration: set sort_by_series=true for existing word_cloud charts to preserve backward compatibility Fixes #39072
1 parent c31db93 commit 6125454

4 files changed

Lines changed: 120 additions & 6 deletions

File tree

superset-frontend/plugins/plugin-chart-word-cloud/src/plugin/buildQuery.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,16 @@ import { buildQueryContext, QueryFormOrderBy } from '@superset-ui/core';
2121
import { WordCloudFormData } from '../types';
2222

2323
export default function buildQuery(formData: WordCloudFormData) {
24-
const { metric, sort_by_metric, series, row_limit } = formData;
24+
const { metric, sort_by_metric, sort_by_series, series, row_limit } =
25+
formData;
2526
const orderby: QueryFormOrderBy[] = [];
2627
const shouldApplyOrderBy =
2728
row_limit !== undefined && row_limit !== null && row_limit !== 0;
2829

2930
if (sort_by_metric && metric) {
3031
orderby.push([metric, false]);
31-
} else if (series) {
32+
}
33+
if (sort_by_series && series) {
3234
orderby.push([series, true]);
3335
}
3436

superset-frontend/plugins/plugin-chart-word-cloud/src/plugin/controlPanel.tsx

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,21 @@ const config: ControlPanelConfig = {
3535
['adhoc_filters'],
3636
['row_limit'],
3737
['sort_by_metric'],
38+
[
39+
{
40+
name: 'sort_by_series',
41+
config: {
42+
type: 'CheckboxControl',
43+
label: t('Sort by series'),
44+
default: false,
45+
description: t(
46+
'Include a secondary sort by series name (ascending). '
47+
+ 'This produces consistent ordering when multiple series have the '
48+
+ 'same metric value, but may reduce query performance on some databases.',
49+
),
50+
},
51+
},
52+
],
3853
],
3954
},
4055
{

superset-frontend/plugins/plugin-chart-word-cloud/test/buildQuery.test.ts

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,37 +36,55 @@ describe('plugin-chart-word-cloud', () => {
3636
expect(query.columns).toEqual(['foo']);
3737
});
3838

39-
test('should order by series ASC when sort_by_metric is false', () => {
39+
test('should not include orderby when neither sort option is enabled', () => {
4040
const queryContext = buildQuery({
4141
...basicFormData,
4242
metric: 'count',
4343
sort_by_metric: false,
44+
sort_by_series: false,
4445
row_limit: 100,
4546
});
4647
const [query] = queryContext.queries;
47-
expect(query.orderby).toEqual([['foo', true]]);
48+
expect(query.orderby).toBeUndefined();
4849
});
4950

5051
test('should order by metric DESC only when sort_by_metric is true', () => {
5152
const queryContext = buildQuery({
5253
...basicFormData,
5354
metric: 'count',
5455
sort_by_metric: true,
56+
sort_by_series: false,
5557
row_limit: 100,
5658
});
5759
const [query] = queryContext.queries;
5860
expect(query.orderby).toEqual([['count', false]]);
5961
});
6062

61-
test('should not include secondary series sort when sort_by_metric is true', () => {
63+
test('should order by series ASC only when sort_by_series is true', () => {
64+
const queryContext = buildQuery({
65+
...basicFormData,
66+
metric: 'count',
67+
sort_by_metric: false,
68+
sort_by_series: true,
69+
row_limit: 100,
70+
});
71+
const [query] = queryContext.queries;
72+
expect(query.orderby).toEqual([['foo', true]]);
73+
});
74+
75+
test('should order by metric DESC then series ASC when both are true', () => {
6276
const queryContext = buildQuery({
6377
...basicFormData,
6478
metric: 'count',
6579
sort_by_metric: true,
80+
sort_by_series: true,
6681
row_limit: 100,
6782
});
6883
const [query] = queryContext.queries;
69-
expect(query.orderby).toHaveLength(1);
84+
expect(query.orderby).toEqual([
85+
['count', false],
86+
['foo', true],
87+
]);
7088
});
7189
});
7290
});
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
"""add sort_by_series to word_cloud charts
18+
19+
Revision ID: fd0c8583b46d
20+
Revises: ce6bd21901ab
21+
Create Date: 2026-04-13 19:28:19.021839
22+
23+
"""
24+
25+
from alembic import op
26+
from sqlalchemy import Column, Integer, String, Text
27+
from sqlalchemy.ext.declarative import declarative_base
28+
29+
from superset import db
30+
from superset.migrations.shared.utils import paginated_update
31+
from superset.utils import json
32+
33+
# revision identifiers, used by Alembic.
34+
revision = "fd0c8583b46d"
35+
down_revision = "ce6bd21901ab"
36+
37+
Base = declarative_base()
38+
39+
40+
class Slice(Base):
41+
__tablename__ = "slices"
42+
id = Column(Integer, primary_key=True)
43+
viz_type = Column(String(250))
44+
params = Column(Text)
45+
46+
47+
def upgrade_params(params: dict) -> dict:
48+
if "sort_by_series" not in params:
49+
params["sort_by_series"] = True
50+
return params
51+
52+
53+
def downgrade_params(params: dict) -> dict:
54+
params.pop("sort_by_series", None)
55+
return params
56+
57+
58+
def upgrade():
59+
bind = op.get_bind()
60+
session = db.Session(bind=bind)
61+
62+
for slc in paginated_update(
63+
session.query(Slice).filter(Slice.viz_type == "word_cloud")
64+
):
65+
params = json.loads(slc.params or "{}")
66+
slc.params = json.dumps(upgrade_params(params))
67+
session.close()
68+
69+
70+
def downgrade():
71+
bind = op.get_bind()
72+
session = db.Session(bind=bind)
73+
74+
for slc in paginated_update(
75+
session.query(Slice).filter(Slice.viz_type == "word_cloud")
76+
):
77+
params = json.loads(slc.params or "{}")
78+
slc.params = json.dumps(downgrade_params(params))
79+
session.close()

0 commit comments

Comments
 (0)