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

(2.1) Handles Capture feedback form network connection issues #4364

Open
wants to merge 17 commits into
base: antonis/3859-newCaptureFeedbackAPI-Form
Choose a base branch
from
Open
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
31 changes: 19 additions & 12 deletions packages/core/src/js/feedback/FeedbackForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { sentryLogo } from './branding';
import { defaultConfiguration } from './defaults';
import defaultStyles from './FeedbackForm.styles';
import type { FeedbackFormProps, FeedbackFormState, FeedbackFormStyles,FeedbackGeneralConfiguration, FeedbackTextConfiguration } from './FeedbackForm.types';
import { checkInternetConnection, isValidEmail } from './utils';

let feedbackFormHandler: (() => void) | null = null;

Expand Down Expand Up @@ -82,7 +83,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor

public handleFeedbackSubmit: () => void = () => {
const { name, email, description } = this.state;
const { onFormClose } = this.props;
const { onSubmitSuccess, onSubmitError, onFormSubmitted } = this.props;
const text: FeedbackTextConfiguration = this.props;

const trimmedName = name?.trim();
Expand All @@ -94,7 +95,7 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
return;
}

if (this.props.shouldValidateEmail && (this.props.isEmailRequired || trimmedEmail.length > 0) && !this._isValidEmail(trimmedEmail)) {
if (this.props.shouldValidateEmail && (this.props.isEmailRequired || trimmedEmail.length > 0) && !isValidEmail(trimmedEmail)) {
Alert.alert(text.errorTitle, text.emailError);
return;
}
Expand All @@ -107,11 +108,22 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
associatedEventId: eventId,
};

onFormClose();
this.setState({ isVisible: false });

captureFeedback(userFeedback);
Alert.alert(text.successMessageText);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
checkInternetConnection(() => { // onConnected
this.setState({ isVisible: false });
captureFeedback(userFeedback);
onSubmitSuccess({ name: trimmedName, email: trimmedEmail, message: trimmedDescription, attachments: undefined });
Alert.alert(text.successMessageText);
onFormSubmitted();
}, () => { // onDisconnected
Alert.alert(text.errorTitle, text.networkError);
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
logger.error(`Feedback form submission failed: ${text.networkError}`);
}, () => { // onError
const errorString = `Feedback form submission failed: ${text.genericError}`;
onSubmitError(new Error(errorString));
Alert.alert(text.errorTitle, text.genericError);
logger.error(errorString);
});
lucas-zimerman marked this conversation as resolved.
Show resolved Hide resolved
};

/**
Expand Down Expand Up @@ -206,9 +218,4 @@ export class FeedbackForm extends React.Component<FeedbackFormProps, FeedbackFor
</SafeAreaView>
);
}

private _isValidEmail = (email: string): boolean => {
const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/
return emailRegex.test(email);
};
}
33 changes: 33 additions & 0 deletions packages/core/src/js/feedback/FeedbackForm.types.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { FeedbackFormData } from '@sentry/core';
import type { ImageStyle, TextStyle, ViewStyle } from 'react-native';

/**
Expand Down Expand Up @@ -126,16 +127,48 @@ export interface FeedbackTextConfiguration {
* The error message when the email is invalid
*/
emailError?: string;

/**
* Message when there is a network error
*/
networkError?: string;

/**
* Message when there is a network error
*/
genericError?: string;
}

/**
* The public callbacks available for the feedback integration
*/
export interface FeedbackCallbacks {
/**
* Callback when form is opened
*/
onFormOpen?: () => void;

/**
* Callback when form is closed and not submitted
*/
onFormClose?: () => void;

/**
* Callback when feedback is successfully submitted
*
* After this you'll see a SuccessMessage on the screen for a moment.
*/
onSubmitSuccess?: (data: FeedbackFormData) => void;

/**
* Callback when feedback is unsuccessfully submitted
*/
onSubmitError?: (error: Error) => void;

/**
* Callback when the feedback form is submitted successfully, and the SuccessMessage is complete, or dismissed
*/
onFormSubmitted?: () => void;
}

/**
Expand Down
21 changes: 21 additions & 0 deletions packages/core/src/js/feedback/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,14 @@ const ERROR_TITLE = 'Error';
const FORM_ERROR = 'Please fill out all required fields.';
const EMAIL_ERROR = 'Please enter a valid email address.';
const SUCCESS_MESSAGE_TEXT = 'Thank you for your report!';
const CONNECTIONS_ERROR_TEXT = 'Unable to send Feedback due to network issues.';
const GENERIC_ERROR_TEXT = 'Unable to send feedback due to an unexpected error.';

export const defaultConfiguration: Partial<FeedbackFormProps> = {
// FeedbackCallbacks
onFormOpen: () => {
// Does nothing by default
},
onFormClose: () => {
if (__DEV__) {
Alert.alert(
Expand All @@ -27,6 +32,20 @@ export const defaultConfiguration: Partial<FeedbackFormProps> = {
);
}
},
onSubmitSuccess: () => {
// Does nothing by default
},
onSubmitError: () => {
// Does nothing by default
},
onFormSubmitted: () => {
if (__DEV__) {
Alert.alert(
'Development note',
'onFormSubmitted callback is not implemented. By default the form is just unmounted.',
);
}
},

// FeedbackGeneralConfiguration
showBranding: true,
Expand All @@ -51,4 +70,6 @@ export const defaultConfiguration: Partial<FeedbackFormProps> = {
formError: FORM_ERROR,
emailError: EMAIL_ERROR,
successMessageText: SUCCESS_MESSAGE_TEXT,
networkError: CONNECTIONS_ERROR_TEXT,
genericError: GENERIC_ERROR_TEXT,
};
21 changes: 21 additions & 0 deletions packages/core/src/js/feedback/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
export const checkInternetConnection = async (
onConnected: () => void,
onDisconnected: () => void,
onError: () => void,
): Promise<void> => {
try {
const response = await fetch('https://sentry.io', { method: 'HEAD' });
if (response.ok) {
onConnected();
} else {
onDisconnected();
}
} catch (error) {
onError();
}
};

export const isValidEmail = (email: string): boolean => {
const emailRegex = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
return emailRegex.test(email);
};
91 changes: 89 additions & 2 deletions packages/core/test/feedback/FeedbackForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ import { Alert } from 'react-native';

import { FeedbackForm } from '../../src/js/feedback/FeedbackForm';
import type { FeedbackFormProps } from '../../src/js/feedback/FeedbackForm.types';
import { checkInternetConnection } from '../../src/js/feedback/utils';

const mockOnFormClose = jest.fn();
const mockOnSubmitSuccess = jest.fn();
const mockOnFormSubmitted = jest.fn();
const mockOnSubmitError = jest.fn();
const mockGetUser = jest.fn(() => ({
email: '[email protected]',
name: 'Test User',
Expand All @@ -15,15 +19,23 @@ const mockGetUser = jest.fn(() => ({
jest.spyOn(Alert, 'alert');

jest.mock('@sentry/core', () => ({
...jest.requireActual('@sentry/core'),
captureFeedback: jest.fn(),
getCurrentScope: jest.fn(() => ({
getUser: mockGetUser,
})),
lastEventId: jest.fn(),
}));
jest.mock('../../src/js/feedback/utils', () => ({
...jest.requireActual('../../src/js/feedback/utils'),
checkInternetConnection: jest.fn(),
}));

const defaultProps: FeedbackFormProps = {
onFormClose: mockOnFormClose,
onSubmitSuccess: mockOnSubmitSuccess,
onFormSubmitted: mockOnFormSubmitted,
onSubmitError: mockOnSubmitError,
formTitle: 'Feedback Form',
nameLabel: 'Name',
namePlaceholder: 'Name Placeholder',
Expand All @@ -38,9 +50,16 @@ const defaultProps: FeedbackFormProps = {
formError: 'Please fill out all required fields.',
emailError: 'The email address is not valid.',
successMessageText: 'Feedback success',
networkError: 'Network error',
genericError: 'Generic error',
};

describe('FeedbackForm', () => {
beforeEach(() => {
(checkInternetConnection as jest.Mock).mockImplementation((onConnected, _onDisconnected, _onError) => {
onConnected();
});
});
afterEach(() => {
jest.clearAllMocks();
});
Expand Down Expand Up @@ -144,7 +163,75 @@ describe('FeedbackForm', () => {
});
});

it('calls onFormClose when the form is submitted successfully', async () => {
it('shows an error message when there is no network connection', async () => {
(checkInternetConnection as jest.Mock).mockImplementationOnce((_onConnected, onDisconnected, _onError) => {
onDisconnected();
});

const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), '[email protected]');
fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');

fireEvent.press(getByText(defaultProps.submitButtonLabel));

await waitFor(() => {
expect(Alert.alert).toHaveBeenCalledWith(defaultProps.errorTitle, defaultProps.networkError);
});
});

it('shows an error message when there is a generic connection', async () => {
(checkInternetConnection as jest.Mock).mockImplementationOnce((_onConnected, _onDisconnected, onError) => {
onError();
});

const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), '[email protected]');
fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');

fireEvent.press(getByText(defaultProps.submitButtonLabel));

await waitFor(() => {
expect(Alert.alert).toHaveBeenCalledWith(defaultProps.errorTitle, defaultProps.genericError);
});
});

it('calls onSubmitError when there is an error', async () => {
(checkInternetConnection as jest.Mock).mockImplementationOnce((_onConnected, _onDisconnected, onError) => {
onError();
});

const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), '[email protected]');
fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');

fireEvent.press(getByText(defaultProps.submitButtonLabel));

await waitFor(() => {
expect(mockOnSubmitError).toHaveBeenCalled();
});
});

it('calls onSubmitSuccess when the form is submitted successfully', async () => {
const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
fireEvent.changeText(getByPlaceholderText(defaultProps.emailPlaceholder), '[email protected]');
fireEvent.changeText(getByPlaceholderText(defaultProps.messagePlaceholder), 'This is a feedback message.');

fireEvent.press(getByText(defaultProps.submitButtonLabel));

await waitFor(() => {
expect(mockOnSubmitSuccess).toHaveBeenCalled();
});
});

it('calls onFormSubmitted when the form is submitted successfully', async () => {
const { getByPlaceholderText, getByText } = render(<FeedbackForm {...defaultProps} />);

fireEvent.changeText(getByPlaceholderText(defaultProps.namePlaceholder), 'John Doe');
Expand All @@ -154,7 +241,7 @@ describe('FeedbackForm', () => {
fireEvent.press(getByText(defaultProps.submitButtonLabel));

await waitFor(() => {
expect(mockOnFormClose).toHaveBeenCalled();
expect(mockOnFormSubmitted).toHaveBeenCalled();
});
});

Expand Down
1 change: 1 addition & 0 deletions samples/react-native/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ const ErrorsTabNavigator = Sentry.withProfiler(
<FeedbackForm
{...props}
onFormClose={props.navigation.goBack}
onFormSubmitted={props.navigation.goBack}
styles={{
submitButton: {
backgroundColor: '#6a1b9a',
Expand Down
Loading