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/03/26 18:34:53 UTC
(superset) branch 3.1 updated (06195b53ce -> e48dae781f)
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a change to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
from 06195b53ce fix: Persist query params appended to permalink (#27601)
new bc603db7d4 fix(AlertReports): clearing custom_width when disabled (#27551)
new 0b9c0d97dc fix(AlertReports): defaulting grace period to undefined (#27552)
new 64d1a8f8c3 fix(Chart Annotation modal): Table and Superset annotation options will paginate, exceeding previous max limit 100 (#27022)
new e48dae781f fix(sqllab): unable to remove table (#27636)
The 4 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.
Summary of changes:
superset-frontend/src/SqlLab/actions/sqlLab.js | 8 +-
.../src/SqlLab/actions/sqlLab.test.js | 24 +-
.../AnnotationLayerControl/AnnotationLayer.jsx | 350 ++++++++++++++-------
.../AnnotationLayer.test.tsx | 125 +++++---
.../src/features/alerts/AlertReportModal.tsx | 3 +-
5 files changed, 358 insertions(+), 152 deletions(-)
(superset) 04/04: fix(sqllab): unable to remove table (#27636)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit e48dae781f2dd0360239c3aa5afc5f07d8ce52c8
Author: JUST.in DO IT <ju...@airbnb.com>
AuthorDate: Tue Mar 26 10:19:50 2024 -0700
fix(sqllab): unable to remove table (#27636)
(cherry picked from commit fa3fea9dd811d3cfdbbfe93f31d34992e603ec60)
---
superset-frontend/src/SqlLab/actions/sqlLab.js | 8 +++++---
.../src/SqlLab/actions/sqlLab.test.js | 24 ++++++++++++++++++++--
2 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js
index cbab24a21e..72dc03f001 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -1125,9 +1125,11 @@ export function removeTables(tables) {
const sync = isFeatureEnabled(FeatureFlag.SQLLAB_BACKEND_PERSISTENCE)
? Promise.all(
tablesToRemove.map(table =>
- SupersetClient.delete({
- endpoint: encodeURI(`/tableschemaview/${table.id}`),
- }),
+ table.initialized
+ ? SupersetClient.delete({
+ endpoint: encodeURI(`/tableschemaview/${table.id}`),
+ })
+ : Promise.resolve(),
),
)
: Promise.resolve();
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
index dd48ed8c7b..20fd53ad38 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -883,7 +883,7 @@ describe('async actions', () => {
it('updates the table schema state in the backend', () => {
expect.assertions(2);
- const table = { id: 1 };
+ const table = { id: 1, initialized: true };
const store = mockStore({});
const expectedActions = [
{
@@ -900,7 +900,10 @@ describe('async actions', () => {
it('deletes multiple tables and updates the table schema state in the backend', () => {
expect.assertions(2);
- const tables = [{ id: 1 }, { id: 2 }];
+ const tables = [
+ { id: 1, initialized: true },
+ { id: 2, initialized: true },
+ ];
const store = mockStore({});
const expectedActions = [
{
@@ -913,6 +916,23 @@ describe('async actions', () => {
expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(2);
});
});
+
+ it('only updates the initialized table schema state in the backend', () => {
+ expect.assertions(2);
+
+ const tables = [{ id: 1 }, { id: 2, initialized: true }];
+ const store = mockStore({});
+ const expectedActions = [
+ {
+ type: actions.REMOVE_TABLES,
+ tables,
+ },
+ ];
+ return store.dispatch(actions.removeTables(tables)).then(() => {
+ expect(store.getActions()).toEqual(expectedActions);
+ expect(fetchMock.calls(updateTableSchemaEndpoint)).toHaveLength(1);
+ });
+ });
});
describe('migrateQueryEditorFromLocalStorage', () => {
(superset) 03/04: fix(Chart Annotation modal): Table and Superset annotation options will paginate, exceeding previous max limit 100 (#27022)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 64d1a8f8c315aceed4b20aa1e13713b36ac13853
Author: Ross Mabbett <92...@users.noreply.github.com>
AuthorDate: Tue Mar 26 12:54:03 2024 -0400
fix(Chart Annotation modal): Table and Superset annotation options will paginate, exceeding previous max limit 100 (#27022)
(cherry picked from commit ce210eebdeeb374611e5b273379a889244f64288)
---
.../AnnotationLayerControl/AnnotationLayer.jsx | 350 ++++++++++++++-------
.../AnnotationLayer.test.tsx | 125 +++++---
2 files changed, 329 insertions(+), 146 deletions(-)
diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
index 563b946823..e29befc808 100644
--- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
+++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.jsx
@@ -33,12 +33,12 @@ import {
withTheme,
} from '@superset-ui/core';
import SelectControl from 'src/explore/components/controls/SelectControl';
+import { AsyncSelect } from 'src/components';
import TextControl from 'src/explore/components/controls/TextControl';
import CheckboxControl from 'src/explore/components/controls/CheckboxControl';
import PopoverSection from 'src/components/PopoverSection';
import ControlHeader from 'src/explore/components/ControlHeader';
import { EmptyStateSmall } from 'src/components/EmptyState';
-import { FILTER_OPTIONS_LIMIT } from 'src/explore/constants';
import {
ANNOTATION_SOURCE_TYPES,
ANNOTATION_TYPES,
@@ -194,28 +194,46 @@ class AnnotationLayer extends React.PureComponent {
hideLine,
// refData
isNew: !name,
- isLoadingOptions: true,
- valueOptions: [],
+ slice: null,
};
this.submitAnnotation = this.submitAnnotation.bind(this);
this.deleteAnnotation = this.deleteAnnotation.bind(this);
this.applyAnnotation = this.applyAnnotation.bind(this);
- this.fetchOptions = this.fetchOptions.bind(this);
+ this.isValidForm = this.isValidForm.bind(this);
+ // Handlers
this.handleAnnotationType = this.handleAnnotationType.bind(this);
this.handleAnnotationSourceType =
this.handleAnnotationSourceType.bind(this);
- this.handleValue = this.handleValue.bind(this);
- this.isValidForm = this.isValidForm.bind(this);
+ this.handleSelectValue = this.handleSelectValue.bind(this);
+ this.handleTextValue = this.handleTextValue.bind(this);
+ // Fetch related functions
+ this.fetchOptions = this.fetchOptions.bind(this);
+ this.fetchCharts = this.fetchCharts.bind(this);
+ this.fetchNativeAnnotations = this.fetchNativeAnnotations.bind(this);
+ this.fetchAppliedAnnotation = this.fetchAppliedAnnotation.bind(this);
+ this.fetchSliceData = this.fetchSliceData.bind(this);
+ this.shouldFetchSliceData = this.shouldFetchSliceData.bind(this);
+ this.fetchAppliedChart = this.fetchAppliedChart.bind(this);
+ this.fetchAppliedNativeAnnotation =
+ this.fetchAppliedNativeAnnotation.bind(this);
+ this.shouldFetchAppliedAnnotation =
+ this.shouldFetchAppliedAnnotation.bind(this);
}
componentDidMount() {
- const { annotationType, sourceType, isLoadingOptions } = this.state;
- this.fetchOptions(annotationType, sourceType, isLoadingOptions);
+ if (this.shouldFetchAppliedAnnotation()) {
+ const { value } = this.state;
+ /* The value prop is the id of the chart/native. This function will set
+ value in state to an object with the id as value.value to be used by
+ AsyncSelect */
+ this.fetchAppliedAnnotation(value);
+ }
}
componentDidUpdate(prevProps, prevState) {
- if (prevState.sourceType !== this.state.sourceType) {
- this.fetchOptions(this.state.annotationType, this.state.sourceType, true);
+ if (this.shouldFetchSliceData(prevState)) {
+ const { value } = this.state;
+ this.fetchSliceData(value.value);
}
}
@@ -237,6 +255,20 @@ class AnnotationLayer extends React.PureComponent {
return sources;
}
+ shouldFetchAppliedAnnotation() {
+ const { value, sourceType } = this.state;
+ return value && requiresQuery(sourceType);
+ }
+
+ shouldFetchSliceData(prevState) {
+ const { value, sourceType } = this.state;
+ const isChart =
+ sourceType !== ANNOTATION_SOURCE_TYPES.NATIVE &&
+ requiresQuery(sourceType);
+ const valueIsNew = value && prevState.value !== value;
+ return valueIsNew && isChart;
+ }
+
isValidFormulaAnnotation(expression, annotationType) {
if (annotationType === ANNOTATION_TYPES.FORMULA) {
return isValidExpression(expression);
@@ -276,6 +308,7 @@ class AnnotationLayer extends React.PureComponent {
annotationType,
sourceType: null,
value: null,
+ slice: null,
});
}
@@ -283,13 +316,17 @@ class AnnotationLayer extends React.PureComponent {
const { sourceType: prevSourceType } = this.state;
if (prevSourceType !== sourceType) {
- this.setState({ sourceType, value: null, isLoadingOptions: true });
+ this.setState({
+ sourceType,
+ value: null,
+ slice: null,
+ });
}
}
- handleValue(value) {
+ handleSelectValue(selectedValueObject) {
this.setState({
- value,
+ value: selectedValueObject,
descriptionColumns: [],
intervalEndColumn: null,
timeColumn: null,
@@ -298,74 +335,172 @@ class AnnotationLayer extends React.PureComponent {
});
}
- fetchOptions(annotationType, sourceType, isLoadingOptions) {
- if (isLoadingOptions) {
- if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) {
- const queryParams = rison.encode({
- page: 0,
- page_size: FILTER_OPTIONS_LIMIT,
- });
- SupersetClient.get({
- endpoint: `/api/v1/annotation_layer/?q=${queryParams}`,
- }).then(({ json }) => {
- const layers = json
- ? json.result.map(layer => ({
- value: layer.id,
- label: layer.name,
- }))
- : [];
- this.setState({
- isLoadingOptions: false,
- valueOptions: layers,
- });
- });
- } else if (requiresQuery(sourceType)) {
- const queryParams = rison.encode({
- filters: [
- {
- col: 'id',
- opr: 'chart_owned_created_favored_by_me',
- value: true,
- },
- ],
- order_column: 'slice_name',
- order_direction: 'asc',
- page: 0,
- page_size: FILTER_OPTIONS_LIMIT,
- });
- SupersetClient.get({
- endpoint: `/api/v1/chart/?q=${queryParams}`,
- }).then(({ json }) => {
- const registry = getChartMetadataRegistry();
- this.setState({
- isLoadingOptions: false,
- valueOptions: json.result
- .filter(x => {
- const metadata = registry.get(x.viz_type);
- return metadata && metadata.canBeAnnotationType(annotationType);
- })
- .map(x => ({
- value: x.id,
- label: x.slice_name,
- slice: {
- ...x,
- data: {
- ...x.form_data,
- groupby: x.form_data.groupby?.map(column =>
- getColumnLabel(column),
- ),
- },
- },
- })),
- });
- });
- } else {
+ handleTextValue(inputValue) {
+ this.setState({
+ value: inputValue,
+ });
+ }
+
+ fetchNativeAnnotations = async (search, page, pageSize) => {
+ const queryParams = rison.encode({
+ filters: [
+ {
+ col: 'name',
+ opr: 'ct',
+ value: search,
+ },
+ ],
+ columns: ['id', 'name'],
+ page,
+ page_size: pageSize,
+ });
+
+ const { json } = await SupersetClient.get({
+ endpoint: `/api/v1/annotation_layer/?q=${queryParams}`,
+ });
+
+ const { result, count } = json;
+
+ const layersArray = result.map(layer => ({
+ value: layer.id,
+ label: layer.name,
+ }));
+
+ return {
+ data: layersArray,
+ totalCount: count,
+ };
+ };
+
+ fetchCharts = async (search, page, pageSize) => {
+ const { annotationType } = this.state;
+
+ const queryParams = rison.encode({
+ filters: [
+ { col: 'slice_name', opr: 'chart_all_text', value: search },
+ {
+ col: 'id',
+ opr: 'chart_owned_created_favored_by_me',
+ value: true,
+ },
+ ],
+ columns: ['id', 'slice_name', 'viz_type'],
+ order_column: 'slice_name',
+ order_direction: 'asc',
+ page,
+ page_size: pageSize,
+ });
+ const { json } = await SupersetClient.get({
+ endpoint: `/api/v1/chart/?q=${queryParams}`,
+ });
+
+ const { result, count } = json;
+ const registry = getChartMetadataRegistry();
+
+ const chartsArray = result
+ .filter(chart => {
+ const metadata = registry.get(chart.viz_type);
+ return metadata && metadata.canBeAnnotationType(annotationType);
+ })
+ .map(chart => ({
+ value: chart.id,
+ label: chart.slice_name,
+ viz_type: chart.viz_type,
+ }));
+
+ return {
+ data: chartsArray,
+ totalCount: count,
+ };
+ };
+
+ fetchOptions = (search, page, pageSize) => {
+ const { sourceType } = this.state;
+
+ if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) {
+ return this.fetchNativeAnnotations(search, page, pageSize);
+ }
+ return this.fetchCharts(search, page, pageSize);
+ };
+
+ fetchSliceData = id => {
+ const queryParams = rison.encode({
+ columns: ['query_context'],
+ });
+ SupersetClient.get({
+ endpoint: `/api/v1/chart/${id}?q=${queryParams}`,
+ }).then(({ json }) => {
+ const { result } = json;
+ const queryContext = result.query_context;
+ const formData = JSON.parse(queryContext).form_data;
+ const dataObject = {
+ data: {
+ ...formData,
+ groupby: formData.groupby?.map(column => getColumnLabel(column)),
+ },
+ };
+ this.setState({
+ slice: dataObject,
+ });
+ });
+ };
+
+ fetchAppliedChart(id) {
+ const { annotationType } = this.state;
+ const registry = getChartMetadataRegistry();
+ const queryParams = rison.encode({
+ columns: ['slice_name', 'query_context', 'viz_type'],
+ });
+ SupersetClient.get({
+ endpoint: `/api/v1/chart/${id}?q=${queryParams}`,
+ }).then(({ json }) => {
+ const { result } = json;
+ const sliceName = result.slice_name;
+ const queryContext = result.query_context;
+ const vizType = result.viz_type;
+ const formData = JSON.parse(queryContext).form_data;
+ const metadata = registry.get(vizType);
+ const canBeAnnotationType =
+ metadata && metadata.canBeAnnotationType(annotationType);
+ if (canBeAnnotationType) {
this.setState({
- isLoadingOptions: false,
- valueOptions: [],
+ value: {
+ value: id,
+ label: sliceName,
+ },
+ slice: {
+ data: {
+ ...formData,
+ groupby: formData.groupby?.map(column => getColumnLabel(column)),
+ },
+ },
});
}
+ });
+ }
+
+ fetchAppliedNativeAnnotation(id) {
+ SupersetClient.get({
+ endpoint: `/api/v1/annotation_layer/${id}`,
+ }).then(({ json }) => {
+ const { result } = json;
+ const layer = result;
+ this.setState({
+ value: {
+ value: layer.id,
+ label: layer.name,
+ },
+ });
+ });
+ }
+
+ fetchAppliedAnnotation(id) {
+ const { sourceType } = this.state;
+
+ if (sourceType === ANNOTATION_SOURCE_TYPES.NATIVE) {
+ return this.fetchAppliedNativeAnnotation(id);
}
+ return this.fetchAppliedChart(id);
}
deleteAnnotation() {
@@ -374,6 +509,7 @@ class AnnotationLayer extends React.PureComponent {
}
applyAnnotation() {
+ const { value, sourceType } = this.state;
if (this.isValidForm()) {
const annotationFields = [
'name',
@@ -385,7 +521,6 @@ class AnnotationLayer extends React.PureComponent {
'width',
'showMarkers',
'hideLine',
- 'value',
'overrides',
'show',
'showLabel',
@@ -401,6 +536,10 @@ class AnnotationLayer extends React.PureComponent {
}
});
+ // Prepare newAnnotation.value for use in runAnnotationQuery()
+ const applicableValue = requiresQuery(sourceType) ? value.value : value;
+ newAnnotation.value = applicableValue;
+
if (newAnnotation.color === AUTOMATIC_COLOR) {
newAnnotation.color = null;
}
@@ -415,29 +554,19 @@ class AnnotationLayer extends React.PureComponent {
this.props.close();
}
- renderOption(option) {
+ renderChartHeader(label, description, value) {
return (
- <span
- css={{
- overflow: 'hidden',
- textOverflow: 'ellipsis',
- whiteSpace: 'nowrap',
- }}
- title={option.label}
- >
- {option.label}
- </span>
+ <ControlHeader
+ hovered
+ label={label}
+ description={description}
+ validationErrors={!value ? ['Mandatory'] : []}
+ />
);
}
renderValueConfiguration() {
- const {
- annotationType,
- sourceType,
- value,
- valueOptions,
- isLoadingOptions,
- } = this.state;
+ const { annotationType, sourceType, value } = this.state;
let label = '';
let description = '';
if (requiresQuery(sourceType)) {
@@ -462,20 +591,15 @@ class AnnotationLayer extends React.PureComponent {
}
if (requiresQuery(sourceType)) {
return (
- <SelectControl
+ <AsyncSelect
+ /* key to force re-render on sourceType change */
+ key={sourceType}
ariaLabel={t('Annotation layer value')}
name="annotation-layer-value"
- showHeader
- hovered
- description={description}
- label={label}
- placeholder=""
- options={valueOptions}
- isLoading={isLoadingOptions}
- value={value}
- onChange={this.handleValue}
- validationErrors={!value ? ['Mandatory'] : []}
- optionRenderer={this.renderOption}
+ header={this.renderChartHeader(label, description, value)}
+ options={this.fetchOptions}
+ value={value || null}
+ onChange={this.handleSelectValue}
notFoundContent={<NotFoundContent />}
/>
);
@@ -490,7 +614,7 @@ class AnnotationLayer extends React.PureComponent {
label={label}
placeholder=""
value={value}
- onChange={this.handleValue}
+ onChange={this.handleTextValue}
validationErrors={
!this.isValidFormulaAnnotation(value, annotationType)
? [t('Bad formula.')]
@@ -507,14 +631,18 @@ class AnnotationLayer extends React.PureComponent {
annotationType,
sourceType,
value,
- valueOptions,
+ slice,
overrides,
titleColumn,
timeColumn,
intervalEndColumn,
descriptionColumns,
} = this.state;
- const { slice } = valueOptions.find(x => x.value === value) || {};
+
+ if (!slice || !value) {
+ return '';
+ }
+
if (sourceType !== ANNOTATION_SOURCE_TYPES.NATIVE && slice) {
const columns = (slice.data.groupby || [])
.concat(slice.data.all_columns || [])
diff --git a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
index 914be73619..813a8ebcb9 100644
--- a/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
+++ b/superset-frontend/src/explore/components/controls/AnnotationLayerControl/AnnotationLayer.test.tsx
@@ -31,21 +31,37 @@ const defaultProps = {
annotationType: ANNOTATION_TYPES_METADATA.FORMULA.value,
};
+const nativeLayerApiRoute = 'glob:*/api/v1/annotation_layer/*';
+const chartApiRoute = /\/api\/v1\/chart\/\?q=.+/;
+const chartApiWithIdRoute = /\/api\/v1\/chart\/\w+\?q=.+/;
+
+const withIdResult = {
+ result: {
+ slice_name: 'Mocked Slice',
+ query_context: JSON.stringify({
+ form_data: {
+ groupby: ['country'],
+ },
+ }),
+ viz_type: 'line',
+ },
+};
+
beforeAll(() => {
const supportedAnnotationTypes = Object.values(ANNOTATION_TYPES_METADATA).map(
value => value.value,
);
- fetchMock.get('glob:*/api/v1/annotation_layer/*', {
- result: [{ label: 'Chart A', value: 'a' }],
+ fetchMock.get(nativeLayerApiRoute, {
+ result: [{ name: 'Chart A', id: 'a' }],
});
- fetchMock.get('glob:*/api/v1/chart/*', {
- result: [
- { id: 'a', slice_name: 'Chart A', viz_type: 'table', form_data: {} },
- ],
+ fetchMock.get(chartApiRoute, {
+ result: [{ id: 'a', slice_name: 'Chart A', viz_type: 'table' }],
});
+ fetchMock.get(chartApiWithIdRoute, withIdResult);
+
setupColors();
getChartMetadataRegistry().registerValue(
@@ -144,7 +160,7 @@ test('triggers removeAnnotationLayer and close when remove button is clicked', a
expect(close).toHaveBeenCalled();
});
-test('renders chart options', async () => {
+test('fetches Superset annotation layer options', async () => {
await waitForRender({
annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
});
@@ -153,12 +169,37 @@ test('renders chart options', async () => {
);
userEvent.click(screen.getByText('Superset annotation'));
expect(await screen.findByText('Annotation layer')).toBeInTheDocument();
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation layer value' }),
+ );
+ expect(await screen.findByText('Chart A')).toBeInTheDocument();
+ expect(fetchMock.calls(nativeLayerApiRoute).length).toBe(1);
+});
+test('fetches chart options', async () => {
+ await waitForRender({
+ annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
+ });
userEvent.click(
screen.getByRole('combobox', { name: 'Annotation source type' }),
);
userEvent.click(screen.getByText('Table'));
expect(await screen.findByText('Chart')).toBeInTheDocument();
+ userEvent.click(
+ screen.getByRole('combobox', { name: 'Annotation layer value' }),
+ );
+ expect(await screen.findByText('Chart A')).toBeInTheDocument();
+ expect(fetchMock.calls(chartApiRoute).length).toBe(1);
+});
+
+test('fetches chart on mount if value present', async () => {
+ await waitForRender({
+ name: 'Test',
+ value: 'a',
+ annotationType: ANNOTATION_TYPES_METADATA.EVENT.value,
+ sourceType: 'Table',
+ });
+ expect(fetchMock.calls(chartApiWithIdRoute).length).toBe(1);
});
test('keeps apply disabled when missing required fields', async () => {
@@ -171,7 +212,7 @@ test('keeps apply disabled when missing required fields', async () => {
);
expect(await screen.findByText('Chart A')).toBeInTheDocument();
userEvent.click(screen.getByText('Chart A'));
-
+ await screen.findByText(/title column/i);
userEvent.click(screen.getByRole('button', { name: 'Automatic Color' }));
userEvent.click(
screen.getByRole('combobox', { name: 'Annotation layer title column' }),
@@ -197,47 +238,61 @@ test('keeps apply disabled when missing required fields', async () => {
expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
});
-test.skip('Disable apply button if formula is incorrect', async () => {
- // TODO: fix flaky test that passes locally but fails on CI
+test('Disable apply button if formula is incorrect', async () => {
await waitForRender({ name: 'test' });
- userEvent.clear(screen.getByLabelText('Formula'));
- userEvent.type(screen.getByLabelText('Formula'), 'x+1');
+ const formulaInput = screen.getByRole('textbox', { name: 'Formula' });
+ const applyButton = screen.getByRole('button', { name: 'Apply' });
+ const okButton = screen.getByRole('button', { name: 'OK' });
+
+ userEvent.type(formulaInput, 'x+1');
+ expect(formulaInput).toHaveValue('x+1');
await waitFor(() => {
- expect(screen.getByLabelText('Formula')).toHaveValue('x+1');
- expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
- expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
+ expect(okButton).toBeEnabled();
+ expect(applyButton).toBeEnabled();
});
- userEvent.clear(screen.getByLabelText('Formula'));
- userEvent.type(screen.getByLabelText('Formula'), 'y = x*2+1');
+ userEvent.clear(formulaInput);
await waitFor(() => {
- expect(screen.getByLabelText('Formula')).toHaveValue('y = x*2+1');
- expect(screen.getByRole('button', { name: 'OK' })).toBeEnabled();
- expect(screen.getByRole('button', { name: 'Apply' })).toBeEnabled();
+ expect(formulaInput).toHaveValue('');
+ });
+ userEvent.type(formulaInput, 'y = x*2+1');
+ expect(formulaInput).toHaveValue('y = x*2+1');
+ await waitFor(() => {
+ expect(okButton).toBeEnabled();
+ expect(applyButton).toBeEnabled();
});
- userEvent.clear(screen.getByLabelText('Formula'));
- userEvent.type(screen.getByLabelText('Formula'), 'y+1');
+ userEvent.clear(formulaInput);
+ await waitFor(() => {
+ expect(formulaInput).toHaveValue('');
+ });
+ userEvent.type(formulaInput, 'y+1');
+ expect(formulaInput).toHaveValue('y+1');
await waitFor(() => {
- expect(screen.getByLabelText('Formula')).toHaveValue('y+1');
- expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
- expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
+ expect(okButton).toBeDisabled();
+ expect(applyButton).toBeDisabled();
});
- userEvent.clear(screen.getByLabelText('Formula'));
- userEvent.type(screen.getByLabelText('Formula'), 'x+');
+ userEvent.clear(formulaInput);
await waitFor(() => {
- expect(screen.getByLabelText('Formula')).toHaveValue('x+');
- expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
- expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
+ expect(formulaInput).toHaveValue('');
+ });
+ userEvent.type(formulaInput, 'x+');
+ expect(formulaInput).toHaveValue('x+');
+ await waitFor(() => {
+ expect(okButton).toBeDisabled();
+ expect(applyButton).toBeDisabled();
});
- userEvent.clear(screen.getByLabelText('Formula'));
- userEvent.type(screen.getByLabelText('Formula'), 'y = z+1');
+ userEvent.clear(formulaInput);
+ await waitFor(() => {
+ expect(formulaInput).toHaveValue('');
+ });
+ userEvent.type(formulaInput, 'y = z+1');
+ expect(formulaInput).toHaveValue('y = z+1');
await waitFor(() => {
- expect(screen.getByLabelText('Formula')).toHaveValue('y = z+1');
- expect(screen.getByRole('button', { name: 'OK' })).toBeDisabled();
- expect(screen.getByRole('button', { name: 'Apply' })).toBeDisabled();
+ expect(okButton).toBeDisabled();
+ expect(applyButton).toBeDisabled();
});
});
(superset) 02/04: fix(AlertReports): defaulting grace period to undefined (#27552)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 0b9c0d97dc25744ddef68d9c00c6616a8f1d2aeb
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Mon Mar 25 12:12:01 2024 -0500
fix(AlertReports): defaulting grace period to undefined (#27552)
(cherry picked from commit 4fce940a9c3566c5dded68aa5cbba26fb562ae69)
---
superset-frontend/src/features/alerts/AlertReportModal.tsx | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index 39261543f7..8449024d13 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -903,7 +903,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
// Need to make sure grace period is not lower than TIMEOUT_MIN
if (value === 0) {
- updateAlertState(target.name, null);
+ updateAlertState(target.name, undefined);
} else {
updateAlertState(
target.name,
(superset) 01/04: fix(AlertReports): clearing custom_width when disabled (#27551)
Posted by mi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
michaelsmolina pushed a commit to branch 3.1
in repository https://gitbox.apache.org/repos/asf/superset.git
commit bc603db7d4a7787d6bcd31668777860595281f94
Author: Jack <41...@users.noreply.github.com>
AuthorDate: Fri Mar 22 13:49:18 2024 -0500
fix(AlertReports): clearing custom_width when disabled (#27551)
(cherry picked from commit 0f6e4041c73bcae931bac0a9daa1837beac5aaf6)
---
superset-frontend/src/features/alerts/AlertReportModal.tsx | 1 +
1 file changed, 1 insertion(+)
diff --git a/superset-frontend/src/features/alerts/AlertReportModal.tsx b/superset-frontend/src/features/alerts/AlertReportModal.tsx
index ae4feda653..39261543f7 100644
--- a/superset-frontend/src/features/alerts/AlertReportModal.tsx
+++ b/superset-frontend/src/features/alerts/AlertReportModal.tsx
@@ -628,6 +628,7 @@ const AlertReportModal: FunctionComponent<AlertReportModalProps> = ({
chart: contentType === 'chart' ? currentAlert?.chart?.value : null,
dashboard:
contentType === 'dashboard' ? currentAlert?.dashboard?.value : null,
+ custom_width: isScreenshot ? currentAlert?.custom_width : undefined,
database: currentAlert?.database?.value,
owners: (currentAlert?.owners || []).map(
owner => (owner as MetaObject).value || owner.id,