riahk commented on a change in pull request #12081:
URL:
https://github.com/apache/incubator-superset/pull/12081#discussion_r545363680
##########
File path: superset-frontend/src/views/CRUD/alert/AlertList.tsx
##########
@@ -182,27 +184,33 @@ function AlertList({
row: {
original: { last_state: lastState },
},
- }: any) => <AlertStatusIcon state={lastState} />,
+ }: any) => (
+ <AlertStatusIcon
+ state={lastState}
+ isReportEnabled={isReportEnabled}
+ />
+ ),
accessor: 'last_state',
size: 'xs',
disableSortBy: true,
},
- {
- accessor: 'name',
- Header: t('Name'),
- },
{
Cell: ({
row: {
- original: { recipients },
+ original: { last_eval_dttm: lastEvalDttm },
},
- }: any) =>
- recipients.map((r: any) => (
- <RecipientIcon key={r.id} type={r.type} />
- )),
- accessor: 'recipients',
- Header: t('Notification Method'),
- disableSortBy: true,
+ }: any) => {
+ return lastEvalDttm
+ ? moment.utc(lastEvalDttm).local().format(DATETIME_WITH_TIME_ZONE)
Review comment:
Unrelated but we should update other lists where we use `moment` to
properly handle timezones
##########
File path: superset-frontend/src/views/CRUD/alert/components/AlertStatusIcon.tsx
##########
@@ -22,53 +22,87 @@ import { Tooltip } from 'src/common/components/Tooltip';
import Icon, { IconName } from 'src/components/Icon';
import { AlertState } from '../types';
-const StatusIcon = styled(Icon)<{ status: string }>`
- color: ${({ status, theme }) => {
+const StatusIcon = styled(Icon)<{ status: string; isReportEnabled: boolean }>`
+ color: ${({ status, theme, isReportEnabled }) => {
switch (status) {
case AlertState.working:
- return theme.colors.alert.base;
+ return theme.colors.primary.base;
case AlertState.error:
return theme.colors.error.base;
case AlertState.success:
+ return isReportEnabled
+ ? theme.colors.success.base
+ : theme.colors.alert.base;
+ case AlertState.noop:
return theme.colors.success.base;
+ case AlertState.grace:
+ return theme.colors.alert.base;
default:
return theme.colors.grayscale.base;
}
}};
`;
-export default function AlertStatusIcon({ state }: { state: string }) {
+export default function AlertStatusIcon({
+ state,
+ isReportEnabled = false,
+}: {
+ state: string;
+ isReportEnabled: boolean;
+}) {
const lastStateConfig = {
name: '',
label: '',
status: '',
};
switch (state) {
case AlertState.success:
- lastStateConfig.name = 'check';
- lastStateConfig.label = t(`${AlertState.success}`);
+ lastStateConfig.name = isReportEnabled ? 'check' : 'alert-solid-small';
+ lastStateConfig.label = isReportEnabled
+ ? t('Report Sent')
+ : t('Alert Triggered, Notification Sent');
lastStateConfig.status = AlertState.success;
break;
case AlertState.working:
- lastStateConfig.name = 'exclamation';
- lastStateConfig.label = t(`${AlertState.working}`);
+ lastStateConfig.name = 'running';
+ lastStateConfig.label = isReportEnabled
+ ? t('Report Sending')
+ : t('Alert Running');
lastStateConfig.status = AlertState.working;
break;
case AlertState.error:
lastStateConfig.name = 'x-small';
- lastStateConfig.label = t(`${AlertState.error}`);
+ lastStateConfig.label = isReportEnabled
+ ? t('Report Failed')
+ : t('Alert Failed');
lastStateConfig.status = AlertState.error;
break;
+ case AlertState.noop:
+ lastStateConfig.name = 'check';
+ lastStateConfig.label = t('Nothing Triggered');
+ lastStateConfig.status = AlertState.noop;
+ break;
+ case AlertState.grace:
+ lastStateConfig.name = 'alert-solid-small';
+ lastStateConfig.label = t('Alert Triggered, In Grace Period');
+ lastStateConfig.status = AlertState.grace;
+ break;
default:
- lastStateConfig.name = 'exclamation';
- lastStateConfig.label = t(`${AlertState.working}`);
- lastStateConfig.status = AlertState.working;
+ lastStateConfig.name = 'check';
+ lastStateConfig.label = t('Nothing Triggered');
+ lastStateConfig.status = AlertState.noop;
}
return (
<Tooltip title={lastStateConfig.label} placement="bottom">
<StatusIcon
name={lastStateConfig.name as IconName}
status={lastStateConfig.status}
+ isReportEnabled={isReportEnabled}
+ viewBox={
+ lastStateConfig.name === 'alert-solid-small'
+ ? '-6 -6 24 24'
+ : '0 0 24 24'
Review comment:
Why do we need to do this, anyway? The viewbox on that icon is `0 0 24
24`, is there something strange going on with the spacing?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
