diff --git a/src/WorkItemMigrator/JiraExport/IJiraProvider.cs b/src/WorkItemMigrator/JiraExport/IJiraProvider.cs index 83696f7a..2b9e0052 100644 --- a/src/WorkItemMigrator/JiraExport/IJiraProvider.cs +++ b/src/WorkItemMigrator/JiraExport/IJiraProvider.cs @@ -29,7 +29,11 @@ public interface IJiraProvider bool GetCustomFieldSerializer(string customType, out ICustomFieldValueSerializer serializer); + /// string GetCustomId(string propertyName); + /// + List GetCustomIdList(string propertyName); + Task>> DownloadAttachments(JiraRevision rev); IEnumerable GetCommitRepositories(string issueId); diff --git a/src/WorkItemMigrator/JiraExport/JiraMapper.cs b/src/WorkItemMigrator/JiraExport/JiraMapper.cs index 68438670..aa3cf79a 100644 --- a/src/WorkItemMigrator/JiraExport/JiraMapper.cs +++ b/src/WorkItemMigrator/JiraExport/JiraMapper.cs @@ -381,16 +381,19 @@ private HashSet InitializeTypeMappings() return types; } - private Func IfChanged(string sourceField, bool isCustomField, Func mapperFunc = null) + internal Func IfChanged(string sourceField, bool isCustomField, Func mapperFunc = null) { + List sourceFields = null; if (isCustomField) { - sourceField = _jiraProvider.GetCustomId(sourceField) ?? sourceField; + sourceFields = _jiraProvider.GetCustomIdList(sourceField); } + sourceFields = sourceFields ?? new List { sourceField }; + return (r) => { - if (r.Fields.TryGetValue(sourceField.ToLower(), out object value)) + if (r.Fields.TryGetFirstValue(sourceFields, out object value)) { if (mapperFunc != null) { diff --git a/src/WorkItemMigrator/JiraExport/JiraProvider.cs b/src/WorkItemMigrator/JiraExport/JiraProvider.cs index bd93cb0f..89c79e53 100644 --- a/src/WorkItemMigrator/JiraExport/JiraProvider.cs +++ b/src/WorkItemMigrator/JiraExport/JiraProvider.cs @@ -412,9 +412,30 @@ public string GetUserEmail(string usernameOrAccountId) } } + /// + /// Return the custom field id corresponding to the given field/property name. + /// + /// Blindly chooses the first matching value. This is often right, but doesn't handle the scenario where a + /// Jira instance has multiple custom fields with the same name. To handle that scenario, use instead. public string GetCustomId(string propertyName) { - var customId = string.Empty; + var allIds = GetCustomIdList(propertyName); + var customId = allIds.FirstOrDefault(); + if (allIds?.Count() > 1) + { + Logger.Log(LogLevel.Warning, $"Multiple fields found for {propertyName}. Selecting {customId}."); + } + + return customId; + } + + /// + /// Returns a list of custom field ids corresponding to the given field/property name. + /// + /// The property/field name of the custom field. + /// One or more custom field ids that match the name provided. + public List GetCustomIdList(string propertyName) + { JArray response = null; if (JiraNameFieldCache == null) @@ -423,19 +444,24 @@ public string GetCustomId(string propertyName) JiraNameFieldCache = CreateFieldCacheLookup(response, "name", "id"); } - customId = GetItemFromFieldCache(propertyName, JiraNameFieldCache); + var customIds = GetItemListFromFieldCache(propertyName, JiraNameFieldCache); - if (string.IsNullOrEmpty(customId)) + if (!customIds.Any()) { if (JiraKeyFieldCache == null) { response = response ?? (JArray)_jiraServiceWrapper.RestClient.ExecuteRequestAsync(Method.GET, $"{JiraApiV2}/field").Result; JiraKeyFieldCache = CreateFieldCacheLookup(response, "key", "id"); } - customId = GetItemFromFieldCache(propertyName, JiraKeyFieldCache); + customIds = GetItemListFromFieldCache(propertyName, JiraKeyFieldCache); + } + + if (customIds.Count == 0) + { + Logger.Log(LogLevel.Warning, $"Custom field {propertyName} could not be found."); } - return customId; + return customIds; } private ILookup CreateFieldCacheLookup(JArray response, string key, string value) @@ -446,19 +472,18 @@ private ILookup CreateFieldCacheLookup(JArray response, string k .ToLookup(l => l.key, l => l.value); } - private string GetItemFromFieldCache(string propertyName, ILookup cache) + /// + /// Returns a list of cache values corresponding to the given name. + /// + /// The name to lookup in the cache. + /// The cache to be interrogated. + /// + private List GetItemListFromFieldCache(string propertyName, ILookup cache) { - string customId = null; - var query = cache.FirstOrDefault(x => x.Key.Equals(propertyName.ToLower())); - if (query != null) - { - customId = query.Any() ? query.First() : null; - if (query.Count() > 1) - { - Logger.Log(LogLevel.Warning, $"Multiple fields found for {propertyName}. Selecting {customId}."); - } - } - return customId; + var fieldList = cache.FirstOrDefault(x => x.Key.Equals(propertyName.ToLower()))?.ToList() + ?? new List(0); + + return fieldList; } public IEnumerable GetCommitRepositories(string issueId) diff --git a/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs b/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs index 565e4976..bdc22cf3 100644 --- a/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs +++ b/src/WorkItemMigrator/JiraExport/RevisionUtils/FieldMapperUtils.cs @@ -12,8 +12,12 @@ using System.IO; using System.Linq; using System.Reflection; +using System.Runtime.CompilerServices; using System.Text.RegularExpressions; +[assembly: InternalsVisibleTo("Migration.Jira-Export.Tests")] + + namespace JiraExport { public static class FieldMapperUtils @@ -183,6 +187,27 @@ public static object MapSprint(string iterationPathsString) return iterationPath; } + /// + /// Find the first custom field with the right name that has a value. + /// + /// JiraRevision.Fields dictionary. + /// The IDs of the Custom Fields to search - more than one custom field can have the same name. + /// The value of the first field in the list that has a value. Null if no values are found. + /// True if one of the identified custom fields contains a value. + internal static bool TryGetFirstValue(this Dictionary fields, List customFieldIds, out object fieldValueObject) + { + foreach(var name in customFieldIds) + { + if (fields.TryGetValue(name, out fieldValueObject) && fieldValueObject != null) + { + return true; + } + } + + fieldValueObject = null; + return false; + } + private static readonly Dictionary CalculatedLexoRanks = new Dictionary(); private static readonly Dictionary CalculatedRanks = new Dictionary(); diff --git a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs index 942d86ca..1ec1f236 100644 --- a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs +++ b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/JiraMapperTests.cs @@ -8,6 +8,7 @@ using Newtonsoft.Json.Linq; using NSubstitute; using NUnit.Framework; +using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Linq; @@ -320,5 +321,107 @@ private JiraItem CreateJiraItem() return jiraItem; } + + + /* + * ******* IfChanged() ****** + */ + + [Test] + public void IfChanged_SourceFieldExists_ReturnsTrueAndValue() + { + // Arrange + var jiraMapper = createJiraMapper(); + //var revision = Substitute.For(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + revision.Fields.Add("sourceField", "value"); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("sourceField", false) (revision); + + // Assert + Assert.IsTrue(boolResult); + Assert.AreEqual("value", valueResult); + } + + [Test] + public void IfChanged_SourceFieldDoesNotExist_ReturnsFalseAndNull() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("sourceField", false) (revision); + + // Assert + Assert.IsFalse(boolResult); + Assert.IsNull(valueResult); + } + + [Test] + public void IfChanged_CustomFieldExists_ReturnsTrueAndValue() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + revision.Fields.Add("customField", "value"); + + var jiraProvider = Substitute.For(); + jiraProvider.GetCustomIdList("customField").Returns(new List { "customField" }); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("customField", true) (revision); + + // Assert + Assert.IsTrue(boolResult); + Assert.AreEqual("value", valueResult); + } + + [Test] + public void IfChanged_CustomFieldDoesNotExist_ReturnsFalseAndNull() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + + //_jiraProvider.GetCustomIdList("customField").Returns(new List { "customField" }); + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("customField", true) (revision); + + // Assert + Assert.IsFalse(boolResult); + Assert.IsNull(valueResult); + } + + [Test] + public void IfChanged_SourceFieldExistsWithMapperFunc_ReturnsTrueAndMappedValue() + { + // Arrange + var jiraMapper = createJiraMapper(); + var item = _fixture.Create(); + var revision = new JiraRevision(parentItem: null); + revision.Fields = new Dictionary(); + revision.Fields.Add("sourceField", 10); + + // mapper takes expected 10 value and concatenates "XX" on the end. + Func mapperFunc = (value) => value.ToString() + "XX"; + + // Act + var (boolResult, valueResult) = jiraMapper.IfChanged("sourceField", false, mapperFunc) (revision); + + // Assert + Assert.IsTrue(boolResult); + Assert.AreEqual("10XX", valueResult); + } } } \ No newline at end of file diff --git a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs index 9b9bb7e4..b7a5a3cd 100644 --- a/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs +++ b/src/WorkItemMigrator/tests/Migration.Jira-Export.Tests/RevisionUtils/FieldMapperUtilsTests.cs @@ -502,5 +502,132 @@ public void { Assert.That(FieldMapperUtils.MapLexoRank("0|hzyxfj:hzyxfj"), Is.EqualTo(1088341183.1088341M)); } + + + /** + **************** TryGetFirstValue Tests **************** + */ + + [Test] + public void TryGetFirstValue_ShouldReturnTrue_WhenMatchingFieldExists() + { + // Arrange + var fields = new Dictionary + { + { "field1", "value1" }, + { "field2", "value2" }, + { "field3", "value3" } + }; + var customFieldIds = new List { "field2", "field3" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsTrue(result); + Assert.AreEqual("value2", fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenNoMatchingFieldExists() + { + // Arrange + var fields = new Dictionary + { + { "field1", "value1" }, + { "field2", "value2" }, + { "field3", "value3" } + }; + var customFieldIds = new List { "field4", "field5" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenFieldsDictionaryIsEmpty() + { + // Arrange + var fields = new Dictionary(); + var customFieldIds = new List { "field1", "field2" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenCustomFieldIdsListIsEmpty() + { + // Arrange + var fields = new Dictionary + { + { "field1", "value1" }, + { "field2", "value2" }, + { "field3", "value3" } + }; + var customFieldIds = new List(); + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } + + [Test] + public void TryGetFirstValue_ShouldReturnTrue_WhenMultipleMatchingFieldsExistButOnlyOneHasNonNullValue() + { + // Arrange + var fields = new Dictionary + { + { "field1", null }, + { "field2", "value2" }, + { "field3", null } + }; + var customFieldIds = new List { "field1", "field3", "field2" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsTrue(result); + Assert.AreEqual("value2", fieldValueObject); + } + + + [Test] + public void TryGetFirstValue_ShouldReturnFalse_WhenMultipleMatchingFieldsExistButOnlyAllAreNullValue() + { + // Arrange + var fields = new Dictionary + { + { "field1", null }, + { "field2", null }, + { "field3", null } + }; + var customFieldIds = new List { "field1", "field3", "field2" }; + object fieldValueObject; + + // Act + var result = fields.TryGetFirstValue(customFieldIds, out fieldValueObject); + + // Assert + Assert.IsFalse(result); + Assert.IsNull(fieldValueObject); + } } } \ No newline at end of file