Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: [M3-6430] - Event entities should only be linked for true labels #9003

Merged
Show file tree
Hide file tree
Changes from 2 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/) and this p
### Fixed:
- Typesafety of the `<Select />` component #8986
- Clear the Kubernetes Delete Dialog when it is re-opened #9000
- Event entities should only be linked for true labels

### Tech Stories:
- MUIv5 Migration - Components > TagsInput, TagsPanel #8995
Expand Down
49 changes: 49 additions & 0 deletions packages/manager/src/eventMessageGenerator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import getEventMessage, {
eventMessageCreators,
safeSecondaryEntityLabel,
} from './eventMessageGenerator';
import { applyLinking } from './eventMessageGenerator';

beforeEach(() => {
jest.spyOn(console, 'error').mockImplementation(() => {});
Expand Down Expand Up @@ -87,4 +88,52 @@ describe('Event message generation', () => {
).not.toMatch('booted with');
});
});

describe('apply linking to labels', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for adding test coverage!

it('should return empty string if message is falsy', () => {
const mockEvent = {
action: 'create',
entity: null,
secondary_entity: null,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use the eventFactory from packages/manager/src/factories/events.ts to create the mockEvents so we don't have to type cast?

What can we do to avoid message as any?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

message is a required prop, which can't be null which is why it's casted. not particularly important to be very strict with the type here. That being said I refactored some of the unit to use the factory for events 👍

const message = null;
const result = applyLinking(mockEvent as Event, message as any);

expect(result).toEqual('');
});

it('should replace entity label with link if entity and link exist', () => {
const entity = { id: '123', label: 'foo', type: 'linode' };
const mockEvent = { action: 'create', entity, secondary_entity: null };
const message = 'created entity foo';
const link = '/linodes/123';
const result = applyLinking(mockEvent as any, message);

expect(result).toEqual(`created entity <a href="${link}">foo</a> `);
});

it('should replace secondary entity label with link if entity and link exist', () => {
jest.clearAllMocks();

const entity = { id: '123', label: 'foo', type: 'linode' };
const secondary_entity = { id: '456', label: 'bar' };
const mockEvent = { action: 'create', entity, secondary_entity };
const message = 'created secondary_entity for foo';
const link = '/linodes/123';
const result = applyLinking(mockEvent as any, message);

expect(result).toEqual(
`created secondary_entity for <a href="${link}">foo</a> `
);
});

it('should not replace entity label if label is inside backticks', () => {
const entity = { id: '123', label: 'foo' };
const mockEvent = { action: 'create', entity, secondary_entity: null };
const message = 'created `foo`';
const result = applyLinking(mockEvent as any, message);

expect(result).toEqual('created `foo`');
});
});
});
11 changes: 8 additions & 3 deletions packages/manager/src/eventMessageGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ function applyBolding(event: Event, message: string) {
return newMessage;
}

function applyLinking(event: Event, message: string) {
export function applyLinking(event: Event, message: string) {
if (!message) {
return '';
}
Expand All @@ -857,14 +857,19 @@ function applyLinking(event: Event, message: string) {
event.secondary_entity,
false
);

let newMessage = message;

if (event.entity && entityLinkTarget) {
const label = event.entity.label;
const nonTickedLabels = new RegExp(`(?<!\`)${label}`, 'g');

newMessage = newMessage.replace(
event.entity.label,
`<a href="${entityLinkTarget}">${event.entity.label}</a>`
nonTickedLabels,
`<a href="${entityLinkTarget}">${label}</a> `
);
}

if (event.secondary_entity && secondaryEntityLinkTarget) {
newMessage = newMessage.replace(
event.secondary_entity.label,
Expand Down