Skip to content

Commit

Permalink
Add highlights summary to bug report attachment
Browse files Browse the repository at this point in the history
  • Loading branch information
NickEntin committed Dec 22, 2023
1 parent 8360470 commit 67020d5
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 152 deletions.
2 changes: 0 additions & 2 deletions Aardvark.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,6 @@
EA98B9031D4BEAFC00B3A390 /* ARKScreenshotViewController.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ARKScreenshotViewController.m; sourceTree = "<group>"; };
EA98B9311D4BEB6E00B3A390 /* ARKDefaultLogFormatter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARKDefaultLogFormatter.h; sourceTree = "<group>"; };
EA98B9321D4BEB6E00B3A390 /* ARKDefaultLogFormatter.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ARKDefaultLogFormatter.m; sourceTree = "<group>"; };
EA98B9331D4BEB6E00B3A390 /* ARKEmailBugReporter_Testing.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARKEmailBugReporter_Testing.h; sourceTree = "<group>"; };
EA98B9341D4BEB6E00B3A390 /* ARKEmailBugReporter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARKEmailBugReporter.h; sourceTree = "<group>"; };
EA98B9351D4BEB6E00B3A390 /* ARKEmailBugReporter.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = ARKEmailBugReporter.m; sourceTree = "<group>"; };
EA98B9361D4BEB6E00B3A390 /* ARKLogFormatter.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = ARKLogFormatter.h; sourceTree = "<group>"; };
Expand Down Expand Up @@ -381,7 +380,6 @@
3D90DEB720AA9B19006D4924 /* ARKEmailBugReportConfiguration_Protected.h */,
3D6E2D0C20868335007B8013 /* ARKEmailBugReportConfiguration.m */,
EA98B9341D4BEB6E00B3A390 /* ARKEmailBugReporter.h */,
EA98B9331D4BEB6E00B3A390 /* ARKEmailBugReporter_Testing.h */,
EA98B9351D4BEB6E00B3A390 /* ARKEmailBugReporter.m */,
3DA5BF392556602000B6D148 /* AardvarkMailUI.h */,
3DA5BF3A2556602000B6D148 /* Info.plist */,
Expand Down
1 change: 0 additions & 1 deletion AardvarkMailUI.podspec
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ Pod::Spec.new do |s|
s.ios.deployment_target = '14.0'

s.source_files = 'Sources/AardvarkMailUI/**/*.{h,m,swift}'
s.private_header_files = 'Sources/AardvarkMailUI/**/*_Testing.h', 'Sources/AardvarkMailUI/PrivateCategories/*.h'

s.dependency 'Aardvark', '~> 5.0.0'
end
10 changes: 9 additions & 1 deletion Sources/Aardvark/BugReporting/ARKBugReportAttachment.swift
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,16 @@ public final class ARKBugReportAttachment: NSObject {

// MARK: - Life Cycle

@objc public init(fileName: String, data: Data, dataMIMEType: String) {
@objc public init(
fileName: String,
data: Data,
dataMIMEType: String,
highlightsSummary: String? = nil
) {
self.fileName = fileName
self.data = data
self.dataMIMEType = dataMIMEType
self.highlightsSummary = highlightsSummary
}

// MARK: - Public Properties
Expand All @@ -43,4 +49,6 @@ public final class ARKBugReportAttachment: NSObject {
/// MIME types are as specified by the IANA: <http://www.iana.org/assignments/media-types/>.
@objc public let dataMIMEType: String

@objc public let highlightsSummary: String?

}
22 changes: 18 additions & 4 deletions Sources/Aardvark/BugReporting/LogStoreAttachmentGenerator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,14 @@ public final class LogStoreAttachmentGenerator: NSObject {
/// - parameter messageFormatter: The formatter used to format messages in the logs attachment.
/// - parameter includeLatestScreenshot: Whether an attachment should be generated for the last screenshot in the
/// log store, if one exists.
/// - parameter numberOfErrorsInHighlights: The number of errors to include in the highlights for the log store.
/// - parameter completionQueue: The queue on which the completion should be called.
/// - parameter completion: The completion to be called once the attachments have been generated.
public static func attachments(
for logStore: ARKLogStore,
messageFormatter: ARKLogFormatter = ARKDefaultLogFormatter(),
includeLatestScreenshot: Bool,
numberOfErrorsInHighlights: Int = 3,
completionQueue: DispatchQueue,
completion: @escaping (Attachments) -> Void
) {
Expand All @@ -63,7 +65,8 @@ public final class LogStoreAttachmentGenerator: NSObject {
let logsAttachment = attachment(
for: logMessages,
using: messageFormatter,
logStoreName: logStore.name
logStoreName: logStore.name,
numberOfErrorsInHighlights: numberOfErrorsInHighlights
)

completionQueue.async {
Expand Down Expand Up @@ -107,11 +110,13 @@ public final class LogStoreAttachmentGenerator: NSObject {
/// - parameter logMessages: The log messages to be included in the attachment.
/// - parameter logFormatter: The formatter with which to format the log messages.
/// - parameter logStoreName: The name of the log store from which the logs were collected.
@objc(attachmentForLogMessages:usingLogFormatter:logStoreName:)
/// - parameter numberOfErrorsInHighlights: The number of errors to include in the highlights for the log store.
@objc(attachmentForLogMessages:usingLogFormatter:logStoreName:numberOfErrorsInHighlights:)
public static func attachment(
for logMessages: [ARKLogMessage],
using logFormatter: ARKLogFormatter = ARKDefaultLogFormatter(),
logStoreName: String?
logStoreName: String?,
numberOfErrorsInHighlights: Int = 3
) -> ARKBugReportAttachment? {
guard !logMessages.isEmpty else {
return nil
Expand All @@ -124,10 +129,19 @@ public final class LogStoreAttachmentGenerator: NSObject {

let fileName = logsFileName(for: logStoreName, fileType: "txt")

let recentErrorSummary = logMessages
.lazy
.reversed()
.filter { $0.type == .error }
.prefix(numberOfErrorsInHighlights)
.map { $0.text }
.joined(separator: "\n")

return ARKBugReportAttachment(
fileName: fileName,
data: formattedLogData,
dataMIMEType: "text/plain"
dataMIMEType: "text/plain",
highlightsSummary: recentErrorSummary.isEmpty ? nil : recentErrorSummary
)
}

Expand Down
4 changes: 2 additions & 2 deletions Sources/AardvarkMailUI/ARKEmailBugReporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ typedef void (^ARKEmailBugReporterCustomPromptCompletionBlock)(ARKEmailBugReport
@property (nonatomic) BOOL attachesViewHierarchyDescription;

/// Returns an attachment containing the log messages. Defaults to a plain text attachment containing each log message formatted using the bug reporter's `logFormatter`.
- (nullable ARKBugReportAttachment *)attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName __attribute__((deprecated("Use the async version of this method that takes a completion handler: attachmentForLogMessages:inLogStoreNamed:completion: instead.")));
- (nullable ARKBugReportAttachment *)attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName numberOfErrorsInHighlights:(NSInteger)numberOfErrorsInHighlights __attribute__((deprecated("Use the async version of this method that takes a completion handler: attachmentForLogMessages:inLogStoreNamed:completion: instead.")));

/// Returns an attachment containing the log messages to a completion handler. Defaults to a plain text attachment containing each log message formatted using the bug reporter's `logFormatter`.
- (void) attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName completion: (ARKAttachmentGeneratorCompletionBlock _Nonnull) completionHandler;
- (void) attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName numberOfErrorsInHighlights:(NSInteger)numberOfErrorsInHighlights completion: (ARKAttachmentGeneratorCompletionBlock _Nonnull) completionHandler;

@end
75 changes: 30 additions & 45 deletions Sources/AardvarkMailUI/ARKEmailBugReporter.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
@import MessageUI;

#import "ARKEmailBugReporter.h"
#import "ARKEmailBugReporter_Testing.h"

#import "ARKEmailBugReportConfiguration.h"
#import "ARKEmailBugReportConfiguration_Protected.h"
Expand Down Expand Up @@ -171,14 +170,15 @@ - (NSArray *)logStores;

#pragma mark - Public Methods

- (ARKBugReportAttachment *)attachmentForLogMessages:(NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(NSString *)logStoreName;
- (ARKBugReportAttachment *)attachmentForLogMessages:(NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(NSString *)logStoreName numberOfErrorsInHighlights:(NSInteger)numberOfErrorsInHighlights;
{
return [ARKLogStoreAttachmentGenerator attachmentForLogMessages:logMessages
usingLogFormatter:self.logFormatter
logStoreName:logStoreName];
logStoreName:logStoreName
numberOfErrorsInHighlights:numberOfErrorsInHighlights];
}

- (void)attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName completion:(ARKAttachmentGeneratorCompletionBlock _Nonnull)completionHandler {
- (void)attachmentForLogMessages:(nonnull NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(nonnull NSString *)logStoreName numberOfErrorsInHighlights:(NSInteger)numberOfErrorsInHighlights completion:(ARKAttachmentGeneratorCompletionBlock _Nonnull)completionHandler {
[ARKLogStoreAttachmentGenerator attachmentForLogMessages:logMessages
usingLogFormatter:self.logFormatter
logStoreName:logStoreName
Expand Down Expand Up @@ -278,12 +278,17 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf
[logStoresToLogMessagesMap setObject:logMessages forKey:logStore];

dispatch_group_enter(logStoreRetrievalDispatchGroup);
[self attachmentForLogMessages:logMessages inLogStoreNamed:[logStore name] completion:^(ARKBugReportAttachment * _Nullable attachment) {
if (attachment != NULL) {
[logStoresToLogMessagesAttachmentMap setObject:attachment forKey:logStore];
}
dispatch_group_leave(logStoreRetrievalDispatchGroup);
}];
[self attachmentForLogMessages:logMessages
inLogStoreNamed:[logStore name]
numberOfErrorsInHighlights:[MFMailComposeViewController canSendMail]
? self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreAvailable
: self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreUnavailable
completion:^(ARKBugReportAttachment * _Nullable attachment) {
if (attachment != NULL) {
[logStoresToLogMessagesAttachmentMap setObject:attachment forKey:logStore];
}
dispatch_group_leave(logStoreRetrievalDispatchGroup);
}];

dispatch_group_leave(logStoreRetrievalDispatchGroup);
}];
Expand Down Expand Up @@ -321,20 +326,15 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf
fileName:logsAttachment.fileName];
}

NSMutableString *const emailBodyForLogStore = [NSMutableString new];
BOOL appendToEmailBody = NO;
if (logsAttachment.highlightsSummary.length) {
NSMutableString *const emailBodyForLogStore = [NSMutableString new];

if (logStore.name.length) {
[emailBodyForLogStore appendFormat:@"%@:\n", logStore.name];
}
if (logStore.name.length) {
[emailBodyForLogStore appendFormat:@"%@:\n", logStore.name];
}

NSString *const recentErrorLogs = [self _recentErrorLogMessagesAsPlainText:logMessages count:self.numberOfRecentErrorLogsToIncludeInEmailBodyWhenAttachmentsAreAvailable];
if (recentErrorLogs.length) {
[emailBodyForLogStore appendFormat:@"%@\n", recentErrorLogs];
appendToEmailBody = YES;
}
[emailBodyForLogStore appendFormat:@"%@\n", logsAttachment.highlightsSummary];

if (appendToEmailBody) {
[emailBody appendString:emailBodyForLogStore];
}
}
Expand All @@ -348,6 +348,10 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf

for (ARKBugReportAttachment *attachment in configuration.additionalAttachments) {
[self.mailComposeViewController addAttachmentData:attachment.data mimeType:attachment.dataMIMEType fileName:attachment.fileName];

if (attachment.highlightsSummary.length) {
[emailBody appendFormat:@"\n%@\n", attachment.highlightsSummary];
}
}

[self.mailComposeViewController setMessageBody:emailBody isHTML:NO];
Expand All @@ -360,8 +364,11 @@ - (void)_createBugReportWithConfiguration:(ARKEmailBugReportConfiguration *)conf
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]];
ARKBugReportAttachment *const logsAttachment = [logStoresToLogMessagesAttachmentMap objectForKey:logStore];

if (logsAttachment.highlightsSummary.length) {
[emailBody appendFormat:@"%@\n", logsAttachment.highlightsSummary];
}
}

NSURL *const composeEmailURL = [self _emailURLWithRecipients:@[self.bugReportRecipientEmailAddress] CC:@"" subject:configuration.prefilledEmailSubject body:emailBody];
Expand Down Expand Up @@ -420,28 +427,6 @@ - (NSMutableString *)_prefilledEmailBodyWithEmailBodyAdditions:(nullable NSDicti
return prefilledEmailBodyWithEmailBodyAdditions;
}

- (NSString *)_recentErrorLogMessagesAsPlainText:(NSArray *)logMessages count:(NSUInteger)errorLogsToInclude;
{
NSMutableString *recentErrorLogs = [NSMutableString new];
NSUInteger failuresFound = 0;
for (ARKLogMessage *log in [logMessages reverseObjectEnumerator]) {
if(log.type == ARKLogTypeError) {
[recentErrorLogs appendFormat:@"%@\n", log];

if(++failuresFound >= errorLogsToInclude) {
break;
}
}
}

if (recentErrorLogs.length > 0 ) {
// Remove the final newline and create an immutable string.
return [recentErrorLogs stringByReplacingCharactersInRange:NSMakeRange(recentErrorLogs.length - 1, 1) withString:@""];
} else {
return @"";
}
}

- (NSURL *)_emailURLWithRecipients:(NSArray *)recipients CC:(NSString *)CCLine subject:(NSString *)subjectLine body:(NSString *)bodyText;
{
NSString *const defaultPrefix = @"mailto:";
Expand Down
26 changes: 0 additions & 26 deletions Sources/AardvarkMailUI/ARKEmailBugReporter_Testing.h

This file was deleted.

71 changes: 0 additions & 71 deletions Sources/AardvarkMailUITests/ARKEmailBugReporterTests.m
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
@import XCTest;

#import "ARKEmailBugReporter.h"
#import "ARKEmailBugReporter_Testing.h"

#import "ARKDataArchive.h"
#import "ARKDataArchive_Testing.h"
Expand Down Expand Up @@ -63,76 +62,6 @@ - (void)setUp;

#pragma mark - Behavior Tests

- (void)test_recentErrorLogMessagesAsPlainText_countRespected;
{
NSMutableArray *numbers = [NSMutableArray new];
for (int i = 0; i < self.logStore.maximumLogMessageCount; i++) {
[numbers addObject:@(i)];
}

// Concurrently add all of the logs.
[numbers enumerateObjectsWithOptions:NSEnumerationConcurrent usingBlock:^(NSNumber *number, NSUInteger idx, BOOL *stop) {
ARKLogWithType(ARKLogTypeError, nil, @"%@", number);
}];

NSUInteger const numberOfRecentErrorLogs = 5;
XCTestExpectation *expectation = [self expectationWithDescription:NSStringFromSelector(_cmd)];
[self.logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) {
NSString *recentErrorLogs = [self.bugReporter _recentErrorLogMessagesAsPlainText:logMessages count:numberOfRecentErrorLogs];
XCTAssertEqual([recentErrorLogs componentsSeparatedByString:@"\n"].count, numberOfRecentErrorLogs);

[expectation fulfill];
}];

[self waitForExpectationsWithTimeout:5.0 handler:nil];
}

- (void)test_recentErrorLogMessagesAsPlainText_returnsNilIfNoErrorLogsPresent;
{
NSUInteger const numberOfRecentErrorLogs = 5;
XCTestExpectation *expectation = [self expectationWithDescription:NSStringFromSelector(_cmd)];
[self.logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) {
__block NSString *recentErrorLogs = [self.bugReporter _recentErrorLogMessagesAsPlainText:logMessages count:numberOfRecentErrorLogs];

XCTAssertEqualObjects(recentErrorLogs, @"");

ARKLog(@"This is not an error");

[self.logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) {
recentErrorLogs = [self.bugReporter _recentErrorLogMessagesAsPlainText:logMessages count:numberOfRecentErrorLogs];

XCTAssertEqualObjects(recentErrorLogs, @"");

[expectation fulfill];
}];
}];

[self waitForExpectationsWithTimeout:5.0 handler:nil];
}

- (void)test_recentErrorLogMessagesAsPlainText_returnsNilIfRecentErrorLogsIsZero;
{
NSUInteger const numberOfRecentErrorLogs = 0;
XCTestExpectation *expectation = [self expectationWithDescription:NSStringFromSelector(_cmd)];
[self.logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) {
__block NSString *recentErrorLogs = [self.bugReporter _recentErrorLogMessagesAsPlainText:logMessages count:numberOfRecentErrorLogs];

XCTAssertEqualObjects(recentErrorLogs, @"");

ARKLog(@"This is not an error");

[self.logStore retrieveAllLogMessagesWithCompletionHandler:^(NSArray *logMessages) {
recentErrorLogs = [self.bugReporter _recentErrorLogMessagesAsPlainText:logMessages count:numberOfRecentErrorLogs];

XCTAssertEqualObjects(recentErrorLogs, @"");

[expectation fulfill];
}];
}];

[self waitForExpectationsWithTimeout:5.0 handler:nil];
}

- (void)test_addLogStores_enforcesARKLogStoreClass;
{
XCTAssertEqualObjects(self.bugReporter.logStores, @[self.logStore]);
Expand Down
Loading

0 comments on commit 67020d5

Please sign in to comment.