Skip to content

Commit

Permalink
M3-3842 Change: Update event toasts (#6069)
Browse files Browse the repository at this point in the history 
* M3-3842 Add toasts for clone/resize/etc. events

- Add handling for linode_clone to ToastNotification
- Update messaging in eventMessageGenerator for clone events
(using secondary entity)
- Hide (Unknown) display when duration of an event is null

* Add messages and toasts for resize and migration events

* Refactor toastnotifications to reduce boilerplate

* Fix tests

* Review feedback 1

- Fix copy errors in toast messages
- Simplify default logic
- Add volume label display for detach/attach toasts

* Review feedback 2
 Use safeSecondaryLabel to refactor messaging for linode clone events

* Fix typos (extra spaces)
  • Loading branch information
@Jskobos
Jskobos authored Feb 12, 2020
1 parent b8fec81 commit 1084e3a
Show file tree
Hide file tree
Showing 5 changed files with 161 additions and 164 deletions.
61 changes: 39 additions & 22 deletions packages/manager/src/eventMessageGenerator.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,11 +252,24 @@ export const eventMessageCreators: { [index: string]: CreatorsForStatus } = {
`Linode ${e.entity?.label ?? ''} has been booted (Lish initiated boot).`
},
linode_clone: {
scheduled: e => `Linode ${e.entity!.label} is scheduled to be cloned.`,
started: e => `Linode ${e.entity!.label} is being cloned.`,
failed: e => `Linode ${e.entity!.label} could not be cloned.`,
finished: e => `Linode ${e.entity!.label} has been cloned.`,
notification: e => `Linode ${e.entity!.label} is scheduled to be cloned.`
scheduled: e =>
`Linode ${e.entity?.label ??
''} is scheduled to be cloned${safeSecondaryEntityLabel(
e,
' to',
''
)}.`,
started: e =>
`Linode ${e.entity?.label ??
''} is being cloned${safeSecondaryEntityLabel(e, ' to', '')}.`,
failed: e =>
`Linode ${e.entity?.label ??
''} could not be cloned${safeSecondaryEntityLabel(e, ' to', '')}.`,
finished: e =>
`Linode ${e.entity?.label ??
''} has been cloned${safeSecondaryEntityLabel(e, ' to', '')}.`,
notification: e =>
`Linode ${e.entity?.label ?? ''} is scheduled to be cloned.`
},
linode_create: {
scheduled: e => `Linode ${e.entity!.label} is scheduled for creation.`,
Expand All @@ -277,10 +290,11 @@ export const eventMessageCreators: { [index: string]: CreatorsForStatus } = {
notification: e => `An IP was deleted from Linode ${e.entity!.id}`
},
linode_migrate: {
scheduled: e => `Linode ${e.entity!.label} is scheduled for migration.`,
started: e => `Linode ${e.entity!.label} is being migrated.`,
failed: e => `Migration failed for Linode ${e.entity!.label}.`,
finished: e => `Linode ${e.entity!.label} has been migrated.`
scheduled: e =>
`Linode ${e.entity?.label ?? ''} is scheduled for migration.`,
started: e => `Linode ${e.entity?.label ?? ''} is being migrated.`,
failed: e => `Migration failed for Linode ${e.entity?.label ?? ''}.`,
finished: e => `Linode ${e.entity?.label ?? ''} has been migrated.`
},
// This event type isn't currently being displayed, but I added a message here just in case.
linode_migrate_datacenter_create: {
Expand All @@ -289,22 +303,24 @@ export const eventMessageCreators: { [index: string]: CreatorsForStatus } = {
},
// These are the same as the messages for `linode_migrate`.
linode_migrate_datacenter: {
scheduled: e => `Linode ${e.entity!.label} is scheduled for migration.`,
started: e => `Linode ${e.entity!.label} is being migrated.`,
failed: e => `Migration failed for Linode ${e.entity!.label}.`,
finished: e => `Linode ${e.entity!.label} has been migrated.`
scheduled: e =>
`Linode ${e.entity?.label ?? ''} is scheduled for migration.`,
started: e => `Linode ${e.entity?.label ?? ''} is being migrated.`,
failed: e => `Migration failed for Linode ${e.entity?.label ?? ''}.`,
finished: e => `Linode ${e.entity?.label ?? ''} has been migrated.`
},
// This event type isn't currently being displayed, but I added a message here just in case.
linode_mutate_create: {
notification: e =>
`Upgrade for Linode ${e.entity!.label} has been initiated.`
},
linode_mutate: {
scheduled: e => `Linode ${e.entity!.label} is scheduled for an upgrade.`,
started: e => `Linode ${e.entity!.label} is being upgraded.`,
failed: e => `Linode ${e.entity!.label} could not be upgraded.`,
finished: e => `Linode ${e.entity!.label} has been upgraded.`,
notification: e => `Linode ${e.entity!.label} is being upgraded.`
scheduled: e =>
`Linode ${e.entity?.label ?? ''} is scheduled for an upgrade.`,
started: e => `Linode ${e.entity?.label ?? ''} is being upgraded.`,
failed: e => `Linode ${e.entity?.label ?? ''} could not be upgraded.`,
finished: e => `Linode ${e.entity?.label ?? ''} has been upgraded.`,
notification: e => `Linode ${e.entity?.label ?? ''} is being upgraded.`
},
linode_reboot: {
scheduled: e =>
Expand Down Expand Up @@ -344,10 +360,11 @@ export const eventMessageCreators: { [index: string]: CreatorsForStatus } = {
`Resize for Linode ${e.entity!.label} has been initiated.`
},
linode_resize: {
scheduled: e => `Linode ${e.entity!.label} is scheduled for resizing.`,
started: e => `Linode ${e.entity!.label} is resizing.`,
failed: e => `Linode ${e.entity!.label} could not be resized`,
finished: e => `Linode ${e.entity!.label} has been resized.`
scheduled: e =>
`Linode ${e.entity?.label ?? ''} is scheduled for resizing.`,
started: e => `Linode ${e.entity?.label ?? ''} is resizing.`,
failed: e => `Linode ${e.entity?.label ?? ''} could not be resized`,
finished: e => `Linode ${e.entity?.label ?? ''} has been resized.`
},
linode_shutdown: {
scheduled: e => `Linode ${e.entity!.label} is scheduled for shutdown.`,
Expand Down
253 changes: 116 additions & 137 deletions packages/manager/src/features/ToastNotifications/ToastNotifications.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Event, EventStatus } from 'linode-js-sdk/lib/account/types';
import { withSnackbar, WithSnackbarProps } from 'notistack';
import { pathOr } from 'ramda';
import { curry } from 'ramda';
import * as React from 'react';
import 'rxjs/add/operator/bufferTime';
import 'rxjs/add/operator/filter';
Expand All @@ -8,6 +9,41 @@ import 'rxjs/add/operator/merge';
import { Subscription } from 'rxjs/Subscription';
import { events$ } from 'src/events';

/**
* Boilerplate for sending a success toast
* when an event succeeds and a failure toast
* when it fails. Intended to be partially applied
* with the snackbar method and event status.
*
* Note: due to the nature of currying, if you want to
* omit the success or failure message, you must manually pass `undefined`
* as the final argument, otherwise you get a function that's expecting
* a failureMessage instead of the toasts you want.
*/
export const toastSuccessAndFailure = curry(
(
enqueueSnackbar: WithSnackbarProps['enqueueSnackbar'],
eventStatus: EventStatus,
successMessage: string | undefined,
failureMessage: string | undefined
) => {
if (
['finished', 'notification'].includes(eventStatus) &&
Boolean(successMessage)
) {
return enqueueSnackbar(successMessage, { variant: 'success' });
} else if (['failed'].includes(eventStatus) && Boolean(failureMessage)) {
return enqueueSnackbar(failureMessage, { variant: 'error' });
} else {
return;
}
}
);

export const getLabel = (event: Event) => event.entity?.label ?? '';
export const getSecondaryLabel = (event: Event) =>
event.secondary_entity?.label ?? '';

class ToastNotifications extends React.PureComponent<WithSnackbarProps, {}> {
subscription: Subscription;

Expand All @@ -16,143 +52,86 @@ class ToastNotifications extends React.PureComponent<WithSnackbarProps, {}> {
.filter(e => !e._initial)
.map(event => {
const { enqueueSnackbar } = this.props;

if (
event.action === 'volume_detach' &&
['finished', 'notification'].includes(event.status)
) {
return enqueueSnackbar(`Volume successfully detached.`, {
variant: 'success'
});
}

if (
event.action === 'volume_attach' &&
['finished', 'notification'].includes(event.status)
) {
return enqueueSnackbar(`Volume successfully attached.`, {
variant: 'success'
});
}

if (
event.action === 'volume_create' &&
['finished', 'notification'].includes(event.status)
) {
return enqueueSnackbar(`Volume successfully created.`, {
variant: 'success'
});
const _toast = toastSuccessAndFailure(enqueueSnackbar, event.status);
const label = getLabel(event);
const secondaryLabel = getSecondaryLabel(event);
switch (event.action) {
case 'volume_attach':
return _toast(
`Volume ${label} successfully attached.`,
`Error attaching Volume ${label}.`
);
case 'volume_detach':
return _toast(
`Volume ${label} successfully detached.`,
`Error detaching Volume ${label}.`
);
case 'volume_create':
return _toast(
`Volume ${label} successfully created.`,
`Error creating Volume ${label}.`
);
case 'volume_delete':
return _toast(
`Volume successfully deleted.`,
`Error deleting Volume.`
);
case 'disk_imagize':
return _toast(
`Image ${secondaryLabel} created successfully.`,
`Error creating Image ${secondaryLabel}.`
);
case 'image_delete':
return _toast(
`Image ${label} deleted successfully.`,
`Error deleting Image ${label}.`
);
case 'disk_delete':
return _toast(
`Disk ${secondaryLabel} deleted successfully.`,
`Unable to delete disk ${secondaryLabel} ${
label ? ` on ${label}` : ''
}. Is it attached to a configuration profile that is in use?`
);
case 'linode_snapshot':
return _toast(
undefined,
`There was an error creating a snapshot on Linode ${label}.`
);
/**
* These create/delete failures are hypothetical.
* We don't know if it's possible for these to fail,
* but are including handling to be safe.
*/
case 'linode_config_delete':
return _toast(
undefined,
`Error deleting config ${secondaryLabel}.`
);
case 'linode_config_create':
return _toast(
undefined,
`Error creating config ${secondaryLabel}.`
);
case 'linode_clone':
return _toast(
`Linode ${label} has been cloned successfully to ${secondaryLabel}.`,
`Error cloning Linode ${label}.`
);
case 'linode_migrate_datacenter':
case 'linode_migrate':
return _toast(
`Linode ${label} has been migrated successfully.`,
`Error migrating Linode ${label}.`
);
case 'linode_resize':
return _toast(
`Linode ${label} has been resized successfully.`,
`Error resizing Linode ${label}.`
);
default:
return;
}

if (
event.action === 'volume_delete' &&
['finished', 'notification'].includes(event.status)
) {
return enqueueSnackbar(`Volume successfully deleted.`, {
variant: 'success'
});
}

if (event.action === 'disk_imagize' && event.status === 'failed') {
return enqueueSnackbar(
`There was an error creating image ${event.secondary_entity
?.label ?? ''}.`,
{
variant: 'error'
}
);
}

if (event.action === 'image_delete' && event.status === 'failed') {
return enqueueSnackbar(
`There was an error deleting image ${event.entity?.label ?? ''}.`,
{
variant: 'error'
}
);
}

if (
event.action === 'disk_imagize' &&
['finished', 'notification'].includes(event.status)
) {
return enqueueSnackbar(
`Image ${event.secondary_entity?.label ??
''} created successfully.`,
{
variant: 'success'
}
);
}

if (
event.action === 'image_delete' &&
['finished', 'notification'].includes(event.status)
) {
return enqueueSnackbar(
`Image ${event.entity?.label + ' ' ?? ''}deleted successfully.`,
{
variant: 'success'
}
);
}

if (event.action === 'volume_create' && event.status === 'failed') {
return enqueueSnackbar(
`There was an error attaching volume ${event.entity &&
event.entity.label}.`,
{ variant: 'error' }
);
}

if (event.action === 'disk_delete' && event.status === 'failed') {
const label = pathOr('', ['secondary_entity', 'label'], event);
const linode = pathOr(false, ['entity', 'label'], event);
return enqueueSnackbar(
`Unable to delete disk ${label} ${
linode ? ` on ${linode}` : ''
}. Is it attached to a configuration profile that is in use?`,
{ variant: 'error' }
);
}

if (event.action === 'linode_snapshot' && event.status === 'failed') {
return enqueueSnackbar(
`There was an error creating a snapshot on Linode ${event.entity?.label}.`,
{ variant: 'error' }
);
}

/**
* These create/delete failures are hypothetical.
* We don't know if it's possible for these to fail,
* but are including handling to be safe.
*/
if (
event.action === 'linode_config_delete' &&
['failed'].includes(event.status)
) {
const entity = event.secondary_entity
? event.secondary_entity.label
: '';
return enqueueSnackbar(`Error deleting config ${entity}.`, {
variant: 'error'
});
}

if (
event.action === 'linode_config_create' &&
['failed'].includes(event.status)
) {
const entity = event.secondary_entity
? event.secondary_entity.label
: '';
return enqueueSnackbar(`Error creating config ${entity}.`, {
variant: 'error'
});
}

return;
})
/**
* In the somewhat unlikely scenario that we get a flood of events, we're
Expand Down
3 changes: 2 additions & 1 deletion packages/manager/src/features/linodes/LinodesDetail/LinodeSummary/ActivityRow.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export const ActivityRow: React.StatelessComponent<CombinedProps> = props => {
const { classes, event } = props;

const message = eventMessageGenerator(event);
const duration = formatEventSeconds(event.duration);

if (!message) {
return null;
Expand All @@ -58,7 +59,7 @@ export const ActivityRow: React.StatelessComponent<CombinedProps> = props => {
>
<Grid item>
<Typography>
{displayedMessage} ({formatEventSeconds(event.duration)})
{displayedMessage} {duration && `(${duration})`}
</Typography>
</Grid>
<Grid item>
Expand Down
6 changes: 3 additions & 3 deletions packages/manager/src/utilities/minute-conversion/minute-conversion.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ describe('Human-Readable Minute Conversion', () => {

describe('Event seconds conversion', () => {
it('should format event seconds correctly', () => {
expect(formatEventSeconds(null)).toBe('Unknown');
expect(formatEventSeconds(undefined as any)).toBe('Unknown');
expect(formatEventSeconds(0)).toBe('Unknown');
expect(formatEventSeconds(null)).toBe('');
expect(formatEventSeconds(undefined as any)).toBe('');
expect(formatEventSeconds(0)).toBe('');

expect(formatEventSeconds(3600)).toBe('1 hour');
expect(formatEventSeconds(3660)).toBe('1 hour, 1 minute');
Expand Down
Loading

0 comments on commit 1084e3a

Please sign in to comment.