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(feedback): last minute tweaks to apis to align implementation and docs #4873

Merged
merged 16 commits into from
Feb 22, 2025
Merged
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
5 changes: 5 additions & 0 deletions Samples/iOS-ObjectiveC/iOS-ObjectiveC/AppDelegate.m
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,14 @@ - (BOOL)application:(UIApplication *)application
uiForm.submitButtonLabel = @"Report that jank";
uiForm.messagePlaceholder
= @"Describe the nature of the jank. Its essence, if you will.";
uiForm.useSentryUser = YES;
};
config.configureTheme = ^(SentryUserFeedbackThemeConfiguration *_Nonnull theme) {
theme.font = [UIFont fontWithName:@"ChalkboardSE-Regular" size:25];
theme.outlineStyle =
[[SentryFormElementOutlineStyle alloc] initWithColor:UIColor.purpleColor
cornerRadius:10
outlineWidth:4];
};
config.onSubmitSuccess = ^(NSDictionary<NSString *, id> *_Nonnull info) {
NSString *name = info[@"name"] ?: @"$shakespearean_insult_name";
Expand Down
129 changes: 84 additions & 45 deletions Samples/iOS-Swift/iOS-Swift-UITests/UserFeedbackUITests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -123,25 +123,16 @@ extension UserFeedbackUITests {
try assertHookMarkersNotExist()

widgetButton.tap()

XCTAssert(nameField.waitForExistence(timeout: 1))
try assertOnlyHookMarkersExist(names: [.onFormOpen])

nameField.tap()
let testName = "Andrew"
nameField.typeText(testName)

emailField.tap()
let testEmail = "[email protected]"
emailField.typeText(testEmail)

messageTextView.tap()
let testMessage = "UITest user feedback"
messageTextView.typeText(testMessage)

fillInFields(testMessage, testName, testEmail)

sendButton.tap()

XCTAssert(widgetButton.waitForExistence(timeout: 1))
submit()

try assertOnlyHookMarkersExist(names: [.onFormClose, .onSubmitSuccess])
XCTAssertEqual(try dictionaryFromSuccessHookFile(), ["message": "UITest user feedback", "email": testEmail, "name": testName])
Expand All @@ -157,18 +148,7 @@ extension UserFeedbackUITests {

cancelButton.tap()

extrasAreaTabBarButton.tap()
app.buttons["io.sentry.ui-test.button.get-latest-envelope"].tap()
let marshaledDataBase64 = try XCTUnwrap(dataMarshalingField.value as? String)
let data = try XCTUnwrap(Data(base64Encoded: marshaledDataBase64))
let dict = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any])
XCTAssertEqual(try XCTUnwrap(dict["event_type"] as? String), "feedback")
XCTAssertEqual(try XCTUnwrap(dict["message"] as? String), testMessage)
XCTAssertEqual(try XCTUnwrap(dict["contact_email"] as? String), testEmail)
XCTAssertEqual(try XCTUnwrap(dict["source"] as? String), "widget")
XCTAssertEqual(try XCTUnwrap(dict["name"] as? String), testName)
XCTAssertNotNil(dict["event_id"])
XCTAssertEqual(try XCTUnwrap(dict["item_header_type"] as? String), "feedback")
try assertEnvelopeContents(testMessage, testEmail, testName)
}

func testSubmitFullyFilledForm() throws {
Expand All @@ -184,17 +164,13 @@ extension UserFeedbackUITests {
try assertHookMarkersNotExist()

widgetButton.tap()

XCTAssert(nameField.waitForExistence(timeout: 1))
try assertOnlyHookMarkersExist(names: [.onFormOpen])

messageTextView.tap()
let testMessage = "UITest user feedback"
messageTextView.typeText(testMessage)

sendButton.tap()
fillInFields(testMessage)

XCTAssert(widgetButton.waitForExistence(timeout: 1))
submit()

try assertOnlyHookMarkersExist(names: [.onFormClose, .onSubmitSuccess])
XCTAssertEqual(try dictionaryFromSuccessHookFile(), ["message": "UITest user feedback", "email": testContactEmail, "name": testName])
Expand Down Expand Up @@ -248,15 +224,13 @@ extension UserFeedbackUITests {
try assertHookMarkersNotExist()

widgetButton.tap()

XCTAssert(sendButton.waitForExistence(timeout: 1))
try assertOnlyHookMarkersExist(names: [.onFormOpen])

messageTextView.tap()
messageTextView.typeText("UITest user feedback")

sendButton.tap()

XCTAssert(widgetButton.waitForExistence(timeout: 1))
submit()

try assertOnlyHookMarkersExist(names: [.onFormClose, .onSubmitSuccess])
XCTAssertEqual(try dictionaryFromSuccessHookFile(), ["name": testName, "message": "UITest user feedback", "email": testContactEmail])
Expand All @@ -279,7 +253,7 @@ extension UserFeedbackUITests {
try assertHookMarkersNotExist()

widgetButton.tap()

XCTAssert(sendButton.waitForExistence(timeout: 1))
try assertOnlyHookMarkersExist(names: [.onFormOpen])

messageTextView.tap()
Expand Down Expand Up @@ -316,7 +290,7 @@ extension UserFeedbackUITests {
try assertHookMarkersNotExist()

widgetButton.tap()

XCTAssert(sendButton.waitForExistence(timeout: 1))
try assertOnlyHookMarkersExist(names: [.onFormOpen])

messageTextView.tap()
Expand Down Expand Up @@ -344,11 +318,30 @@ extension UserFeedbackUITests {

// MARK: Tests validating screenshot functionality

func testAddingAndRemovingScreenshots() {
func testAddingScreenshots() throws {
launchApp(args: ["--io.sentry.feedback.inject-screenshot"])
XCTAssert(removeScreenshotButton.isHittable)

let testMessage = "UITest user feedback"
fillInFields(testMessage)

submit()

try assertEnvelopeContents(testMessage, attachments: true)
}

func testAddingAndRemovingScreenshots() throws {
launchApp(args: ["--io.sentry.feedback.inject-screenshot"])
XCTAssert(removeScreenshotButton.isHittable)
removeScreenshotButton.tap()
XCTAssertFalse(removeScreenshotButton.isHittable)

let testMessage = "UITest user feedback"
fillInFields(testMessage)

submit()

try assertEnvelopeContents(testMessage)
}

// MARK: Tests validating error cases
Expand All @@ -361,7 +354,7 @@ extension UserFeedbackUITests {

widgetButton.tap()

sendButton.tap()
submit(expectingError: true)

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information before submitting. Please check the following field: description."].exists)
Expand All @@ -386,7 +379,7 @@ extension UserFeedbackUITests {
XCTAssertFalse(app.staticTexts["Thy name (Required)"].exists)
XCTAssert(app.staticTexts["Thy complaint (Required)"].exists)

sendButton.tap()
submit(expectingError: true)

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information before submitting. Please check the following fields: thine email and thy complaint."].exists)
Expand All @@ -413,7 +406,7 @@ extension UserFeedbackUITests {
XCTAssert(app.staticTexts["Thy name (Required)"].exists)
XCTAssert(app.staticTexts["Thy complaint (Required)"].exists)

sendButton.tap()
submit(expectingError: true)

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts.element(matching: NSPredicate(format: "label LIKE 'You must provide all required information before submitting. Please check the following fields: thy name, thine email and thy complaint.'")).exists)
Expand All @@ -432,7 +425,7 @@ extension UserFeedbackUITests {

widgetButton.tap()

sendButton.tap()
submit(expectingError: true)

XCTAssert(app.staticTexts["Error"].exists)
XCTAssert(app.staticTexts["You must provide all required information before submitting. Please check the following field: description."].exists)
Expand All @@ -457,7 +450,7 @@ extension UserFeedbackUITests {

widgetButton.tap()

sendButton.tap()
submit(expectingError: true)

XCTAssert(app.staticTexts["Error"].exists)
app.buttons["OK"].tap()
Expand All @@ -468,9 +461,7 @@ extension UserFeedbackUITests {
messageTextView.tap()
messageTextView.typeText("UITest user feedback")

sendButton.tap()

XCTAssert(widgetButton.waitForExistence(timeout: 1))
submit()

try assertOnlyHookMarkersExist(names: [.onFormClose, .onSubmitSuccess])
XCTAssertEqual(try dictionaryFromSuccessHookFile(), ["name": testName, "message": "UITest user feedback", "email": testContactEmail])
Expand Down Expand Up @@ -518,6 +509,13 @@ extension UserFeedbackUITests {

// MARK: Form hook test helpers
extension UserFeedbackUITests {
func submit(expectingError: Bool = false) {
sendButton.tap()
if !expectingError {
XCTAssert(widgetButton.waitForExistence(timeout: 1))
}
}

func path(for marker: HookMarkerFile) throws -> String {
let appSupportDirectory = try XCTUnwrap(appSupportDirectory)
return "\(appSupportDirectory)/io.sentry/feedback/\(marker.rawValue)"
Expand Down Expand Up @@ -561,6 +559,47 @@ extension UserFeedbackUITests {
app.buttons["io.sentry.ui-test.button.get-application-support-directory"].tap()
appSupportDirectory = try XCTUnwrap(dataMarshalingField.value as? String)
}

func fillInFields(_ testMessage: String, _ testName: String? = nil, _ testEmail: String? = nil) {
if let testName = testName {
nameField.tap()
nameField.typeText(testName)
}

if let testEmail = testEmail {
emailField.tap()
emailField.typeText(testEmail)
}

messageTextView.tap()
messageTextView.typeText(testMessage)
}

func assertEnvelopeContents(_ testMessage: String, _ testEmail: String? = nil, _ testName: String? = nil, attachments: Bool = false) throws {
extrasAreaTabBarButton.tap()
app.buttons["io.sentry.ui-test.button.get-latest-envelope"].tap()
let marshaledDataBase64 = try XCTUnwrap(dataMarshalingField.value as? String)
let data = try XCTUnwrap(Data(base64Encoded: marshaledDataBase64))
let dict = try XCTUnwrap(JSONSerialization.jsonObject(with: data) as? [String: Any])
XCTAssertEqual(try XCTUnwrap(dict["event_type"] as? String), "feedback")
XCTAssertEqual(try XCTUnwrap(dict["message"] as? String), testMessage)
if let testEmail = testEmail {
XCTAssertEqual(try XCTUnwrap(dict["contact_email"] as? String), testEmail)
}
XCTAssertEqual(try XCTUnwrap(dict["source"] as? String), "widget")
if let testName = testName {
XCTAssertEqual(try XCTUnwrap(dict["name"] as? String), testName)
}
XCTAssertNotNil(dict["event_id"])
XCTAssertEqual(try XCTUnwrap(dict["item_header_type"] as? String), "feedback")
if attachments {
XCTAssertNotNil(dict["feedback_attachments"])
let screenshotDataStrings = try XCTUnwrap(dict["feedback_attachments"] as? [String])
XCTAssertEqual(screenshotDataStrings.count, 1)
let screenshotDataString = try XCTUnwrap(screenshotDataStrings.first)
XCTAssertNotNil(UIImage(data: try XCTUnwrap(Data(base64Encoded: screenshotDataString))))
}
}
}

//swiftlint:enable file_length
9 changes: 0 additions & 9 deletions Samples/iOS-Swift/iOS-Swift/ErrorsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,6 @@ class ErrorsViewController: UIViewController {
// It contains all data but mutations only influence the event being sent
scope.setTag(value: "value", key: "myTag")
}

if !ProcessInfo.processInfo.arguments.contains("--io.sentry.feedback.auto-inject-widget") {
let alert = UIAlertController(title: "Uh-oh!", message: "There was an error. Would you like to tell us what happened?", preferredStyle: .alert)
alert.addAction(.init(title: "Yes", style: .default, handler: { _ in
SentrySDK.showUserFeedbackForm()
}))
alert.addAction(.init(title: "No", style: .cancel))
self.present(alert, animated: true)
}
}
}

Expand Down
24 changes: 20 additions & 4 deletions Samples/iOS-Swift/iOS-Swift/ExtraViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,14 @@
}

enum EnvelopeContent {
case image(Data)
/// String contents are base64 encoded image data
case image(String)

case rawText(String)
case json([String: Any])

/// String contents are base64 encoded image data
case feedbackAttachment(String)
}

func displayError(message: String) {
Expand Down Expand Up @@ -280,18 +285,29 @@
displayError(message: "\(envelopePath) had no contents.")
return nil
}
var waitingForFeedbackAttachment = false
let parsedEnvelopeContents = envelopeFileContents.split(separator: "\n").map { line in
if let imageData = Data(base64Encoded: String(line), options: []) {

Check warning on line 290 in Samples/iOS-Swift/iOS-Swift/ExtraViewController.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift6 Simulator

value 'imageData' was defined but never used; consider replacing with boolean test
return EnvelopeContent.image(imageData)
guard !waitingForFeedbackAttachment else {
waitingForFeedbackAttachment = false
return EnvelopeContent.feedbackAttachment(String(line))
}
return EnvelopeContent.image(String(line))
} else if let data = line.data(using: .utf8), let json = try? JSONSerialization.jsonObject(with: data) as? [String: Any] {
if let type = json["attachment_type"] as? String, type == "event.attachment" {
waitingForFeedbackAttachment = true
}
return EnvelopeContent.json(json)
} else {
return EnvelopeContent.rawText(String(line))
}
}
let contentsForUITest = parsedEnvelopeContents.reduce(into: [String: Any]()) { result, item in
if case let .json(json) = item {
insertValues(from: json, into: &result)
switch item {
case let .rawText(text): result["text"] = text
case let .image(base64Data): result["scope_images"] = (result["scope_images"] as? [String]) ?? [] + [base64Data]
case let .feedbackAttachment(base64Data): result["feedback_attachments"] = (result["feedback_attachments"] as? [String]) ?? [] + [base64Data]
case let .json(json): insertValues(from: json, into: &result)
}
}
guard let data = try? JSONSerialization.data(withJSONObject: contentsForUITest) else {
Expand Down
4 changes: 2 additions & 2 deletions Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
options.enableNetworkBreadcrumbs = enableNetworkBreadcrumbs
options.enableSwizzling = enableSwizzling
options.enableCrashHandler = enableCrashHandling
options.enableTracing = enableTracing

Check warning on line 57 in Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift

View workflow job for this annotation

GitHub Actions / UI Tests for iOS-Swift6 Simulator

'enableTracing' is deprecated: Use tracesSampleRate or tracesSampler instead
options.enablePersistingTracesWhenCrashing = true
options.attachScreenshot = enableAttachScreenshot
options.attachViewHierarchy = enableAttachViewHierarchy
Expand Down Expand Up @@ -153,6 +153,7 @@
}

func configureFeedbackForm(config: SentryUserFeedbackFormConfiguration) {
config.useSentryUser = !args.contains("--io.sentry.feedback.dont-use-sentry-user")
config.formTitle = "Jank Report"
config.isEmailRequired = args.contains("--io.sentry.feedback.require-email")
config.isNameRequired = args.contains("--io.sentry.feedback.require-name")
Expand Down Expand Up @@ -181,7 +182,7 @@
fontFamily = "ChalkboardSE-Regular"
}
config.fontFamily = fontFamily
config.outlineStyle = .init(outlineColor: .purple)
config.outlineStyle = .init(color: .purple)
config.foreground = .purple
config.background = .init(red: 0.95, green: 0.9, blue: 0.95, alpha: 1)
config.submitBackground = .orange
Expand All @@ -199,7 +200,6 @@
return
}

config.useSentryUser = !args.contains("--io.sentry.feedback.dont-use-sentry-user")
config.animations = !args.contains("--io.sentry.feedback.no-animations")
config.useShakeGesture = true
config.showFormForScreenshots = true
Expand Down
2 changes: 1 addition & 1 deletion Sources/Sentry/SentryClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -617,7 +617,7 @@
alwaysAttachStacktrace:NO];
SentryTraceContext *traceContext = [self getTraceStateWithEvent:preparedEvent withScope:scope];
NSArray *attachments = [[self attachmentsForEvent:preparedEvent scope:scope]
arrayByAddingObjectsFromArray:[feedback attachments]];
arrayByAddingObjectsFromArray:[feedback attachmentsForEnvelope]];

Check warning on line 620 in Sources/Sentry/SentryClient.m

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryClient.m#L620

Added line #L620 was not covered by tests

[self.transportAdapter sendEvent:preparedEvent
traceContext:traceContext
Expand Down
7 changes: 0 additions & 7 deletions Sources/Sentry/SentrySDK.m
Original file line number Diff line number Diff line change
Expand Up @@ -421,13 +421,6 @@ + (void)captureFeedback:(SentryFeedback *)feedback
[SentrySDK.currentHub captureFeedback:feedback];
}

#if TARGET_OS_IOS && SENTRY_HAS_UIKIT
+ (void)showUserFeedbackForm
{
// TODO: implement
}
#endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT

+ (void)addBreadcrumb:(SentryBreadcrumb *)crumb
{
[SentrySDK.currentHub addBreadcrumb:crumb];
Expand Down
Loading
Loading