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.

Sign up for GitHub

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

Jump to bottom

Add highlights summary to bug report attachment #124

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add highlights summary to bug report attachment #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions Aardvark.xcodeproj/project.pbxproj
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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?

}
36 changes: 30 additions & 6 deletions Sources/Aardvark/BugReporting/LogStoreAttachmentGenerator.swift
View file
Open in desktop
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: UInt = 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:)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a breaking change for Aardvark. This one would be fairly easy to work around (by creating a separate method, rather than adding a parameter with a default value). I could go either way on this one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

2¢ For such a small change, I think the workaround is preferable.

public static func attachment(
for logMessages: [ARKLogMessage],
using logFormatter: ARKLogFormatter = ARKDefaultLogFormatter(),
logStoreName: String?
logStoreName: String?,
numberOfErrorsInHighlights: UInt = 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Um, I'll be lazy: is .lazy doing anything here, given the isEmpty call just below? (I don't know!)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the lazy still reduces work cause it at least keeps it from doing the map past the prefix. I'm not 100% sure though, I might be overestimating the power of lazy. At the very least it doesn't seem to hurt anything.

.reversed()
.filter { $0.type == .error }
.prefix(Int(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 All @@ -138,11 +152,12 @@ public final class LogStoreAttachmentGenerator: NSObject {
/// - 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.
/// - parameter completion: A completion handler that retuns void, and is passed an optional attachment.
@objc(attachmentForLogMessages:usingLogFormatter:logStoreName:completion:)
@objc(attachmentForLogMessages:usingLogFormatter:logStoreName:numberOfErrorsInHighlights:completion:)
public static func attachment(
for logMessages: [ARKLogMessage],
using logFormatter: ARKLogFormatter = ARKDefaultLogFormatter(),
logStoreName: String?,
numberOfErrorsInHighlights: UInt = 3,
completion: @escaping @convention(block)(ARKBugReportAttachment?) -> Void
) -> Void {
guard !logMessages.isEmpty else {
Expand All @@ -160,10 +175,19 @@ public final class LogStoreAttachmentGenerator: NSObject {

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

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

let attachment = ARKBugReportAttachment(
fileName: fileName,
data: formattedLogData,
dataMIMEType: "text/plain"
dataMIMEType: "text/plain",
highlightsSummary: recentErrorSummary.isEmpty ? nil : recentErrorSummary
)

DispatchQueue.main.async {
Expand Down
4 changes: 2 additions & 2 deletions Sources/AardvarkMailUI/ARKEmailBugReporter.h
View file
Open in desktop
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:(NSUInteger)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:(NSUInteger)numberOfErrorsInHighlights completion: (ARKAttachmentGeneratorCompletionBlock _Nonnull) completionHandler;

@end
76 changes: 31 additions & 45 deletions Sources/AardvarkMailUI/ARKEmailBugReporter.m
View file
Open in desktop
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,17 +170,19 @@ - (NSArray *)logStores;

#pragma mark - Public Methods

- (ARKBugReportAttachment *)attachmentForLogMessages:(NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(NSString *)logStoreName;
- (ARKBugReportAttachment *)attachmentForLogMessages:(NSArray<ARKLogMessage *> *)logMessages inLogStoreNamed:(NSString *)logStoreName numberOfErrorsInHighlights:(NSUInteger)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:(NSUInteger)numberOfErrorsInHighlights completion:(ARKAttachmentGeneratorCompletionBlock _Nonnull)completionHandler {
[ARKLogStoreAttachmentGenerator attachmentForLogMessages:logMessages
usingLogFormatter:self.logFormatter
logStoreName:logStoreName
numberOfErrorsInHighlights:numberOfErrorsInHighlights
completion:^(ARKBugReportAttachment *attachment) {
dispatch_async(dispatch_get_main_queue(), ^{
completionHandler(attachment);
Expand Down Expand Up @@ -278,12 +279,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 +327,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 +349,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 +365,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 +428,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
View file
Open in desktop

This file was deleted.

Loading
Loading