From f84ce1c162caaa459bb846b86f019c284a7602e2 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Sun, 6 May 2018 13:21:32 -0700 Subject: [PATCH 1/9] Add delegate for implementing custom prompting behavior via the `ARKEmailBugReporterPromptingDelegate` protocol. Also fixes a bug where the email dialog isn't shown when no log messages are included in the report. Previously, this could happen when the log stores are empty or the `emailAttachmentAdditionsDelegate` returns false for all log stores. The custom prompt adds another case of this bug by offering users the chance to filter out all log store. --- Aardvark.xcodeproj/project.pbxproj | 8 + Aardvark/ARKEmailBugReportConfiguration.h | 44 +++ Aardvark/ARKEmailBugReportConfiguration.m | 26 ++ Aardvark/ARKEmailBugReporter.h | 18 ++ Aardvark/ARKEmailBugReporter.m | 317 ++++++++++++---------- Aardvark/Aardvark.h | 1 + 6 files changed, 276 insertions(+), 138 deletions(-) create mode 100644 Aardvark/ARKEmailBugReportConfiguration.h create mode 100644 Aardvark/ARKEmailBugReportConfiguration.m diff --git a/Aardvark.xcodeproj/project.pbxproj b/Aardvark.xcodeproj/project.pbxproj index 0f35ab26..dd811890 100644 --- a/Aardvark.xcodeproj/project.pbxproj +++ b/Aardvark.xcodeproj/project.pbxproj @@ -7,6 +7,8 @@ objects = { /* Begin PBXBuildFile section */ + 3D6E2D0E20868335007B8013 /* ARKEmailBugReportConfiguration.m in Sources */ = {isa = PBXBuildFile; fileRef = 3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */; }; + 3D6E2D0F20868335007B8013 /* ARKEmailBugReportConfiguration.h in Headers */ = {isa = PBXBuildFile; fileRef = 3D6E2D0D20868335007B8013 /* ARKEmailBugReportConfiguration.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3DED97142006D35B007FC95C /* ARKEmailAttachment.h in Headers */ = {isa = PBXBuildFile; fileRef = 3DED97122006D35B007FC95C /* ARKEmailAttachment.h */; }; 3DED97152006D35B007FC95C /* ARKEmailAttachment.m in Sources */ = {isa = PBXBuildFile; fileRef = 3DED97132006D35B007FC95C /* ARKEmailAttachment.m */; }; 4551A2D91BDAD10E00F216D0 /* Aardvark.h in Headers */ = {isa = PBXBuildFile; fileRef = EAD1442419E073FB0065A1FF /* Aardvark.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -94,6 +96,8 @@ /* End PBXContainerItemProxy section */ /* Begin PBXFileReference section */ + 3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = ARKEmailBugReportConfiguration.m; path = Aardvark/ARKEmailBugReportConfiguration.m; sourceTree = ""; }; + 3D6E2D0D20868335007B8013 /* ARKEmailBugReportConfiguration.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ARKEmailBugReportConfiguration.h; path = Aardvark/ARKEmailBugReportConfiguration.h; sourceTree = ""; }; 3DED97122006D35B007FC95C /* ARKEmailAttachment.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARKEmailAttachment.h; path = Aardvark/ARKEmailAttachment.h; sourceTree = ""; }; 3DED97132006D35B007FC95C /* ARKEmailAttachment.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = ARKEmailAttachment.m; path = Aardvark/ARKEmailAttachment.m; sourceTree = ""; }; 4551A2C21BDACF9000F216D0 /* Aardvark.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Aardvark.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -295,6 +299,8 @@ EA98B9311D4BEB6E00B3A390 /* ARKDefaultLogFormatter.h */, EA98B9321D4BEB6E00B3A390 /* ARKDefaultLogFormatter.m */, EA98B9331D4BEB6E00B3A390 /* ARKEmailBugReporter_Testing.h */, + 3D6E2D0D20868335007B8013 /* ARKEmailBugReportConfiguration.h */, + 3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */, EA98B9341D4BEB6E00B3A390 /* ARKEmailBugReporter.h */, EA98B9351D4BEB6E00B3A390 /* ARKEmailBugReporter.m */, 3DED97122006D35B007FC95C /* ARKEmailAttachment.h */, @@ -419,6 +425,7 @@ EA98B9161D4BEB4900B3A390 /* ARKIndividualLogViewController.h in Headers */, EA98B9181D4BEB4900B3A390 /* ARKLogTableViewController.h in Headers */, EA98B93A1D4BEB6E00B3A390 /* ARKDefaultLogFormatter.h in Headers */, + 3D6E2D0F20868335007B8013 /* ARKEmailBugReportConfiguration.h in Headers */, EA98B93E1D4BEB6E00B3A390 /* ARKEmailBugReporter_Testing.h in Headers */, EA98B9141D4BEB4100B3A390 /* ARKLogDistributor+UIAdditions.h in Headers */, EA98B9381D4BEB6E00B3A390 /* ARKBugReporter.h in Headers */, @@ -622,6 +629,7 @@ EA98B9121D4BEB3D00B3A390 /* ARKLogDistributor+UIAdditions.m in Sources */, EA98B94E1D4BF37000B3A390 /* UIApplication+ARKAdditions.swift in Sources */, EA98B9621D4BFA1700B3A390 /* Aardvark.swift in Sources */, + 3D6E2D0E20868335007B8013 /* ARKEmailBugReportConfiguration.m in Sources */, EA98B91A1D4BEB4900B3A390 /* ARKLogTableViewController.m in Sources */, ); runOnlyForDeploymentPostprocessing = 0; diff --git a/Aardvark/ARKEmailBugReportConfiguration.h b/Aardvark/ARKEmailBugReportConfiguration.h new file mode 100644 index 00000000..343f3e00 --- /dev/null +++ b/Aardvark/ARKEmailBugReportConfiguration.h @@ -0,0 +1,44 @@ +// +// ARKEmailBugReportConfiguration.h +// Aardvark +// +// Created by Nick Entin on 4/14/18. +// Copyright 2018 Square, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +//    http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#import + +@class ARKLogStore; +@class ARKEmailAttachment; + + +@interface ARKEmailBugReportConfiguration : NSObject + +/// The email subject that will be prefilled when the email dialog is presented to the user. +@property (nonnull, nonatomic, copy) NSString *prefilledEmailSubject; + +/// The log stores that will be included as attachments on the email. +@property (nonnull, nonatomic, copy) NSArray *logStores; + +/// Controls whether or not a screenshot should be attached to the email, when available. +@property (nonatomic) BOOL includesScreenshot; + +/// Controls whether or not a view hierarchy description should be attached to the email, when available. +@property (nonatomic) BOOL includesViewHierarchyDescription; + +/// Additional attachments to include on the email. +@property (nullable, nonatomic, copy) NSArray *additionalAttachments; + +@end diff --git a/Aardvark/ARKEmailBugReportConfiguration.m b/Aardvark/ARKEmailBugReportConfiguration.m new file mode 100644 index 00000000..5b2e00b4 --- /dev/null +++ b/Aardvark/ARKEmailBugReportConfiguration.m @@ -0,0 +1,26 @@ +// +// ARKEmailBugReportConfiguration.m +// Aardvark +// +// Created by Nick Entin on 4/14/18. +// Copyright 2018 Square, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +//    http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#import "ARKEmailBugReportConfiguration.h" + + +@implementation ARKEmailBugReportConfiguration + +@end diff --git a/Aardvark/ARKEmailBugReporter.h b/Aardvark/ARKEmailBugReporter.h index 6c5ddd42..36483749 100644 --- a/Aardvark/ARKEmailBugReporter.h +++ b/Aardvark/ARKEmailBugReporter.h @@ -25,6 +25,7 @@ @class ARKEmailAttachment; +@class ARKEmailBugReportConfiguration; @class ARKEmailBugReporter; @class ARKLogStore; @protocol ARKLogFormatter; @@ -53,6 +54,20 @@ @end +typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReportConfiguration *_Nullable configuration); + +@protocol ARKEmailBugReporterPromptingDelegate + +@required + +/// Called on the main thread when a bug is filed to signal that a bug report prompt should be presented to the user. +/// The `completion` should be called with either an updated configuration to present the email dialog, or `nil` to signal that the prompt was cancelled. +/// When the initial `configuration` has `includesScreenshot` or `includesViewHierarchyDescription` false, setting the field to true will have no effect. +- (void)showBugReportingPromptForConfiguration:(nonnull ARKEmailBugReportConfiguration *)configuration completion:(nonnull ARKEmailBugReporterCustomPromptCompletionBlock)completion; + +@end + + /// Composes a bug report that is sent via email. @interface ARKEmailBugReporter : NSObject @@ -73,6 +88,9 @@ /// The email attachment delegate, responsible for providing additional attachments and filtering which log stores to include in the bug report at the time the bug is filed. @property (nullable, nonatomic, weak) id emailAttachmentAdditionsDelegate; +/// The prompting delegate, responsible for showing a prompt to file a bug report. +@property (nullable, nonatomic, weak) id promptingDelegate; + /// The formatter used to prepare the log for entry into an email. Defaults to a vanilla instance of ARKDefaultLogFormatter. @property (nonnull, nonatomic) id logFormatter; diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index 46bcde64..501391fb 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -26,6 +26,7 @@ #import "AardvarkDefines.h" #import "ARKDefaultLogFormatter.h" #import "ARKEmailAttachment.h" +#import "ARKEmailBugReportConfiguration.h" #import "ARKScreenshotLogging.h" #import "ARKLogMessage.h" #import "ARKLogStore.h" @@ -133,7 +134,16 @@ - (void)_ARK_appendRecursiveViewControllerMappingToMapTable:(NSMapTable +@interface ARKDefaultPromptPresenter : NSObject + +@property (nonatomic) ARKEmailBugReportConfiguration *configuration; + +@property (nonatomic) ARKEmailBugReporterCustomPromptCompletionBlock completion; + +@end + + +@interface ARKEmailBugReporter () @property (nonatomic) UIView *screenFlashView; @@ -230,7 +240,7 @@ - (void)composeBugReportWithScreenshot:(BOOL)attachScreenshot; [self.screenFlashView.layer addAnimation:screenFlash forKey:ARKScreenshotFlashAnimationKey]; } else { - [self _showBugTitleCaptureAlert]; + [self _showBugReportPrompt]; } } @@ -273,7 +283,7 @@ - (void)animationDidStop:(CAAnimation *)animation finished:(BOOL)finished; [self.screenFlashView removeFromSuperview]; self.screenFlashView = nil; - [self _showBugTitleCaptureAlert]; + [self _showBugReportPrompt]; } #pragma mark - MFMailComposeViewControllerDelegate @@ -285,22 +295,6 @@ - (void)mailComposeController:(MFMailComposeViewController *)controller didFinis }]; } -#pragma mark - UIAlertViewDelegate - -- (void)alertView:(UIAlertView *)alertView didDismissWithButtonIndex:(NSInteger)buttonIndex; -{ - if (alertView.firstOtherButtonIndex == buttonIndex) { - NSString *bugTitle = [alertView textFieldAtIndex:0].text; - - [self _createBugReportWithTitle:bugTitle]; - } -} - -- (BOOL)alertViewShouldEnableFirstOtherButton:(UIAlertView *)alertView; -{ - return [alertView textFieldAtIndex:0].text.length > 0; -} - #pragma mark - Properties - (UIWindow *)emailComposeWindow; @@ -342,64 +336,20 @@ - (NSString *)formattedLogMessagesAttachmentExtension; #pragma mark - Private Methods -- (void)_stealFirstResponder; -{ - ARKInvisibleView *invisibleView = [ARKInvisibleView new]; - invisibleView.layer.opacity = 0.0; - [[UIApplication sharedApplication].keyWindow addSubview:invisibleView]; - [invisibleView becomeFirstResponder]; - [invisibleView removeFromSuperview]; -} - -- (void)_showBugTitleCaptureAlert; +- (void)_showBugReportPrompt; { - /* - iOS 8 often fails to transfer the keyboard from a focused text field to a UIAlertView's text field. - Transfer first responder to an invisble view when a debug screenshot is captured to make bug filing itself bug-free. - */ - [self _stealFirstResponder]; - - NSString * const title = NSLocalizedString(@"What Went Wrong?", @"Title text for alert asking user to describe a bug they just encountered"); - NSString * const message = NSLocalizedString(@"Please briefly summarize the issue you just encountered. You’ll be asked for more details later.", @"Subtitle text for alert asking user to describe a bug they just encountered"); - NSString * const composeReportButtonTitle = NSLocalizedString(@"Compose Report", @"Button title to compose bug report"); - NSString * const cancelButtonTitle = NSLocalizedString(@"Cancel", @"Button title to not compose a bug report"); - - UIAlertController *const alertController = [UIAlertController alertControllerWithTitle:title message:message preferredStyle:UIAlertControllerStyleAlert]; - - [alertController addAction:[UIAlertAction actionWithTitle:composeReportButtonTitle style:UIAlertActionStyleDefault handler:^(UIAlertAction *action) { - UITextField *textfield = [alertController.textFields firstObject]; - [self _createBugReportWithTitle:textfield.text]; - }]]; - - [alertController addAction:[UIAlertAction actionWithTitle:cancelButtonTitle style:UIAlertActionStyleDefault handler:NULL]]; - - [alertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { - [self _configureAlertTextfield:textField]; + id const promptPresenter = (self.promptingDelegate ?: [ARKDefaultPromptPresenter new]); + [promptPresenter showBugReportingPromptForConfiguration:[self _configurationWithCurrentSettings] completion:^(ARKEmailBugReportConfiguration * _Nullable configuration) { + if (configuration != nil) { + [self _createBugReportWithConfiguration:configuration]; + } }]; - - UIViewController *viewControllerToPresentAlertController = [UIApplication sharedApplication].keyWindow.rootViewController; - while (viewControllerToPresentAlertController.presentedViewController != nil) { - viewControllerToPresentAlertController = viewControllerToPresentAlertController.presentedViewController; - } - - /* - Disabling animations here to avoid potential crashes resulting from unexpected view state in UIKit - */ - [viewControllerToPresentAlertController presentViewController:alertController animated:NO completion:NULL]; - } -- (void)_configureAlertTextfield:(UITextField *)textField +- (ARKEmailBugReportConfiguration *)_configurationWithCurrentSettings; { - textField.autocapitalizationType = UITextAutocapitalizationTypeSentences; - textField.autocorrectionType = UITextAutocorrectionTypeYes; - textField.spellCheckingType = UITextSpellCheckingTypeYes; - textField.returnKeyType = UIReturnKeyDone; -} - -- (void)_createBugReportWithTitle:(NSString *)title; -{ - NSArray *logStores; + ARKEmailBugReportConfiguration *const configuration = [[ARKEmailBugReportConfiguration alloc] init]; + if (self.emailAttachmentAdditionsDelegate != nil) { NSMutableArray *const filteredLogStores = [NSMutableArray arrayWithCapacity:self.logStores.count]; for (ARKLogStore *logStore in self.logStores) { @@ -407,11 +357,22 @@ - (void)_createBugReportWithTitle:(NSString *)title; [filteredLogStores addObject:logStore]; } } - logStores = filteredLogStores; + configuration.logStores = filteredLogStores; + + configuration.additionalAttachments = [self.emailAttachmentAdditionsDelegate additionalEmailAttachmentsForEmailBugReporter:self]; + } else { - logStores = [self.logStores copy]; + configuration.logStores = [self.logStores copy]; } + configuration.includesScreenshot = self.attachScreenshotToNextBugReport; + configuration.includesViewHierarchyDescription = (self.attachScreenshotToNextBugReport && self.attachesViewHierarchyDescriptionWithScreenshot); + + return configuration; +} + +- (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)configuration; +{ NSMapTable *logStoresToLogMessagesMap = [NSMapTable new]; NSDictionary *emailBodyAdditions = [self.emailBodyAdditionsDelegate emailBodyAdditionsForEmailBugReporter:self]; @@ -419,78 +380,84 @@ - (void)_createBugReportWithTitle:(NSString *)title; self.mailComposeViewController = [MFMailComposeViewController new]; [self.mailComposeViewController setToRecipients:@[self.bugReportRecipientEmailAddress]]; - [self.mailComposeViewController setSubject:title]; + [self.mailComposeViewController setSubject:configuration.prefilledEmailSubject]; + dispatch_group_t logStoreRetrievalDispatchGroup = dispatch_group_create(); + dispatch_group_enter(logStoreRetrievalDispatchGroup); + + NSArray *const logStores = configuration.logStores; for (ARKLogStore *logStore in logStores) { + dispatch_group_enter(logStoreRetrievalDispatchGroup); [logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) { [logStoresToLogMessagesMap setObject:logMessages forKey:logStore]; + dispatch_group_leave(logStoreRetrievalDispatchGroup); + }]; + } + + // Once all log messages have been retrieved, attach the data and show the compose window. + dispatch_group_notify(logStoreRetrievalDispatchGroup, dispatch_get_main_queue(), ^{ + NSMutableString *emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; + + for (ARKLogStore *logStore in logStores) { + NSArray *logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; - // Only attach data once all log messages have been retrieved. - if (logStoresToLogMessagesMap.count == logStores.count) { - NSMutableString *emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; - - for (ARKLogStore *logStore in logStores) { - NSArray *logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; - - NSString *screenshotFileName = [NSLocalizedString(@"screenshot", @"File name of a screenshot") stringByAppendingPathExtension:@"png"]; - NSString *logsFileName = [NSLocalizedString(@"logs", @"File name for logs attachments") stringByAppendingPathExtension:[self formattedLogMessagesAttachmentExtension]]; - NSString *viewHierarchyFileName = [NSLocalizedString(@"view_hierarchy", @"File name for view hierarchy attachment") stringByAppendingPathExtension:@"txt"]; - NSMutableString *emailBodyForLogStore = [NSMutableString new]; - BOOL appendToEmailBody = NO; - - if (logStore.name.length) { - [emailBodyForLogStore appendFormat:@"%@:\n", logStore.name]; - screenshotFileName = [logStore.name stringByAppendingFormat:@"_%@", screenshotFileName]; - logsFileName = [logStore.name stringByAppendingFormat:@"_%@", logsFileName]; - } - - NSString *recentErrorLogs = [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreAvailable]; - if (recentErrorLogs.length) { - [emailBodyForLogStore appendFormat:@"%@\n", recentErrorLogs]; - appendToEmailBody = YES; - } - - if (appendToEmailBody) { - [emailBody appendString:emailBodyForLogStore]; - } - - - if (self.attachScreenshotToNextBugReport) { - NSData *const mostRecentImage = [self _mostRecentImageAsPNG:logMessages]; - if (mostRecentImage.length > 0) { - [self.mailComposeViewController addAttachmentData:mostRecentImage mimeType:@"image/png" fileName:screenshotFileName]; - } - } - - if (self.viewHierarchyDescription != nil) { - NSData *const viewHierarchyData = [self.viewHierarchyDescription dataUsingEncoding:NSUTF8StringEncoding]; - if (viewHierarchyData.length > 0) { - [self.mailComposeViewController addAttachmentData:viewHierarchyData mimeType:@"text/plain" fileName:viewHierarchyFileName]; - } - self.viewHierarchyDescription = nil; - } - - NSData *formattedLogs = [self formattedLogMessagesAsData:logMessages]; - if (formattedLogs.length) { - [self.mailComposeViewController addAttachmentData:formattedLogs mimeType:[self formattedLogMessagesDataMIMEType] fileName:logsFileName]; - } - } - - if (self.emailAttachmentAdditionsDelegate != nil) { - NSArray *const additionalAttachments = [self.emailAttachmentAdditionsDelegate additionalEmailAttachmentsForEmailBugReporter:self]; - for (ARKEmailAttachment *attachment in additionalAttachments) { - [self.mailComposeViewController addAttachmentData:attachment.data mimeType:attachment.dataMIMEType fileName:attachment.fileName]; - } + NSString *screenshotFileName = [NSLocalizedString(@"screenshot", @"File name of a screenshot") stringByAppendingPathExtension:@"png"]; + NSString *logsFileName = [NSLocalizedString(@"logs", @"File name for logs attachments") stringByAppendingPathExtension:[self formattedLogMessagesAttachmentExtension]]; + NSMutableString *emailBodyForLogStore = [NSMutableString new]; + BOOL appendToEmailBody = NO; + + if (logStore.name.length) { + [emailBodyForLogStore appendFormat:@"%@:\n", logStore.name]; + screenshotFileName = [logStore.name stringByAppendingFormat:@"_%@", screenshotFileName]; + logsFileName = [logStore.name stringByAppendingFormat:@"_%@", logsFileName]; + } + + NSString *recentErrorLogs = [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreAvailable]; + if (recentErrorLogs.length) { + [emailBodyForLogStore appendFormat:@"%@\n", recentErrorLogs]; + appendToEmailBody = YES; + } + + if (appendToEmailBody) { + [emailBody appendString:emailBodyForLogStore]; + } + + + if (configuration.includesScreenshot && self.attachScreenshotToNextBugReport) { + NSData *const mostRecentImage = [self _mostRecentImageAsPNG:logMessages]; + if (mostRecentImage.length > 0) { + [self.mailComposeViewController addAttachmentData:mostRecentImage mimeType:@"image/png" fileName:screenshotFileName]; } - - [self.mailComposeViewController setMessageBody:emailBody isHTML:NO]; - self.mailComposeViewController.mailComposeDelegate = self; - [self _showEmailComposeWindow]; } - }]; - } + + NSData *formattedLogs = [self formattedLogMessagesAsData:logMessages]; + if (formattedLogs.length) { + [self.mailComposeViewController addAttachmentData:formattedLogs mimeType:[self formattedLogMessagesDataMIMEType] fileName:logsFileName]; + } + } + + if (configuration.includesViewHierarchyDescription && self.viewHierarchyDescription != nil) { + NSString *viewHierarchyFileName = [NSLocalizedString(@"view_hierarchy", @"File name for view hierarchy attachment") stringByAppendingPathExtension:@"txt"]; + NSData *const viewHierarchyData = [self.viewHierarchyDescription dataUsingEncoding:NSUTF8StringEncoding]; + if (viewHierarchyData.length > 0) { + [self.mailComposeViewController addAttachmentData:viewHierarchyData mimeType:@"text/plain" fileName:viewHierarchyFileName]; + } + } + self.viewHierarchyDescription = nil; + + for (ARKEmailAttachment *attachment in configuration.additionalAttachments) { + [self.mailComposeViewController addAttachmentData:attachment.data mimeType:attachment.dataMIMEType fileName:attachment.fileName]; + } + + [self.mailComposeViewController setMessageBody:emailBody isHTML:NO]; + self.mailComposeViewController.mailComposeDelegate = self; + [self _showEmailComposeWindow]; + }); + + dispatch_group_leave(logStoreRetrievalDispatchGroup); } else { + NSArray *const logStores = configuration.logStores; for (ARKLogStore *logStore in logStores) { [logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) { [logStoresToLogMessagesMap setObject:logMessages forKey:logStore]; @@ -504,7 +471,7 @@ - (void)_createBugReportWithTitle:(NSString *)title; [emailBody appendFormat:@"%@\n", [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreUnavailable]]; } - NSURL *const composeEmailURL = [self _emailURLWithRecipients:@[self.bugReportRecipientEmailAddress] CC:@"" subject:title body:emailBody]; + NSURL *const composeEmailURL = [self _emailURLWithRecipients:@[self.bugReportRecipientEmailAddress] CC:@"" subject:configuration.prefilledEmailSubject body:emailBody]; if (composeEmailURL != nil) { [[UIApplication sharedApplication] openURL:composeEmailURL]; } @@ -652,3 +619,77 @@ - (BOOL)canBecomeFirstResponder; } @end + + +@implementation ARKDefaultPromptPresenter + +- (void)showBugReportingPromptForConfiguration:(nonnull ARKEmailBugReportConfiguration *)configuration completion:(nonnull ARKEmailBugReporterCustomPromptCompletionBlock)completion { + self.configuration = configuration; + self.completion = completion; + + [self _showBugTitleCaptureAlert]; +} + +- (void)_showBugTitleCaptureAlert; +{ + /* + iOS 8 often fails to transfer the keyboard from a focused text field to a UIAlertView's text field. + Transfer first responder to an invisble view when a debug screenshot is captured to make bug filing itself bug-free. + */ + [self _stealFirstResponder]; + + NSString * const title = NSLocalizedString(@"What Went Wrong?", @"Title text for alert asking user to describe a bug they just encountered"); + NSString * const message = NSLocalizedString(@"Please briefly summarize the issue you just encountered. You’ll be asked for more details later.", @"Subtitle text for alert asking user to describe a bug they just encountered"); + NSString * const composeReportButtonTitle = NSLocalizedString(@"Compose Report", @"Button title to compose bug report"); + NSString * const cancelButtonTitle = NSLocalizedString(@"Cancel", @"Button title to not compose a bug report"); + + UIAlertController *const alertController = [UIAlertController alertControllerWithTitle:title message:message preferredStyle:UIAlertControllerStyleAlert]; + + [alertController addAction:[UIAlertAction actionWithTitle:composeReportButtonTitle style:UIAlertActionStyleDefault handler:^(UIAlertAction *action) { + UITextField *textfield = [alertController.textFields firstObject]; + [self _createBugReportWithTitle:textfield.text]; + }]]; + + [alertController addAction:[UIAlertAction actionWithTitle:cancelButtonTitle style:UIAlertActionStyleDefault handler:^(UIAlertAction *action) { + self.completion(nil); + }]]; + + [alertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { + [self _configureAlertTextfield:textField]; + }]; + + UIViewController *viewControllerToPresentAlertController = [UIApplication sharedApplication].keyWindow.rootViewController; + while (viewControllerToPresentAlertController.presentedViewController != nil) { + viewControllerToPresentAlertController = viewControllerToPresentAlertController.presentedViewController; + } + + /* + Disabling animations here to avoid potential crashes resulting from unexpected view state in UIKit + */ + [viewControllerToPresentAlertController presentViewController:alertController animated:NO completion:NULL]; +} + +- (void)_stealFirstResponder; +{ + ARKInvisibleView *invisibleView = [ARKInvisibleView new]; + invisibleView.layer.opacity = 0.0; + [[UIApplication sharedApplication].keyWindow addSubview:invisibleView]; + [invisibleView becomeFirstResponder]; + [invisibleView removeFromSuperview]; +} + +- (void)_configureAlertTextfield:(UITextField *)textField +{ + textField.autocapitalizationType = UITextAutocapitalizationTypeSentences; + textField.autocorrectionType = UITextAutocorrectionTypeYes; + textField.spellCheckingType = UITextSpellCheckingTypeYes; + textField.returnKeyType = UIReturnKeyDone; +} + +- (void)_createBugReportWithTitle:(NSString *)title; +{ + self.configuration.prefilledEmailSubject = title; + self.completion(self.configuration); +} + +@end diff --git a/Aardvark/Aardvark.h b/Aardvark/Aardvark.h index 537a02d2..473e66f3 100644 --- a/Aardvark/Aardvark.h +++ b/Aardvark/Aardvark.h @@ -32,6 +32,7 @@ FOUNDATION_EXPORT const unsigned char Aardvark_iOSVersionString[]; #import #import #import +#import #import #import #import From afe2c799b1bfa5fef469ac3c075ebd6e417dc653 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Tue, 8 May 2018 18:33:39 -0700 Subject: [PATCH 2/9] Address feedback from @dfed --- Aardvark/ARKEmailBugReportConfiguration.h | 11 ++++++----- Aardvark/ARKEmailBugReportConfiguration.m | 13 +++++++++++++ Aardvark/ARKEmailBugReporter.h | 2 +- Aardvark/ARKEmailBugReporter.m | 17 +++++++++-------- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/Aardvark/ARKEmailBugReportConfiguration.h b/Aardvark/ARKEmailBugReportConfiguration.h index 343f3e00..7e4ebae6 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.h +++ b/Aardvark/ARKEmailBugReportConfiguration.h @@ -24,21 +24,22 @@ @class ARKEmailAttachment; +/// Configuration object describing the contents of an email bug report. @interface ARKEmailBugReportConfiguration : NSObject -/// The email subject that will be prefilled when the email dialog is presented to the user. +/// The email subject that will be prefilled when the email dialog is presented to the user. Defaults to an empty string. @property (nonnull, nonatomic, copy) NSString *prefilledEmailSubject; -/// The log stores that will be included as attachments on the email. +/// The log stores that will be included as attachments on the email. Defaults to an empty array. @property (nonnull, nonatomic, copy) NSArray *logStores; -/// Controls whether or not a screenshot should be attached to the email, when available. +/// Controls whether or not a screenshot should be attached to the email, when available. Defaults to NO. @property (nonatomic) BOOL includesScreenshot; -/// Controls whether or not a view hierarchy description should be attached to the email, when available. +/// Controls whether or not a view hierarchy description should be attached to the email, when available. Defaults to NO. @property (nonatomic) BOOL includesViewHierarchyDescription; -/// Additional attachments to include on the email. +/// Additional attachments to include on the email. Defaults to nil. @property (nullable, nonatomic, copy) NSArray *additionalAttachments; @end diff --git a/Aardvark/ARKEmailBugReportConfiguration.m b/Aardvark/ARKEmailBugReportConfiguration.m index 5b2e00b4..8e7fc2ce 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.m +++ b/Aardvark/ARKEmailBugReportConfiguration.m @@ -23,4 +23,17 @@ @implementation ARKEmailBugReportConfiguration +- (instancetype)init; +{ + self = [super init]; + + _prefilledEmailSubject = @""; + _logStores = @[]; + _includesScreenshot = NO; + _includesViewHierarchyDescription = NO; + _additionalAttachments = nil; + + return self; +} + @end diff --git a/Aardvark/ARKEmailBugReporter.h b/Aardvark/ARKEmailBugReporter.h index 36483749..c4c81bb8 100644 --- a/Aardvark/ARKEmailBugReporter.h +++ b/Aardvark/ARKEmailBugReporter.h @@ -63,7 +63,7 @@ typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReport /// Called on the main thread when a bug is filed to signal that a bug report prompt should be presented to the user. /// The `completion` should be called with either an updated configuration to present the email dialog, or `nil` to signal that the prompt was cancelled. /// When the initial `configuration` has `includesScreenshot` or `includesViewHierarchyDescription` false, setting the field to true will have no effect. -- (void)showBugReportingPromptForConfiguration:(nonnull ARKEmailBugReportConfiguration *)configuration completion:(nonnull ARKEmailBugReporterCustomPromptCompletionBlock)completion; +- (void)showBugReportingPromptForConfiguration:(ARKEmailBugReportConfiguration *_Nonnull)configuration completion:(ARKEmailBugReporterCustomPromptCompletionBlock _Nonnull)completion; @end diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index 501391fb..6b955731 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -396,10 +396,10 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf // Once all log messages have been retrieved, attach the data and show the compose window. dispatch_group_notify(logStoreRetrievalDispatchGroup, dispatch_get_main_queue(), ^{ - NSMutableString *emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; + NSMutableString *const emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; for (ARKLogStore *logStore in logStores) { - NSArray *logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; + NSArray *const logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; NSString *screenshotFileName = [NSLocalizedString(@"screenshot", @"File name of a screenshot") stringByAppendingPathExtension:@"png"]; NSString *logsFileName = [NSLocalizedString(@"logs", @"File name for logs attachments") stringByAppendingPathExtension:[self formattedLogMessagesAttachmentExtension]]; @@ -412,7 +412,7 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf logsFileName = [logStore.name stringByAppendingFormat:@"_%@", logsFileName]; } - NSString *recentErrorLogs = [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreAvailable]; + NSString *const recentErrorLogs = [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreAvailable]; if (recentErrorLogs.length) { [emailBodyForLogStore appendFormat:@"%@\n", recentErrorLogs]; appendToEmailBody = YES; @@ -422,7 +422,6 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf [emailBody appendString:emailBodyForLogStore]; } - if (configuration.includesScreenshot && self.attachScreenshotToNextBugReport) { NSData *const mostRecentImage = [self _mostRecentImageAsPNG:logMessages]; if (mostRecentImage.length > 0) { @@ -430,14 +429,14 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf } } - NSData *formattedLogs = [self formattedLogMessagesAsData:logMessages]; + NSData *const formattedLogs = [self formattedLogMessagesAsData:logMessages]; if (formattedLogs.length) { [self.mailComposeViewController addAttachmentData:formattedLogs mimeType:[self formattedLogMessagesDataMIMEType] fileName:logsFileName]; } } if (configuration.includesViewHierarchyDescription && self.viewHierarchyDescription != nil) { - NSString *viewHierarchyFileName = [NSLocalizedString(@"view_hierarchy", @"File name for view hierarchy attachment") stringByAppendingPathExtension:@"txt"]; + NSString *const viewHierarchyFileName = [NSLocalizedString(@"view_hierarchy", @"File name for view hierarchy attachment") stringByAppendingPathExtension:@"txt"]; NSData *const viewHierarchyData = [self.viewHierarchyDescription dataUsingEncoding:NSUTF8StringEncoding]; if (viewHierarchyData.length > 0) { [self.mailComposeViewController addAttachmentData:viewHierarchyData mimeType:@"text/plain" fileName:viewHierarchyFileName]; @@ -464,10 +463,10 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf // Only append logs once all log messages have been retrieved. if (logStoresToLogMessagesMap.count == logStores.count) { - NSMutableString *emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; + NSMutableString *const emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; for (ARKLogStore *logStore in logStores) { - NSArray *logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; + NSArray *const logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; [emailBody appendFormat:@"%@\n", [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreUnavailable]]; } @@ -652,6 +651,7 @@ - (void)_showBugTitleCaptureAlert; [alertController addAction:[UIAlertAction actionWithTitle:cancelButtonTitle style:UIAlertActionStyleDefault handler:^(UIAlertAction *action) { self.completion(nil); + self.completion = nil; }]]; [alertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { @@ -690,6 +690,7 @@ - (void)_createBugReportWithTitle:(NSString *)title; { self.configuration.prefilledEmailSubject = title; self.completion(self.configuration); + self.completion = nil; } @end From d412dbb86b4d3a0cf317bb21b2391e5d82f67513 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Tue, 8 May 2018 18:53:25 -0700 Subject: [PATCH 3/9] Address feedback from @dfed --- Aardvark/ARKEmailBugReporter.h | 4 ++-- Aardvark/ARKEmailBugReporter.m | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/Aardvark/ARKEmailBugReporter.h b/Aardvark/ARKEmailBugReporter.h index c4c81bb8..39248998 100644 --- a/Aardvark/ARKEmailBugReporter.h +++ b/Aardvark/ARKEmailBugReporter.h @@ -61,7 +61,7 @@ typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReport @required /// Called on the main thread when a bug is filed to signal that a bug report prompt should be presented to the user. -/// The `completion` should be called with either an updated configuration to present the email dialog, or `nil` to signal that the prompt was cancelled. +/// The `completion` should be called on the main thread with either an updated configuration to present the email dialog, or `nil` to signal that the prompt was cancelled. /// When the initial `configuration` has `includesScreenshot` or `includesViewHierarchyDescription` false, setting the field to true will have no effect. - (void)showBugReportingPromptForConfiguration:(ARKEmailBugReportConfiguration *_Nonnull)configuration completion:(ARKEmailBugReporterCustomPromptCompletionBlock _Nonnull)completion; @@ -88,7 +88,7 @@ typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReport /// The email attachment delegate, responsible for providing additional attachments and filtering which log stores to include in the bug report at the time the bug is filed. @property (nullable, nonatomic, weak) id emailAttachmentAdditionsDelegate; -/// The prompting delegate, responsible for showing a prompt to file a bug report. +/// The prompting delegate, responsible for showing a prompt to file a bug report. When nil, an alert view will be shown prompting the user to input a title for the bug report. Defaults to nil. @property (nullable, nonatomic, weak) id promptingDelegate; /// The formatter used to prepare the log for entry into an email. Defaults to a vanilla instance of ARKDefaultLogFormatter. diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index 6b955731..ce8ac7b1 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -136,9 +136,9 @@ - (void)_ARK_appendRecursiveViewControllerMappingToMapTable:(NSMapTable -@property (nonatomic) ARKEmailBugReportConfiguration *configuration; +@property (nonatomic, nonnull) ARKEmailBugReportConfiguration *configuration; -@property (nonatomic) ARKEmailBugReporterCustomPromptCompletionBlock completion; +@property (nonatomic, nullable) ARKEmailBugReporterCustomPromptCompletionBlock completion; @end @@ -342,13 +342,15 @@ - (void)_showBugReportPrompt; [promptPresenter showBugReportingPromptForConfiguration:[self _configurationWithCurrentSettings] completion:^(ARKEmailBugReportConfiguration * _Nullable configuration) { if (configuration != nil) { [self _createBugReportWithConfiguration:configuration]; + } else { + // If the configuration is nil, the callee has signaled that we should not show a bug report. In the future, we can clean up any persisted state here as necessary. } }]; } - (ARKEmailBugReportConfiguration *)_configurationWithCurrentSettings; { - ARKEmailBugReportConfiguration *const configuration = [[ARKEmailBugReportConfiguration alloc] init]; + ARKEmailBugReportConfiguration *const configuration = [ARKEmailBugReportConfiguration new]; if (self.emailAttachmentAdditionsDelegate != nil) { NSMutableArray *const filteredLogStores = [NSMutableArray arrayWithCapacity:self.logStores.count]; From 7ebd7c6d1fed9169028709cbac5b74ec0ba8d2da Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Tue, 8 May 2018 19:02:14 -0700 Subject: [PATCH 4/9] Bump version to 3.4.0 --- Aardvark.podspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Aardvark.podspec b/Aardvark.podspec index b1ab7cf2..a1f4f83a 100644 --- a/Aardvark.podspec +++ b/Aardvark.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'Aardvark' - s.version = '3.3.2' + s.version = '3.4.0' s.license = 'Apache License, Version 2.0' s.summary = 'Aardvark is a library that makes it dead simple to create actionable bug reports.' s.homepage = 'https://github.com/square/Aardvark' From 0cb6208cb72f3093f0a33047e673aa9463e6343c Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Wed, 9 May 2018 14:22:21 -0700 Subject: [PATCH 5/9] Address feedback from @pwesten --- Aardvark/ARKEmailBugReporter.m | 30 +++++------------------------- 1 file changed, 5 insertions(+), 25 deletions(-) diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index ce8ac7b1..b276f44e 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -136,10 +136,6 @@ - (void)_ARK_appendRecursiveViewControllerMappingToMapTable:(NSMapTable -@property (nonatomic, nonnull) ARKEmailBugReportConfiguration *configuration; - -@property (nonatomic, nullable) ARKEmailBugReporterCustomPromptCompletionBlock completion; - @end @@ -340,10 +336,9 @@ - (void)_showBugReportPrompt; { id const promptPresenter = (self.promptingDelegate ?: [ARKDefaultPromptPresenter new]); [promptPresenter showBugReportingPromptForConfiguration:[self _configurationWithCurrentSettings] completion:^(ARKEmailBugReportConfiguration * _Nullable configuration) { + // If the configuration is nil, the callee has signaled that we should not show a bug report. In the future, we can clean up any persisted state here as necessary. if (configuration != nil) { [self _createBugReportWithConfiguration:configuration]; - } else { - // If the configuration is nil, the callee has signaled that we should not show a bug report. In the future, we can clean up any persisted state here as necessary. } }]; } @@ -625,17 +620,9 @@ - (BOOL)canBecomeFirstResponder; @implementation ARKDefaultPromptPresenter - (void)showBugReportingPromptForConfiguration:(nonnull ARKEmailBugReportConfiguration *)configuration completion:(nonnull ARKEmailBugReporterCustomPromptCompletionBlock)completion { - self.configuration = configuration; - self.completion = completion; - - [self _showBugTitleCaptureAlert]; -} - -- (void)_showBugTitleCaptureAlert; -{ /* iOS 8 often fails to transfer the keyboard from a focused text field to a UIAlertView's text field. - Transfer first responder to an invisble view when a debug screenshot is captured to make bug filing itself bug-free. + Transfer first responder to an invisible view when a debug screenshot is captured to make bug filing itself bug-free. */ [self _stealFirstResponder]; @@ -648,12 +635,12 @@ - (void)_showBugTitleCaptureAlert; [alertController addAction:[UIAlertAction actionWithTitle:composeReportButtonTitle style:UIAlertActionStyleDefault handler:^(UIAlertAction *action) { UITextField *textfield = [alertController.textFields firstObject]; - [self _createBugReportWithTitle:textfield.text]; + configuration.prefilledEmailSubject = textfield.text ?: @""; + completion(configuration); }]]; [alertController addAction:[UIAlertAction actionWithTitle:cancelButtonTitle style:UIAlertActionStyleDefault handler:^(UIAlertAction *action) { - self.completion(nil); - self.completion = nil; + completion(nil); }]]; [alertController addTextFieldWithConfigurationHandler:^(UITextField *textField) { @@ -688,11 +675,4 @@ - (void)_configureAlertTextfield:(UITextField *)textField textField.returnKeyType = UIReturnKeyDone; } -- (void)_createBugReportWithTitle:(NSString *)title; -{ - self.configuration.prefilledEmailSubject = title; - self.completion(self.configuration); - self.completion = nil; -} - @end From 74403e449c1920b3facbd3fada661e6909aad574 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Wed, 9 May 2018 16:20:16 -0700 Subject: [PATCH 6/9] Address feedback from @pwesten --- Aardvark/ARKEmailBugReporter.m | 64 +++++++++++++++------------------- 1 file changed, 28 insertions(+), 36 deletions(-) diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index b276f44e..a9a91ccb 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -373,24 +373,24 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf NSMapTable *logStoresToLogMessagesMap = [NSMapTable new]; NSDictionary *emailBodyAdditions = [self.emailBodyAdditionsDelegate emailBodyAdditionsForEmailBugReporter:self]; + dispatch_group_t logStoreRetrievalDispatchGroup = dispatch_group_create(); + dispatch_group_enter(logStoreRetrievalDispatchGroup); + + NSArray *const logStores = configuration.logStores; + for (ARKLogStore *logStore in logStores) { + dispatch_group_enter(logStoreRetrievalDispatchGroup); + [logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) { + [logStoresToLogMessagesMap setObject:logMessages forKey:logStore]; + dispatch_group_leave(logStoreRetrievalDispatchGroup); + }]; + } + if ([MFMailComposeViewController canSendMail]) { self.mailComposeViewController = [MFMailComposeViewController new]; [self.mailComposeViewController setToRecipients:@[self.bugReportRecipientEmailAddress]]; [self.mailComposeViewController setSubject:configuration.prefilledEmailSubject]; - dispatch_group_t logStoreRetrievalDispatchGroup = dispatch_group_create(); - dispatch_group_enter(logStoreRetrievalDispatchGroup); - - NSArray *const logStores = configuration.logStores; - for (ARKLogStore *logStore in logStores) { - dispatch_group_enter(logStoreRetrievalDispatchGroup); - [logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) { - [logStoresToLogMessagesMap setObject:logMessages forKey:logStore]; - dispatch_group_leave(logStoreRetrievalDispatchGroup); - }]; - } - // Once all log messages have been retrieved, attach the data and show the compose window. dispatch_group_notify(logStoreRetrievalDispatchGroup, dispatch_get_main_queue(), ^{ NSMutableString *const emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; @@ -400,7 +400,7 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf NSString *screenshotFileName = [NSLocalizedString(@"screenshot", @"File name of a screenshot") stringByAppendingPathExtension:@"png"]; NSString *logsFileName = [NSLocalizedString(@"logs", @"File name for logs attachments") stringByAppendingPathExtension:[self formattedLogMessagesAttachmentExtension]]; - NSMutableString *emailBodyForLogStore = [NSMutableString new]; + NSMutableString *const emailBodyForLogStore = [NSMutableString new]; BOOL appendToEmailBody = NO; if (logStore.name.length) { @@ -450,31 +450,23 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf [self _showEmailComposeWindow]; }); - dispatch_group_leave(logStoreRetrievalDispatchGroup); - } else { - NSArray *const logStores = configuration.logStores; - for (ARKLogStore *logStore in logStores) { - [logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) { - [logStoresToLogMessagesMap setObject:logMessages forKey:logStore]; - - // Only append logs once all log messages have been retrieved. - if (logStoresToLogMessagesMap.count == logStores.count) { - NSMutableString *const emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; - - for (ARKLogStore *logStore in logStores) { - NSArray *const logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; - [emailBody appendFormat:@"%@\n", [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreUnavailable]]; - } - - NSURL *const composeEmailURL = [self _emailURLWithRecipients:@[self.bugReportRecipientEmailAddress] CC:@"" subject:configuration.prefilledEmailSubject body:emailBody]; - if (composeEmailURL != nil) { - [[UIApplication sharedApplication] openURL:composeEmailURL]; - } - } - }]; - } + dispatch_group_notify(logStoreRetrievalDispatchGroup, dispatch_get_main_queue(), ^{ + NSMutableString *const emailBody = [self _prefilledEmailBodyWithEmailBodyAdditions:emailBodyAdditions]; + + for (ARKLogStore *logStore in logStores) { + NSArray *const logMessages = [logStoresToLogMessagesMap objectForKey:logStore]; + [emailBody appendFormat:@"%@\n", [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreUnavailable]]; + } + + NSURL *const composeEmailURL = [self _emailURLWithRecipients:@[self.bugReportRecipientEmailAddress] CC:@"" subject:configuration.prefilledEmailSubject body:emailBody]; + if (composeEmailURL != nil) { + [[UIApplication sharedApplication] openURL:composeEmailURL]; + } + }); } + + dispatch_group_leave(logStoreRetrievalDispatchGroup); } - (void)_showEmailComposeWindow; From df47e3d23b3540a3d16df1f3a96f186bc51c88a1 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Mon, 14 May 2018 22:59:48 -0700 Subject: [PATCH 7/9] Make and properties on read only, and provide methods, so that they are one-way mutable --- Aardvark.xcodeproj/project.pbxproj | 4 +++ Aardvark/ARKEmailBugReportConfiguration.h | 13 ++++++-- Aardvark/ARKEmailBugReportConfiguration.m | 25 ++++++++++++++-- ...ARKEmailBugReportConfiguration_Protected.h | 30 +++++++++++++++++++ Aardvark/ARKEmailBugReporter.m | 7 ++--- 5 files changed, 70 insertions(+), 9 deletions(-) create mode 100644 Aardvark/ARKEmailBugReportConfiguration_Protected.h diff --git a/Aardvark.xcodeproj/project.pbxproj b/Aardvark.xcodeproj/project.pbxproj index dd811890..7de93334 100644 --- a/Aardvark.xcodeproj/project.pbxproj +++ b/Aardvark.xcodeproj/project.pbxproj @@ -9,6 +9,7 @@ /* Begin PBXBuildFile section */ 3D6E2D0E20868335007B8013 /* ARKEmailBugReportConfiguration.m in Sources */ = {isa = PBXBuildFile; fileRef = 3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */; }; 3D6E2D0F20868335007B8013 /* ARKEmailBugReportConfiguration.h in Headers */ = {isa = PBXBuildFile; fileRef = 3D6E2D0D20868335007B8013 /* ARKEmailBugReportConfiguration.h */; settings = {ATTRIBUTES = (Public, ); }; }; + 3D90DEBA20AAABAB006D4924 /* ARKEmailBugReportConfiguration_Protected.h in Headers */ = {isa = PBXBuildFile; fileRef = 3D90DEB720AA9B19006D4924 /* ARKEmailBugReportConfiguration_Protected.h */; }; 3DED97142006D35B007FC95C /* ARKEmailAttachment.h in Headers */ = {isa = PBXBuildFile; fileRef = 3DED97122006D35B007FC95C /* ARKEmailAttachment.h */; }; 3DED97152006D35B007FC95C /* ARKEmailAttachment.m in Sources */ = {isa = PBXBuildFile; fileRef = 3DED97132006D35B007FC95C /* ARKEmailAttachment.m */; }; 4551A2D91BDAD10E00F216D0 /* Aardvark.h in Headers */ = {isa = PBXBuildFile; fileRef = EAD1442419E073FB0065A1FF /* Aardvark.h */; settings = {ATTRIBUTES = (Public, ); }; }; @@ -98,6 +99,7 @@ /* Begin PBXFileReference section */ 3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; name = ARKEmailBugReportConfiguration.m; path = Aardvark/ARKEmailBugReportConfiguration.m; sourceTree = ""; }; 3D6E2D0D20868335007B8013 /* ARKEmailBugReportConfiguration.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; name = ARKEmailBugReportConfiguration.h; path = Aardvark/ARKEmailBugReportConfiguration.h; sourceTree = ""; }; + 3D90DEB720AA9B19006D4924 /* ARKEmailBugReportConfiguration_Protected.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARKEmailBugReportConfiguration_Protected.h; path = Aardvark/ARKEmailBugReportConfiguration_Protected.h; sourceTree = ""; }; 3DED97122006D35B007FC95C /* ARKEmailAttachment.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = ARKEmailAttachment.h; path = Aardvark/ARKEmailAttachment.h; sourceTree = ""; }; 3DED97132006D35B007FC95C /* ARKEmailAttachment.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; name = ARKEmailAttachment.m; path = Aardvark/ARKEmailAttachment.m; sourceTree = ""; }; 4551A2C21BDACF9000F216D0 /* Aardvark.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Aardvark.framework; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -300,6 +302,7 @@ EA98B9321D4BEB6E00B3A390 /* ARKDefaultLogFormatter.m */, EA98B9331D4BEB6E00B3A390 /* ARKEmailBugReporter_Testing.h */, 3D6E2D0D20868335007B8013 /* ARKEmailBugReportConfiguration.h */, + 3D90DEB720AA9B19006D4924 /* ARKEmailBugReportConfiguration_Protected.h */, 3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */, EA98B9341D4BEB6E00B3A390 /* ARKEmailBugReporter.h */, EA98B9351D4BEB6E00B3A390 /* ARKEmailBugReporter.m */, @@ -417,6 +420,7 @@ buildActionMask = 2147483647; files = ( 4551A2D91BDAD10E00F216D0 /* Aardvark.h in Headers */, + 3D90DEBA20AAABAB006D4924 /* ARKEmailBugReportConfiguration_Protected.h in Headers */, 3DED97142006D35B007FC95C /* ARKEmailAttachment.h in Headers */, EA98B9401D4BEB6E00B3A390 /* ARKEmailBugReporter.h in Headers */, 4551A30A1BDAF93A00F216D0 /* ARKScreenshotLogging.h in Headers */, diff --git a/Aardvark/ARKEmailBugReportConfiguration.h b/Aardvark/ARKEmailBugReportConfiguration.h index 7e4ebae6..59c9a5b2 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.h +++ b/Aardvark/ARKEmailBugReportConfiguration.h @@ -27,6 +27,9 @@ /// Configuration object describing the contents of an email bug report. @interface ARKEmailBugReportConfiguration : NSObject +- (instancetype)init NS_UNAVAILABLE; ++ (instancetype)new NS_UNAVAILABLE; + /// The email subject that will be prefilled when the email dialog is presented to the user. Defaults to an empty string. @property (nonnull, nonatomic, copy) NSString *prefilledEmailSubject; @@ -34,12 +37,18 @@ @property (nonnull, nonatomic, copy) NSArray *logStores; /// Controls whether or not a screenshot should be attached to the email, when available. Defaults to NO. -@property (nonatomic) BOOL includesScreenshot; +@property (nonatomic, readonly) BOOL includesScreenshot; /// Controls whether or not a view hierarchy description should be attached to the email, when available. Defaults to NO. -@property (nonatomic) BOOL includesViewHierarchyDescription; +@property (nonatomic, readonly) BOOL includesViewHierarchyDescription; /// Additional attachments to include on the email. Defaults to nil. @property (nullable, nonatomic, copy) NSArray *additionalAttachments; +/// Removes the screenshot from the bug report, if one is included. +- (void)removeScreenshot; + +/// Removes the view hierarchy description from the bug report, if one is included. +- (void)removeViewHierarchyDescription; + @end diff --git a/Aardvark/ARKEmailBugReportConfiguration.m b/Aardvark/ARKEmailBugReportConfiguration.m index 8e7fc2ce..df521bc2 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.m +++ b/Aardvark/ARKEmailBugReportConfiguration.m @@ -19,21 +19,40 @@ // #import "ARKEmailBugReportConfiguration.h" +#import "ARKEmailBugReportConfiguration_Protected.h" + + +@interface ARKEmailBugReportConfiguration () + +@property (nonatomic, readwrite) BOOL includesScreenshot; +@property (nonatomic, readwrite) BOOL includesViewHierarchyDescription; + +@end @implementation ARKEmailBugReportConfiguration -- (instancetype)init; +- (instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescription:(BOOL)includesViewHierarchyDescription; { self = [super init]; _prefilledEmailSubject = @""; _logStores = @[]; - _includesScreenshot = NO; - _includesViewHierarchyDescription = NO; + _includesScreenshot = includesScreenshot; + _includesViewHierarchyDescription = includesViewHierarchyDescription; _additionalAttachments = nil; return self; } +- (void)removeScreenshot; +{ + self.includesScreenshot = NO; +} + +- (void)removeViewHierarchyDescription; +{ + self.includesViewHierarchyDescription = NO; +} + @end diff --git a/Aardvark/ARKEmailBugReportConfiguration_Protected.h b/Aardvark/ARKEmailBugReportConfiguration_Protected.h new file mode 100644 index 00000000..45024167 --- /dev/null +++ b/Aardvark/ARKEmailBugReportConfiguration_Protected.h @@ -0,0 +1,30 @@ +// +// ARKEmailBugReportConfiguration_Protected.h +// Aardvark +// +// Created by Nick Entin on 5/14/18. +// Copyright 2018 Square, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +//    http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +@import Foundation; + +#import "ARKEmailBugReportConfiguration.h" + + +@interface ARKEmailBugReportConfiguration (Protected) + +- (instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescription:(BOOL)includesViewHierarchyDescription; + +@end diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index a9a91ccb..e01ae1e8 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -27,6 +27,7 @@ #import "ARKDefaultLogFormatter.h" #import "ARKEmailAttachment.h" #import "ARKEmailBugReportConfiguration.h" +#import "ARKEmailBugReportConfiguration_Protected.h" #import "ARKScreenshotLogging.h" #import "ARKLogMessage.h" #import "ARKLogStore.h" @@ -345,7 +346,8 @@ - (void)_showBugReportPrompt; - (ARKEmailBugReportConfiguration *)_configurationWithCurrentSettings; { - ARKEmailBugReportConfiguration *const configuration = [ARKEmailBugReportConfiguration new]; + ARKEmailBugReportConfiguration *const configuration = [[ARKEmailBugReportConfiguration alloc] initWithScreenshot:self.attachScreenshotToNextBugReport + viewHierarchyDescription:(self.attachScreenshotToNextBugReport && self.attachesViewHierarchyDescriptionWithScreenshot)]; if (self.emailAttachmentAdditionsDelegate != nil) { NSMutableArray *const filteredLogStores = [NSMutableArray arrayWithCapacity:self.logStores.count]; @@ -362,9 +364,6 @@ - (ARKEmailBugReportConfiguration *)_configurationWithCurrentSettings; configuration.logStores = [self.logStores copy]; } - configuration.includesScreenshot = self.attachScreenshotToNextBugReport; - configuration.includesViewHierarchyDescription = (self.attachScreenshotToNextBugReport && self.attachesViewHierarchyDescriptionWithScreenshot); - return configuration; } From b17bc059026ab3248e8990f0a7e248712e5f9746 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Tue, 15 May 2018 11:00:24 -0700 Subject: [PATCH 8/9] Add missing nullability specifiers --- Aardvark/ARKEmailBugReportConfiguration.h | 4 ++-- Aardvark/ARKEmailBugReportConfiguration_Protected.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Aardvark/ARKEmailBugReportConfiguration.h b/Aardvark/ARKEmailBugReportConfiguration.h index 59c9a5b2..59295bf4 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.h +++ b/Aardvark/ARKEmailBugReportConfiguration.h @@ -27,8 +27,8 @@ /// Configuration object describing the contents of an email bug report. @interface ARKEmailBugReportConfiguration : NSObject -- (instancetype)init NS_UNAVAILABLE; -+ (instancetype)new NS_UNAVAILABLE; +- (nonnull instancetype)init NS_UNAVAILABLE; ++ (nonnull instancetype)new NS_UNAVAILABLE; /// The email subject that will be prefilled when the email dialog is presented to the user. Defaults to an empty string. @property (nonnull, nonatomic, copy) NSString *prefilledEmailSubject; diff --git a/Aardvark/ARKEmailBugReportConfiguration_Protected.h b/Aardvark/ARKEmailBugReportConfiguration_Protected.h index 45024167..b6fef29b 100644 --- a/Aardvark/ARKEmailBugReportConfiguration_Protected.h +++ b/Aardvark/ARKEmailBugReportConfiguration_Protected.h @@ -25,6 +25,6 @@ @interface ARKEmailBugReportConfiguration (Protected) -- (instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescription:(BOOL)includesViewHierarchyDescription; +- (nonnull instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescription:(BOOL)includesViewHierarchyDescription; @end From de292f48bd68c8834e37f2a6ce46966a316cd6d1 Mon Sep 17 00:00:00 2001 From: Nicholas Entin Date: Mon, 25 Jun 2018 12:03:40 -0700 Subject: [PATCH 9/9] Address feedback from @pwesten --- Aardvark/ARKEmailBugReportConfiguration.h | 12 ++++++------ Aardvark/ARKEmailBugReportConfiguration.m | 6 +++--- Aardvark/ARKEmailBugReporter.m | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Aardvark/ARKEmailBugReportConfiguration.h b/Aardvark/ARKEmailBugReportConfiguration.h index 59295bf4..94727fd3 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.h +++ b/Aardvark/ARKEmailBugReportConfiguration.h @@ -42,13 +42,13 @@ /// Controls whether or not a view hierarchy description should be attached to the email, when available. Defaults to NO. @property (nonatomic, readonly) BOOL includesViewHierarchyDescription; -/// Additional attachments to include on the email. Defaults to nil. -@property (nullable, nonatomic, copy) NSArray *additionalAttachments; +/// Additional attachments to include on the email. Defaults to an empty array. +@property (nonnull, nonatomic, copy) NSArray *additionalAttachments; -/// Removes the screenshot from the bug report, if one is included. -- (void)removeScreenshot; +/// Excludes the screenshot from the bug report, if one is included. +- (void)excludeScreenshot; -/// Removes the view hierarchy description from the bug report, if one is included. -- (void)removeViewHierarchyDescription; +/// Excludes the view hierarchy description from the bug report, if one is included. +- (void)excludeViewHierarchyDescription; @end diff --git a/Aardvark/ARKEmailBugReportConfiguration.m b/Aardvark/ARKEmailBugReportConfiguration.m index df521bc2..4baf8d14 100644 --- a/Aardvark/ARKEmailBugReportConfiguration.m +++ b/Aardvark/ARKEmailBugReportConfiguration.m @@ -40,17 +40,17 @@ - (instancetype)initWithScreenshot:(BOOL)includesScreenshot viewHierarchyDescrip _logStores = @[]; _includesScreenshot = includesScreenshot; _includesViewHierarchyDescription = includesViewHierarchyDescription; - _additionalAttachments = nil; + _additionalAttachments = @[]; return self; } -- (void)removeScreenshot; +- (void)excludeScreenshot; { self.includesScreenshot = NO; } -- (void)removeViewHierarchyDescription; +- (void)excludeViewHierarchyDescription; { self.includesViewHierarchyDescription = NO; } diff --git a/Aardvark/ARKEmailBugReporter.m b/Aardvark/ARKEmailBugReporter.m index e01ae1e8..af01d077 100644 --- a/Aardvark/ARKEmailBugReporter.m +++ b/Aardvark/ARKEmailBugReporter.m @@ -358,7 +358,7 @@ - (ARKEmailBugReportConfiguration *)_configurationWithCurrentSettings; } configuration.logStores = filteredLogStores; - configuration.additionalAttachments = [self.emailAttachmentAdditionsDelegate additionalEmailAttachmentsForEmailBugReporter:self]; + configuration.additionalAttachments = [self.emailAttachmentAdditionsDelegate additionalEmailAttachmentsForEmailBugReporter:self] ?: @[]; } else { configuration.logStores = [self.logStores copy];