Skip to content
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@
### Internal

- Fix `SentrySDK.internal.replay.replayId` returning nil for buffered replays (#8976)
- Mark the fabricated `mach` and `signal` crash mechanisms as `synthetic` so an Apple crash groups with the identical crash reported by the other Sentry SDKs, and so a mach-caught and a signal-caught report of the same bug no longer split into two issues (#8919)
- Set `mechanism.handled` to `false` on crash reports that carry no mach context, which previously left it unset (#8919)
- Prevent managed user feedback from submitting messages longer than 4096 Unicode scalars, and show the limit in the feedback form. (#8973)

## 9.27.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,18 @@ extension SentryUserFeedbackFormController: SentryUserFeedbackFormViewModelDeleg
}
}

guard case let SentryUserFeedbackFormViewModel.InputError.validationError(missing, _) = error,
let errorDescription = error.errorDescription else {
guard let errorDescription = error.errorDescription else {
SentrySDKLog.warning("Unexpected error type.")
presentAlert(message: config.formConfig.unexpectedErrorText, errorCode: 2, info: [NSLocalizedDescriptionKey: "Client error: ."])
return
}

presentAlert(message: errorDescription, errorCode: 1, info: ["missing_fields": missing, NSLocalizedDescriptionKey: "The user did not complete the feedback form."])
switch error {
case .validationError(let missing, _):
presentAlert(message: errorDescription, errorCode: 1, info: ["missing_fields": missing, NSLocalizedDescriptionKey: "The user did not complete the feedback form."])
case .messageTooLong:
presentAlert(message: errorDescription, errorCode: 1, info: [NSLocalizedDescriptionKey: errorDescription])
}
}
}

Comment thread
sentry[bot] marked this conversation as resolved.
Expand Down Expand Up @@ -313,6 +317,7 @@ extension SentryUserFeedbackFormController: UITextViewDelegate {
/// Updates validation state when the feedback message changes.
public func textViewDidChange(_ textView: UITextView) {
viewModel.messageTextViewPlaceholder.isHidden = textView.text != ""
viewModel.updateMessageCharacterCount()
viewModel.updateSubmitButtonAccessibilityHint()
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ protocol SentryUserFeedbackFormViewModelDelegate: NSObjectProtocol {

@objcMembers
@_spi(Private) public class SentryUserFeedbackFormViewModel: NSObject {
// The backend uses Python code-point length, which matches Swift Unicode scalars.
static let maxMessageLength = 4_096

let config: SentryUserFeedbackConfiguration
unowned let controller: SentryUserFeedbackFormController
weak var delegate: SentryUserFeedbackFormViewModelDelegate?
Expand Down Expand Up @@ -126,6 +129,17 @@ protocol SentryUserFeedbackFormViewModelDelegate: NSObjectProtocol {
textView.accessibilityIdentifier = "io.sentry.feedback.form.message"
return textView
}()

lazy var messageCharacterCountLabel = {
let label = UILabel(frame: .zero)
label.font = config.theme.scaledFont(style: .caption1)
label.adjustsFontForContentSizeCategory = true
label.textAlignment = .right
label.accessibilityIdentifier = "io.sentry.feedback.form.message-character-count"
label.accessibilityTraits.insert(.updatesFrequently)
updateMessageCharacterCount(label: label)
return label
}()

lazy var screenshotImageView = {
let iv = UIImageView()
Expand Down Expand Up @@ -227,6 +241,7 @@ protocol SentryUserFeedbackFormViewModelDelegate: NSObjectProtocol {

let messageAndScreenshotStack = UIStackView(arrangedSubviews: [
self.messageTextView,
self.messageCharacterCountLabel,
self.addScreenshotButton,
self.removeScreenshotStack
])
Expand Down Expand Up @@ -399,6 +414,17 @@ extension SentryUserFeedbackFormViewModel {
case .failure(let error): submitButton.accessibilityHint = error.errorDescription
}
}

func updateMessageCharacterCount() {
updateMessageCharacterCount(label: messageCharacterCountLabel)
}

private func updateMessageCharacterCount(label: UILabel) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

h: Our user feedback UI is standardized across SDKs, so we must do an internal alignment with other SDK maintainers if adding this footer caption label is fine.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sg, please let me know if anything needs to be changed. ty

let count = messageTextView.text?.unicodeScalars.count ?? 0
label.text = "\(count) / \(Self.maxMessageLength)"
label.accessibilityLabel = "\(count) of \(Self.maxMessageLength) characters used"
label.textColor = count > Self.maxMessageLength ? config.theme.errorColor : config.theme.foreground
}

func themeElements() {
[fullNameTextField, emailTextField].forEach {
Expand Down Expand Up @@ -499,7 +525,9 @@ extension SentryUserFeedbackFormViewModel {
}

// include the message they'll submit
var messageLength = 0
if let message = messageTextView.textOrNil {
messageLength = message.unicodeScalars.count
hint.append("with message: \(message)")
} else {
missing.append(config.formConfig.messageLabel.lowercased())
Comment thread
llirik0 marked this conversation as resolved.
Expand All @@ -510,17 +538,24 @@ extension SentryUserFeedbackFormViewModel {
let result = SentryUserFeedbackFormValidation.failure(InputError.validationError(missingFields: missing, localizedError: localizedError))
return result
}

guard messageLength <= Self.maxMessageLength else {
return .failure(.messageTooLong(maximumLength: Self.maxMessageLength))
}

return SentryUserFeedbackFormValidation.success(hint.joined(separator: " ").appending("."))
}

enum InputError: LocalizedError {
case validationError(missingFields: [String], localizedError: String)
case messageTooLong(maximumLength: Int)

var description: String {
switch self {
case .validationError(_, let localizedError):
return localizedError
case .messageTooLong(let maximumLength):
return "The description must not exceed \(maximumLength) characters."
}
}

Expand Down
158 changes: 158 additions & 0 deletions Tests/SentryTests/Integrations/Feedback/SentryFeedbackTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,164 @@ class SentryFeedbackTests: XCTestCase {
XCTAssertEqual(attachments[2].contentType, "video/mp4")
}

func testValidate_whenMessageExceedsMaximumLength_shouldReturnSpecificError() throws {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "a", count: 4_097)

// -- Act --
let result = sut.viewModel.validate()

// -- Assert --
guard case .failure(let error) = result else {
return XCTFail("Expected an over-limit message to fail validation.")
}
XCTAssertEqual(error.errorDescription, "The description must not exceed 4096 characters.")
}

func testValidate_whenMessageIsAtMaximumLength_shouldSucceed() {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "a", count: 4_096)

// -- Act --
let result = sut.viewModel.validate()

// -- Assert --
guard case .success = result else {
return XCTFail("Expected a message at the limit to validate.")
}
}

func testValidate_whenDecomposedMessageIsAtMaximumScalarLength_shouldSucceed() {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "e\u{301}", count: 2_048)

// -- Act --
let result = sut.viewModel.validate()

// -- Assert --
guard case .success = result else {
return XCTFail("Expected 4096 Unicode scalars to validate.")
}
}

func testValidate_whenDecomposedMessageExceedsMaximumScalarLength_shouldFail() throws {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "e\u{301}", count: 2_048) + "a"

// -- Act --
let result = sut.viewModel.validate()

// -- Assert --
guard case .failure(let error) = result else {
return XCTFail("Expected 4097 Unicode scalars to fail validation.")
}
XCTAssertEqual(error.errorDescription, "The description must not exceed 4096 characters.")
}

func testValidate_whenRequiredFieldsAreMissingAndMessageIsTooLong_shouldReportMissingFields() throws {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
config.formConfig.isNameRequired = true
config.formConfig.isEmailRequired = true
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "a", count: 4_097)

// -- Act --
let result = sut.viewModel.validate()

// -- Assert --
guard case .failure(let error) = result else {
return XCTFail("Expected missing required fields to fail validation.")
}
XCTAssertEqual(error.errorDescription, "You must provide all required information before submitting. Please check the following fields: name and email.")
}

func testMessageCharacterCount_whenTextChanges_shouldCountUnicodeScalars() {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = "e\u{301}"

// -- Act --
sut.textViewDidChange(sut.viewModel.messageTextView)

// -- Assert --
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.text, "2 / 4096")
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.accessibilityLabel, "2 of 4096 characters used")
XCTAssertTrue(sut.viewModel.messageCharacterCountLabel.accessibilityTraits.contains(.updatesFrequently))
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.textColor, config.theme.foreground)
}

func testMessageCharacterCount_whenMessageExceedsMaximumLength_shouldUseErrorColor() {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "a", count: 4_097)

// -- Act --
sut.textViewDidChange(sut.viewModel.messageTextView)

// -- Assert --
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.text, "4097 / 4096")
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.textColor, config.theme.errorColor)
}

func testMessageCharacterCount_whenTextIsNil_shouldShowZero() {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = nil

// -- Act --
sut.viewModel.updateMessageCharacterCount()

// -- Assert --
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.text, "0 / 4096")
}

func testMessageCharacterCount_whenFontFamilyConfigured_shouldUseThemeFont() {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
config.theme.fontFamily = "Helvetica"

// -- Act --
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)

// -- Assert --
XCTAssertEqual(sut.viewModel.messageCharacterCountLabel.font.familyName, "Helvetica")
}

#if !targetEnvironment(macCatalyst)
func testSubmitFeedback_whenMessageExceedsMaximumLength_shouldPresentSpecificError() throws {
// -- Arrange --
let config = SentryUserFeedbackConfiguration()
config.animations = false
let sut = SentryUserFeedbackFormController(preparedConfig: config, screenshot: nil)
sut.viewModel.messageTextView.text = String(repeating: "a", count: 4_097)
let window = UIWindow(windowScene: Self.mockWindowScene)
window.rootViewController = sut
window.makeKeyAndVisible()
addTeardownBlock { [window] in
window.isHidden = true
}

// -- Act --
sut.submitFeedback()

// -- Assert --
let alert = try XCTUnwrap(sut.presentedViewController as? UIAlertController)
XCTAssertEqual(alert.message, "The description must not exceed 4096 characters.")
}
#endif

private let inputCombinations: [FeedbackTestCase] = [
// base case: don't require name or email, don't input a name or email, don't input a message or screenshot
(config: (requiresName: false, requiresEmail: false, nameInput: nil, emailInput: nil, messageInput: nil, includeScreenshot: false), shouldValidate: false, expectedSubmitButtonAccessibilityHint: "You must provide all required information before submitting. Please check the following field: description."),
Expand Down