You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/06/18 22:55:36 UTC
(superset) branch master updated: fix(alerts): don't show a never-run report as a green success (#29622) (#41121)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 3eae8cd6148 fix(alerts): don't show a never-run report as a green success (#29622) (#41121)
3eae8cd6148 is described below
commit 3eae8cd6148ffc45ceb63308977b9f8680d09056
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Thu Jun 18 15:55:30 2026 -0700
fix(alerts): don't show a never-run report as a green success (#29622) (#41121)
Co-authored-by: Claude Code <no...@anthropic.com>
---
.../alerts/components/AlertStatusIcon.test.tsx | 75 ++++++++++++++++++++++
.../features/alerts/components/AlertStatusIcon.tsx | 30 ++++-----
2 files changed, 90 insertions(+), 15 deletions(-)
diff --git a/superset-frontend/src/features/alerts/components/AlertStatusIcon.test.tsx b/superset-frontend/src/features/alerts/components/AlertStatusIcon.test.tsx
new file mode 100644
index 00000000000..9f552128029
--- /dev/null
+++ b/superset-frontend/src/features/alerts/components/AlertStatusIcon.test.tsx
@@ -0,0 +1,75 @@
+/**
+ * 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 userEvent from '@testing-library/user-event';
+import { render, screen } from 'spec/helpers/testing-library';
+import AlertStatusIcon from './AlertStatusIcon';
+import { AlertState } from '../types';
+
+const renderIcon = (state: string, isReportEnabled = false) =>
+ render(<AlertStatusIcon state={state} isReportEnabled={isReportEnabled} />);
+
+test('renders a check icon for the Success state', () => {
+ const { container } = renderIcon(AlertState.Success);
+ expect(container.querySelector('[data-icon="check"]')).toBeInTheDocument();
+});
+
+test('renders a close icon for the Error state', () => {
+ const { container } = renderIcon(AlertState.Error);
+ expect(container.querySelector('[data-icon="close"]')).toBeInTheDocument();
+});
+
+test('renders a warning icon for the Grace state', () => {
+ const { container } = renderIcon(AlertState.Grace);
+ expect(container.querySelector('[data-icon="warning"]')).toBeInTheDocument();
+});
+
+// Regression for issue #29622: a report that has never run has
+// last_state="Not triggered" (the backend default), which previously rendered
+// a green success check and looked like a successful run. It should now render
+// a neutral calendar icon instead.
+test('renders a neutral calendar icon (not a success check) for the Not triggered state', () => {
+ const { container } = renderIcon(AlertState.Noop);
+ expect(container.querySelector('[data-icon="calendar"]')).toBeInTheDocument();
+ expect(
+ container.querySelector('[data-icon="check"]'),
+ ).not.toBeInTheDocument();
+});
+
+test('renders the neutral calendar icon for an unknown/never-run state', () => {
+ const { container } = renderIcon('');
+ expect(container.querySelector('[data-icon="calendar"]')).toBeInTheDocument();
+ expect(
+ container.querySelector('[data-icon="check"]'),
+ ).not.toBeInTheDocument();
+});
+
+test('labels the Not triggered state "Report not yet run" for reports', async () => {
+ renderIcon(AlertState.Noop, /* isReportEnabled */ true);
+ userEvent.hover(screen.getByRole('img'));
+ expect(await screen.findByText('Report not yet run')).toBeInTheDocument();
+ // Guards the isReportEnabled split: before this fix the Noop label was
+ // unconditionally "Nothing triggered" for reports too.
+ expect(screen.queryByText('Nothing triggered')).not.toBeInTheDocument();
+});
+
+test('labels the Not triggered state "Nothing triggered" for alerts', async () => {
+ renderIcon(AlertState.Noop, /* isReportEnabled */ false);
+ userEvent.hover(screen.getByRole('img'));
+ expect(await screen.findByText('Nothing triggered')).toBeInTheDocument();
+});
diff --git a/superset-frontend/src/features/alerts/components/AlertStatusIcon.tsx b/superset-frontend/src/features/alerts/components/AlertStatusIcon.tsx
index 1d33fd2c6bb..e545ed028e3 100644
--- a/superset-frontend/src/features/alerts/components/AlertStatusIcon.tsx
+++ b/superset-frontend/src/features/alerts/components/AlertStatusIcon.tsx
@@ -22,11 +22,7 @@ import { Tooltip } from '@superset-ui/core/components';
import { Icons } from '@superset-ui/core/components/Icons';
import { AlertState } from '../types';
-function getStatusColor(
- status: string,
- isReportEnabled: boolean,
- theme: SupersetTheme,
-) {
+function getStatusColor(status: string, theme: SupersetTheme) {
switch (status) {
case AlertState.Working:
return theme.colorPrimaryText;
@@ -35,7 +31,12 @@ function getStatusColor(
case AlertState.Success:
return theme.colorSuccessText;
case AlertState.Noop:
- return theme.colorSuccessText;
+ // "Not triggered" is the default state for a report/alert that has not
+ // run yet (and for an alert that ran without its condition matching).
+ // Neither is a success, so use a neutral color instead of the green
+ // success color, which previously made an unexecuted report look like it
+ // had succeeded. See issue #29622.
+ return theme.colorTextSecondary;
case AlertState.Grace:
return theme.colorErrorText;
default:
@@ -51,6 +52,9 @@ export default function AlertStatusIcon({
isReportEnabled: boolean;
}) {
const theme = useTheme();
+ const noopLabel = isReportEnabled
+ ? t('Report not yet run')
+ : t('Nothing triggered');
const lastStateConfig = {
icon: Icons.CheckOutlined,
label: '',
@@ -79,8 +83,8 @@ export default function AlertStatusIcon({
lastStateConfig.status = AlertState.Error;
break;
case AlertState.Noop:
- lastStateConfig.icon = Icons.CheckOutlined;
- lastStateConfig.label = t('Nothing triggered');
+ lastStateConfig.icon = Icons.CalendarOutlined;
+ lastStateConfig.label = noopLabel;
lastStateConfig.status = AlertState.Noop;
break;
case AlertState.Grace:
@@ -89,8 +93,8 @@ export default function AlertStatusIcon({
lastStateConfig.status = AlertState.Grace;
break;
default:
- lastStateConfig.icon = Icons.CheckOutlined;
- lastStateConfig.label = t('Nothing triggered');
+ lastStateConfig.icon = Icons.CalendarOutlined;
+ lastStateConfig.label = noopLabel;
lastStateConfig.status = AlertState.Noop;
}
const Icon = lastStateConfig.icon;
@@ -111,11 +115,7 @@ export default function AlertStatusIcon({
>
<Icon
iconSize="m"
- iconColor={getStatusColor(
- lastStateConfig.status,
- isReportEnabled,
- theme,
- )}
+ iconColor={getStatusColor(lastStateConfig.status, theme)}
/>
</span>
</Tooltip>