Skip to content

fix(query): order by adhoc metrics should trigger group by #13434

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ktmud merged 19 commits into from
Mar 17, 2021
Merged

fix(query): order by adhoc metrics should trigger group by #13434

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
edc9e7f
fix(query): properly select adhoc metrics in orderby
ktmud Mar 3, 2021
960b8fc
Fix many cases
ktmud Mar 4, 2021
001d250
Throw error when sql is empty
ktmud Mar 4, 2021
01917ef
Remove orderby only columns from output
ktmud Mar 5, 2021
ffc4166
Small refactor and more test cases
ktmud Mar 5, 2021
97a0f05
No need to add sort by only metrics
ktmud Mar 8, 2021
1abccb1
Allow `metrics` to be None
ktmud Mar 8, 2021
f2512c8
Fix tests
ktmud Mar 10, 2021
ca42813
Fix some typing error
ktmud Mar 11, 2021
2eb4aab
Let metrics be None
ktmud Mar 12, 2021
2e0614a
Always use alias in orderby for metrics
ktmud Mar 14, 2021
b141a7f
Bump table chart version and migrate histogram to typescript
ktmud Mar 14, 2021
f17a006
Fix Histogram without groupby
ktmud Mar 14, 2021
263f3fa
Fix Presto birth names test
ktmud Mar 14, 2021
d189a80
Remove unused props
ktmud Mar 14, 2021
937cbe7
Clean up
ktmud Mar 14, 2021
2128a6b
Raw records mode should not aggregate
ktmud Mar 15, 2021
2cb2a80
Address PR comments
ktmud Mar 16, 2021
7eea891
Orderby col is not optional
ktmud Mar 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .../explore/visualizations/histogram.test.js → .../explore/visualizations/histogram.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
import { QueryFormData } from '@superset-ui/core';

describe('Visualization > Histogram', () => {
const HISTOGRAM_FORM_DATA = {
const HISTOGRAM_FORM_DATA: QueryFormData = {
datasource: '3__table',
viz_type: 'histogram',
slice_id: 60,
Expand All @@ -36,7 +38,7 @@ describe('Visualization > Histogram', () => {
normalized: false,
};

function verify(formData) {
function verify(formData: QueryFormData) {
cy.visitChartByParams(JSON.stringify(formData));
cy.verifySliceSuccess({ waitAlias: '@getJson', chartSelector: 'svg' });
}
Expand Down Expand Up @@ -73,12 +75,10 @@ describe('Visualization > Histogram', () => {
adhoc_filters: [
{
expressionType: 'SIMPLE',
clause: 'WHERE',
subject: 'state',
operator: '==',
comparator: 'CA',
clause: 'WHERE',
sqlExpression: null,
filterOptionName: 'filter_tqx1en70hh_7nksse7nqic',
},
],
});
Expand Down
22 changes: 19 additions & 3 deletions superset-frontend/cypress-base/cypress/integration/explore/visualizations/table.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,21 @@ describe('Visualization > Table', () => {
cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'table' });
});

it('Test table with groupby + order by + no metric', () => {
cy.visitChartByParams({
...VIZ_DEFAULTS,
metrics: [],
groupby: ['name'],
timeseries_limit_metric: NUM_METRIC,
order_desc: true,
});
// should contain only the group by column
cy.get('.chart-container th').its('length').should('eq', 1);
// should order correctly
cy.get('.chart-container td:eq(0)').contains('Michael');
cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'table' });
});

it('Test table with groupby and limit', () => {
const limit = 10;
const formData = {
Expand All @@ -172,9 +187,9 @@ describe('Visualization > Table', () => {
...VIZ_DEFAULTS,
// should still work when query_mode is not-set/invalid
query_mode: undefined,
all_columns: ['name'],
all_columns: ['state'],
metrics: [],
row_limit: 10,
row_limit: 100,
});

// should display in raw records mode
Expand All @@ -183,8 +198,9 @@ describe('Visualization > Table', () => {
cy.get('div[data-test="groupby"]').should('not.exist');

cy.verifySliceSuccess({ waitAlias: '@chartData', chartSelector: 'table' });
cy.get('[data-test="row-count-label"]').contains('100 rows');

// should allow switch to aggregate mode
// should allow switch back to aggregate mode
cy.get('div[data-test="query_mode"] .btn').contains('Aggregate').click();
cy.get('div[data-test="query_mode"] .btn.active').contains('Aggregate');
cy.get('div[data-test="all_columns"]').should('not.exist');
Expand Down
Loading