Skip to content

Fix TfsSharedQueryProcessor to preserve area selection with @CurrentIteration macro #2716

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.

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using MigrationTools.Endpoints;
using MigrationTools.Tests;
using MigrationTools.Processors.Infrastructure;
using System.Reflection;
using System;

namespace MigrationTools.Processors.Tests
{
Expand Down Expand Up @@ -47,5 +49,54 @@ public void BasicRun()
x.Execute();
Assert.IsNotNull(x);
}

[TestMethod("TfsSharedQueryProcessorTests_PreserveMacros"), TestCategory("L0")]
public void PreserveMacros_WithCurrentIterationAndAreaPath_PreservesQueryStructure()
{
// This test verifies that @CurrentIteration macros with area path selections are preserved
var processor = GetTfsSharedQueryProcessor();

// The original query text with both @CurrentIteration and area path
string originalQuery = "SELECT [System.Id] FROM WorkItems WHERE [System.TeamProject] = @TeamProject AND [System.AreaPath] = 'ProjectArea' AND [System.IterationPath] = @CurrentIteration";

// Use reflection to access the private PreserveMacros method
MethodInfo preserveMacrosMethod = typeof(TfsSharedQueryProcessor).GetMethod("PreserveMacros",
BindingFlags.NonPublic | BindingFlags.Instance);

// Invoke the method to test it
string result = (string)preserveMacrosMethod.Invoke(processor, new object[] { originalQuery });

// Verify that the area path condition is preserved in the result
Assert.IsTrue(result.Contains("[System.AreaPath]"), "Area path condition should be preserved");
Assert.IsTrue(result.Contains("@CurrentIteration"), "@CurrentIteration macro should be preserved");
}

[TestMethod("TfsSharedQueryProcessorTests_PreserveMacros_Reordering"), TestCategory("L0")]
public void PreserveMacros_ReordersConditionsCorrectly()
{
// This test verifies that conditions are properly reordered when area path comes after iteration path
var processor = GetTfsSharedQueryProcessor();

// Original query with iteration path before area path (which causes the issue)
string originalQuery = "SELECT [System.Id] FROM WorkItems WHERE [System.TeamProject] = @TeamProject AND [System.IterationPath] = @CurrentIteration AND [System.AreaPath] = 'ProjectArea'";

// Use reflection to access the private PreserveMacros method
MethodInfo preserveMacrosMethod = typeof(TfsSharedQueryProcessor).GetMethod("PreserveMacros",
BindingFlags.NonPublic | BindingFlags.Instance);

// Invoke the method to test it
string result = (string)preserveMacrosMethod.Invoke(processor, new object[] { originalQuery });

// Verify that the area path condition is preserved and now comes before iteration path
Assert.IsTrue(result.Contains("[System.AreaPath]"), "Area path condition should be preserved");
Assert.IsTrue(result.Contains("@CurrentIteration"), "@CurrentIteration macro should be preserved");

// Check that area path now comes before iteration path in the reordered query
int areaPathIndex = result.IndexOf("[System.AreaPath]");
int iterationPathIndex = result.IndexOf("[System.IterationPath]");

Assert.IsTrue(areaPathIndex < iterationPathIndex,
"Area path condition should come before iteration path in the reordered query");
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Diagnostics;
using System.Linq;
using System.Text.RegularExpressions;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Microsoft.TeamFoundation.WorkItemTracking.Client;
Expand All @@ -26,7 +27,7 @@
private int _totalQueriesMigrated;
private int _totalQueryFailed;

public TfsSharedQueryProcessor(IOptions<TfsSharedQueryProcessorOptions> options, CommonTools commonTools, ProcessorEnricherContainer processorEnrichers, IServiceProvider services, ITelemetryLogger telemetry, ILogger<Processor> logger) : base(options, commonTools, processorEnrichers, services, telemetry, logger)

Check warning on line 30 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Update this logger to use its enclosing type. (https://rules.sonarsource.com/csharp/RSPEC-6672)

Check warning on line 30 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Update this logger to use its enclosing type. (https://rules.sonarsource.com/csharp/RSPEC-6672)
{
}

Expand All @@ -49,14 +50,14 @@
Log.LogInformation("Found {0} root level child WIQ folders", sourceQueryHierarchy.Count);
//////////////////////////////////////////////////

foreach (QueryFolder query in sourceQueryHierarchy)

Check warning on line 53 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Either change the type of 'query' to 'QueryItem' or iterate on a generic collection of type 'QueryFolder'. (https://rules.sonarsource.com/csharp/RSPEC-3217)

Check warning on line 53 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Either change the type of 'query' to 'QueryItem' or iterate on a generic collection of type 'QueryFolder'. (https://rules.sonarsource.com/csharp/RSPEC-3217)
{
MigrateFolder(targetQueryHierarchy, query, targetQueryHierarchy);
}

stopwatch.Stop();
Log.LogInformation("Folders scanned {totalFoldersAttempted}", _totalFoldersAttempted);

Check warning on line 59 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)

Check warning on line 59 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)
Log.LogInformation("Queries Found:{totalQueriesAttempted} Skipped:{totalQueriesSkipped} Migrated:{totalQueriesMigrated} Failed:{totalQueryFailed}", _totalQueriesAttempted, _totalQueriesSkipped, _totalQueriesMigrated, _totalQueryFailed);

Check warning on line 60 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)

Check warning on line 60 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)
Log.LogInformation("DONE in {Elapsed} seconds", stopwatch.Elapsed.ToString("c"));
}

Expand All @@ -71,7 +72,7 @@
// We only migrate non-private folders and their contents
if (sourceFolder.IsPersonal)
{
Log.LogInformation("Found a personal folder {sourceFolderName}. Migration only available for shared Team Query folders", sourceFolder.Name);

Check warning on line 75 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)

Check warning on line 75 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)
return;
}

Expand All @@ -81,7 +82,7 @@
var requiredPath = sourceFolder.Path.Replace($"{Source.Project}/", $"{Target.Project}/");

// Is the project name to be used in the migration as an extra folder level?
if (Options.PrefixProjectToNodes == true)

Check warning on line 85 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Remove the unnecessary Boolean literal(s). (https://rules.sonarsource.com/csharp/RSPEC-1125)

Check warning on line 85 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Remove the unnecessary Boolean literal(s). (https://rules.sonarsource.com/csharp/RSPEC-1125)
{
// we need to inject the team name as a folder in the structure
requiredPath = requiredPath.Replace(Options.SharedFolderName, $"{Options.SharedFolderName}/{Source.Project}");
Expand Down Expand Up @@ -145,9 +146,12 @@
if (parentFolder.FirstOrDefault(q => q.Name == query.Name) != null)
{
_totalQueriesSkipped++;
Log.LogWarning("Skipping query '{queryName}' as already exists", query.Name);

Check warning on line 149 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)

Check warning on line 149 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Use PascalCase for named placeholders. (https://rules.sonarsource.com/csharp/RSPEC-6678)
return;
}
// Get the original query text for debugging
string originalQueryText = query.QueryText;

// Sort out any path issues in the quertText
var fixedQueryText = query.QueryText.Replace($"'{Source.Project}", $"'{Target.Project}"); // the ' should only items at the start of areapath etc.

Expand All @@ -164,6 +168,12 @@
fixedQueryText = fixedQueryText.Replace(sourceField, Options.SourceToTargetFieldMappings[sourceField]);
}
}

// Special handling for @CurrentIteration macro to ensure it's preserved correctly
fixedQueryText = PreserveMacros(fixedQueryText);

Log.LogDebug("Original Query Text: '{queryText}'", originalQueryText);
Log.LogDebug("Fixed Query Text: '{fixedQueryText}'", fixedQueryText);

// you cannot just add an item from one store to another, we need to create a new object
var queryCopy = new QueryDefinition(query.Name, fixedQueryText);
Expand All @@ -178,11 +188,152 @@
catch (Exception ex)
{
_totalQueryFailed++;
Log.LogDebug("Source Query: '{query}'");
Log.LogDebug("Target Query: '{fixedQueryText}'");
Log.LogDebug("Source Query: '{query}'", originalQueryText);
Log.LogDebug("Target Query: '{fixedQueryText}'", fixedQueryText);
Log.LogError(ex, "Error saving query '{queryName}', probably due to invalid area or iteration paths", query.Name);
targetHierarchy.Refresh(); // get the tree without the last edit
}
}

/// <summary>
/// Ensures that special macros like @CurrentIteration are preserved in the query text
/// </summary>
/// <param name="queryText">The query text to process</param>
/// <returns>Updated query text with preserved macros</returns>
private string PreserveMacros(string queryText)

Check warning on line 203 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Refactor this method to reduce its Cognitive Complexity from 50 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)

Check warning on line 203 in src/MigrationTools.Clients.TfsObjectModel/Processors/TfsSharedQueryProcessor.cs

View workflow job for this annotation

GitHub Actions / Build, Test, Sonar Cloud Analysis, & Package

Refactor this method to reduce its Cognitive Complexity from 50 to the 15 allowed. (https://rules.sonarsource.com/csharp/RSPEC-3776)
{
// If the query doesn't contain @CurrentIteration, no special handling needed
if (!queryText.Contains("@CurrentIteration"))
{
return queryText;
}

Log.LogInformation("Found @CurrentIteration in query, applying special handling");

try
{
// Based on the issue, when a query contains @CurrentIteration, the area path selections are lost
// The specific issue shown in the screenshot is that the area selection is missing in the migrated query

// This is a direct fix for the issue where area path selection is missing with @CurrentIteration
// We need to ensure the area path conditions are properly maintained in the migrated query

// The issue appears when [System.IterationPath] = @CurrentIteration is used,
// which may cause area path selections to be dropped during migration

// The WIQL query syntax for @CurrentIteration can vary across different queries
// but the core issue is that area path selections get lost in the process

// Since the TFS Object Model may not fully support this scenario (as per Mr. Hinsh's comment),
// we need to ensure the area path selection is explicitly preserved
string areaPathPattern = @"(\[System\.AreaPath\]\s*(?:=|UNDER)\s*(?:'[^']+'|@\w+))";

// Look for area path conditions in the query
MatchCollection areaMatches = Regex.Matches(queryText, areaPathPattern);

if (areaMatches.Count > 0)
{
// The query has area path conditions that need to be preserved
Log.LogInformation("Found area path conditions with @CurrentIteration - ensuring they're preserved");

// In the WIQL parser, area path conditions might be dropped when @CurrentIteration is present
// This specific fix addresses that by restructuring the query to ensure area paths are preserved

// Extract the area path conditions and ensure they're properly formatted
foreach (Match match in areaMatches)
{
string areaCondition = match.Groups[1].Value;
Log.LogDebug("Area path condition: {0}", areaCondition);
}

// The key fix: Move @CurrentIteration references after area path conditions
// This ensures that area path selections aren't dropped during migration
int areaPathIndex = queryText.IndexOf("[System.AreaPath]", StringComparison.OrdinalIgnoreCase);
int iterationPathIndex = queryText.IndexOf("[System.IterationPath]", StringComparison.OrdinalIgnoreCase);

// Only reorder if both exist and area path comes after iteration path (which may cause the issue)
if (areaPathIndex > 0 && iterationPathIndex > 0 && areaPathIndex > iterationPathIndex)
{
Log.LogInformation("Reordering conditions to ensure area path is preserved");

// Extract the WHERE clause
int whereIndex = queryText.IndexOf("WHERE ", StringComparison.OrdinalIgnoreCase);
if (whereIndex > 0)
{
// Extract the part before WHERE
string beforeWhere = queryText.Substring(0, whereIndex + 6); // +6 includes "WHERE "

// Extract the conditions after WHERE
string whereConditions = queryText.Substring(whereIndex + 6);

// Split conditions by AND
string[] conditions = whereConditions.Split(new[] { " AND " }, StringSplitOptions.None);

// Identify area path and iteration path conditions
string areaPathCondition = null;
string iterationPathCondition = null;

for (int i = 0; i < conditions.Length; i++)
{
if (conditions[i].Contains("[System.AreaPath]"))
{
areaPathCondition = conditions[i];
conditions[i] = null; // Mark for removal
}
else if (conditions[i].Contains("[System.IterationPath]") && conditions[i].Contains("@CurrentIteration"))
{
iterationPathCondition = conditions[i];
conditions[i] = null; // Mark for removal
}
}

// Rebuild the WHERE conditions with area path before iteration path
string newWhereConditions = "";

// Add area path first if it exists
if (areaPathCondition != null)
{
newWhereConditions = areaPathCondition;
}

// Add iteration path next if it exists
if (iterationPathCondition != null)
{
if (newWhereConditions.Length > 0)
{
newWhereConditions += " AND ";
}
newWhereConditions += iterationPathCondition;
}

// Add other conditions
foreach (string condition in conditions)
{
if (!string.IsNullOrEmpty(condition))
{
if (newWhereConditions.Length > 0)
{
newWhereConditions += " AND ";
}
newWhereConditions += condition;
}
}

// Rebuild the full query
string newQueryText = beforeWhere + newWhereConditions;

Log.LogDebug("Reordered query: {0}", newQueryText);
return newQueryText;
}
}
}
}
catch (Exception ex)
{
Log.LogWarning(ex, "Error while processing @CurrentIteration in query");
}

return queryText;
}
}
}
Loading