Skip to content

Conversation

ma-chni
Copy link

@ma-chni ma-chni commented Sep 17, 2025

This PR introduces a new method PatchClosedDate to ensure that the ClosedDate field is properly synchronized when migrating work items.

Key Changes

  • Added logic to check if the target work item is in a Closed or Done state before attempting to patch the ClosedDate.

  • Extracts the ClosedDate value from the source work item, prioritizing:

  • Microsoft.VSTS.Common.ClosedDate

  • System.ClosedDate

  • Ensures the target work item has an appropriate ClosedDate field (Microsoft.VSTS.Common.ClosedDate or System.ClosedDate).

  • Uses WorkItemTrackingHttpClient with PAT authentication to update the ClosedDate field in the target work item, bypassing rules when necessary.

  • Added logging for both success and failure cases to improve traceability.

Why

Previously, ClosedDate values were not being migrated, leading to missing or incorrect closure timestamps in target work items. This change ensures accurate historical data is preserved when migrating closed/done items.

Notes

  • If the target work item lacks both Microsoft.VSTS.Common.ClosedDate and System.ClosedDate, a warning is logged and no update is applied.

  • Errors during patching are caught and logged without interrupting the overall migration process.

@MrHinsh

@ma-chni ma-chni requested a review from MrHinsh as a code owner September 17, 2025 06:18
@CLAassistant
Copy link

CLAassistant commented Sep 17, 2025

CLA assistant check
All committers have signed the CLA.

@MrHinsh MrHinsh self-assigned this Sep 17, 2025
@MrHinsh MrHinsh requested a review from Copilot September 17, 2025 20:28
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds functionality to preserve ClosedDate field values when migrating work items, ensuring historical closure timestamps are maintained in the target work items. The implementation adds a new PatchClosedDate method that synchronizes the ClosedDate field after the primary work item migration is complete.

Key changes:

  • Added a post-migration step to patch ClosedDate values for work items in Closed or Done states
  • Implemented REST API-based patching using WorkItemTrackingHttpClient with bypass rules capability
  • Added comprehensive error handling and logging for the patching process

});

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.

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.

Comment on lines +965 to +967
var targetItem = targetWorkItem.ToWorkItem();
var state = targetItem.Fields["System.State"].Value?.ToString();
if (!(state == "Closed" || state == "Done"))
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.

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);
}

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.

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.

@MrHinsh MrHinsh marked this pull request as draft September 22, 2025 18:41
@MrHinsh MrHinsh removed their assignment Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants