diff --git a/Mail2Bug/Config.cs b/Mail2Bug/Config.cs index 47d7a93..515e9d4 100644 --- a/Mail2Bug/Config.cs +++ b/Mail2Bug/Config.cs @@ -130,6 +130,7 @@ public enum ProcessingStrategyType public bool ApplyOverridesDuringUpdate { get; set; } public bool AttachOriginalMessage { get; set; } public bool AttachUpdateMessages { get; set; } + public bool EnableExperimentalHtmlFeatures { get; set; } public ProcessingStrategyType ProcessingStrategy = ProcessingStrategyType.SimpleBugStrategy; } diff --git a/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs b/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs index 0026371..4263d24 100644 --- a/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs +++ b/Mail2Bug/Email/EWS/EWSIncomingFileAttachment.cs @@ -28,6 +28,12 @@ public string SaveAttachmentToFile(string filename) return filename; } + public string ContentId + { + get => _attachment.ContentId; + set => _attachment.ContentId = value; + } + private static readonly ILog Logger = LogManager.GetLogger(typeof(EWSIncomingFileAttachment)); } } diff --git a/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs b/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs index 4bef1c2..0955f10 100644 --- a/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs +++ b/Mail2Bug/Email/EWS/EWSIncomingItemAttachment.cs @@ -38,6 +38,12 @@ public string SaveAttachmentToFile(string filename) return filename; } + public string ContentId + { + get => _attachment.ContentId; + set => _attachment.ContentId = value; + } + private static readonly ILog Logger = LogManager.GetLogger(typeof (EWSIncomingItemAttachment)); } } diff --git a/Mail2Bug/Email/EWS/EWSIncomingMessage.cs b/Mail2Bug/Email/EWS/EWSIncomingMessage.cs index e7f0c27..538eda5 100644 --- a/Mail2Bug/Email/EWS/EWSIncomingMessage.cs +++ b/Mail2Bug/Email/EWS/EWSIncomingMessage.cs @@ -116,9 +116,9 @@ public string SaveToFile(string filename) return filename; } - public string GetLastMessageText() + public string GetLastMessageText(bool enableExperimentalHtmlFeatures) { - return EmailBodyProcessingUtils.GetLastMessageText(this); + return EmailBodyProcessingUtils.GetLastMessageText(this, enableExperimentalHtmlFeatures); } public void Delete() diff --git a/Mail2Bug/Email/EmailBodyProcessingUtils.cs b/Mail2Bug/Email/EmailBodyProcessingUtils.cs index cbf9f49..c4020f2 100644 --- a/Mail2Bug/Email/EmailBodyProcessingUtils.cs +++ b/Mail2Bug/Email/EmailBodyProcessingUtils.cs @@ -3,12 +3,18 @@ using System.Linq; using System.Text; using System.Text.RegularExpressions; +using CsQuery; namespace Mail2Bug.Email { public class EmailBodyProcessingUtils { - public static string GetLastMessageText(IIncomingEmailMessage message) + public static string GetLastMessageText(IIncomingEmailMessage message, bool enableExperimentalHtmlFeatures) + { + return enableExperimentalHtmlFeatures && message.IsHtmlBody ? GetLastMessageText_Html(message.RawBody) : GetLastMessageText_PlainText(message); + } + + private static string GetLastMessageText_PlainText(IIncomingEmailMessage message) { var lastMessage = new StringBuilder(); lastMessage.Append(message.PlainTextBody); @@ -23,6 +29,63 @@ public static string GetLastMessageText(IIncomingEmailMessage message) return lastMessage.ToString(); } + public static string GetLastMessageText_Html(string rawBody) + { + CQ dom = rawBody; + + const string messageSeparatorStyle = "border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in"; + + foreach (IDomObject element in dom["*"]) + { + // Lots of email clients insert html elements as message delimiters which have styling but no inner text + // This block checks for some of these patterns + if (element.NodeName == "DIV") + { + if (element.Id == "divRplyFwdMsg" || element.Id == "x_divRplyFwdMsg" || messageSeparatorStyle.Equals(element.GetAttribute("style"))) + { + IDomContainer parent = element.ParentNode; + element.Remove(); + RemoveSubsequent(parent); + break; + } + } + + if (!element.ChildElements.Any() && !string.IsNullOrWhiteSpace(element.InnerText)) + { + var separatorIndex = IndexOfAny(element.InnerText, MessageBorderMarkers); + if (separatorIndex.HasValue) + { + element.InnerText = element.InnerText.Substring(0, separatorIndex.Value); + RemoveSubsequent(element); + break; + } + } + } + + return dom.Render(); + } + + private static void RemoveSubsequent(IDomObject element) + { + IDomObject nextNode; + while ((nextNode = element.NextSibling) != null) + { + nextNode.Remove(); + } + + if (element.ParentNode is IDomObject parentElement) + { + RemoveSubsequent(parentElement); + } + } + + private static readonly string[] MessageBorderMarkers = + { + "_____", + "-----Original Message", + "From:", + }; + /// /// Get the seperate denotation between reply content and original content. /// @@ -30,20 +93,17 @@ public static string GetLastMessageText(IIncomingEmailMessage message) /// private static int GetReplySeperatorIndex(string description) { - var indices = new List - { - description.IndexOf("_____", StringComparison.Ordinal), - description.IndexOf("-----Original Message", StringComparison.Ordinal), - description.IndexOf("From:", StringComparison.Ordinal) - }; - - indices.RemoveAll(x => x < 0); - if (indices.Count == 0) - { - return description.Length; - } + return IndexOfAny(description, MessageBorderMarkers) ?? description.Length; + } - return indices.Min(); + private static int? IndexOfAny(string text, IEnumerable searchTerms) + { + return searchTerms + .Select(search => text.IndexOf(search, StringComparison.Ordinal)) + .Where(index => index >= 0) + .OrderBy(index => index) + .Cast() + .FirstOrDefault(); } public static string ConvertHtmlMessageToPlainText(string text) @@ -78,5 +138,35 @@ private static string ResolveOridnalToChar(Match ordinalMatch) var unicodeChar = (char)int.Parse(ordinalMatch.Groups["ordinal"].Value); return new string(new[] {unicodeChar}); } + + /// + /// If an email embeds an email inline in its html, that embedded image won't display correctly unless we modify the html. + /// This method does that, given information about email's known attachments + /// + public static string UpdateEmbeddedImageLinks(string originalHtml, System.Collections.Generic.IReadOnlyCollection attachments) + { + if (attachments == null || attachments.Count == 0) + { + return originalHtml; + } + + CQ dom = originalHtml; + foreach (var attachment in attachments) + { + string originalImgSrc = $"cid:{attachment.ContentId}"; + var matchingImgLinks = dom[$"img[src$='{originalImgSrc}']"]; + + // This may point to the file on the local file-system if we haven't yet uploaded the attachment + // However, the work item APIs seem to magically 'just work' with this and update the URI to point to the uploaded location + // If for some reason that stops working, we'd need to either infer the uploaded URI or upload first and mutate the html afterward + var newSrc = new Uri(attachment.FilePath); + foreach (IDomObject img in matchingImgLinks) + { + img.SetAttribute("src", newSrc.AbsoluteUri); + } + } + + return dom.Render(); + } } } diff --git a/Mail2Bug/Email/IIncomingEmailAttachment.cs b/Mail2Bug/Email/IIncomingEmailAttachment.cs index 4c42cd5..d8b32a7 100644 --- a/Mail2Bug/Email/IIncomingEmailAttachment.cs +++ b/Mail2Bug/Email/IIncomingEmailAttachment.cs @@ -7,5 +7,6 @@ public interface IIncomingEmailAttachment { string SaveAttachmentToFile(); string SaveAttachmentToFile(string filename); + string ContentId { get; set; } } } diff --git a/Mail2Bug/Email/IIncomingEmailMessage.cs b/Mail2Bug/Email/IIncomingEmailMessage.cs index 143b708..05d1ab7 100644 --- a/Mail2Bug/Email/IIncomingEmailMessage.cs +++ b/Mail2Bug/Email/IIncomingEmailMessage.cs @@ -43,7 +43,7 @@ public interface IIncomingEmailMessage /// messages dropped, whereas the body includes all of that "history". /// /// - string GetLastMessageText(); + string GetLastMessageText(bool enableExperimentalHtmlFeatures); /// /// Deletes the message diff --git a/Mail2Bug/Email/MessageAttachmentCollection.cs b/Mail2Bug/Email/MessageAttachmentCollection.cs new file mode 100644 index 0000000..6d9ceb4 --- /dev/null +++ b/Mail2Bug/Email/MessageAttachmentCollection.cs @@ -0,0 +1,41 @@ +using System; +using System.CodeDom.Compiler; +using System.Collections.Generic; +using System.Linq; + +namespace Mail2Bug.Email +{ + /// + /// Collection of Exchange email attachments that have been downloaded locally + /// + public class MessageAttachmentCollection : IDisposable + { + private readonly List _attachments; + private readonly TempFileCollection _tempFileCollection; + + public IReadOnlyCollection Attachments => _attachments; + public IEnumerable LocalFilePaths => _attachments.Select(a => a.FilePath); + + public MessageAttachmentCollection() + { + _attachments = new List(); + _tempFileCollection = new TempFileCollection(); + } + + public void Add(string localFilePath, string contentId) + { + _attachments.Add(new MessageAttachmentInfo(localFilePath, contentId)); + _tempFileCollection.AddFile(localFilePath, keepFile: false); + } + + public void DeleteLocalFiles() + { + _tempFileCollection.Delete(); + } + + public void Dispose() + { + DeleteLocalFiles(); + } + } +} \ No newline at end of file diff --git a/Mail2Bug/Email/MessageAttachmentInfo.cs b/Mail2Bug/Email/MessageAttachmentInfo.cs new file mode 100644 index 0000000..3d8859b --- /dev/null +++ b/Mail2Bug/Email/MessageAttachmentInfo.cs @@ -0,0 +1,18 @@ +namespace Mail2Bug.Email +{ + /// + /// Represents basic information about an Exchange email attachment that has been downloaded locally and has a known exchange content id + /// + public class MessageAttachmentInfo + { + public MessageAttachmentInfo(string filePath, string contentId) + { + FilePath = filePath; + ContentId = contentId; + } + + public string FilePath { get; } + + public string ContentId { get; } + } +} \ No newline at end of file diff --git a/Mail2Bug/Mail2Bug.csproj b/Mail2Bug/Mail2Bug.csproj index 270c8e0..0369b6d 100644 --- a/Mail2Bug/Mail2Bug.csproj +++ b/Mail2Bug/Mail2Bug.csproj @@ -36,6 +36,9 @@ 4 + + ..\packages\CsQuery.1.3.4\lib\net40\CsQuery.dll + ..\packages\Hyak.Common.1.0.2\lib\net45\Hyak.Common.dll True @@ -320,6 +323,8 @@ + + diff --git a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs index 86c640b..51b5b69 100644 --- a/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs +++ b/Mail2Bug/MessageProcessingStrategies/SimpleBugStrategy.cs @@ -1,7 +1,6 @@ using System; using System.CodeDom.Compiler; using System.Collections.Generic; -using System.Globalization; using System.IO; using System.Linq; using log4net; @@ -49,31 +48,33 @@ private void NewWorkItem(IIncomingEmailMessage message) { var workItemUpdates = new Dictionary(); - InitWorkItemFields(message, workItemUpdates); + using (var attachments = SaveAttachments(message)) + { + InitWorkItemFields(message, workItemUpdates, attachments); - var workItemId = _workItemManager.CreateWorkItem(workItemUpdates); - Logger.InfoFormat("Added new work item {0} for message with subject: {1} (conversation index:{2})", - workItemId, message.Subject, message.ConversationId); + int workItemId = _workItemManager.CreateWorkItem(workItemUpdates, attachments); + Logger.InfoFormat("Added new work item {0} for message with subject: {1} (conversation index:{2})", + workItemId, message.Subject, message.ConversationId); - try - { - // Since the work item *has* been created, failures in this stage are not treated as critical - var overrides = new OverridesExtractor(_config).GetOverrides(message); - TryApplyFieldOverrides(overrides, workItemId); - ProcessAttachments(message, workItemId); + try + { + // Since the work item *has* been created, failures in this stage are not treated as critical + var overrides = new OverridesExtractor(_config).GetOverrides(message); + TryApplyFieldOverrides(overrides, workItemId); - if (_config.WorkItemSettings.AttachOriginalMessage) + if (_config.WorkItemSettings.AttachOriginalMessage) + { + AttachMessageToWorkItem(message, workItemId, "OriginalMessage"); + } + } + catch (Exception ex) { - AttachMessageToWorkItem(message, workItemId, "OriginalMessage"); + Logger.ErrorFormat("Exception caught while applying settings to work item {0}\n{1}", workItemId, ex); } - } - catch (Exception ex) - { - Logger.ErrorFormat("Exception caught while applying settings to work item {0}\n{1}", workItemId, ex); - } - var workItem = _workItemManager.GetWorkItemFields(workItemId); - _ackEmailHandler.SendAckEmail(message, workItem); + var workItem = _workItemManager.GetWorkItemFields(workItemId); + _ackEmailHandler.SendAckEmail(message, workItem); + } } private void AttachMessageToWorkItem(IIncomingEmailMessage message, int workItemId, string prefix) @@ -88,11 +89,11 @@ private void AttachMessageToWorkItem(IIncomingEmailMessage message, int workItem // Remove the file once we're done attaching it tfc.AddFile(filePath, false); - _workItemManager.AttachFiles(workItemId, new List { filePath }); + _workItemManager.AttachFiles(workItemId, new List { new MessageAttachmentInfo(filePath, string.Empty) }); } } - private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary workItemUpdates) + private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary workItemUpdates, MessageAttachmentCollection attachments) { var resolver = new SpecialValueResolver(message, _workItemManager.GetNameResolver()); @@ -101,10 +102,17 @@ private void InitWorkItemFields(IIncomingEmailMessage message, Dictionary overrides, int workItemId) @@ -118,7 +126,7 @@ private void TryApplyFieldOverrides(Dictionary overrides, int wo try { Logger.DebugFormat("Overrides found. Calling 'ModifyWorkItem'"); - _workItemManager.ModifyWorkItem(workItemId, "", overrides); + _workItemManager.ModifyWorkItem(workItemId, "", false, overrides, null); } catch (Exception ex) { @@ -139,20 +147,29 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) workItemUpdates["Changed By"] = resolver.Sender; } + bool isHtmlEnabled = _config.WorkItemSettings.EnableExperimentalHtmlFeatures; + bool commentIsHtml = message.IsHtmlBody && isHtmlEnabled; + + string lastMessageText = message.GetLastMessageText(isHtmlEnabled); if (_config.WorkItemSettings.ApplyOverridesDuringUpdate) { + // OverrideExtractor can't currently handle HTML input, so we need to make sure we pass it the plain text version + string lastMessagePlainText = commentIsHtml + ? message.GetLastMessageText(enableExperimentalHtmlFeatures: false) + : lastMessageText; + var extractor = new OverridesExtractor(_config); - var overrides = extractor.GetOverrides(message.GetLastMessageText()); + var overrides = extractor.GetOverrides(lastMessagePlainText); Logger.DebugFormat("Found {0} overrides for update message", overrides.Count); overrides.ToList().ForEach(x => workItemUpdates[x.Key] = x.Value); } - // Construct the text to be appended - _workItemManager.ModifyWorkItem(workItemId, message.GetLastMessageText(), workItemUpdates); - - ProcessAttachments(message, workItemId); + using (var attachments = SaveAttachments(message)) + { + _workItemManager.ModifyWorkItem(workItemId, lastMessageText, commentIsHtml, workItemUpdates, attachments); + } if (_config.WorkItemSettings.AttachUpdateMessages) { @@ -160,32 +177,24 @@ private void UpdateWorkItem(IIncomingEmailMessage message, int workItemId) } } - private void ProcessAttachments(IIncomingEmailMessage message, int workItemId) - { - var attachmentFiles = SaveAttachments(message); - _workItemManager.AttachFiles(workItemId, (from object file in attachmentFiles select file.ToString()).ToList()); - attachmentFiles.Delete(); - } - /// /// Take attachments from the current mail message and put them in a work item /// /// - private static TempFileCollection SaveAttachments(IIncomingEmailMessage message) + private static MessageAttachmentCollection SaveAttachments(IIncomingEmailMessage message) { - var attachmentFiles = new TempFileCollection(); - + var result = new MessageAttachmentCollection(); foreach (var attachment in message.Attachments) { var filename = attachment.SaveAttachmentToFile(); if (filename != null) { - attachmentFiles.AddFile(filename, false); + result.Add(filename, attachment.ContentId); Logger.InfoFormat("Attachment saved to file {0}", filename); } } - return attachmentFiles; + return result; } private static readonly ILog Logger = LogManager.GetLogger(typeof(SimpleBugStrategy)); diff --git a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs index 10df5e3..ec188a5 100644 --- a/Mail2Bug/WorkItemManagement/IWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/IWorkItemManager.cs @@ -1,24 +1,29 @@ using System.Collections.Generic; +using Mail2Bug.Email; using Mail2Bug.MessageProcessingStrategies; namespace Mail2Bug.WorkItemManagement { public interface IWorkItemManager { - void AttachFiles(int workItemId, List fileList); + void AttachFiles(int workItemId, IReadOnlyCollection fileList); SortedList WorkItemsCache { get; } void CacheWorkItem(int workItemId); /// Field Values + /// /// Bug ID - int CreateWorkItem(Dictionary values); + int CreateWorkItem(Dictionary values, MessageAttachmentCollection attachments); /// The ID of the bug to modify /// Comment to add to description + /// /// List of fields to change - void ModifyWorkItem(int workItemId, string comment, Dictionary values); + /// + void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values, + MessageAttachmentCollection attachments); INameResolver GetNameResolver(); diff --git a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs index 0c68071..18c9dce 100644 --- a/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs +++ b/Mail2Bug/WorkItemManagement/TFSWorkItemManager.cs @@ -6,6 +6,7 @@ using System.Net; using System.Text; using log4net; +using Mail2Bug.Email; using Mail2Bug.ExceptionClasses; using Mail2Bug.Helpers; using Mail2Bug.MessageProcessingStrategies; @@ -242,7 +243,7 @@ private string GetPatFromConfig() _config.TfsServerConfig.ServiceIdentityPatKeyVaultSecret); } - public void AttachFiles(int workItemId, List fileList) + public void AttachFiles(int workItemId, IReadOnlyCollection messageAttachments) { if (workItemId <= 0) return; @@ -251,7 +252,11 @@ public void AttachFiles(int workItemId, List fileList) WorkItem workItem = _tfsStore.GetWorkItem(workItemId); workItem.Open(); - fileList.ForEach(file => workItem.Attachments.Add(new Attachment(file))); + foreach (var attachment in messageAttachments) + { + workItem.Attachments.Add(new Attachment(attachment.FilePath)); + } + ValidateAndSaveWorkItem(workItem); } catch (Exception exception) @@ -261,8 +266,9 @@ public void AttachFiles(int workItemId, List fileList) } /// The list of fields and their desired values to apply to the work item + /// /// Work item ID of the newly created work item - public int CreateWorkItem(Dictionary values) + public int CreateWorkItem(Dictionary values, MessageAttachmentCollection attachments) { if (values == null) { @@ -294,6 +300,11 @@ public int CreateWorkItem(Dictionary values) TryApplyFieldValue(workItem, AssignedToFieldKey, values[AssignedToFieldKey]); } + foreach (var attachmentPath in attachments.LocalFilePaths) + { + workItem.Attachments.Add(new Attachment(attachmentPath)); + } + ValidateAndSaveWorkItem(workItem); CacheWorkItem(workItem); @@ -302,8 +313,10 @@ public int CreateWorkItem(Dictionary values) /// The ID of the work item to modify /// Comment to add to description + /// /// List of fields to change - public void ModifyWorkItem(int workItemId, string comment, Dictionary values) + /// + public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values, MessageAttachmentCollection attachments) { if (workItemId <= 0) return; @@ -311,12 +324,46 @@ public void ModifyWorkItem(int workItemId, string comment, Dictionary"); + if (commentIsHtml) + { + workItem.History = EmailBodyProcessingUtils.UpdateEmbeddedImageLinks(comment, attachments.Attachments); + } + else + { + workItem.History = comment.Replace("\n", "
"); + } + foreach (var key in values.Keys) { TryApplyFieldValue(workItem, key, values[key]); } + if (attachments != null) + { + string GetAttachmentKey(string fileName, long length) + { + return $"{fileName}|{length}"; + } + + var existingAttachments = new HashSet(workItem.Attachments + .OfType() + .Select(attachment => GetAttachmentKey(attachment.Name, attachment.Length))); + + foreach (var attachment in attachments.Attachments) + { + var localFileInfo = new FileInfo(attachment.FilePath); + string key = GetAttachmentKey(localFileInfo.Name, localFileInfo.Length); + + // If there's already an attachment with the same file name and size, don't bother re-uploading it + // TODO: this may break embedded images for html replies since we update the img tag above to point to a local path we don't upload + // However, we haven't confirmed this breaks and replying with an identical image is unlikely, so we ignore this for now + if (!existingAttachments.Contains(key)) + { + workItem.Attachments.Add(new Attachment(attachment.FilePath)); + } + } + } + ValidateAndSaveWorkItem(workItem); workItem.Save(); diff --git a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs index 623d273..43460a8 100644 --- a/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs +++ b/Mail2Bug/WorkItemManagement/WorkItemManagerMock.cs @@ -4,6 +4,7 @@ using System.IO; using System.Linq; using log4net; +using Mail2Bug.Email; using Mail2Bug.MessageProcessingStrategies; namespace Mail2Bug.WorkItemManagement @@ -20,9 +21,9 @@ public WorkItemManagerMock(string keyField, INameResolver resolver = null) WorkItemsCache = new SortedList(); } - public void AttachFiles(int workItemId, List fileList) + public void AttachFiles(int workItemId, IReadOnlyCollection fileList) { - foreach (var filename in fileList.Where(filename => !File.Exists(filename))) + foreach (var filename in fileList.Where(filename => !File.Exists(filename.FilePath))) { Logger.ErrorFormat("Couldn't find attachment file {0}", filename); } @@ -37,7 +38,7 @@ public void AttachFiles(int workItemId, List fileList) Attachments[workItemId] = new List(); } - Attachments[workItemId].AddRange(fileList); + Attachments[workItemId].AddRange(fileList.Select(f => f.FilePath)); } public void CacheWorkItem(int workItemId) @@ -54,7 +55,7 @@ public void CacheWorkItem(int workItemId) WorkItemsCache[conversationId] = workItemId; } - public int CreateWorkItem(Dictionary values) + public int CreateWorkItem(Dictionary values, MessageAttachmentCollection attachments) { if (ThrowOnCreateBug != null) throw ThrowOnCreateBug; @@ -80,7 +81,7 @@ public int CreateWorkItem(Dictionary values) return id; } - public void ModifyWorkItem(int workItemId, string comment, Dictionary values) + public void ModifyWorkItem(int workItemId, string comment, bool commentIsHtml, Dictionary values, MessageAttachmentCollection attachments) { if (ThrowOnModifyBug != null) throw ThrowOnModifyBug; diff --git a/Mail2Bug/packages.config b/Mail2Bug/packages.config index 2bb9b80..a8c4648 100644 --- a/Mail2Bug/packages.config +++ b/Mail2Bug/packages.config @@ -1,5 +1,6 @@  + diff --git a/Mail2BugUnitTests/AckTemplateUnitTest.cs b/Mail2BugUnitTests/AckTemplateUnitTest.cs index 3e946bb..305a706 100644 --- a/Mail2BugUnitTests/AckTemplateUnitTest.cs +++ b/Mail2BugUnitTests/AckTemplateUnitTest.cs @@ -130,7 +130,7 @@ public void TestPlaceholderWithDefault() fields["Assigned To"] = null; fields["Priority"] = null; - var placeholdersWithDefaults = placeholders.Where(p => p.Field.Equals("BugOwner") || p.Field.Equals("Priority")); + var placeholdersWithDefaults = placeholders.Where(p => p.Field.Equals("BugOwner") || p.Field.Equals("Priority")).ToList(); Assert.AreEqual(2, placeholdersWithDefaults.Count(), "Bad test data; all expected placeholders not found."); var workItem = new WorkItemFieldsMock(fields); diff --git a/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs b/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs index 40d6b11..a36ff08 100644 --- a/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs +++ b/Mail2BugUnitTests/EmailBodyProcessingUtilsUnitTest.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Text; @@ -30,7 +31,7 @@ public void TestGetLastMessageTextBasic() } message.PlainTextBody = bodyBuilder.ToString(); - Assert.AreEqual(lastMessageText, EmailBodyProcessingUtils.GetLastMessageText(message), "Verifying extracted last message text correctness"); + Assert.AreEqual(lastMessageText, EmailBodyProcessingUtils.GetLastMessageText(message, true), "Verifying extracted last message text correctness"); } [TestMethod] @@ -71,6 +72,90 @@ public void TestConvertHtmlMessageToHtmlRegression() } } + [TestMethod] + public void TestUpdateEmbeddedImageLinks_Basic() + { + string original = @" + + + +"; + + IReadOnlyCollection attachmentInfo = new List + { + new MessageAttachmentInfo(@"x:\image.png", "123"), + }; + + // Note: it's acceptable to not preserve whitespace / insert empty HTML tags because we're + // manipulating HTML, not plain text. As long as the rendered page isn't impacted, all is well + string expected = @" + + +"; + + string actual = EmailBodyProcessingUtils.UpdateEmbeddedImageLinks(original, attachmentInfo); + Assert.AreEqual(Normalize(expected), Normalize(actual)); + } + + [TestMethod] + public void TestGetLastMessageText_NoPrevious() + { + string original = @" + +This is a boring email. + +"; + + // Note: it's acceptable to not preserve whitespace / insert empty HTML tags because we're + // manipulating HTML, not plain text. As long as the rendered page isn't impacted, all is well + string expected = @" +This is a boring email. + +"; + + string actual = EmailBodyProcessingUtils.GetLastMessageText_Html(original); + Assert.AreEqual(Normalize(expected), Normalize(actual)); + } + + [TestMethod] + public void TestGetLastMessageText_Previous_FromColon() + { + string original = @" + +
+

This is random text with some custom styling and outlook-generated elements.

+
+
+

From: someAddress;
Subject: RE: Build error +

+
+
+

+   +

+

text of the reply +

+
+
Something after the containing div
+ +"; + + // Note: it's acceptable to not preserve whitespace because it's + // manipulating HTML, not plain text. As long as the rendered page isn't impacted, all is well + // Note that we expect that both + // 1. Elements following the latest message are removed + // 2. Anything in the same element as the latest message but after the start of the previous should be cleared out + string expected = @" +
+

This is random text with some custom styling and outlook-generated elements.

+
+
+

"; + + string actual = EmailBodyProcessingUtils.GetLastMessageText_Html(original); + Assert.AreEqual(Normalize(expected), Normalize(actual)); + } + private static string Normalize(string text) { var normalized = text.Replace("\r\n", "\n"); diff --git a/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs b/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs index 00d4bf4..4c0136b 100644 --- a/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs +++ b/Mail2BugUnitTests/Mocks/Email/IncomingAttachmentMock.cs @@ -37,6 +37,8 @@ public string SaveAttachmentToFile(string filename) return filename; } + public string ContentId { get; set; } + public Exception ExceptionToThrow { get; set; } public byte[] Data = new byte[1]; diff --git a/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs b/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs index 493da6c..d9f6f8a 100644 --- a/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs +++ b/Mail2BugUnitTests/Mocks/Email/IncomingEmailMessageMock.cs @@ -49,7 +49,7 @@ public static IncomingEmailMessageMock CreateWithRandomData(int numAttachments) mock.CcNames = GetRandomNamesList(mock.CcAddresses.Count()); mock.SentOn = new DateTime(Rand.Next(2012, 2525), Rand.Next(1, 12), Rand.Next(1, 28)); mock.ReceivedOn = new DateTime(Rand.Next(2012, 2525), Rand.Next(1, 12), Rand.Next(1, 28)); - mock.IsHtmlBody = Rand.Next(0, 1) == 0; + mock.IsHtmlBody = false; // this isn't html unless the creator specifically makes it so var attachments = new List(numAttachments); for (var i = 0; i < numAttachments; i++) @@ -103,9 +103,9 @@ public string SaveToFile(string filename) return filename; } - public string GetLastMessageText() + public string GetLastMessageText(bool enableExperimentalHtmlFeatures) { - return EmailBodyProcessingUtils.GetLastMessageText(this); + return EmailBodyProcessingUtils.GetLastMessageText(this, enableExperimentalHtmlFeatures); } public void CopyToFolder(string destinationFolder) diff --git a/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs b/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs index 11746dc..a80c2fa 100644 --- a/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs +++ b/Mail2BugUnitTests/SimpleBugStrategyUnitTest.cs @@ -201,7 +201,7 @@ public void TestProcessingEmailThreadImpl(bool overrideChangedBy) { expectedValues["Changed By"] = message3.SenderName; } - expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(message2.GetLastMessageText() + message3.GetLastMessageText()); + expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(message2.GetLastMessageText(true) + message3.GetLastMessageText(true)); ValidateBugValues(expectedValues, bugFields); } @@ -249,7 +249,7 @@ public void TestApplyingOverridesInUpdateMessage() expectedValues["Changed By"] = message4.SenderName; expectedValues[WorkItemManagerMock.HistoryField] = - TextUtils.FixLineBreaks(message2.GetLastMessageText() + message3.GetLastMessageText() + message4.GetLastMessageText()); + TextUtils.FixLineBreaks(message2.GetLastMessageText(true) + message3.GetLastMessageText(true) + message4.GetLastMessageText(true)); expectedValues[mnemonicDef.Field] = mnemonicDef.Value; expectedValues[explicitOverride1.Key] = explicitOverride1.Value; @@ -257,6 +257,39 @@ public void TestApplyingOverridesInUpdateMessage() Assert.IsFalse(bugFields.ContainsKey(explicitOverride2.Key)); } + [TestMethod] + public void TestApplyingOverridesInUpdateMessage_Html() + { + var seed = _rand.Next(); + + Logger.InfoFormat("Using seed {0}", seed); + + var mailManager = new MailManagerMock(); + var explicitOverride1 = new KeyValuePair("IsThisExplicit?","Indeed"); + var explicitLine1 = string.Format("\n###{0}:{1}", explicitOverride1.Key, explicitOverride1.Value); + var message1 = mailManager.AddMessage(false); + + var htmlText = string.Format("

{0}

", explicitLine1); + var message2 = mailManager.AddReply(message1, htmlText); + message2.IsHtmlBody = true; + message2.PlainTextBody = explicitLine1; + + var instanceConfig = GetConfig().Instances.First(); + instanceConfig.WorkItemSettings.ApplyOverridesDuringUpdate = true; + instanceConfig.WorkItemSettings.EnableExperimentalHtmlFeatures = true; + + var workItemManagerMock = new WorkItemManagerMock(instanceConfig.WorkItemSettings.ConversationIndexFieldName); + ProcessMailbox(mailManager, instanceConfig, workItemManagerMock); + + Assert.AreEqual(1, workItemManagerMock.Bugs.Count, "Only one bug should exist"); + var bug = workItemManagerMock.Bugs.First(); + var bugFields = bug.Value; + + Assert.IsTrue(bugFields.ContainsKey(explicitOverride1.Key)); + string actualValue = bugFields[explicitOverride1.Key]; + Assert.AreEqual(explicitOverride1.Value, actualValue); + } + [TestMethod] public void TestAttachingUpdateMessages() { @@ -284,7 +317,7 @@ public void TestAttachingUpdateMessages() expectedValues["Changed By"] = message3.SenderName; expectedValues[WorkItemManagerMock.HistoryField] = - TextUtils.FixLineBreaks(message2.GetLastMessageText() + message3.GetLastMessageText()); + TextUtils.FixLineBreaks(message2.GetLastMessageText(true) + message3.GetLastMessageText(true)); ValidateBugValues(expectedValues, bugFields); @@ -325,7 +358,7 @@ public void TestAppendOnlyMessageBody() var expectedValues = new Dictionary(); expectedValues["Changed By"] = appendOnlyMessage.SenderName; - expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText()); + expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText(false)); Assert.AreEqual(2, workItemManagerMock.Bugs.Count, "Only one bug should exist"); ValidateBugValues(expectedValues, workItemManagerMock.Bugs[newBugId]); @@ -388,7 +421,7 @@ public void TestAppendOnlyMessageSubject() var expectedValues = new Dictionary(); expectedValues["Changed By"] = appendOnlyMessage.SenderName; - expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText()); + expectedValues[WorkItemManagerMock.HistoryField] = TextUtils.FixLineBreaks(appendOnlyMessage.GetLastMessageText(true)); Assert.AreEqual(2, workItemManagerMock.Bugs.Count, "Only one bug should exist"); ValidateBugValues(expectedValues, workItemManagerMock.Bugs[newBugId]);