diff --git a/.rat-excludes b/.rat-excludes index a8460b00ec9..6d37fdd7a91 100644 --- a/.rat-excludes +++ b/.rat-excludes @@ -73,6 +73,7 @@ ibm-db2.svg postgresql.svg snowflake.svg ydb.svg +loading.svg # docs-related erd.puml diff --git a/docker-compose-light.yml b/docker-compose-light.yml index ae176c0c339..1910699be4a 100644 --- a/docker-compose-light.yml +++ b/docker-compose-light.yml @@ -118,6 +118,8 @@ services: POSTGRES_DB: superset_light SUPERSET__SQLALCHEMY_EXAMPLES_URI: "duckdb:////app/data/examples.duckdb" SUPERSET_CONFIG_PATH: /app/docker/pythonpath_dev/superset_config_docker_light.py + GITHUB_HEAD_REF: ${GITHUB_HEAD_REF:-} + GITHUB_SHA: ${GITHUB_SHA:-} superset-init-light: build: diff --git a/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.stories.tsx b/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.stories.tsx index 98da9c1c155..92300215bb3 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.stories.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.stories.tsx @@ -16,16 +16,22 @@ * specific language governing permissions and limitations * under the License. */ -import type { LoadingProps, PositionOption } from './types'; +import type { LoadingProps, PositionOption, SizeOption } from './types'; import { Loading } from '.'; export default { title: 'Components/Loading', component: Loading, - includeStories: ['LoadingGallery', 'InteractiveLoading'], + includeStories: [ + 'LoadingGallery', + 'SizeAndOpacityShowcase', + 'ContextualExamples', + 'InteractiveLoading', + ], }; export const POSITIONS: PositionOption[] = ['normal', 'floating', 'inline']; +export const SIZES: SizeOption[] = ['s', 'm', 'l']; export const LoadingGallery = () => ( <> @@ -47,12 +53,171 @@ export const LoadingGallery = () => ( ); LoadingGallery.parameters = { - actions: { - disable: true, - }, - controls: { - disable: true, - }, + actions: { disable: true }, + controls: { disable: true }, +}; + +export const SizeAndOpacityShowcase = () => ( +
+

Size and Opacity Combinations

+
+
+ Size +
+
+ Normal +
+
+ Muted +
+
+ Usage Context +
+ + {SIZES.map(size => ( + <> +
+ {size.toUpperCase()} ( + {size === 's' ? '40px' : size === 'm' ? '70px' : '100px'}) +
+
+ +
+
+ +
+
+ {size === 's' && 'Filter bars, inline elements'} + {size === 'm' && 'Explore pages, medium content'} + {size === 'l' && 'Main loading, full pages'} +
+ + ))} +
+
+); + +SizeAndOpacityShowcase.parameters = { + actions: { disable: true }, + controls: { disable: true }, +}; + +export const ContextualExamples = () => ( +
+

Contextual Usage Examples

+ +
+

Filter Bar (size="s", muted)

+
+ Filter 1: + + Filter 2: + + Filter 3: + +
+
+ +
+

Dashboard Chart Grid (size="s", muted)

+
+ {Array.from({ length: 6 }, (_, i) => ( +
+ +
+ ))} +
+
+ +
+

Explore Page (size="m")

+
+ +
+
+ +
+

Main Application Loading (size="l")

+
+ +
+
+
+); + +ContextualExamples.parameters = { + actions: { disable: true }, + controls: { disable: true }, }; export const InteractiveLoading = (args: LoadingProps) => ; @@ -60,6 +225,8 @@ export const InteractiveLoading = (args: LoadingProps) => ; InteractiveLoading.args = { image: '', className: '', + size: 'm', + muted: false, }; InteractiveLoading.argTypes = { @@ -68,4 +235,13 @@ InteractiveLoading.argTypes = { control: { type: 'select' }, options: POSITIONS, }, + size: { + name: 'size', + control: { type: 'select' }, + options: SIZES, + }, + muted: { + name: 'muted', + control: { type: 'boolean' }, + }, }; diff --git a/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.test.tsx index 65893e8fc0a..9b86c3dc2b0 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.test.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Loading/Loading.test.tsx @@ -55,7 +55,8 @@ test('support for extra classes', () => { test('Different image path', () => { render(); const loading = screen.getByRole('status'); - const imagePath = loading.getAttribute('src'); + const image = loading.querySelector('img'); + const imagePath = image?.getAttribute('src'); expect(loading).toBeInTheDocument(); expect(imagePath).toBe('/src/assets/images/loading.gif'); }); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Loading/index.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Loading/index.test.tsx new file mode 100644 index 00000000000..6b0449a4424 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/components/Loading/index.test.tsx @@ -0,0 +1,107 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import { render, screen } from '@superset-ui/core/spec'; +import * as themeModule from '../../theme'; +import { Loading } from '.'; + +// Mock the loading SVG import since it's a file stub in tests +jest.mock('../assets', () => ({ + Loading: () => , +})); + +const mockUseTheme = jest.fn(); + +beforeEach(() => { + mockUseTheme.mockReset(); + jest.spyOn(themeModule, 'useTheme').mockImplementation(mockUseTheme); +}); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +test('uses default spinner when no theme spinner configured', () => { + mockUseTheme.mockReturnValue({}); + + render(); + const loading = screen.getByRole('status'); + expect(loading).toBeInTheDocument(); + // Now renders SVG component instead of img with src + expect( + loading.querySelector('[data-test="default-loading-svg"]'), + ).toBeInTheDocument(); +}); + +test('uses brandSpinnerUrl from theme when configured', () => { + mockUseTheme.mockReturnValue({ + brandSpinnerUrl: '/custom/spinner.png', + }); + + render(); + const loading = screen.getByRole('status'); + expect(loading).toBeInTheDocument(); + const img = loading.querySelector('img'); + expect(img).toHaveAttribute('src', '/custom/spinner.png'); +}); + +test('uses brandSpinnerSvg from theme when configured', () => { + const svgContent = ''; + mockUseTheme.mockReturnValue({ + brandSpinnerSvg: svgContent, + }); + + render(); + const loading = screen.getByRole('status'); + expect(loading).toBeInTheDocument(); + const img = loading.querySelector('img'); + const src = img?.getAttribute('src'); + expect(src).toContain('data:image/svg+xml;base64,'); + expect(atob(src!.split(',')[1])).toBe(svgContent); +}); + +test('brandSpinnerSvg takes precedence over brandSpinnerUrl', () => { + const svgContent = ''; + mockUseTheme.mockReturnValue({ + brandSpinnerUrl: '/custom/spinner.png', + brandSpinnerSvg: svgContent, + }); + + render(); + const loading = screen.getByRole('status'); + expect(loading).toBeInTheDocument(); + const img = loading.querySelector('img'); + const src = img?.getAttribute('src'); + expect(src).toContain('data:image/svg+xml;base64,'); + expect(src).not.toBe('/custom/spinner.png'); +}); + +test('explicit image prop takes precedence over theme spinners', () => { + const svgContent = ''; + mockUseTheme.mockReturnValue({ + brandSpinnerUrl: '/custom/spinner.png', + brandSpinnerSvg: svgContent, + }); + + render(); + const loading = screen.getByRole('status'); + expect(loading).toBeInTheDocument(); + const img = loading.querySelector('img'); + expect(img).toHaveAttribute('src', '/explicit/spinner.gif'); +}); diff --git a/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx b/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx index 1c71d56a7d3..d37e39a5227 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx +++ b/superset-frontend/packages/superset-ui-core/src/components/Loading/index.tsx @@ -18,28 +18,40 @@ */ import cls from 'classnames'; -import { styled } from '../../theme'; -import { Loading as Loader } from '../assets'; -import type { LoadingProps } from './types'; +import { styled, useTheme } from '../../theme'; +import { Loading as LoaderSvg } from '../assets'; +import type { LoadingProps, SizeOption } from './types'; -const LoaderImg = styled.img` +const SIZE_MAP: Record = { + s: '40px', + m: '70px', + l: '100px', +}; + +const LoaderWrapper = styled.div<{ + $spinnerWidth: string; + $spinnerHeight: string; + $opacity: number; +}>` z-index: 99; - width: 50px; - height: unset; + width: ${({ $spinnerWidth }) => $spinnerWidth}; + height: ${({ $spinnerHeight }) => $spinnerHeight}; + opacity: ${({ $opacity }) => $opacity}; position: relative; - margin: 10px; - &.inline { - margin: 0px; - width: 30px; + margin: 0; + padding: 0; + + & > svg, + & > img { + width: 100%; + height: 100%; } + &.inline-centered { margin: 0 auto; - width: 30px; display: block; } &.floating { - padding: 0; - margin: 0; position: absolute; left: 50%; top: 50%; @@ -50,17 +62,47 @@ export function Loading({ position = 'floating', image, className, + size = 'm', + muted = false, }: LoadingProps) { + const theme = useTheme(); + + // Determine size from size prop + const spinnerSize = SIZE_MAP[size]; + + // Opacity - muted reduces to 0.25, otherwise full opacity + const opacity = muted ? 0.25 : 1.0; + + // Render spinner content + const renderSpinner = () => { + // Precedence: explicit image prop > brandSpinnerSvg > brandSpinnerUrl > default SVG + if (image) { + return Loading...; + } + if (theme.brandSpinnerSvg) { + const svgDataUri = `data:image/svg+xml;base64,${btoa(theme.brandSpinnerSvg)}`; + return Loading...; + } + if (theme.brandSpinnerUrl) { + return Loading...; + } + // Default: use the imported SVG component + return ; + }; + return ( - + > + {renderSpinner()} + ); } diff --git a/superset-frontend/packages/superset-ui-core/src/components/Loading/types.ts b/superset-frontend/packages/superset-ui-core/src/components/Loading/types.ts index 28f013d9720..1edfd5dbbde 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/Loading/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/components/Loading/types.ts @@ -22,8 +22,12 @@ export type PositionOption = | 'inline-centered' | 'normal'; +export type SizeOption = 's' | 'm' | 'l'; + export interface LoadingProps { position?: PositionOption; className?: string; image?: string; + size?: SizeOption; + muted?: boolean; } diff --git a/superset-frontend/packages/superset-ui-core/src/components/assets/images/index.ts b/superset-frontend/packages/superset-ui-core/src/components/assets/images/index.ts index c272c1186cd..4d6081f4a0d 100644 --- a/superset-frontend/packages/superset-ui-core/src/components/assets/images/index.ts +++ b/superset-frontend/packages/superset-ui-core/src/components/assets/images/index.ts @@ -16,6 +16,6 @@ * specific language governing permissions and limitations * under the License. */ -import Loading from './loading.gif'; +import LoadingSvg from './loading.svg'; -export { Loading }; +export { LoadingSvg as Loading }; diff --git a/superset-frontend/packages/superset-ui-core/src/components/assets/images/loading.svg b/superset-frontend/packages/superset-ui-core/src/components/assets/images/loading.svg new file mode 100644 index 00000000000..ffb9eb0ce73 --- /dev/null +++ b/superset-frontend/packages/superset-ui-core/src/components/assets/images/loading.svg @@ -0,0 +1,14 @@ + + + + + + + + + + + + + + diff --git a/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx b/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx index 3d220414559..308eda8cce1 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx +++ b/superset-frontend/packages/superset-ui-core/src/theme/Theme.tsx @@ -69,6 +69,10 @@ export class Theme { brandLogoHref: '/', brandLogoHeight: '24px', + // Spinner + brandSpinnerUrl: undefined, + brandSpinnerSvg: undefined, + // Default colors colorPrimary: '#2893B3', // NOTE: previous lighter primary color was #20a7c9 colorLink: '#2893B3', diff --git a/superset-frontend/packages/superset-ui-core/src/theme/types.ts b/superset-frontend/packages/superset-ui-core/src/theme/types.ts index 39dfc4d6717..a391e80037d 100644 --- a/superset-frontend/packages/superset-ui-core/src/theme/types.ts +++ b/superset-frontend/packages/superset-ui-core/src/theme/types.ts @@ -123,6 +123,10 @@ export interface SupersetSpecificTokens { brandLogoMargin: string; brandLogoHref: string; brandLogoHeight: string; + + // Spinner-related + brandSpinnerUrl?: string; + brandSpinnerSvg?: string; } /** diff --git a/superset-frontend/src/components/Chart/Chart.tsx b/superset-frontend/src/components/Chart/Chart.tsx index 8e6de71c66b..201b6769536 100644 --- a/superset-frontend/src/components/Chart/Chart.tsx +++ b/superset-frontend/src/components/Chart/Chart.tsx @@ -253,7 +253,10 @@ class Chart extends PureComponent { data-test="chart-container" height={height} > - + ); } @@ -278,7 +281,11 @@ class Chart extends PureComponent { return ( - + {message} ); @@ -296,7 +303,10 @@ class Chart extends PureComponent { data-test={this.props.vizType} /> ) : ( - + )} ); diff --git a/superset-frontend/src/components/ListView/ListView.tsx b/superset-frontend/src/components/ListView/ListView.tsx index bdb215770b3..574327a2476 100644 --- a/superset-frontend/src/components/ListView/ListView.tsx +++ b/superset-frontend/src/components/ListView/ListView.tsx @@ -28,6 +28,7 @@ import { Checkbox, Icons, EmptyState, + Loading, type EmptyStateProps, } from '@superset-ui/core/components'; import CardCollection from './CardCollection'; @@ -93,6 +94,16 @@ const ListViewStyles = styled.div` `} `; +const FullPageLoadingWrapper = styled.div` + ${({ theme }) => ` + display: flex; + align-items: center; + justify-content: center; + min-height: 50vh; + padding: ${theme.sizeUnit * 16}px; + `} +`; + const BulkSelectWrapper = styled(Alert)` ${({ theme }) => ` border-radius: 0; @@ -444,28 +455,36 @@ export function ListView({ /> )} {viewMode === 'table' && ( - { - const row = rows.find(r => r.id === rowId); - if (row) { - prepareRow(row); - row.toggleRowSelected(value); - } - }} - toggleAllRowsSelected={toggleAllRowsSelected} - /> + <> + {loading && rows.length === 0 ? ( + + + + ) : ( + 0} + highlightRowId={highlightRowId} + columnsForWrapText={columnsForWrapText} + bulkSelectEnabled={bulkSelectEnabled} + selectedFlatRows={selectedFlatRows} + toggleRowSelected={(rowId, value) => { + const row = rows.find(r => r.id === rowId); + if (row) { + prepareRow(row); + row.toggleRowSelected(value); + } + }} + toggleAllRowsSelected={toggleAllRowsSelected} + /> + )} + )} {!loading && rows.length === 0 && ( diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx index 7328464ced2..322d0295996 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx @@ -244,17 +244,17 @@ describe('DashboardBuilder', () => { }); it('should not display a loading spinner when saving is not in progress', () => { - const { queryByAltText } = setup(); + const { queryByTestId } = setup(); - expect(queryByAltText('Loading...')).not.toBeInTheDocument(); + expect(queryByTestId('loading-indicator')).not.toBeInTheDocument(); }); it('should display a loading spinner when saving is in progress', async () => { - const { findByAltText } = setup({ + const { findByTestId } = setup({ dashboardState: { ...mockState.dashboardState, dashboardIsSaving: true }, }); - expect(await findByAltText('Loading...')).toBeVisible(); + expect(await findByTestId('loading-indicator')).toBeVisible(); }); it('should set FilterBar width by useStoredSidebarWidth', () => { diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx index 97a6327f751..4e0d3a45fd2 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/FilterControls/FilterValue.tsx @@ -355,7 +355,7 @@ const FilterValue: FC = ({ return ( {isLoading ? ( - + ) : ( = ({ {!isInitialized ? ( - + ) : ( <> diff --git a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx index c36086332e1..dd30d8d869f 100644 --- a/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx +++ b/superset-frontend/src/dashboard/components/nativeFilters/FilterBar/Vertical.tsx @@ -223,7 +223,7 @@ const VerticalFilterBar: FC = ({
{!isInitialized ? (
- +
) : (
diff --git a/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.test.tsx b/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.test.tsx index 03556a5c6fe..9770240d97d 100644 --- a/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.test.tsx +++ b/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.test.tsx @@ -19,7 +19,6 @@ import { render, screen } from 'spec/helpers/testing-library'; import DatasetPanel, { REFRESHING, - ALT_LOADING, tableColumnDefinition, COLUMN_TITLE, } from 'src/features/datasets/AddDataset/DatasetPanel/DatasetPanel'; @@ -101,8 +100,8 @@ describe('DatasetPanel', () => { }, ); - const blankDatasetImg = screen.getByAltText(ALT_LOADING); - expect(blankDatasetImg).toBeVisible(); + const loadingIndicator = screen.getByTestId('loading-indicator'); + expect(loadingIndicator).toBeVisible(); const blankDatasetTitle = screen.getByText(REFRESHING); expect(blankDatasetTitle).toBeVisible(); }); diff --git a/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.tsx b/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.tsx index c4bc66c55cf..8655cf84508 100644 --- a/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.tsx +++ b/superset-frontend/src/features/datasets/AddDataset/DatasetPanel/DatasetPanel.tsx @@ -18,13 +18,11 @@ */ import { t, styled } from '@superset-ui/core'; import { Icons } from '@superset-ui/core/components/Icons'; -import { Alert, Image } from '@superset-ui/core/components'; +import { Alert, Loading } from '@superset-ui/core/components'; import Table, { ColumnsType, TableSize, } from '@superset-ui/core/components/Table'; -// @ts-ignore -import LOADING_GIF from 'src/assets/images/loading.gif'; import { DatasetObject } from 'src/features/datasets/AddDataset/types'; import { ITableColumn } from './types'; import MessageContent from './MessageContent'; @@ -173,7 +171,6 @@ const StyledAlert = styled(Alert)` export const REFRESHING = t('Refreshing columns'); export const COLUMN_TITLE = t('Table columns'); -export const ALT_LOADING = t('Loading'); const pageSizeOptions = ['5', '10', '15', '25']; const DEFAULT_PAGE_SIZE = 25; @@ -270,7 +267,7 @@ const DatasetPanel = ({ loader = ( - {ALT_LOADING} +
{REFRESHING}
diff --git a/superset/config.py b/superset/config.py index 2d3d2936d53..b41992fe1e8 100644 --- a/superset/config.py +++ b/superset/config.py @@ -59,10 +59,10 @@ from superset.superset_typing import CacheConfig from superset.tasks.types import ExecutorType from superset.themes.types import Theme from superset.utils import core as utils -from superset.utils.core import NO_TIME_RANGE, parse_boolean_string, QuerySource from superset.utils.encrypt import SQLAlchemyUtilsAdapter from superset.utils.log import DBEventLogger from superset.utils.logging_configurator import DefaultLoggingConfigurator +from superset.utils.version import get_dev_env_label logger = logging.getLogger(__name__) @@ -179,7 +179,7 @@ SUPERSET_CLIENT_RETRY_JITTER_MAX = 1000 # Maximum random jitter in milliseconds SUPERSET_CLIENT_RETRY_STATUS_CODES = [502, 503, 504] # default time filter in explore # values may be "Last day", "Last week", " : now", etc. -DEFAULT_TIME_FILTER = NO_TIME_RANGE +DEFAULT_TIME_FILTER = utils.NO_TIME_RANGE # This is an important setting, and should be lower than your # [load balancer / proxy / envoy / kong / ...] timeout settings. @@ -287,7 +287,7 @@ WTF_CSRF_EXEMPT_LIST = [ ] # Whether to run the web server in debug mode or not -DEBUG = parse_boolean_string(os.environ.get("FLASK_DEBUG")) +DEBUG = utils.parse_boolean_string(os.environ.get("FLASK_DEBUG")) FLASK_USE_RELOAD = True # Enable profiling of Python calls. Turn this on and append ``?_instrument=1`` @@ -654,7 +654,7 @@ SSH_TUNNEL_PACKET_TIMEOUT_SEC = 1.0 # Feature flags may also be set via 'SUPERSET_FEATURE_' prefixed environment vars. DEFAULT_FEATURE_FLAGS.update( { - k[len("SUPERSET_FEATURE_") :]: parse_boolean_string(v) + k[len("SUPERSET_FEATURE_") :]: utils.parse_boolean_string(v) for k, v in os.environ.items() if re.search(r"^SUPERSET_FEATURE_\w+", k) } @@ -663,7 +663,7 @@ DEFAULT_FEATURE_FLAGS.update( # This function can be overridden to customize the name of the user agent # triggering the query. -USER_AGENT_FUNC: Callable[[Database, QuerySource | None], str] | None = None +USER_AGENT_FUNC: Callable[[Database, utils.QuerySource | None], str] | None = None # This is merely a default. FEATURE_FLAGS: dict[str, bool] = {} @@ -2087,6 +2087,8 @@ ZIP_FILE_MAX_COMPRESS_RATIO = 200.0 # Configuration for environment tag shown on the navbar. Setting 'text' to '' will hide the tag. # noqa: E501 # 'color' support only Ant Design semantic colors (e.g., 'error', 'warning', 'success', 'processing', 'default) # noqa: E501 + + ENVIRONMENT_TAG_CONFIG = { "variable": "SUPERSET_ENV", "values": { @@ -2095,8 +2097,8 @@ ENVIRONMENT_TAG_CONFIG = { "text": "flask-debug", }, "development": { - "color": "error", - "text": "Development", + "color": "processing", + "text": get_dev_env_label(), }, "production": { "color": "", diff --git a/superset/templates/superset/spa.html b/superset/templates/superset/spa.html index c64c61cdd3c..e849a21881c 100644 --- a/superset/templates/superset/spa.html +++ b/superset/templates/superset/spa.html @@ -70,21 +70,34 @@ {% block body %}
- + {% set tokens = theme_tokens | default({}) %} + {% set spinner_style = "width: 70px; height: auto; position: absolute; top: 50%; left: 50%; transform: translate(-50%, -50%);" %} + + {% if spinner_svg %} + +
+ {{ spinner_svg | safe }} +
+ {% elif tokens.get('brandSpinnerUrl') %} + + Loading... + {% else %} + +
+ Loading... +
+ {% endif %}
{% endblock %} {% block tail_js %} - {{ js_bundle(assets_prefix, entry) }} + {% if entry %} + {{ js_bundle(assets_prefix, entry) }} + {% endif %} {% endblock %} ", "", svg_content, flags=re.IGNORECASE | re.DOTALL + ) + content = re.sub(r"javascript:", "", content, flags=re.IGNORECASE) + content = re.sub(r"data:[^;]*;[^,]*,.*javascript", "", content, flags=re.IGNORECASE) + + # Remove event handlers (simple catch-all approach) + content = re.sub(r"\bon\w+\s*=", "", content, flags=re.IGNORECASE) + + # Remove other suspicious patterns + content = re.sub( + r"]*>.*?", "", content, flags=re.IGNORECASE | re.DOTALL + ) + content = re.sub( + r"]*>.*?", "", content, flags=re.IGNORECASE | re.DOTALL + ) + content = re.sub(r"]*>", "", content, flags=re.IGNORECASE) + + return content + + +def sanitize_url(url: str) -> str: + """Sanitize URL using urllib.parse to block dangerous schemes. + + Simple validation using standard library. Allows relative URLs and + safe absolute URLs while blocking javascript: and other dangerous schemes. + + Args: + url: Raw URL string + + Returns: + str: Sanitized URL or empty string if dangerous + """ + if not url or not url.strip(): + return "" + + url = url.strip() + + # Relative URLs are safe + if url.startswith("/"): + return url + + try: + from urllib.parse import urlparse + + parsed = urlparse(url) + + # Allow safe schemes only + if parsed.scheme.lower() in {"http", "https", ""}: + return url + + # Block everything else (javascript:, data:, etc.) + return "" + + except Exception: + return "" + + def readfile(file_path: str) -> str | None: with open(file_path) as f: content = f.read() diff --git a/superset/utils/version.py b/superset/utils/version.py new file mode 100644 index 00000000000..f57f41d085f --- /dev/null +++ b/superset/utils/version.py @@ -0,0 +1,115 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Centralized version metadata utilities for Apache Superset. +""" + +import os +import subprocess +from typing import Any + +from flask import current_app as app + + +def get_version_metadata() -> dict[str, Any]: + """ + Get version metadata with backward compatibility. + + Returns all the fields that existing code expects. + """ + # Start with app config for backward compatibility + metadata = { + "version_string": app.config.get("VERSION_STRING", "unknown"), + "version_sha": app.config.get("VERSION_SHA", ""), + "build_number": app.config.get("BUILD_NUMBER"), + } + + # Get Git info from GitHub Actions or local git + if github_sha := os.environ.get("GITHUB_SHA"): + metadata["full_sha"] = github_sha + if not metadata["version_sha"]: + metadata["version_sha"] = github_sha[:8] + + # Get branch name + branch_name = ( + os.environ.get("GITHUB_HEAD_REF") + or os.environ.get("GITHUB_REF_NAME") + or _get_local_branch() + ) + if branch_name: + metadata["branch_name"] = branch_name + + return metadata + + +def get_dev_env_label() -> str: + """ + Generate development environment label with branch/SHA info. + + No Flask app dependency - safe to call during config loading. + + Returns: + Simple string like "branch@sha" or just "@sha" or "" + """ + # Get branch and SHA from environment or git + branch = ( + os.environ.get("GITHUB_HEAD_REF") + or os.environ.get("GITHUB_REF_NAME") + or _get_local_branch() + ) + + sha = os.environ.get("GITHUB_SHA") or _get_local_sha() + if sha: + sha = sha[:8] # Short SHA + + # Build label + if branch and sha: + return f"{branch}@{sha}" + elif sha: + return f"@{sha}" + elif branch: + return branch + else: + return "" + + +def _get_local_branch() -> str | None: + """Get branch from local git as fallback.""" + try: + output = subprocess.check_output( # noqa: S603 + ["git", "rev-parse", "--abbrev-ref", "HEAD"], # noqa: S607 + stderr=subprocess.DEVNULL, + timeout=5, + ) + branch = output.decode().strip() + return None if branch == "HEAD" else branch + except Exception: # pylint: disable=broad-except + return None + + +def _get_local_sha() -> str | None: + """Get SHA from local git as fallback.""" + try: + output = subprocess.check_output( # noqa: S603 + ["git", "rev-parse", "HEAD"], # noqa: S607 + stderr=subprocess.DEVNULL, + timeout=5, + ) + return output.decode().strip() + except Exception: # pylint: disable=broad-except + return None diff --git a/superset/views/base.py b/superset/views/base.py index 393e9ac574a..000ab652909 100644 --- a/superset/views/base.py +++ b/superset/views/base.py @@ -67,6 +67,7 @@ from superset.themes.utils import ( ) from superset.utils import core as utils, json from superset.utils.filters import get_dataset_access_filters +from superset.utils.version import get_version_metadata from superset.views.error_handling import json_error_response from .utils import bootstrap_user_data, get_config_value @@ -214,20 +215,29 @@ class BaseSupersetView(BaseView): ) def render_app_template( - self, extra_bootstrap_data: dict[str, Any] | None = None + self, + extra_bootstrap_data: dict[str, Any] | None = None, + entry: str | None = "spa", + **template_kwargs: Any, ) -> FlaskResponse: - payload = { - "user": bootstrap_user_data(g.user, include_perms=True), - "common": common_bootstrap_payload(), - **(extra_bootstrap_data or {}), - } - return self.render_template( - "superset/spa.html", - entry="spa", - bootstrap_data=json.dumps( - payload, default=json.pessimistic_json_iso_dttm_ser - ), + """ + Render spa.html template with standardized context including spinner logic. + + This centralizes all spa.html rendering to ensure consistent spinner behavior + and reduce code duplication across view methods. + + Args: + extra_bootstrap_data: Additional data for frontend bootstrap payload + entry: Entry point name (spa, explore, embedded) + **template_kwargs: Additional template variables + + Returns: + Flask response from render_template + """ + context = get_spa_template_context( + entry, extra_bootstrap_data, **template_kwargs ) + return self.render_template("superset/spa.html", **context) def get_environment_tag() -> dict[str, Any]: @@ -263,6 +273,9 @@ def menu_data(user: User) -> dict[str, Any]: if callable(brand_text := app.config["LOGO_RIGHT_TEXT"]): brand_text = brand_text() + # Get centralized version metadata + version_metadata = get_version_metadata() + return { "menu": appbuilder.menu.get_data(), "brand": { @@ -282,9 +295,9 @@ def menu_data(user: User) -> dict[str, Any]: "documentation_url": app.config["DOCUMENTATION_URL"], "documentation_icon": app.config["DOCUMENTATION_ICON"], "documentation_text": app.config["DOCUMENTATION_TEXT"], - "version_string": app.config["VERSION_STRING"], - "version_sha": app.config["VERSION_SHA"], - "build_number": app.config["BUILD_NUMBER"], + "version_string": version_metadata.get("version_string"), + "version_sha": version_metadata.get("version_sha"), + "build_number": version_metadata.get("build_number"), "languages": languages, "show_language_picker": len(languages) > 1, "user_is_anonymous": user.is_anonymous, @@ -365,6 +378,36 @@ def get_theme_bootstrap_data() -> dict[str, Any]: } +def get_default_spinner_svg() -> str | None: + """ + Load and cache the default spinner SVG content from frontend assets. + + Returns: + str | None: SVG content as string, or None if file not found + """ + try: + # Path to frontend source SVG file (used by both frontend and backend) + svg_path = os.path.join( + os.path.dirname(__file__), + "..", + "..", + "superset-frontend", + "packages", + "superset-ui-core", + "src", + "components", + "assets", + "images", + "loading.svg", + ) + + with open(svg_path, "r", encoding="utf-8") as f: + return f.read().strip() + except (FileNotFoundError, OSError, UnicodeDecodeError) as e: + logger.warning(f"Could not load default spinner SVG: {e}") + return None + + @cache_manager.cache.memoize(timeout=60) def cached_common_bootstrap_data( # pylint: disable=unused-argument user_id: int | None, locale: Locale | None @@ -464,6 +507,70 @@ def common_bootstrap_payload() -> dict[str, Any]: } +def get_spa_payload(extra_data: dict[str, Any] | None = None) -> dict[str, Any]: + """Generate standardized payload for spa.html template rendering. + + Centralizes the common payload structure used across all spa.html renders. + + Args: + extra_data: Additional data to include in payload + + Returns: + dict[str, Any]: Complete payload for spa.html template + """ + payload = { + "user": bootstrap_user_data(g.user, include_perms=True), + "common": common_bootstrap_payload(), + **(extra_data or {}), + } + return payload + + +def get_spa_template_context( + entry: str | None = "spa", + extra_bootstrap_data: dict[str, Any] | None = None, + **template_kwargs: Any, +) -> dict[str, Any]: + """Generate standardized template context for spa.html rendering. + + Centralizes spa.html template context to eliminate duplication while + preserving Flask-AppBuilder context requirements. + + Args: + entry: Entry point name (spa, explore, embedded) + extra_bootstrap_data: Additional data for frontend bootstrap payload + **template_kwargs: Additional template variables + + Returns: + dict[str, Any]: Template context for spa.html + """ + payload = get_spa_payload(extra_bootstrap_data) + + # Extract theme data for template access + theme_data = get_theme_bootstrap_data().get("theme", {}) + default_theme = theme_data.get("default", {}) + theme_tokens = default_theme.get("token", {}) + + # Determine spinner content with precedence: theme SVG > theme URL > default SVG + spinner_svg = None + if theme_tokens.get("brandSpinnerSvg"): + # Use custom SVG from theme + spinner_svg = theme_tokens["brandSpinnerSvg"] + elif not theme_tokens.get("brandSpinnerUrl"): + # No custom URL either, use default SVG + spinner_svg = get_default_spinner_svg() + + return { + "entry": entry, + "bootstrap_data": json.dumps( + payload, default=json.pessimistic_json_iso_dttm_ser + ), + "theme_tokens": theme_tokens, + "spinner_svg": spinner_svg, + **template_kwargs, + } + + class SupersetListWidget(ListWidget): # pylint: disable=too-few-public-methods template = "superset/fab_overrides/list.html" @@ -473,17 +580,8 @@ class SupersetModelView(ModelView): list_widget = SupersetListWidget def render_app_template(self) -> FlaskResponse: - payload = { - "user": bootstrap_user_data(g.user, include_perms=True), - "common": common_bootstrap_payload(), - } - return self.render_template( - "superset/spa.html", - entry="spa", - bootstrap_data=json.dumps( - payload, default=json.pessimistic_json_iso_dttm_ser - ), - ) + context = get_spa_template_context() + return self.render_template("superset/spa.html", **context) class DeleteMixin: # pylint: disable=too-few-public-methods diff --git a/superset/views/core.py b/superset/views/core.py index 384e88d159e..e836b8b5468 100755 --- a/superset/views/core.py +++ b/superset/views/core.py @@ -579,11 +579,8 @@ class Superset(BaseSupersetView): else: title = _("Explore") - return self.render_template( - "superset/spa.html", - bootstrap_data=json.dumps( - bootstrap_data, default=json.pessimistic_json_iso_dttm_ser - ), + return self.render_app_template( + extra_bootstrap_data=bootstrap_data, entry="explore", title=title, standalone_mode=standalone_mode, @@ -824,17 +821,13 @@ class Superset(BaseSupersetView): ), ) - return self.render_template( - "superset/spa.html", - entry="spa", + bootstrap_payload = { + "user": bootstrap_user_data(g.user, include_perms=True), + "common": common_bootstrap_payload(), + } + return self.render_app_template( + extra_bootstrap_data=bootstrap_payload, title=dashboard.dashboard_title, # dashboard title is always visible - bootstrap_data=json.dumps( - { - "user": bootstrap_user_data(g.user, include_perms=True), - "common": common_bootstrap_payload(), - }, - default=json.pessimistic_json_iso_dttm_ser, - ), standalone_mode=ReservedUrlParameters.is_standalone_mode(), ) @@ -937,13 +930,7 @@ class Superset(BaseSupersetView): "common": common_bootstrap_payload(), } - return self.render_template( - "superset/spa.html", - entry="spa", - bootstrap_data=json.dumps( - payload, default=json.pessimistic_json_iso_dttm_ser - ), - ) + return self.render_app_template(extra_bootstrap_data=payload) @has_access @event_logger.log_this diff --git a/superset/views/dashboard/views.py b/superset/views/dashboard/views.py index d5dd27718d3..23c0bc5b123 100644 --- a/superset/views/dashboard/views.py +++ b/superset/views/dashboard/views.py @@ -29,7 +29,6 @@ from superset import db, event_logger, is_feature_enabled from superset.constants import MODEL_VIEW_RW_METHOD_PERMISSION_MAP, RouteMethod from superset.models.dashboard import Dashboard as DashboardModel from superset.superset_typing import FlaskResponse -from superset.utils import json from superset.views.base import ( BaseSupersetView, common_bootstrap_payload, @@ -122,10 +121,6 @@ class Dashboard(BaseSupersetView): "embedded": {"dashboard_id": dashboard_id_or_slug}, } - return self.render_template( - "superset/spa.html", - entry="embedded", - bootstrap_data=json.dumps( - bootstrap_data, default=json.pessimistic_json_iso_dttm_ser - ), + return self.render_app_template( + extra_bootstrap_data=bootstrap_data, entry="embedded" ) diff --git a/superset/views/health.py b/superset/views/health.py index 46179fda9c5..d04b2a444df 100644 --- a/superset/views/health.py +++ b/superset/views/health.py @@ -14,7 +14,7 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. -from flask import Blueprint, current_app as app +from flask import Blueprint, current_app as app, jsonify from superset import talisman from superset.stats_logger import BaseStatsLogger @@ -31,3 +31,15 @@ def health() -> FlaskResponse: stats_logger: BaseStatsLogger = app.config["STATS_LOGGER"] stats_logger.incr("health") return "OK" + + +@health_blueprint.route("/version") +@talisman(force_https=False) +def version() -> FlaskResponse: + """ + Return comprehensive version information including Git SHA + and branch when available. + """ + from superset.utils.version import get_version_metadata + + return jsonify(get_version_metadata()) diff --git a/tests/unit_tests/themes/test_utils.py b/tests/unit_tests/themes/test_utils.py index b576604a998..097a7834f76 100644 --- a/tests/unit_tests/themes/test_utils.py +++ b/tests/unit_tests/themes/test_utils.py @@ -23,6 +23,7 @@ from superset.themes.utils import ( _is_valid_algorithm, _is_valid_theme_mode, is_valid_theme, + sanitize_theme_tokens, ) @@ -75,3 +76,40 @@ def test_is_valid_algorithm(algorithm, expected): ) def test_is_valid_theme(theme, expected): assert is_valid_theme(theme) is expected + + +def test_sanitize_theme_tokens_with_svg(): + """Test that theme tokens with SVG content get sanitized.""" + theme_config = { + "token": { + "brandSpinnerSvg": ( + '' + ), + "colorPrimary": "#ff0000", + } + } + result = sanitize_theme_tokens(theme_config) + + assert "script" not in result["token"]["brandSpinnerSvg"].lower() + assert result["token"]["colorPrimary"] == "#ff0000" # Other tokens unchanged + + +def test_sanitize_theme_tokens_with_url(): + """Test that theme tokens with URL get sanitized.""" + theme_config = { + "token": { + "brandSpinnerUrl": "javascript:alert('xss')", + "colorPrimary": "#ff0000", + } + } + result = sanitize_theme_tokens(theme_config) + + assert result["token"]["brandSpinnerUrl"] == "" # Blocked + assert result["token"]["colorPrimary"] == "#ff0000" # Unchanged + + +def test_sanitize_theme_tokens_no_spinner_tokens(): + """Test that themes without spinner tokens are unchanged.""" + theme_config = {"token": {"colorPrimary": "#ff0000", "fontFamily": "Arial"}} + result = sanitize_theme_tokens(theme_config) + assert result == theme_config diff --git a/tests/unit_tests/utils/test_core.py b/tests/unit_tests/utils/test_core.py index 19ef114e4c5..37b8ed1877a 100644 --- a/tests/unit_tests/utils/test_core.py +++ b/tests/unit_tests/utils/test_core.py @@ -46,6 +46,8 @@ from superset.utils.core import ( QueryObjectFilterClause, QuerySource, remove_extra_adhoc_filters, + sanitize_svg_content, + sanitize_url, ) from tests.conftest import with_config @@ -1122,3 +1124,41 @@ def test_get_stacktrace(): except Exception: stacktrace = get_stacktrace() assert stacktrace is None + + +def test_sanitize_svg_content_safe(): + """Test that safe SVG content is preserved.""" + safe_svg = '' + result = sanitize_svg_content(safe_svg) + assert "svg" in result + assert "rect" in result + + +def test_sanitize_svg_content_removes_scripts(): + """Test that nh3 removes dangerous script content.""" + malicious_svg = '' + result = sanitize_svg_content(malicious_svg) + assert "script" not in result.lower() + assert "alert" not in result + + +def test_sanitize_url_relative(): + """Test that relative URLs are allowed.""" + assert sanitize_url("/static/spinner.gif") == "/static/spinner.gif" + + +def test_sanitize_url_safe_absolute(): + """Test that safe absolute URLs are allowed.""" + assert ( + sanitize_url("https://example.com/spinner.gif") + == "https://example.com/spinner.gif" + ) + assert ( + sanitize_url("http://localhost/spinner.png") == "http://localhost/spinner.png" + ) + + +def test_sanitize_url_blocks_dangerous(): + """Test that dangerous URL schemes are blocked.""" + assert sanitize_url("javascript:alert('xss')") == "" + assert sanitize_url("data:text/html,") == ""