Skip to content

Commit

Permalink
Merge branch 'master' into msyavuz/refactor/leftover-input
Browse files Browse the repository at this point in the history
  • Loading branch information
msyavuz committed Mar 8, 2025
2 parents 3d7bb2c + 33aa903 commit 4f9e30a
Show file tree
Hide file tree
Showing 36 changed files with 1,577 additions and 747 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/pre-commit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,12 @@ jobs:
- name: Install Frontend Dependencies
run: |
cd superset-frontend
npm install
npm ci
- name: Install Docs Dependencies
run: |
cd docs
yarn install
yarn install --immutable
- name: pre-commit
run: |
Expand All @@ -66,7 +66,7 @@ jobs:
if [ "${PRE_COMMIT_EXIT_CODE}" -ne 0 ]; then
echo "❌ Pre-commit check failed (exit code: ${EXIT_CODE})."
else
echo "❌ Git working directory is dirty after running pre-commit."
echo "❌ Git working directory is dirty."
echo "📌 This likely means that pre-commit made changes that were not committed."
echo "🔍 Modified files:"
git diff --name-only
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tech-debt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ jobs:
node-version-file: './superset-frontend/.nvmrc'

- name: Install Dependencies
run: npm install
run: npm ci
working-directory: ./superset-frontend

- name: Run Script
Expand Down
1 change: 1 addition & 0 deletions RESOURCES/INTHEWILD.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ Join our growing community!
- [Astronomer](https://www.astronomer.io) [@ryw]
- [Avesta Technologies](https://avestatechnologies.com/) [@TheRum]
- [Caizin](https://caizin.com/) [@tejaskatariya]
- [Canonical](https://canonical.com)
- [Careem](https://www.careem.com/) [@samraHanif0340]
- [Cloudsmith](https://cloudsmith.io) [@alancarson]
- [Cyberhaven](https://www.cyberhaven.com/) [@toliver-ch]
Expand Down
2 changes: 1 addition & 1 deletion docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
"@typescript-eslint/eslint-plugin": "^5.0.0",
"@typescript-eslint/parser": "^5.0.0",
"eslint": "^8.0.0",
"eslint-config-prettier": "^8.0.0",
"eslint-config-prettier": "^10.0.2",
"eslint-plugin-prettier": "^4.0.0",
"eslint-plugin-react": "^7.0.0",
"prettier": "^2.0.0",
Expand Down
8 changes: 4 additions & 4 deletions docs/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5307,10 +5307,10 @@ escape-string-regexp@^5.0.0:
resolved "https://registry.npmjs.org/escape-string-regexp/-/escape-string-regexp-5.0.0.tgz"
integrity sha512-/veY75JbMK4j1yjvuUxuVsiS/hr/4iHs9FTT6cgTexxdE0Ly/glccBAkloH/DofkjRbZU3bnoj38mOmhkZ0lHw==

eslint-config-prettier@^8.0.0:
version "8.10.0"
resolved "https://registry.npmjs.org/eslint-config-prettier/-/eslint-config-prettier-8.10.0.tgz"
integrity sha512-SM8AMJdeQqRYT9O9zguiruQZaN7+z+E4eAP9oiLNGKMtomwaB1E9dcgUD6ZAn/eQAb52USbvezbiljfZUhbJcg==
eslint-config-prettier@^10.0.2:
version "10.0.2"
resolved "https://registry.yarnpkg.com/eslint-config-prettier/-/eslint-config-prettier-10.0.2.tgz#47444de8aa104ce82c2f91ad2a5e96b62c01e20d"
integrity sha512-1105/17ZIMjmCOJOPNfVdbXafLCLj3hPmkmB7dLgt7XsQ/zkxSuDerE/xgO3RxoHysR1N1whmquY0lSn2O0VLg==

eslint-plugin-prettier@^4.0.0:
version "4.2.1"
Expand Down
11 changes: 1 addition & 10 deletions superset-frontend/src/components/Chart/Chart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
logging,
QueryFormData,
styled,
ErrorTypeEnum,
t,
SqlaFormData,
ClientErrorObject,
Expand Down Expand Up @@ -240,15 +239,7 @@ class Chart extends PureComponent<ChartProps, {}> {
height,
datasetsStatus,
} = this.props;
let error = queryResponse?.errors?.[0];
if (error === undefined) {
error = {
error_type: ErrorTypeEnum.FRONTEND_NETWORK_ERROR,
level: 'error',
message: t('Check your network connection'),
extra: null,
};
}
const error = queryResponse?.errors?.[0];
const message = chartAlert || queryResponse?.message;

// if datasource is still loading, don't render JS errors
Expand Down
84 changes: 84 additions & 0 deletions superset-frontend/src/components/Chart/ChartErrorMessage.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**
* 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 'spec/helpers/testing-library';
import '@testing-library/jest-dom';
import { ChartSource } from 'src/types/ChartSource';
import { useChartOwnerNames } from 'src/hooks/apiResources';
import { ResourceStatus } from 'src/hooks/apiResources/apiResources';
import { ErrorType } from '@superset-ui/core';
import { ChartErrorMessage } from './ChartErrorMessage';
import { ErrorMessageComponentProps } from '../ErrorMessage/types';
import getErrorMessageComponentRegistry from '../ErrorMessage/getErrorMessageComponentRegistry';

// Mock the useChartOwnerNames hook
jest.mock('src/hooks/apiResources', () => ({
useChartOwnerNames: jest.fn(),
}));

const mockUseChartOwnerNames = useChartOwnerNames as jest.MockedFunction<
typeof useChartOwnerNames
>;

const ERROR_MESSAGE_COMPONENT = (props: ErrorMessageComponentProps) => (
<>
<div>Test error</div>
<div>{props.subtitle}</div>
</>
);

describe('ChartErrorMessage', () => {
const defaultProps = {
chartId: '1',
subtitle: 'Test subtitle',
source: 'test_source' as ChartSource,
};

it('renders the default error message when error is null', () => {
mockUseChartOwnerNames.mockReturnValue({
result: null,
status: ResourceStatus.Loading,
error: null,
});
render(<ChartErrorMessage {...defaultProps} />);

expect(screen.getByText('Data error')).toBeInTheDocument();
expect(screen.getByText('Test subtitle')).toBeInTheDocument();
});

it('renders the error message that is passed in from the error', () => {
getErrorMessageComponentRegistry().registerValue(
'VALID_KEY',
ERROR_MESSAGE_COMPONENT,
);
render(
<ChartErrorMessage
{...defaultProps}
error={{
error_type: 'VALID_KEY' as unknown as ErrorType,
message: 'Subtitle',
level: 'error',
extra: {},
}}
/>,
);

expect(screen.getByText('Test error')).toBeInTheDocument();
expect(screen.getByText('Test subtitle')).toBeInTheDocument();
});
});
10 changes: 9 additions & 1 deletion superset-frontend/src/components/Chart/ChartErrorMessage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ export type Props = {
stackTrace?: string;
} & Omit<ClientErrorObject, 'error'>;

const DEFAULT_CHART_ERROR = 'Data error';

export const ChartErrorMessage: FC<Props> = ({ chartId, error, ...props }) => {
// fetches the chart owners and adds them to the extra data of the error message
const { result: owners } = useChartOwnerNames(chartId);
Expand All @@ -42,5 +44,11 @@ export const ChartErrorMessage: FC<Props> = ({ chartId, error, ...props }) => {
extra: { ...error.extra, owners },
};

return <ErrorMessageWithStackTrace {...props} error={ownedError} />;
return (
<ErrorMessageWithStackTrace
{...props}
error={ownedError}
title={DEFAULT_CHART_ERROR}
/>
);
};
2 changes: 2 additions & 0 deletions superset-frontend/src/components/DatabaseSelector/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ export default function DatabaseSelector({
options={catalogOptions}
showSearch
value={currentCatalog || undefined}
allowClear
/>,
refreshIcon,
);
Expand All @@ -432,6 +433,7 @@ export default function DatabaseSelector({
options={schemaOptions}
showSearch
value={currentSchema}
allowClear
/>,
refreshIcon,
);
Expand Down
14 changes: 12 additions & 2 deletions superset-frontend/src/components/Datasource/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,11 @@ export function updateColumns(prevCols, newCols, addSuccessToast) {
added: [],
modified: [],
removed: prevCols
.map(col => col.column_name)
.filter(col => !databaseColumnNames.includes(col)),
.filter(
col =>
!(col.expression || databaseColumnNames.includes(col.column_name)),
)
.map(col => col.column_name),
finalColumns: [],
};
newCols.forEach(col => {
Expand Down Expand Up @@ -89,6 +92,13 @@ export function updateColumns(prevCols, newCols, addSuccessToast) {
columnChanges.finalColumns.push(currentCol);
}
});
// push all calculated columns
prevCols
.filter(col => col.expression)
.forEach(col => {
columnChanges.finalColumns.push(col);
});

if (columnChanges.modified.length) {
addSuccessToast(
tn(
Expand Down
Loading

0 comments on commit 4f9e30a

Please sign in to comment.