Skip to content

Commit 4acd201

Browse files
authored
fix(context): Address a few small context bugs (#72013)
Depends on #72008 to avoid merge conflicts. Fixes: - Empty metadata no longer annotates text. E.g. context with key `constructor` would return a meta of `{}` because `Object.constructor` always exists. Now it checks if that meta object is empty, rather than if it exists. - Custom context is no longer Title-Cased or Start Cased (to retain exactly what users send) - Unknown browsers from showing a (?) icon in context cards - Adds title to icons for legibility of smaller icons ![image](https://github.com/getsentry/sentry/assets/35509934/b0c35acc-168b-421e-97b9-c773e7528035) ![image](https://github.com/getsentry/sentry/assets/35509934/2b1b24f9-91cd-4479-9eb3-4e31746afe21) **todo** - [x] Fix any failing tests - [x] Add a fix for icon sizing (e.g. samsung, ios devices)
1 parent 41087cc commit 4acd201

File tree

6 files changed

+25
-18
lines changed

6 files changed

+25
-18
lines changed

static/app/components/events/contexts/contextCard.spec.tsx

+7-5
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
1-
import startCase from 'lodash/startCase';
21
import {EventFixture} from 'sentry-fixture/event';
32
import {GroupFixture} from 'sentry-fixture/group';
43
import {ProjectFixture} from 'sentry-fixture/project';
54

65
import {render, screen} from 'sentry-test/reactTestingLibrary';
76

87
import ContextCard from 'sentry/components/events/contexts/contextCard';
8+
import * as utils from 'sentry/components/events/contexts/utils';
99

1010
describe('ContextCard', function () {
1111
const group = GroupFixture();
@@ -42,7 +42,7 @@ describe('ContextCard', function () {
4242
/>
4343
);
4444

45-
expect(screen.getByText(startCase(alias))).toBeInTheDocument();
45+
expect(screen.getByText(alias)).toBeInTheDocument();
4646
Object.entries(simpleContext).forEach(([key, value]) => {
4747
expect(screen.getByText(key)).toBeInTheDocument();
4848
expect(screen.getByText(value)).toBeInTheDocument();
@@ -57,10 +57,11 @@ describe('ContextCard', function () {
5757

5858
it('renders with icons if able', function () {
5959
const event = EventFixture();
60+
const iconSpy = jest.spyOn(utils, 'getContextIcon');
6061

6162
const browserContext = {
6263
type: 'browser',
63-
name: 'Firefox',
64+
name: 'firefox',
6465
version: 'Infinity',
6566
};
6667
const browserCard = render(
@@ -73,7 +74,8 @@ describe('ContextCard', function () {
7374
project={project}
7475
/>
7576
);
76-
expect(screen.getByRole('img')).toBeInTheDocument();
77+
expect(iconSpy.mock.results[0].value.props.name).toBe('firefox');
78+
iconSpy.mockReset();
7779
browserCard.unmount();
7880

7981
const unknownContext = {
@@ -91,7 +93,7 @@ describe('ContextCard', function () {
9193
project={project}
9294
/>
9395
);
94-
expect(screen.queryByRole('img')).not.toBeInTheDocument();
96+
expect(iconSpy.mock.results[0].value).toBeUndefined();
9597
});
9698

9799
it('renders the annotated text and errors', function () {

static/app/components/events/contexts/contextIcon.tsx

+8-2
Original file line numberDiff line numberDiff line change
@@ -138,18 +138,24 @@ function getLogoImage(name: string) {
138138

139139
type Props = {
140140
name: string;
141+
hideUnknown?: boolean;
141142
size?: IconSize;
142143
};
143144

144-
function ContextIcon({name, size: providedSize = 'xl'}: Props) {
145+
function ContextIcon({name, size: providedSize = 'xl', hideUnknown = false}: Props) {
145146
const theme = useTheme();
146147
const size = theme.iconSizes[providedSize];
147148

148149
// Apply darkmode CSS to icon when in darkmode
149150
const isDarkmode = useLegacyStore(ConfigStore).theme === 'dark';
150151
const extraCass = isDarkmode && INVERT_IN_DARKMODE.includes(name) ? darkCss : null;
151152

152-
return <img height={size} width={size} css={extraCass} src={getLogoImage(name)} />;
153+
const imageName = getLogoImage(name);
154+
if (hideUnknown && imageName === logoUnknown) {
155+
return null;
156+
}
157+
158+
return <img height={size} width={size} css={extraCass} src={imageName} />;
153159
}
154160

155161
export default ContextIcon;

static/app/components/events/contexts/operatingSystem/index.spec.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ describe('operating system event context', function () {
4747
},
4848
});
4949

50-
expect(screen.getByText('Raw Description')).toBeInTheDocument(); // subject
50+
expect(screen.getByText('raw_description')).toBeInTheDocument(); // subject
5151
await userEvent.hover(screen.getByText(/redacted/));
5252
expect(
5353
await screen.findByText(

static/app/components/events/contexts/utils.spec.tsx

+2-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ describe('contexts utils', function () {
2424
const unknownData = getUnknownData({allData, knownKeys});
2525

2626
expect(unknownData).toEqual([
27-
{key: 'username', value: 'a', subject: 'Username', meta: undefined},
28-
{key: 'count', value: 1000, subject: 'Count', meta: undefined},
27+
{key: 'username', value: 'a', subject: 'username', meta: undefined},
28+
{key: 'count', value: 1000, subject: 'count', meta: undefined},
2929
]);
3030
});
3131
});

static/app/components/events/contexts/utils.tsx

+5-7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import {Fragment} from 'react';
22
import styled from '@emotion/styled';
3-
import startCase from 'lodash/startCase';
43
import moment from 'moment-timezone';
54

65
import UserAvatar from 'sentry/components/avatar/userAvatar';
@@ -19,7 +18,6 @@ import type {
1918
Project,
2019
} from 'sentry/types';
2120
import {defined} from 'sentry/utils';
22-
import {toTitleCase} from 'sentry/utils/string/toTitleCase';
2321

2422
import {AppEventContext, getKnownAppContextData, getUnknownAppContextData} from './app';
2523
import {
@@ -292,7 +290,7 @@ export function getUnknownData({
292290
.map(([key, value]) => ({
293291
key,
294292
value,
295-
subject: startCase(key),
293+
subject: key,
296294
meta: meta?.[key]?.[''],
297295
}));
298296
}
@@ -311,7 +309,7 @@ export function getContextTitle({
311309
}
312310

313311
if (!defined(type)) {
314-
return toTitleCase(alias);
312+
return alias;
315313
}
316314

317315
switch (type) {
@@ -346,10 +344,10 @@ export function getContextTitle({
346344
case 'laravel':
347345
return t('Laravel Context');
348346
default:
349-
return toTitleCase(alias);
347+
return alias;
350348
}
351349
default:
352-
return toTitleCase(type);
350+
return type;
353351
}
354352
}
355353

@@ -406,7 +404,7 @@ export function getContextIcon({
406404
if (iconName.length === 0) {
407405
return null;
408406
}
409-
return <ContextIcon name={iconName} size="sm" />;
407+
return <ContextIcon name={iconName} size="sm" hideUnknown />;
410408
}
411409

412410
export function getFormattedContextData({

static/app/components/events/meta/annotatedText/valueElement.tsx

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import {Fragment, isValidElement} from 'react';
22

33
import {t} from 'sentry/locale';
4+
import {isEmptyObject} from 'sentry/utils/object/isEmptyObject';
45

56
import {Redaction} from './redaction';
67

@@ -14,7 +15,7 @@ type Props = {
1415
// first place. It's much more likely that `withMeta` is buggy or improperly
1516
// used than that this component has a bug.
1617
export function ValueElement({value, meta}: Props) {
17-
if (!!value && meta) {
18+
if (!!value && !isEmptyObject(meta)) {
1819
return <Redaction>{value}</Redaction>;
1920
}
2021

0 commit comments

Comments
 (0)