Skip to content

Commit c352c30

Browse files
fix(feedback): Align get user with JS implementation (#4901)
* fix(feedback): Align get user with JS implementation * Fix lint issue * Adds changelog * Optimise scope usage * Adds tests * Update CHANGELOG.md Co-authored-by: Krystof Woldrich <[email protected]> * Use current or isolation user is set without checks * Fix changelog --------- Co-authored-by: Krystof Woldrich <[email protected]>
1 parent f2c6fa5 commit c352c30

File tree

3 files changed

+106
-20
lines changed

3 files changed

+106
-20
lines changed

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@
66
> make sure you follow our [migration guide](https://docs.sentry.io/platforms/react-native/migration/) first.
77
<!-- prettier-ignore-end -->
88
9+
## Unreleased
10+
11+
### Fixes
12+
13+
- User set by `Sentry.setUser` is prefilled in Feedback Widget ([#4901](https://github.com/getsentry/sentry-react-native/pull/4901))
14+
- User data are considered from all scopes in the following order current, isolation and global.
15+
916
## 6.15.1
1017

1118
### Dependencies

packages/core/src/js/feedback/FeedbackWidget.tsx

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable max-lines */
2-
import type { SendFeedbackParams } from '@sentry/core';
3-
import { captureFeedback, getCurrentScope, lastEventId, logger } from '@sentry/core';
2+
import type { SendFeedbackParams, User } from '@sentry/core';
3+
import { captureFeedback, getCurrentScope, getGlobalScope, getIsolationScope, lastEventId, logger } from '@sentry/core';
44
import * as React from 'react';
55
import type { KeyboardTypeOptions ,
66
NativeEventSubscription} from 'react-native';
@@ -55,8 +55,8 @@ export class FeedbackWidget extends React.Component<FeedbackWidgetProps, Feedbac
5555

5656
const currentUser = {
5757
useSentryUser: {
58-
email: this.props?.useSentryUser?.email || getCurrentScope()?.getUser()?.email || '',
59-
name: this.props?.useSentryUser?.name || getCurrentScope()?.getUser()?.name || '',
58+
email: this.props?.useSentryUser?.email || this._getUser()?.email || '',
59+
name: this.props?.useSentryUser?.name || this._getUser()?.name || '',
6060
}
6161
}
6262

@@ -406,6 +406,18 @@ export class FeedbackWidget extends React.Component<FeedbackWidgetProps, Feedbac
406406
return this.state.filename !== undefined && this.state.attachment !== undefined && this.state.attachmentUri !== undefined;
407407
}
408408

409+
private _getUser = (): User | undefined => {
410+
const currentUser = getCurrentScope().getUser();
411+
if (currentUser) {
412+
return currentUser;
413+
}
414+
const isolationUser = getIsolationScope().getUser();
415+
if (isolationUser) {
416+
return isolationUser;
417+
}
418+
return getGlobalScope().getUser();
419+
}
420+
409421
private _showImageRetrievalDevelopmentNote = (): void => {
410422
if (isExpoGo()) {
411423
feedbackAlertDialog(

packages/core/test/feedback/FeedbackWidget.test.tsx

Lines changed: 83 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,18 +13,27 @@ const mockOnAddScreenshot = jest.fn();
1313
const mockOnSubmitSuccess = jest.fn();
1414
const mockOnFormSubmitted = jest.fn();
1515
const mockOnSubmitError = jest.fn();
16-
const mockGetUser = jest.fn(() => ({
16+
17+
const mockCurrentScopeGetUser = jest.fn(() => ({
1718
1819
name: 'Test User',
1920
}));
21+
const mockIsolationScopeGetUser = jest.fn();
22+
const mockGlobalScopeGetUser = jest.fn();
2023

2124
jest.spyOn(Alert, 'alert');
2225

2326
jest.mock('@sentry/core', () => ({
2427
...jest.requireActual('@sentry/core'),
2528
captureFeedback: jest.fn(),
2629
getCurrentScope: jest.fn(() => ({
27-
getUser: mockGetUser,
30+
getUser: mockCurrentScopeGetUser,
31+
})),
32+
getIsolationScope: jest.fn(() => ({
33+
getUser: mockIsolationScopeGetUser,
34+
})),
35+
getGlobalScope: jest.fn(() => ({
36+
getUser: mockGlobalScopeGetUser,
2837
})),
2938
lastEventId: jest.fn(),
3039
}));
@@ -99,6 +108,12 @@ const customStyles: FeedbackWidgetStyles = {
99108
};
100109

101110
describe('FeedbackWidget', () => {
111+
beforeEach(() => {
112+
mockIsolationScopeGetUser.mockReturnValue(undefined);
113+
mockGlobalScopeGetUser.mockReturnValue(undefined);
114+
FeedbackWidget.reset();
115+
});
116+
102117
afterEach(() => {
103118
jest.clearAllMocks();
104119
});
@@ -163,25 +178,77 @@ describe('FeedbackWidget', () => {
163178
expect(queryByTestId('sentry-logo')).toBeNull();
164179
});
165180

166-
it('name and email are prefilled when sentry user is set', () => {
167-
const { getByPlaceholderText } = render(<FeedbackWidget {...defaultProps} />);
181+
describe('User data prefilling', () => {
182+
const users = {
183+
prop: { name: 'Prop User', email: '[email protected]' },
184+
current: { name: 'Current User', email: '[email protected]' },
185+
isolation: { name: 'Isolation User', email: '[email protected]' },
186+
global: { name: 'Global User', email: '[email protected]' },
187+
};
188+
189+
it('prefills from useSentryUser prop when provided', () => {
190+
mockCurrentScopeGetUser.mockReturnValue(users.current);
191+
const { getByPlaceholderText } = render(
192+
<FeedbackWidget {...defaultProps} useSentryUser={users.prop} />,
193+
);
194+
expect(getByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe(users.prop.name);
195+
expect(getByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe(users.prop.email);
196+
});
168197

169-
const nameInput = getByPlaceholderText(defaultProps.namePlaceholder);
170-
const emailInput = getByPlaceholderText(defaultProps.emailPlaceholder);
198+
it('prefills from currentScope when useSentryUser prop is not set', () => {
199+
mockCurrentScopeGetUser.mockReturnValue(users.current);
200+
mockIsolationScopeGetUser.mockReturnValue(users.isolation);
201+
mockGlobalScopeGetUser.mockReturnValue(users.global);
202+
203+
const { getByPlaceholderText } = render(<FeedbackWidget {...defaultProps} />);
204+
expect(getByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe(users.current.name);
205+
expect(getByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe(users.current.email);
206+
});
171207

172-
expect(nameInput.props.value).toBe('Test User');
173-
expect(emailInput.props.value).toBe('[email protected]');
208+
it('prefills from isolationScope when useSentryUser prop and currentScope user are not set', () => {
209+
mockCurrentScopeGetUser.mockReturnValue(undefined);
210+
mockIsolationScopeGetUser.mockReturnValue(users.isolation);
211+
mockGlobalScopeGetUser.mockReturnValue(users.global);
212+
213+
const { getByPlaceholderText } = render(<FeedbackWidget {...defaultProps} />);
214+
expect(getByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe(users.isolation.name);
215+
expect(getByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe(users.isolation.email);
216+
});
217+
218+
it('prefills from globalScope when useSentryUser prop, currentScope, and isolationScope users are not set', () => {
219+
mockCurrentScopeGetUser.mockReturnValue(undefined);
220+
mockIsolationScopeGetUser.mockReturnValue(undefined);
221+
mockGlobalScopeGetUser.mockReturnValue(users.global);
222+
223+
const { getByPlaceholderText } = render(<FeedbackWidget {...defaultProps} />);
224+
expect(getByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe(users.global.name);
225+
expect(getByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe(users.global.email);
226+
});
227+
228+
it('prefills with empty strings if no user data is available from props or any scope', () => {
229+
mockCurrentScopeGetUser.mockReturnValue(undefined);
230+
mockIsolationScopeGetUser.mockReturnValue(undefined);
231+
mockGlobalScopeGetUser.mockReturnValue(undefined);
232+
233+
const { getByPlaceholderText } = render(<FeedbackWidget {...defaultProps} />);
234+
expect(getByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe('');
235+
expect(getByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe('');
236+
});
174237
});
175238

176-
it('ensure getUser is called only after the component is rendered', () => {
177-
// Ensure getUser is not called before render
178-
expect(mockGetUser).not.toHaveBeenCalled();
239+
it('ensure scope getUser methods are called during initialization when useSentryUser prop is not set', () => {
240+
// Ensure scope getUser methods are not called before render
241+
expect(mockCurrentScopeGetUser).not.toHaveBeenCalled();
242+
expect(mockIsolationScopeGetUser).not.toHaveBeenCalled();
243+
expect(mockGlobalScopeGetUser).not.toHaveBeenCalled();
179244

180245
// Render the component
181-
render(<FeedbackWidget />);
246+
render(<FeedbackWidget {...defaultProps} />);
182247

183-
// After rendering, check that getUser was called twice (email and name)
184-
expect(mockGetUser).toHaveBeenCalledTimes(2);
248+
// After rendering, _getUser is called twice (for name and email).
249+
expect(mockCurrentScopeGetUser).toHaveBeenCalledTimes(2);
250+
expect(mockIsolationScopeGetUser).toHaveBeenCalledTimes(2);
251+
expect(mockGlobalScopeGetUser).toHaveBeenCalledTimes(2);
185252
});
186253

187254
it('shows an error message if required fields are empty', async () => {
@@ -399,8 +466,8 @@ describe('FeedbackWidget', () => {
399466
unmount();
400467
const { queryByPlaceholderText } = render(<FeedbackWidget {...defaultProps} />);
401468

402-
expect(queryByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe('Test User');
403-
expect(queryByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe('[email protected]');
469+
expect(queryByPlaceholderText(defaultProps.namePlaceholder).props.value).toBe('');
470+
expect(queryByPlaceholderText(defaultProps.emailPlaceholder).props.value).toBe('');
404471
expect(queryByPlaceholderText(defaultProps.messagePlaceholder).props.value).toBe('');
405472
});
406473

0 commit comments

Comments
 (0)