You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2024/11/19 20:24:49 UTC
(superset) branch airbnb created (now 264eebb3f7)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
at 264eebb3f7 fix(Dashboard): Native & Cross-Filters Scoping Performance (#30881)
This branch includes the following new commits:
new bde69e4813 fix: Fixing incomplete string escaping. (#29772)
new 5733e533ae fix: Graph chart colors (#30851)
new b1c7b819ae fix: Exception handling for SQL Lab views (#30897)
new 938b1fd3c1 fix(TimezoneSelector): Failing unit tests due to timezone change (#30828)
new 096fdea542 fix(explore): column data type tooltip format (#30588)
new dd805269a6 fix: blocks UI elements on right side (#30886)
new 264eebb3f7 fix(Dashboard): Native & Cross-Filters Scoping Performance (#30881)
The 7 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/07: fix: Fixing incomplete string escaping. (#29772)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit bde69e4813c2cff758cce38f59d8469bfb5edb4a
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Tue Jul 30 16:36:26 2024 -0600
fix: Fixing incomplete string escaping. (#29772)
(cherry picked from commit 2bce20f790a31f3342bda61d830365e91e518fb3)
---
superset-frontend/src/explore/exploreUtils/index.js | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/superset-frontend/src/explore/exploreUtils/index.js b/superset-frontend/src/explore/exploreUtils/index.js
index 4fc8147a18..f468854955 100644
--- a/superset-frontend/src/explore/exploreUtils/index.js
+++ b/superset-frontend/src/explore/exploreUtils/index.js
@@ -316,7 +316,7 @@ export const getSimpleSQLExpression = (subject, operator, comparator) => {
.map(val => optionLabel(val))
.map(
val =>
- `${quote}${isString ? String(val).replace("'", "''") : val}${quote}`,
+ `${quote}${isString ? String(val).replace(/'/g, "''") : val}${quote}`,
);
if (comparatorArray.length > 0) {
expression += ` ${prefix}${formattedComparators.join(', ')}${suffix}`;
(superset) 04/07: fix(TimezoneSelector): Failing unit tests due to timezone change (#30828)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 938b1fd3c191fea50307d4c53ef3a0a19593372d
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Mon Nov 4 16:08:44 2024 +0200
fix(TimezoneSelector): Failing unit tests due to timezone change (#30828)
(cherry picked from commit 5820d31b5c15d706fc26e6d6fb33b4ff1b16f4c3)
---
.../TimezoneSelector/TimezoneSelector.DaylightSavingTime.test.tsx | 5 ++++-
.../src/components/TimezoneSelector/TimezoneSelector.test.tsx | 8 +++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.DaylightSavingTime.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.DaylightSavingTime.test.tsx
index 81dbc6fdcd..395d48ad99 100644
--- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.DaylightSavingTime.test.tsx
+++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.DaylightSavingTime.test.tsx
@@ -30,10 +30,13 @@ const loadComponent = (mockCurrentTime?: string) => {
return new Promise<FC<TimezoneSelectorProps>>(resolve => {
const { default: TimezoneSelector } = module.require('./index');
resolve(TimezoneSelector);
- jest.useRealTimers();
});
};
+afterEach(() => {
+ jest.useRealTimers();
+});
+
test('render timezones in correct order for daylight saving time', async () => {
const TimezoneSelector = await loadComponent('2022-07-01');
const onTimezoneChange = jest.fn();
diff --git a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx
index 173118c7e8..7866cd2261 100644
--- a/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx
+++ b/superset-frontend/src/components/TimezoneSelector/TimezoneSelector.test.tsx
@@ -30,7 +30,6 @@ const loadComponent = (mockCurrentTime?: string) => {
return new Promise<FC<TimezoneSelectorProps>>(resolve => {
const { default: TimezoneSelector } = module.require('./index');
resolve(TimezoneSelector);
- jest.useRealTimers();
});
};
@@ -44,6 +43,10 @@ const openSelectMenu = () => {
jest.spyOn(moment.tz, 'guess').mockReturnValue('America/New_York');
+afterEach(() => {
+ jest.useRealTimers();
+});
+
test('use the timezone from `moment` if no timezone provided', async () => {
const TimezoneSelector = await loadComponent('2022-01-01');
const onTimezoneChange = jest.fn();
@@ -89,7 +92,6 @@ test('render timezones in correct oder for standard time', async () => {
expect(options[0]).toHaveTextContent('GMT -05:00 (Eastern Standard Time)');
expect(options[1]).toHaveTextContent('GMT -11:00 (Pacific/Pago_Pago)');
expect(options[2]).toHaveTextContent('GMT -10:00 (Hawaii Standard Time)');
- expect(options[3]).toHaveTextContent('GMT -10:00 (America/Adak)');
});
test('can select a timezone values and returns canonical timezone name', async () => {
@@ -106,7 +108,7 @@ test('can select a timezone values and returns canonical timezone name', async (
const searchInput = screen.getByRole('combobox');
// search for mountain time
- await userEvent.type(searchInput, 'mou', { delay: 10 });
+ await userEvent.type(searchInput, 'mou');
const findTitle = 'GMT -07:00 (Mountain Standard Time)';
const selectOption = await screen.findByTitle(findTitle);
userEvent.click(selectOption);
(superset) 07/07: fix(Dashboard): Native & Cross-Filters Scoping Performance (#30881)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 264eebb3f73ae7fc1f996886fe7be14471fbaf0a
Author: Geido <60...@users.noreply.github.com>
AuthorDate: Fri Nov 8 19:57:13 2024 +0200
fix(Dashboard): Native & Cross-Filters Scoping Performance (#30881)
(cherry picked from commit f4c36a6d055205a3e9ccd9131c317fa79dc30b83)
---
.../src/dashboard/components/Dashboard.jsx | 9 +-
.../src/dashboard/components/Dashboard.test.jsx | 22 +---
.../src/dashboard/util/getRelatedCharts.test.ts | 24 ++--
.../src/dashboard/util/getRelatedCharts.ts | 138 ++++++++-------------
.../util/useFilterFocusHighlightStyles.test.tsx | 24 +---
.../util/useFilterFocusHighlightStyles.ts | 15 +--
6 files changed, 86 insertions(+), 146 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/Dashboard.jsx b/superset-frontend/src/dashboard/components/Dashboard.jsx
index 1a852edda8..ddbc2441a5 100644
--- a/superset-frontend/src/dashboard/components/Dashboard.jsx
+++ b/superset-frontend/src/dashboard/components/Dashboard.jsx
@@ -224,6 +224,7 @@ class Dashboard extends PureComponent {
ownDataCharts,
this.appliedOwnDataCharts,
);
+
[...allKeys].forEach(filterKey => {
if (
!currFilterKeys.includes(filterKey) &&
@@ -231,12 +232,12 @@ class Dashboard extends PureComponent {
) {
// filterKey is removed?
affectedChartIds.push(
- ...getRelatedCharts(appliedFilters, activeFilters, slices)[filterKey],
+ ...getRelatedCharts(filterKey, appliedFilters[filterKey], slices),
);
} else if (!appliedFilterKeys.includes(filterKey)) {
// filterKey is newly added?
affectedChartIds.push(
- ...getRelatedCharts(activeFilters, appliedFilters, slices)[filterKey],
+ ...getRelatedCharts(filterKey, activeFilters[filterKey], slices),
);
} else {
// if filterKey changes value,
@@ -251,9 +252,7 @@ class Dashboard extends PureComponent {
)
) {
affectedChartIds.push(
- ...getRelatedCharts(activeFilters, appliedFilters, slices)[
- filterKey
- ],
+ ...getRelatedCharts(filterKey, activeFilters[filterKey], slices),
);
}
diff --git a/superset-frontend/src/dashboard/components/Dashboard.test.jsx b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
index cd76787a21..e3421ee040 100644
--- a/superset-frontend/src/dashboard/components/Dashboard.test.jsx
+++ b/superset-frontend/src/dashboard/components/Dashboard.test.jsx
@@ -157,9 +157,7 @@ describe('Dashboard', () => {
});
it('should call refresh when native filters changed', () => {
- getRelatedCharts.mockReturnValue({
- [NATIVE_FILTER_ID]: [230],
- });
+ getRelatedCharts.mockReturnValue([230]);
wrapper.setProps({
activeFilters: {
...OVERRIDE_FILTERS,
@@ -191,13 +189,7 @@ describe('Dashboard', () => {
});
it('should call refresh if a filter is added', () => {
- getRelatedCharts.mockReturnValue({
- '1_region': [1],
- '2_country_name': [1, 2],
- '3_region': [1],
- '3_country_name': [],
- gender: [1],
- });
+ getRelatedCharts.mockReturnValue([1]);
const newFilter = {
gender: { values: ['boy', 'girl'], scope: [1] },
};
@@ -209,12 +201,7 @@ describe('Dashboard', () => {
});
it('should call refresh if a filter is removed', () => {
- getRelatedCharts.mockReturnValue({
- '1_region': [1],
- '2_country_name': [1, 2],
- '3_region': [1],
- '3_country_name': [],
- });
+ getRelatedCharts.mockReturnValue([]);
wrapper.setProps({
activeFilters: {},
});
@@ -223,6 +210,7 @@ describe('Dashboard', () => {
});
it('should call refresh if a filter is changed', () => {
+ getRelatedCharts.mockReturnValue([1]);
const newFilters = {
...OVERRIDE_FILTERS,
'1_region': { values: ['Canada'], scope: [1] },
@@ -236,6 +224,7 @@ describe('Dashboard', () => {
});
it('should call refresh with multiple chart ids', () => {
+ getRelatedCharts.mockReturnValue([1, 2]);
const newFilters = {
...OVERRIDE_FILTERS,
'2_country_name': { values: ['New Country'], scope: [1, 2] },
@@ -262,6 +251,7 @@ describe('Dashboard', () => {
});
it('should call refresh with empty [] if a filter is changed but scope is not applicable', () => {
+ getRelatedCharts.mockReturnValue([]);
const newFilters = {
...OVERRIDE_FILTERS,
'3_country_name': { values: ['CHINA'], scope: [] },
diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
index 874b912d90..a1ba74a20c 100644
--- a/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
+++ b/superset-frontend/src/dashboard/util/getRelatedCharts.test.ts
@@ -49,10 +49,8 @@ test('Return all chart ids in global scope with native filters', () => {
} as unknown as Filter,
};
- const result = getRelatedCharts(filters, null, slices);
- expect(result).toEqual({
- filterKey1: [1, 2, 3],
- });
+ const result = getRelatedCharts('filterKey1', filters.filterKey1, slices);
+ expect(result).toEqual([1, 2, 3]);
});
test('Return only chart ids in specific scope with native filters', () => {
@@ -74,10 +72,8 @@ test('Return only chart ids in specific scope with native filters', () => {
} as unknown as Filter,
};
- const result = getRelatedCharts(filters, null, slices);
- expect(result).toEqual({
- filterKey1: [1, 3],
- });
+ const result = getRelatedCharts('filterKey1', filters.filterKey1, slices);
+ expect(result).toEqual([1, 3]);
});
test('Return all chart ids with cross filter in global scope', () => {
@@ -90,10 +86,8 @@ test('Return all chart ids with cross filter in global scope', () => {
} as AppliedCrossFilterType,
};
- const result = getRelatedCharts(filters, null, slices);
- expect(result).toEqual({
- '3': [1, 2],
- });
+ const result = getRelatedCharts('3', filters['3'], slices);
+ expect(result).toEqual([1, 2]);
});
test('Return only chart ids in specific scope with cross filter', () => {
@@ -108,8 +102,6 @@ test('Return only chart ids in specific scope with cross filter', () => {
} as AppliedCrossFilterType,
};
- const result = getRelatedCharts(filters, null, slices);
- expect(result).toEqual({
- '1': [2],
- });
+ const result = getRelatedCharts('1', filters['1'], slices);
+ expect(result).toEqual([2]);
});
diff --git a/superset-frontend/src/dashboard/util/getRelatedCharts.ts b/superset-frontend/src/dashboard/util/getRelatedCharts.ts
index 6c63e0d4b1..b3d55ead2a 100644
--- a/superset-frontend/src/dashboard/util/getRelatedCharts.ts
+++ b/superset-frontend/src/dashboard/util/getRelatedCharts.ts
@@ -32,25 +32,25 @@ function isGlobalScope(scope: number[], slices: Record<string, Slice>) {
}
function getRelatedChartsForSelectFilter(
- nativeFilter: AppliedNativeFilterType | Filter,
slices: Record<string, Slice>,
chartsInScope: number[],
-) {
- return Object.values(slices)
- .filter(slice => {
- const { slice_id } = slice;
- // all have been selected, always apply
- if (isGlobalScope(chartsInScope, slices)) return true;
- // hand-picked in scope, always apply
- if (chartsInScope.includes(slice_id)) return true;
+): number[] {
+ // all have been selected, always apply
+ if (isGlobalScope(chartsInScope, slices)) {
+ return Object.keys(slices).map(Number);
+ }
+
+ const chartsInScopeSet = new Set(chartsInScope);
- return false;
- })
- .map(slice => slice.slice_id);
+ return Object.values(slices).reduce((result: number[], slice) => {
+ if (chartsInScopeSet.has(slice.slice_id)) {
+ result.push(slice.slice_id);
+ }
+ return result;
+ }, []);
}
function getRelatedChartsForCrossFilter(
filterKey: string,
- crossFilter: AppliedCrossFilterType,
slices: Record<string, Slice>,
scope: number[],
): number[] {
@@ -58,86 +58,56 @@ function getRelatedChartsForCrossFilter(
if (!sourceSlice) return [];
- return Object.values(slices)
- .filter(slice => {
- // cross-filter emitter
- if (slice.slice_id === Number(filterKey)) return false;
- // hand-picked in the scope, always apply
- const fullScope = [
- ...scope.filter(s => String(s) !== filterKey),
- Number(filterKey),
- ];
- if (isGlobalScope(fullScope, slices)) return true;
- // hand-picked in the scope, always apply
- if (scope.includes(slice.slice_id)) return true;
+ const fullScope = [
+ ...scope.filter(s => String(s) !== filterKey),
+ Number(filterKey),
+ ];
+ const scopeSet = new Set(scope);
- return false;
- })
- .map(slice => slice.slice_id);
+ return Object.values(slices).reduce((result: number[], slice) => {
+ if (slice.slice_id === Number(filterKey)) {
+ return result;
+ }
+ // Check if it's in the global scope
+ if (isGlobalScope(fullScope, slices)) {
+ result.push(slice.slice_id);
+ return result;
+ }
+ // Check if it's hand-picked in scope
+ if (scopeSet.has(slice.slice_id)) {
+ result.push(slice.slice_id);
+ }
+ return result;
+ }, []);
}
export function getRelatedCharts(
- filters: Record<
- string,
- AppliedNativeFilterType | AppliedCrossFilterType | Filter
- >,
- checkFilters: Record<
- string,
- AppliedNativeFilterType | AppliedCrossFilterType | Filter
- > | null,
+ filterKey: string,
+ filter: AppliedNativeFilterType | AppliedCrossFilterType | Filter,
slices: Record<string, Slice>,
) {
- const related = Object.entries(filters).reduce((acc, [filterKey, filter]) => {
- const isCrossFilter =
- Object.keys(slices).includes(filterKey) &&
- isAppliedCrossFilterType(filter);
+ let related: number[] = [];
+ const isCrossFilter =
+ Object.keys(slices).includes(filterKey) && isAppliedCrossFilterType(filter);
- const chartsInScope = Array.isArray(filter.scope)
- ? filter.scope
- : (filter as Filter).chartsInScope ?? [];
+ const chartsInScope = Array.isArray(filter.scope)
+ ? filter.scope
+ : ((filter as Filter).chartsInScope ?? []);
- if (isCrossFilter) {
- const checkFilter = checkFilters?.[filterKey] as AppliedCrossFilterType;
- const crossFilter = filter as AppliedCrossFilterType;
- const wasRemoved = !!(
- ((filter.values && filter.values.filters === undefined) ||
- filter.values?.filters?.length === 0) &&
- checkFilter?.values?.filters?.length
- );
- const actualCrossFilter = wasRemoved ? checkFilter : crossFilter;
+ if (isCrossFilter) {
+ related = getRelatedChartsForCrossFilter(filterKey, slices, chartsInScope);
+ }
- return {
- ...acc,
- [filterKey]: getRelatedChartsForCrossFilter(
- filterKey,
- actualCrossFilter,
- slices,
- chartsInScope,
- ),
- };
- }
-
- const nativeFilter = filter as AppliedNativeFilterType | Filter;
- // on highlight, a standard native filter is passed
- // on apply, an applied native filter is passed
- if (
- isAppliedNativeFilterType(nativeFilter) ||
- isNativeFilter(nativeFilter)
- ) {
- return {
- ...acc,
- [filterKey]: getRelatedChartsForSelectFilter(
- nativeFilter,
- slices,
- chartsInScope,
- ),
- };
- }
- return {
- ...acc,
- [filterKey]: chartsInScope,
- };
- }, {});
+ const nativeFilter = filter as AppliedNativeFilterType | Filter;
+ // on highlight, a standard native filter is passed
+ // on apply, an applied native filter is passed
+ if (
+ !isCrossFilter ||
+ isAppliedNativeFilterType(nativeFilter) ||
+ isNativeFilter(nativeFilter)
+ ) {
+ related = getRelatedChartsForSelectFilter(slices, chartsInScope);
+ }
return related;
}
diff --git a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
index ca19c63d51..80e6038c07 100644
--- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
+++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.test.tsx
@@ -61,9 +61,7 @@ describe('useFilterFocusHighlightStyles', () => {
});
it('should return unfocused styles if chart is not in scope of focused native filter', async () => {
- mockGetRelatedCharts.mockReturnValue({
- 'test-filter': [],
- });
+ mockGetRelatedCharts.mockReturnValue([]);
const store = createMockStore({
nativeFilters: {
focusedFilterId: 'test-filter',
@@ -83,9 +81,7 @@ describe('useFilterFocusHighlightStyles', () => {
});
it('should return unfocused styles if chart is not in scope of hovered native filter', async () => {
- mockGetRelatedCharts.mockReturnValue({
- 'test-filter': [],
- });
+ mockGetRelatedCharts.mockReturnValue([]);
const store = createMockStore({
nativeFilters: {
hoveredFilterId: 'test-filter',
@@ -106,9 +102,7 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return focused styles if chart is in scope of focused native filter', async () => {
const chartId = 18;
- mockGetRelatedCharts.mockReturnValue({
- testFilter: [chartId],
- });
+ mockGetRelatedCharts.mockReturnValue([chartId]);
const store = createMockStore({
nativeFilters: {
focusedFilterId: 'testFilter',
@@ -129,9 +123,7 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return focused styles if chart is in scope of hovered native filter', async () => {
const chartId = 18;
- mockGetRelatedCharts.mockReturnValue({
- testFilter: [chartId],
- });
+ mockGetRelatedCharts.mockReturnValue([chartId]);
const store = createMockStore({
nativeFilters: {
hoveredFilterId: 'testFilter',
@@ -152,9 +144,7 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return unfocused styles if focusedFilterField is targeting a different chart', async () => {
const chartId = 18;
- mockGetRelatedCharts.mockReturnValue({
- testFilter: [],
- });
+ mockGetRelatedCharts.mockReturnValue([]);
const store = createMockStore({
dashboardState: {
focusedFilterField: {
@@ -178,9 +168,7 @@ describe('useFilterFocusHighlightStyles', () => {
it('should return focused styles if focusedFilterField chart equals our own', async () => {
const chartId = 18;
- mockGetRelatedCharts.mockReturnValue({
- testFilter: [chartId],
- });
+ mockGetRelatedCharts.mockReturnValue([chartId]);
const store = createMockStore({
dashboardState: {
focusedFilterField: {
diff --git a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
index b28d0a3771..aa636cb1ee 100644
--- a/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
+++ b/superset-frontend/src/dashboard/util/useFilterFocusHighlightStyles.ts
@@ -54,18 +54,19 @@ const useFilterFocusHighlightStyles = (chartId: number) => {
const slices =
useSelector((state: RootState) => state.sliceEntities.slices) || {};
- const relatedCharts = getRelatedCharts(
- nativeFilters.filters as Record<string, Filter>,
- null,
- slices,
- );
-
const highlightedFilterId =
nativeFilters?.focusedFilterId || nativeFilters?.hoveredFilterId;
+
if (!(focusedFilterScope || highlightedFilterId)) {
return {};
}
+ const relatedCharts = getRelatedCharts(
+ highlightedFilterId as string,
+ nativeFilters.filters[highlightedFilterId as string] as Filter,
+ slices,
+ );
+
// we use local styles here instead of a conditionally-applied class,
// because adding any conditional class to this container
// causes performance issues in Chrome.
@@ -80,7 +81,7 @@ const useFilterFocusHighlightStyles = (chartId: number) => {
};
if (highlightedFilterId) {
- if (relatedCharts[highlightedFilterId]?.includes(chartId)) {
+ if (relatedCharts.includes(chartId)) {
return focusedChartStyles;
}
} else if (
(superset) 03/07: fix: Exception handling for SQL Lab views (#30897)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit b1c7b819ae5fdad59aa50809d387a2d4c4778181
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Tue Nov 12 13:54:58 2024 -0300
fix: Exception handling for SQL Lab views (#30897)
(cherry picked from commit c2885a166e961af62c0e15cd541133fc44b9669d)
---
superset/views/sql_lab/views.py | 285 +++++++++++++++++++---------------
tests/integration_tests/core_tests.py | 1 -
2 files changed, 157 insertions(+), 129 deletions(-)
diff --git a/superset/views/sql_lab/views.py b/superset/views/sql_lab/views.py
index 29d0fdba75..29cf640d8c 100644
--- a/superset/views/sql_lab/views.py
+++ b/superset/views/sql_lab/views.py
@@ -29,11 +29,12 @@ from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod
from superset.models.sql_lab import Query, SavedQuery, TableSchema, TabState
from superset.superset_typing import FlaskResponse
from superset.utils import json
-from superset.utils.core import get_user_id
+from superset.utils.core import error_msg_from_exception, get_user_id
from superset.views.base import (
BaseSupersetView,
DeleteMixin,
DeprecateModelViewMixin,
+ json_error_response,
json_success,
SupersetModelView,
)
@@ -84,48 +85,56 @@ class TabStateView(BaseSupersetView):
@has_access_api
@expose("/", methods=("POST",))
def post(self) -> FlaskResponse:
- query_editor = json.loads(request.form["queryEditor"])
- tab_state = TabState(
- user_id=get_user_id(),
- # This is for backward compatibility
- label=query_editor.get("name")
- or query_editor.get("title", __("Untitled Query")),
- active=True,
- database_id=query_editor["dbId"],
- catalog=query_editor.get("catalog"),
- schema=query_editor.get("schema"),
- sql=query_editor.get("sql", "SELECT ..."),
- query_limit=query_editor.get("queryLimit"),
- hide_left_bar=query_editor.get("hideLeftBar"),
- saved_query_id=query_editor.get("remoteId"),
- template_params=query_editor.get("templateParams"),
- )
- (
- db.session.query(TabState)
- .filter_by(user_id=get_user_id())
- .update({"active": False})
- )
- db.session.add(tab_state)
- db.session.commit()
- return json_success(json.dumps({"id": tab_state.id}))
+ try:
+ query_editor = json.loads(request.form["queryEditor"])
+ tab_state = TabState(
+ user_id=get_user_id(),
+ # This is for backward compatibility
+ label=query_editor.get("name")
+ or query_editor.get("title", __("Untitled Query")),
+ active=True,
+ database_id=query_editor["dbId"],
+ catalog=query_editor.get("catalog"),
+ schema=query_editor.get("schema"),
+ sql=query_editor.get("sql", "SELECT ..."),
+ query_limit=query_editor.get("queryLimit"),
+ hide_left_bar=query_editor.get("hideLeftBar"),
+ saved_query_id=query_editor.get("remoteId"),
+ template_params=query_editor.get("templateParams"),
+ )
+ (
+ db.session.query(TabState)
+ .filter_by(user_id=get_user_id())
+ .update({"active": False})
+ )
+ db.session.add(tab_state)
+ db.session.commit()
+ return json_success(json.dumps({"id": tab_state.id}))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("/<int:tab_state_id>", methods=("DELETE",))
def delete(self, tab_state_id: int) -> FlaskResponse:
- owner_id = _get_owner_id(tab_state_id)
- if owner_id is None:
- return Response(status=404)
- if owner_id != get_user_id():
- return Response(status=403)
-
- db.session.query(TabState).filter(TabState.id == tab_state_id).delete(
- synchronize_session=False
- )
- db.session.query(TableSchema).filter(
- TableSchema.tab_state_id == tab_state_id
- ).delete(synchronize_session=False)
- db.session.commit()
- return json_success(json.dumps("OK"))
+ try:
+ owner_id = _get_owner_id(tab_state_id)
+ if owner_id is None:
+ return Response(status=404)
+ if owner_id != get_user_id():
+ return Response(status=403)
+
+ db.session.query(TabState).filter(TabState.id == tab_state_id).delete(
+ synchronize_session=False
+ )
+ db.session.query(TableSchema).filter(
+ TableSchema.tab_state_id == tab_state_id
+ ).delete(synchronize_session=False)
+ db.session.commit()
+ return json_success(json.dumps("OK"))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("/<int:tab_state_id>", methods=("GET",))
@@ -146,19 +155,23 @@ class TabStateView(BaseSupersetView):
@has_access_api
@expose("<int:tab_state_id>/activate", methods=("POST",))
def activate(self, tab_state_id: int) -> FlaskResponse:
- owner_id = _get_owner_id(tab_state_id)
- if owner_id is None:
- return Response(status=404)
- if owner_id != get_user_id():
- return Response(status=403)
-
- (
- db.session.query(TabState)
- .filter_by(user_id=get_user_id())
- .update({"active": TabState.id == tab_state_id})
- )
- db.session.commit()
- return json_success(json.dumps(tab_state_id))
+ try:
+ owner_id = _get_owner_id(tab_state_id)
+ if owner_id is None:
+ return Response(status=404)
+ if owner_id != get_user_id():
+ return Response(status=403)
+
+ (
+ db.session.query(TabState)
+ .filter_by(user_id=get_user_id())
+ .update({"active": TabState.id == tab_state_id})
+ )
+ db.session.commit()
+ return json_success(json.dumps(tab_state_id))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("<int:tab_state_id>", methods=("PUT",))
@@ -169,102 +182,118 @@ class TabStateView(BaseSupersetView):
if owner_id != get_user_id():
return Response(status=403)
- fields = {k: json.loads(v) for k, v in request.form.to_dict().items()}
- if client_id := fields.get("latest_query_id"):
- query = db.session.query(Query).filter_by(client_id=client_id).one_or_none()
- if not query:
- return self.json_response({"error": "Bad request"}, status=400)
- db.session.query(TabState).filter_by(id=tab_state_id).update(fields)
- db.session.commit()
- return json_success(json.dumps(tab_state_id))
+ try:
+ fields = {k: json.loads(v) for k, v in request.form.to_dict().items()}
+ db.session.query(TabState).filter_by(id=tab_state_id).update(fields)
+ db.session.commit()
+ return json_success(json.dumps(tab_state_id))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("<int:tab_state_id>/migrate_query", methods=("POST",))
def migrate_query(self, tab_state_id: int) -> FlaskResponse:
- owner_id = _get_owner_id(tab_state_id)
- if owner_id is None:
- return Response(status=404)
- if owner_id != get_user_id():
- return Response(status=403)
-
- client_id = json.loads(request.form["queryId"])
- db.session.query(Query).filter_by(client_id=client_id).update(
- {"sql_editor_id": tab_state_id}
- )
- db.session.commit()
- return json_success(json.dumps(tab_state_id))
+ try:
+ owner_id = _get_owner_id(tab_state_id)
+ if owner_id is None:
+ return Response(status=404)
+ if owner_id != get_user_id():
+ return Response(status=403)
+
+ client_id = json.loads(request.form["queryId"])
+ db.session.query(Query).filter_by(client_id=client_id).update(
+ {"sql_editor_id": tab_state_id}
+ )
+ db.session.commit()
+ return json_success(json.dumps(tab_state_id))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("<int:tab_state_id>/query/<client_id>", methods=("DELETE",))
def delete_query(self, tab_state_id: int, client_id: str) -> FlaskResponse:
- # Before deleting the query, ensure it's not tied to any
- # active tab as the last query. If so, replace the query
- # with the latest one created in that tab
- tab_state_query = db.session.query(TabState).filter_by(
- id=tab_state_id, latest_query_id=client_id
- )
- if tab_state_query.count():
- query = (
- db.session.query(Query)
- .filter(
- and_(
- Query.client_id != client_id,
- Query.user_id == get_user_id(),
- Query.sql_editor_id == str(tab_state_id),
- ),
- )
- .order_by(Query.id.desc())
- .first()
- )
- tab_state_query.update(
- {"latest_query_id": query.client_id if query else None}
+ try:
+ # Before deleting the query, ensure it's not tied to any
+ # active tab as the last query. If so, replace the query
+ # with the latest one created in that tab
+ tab_state_query = db.session.query(TabState).filter_by(
+ id=tab_state_id, latest_query_id=client_id
)
+ if tab_state_query.count():
+ query = (
+ db.session.query(Query)
+ .filter(
+ and_(
+ Query.client_id != client_id,
+ Query.user_id == get_user_id(),
+ Query.sql_editor_id == str(tab_state_id),
+ ),
+ )
+ .order_by(Query.id.desc())
+ .first()
+ )
+ tab_state_query.update(
+ {"latest_query_id": query.client_id if query else None}
+ )
- db.session.query(Query).filter_by(
- client_id=client_id,
- user_id=get_user_id(),
- sql_editor_id=str(tab_state_id),
- ).delete(synchronize_session=False)
- db.session.commit()
- return json_success(json.dumps("OK"))
+ db.session.query(Query).filter_by(
+ client_id=client_id,
+ user_id=get_user_id(),
+ sql_editor_id=str(tab_state_id),
+ ).delete(synchronize_session=False)
+ db.session.commit()
+ return json_success(json.dumps("OK"))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
class TableSchemaView(BaseSupersetView):
@has_access_api
@expose("/", methods=("POST",))
def post(self) -> FlaskResponse:
- table = json.loads(request.form["table"])
-
- # delete any existing table schema
- db.session.query(TableSchema).filter(
- TableSchema.tab_state_id == table["queryEditorId"],
- TableSchema.database_id == table["dbId"],
- TableSchema.catalog == table.get("catalog"),
- TableSchema.schema == table["schema"],
- TableSchema.table == table["name"],
- ).delete(synchronize_session=False)
-
- table_schema = TableSchema(
- tab_state_id=table["queryEditorId"],
- database_id=table["dbId"],
- catalog=table.get("catalog"),
- schema=table["schema"],
- table=table["name"],
- description=json.dumps(table),
- expanded=True,
- )
- db.session.add(table_schema)
- db.session.commit()
- return json_success(json.dumps({"id": table_schema.id}))
+ try:
+ table = json.loads(request.form["table"])
+
+ # delete any existing table schema
+ db.session.query(TableSchema).filter(
+ TableSchema.tab_state_id == table["queryEditorId"],
+ TableSchema.database_id == table["dbId"],
+ TableSchema.catalog == table.get("catalog"),
+ TableSchema.schema == table["schema"],
+ TableSchema.table == table["name"],
+ ).delete(synchronize_session=False)
+
+ table_schema = TableSchema(
+ tab_state_id=table["queryEditorId"],
+ database_id=table["dbId"],
+ catalog=table.get("catalog"),
+ schema=table["schema"],
+ table=table["name"],
+ description=json.dumps(table),
+ expanded=True,
+ )
+ db.session.add(table_schema)
+ db.session.commit()
+ return json_success(json.dumps({"id": table_schema.id}))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("/<int:table_schema_id>", methods=("DELETE",))
def delete(self, table_schema_id: int) -> FlaskResponse:
- db.session.query(TableSchema).filter(TableSchema.id == table_schema_id).delete(
- synchronize_session=False
- )
- db.session.commit()
- return json_success(json.dumps("OK"))
+ try:
+ db.session.query(TableSchema).filter(
+ TableSchema.id == table_schema_id
+ ).delete(synchronize_session=False)
+ db.session.commit()
+ return json_success(json.dumps("OK"))
+ except Exception as ex: # pylint: disable=broad-except
+ db.session.rollback()
+ return json_error_response(error_msg_from_exception(ex), 400)
@has_access_api
@expose("/<int:table_schema_id>/expanded", methods=("POST",))
diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py
index 44b7ef26e6..2cc8c8f5ca 100644
--- a/tests/integration_tests/core_tests.py
+++ b/tests/integration_tests/core_tests.py
@@ -1015,7 +1015,6 @@ class TestCore(SupersetTestCase):
data = {"sql": json.dumps("select 1"), "latest_query_id": json.dumps(client_id)}
response = self.client.put(f"/tabstateview/{tab_state_id}", data=data)
self.assertEqual(response.status_code, 400)
- self.assertEqual(response.json["error"], "Bad request")
# generate query
db.session.add(Query(client_id=client_id, database_id=1))
db.session.commit()
(superset) 06/07: fix: blocks UI elements on right side (#30886)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit dd805269a6064d8293e47ef6b8c35972a97ea94e
Author: Sukuna <90...@users.noreply.github.com>
AuthorDate: Fri Nov 15 22:13:57 2024 +0530
fix: blocks UI elements on right side (#30886)
Co-authored-by: Evan Rusackas <ev...@preset.io>
(cherry picked from commit df479940a6afdff36826042a1923ed804cbfe337)
---
.../components/gridComponents/ChartHolder.tsx | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx
index 2f4a77c39b..d31e240e4f 100644
--- a/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx
+++ b/superset-frontend/src/dashboard/components/gridComponents/ChartHolder.tsx
@@ -21,7 +21,7 @@ import { useState, useMemo, useCallback, useEffect } from 'react';
import { ResizeCallback, ResizeStartCallback } from 're-resizable';
import cx from 'classnames';
import { useSelector } from 'react-redux';
-import { css } from '@superset-ui/core';
+import { css, useTheme } from '@superset-ui/core';
import { LayoutItem, RootState } from 'src/dashboard/types';
import AnchorLink from 'src/dashboard/components/AnchorLink';
import Chart from 'src/dashboard/containers/Chart';
@@ -70,15 +70,6 @@ interface ChartHolderProps {
isInView: boolean;
}
-const fullSizeStyle = css`
- && {
- position: fixed;
- z-index: 3000;
- left: 0;
- top: 0;
- }
-`;
-
const ChartHolder: React.FC<ChartHolderProps> = ({
id,
parentId,
@@ -102,6 +93,16 @@ const ChartHolder: React.FC<ChartHolderProps> = ({
setFullSizeChartId,
isInView,
}) => {
+ const theme = useTheme();
+ const fullSizeStyle = css`
+ && {
+ position: fixed;
+ z-index: 3000;
+ left: 0;
+ top: 0;
+ padding: ${theme.gridUnit * 2}px;
+ }
+ `;
const { chartId } = component.meta;
const isFullSize = fullSizeChartId === chartId;
(superset) 05/07: fix(explore): column data type tooltip format (#30588)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 096fdea542780279809c18d2eae53809728c66d5
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Tue Oct 29 18:47:20 2024 -0700
fix(explore): column data type tooltip format (#30588)
(cherry picked from commit 73768f63134aacda43cba01ea55e6b2347b78733)
---
.../superset-ui-chart-controls/src/components/labelUtils.tsx | 10 ++--------
.../test/components/labelUtils.test.tsx | 2 +-
2 files changed, 3 insertions(+), 9 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
index 66b25416f8..03af5c13e8 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/components/labelUtils.tsx
@@ -55,8 +55,7 @@ const TooltipSection = ({
text: ReactNode;
}) => (
<TooltipSectionWrapper>
- <TooltipSectionLabel>{label}</TooltipSectionLabel>
- <span>{text}</span>
+ <TooltipSectionLabel>{label}</TooltipSectionLabel>: <span>{text}</span>
</TooltipSectionWrapper>
);
@@ -71,12 +70,7 @@ export const getColumnTypeTooltipNode = (column: ColumnMeta): ReactNode => {
return null;
}
- return (
- <TooltipSection
- label={t('Column datatype')}
- text={column.type.toLowerCase()}
- />
- );
+ return <TooltipSection label={t('Column type')} text={column.type} />;
};
export const getColumnTooltipNode = (
diff --git a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx
index d32081b898..9b5b760f79 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/test/components/labelUtils.test.tsx
@@ -90,7 +90,7 @@ test('should get column datatype rendered as tooltip when column has a type', ()
</>,
);
- expect(screen.getByText('Column datatype')).toBeVisible();
+ expect(screen.getByText('Column type')).toBeVisible();
expect(screen.getByText('text')).toBeVisible();
});
(superset) 02/07: fix: Graph chart colors (#30851)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch airbnb
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 5733e533ae1c0b91939d8ae82732dfe6f475ab18
Author: Michael S. Molina <70...@users.noreply.github.com>
AuthorDate: Mon Nov 11 19:11:17 2024 -0300
fix: Graph chart colors (#30851)
(cherry picked from commit 0e165c1a21a90098adfea8efa5006e4feb2adf11)
---
.../src/Graph/transformProps.ts | 34 +++++++++++++++++++---
.../test/Graph/transformProps.test.ts | 22 ++++++++++++--
2 files changed, 50 insertions(+), 6 deletions(-)
diff --git a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts
index 5d50a5af7b..100d21349e 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/src/Graph/transformProps.ts
@@ -198,6 +198,7 @@ export default function transformProps(
const refs: Refs = {};
const metricLabel = getMetricLabel(metric);
const colorFn = CategoricalColorNamespace.getScale(colorScheme as string);
+ const firstColor = colorFn.range()[0];
const nodes: { [name: string]: number } = {};
const categories: Set<string> = new Set();
const echartNodes: EChartGraphNode[] = [];
@@ -207,7 +208,12 @@ export default function transformProps(
* Get the node id of an existing node,
* or create a new node if it doesn't exist.
*/
- function getOrCreateNode(name: string, col: string, category?: string) {
+ function getOrCreateNode(
+ name: string,
+ col: string,
+ category?: string,
+ color?: string,
+ ) {
if (!(name in nodes)) {
nodes[name] = echartNodes.length;
echartNodes.push({
@@ -221,6 +227,7 @@ export default function transformProps(
...getDefaultTooltip(refs),
...DEFAULT_GRAPH_SERIES_OPTION.tooltip,
},
+ itemStyle: { color },
});
}
const node = echartNodes[nodes[name]];
@@ -248,8 +255,25 @@ export default function transformProps(
const targetCategoryName = targetCategory
? getCategoryName(targetCategory, link[targetCategory])
: undefined;
- const sourceNode = getOrCreateNode(sourceName, source, sourceCategoryName);
- const targetNode = getOrCreateNode(targetName, target, targetCategoryName);
+ const sourceNodeColor = sourceCategoryName
+ ? colorFn(sourceCategoryName)
+ : firstColor;
+ const targetNodeColor = targetCategoryName
+ ? colorFn(targetCategoryName)
+ : firstColor;
+
+ const sourceNode = getOrCreateNode(
+ sourceName,
+ source,
+ sourceCategoryName,
+ sourceNodeColor,
+ );
+ const targetNode = getOrCreateNode(
+ targetName,
+ target,
+ targetCategoryName,
+ targetNodeColor,
+ );
sourceNode.value += value;
targetNode.value += value;
@@ -258,7 +282,9 @@ export default function transformProps(
source: sourceNode.id,
target: targetNode.id,
value,
- lineStyle: {},
+ lineStyle: {
+ color: sourceNodeColor,
+ },
emphasis: {},
select: {},
});
diff --git a/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts b/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts
index 3c2e6e2384..7a55ec23c3 100644
--- a/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts
+++ b/superset-frontend/plugins/plugin-chart-echarts/test/Graph/transformProps.test.ts
@@ -74,6 +74,9 @@ describe('EchartsGraph transformProps', () => {
col: 'source_column',
category: undefined,
id: '0',
+ itemStyle: {
+ color: '#1f77b4',
+ },
label: { show: true },
name: 'source_value_1',
select: {
@@ -88,6 +91,9 @@ describe('EchartsGraph transformProps', () => {
col: 'target_column',
category: undefined,
id: '1',
+ itemStyle: {
+ color: '#1f77b4',
+ },
label: { show: true },
name: 'target_value_1',
select: {
@@ -102,6 +108,9 @@ describe('EchartsGraph transformProps', () => {
col: 'source_column',
category: undefined,
id: '2',
+ itemStyle: {
+ color: '#1f77b4',
+ },
label: { show: true },
name: 'source_value_2',
select: {
@@ -116,6 +125,9 @@ describe('EchartsGraph transformProps', () => {
col: 'target_column',
category: undefined,
id: '3',
+ itemStyle: {
+ color: '#1f77b4',
+ },
label: { show: true },
name: 'target_value_2',
select: {
@@ -132,7 +144,7 @@ describe('EchartsGraph transformProps', () => {
links: [
{
emphasis: { lineStyle: { width: 12 } },
- lineStyle: { width: 6 },
+ lineStyle: { width: 6, color: '#1f77b4' },
select: {
lineStyle: { opacity: 1, width: 9.600000000000001 },
},
@@ -142,7 +154,7 @@ describe('EchartsGraph transformProps', () => {
},
{
emphasis: { lineStyle: { width: 5 } },
- lineStyle: { width: 1.5 },
+ lineStyle: { width: 1.5, color: '#1f77b4' },
select: { lineStyle: { opacity: 1, width: 5 } },
source: '2',
target: '3',
@@ -217,6 +229,9 @@ describe('EchartsGraph transformProps', () => {
data: [
{
id: '0',
+ itemStyle: {
+ color: '#1f77b4',
+ },
col: 'source_column',
name: 'source_value',
value: 11,
@@ -228,6 +243,9 @@ describe('EchartsGraph transformProps', () => {
},
{
id: '1',
+ itemStyle: {
+ color: '#ff7f0e',
+ },
col: 'target_column',
name: 'target_value',
value: 11,