Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Fix notifications bugs #2534

Merged
merged 6 commits into from
Oct 20, 2022
Merged
Changes from 3 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
70 changes: 62 additions & 8 deletions src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public async IAsyncEnumerable<WorkItem> ExistingWorkItems() {
};

foreach (var workItemReference in (await _client.QueryByWiqlAsync(wiql)).WorkItems) {
var item = await _client.GetWorkItemAsync(_project, workItemReference.Id, expand: WorkItemExpand.Fields);
var item = await _client.GetWorkItemAsync(_project, workItemReference.Id, expand: WorkItemExpand.All);
tevoinea marked this conversation as resolved.
Show resolved Hide resolved

var loweredFields = item.Fields.ToDictionary(kvp => kvp.Key.ToLowerInvariant(), kvp => JsonSerializer.Serialize(kvp.Value));
if (postQueryFilter.Any() && !postQueryFilter.All(kvp => {
Expand Down Expand Up @@ -188,7 +188,7 @@ public async Async.Task UpdateExisting(WorkItem item, (string, string)[] notific
var fieldValue = await Render(_config.OnDuplicate.AdoFields[field.Key]);
document.Add(new JsonPatchOperation() {
Operation = VisualStudio.Services.WebApi.Patch.Operation.Replace,
Path = $"/fields/{field}",
Path = $"/fields/{field.Key}",
Value = fieldValue
});
}
Expand Down Expand Up @@ -265,18 +265,72 @@ private async Async.Task<WorkItem> CreateNew() {
}

public async Async.Task Process((string, string)[] notificationInfo) {
var seen = false;
await foreach (var workItem in ExistingWorkItems()) {
await UpdateExisting(workItem, notificationInfo);
seen = true;
var matchingWorkItems = ExistingWorkItems();

var nonDuplicate = matchingWorkItems
.Where(wi => !IsDuplicate(wi));

var numNonDuplicate = await nonDuplicate.CountAsync();

if (numNonDuplicate > 1) {
var nonDuplicateWorkItemIds = nonDuplicate
.Select(wi => wi.Id)
.ToEnumerable();

var matchingWorkItemIds = matchingWorkItems
.Select(wi => wi.Id)
.ToEnumerable();

var extraTags = new List<(string, string)> {
("NonDuplicateWorkItemIds", JsonSerializer.Serialize(nonDuplicateWorkItemIds)),
("MatchingWorkItemIds", JsonSerializer.Serialize(matchingWorkItemIds))
};
extraTags.AddRange(notificationInfo);

_logTracer.WithTags(extraTags).Info($"Found more than 1 matching, non-duplicate work item");
await foreach (var workItem in nonDuplicate) {
await UpdateExisting(workItem, notificationInfo);
}
} else if (numNonDuplicate == 1) {
await UpdateExisting(await nonDuplicate.SingleAsync(), notificationInfo);
}
// We have matching work items but all are duplicates
else if (await matchingWorkItems.AnyAsync()) {
_logTracer.WithTags(notificationInfo).Info($"All matching work items were duplicates, re-opening the oldest one");
var oldestWorkItem = await matchingWorkItems.OrderBy(wi => wi.Id).FirstAsync();
await UpdateExisting(oldestWorkItem, notificationInfo);

if (!seen) {
_ = await _client.AddCommentAsync(
new CommentCreate() {
Text = "This work item was re-opened because OneFuzz could only find related work items that are marked as duplicate."
},
_project,
(int)oldestWorkItem.Id!);
}
// We never saw a work item like this before, it must be new
else {
var entry = await CreateNew();
var adoEventType = "AdoNewItem";
_logTracer.WithTags(notificationInfo).Event($"{adoEventType} {entry.Id:Tag:WorkItemId}");

}
}

private static bool IsDuplicate(WorkItem wi) {
tevoinea marked this conversation as resolved.
Show resolved Hide resolved
// A work item could have System.State == Resolve && System.Reason == Duplicate
// OR it could have System.State == Closed && System.Reason == Duplicate
// I haven't found any other combinations where System.Reason could be duplicate but just to be safe
// we're explicitly _not_ checking the state of the work item to determine if it's duplicate
return (wi.Fields.ContainsKey("System.Reason") && string.Equals(wi.Fields["System.Reason"].ToString(), "Duplicate"))
// Alternatively, the work item can also specify a 'relation' to another work item.
// This is typically used to create parent/child relationships between work items but can also
// Be used to mark duplicates so we should check this as well.
// ADO has 2 relation types relating to duplicates: "Duplicate" and "Duplicate Of"
// When work item A has a link type "Duplicate" to work item B, B automatically gains a link type "Duplicate Of" pointing to A
// It's my understanding that the work item containing the "Duplicate" link should be the original while work items containing
// "Duplicate Of" are the duplicates. That is why we search for the relation type "Duplicate Of".
// "Duplicate Of" has the relation type: "System.LinkTypes.Duplicate-Forward"
// Source: https://learn.microsoft.com/en-us/azure/devops/boards/queries/link-type-reference?view=azure-devops#work-link-types
|| (wi.Relations != null && wi.Relations.Any(relation => string.Equals(relation.Rel, "System.LinkTypes.Duplicate-Forward")));
}
}
}