Skip to content

Add patching for ClosedDate when migrating work items #2991

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add patching for ClosedDate when migrating work items #2991

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7fc46d0
Add patching for ClosedDate when migrating work items
ma-chni Sep 17, 2025
bacb055
Merge branch 'main' into fix-closed-date
MrHinsh Sep 17, 2025
19a5009
Merge branch 'main' into fix-closed-date
MrHinsh Oct 7, 2025
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
60 changes: 60 additions & 0 deletions src/MigrationTools.Clients.TfsObjectModel/Processors/TfsWorkItemMigrationProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -876,6 +876,7 @@ private WorkItemData ReplayRevisions(List<RevisionItem> revisionsToMigrate, Work
targetWorkItem.ToWorkItem().History = history.ToString();
}
targetWorkItem.SaveToAzureDevOps();
PatchClosedDate(sourceWorkItem, targetWorkItem);
Copy link
Member

Choose a reason for hiding this comment

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

This call needs the rest API and needs to be wrapped in a ProductVersion check for not being OnPremisesClassic

Copy link
Author

Choose a reason for hiding this comment

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

I’ve tested this with TFS and confirmed it works. I’ll also run a DevOps → DevOps test to make sure the patch behaves correctly in that environment. That way we’ll cover both major product versions.

Copy link
Member

Choose a reason for hiding this comment

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

We support TFS 2013 which has no rest API's at all.


CommonTools.Attachment.CleanUpAfterSave();
TraceWriteLine(LogEventLevel.Information, "...Saved as {TargetWorkItemId}", new Dictionary<string, object> { { "TargetWorkItemId", targetWorkItem.Id } });
Expand Down Expand Up @@ -961,6 +962,65 @@ private void CheckClosedDateIsValid(WorkItemData sourceWorkItem, WorkItemData ta
}
}

private void PatchClosedDate(WorkItemData sourceWorkItem, WorkItemData targetWorkItem)
{
var targetItem = targetWorkItem.ToWorkItem();
var state = targetItem.Fields["System.State"].Value?.ToString();
Copy link
Member

Choose a reason for hiding this comment

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

This will only work with Closed or Done. It will not work with a custom closed state name

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for pointing that out. For now, the intention was to only support "Closed" and "Done". If we want to handle custom closed state names, could you point me to where we define or retrieve the custom field/state mappings in this project? I’d be happy to extend the implementation to use that instead of hardcoding values.

Copy link
Member

Choose a reason for hiding this comment

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

Its something like:

using Microsoft.TeamFoundation.WorkItemTracking.Client;
using System.Xml;

var project = Target.WorkItems.Project.ToProject();
var workItemType = project.WorkItemTypes[destType];

// Get the full XML definition of the work item type
string xml = workItemType.Export(false);

// Load into XmlDocument for parsing
var doc = new XmlDocument();
doc.LoadXml(xml);

// Select all <STATE> nodes with category="Completed"
var completedStates = doc
    .SelectNodes("//WORKFLOW/STATES/STATE[@category='Completed']")
    .Cast<XmlNode>()
    .Select(node => node.Attributes["value"].Value)
    .ToList();

// Example output
foreach (var state in completedStates)
{
    Console.WriteLine(state);
}

if (!(state == "Closed" || state == "Done"))
Comment on lines +967 to +969
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

The targetItem.ToWorkItem() call is repeated multiple times throughout the method. Consider storing it in a variable at the beginning of the method to improve readability and avoid redundant calls.

Copilot uses AI. Check for mistakes.

{
return;
}

object srcClosedDate = null;
if (sourceWorkItem.ToWorkItem().Fields.Contains("Microsoft.VSTS.Common.ClosedDate"))
{
srcClosedDate = sourceWorkItem.ToWorkItem().Fields["Microsoft.VSTS.Common.ClosedDate"].Value;
}
if (srcClosedDate == null && sourceWorkItem.ToWorkItem().Fields.Contains("System.ClosedDate"))
{
srcClosedDate = sourceWorkItem.ToWorkItem().Fields["System.ClosedDate"].Value;
}
if (srcClosedDate == null)
{
return;
}

try
{
ValidatePatTokenRequirement();
Uri collectionUri = Target.Options.Collection;
string token = Target.Options.Authentication.AccessToken;
VssConnection connection = new(collectionUri, new VssBasicCredential(string.Empty, token));
WorkItemTrackingHttpClient workItemTrackingClient = connection.GetClient<WorkItemTrackingHttpClient>();
JsonPatchDocument patchDocument = [];
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider using explicit constructor syntax new JsonPatchDocument() instead of collection expression for better compatibility with older C# versions and clearer intent.

Suggested change
JsonPatchDocument patchDocument = [];
JsonPatchDocument patchDocument = new JsonPatchDocument();

Copilot uses AI. Check for mistakes.


var closedDateFieldRef = targetItem.Fields.Contains("Microsoft.VSTS.Common.ClosedDate")
? "Microsoft.VSTS.Common.ClosedDate"
: (targetItem.Fields.Contains("System.ClosedDate") ? "System.ClosedDate" : null);

if (closedDateFieldRef == null)
{
Log.LogWarning("Cannot patch ClosedDate: no appropriate field on target.");
return;
}

patchDocument.Add(new JsonPatchOperation
Copy link
Member

Choose a reason for hiding this comment

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

The patch needs an additional date field set immediately after the previous operation. Without it, any subsequent revisions created after the patch will be unable to migrate.

Copy link
Author

Choose a reason for hiding this comment

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

Right now I’m only patching ClosedDate. Could you clarify which additional date field you mean should also be set? Is it ChangedDate, StateChangeDate, or another system field? Once I know the exact field, I can update the patch accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

When patching a work item to fix a field (for example, ClosedDate), the system creates a new revision at the current timestamp.

If later revisions already exist, this causes a problem: those revisions may no longer be migratable because the revision history is no longer consistent.

To avoid this, the patched revision must:

  • Use the ChangedDate of the original revision being corrected.
  • Insert between the original revision and the next one in sequence.

For example, if revision 4 is the one being corrected, the patch must produce a revision between 4 and 5, not at the end of the chain.

Revision history:

1
2
3
4 - Closed (patched)
5
6

Without this ordering, any future migration will fail because the timeline is broken.

{
Operation = Operation.Add,
Path = $"/fields/{closedDateFieldRef}",
Value = srcClosedDate
});

int id = int.Parse(targetWorkItem.Id);
var result = workItemTrackingClient.UpdateWorkItemAsync(patchDocument, id, bypassRules: true).Result;
Copy link

Copilot AI Sep 17, 2025

Choose a reason for hiding this comment

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

Using .Result on async methods can cause deadlocks in certain contexts. Consider making the PatchClosedDate method async and using await, or use ConfigureAwait(false) if the method must remain synchronous.

Copilot uses AI. Check for mistakes.

Log.LogInformation("Patched ClosedDate for work item {id}.", id);
}
catch (Exception ex)
{
Log.LogWarning(ex, "Failed to patch ClosedDate for {id}.", targetWorkItem.Id);
}
}

private bool SkipRevisionWithInvalidIterationPath(WorkItemData targetWorkItemData)
{
if (!Options.SkipRevisionWithInvalidIterationPath)
Expand Down