Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions packages/api-v4/src/cloudpulse/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,13 @@ export interface Alert {
updated_by: string;
}

interface NotificationChannelAlerts {
id: number;
label: string;
interface NotificationChannelAlertInfo {
alert_count: number;
type: 'alerts-definitions';
url: string;
}
interface NotificationChannelBase {
alerts: NotificationChannelAlerts[];
alerts: NotificationChannelAlertInfo;
channel_type: ChannelType;
created: string;
created_by: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ const notificationChannels = notificationChannelFactory
.buildList(26)
.map((ch, i) => {
const isEmail = i % 2 === 0;
const alerts = Array.from({ length: isEmail ? 5 : 3 }).map((_, idx) => ({
id: idx + 1,
label: `Alert-${idx + 1}`,
const alerts = {
alert_count: isEmail ? 5 : 3,
url: `monitor/alert-channels/${i + 1}/alerts`,
type: 'alerts-definitions',
url: 'Sample',
}));
};

if (isEmail) {
return {
Expand Down Expand Up @@ -147,7 +146,7 @@ const VerifyChannelSortingParams = (
);

const order = sortOrderMap[sortOrder];
const orderBy = LabelLookup[columnLabel];
const orderBy = encodeURIComponent(LabelLookup[columnLabel]);

cy.url().should(
'endWith',
Expand Down Expand Up @@ -215,16 +214,7 @@ describe('Notification Channel Listing Page', () => {
}

// Alerts list
expect(item.alerts.length).to.eq(expected.alerts.length);

item.alerts.forEach((alert, aIndex) => {
const expAlert = expected.alerts[aIndex];

expect(alert.id).to.eq(expAlert.id);
expect(alert.label).to.eq(expAlert.label);
expect(alert.type).to.eq(expAlert.type);
expect(alert.url).to.eq(expAlert.url);
});
expect(item.alerts.alert_count).to.eq(expected.alerts.alert_count);
});
});
});
Expand Down Expand Up @@ -254,7 +244,9 @@ describe('Notification Channel Listing Page', () => {

cy.wrap($row).within(() => {
cy.findByText(expected.label).should('be.visible');
cy.findByText(String(expected.alerts.length)).should('be.visible');
cy.findByText(String(expected.alerts.alert_count)).should(
'be.visible'
);
cy.findByText('Email').should('be.visible');
cy.get('td').eq(3).should('have.text', expected.created_by);
cy.findByText(
Expand Down Expand Up @@ -284,11 +276,11 @@ describe('Notification Channel Listing Page', () => {
{
column: 'Alerts',
ascending: [...notificationChannels]
.sort((a, b) => a.alerts.length - b.alerts.length)
.sort((a, b) => a.alerts.alert_count - b.alerts.alert_count)
.map((ch) => ch.id),

descending: [...notificationChannels]
.sort((a, b) => b.alerts.length - a.alerts.length)
.sort((a, b) => b.alerts.alert_count - a.alerts.alert_count)
.map((ch) => ch.id),
},

Expand Down
13 changes: 5 additions & 8 deletions packages/manager/src/factories/cloudpulse/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,11 @@ import type { NotificationChannel } from '@linode/api-v4';

export const notificationChannelFactory =
Factory.Sync.makeFactory<NotificationChannel>({
alerts: [
{
id: Number(Factory.each((i) => i)),
label: String(Factory.each((id) => `Alert-${id}`)),
type: 'alerts-definitions',
url: 'Sample',
},
],
alerts: {
type: 'alerts-definitions',
alert_count: 1,
url: 'monitor/alert-channels/{id}/alerts',
},
channel_type: 'email',
content: {
email: {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { screen } from '@testing-library/react';
import * as React from 'react';

import { notificationChannelFactory } from 'src/factories/cloudpulse/channels';
Expand All @@ -21,4 +22,21 @@ describe('RenderChannelDetails component', () => {
expect(container.getByText(emailAddresses[0])).toBeVisible();
expect(container.getByText(emailAddresses[1])).toBeVisible();
});
it('should render the email channel with usernames if details is present', () => {
const usernames = ['user1', 'user2'];
const mockDataWithDetails: NotificationChannel =
notificationChannelFactory.build({
channel_type: 'email',
content: {},
details: {
email: {
usernames,
recipient_type: 'user',
},
},
});
renderWithTheme(<RenderChannelDetails template={mockDataWithDetails} />);
expect(screen.getByText(usernames[0])).toBeVisible();
expect(screen.getByText(usernames[1])).toBeVisible();
});
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Chip } from '@mui/material';
import * as React from 'react';

import { shouldUseContentsForEmail } from '../../Utils/utils';

import type { NotificationChannel } from '@linode/api-v4';

interface RenderChannelDetailProps {
Expand All @@ -12,9 +14,21 @@ interface RenderChannelDetailProps {
export const RenderChannelDetails = (props: RenderChannelDetailProps) => {
const { template } = props;
if (template.channel_type === 'email') {
return template.content?.email.email_addresses.map((value, index) => (
<Chip key={index} label={value} />
));
const contentEmail = template.content?.email;
const detailEmail = template.details?.email;
const useContents = shouldUseContentsForEmail(template);

const recipients = useContents
? (contentEmail?.email_addresses ?? [])
: (detailEmail?.usernames ?? []);

return (
<>
{recipients.map((value) => (
<Chip key={value} label={value} />
))}
</>
);
}
return null;
};
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import { NotificationChannelListTable } from './NotificationChannelListTable';

const mockScrollToElement = vi.fn();

const ALERT_TYPE = 'alerts-definitions';

describe('NotificationChannelListTable', () => {
it('should render the notification channel table headers', () => {
renderWithTheme(
Expand Down Expand Up @@ -127,11 +125,7 @@ describe('NotificationChannelListTable', () => {

it('should display correct alerts count', () => {
const channel = notificationChannelFactory.build({
alerts: [
{ id: 1, label: 'Alert 1', type: ALERT_TYPE, url: 'url1' },
{ id: 2, label: 'Alert 2', type: ALERT_TYPE, url: 'url2' },
{ id: 3, label: 'Alert 3', type: ALERT_TYPE, url: 'url3' },
],
alerts: { alert_count: 3 },
});

renderWithTheme(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ describe('NotificationChannelTableRow', () => {
it('should render a notification channel row with all fields', () => {
const updated = new Date().toISOString();
const channel = notificationChannelFactory.build({
alerts: [
{ id: 1, label: 'Alert 1', type: 'alerts-definitions', url: 'url1' },
{ id: 2, label: 'Alert 2', type: 'alerts-definitions', url: 'url2' },
],
alerts: {
type: 'alerts-definitions',
alert_count: 2,
url: 'monitor/alert-channels/{id}/alerts',
},
channel_type: 'email',
created_by: 'user1',
label: 'Test Channel',
Expand Down Expand Up @@ -142,7 +143,7 @@ describe('NotificationChannelTableRow', () => {

it('should render zero alerts count when no alerts are associated', () => {
const channel = notificationChannelFactory.build({
alerts: [],
alerts: { alert_count: 0 },
});

renderWithTheme(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export const NotificationChannelTableRow = (
{label}
</Link>
</TableCell>
<TableCell>{alerts.length}</TableCell>
<TableCell>{alerts.alert_count}</TableCell>
<TableCell>{channelTypeMap[channel_type]}</TableCell>
<TableCell>{created_by}</TableCell>
<TableCell>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { NotificationChannel } from '@linode/api-v4';

type ChannelListingTableLabel = {
colName: string;
label: keyof NotificationChannel;
label: `${keyof NotificationChannel}:${string}` | keyof NotificationChannel;
};

export const ChannelListingTableLabelMap: ChannelListingTableLabel[] = [
Expand All @@ -12,7 +12,7 @@ export const ChannelListingTableLabelMap: ChannelListingTableLabel[] = [
},
{
colName: 'Alerts',
label: 'alerts',
label: 'alerts:alert_count',
},
{
colName: 'Channel Type',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { regionFactory } from '@linode/utilities';
import { act, renderHook } from '@testing-library/react';

import { alertFactory, serviceTypesFactory } from 'src/factories';
import { alertFactory, notificationChannelFactory, serviceTypesFactory } from 'src/factories';

Check warning on line 4 in packages/manager/src/features/CloudPulse/Alerts/Utils/utils.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Replace `·alertFactory,·notificationChannelFactory,·serviceTypesFactory·` with `⏎··alertFactory,⏎··notificationChannelFactory,⏎··serviceTypesFactory,⏎` Raw Output: {"ruleId":"prettier/prettier","severity":1,"message":"Replace `·alertFactory,·notificationChannelFactory,·serviceTypesFactory·` with `⏎··alertFactory,⏎··notificationChannelFactory,⏎··serviceTypesFactory,⏎`","line":4,"column":9,"nodeType":null,"messageId":"replace","endLine":4,"endColumn":72,"fix":{"range":[118,181],"text":"

import { useContextualAlertsState } from '../../Utils/utils';
import { transformDimensionValue } from '../CreateAlert/Criteria/DimensionFilterValue/utils';
Expand All @@ -17,6 +17,7 @@
getSchemaWithEntityIdValidation,
getServiceTypeLabel,
handleMultipleError,
shouldUseContentsForEmail,
} from './utils';

import type { AlertValidationSchemaProps } from './utils';
Expand Down Expand Up @@ -497,3 +498,60 @@
).toBe('Test_value');
});
});

describe('shouldUseContentsForEmail', () => {
it('should return false for email channel with valid usernames in details', () => {
const notificationChannel = notificationChannelFactory.build({
channel_type: 'email',
details: {
email: {
usernames: ['user1', 'user2'],
},
},
});
expect(shouldUseContentsForEmail(notificationChannel)).toBe(false);
});

it('should return true for email channel with undefined details', () => {
const notificationChannel = notificationChannelFactory.build({
channel_type: 'email',
details: undefined,

Check warning on line 518 in packages/manager/src/features/CloudPulse/Alerts/Utils/utils.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Use null instead. Raw Output: {"ruleId":"sonarjs/no-undefined-assignment","severity":1,"message":"Use null instead.","line":518,"column":16,"nodeType":"Identifier","messageId":"useNull","endLine":518,"endColumn":25}
});
expect(shouldUseContentsForEmail(notificationChannel)).toBe(true);
});

it('should return true for email channel with undefined details.email', () => {
const notificationChannel = notificationChannelFactory.build({
channel_type: 'email',
details: {
email: undefined,

Check warning on line 527 in packages/manager/src/features/CloudPulse/Alerts/Utils/utils.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Use null instead. Raw Output: {"ruleId":"sonarjs/no-undefined-assignment","severity":1,"message":"Use null instead.","line":527,"column":16,"nodeType":"Identifier","messageId":"useNull","endLine":527,"endColumn":25}
},
});
expect(shouldUseContentsForEmail(notificationChannel)).toBe(true);
});

it('should return true for email channel with undefined usernames', () => {
const notificationChannel = notificationChannelFactory.build({
channel_type: 'email',
details: {
email: {
usernames: undefined,

Check warning on line 538 in packages/manager/src/features/CloudPulse/Alerts/Utils/utils.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Review (manager)

[eslint] reported by reviewdog 🐢 Use null instead. Raw Output: {"ruleId":"sonarjs/no-undefined-assignment","severity":1,"message":"Use null instead.","line":538,"column":22,"nodeType":"Identifier","messageId":"useNull","endLine":538,"endColumn":31}
},
},
});
expect(shouldUseContentsForEmail(notificationChannel)).toBe(true);
});

it('should return true for email channel with empty usernames array', () => {
const notificationChannel = notificationChannelFactory.build({
channel_type: 'email',
details: {
email: {
usernames: [],
recipient_type: 'admin_users',
},
},
});
expect(shouldUseContentsForEmail(notificationChannel)).toBe(true);
});
});
26 changes: 25 additions & 1 deletion packages/manager/src/features/CloudPulse/Alerts/Utils/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,23 @@ export const getAlertChipBorderRadius = (
return '0';
};

/**
* Determines whether to use details.email.usernames (newer API) or content.email.email_addresses (older API)
* for displaying email recipients in notification channels.
*
* @param channel The notification channel to check
* @returns true if we should use content.email.email_addresses, false if we should use details.email.usernames
*/
export const shouldUseContentsForEmail = (
channel: NotificationChannel
): boolean => {
// Use content if: details is missing, details is empty, details.email is empty or details.email.usernames is empty
return !(
channel.channel_type === 'email' && // ensuring it's an email channel to avoid the type error with email property
channel.details?.email?.usernames?.length
);
};

/**
* @param value The notification channel object for which we need to display the chips
* @returns The label and the values that needs to be displayed based on channel type
Expand All @@ -239,9 +256,16 @@ export const getChipLabels = (
value: NotificationChannel
): AlertDimensionsProp => {
if (value.channel_type === 'email') {
const contentEmail = value.content?.email;
const useContent = shouldUseContentsForEmail(value);

const recipients = useContent
? (contentEmail?.email_addresses ?? [])
: (value.details?.email?.usernames ?? []);

return {
label: 'To',
values: value.content?.email.email_addresses ?? [],
values: recipients,
};
} else if (value.channel_type === 'slack') {
return {
Expand Down
6 changes: 6 additions & 0 deletions packages/manager/src/mocks/serverHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3626,6 +3626,12 @@ export const handlers = [
updated: '2023-11-05T04:00:00',
updated_by: 'user3',
created_by: 'admin',
details: {
email: {
usernames: ['user1', 'user2'],
recipient_type: 'user',
},
},
})
);
notificationChannels.push(
Expand Down