diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs index ef6d16fb98e..ef99656cd8d 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/MockGitHubEventClient.cs @@ -63,6 +63,18 @@ public override Task ProcessPendingUpdates(long repositoryId, int issueOrPu { Console.WriteLine("MockGitHubEventClient::ProcessPendingUpdates, Issue Update is null"); } + + if (_labelsToAdd.Count > 0) + { + Console.WriteLine($"MockGitHubEventClient::ProcessPendingUpdates, number of labels to add = {_labelsToAdd.Count} (only 1 call)"); + // Adding labels is a single call to add them all + numUpdates++; + } + + Console.WriteLine($"MockGitHubEventClient::ProcessPendingUpdates, number of labels to remove = {_labelsToRemove.Count}"); + // Removing labels is a call for each one being removed + numUpdates += _labelsToRemove.Count; + Console.WriteLine($"MockGitHubEventClient::ProcessPendingUpdates, number of pending comments = {_gitHubComments.Count}"); numUpdates += _gitHubComments.Count; @@ -434,6 +446,24 @@ public IssueUpdate GetIssueUpdate() return _issueUpdate; } + /// + /// Convenience function for testing, get labels to add stored on the GitHubEventClient + /// + /// List of strings + public List GetLabelsToAdd() + { + return _labelsToAdd; + } + + /// + /// Convenience function for testing, get labels to remove stored on the GitHubEventClient + /// + /// List of strings + public List GetLabelsToRemove() + { + return _labelsToRemove; + } + /// /// Convenience function for testing, get the list of GitHub issues to update. For normal action /// processing this list won't be used as actions make changes to a common IssueUpdate. For scheduled, diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueCommentProcessingTests.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueCommentProcessingTests.cs index c2d78616414..3e03e8296f6 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueCommentProcessingTests.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueCommentProcessingTests.cs @@ -38,25 +38,18 @@ public async Task TestAuthorFeedback(string rule, string payloadFile, RuleState Assert.AreEqual(ruleState == RuleState.On, mockGitHubEventClient.RulesConfiguration.RuleEnabled(rule), $"Rule '{rule}' enabled should have been {ruleState == RuleState.On} but RuleEnabled returned {ruleState != RuleState.On}.'"); var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number); - var numComments = mockGitHubEventClient.GetComments().Count; if (RuleState.On == ruleState) { - // There should be two updates, an IssueUpdate with the label updates and a single comment - Assert.AreEqual(2, totalUpdates, $"The number of updates should have been 2 but was instead, {totalUpdates}"); - Assert.AreEqual(1, numComments, $"{rule} should have created a single comment but {numComments} comments were created."); - - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTeamAttention} removed and {LabelConstants.NeedsAuthorFeedback} added."); + // There should be 2 updates, one label being added and one + Assert.AreEqual(2, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}"); // Verify that NeedsTeamAttention was added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label whcih should have been added."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to Add list does not contain {LabelConstants.NeedsTeamAttention}."); // Verify that NeedsAuthorFeedback was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsAuthorFeedback), $"IssueUpdate contains {LabelConstants.NeedsAuthorFeedback} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsAuthorFeedback), $"Lables to Remove list does not contain {LabelConstants.NeedsAuthorFeedback}."); } else { - Assert.AreEqual(0, numComments, $"{rule} is {ruleState} and should not have created any comments but {numComments} comments were created."); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); + Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have been any updates but {totalUpdates} comments were created."); } } @@ -84,19 +77,15 @@ public async Task TestResetIssueActivity(string rule, string payloadFile, RuleSt var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the label NoRecentActivity removed + // There should be one update, the label NoRecentActivity removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"IssueUpdate is null. {rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NoRecentActivity} removed."); - // Verify that NoRecentActivity was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + // Verify that NoRecentActivity is in the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove list does not contain {LabelConstants.NoRecentActivity}."); } else { Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}"); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } @@ -125,25 +114,24 @@ public async Task TestReopenIssue(string rule, string payloadFile, RuleState rul var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the labels NeedsAuthorFeedback and NoRecentActivity removed - // and NeedsTeamAttention added - Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); + // There should be 4 updates, the issue state change, 1 label added and 2 labels removed + Assert.AreEqual(4, totalUpdates, $"The number of updates should have been 4 but was instead, {totalUpdates}"); // Retrieve the IssueUpdate and verify the expected changes var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"IssueUpdate is null. {rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsAuthorFeedback} and {LabelConstants.NoRecentActivity} removed and {LabelConstants.NeedsTeamAttention} added."); + Assert.IsNotNull(issueUpdate, $"IssueUpdate is null. {rule} is {ruleState} and should have produced an IssueUpdate with its State being {ItemState.Open}."); + Assert.AreEqual(issueUpdate.State, ItemState.Open, $"Issue's State should be {ItemState.Open} but was {issueUpdate.State}"); // Verify that the NeedsTeamAttention label was added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to add should contain {LabelConstants.NeedsTeamAttention} and does not."); // Verify that NeedsAuthorFeedback and NoRecentActivity labels were removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsAuthorFeedback), $"IssueUpdate contains {LabelConstants.NeedsAuthorFeedback} label which should have been removed."); - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsAuthorFeedback), $"Lables to remove should contain {LabelConstants.NeedsAuthorFeedback} and does not."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Lables to remove should contain {LabelConstants.NoRecentActivity} and does not."); } else { - Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}"); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); + Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have produced any updates but produced {totalUpdates} updates."); } } @@ -221,24 +209,20 @@ public async Task TestIssueAddressedCommands_SameUser(string rule, string payloa var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should only be 1 update, the IssueUpdate - Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); + // There should 3 updates, the IssueUpdate with State set to ItemState.Open, 1 label removed and 1 added + Assert.AreEqual(3, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}"); var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - // Verify the IssueUpdate is not null - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate."); - // Verify the IssueUpdate contains the following changes: - // State = ItemState.Open + // Verify the IssueUpdate's State = ItemState.Open Assert.AreEqual(issueUpdate.State, ItemState.Open, $"IssueUpdate's state should be ItemState.Open and was not."); // IssueAddressed label has been removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.IssueAddressed), $"IssueUpdate contains {LabelConstants.IssueAddressed} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.IssueAddressed), $"Labels to remove should contain {LabelConstants.IssueAddressed} and does not."); // NeedsTeamAttention has been added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to add should contain {LabelConstants.NeedsTeamAttention} and does not."); } else { Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}"); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } @@ -268,26 +252,23 @@ public async Task TestIssueAddressedCommands_DifferentUser(string rule, string p var numComments = mockGitHubEventClient.GetComments().Count; if (userHasAdminOrWritePermission) { - // There should only be 1 update, the IssueUpdate - Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); + + // There should 3 updates, the IssueUpdate with State set to ItemState.Open, 1 label removed and 1 added + Assert.AreEqual(3, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}"); var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - // Verify the IssueUpdate is not null - Assert.IsNotNull(issueUpdate, $"{rule} is should have produced an IssueUpdate."); - // Verify the IssueUpdate contains the following changes: - // State = ItemState.Open + // Verify the IssueUpdate's State = ItemState.Open Assert.AreEqual(issueUpdate.State, ItemState.Open, $"IssueUpdate's state should be ItemState.Open and was not."); // IssueAddressed label has been removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.IssueAddressed), $"IssueUpdate contains {LabelConstants.IssueAddressed} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.IssueAddressed), $"Labels to remove should contain {LabelConstants.IssueAddressed} and does not."); // NeedsTeamAttention has been added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Labels to add should contain {LabelConstants.NeedsTeamAttention} and does not."); } else { // If the user does not have admin or write permission then there should be a single comment Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); Assert.AreEqual(1, numComments, $"Without admin or write permissions, 1 comment should have been created but {numComments} comments were created."); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"Without admin or write permissions there should not be an IssueUpdate."); } } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueProcessingTests.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueProcessingTests.cs index bbdc9f00479..5fa838548d6 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueProcessingTests.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/IssueProcessingTests.cs @@ -30,13 +30,18 @@ public class IssueProcessingTests : ProcessingTestBase /// Whether or not the rule is on/off /// Whether or not the AI Service should return labels [Category("static")] - [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, true)] - [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, false)] - [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.Off, false)] - public async Task TestInitialIssueTriage(string rule, string payloadFile, RuleState ruleState, bool AIServiceReturnsLabels) + [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, true, true, true)] + [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, false, true, true)] + [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.Off, false, false, false)] + [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, false, false, true)] + [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, false, true, false)] + [TestCase(RulesConstants.InitialIssueTriage, "Tests.JsonEventPayloads/InitialIssueTriage_issue_opened_no_labels_no_assignee.json", RuleState.On, false, false, false)] + public async Task TestInitialIssueTriage(string rule, string payloadFile, RuleState ruleState, bool AIServiceReturnsLabels, bool isMemberOfOrg, bool hasWriteOrAdmin) { var mockGitHubEventClient = new MockGitHubEventClient(OrgConstants.ProductHeaderName); mockGitHubEventClient.RulesConfiguration.Rules[rule] = ruleState; + mockGitHubEventClient.UserHasPermissionsReturn = hasWriteOrAdmin; + mockGitHubEventClient.IsUserMemberOfOrgReturn = isMemberOfOrg; var rawJson = TestHelpers.GetTestEventPayload(payloadFile); var issueEventPayload = SimpleJsonSerializer.Deserialize(rawJson); List expectedLabels = new List @@ -58,22 +63,35 @@ public async Task TestInitialIssueTriage(string rule, string payloadFile, RuleSt Assert.AreEqual(ruleState == RuleState.On, mockGitHubEventClient.RulesConfiguration.RuleEnabled(rule), $"Rule '{rule}' enabled should have been {ruleState == RuleState.On} but RuleEnabled returned {ruleState != RuleState.On}.'"); if (RuleState.On == ruleState) { + // The only update is labels being added, the number of which depends on the AI label serice returning labels + // and whether the user's org and collaborator permissions Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); if (AIServiceReturnsLabels) { - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamTriage), $"IssueUpdate does not contain {LabelConstants.NeedsTeamTriage} which should have been added when labels were predicted."); - // Verify the labels returned by the AI service + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamTriage), $"Labels to add should contain {LabelConstants.NeedsTeamTriage} which should have been added when labels were predicted."); + // Verify the labels returned by the AI service have been added foreach (string label in expectedLabels) { - Assert.True(issueUpdate.Labels.Contains(label), $"IssueUpdate does not contain {label} which was returned by the AI service and should have been added."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(label), $"Labels to add should contain {label} which was returned by the AI service and should have been added."); } } else { // If the AI Label service doesn't predict labels, the label added is NeedsTriage - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTriage), $"IssueUpdate does not contain {LabelConstants.NeedsTriage} which should have been added when no labels were predicted."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTriage), $"Labels to add should contain {LabelConstants.NeedsTriage} which should have been added when no labels were predicted."); + } + + // If the user is not part of the Azure org AND they don't have write or admin collaborator permissions + // then customer-reported and question labels should be added to the issue + if (!isMemberOfOrg && !hasWriteOrAdmin) + { + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.CustomerReported), $"Labels to add should contain {LabelConstants.CustomerReported} which it should when the user is not part of the org and doesn't have write/admin collaborator permissions."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.Question), $"Labels to add should contain {LabelConstants.Question} which it should when the user is not part of the org and doesn't have write/admin collaborator permissions."); + } + else + { + Assert.False(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.CustomerReported), $"Labels to add contains {LabelConstants.CustomerReported} and shouldn't when the user is part of the org or has write/admin collaborator permissions."); + Assert.False(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.Question), $"Labels to add contains {LabelConstants.Question} and shouldn't when the user is part of the org or has write/admin collaborator permissions."); } } else @@ -124,12 +142,8 @@ public async Task TestManualIssueTriage(string rule, string payloadFile, RuleSta { // There should be one update, an IssueUpdate with the NoRecentActivity label removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTriage} removed."); // Verify that NeedsTriage was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTriage), $"IssueUpdate contains {LabelConstants.NeedsTriage} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTriage), $"Labels to remove should contain {LabelConstants.NeedsTriage} and does not."); } } else @@ -248,10 +262,6 @@ public async Task TestCXPAttention(string rule, string payloadFile, RuleState ru { // There should be one comment created and no other updates Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Verify that no issues update was produced - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNull(issueUpdate, $"{rule} should not have produced an IssueUpdate."); - // Verify that a single comment was created Assert.AreEqual(1, mockGitHubEventClient.GetComments().Count, $"{rule} should have produced a single comment."); } @@ -269,6 +279,9 @@ public async Task TestCXPAttention(string rule, string payloadFile, RuleState ru /// Conditions: Issue is open /// Has "customer-reported" label /// Label removed is "Service Attention" OR "CXP Attention" + /// Issue does not have "Service Attention" OR "CXP Attention" + /// (in other words if both labels are on the issue and one is removed, this + /// shouldn't process) /// Resulting Action: Add "needs-team-triage" label /// /// String, RulesConstants for the rule being tested @@ -276,12 +289,14 @@ public async Task TestCXPAttention(string rule, string payloadFile, RuleState ru /// Whether or not the rule is on/off /// Whether or not the payload already has the needs-team-triage label [Category("static")] - [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention.json", RuleState.Off, false)] - [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention.json", RuleState.On, false)] - [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention.json", RuleState.Off, false)] - [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention.json", RuleState.On, false)] - [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_has_needs-team-triage.json", RuleState.On, true)] - public async Task TestManualTriageAfterExternalAssignment(string rule, string payloadFile, RuleState ruleState, bool alreadyHasNeedsTeamTriage) + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention.json", RuleState.Off, false, false)] + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention.json", RuleState.On, false, true)] + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention_has_service_attention.json", RuleState.On, false, false)] + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention.json", RuleState.Off, false, false)] + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention.json", RuleState.On, false, true)] + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention_has_CXP_attention.json", RuleState.On, false, false)] + [TestCase(RulesConstants.ManualTriageAfterExternalAssignment, "Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_has_needs-team-triage.json", RuleState.On, true, false)] + public async Task TestManualTriageAfterExternalAssignment(string rule, string payloadFile, RuleState ruleState, bool alreadyHasNeedsTeamTriage, bool shouldAddLabel) { var mockGitHubEventClient = new MockGitHubEventClient(OrgConstants.ProductHeaderName); mockGitHubEventClient.RulesConfiguration.Rules[rule] = ruleState; @@ -302,13 +317,17 @@ public async Task TestManualTriageAfterExternalAssignment(string rule, string pa } else { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed - Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTeamTriage} added."); - // Verify that NeedsTeamTriage was added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamTriage), $"IssueUpdate does not the {LabelConstants.NeedsTeamTriage} label which should have been added."); + if (shouldAddLabel) + { + // There should be one update, the label NoRecentActivity should have been added + Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); + // Verify that NeedsTeamTriage was added + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamTriage), $"Labels to add should contain {LabelConstants.NeedsTeamTriage} and does not."); + } + else + { + Assert.AreEqual(0, totalUpdates, $"The issue only 1 of {LabelConstants.CXPAttention} or {LabelConstants.ServiceAttention}. With the other still being on the issue there should have been no updates."); + } } } else @@ -347,14 +366,11 @@ public async Task TestResetIssueActivity(string rule, string payloadFile, RuleSt var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueEventPayload.Repository.Id, issueEventPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed + // There should be one update, the NoRecentActivity label removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NoRecentActivity} removed."); // Verify that NoRecentActivity was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove should contain {LabelConstants.NoRecentActivity} and does not."); } else { @@ -398,14 +414,11 @@ public async Task TestRequireAttentionForNonMilestone(string rule, string payloa var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueEventPayload.Repository.Id, issueEventPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed + // There should be one update, the NeedsTeamAttention label added Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTeamAttention} added."); // Verify that NeedsTeamAttention was added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate does not contain {LabelConstants.NeedsTeamAttention} label which should have been added."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.NeedsTeamAttention), $"Lables to add should contain {LabelConstants.NeedsTeamAttention} and does not."); } else { @@ -446,24 +459,26 @@ public async Task TestAuthorFeedbackNeeded(string rule, string payloadFile, Rule var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueEventPayload.Repository.Id, issueEventPayload.Issue.Number); if (RuleState.On == ruleState) { - // If there are no labels to remove, there should be no updates + // If there are no labels to remove, there should be 1 update which is a comment if (!hasLabelsToRemove) { - Assert.AreEqual(0, totalUpdates, $"The label being removed, {LabelConstants.NeedsTriage}, was not on the Issue and should have not produced any updates."); + Assert.AreEqual(1, totalUpdates, $"With no labels to remove there only be 1 update, a comment."); } else { - // There should be one update, an IssueUpdate with the NeedsTriage, NeedsTeamTriage and NeedsTeamAttention labels removed. - Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); + // There should be 4 updates, NeedsTriage, NeedsTeamTriage and NeedsTeamAttention labels removed and a comment added + Assert.AreEqual(4, totalUpdates, $"The number of updates should have been 3 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTriage} removed."); // Verify that NeedsTriage, NeedsTeamTriage and NeedsTeamAttention were removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTriage), $"IssueUpdate contains {LabelConstants.NeedsTriage} label which should have been removed."); - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamTriage), $"IssueUpdate contains {LabelConstants.NeedsTeamTriage} label which should have been removed."); - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate contains {LabelConstants.NeedsTeamAttention} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTriage), $"Labels to remove should contain {LabelConstants.NeedsTriage} and does not."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTeamTriage), $"Labels to remove should contain {LabelConstants.NeedsTeamTriage} and does not."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTeamAttention), $"Labels to remove should contain {LabelConstants.NeedsTeamAttention} and does not."); } + + // The comment should be created regardless + int numComments = mockGitHubEventClient.GetComments().Count; + Assert.AreEqual(1, numComments, $"There should have been one comment created but instead there were {numComments} created."); + } else { @@ -478,7 +493,7 @@ public async Task TestAuthorFeedbackNeeded(string rule, string payloadFile, Rule /// Conditions: Issue is open /// Label added is "issue-addressed" /// Resulting Action: - /// Remove "needs-triage" label + /// Remove "needs-triage" label if it exists /// Remove "needs-team-triage" label /// Remove "needs-team-attention" label /// Remove "needs-author-feedback" label @@ -514,23 +529,20 @@ public async Task TestIssueAddressed(string rule, string payloadFile, RuleState } else { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed - Assert.AreEqual(2, totalUpdates, $"The number of updates should have been 2 but was instead, {totalUpdates}"); - - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NeedsTriage} removed."); - - // Verify that NeedsTriage was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTriage), $"IssueUpdate contains {LabelConstants.NeedsTriage} label which should have been removed."); - // Verify that NeedsTeamTriage was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamTriage), $"IssueUpdate contains {LabelConstants.NeedsTeamTriage} label which should have been removed."); - // Verify that NeedsTeamAttention was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsTeamAttention), $"IssueUpdate contains {LabelConstants.NeedsTeamAttention} label which should have been removed."); - // Verify that NeedsAuthorFeedback was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NeedsAuthorFeedback), $"IssueUpdate contains {LabelConstants.NeedsAuthorFeedback} label which should have been removed."); - // Verify that NoRecentActivity was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + // There should be one comment and up to 5 labels removed, the no-recent-activity and all needs-* labels removed + // (the test payload has them all but real events will only remove the ones that are there) + Assert.AreEqual(6, totalUpdates, $"The number of updates should have been 2 but was instead, {totalUpdates}"); + + // Verify that NeedsTriage was added to the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTriage), $"Labels to remove should contain {LabelConstants.NeedsTriage} and does not."); + // Verify that NeedsTeamTriage was added to the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTeamTriage), $"Labels to remove should contain {LabelConstants.NeedsTeamTriage} and does not."); + // Verify that NeedsTeamAttention was added to the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsTeamAttention), $"Labels to remove should contain {LabelConstants.NeedsTeamAttention} and does not."); + // Verify that NeedsAuthorFeedback was added to the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NeedsAuthorFeedback), $"Labels to remove should contain {LabelConstants.NeedsAuthorFeedback} and does not."); + // Verify that NoRecentActivity was added to the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove should contain {LabelConstants.NoRecentActivity} and does not."); } // Regardless of whether or not there were labels to remove, a single comment should be created. Assert.AreEqual(1, mockGitHubEventClient.GetComments().Count, $"{rule} should have produced a single comment."); @@ -582,14 +594,11 @@ public async Task TestIssueAddressedReset(string rule, string payloadFile, RuleS var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(issueEventPayload.Repository.Id, issueEventPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed + // There should be one update, the NoRecentActivity label removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.IssueAddressed} removed."); - // Verify that IssueAddressed was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.IssueAddressed), $"IssueUpdate contains {LabelConstants.IssueAddressed} label which should have been removed."); + // Verify that IssueAddressed was added to the remove list + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.IssueAddressed), $"Labels to remove should contain {LabelConstants.IssueAddressed} and does not."); } else { diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestCommentProcessingTests.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestCommentProcessingTests.cs index f92f3960ba0..30d2eb4b54b 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestCommentProcessingTests.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestCommentProcessingTests.cs @@ -47,13 +47,10 @@ public async Task TestResetPullRequestActivity(string rule, string payloadFile, { if (commenterHasPermissionOrIsAuthor) { - // There should be one update, an IssueUpdate with the no-recent-activity removed + // There should be one update, the no-recent-activity label removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NoRecentActivity} removed."); // Verify that NeedsAuthorFeedback was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove should contain {LabelConstants.NoRecentActivity} and does not."); } else { @@ -63,7 +60,6 @@ public async Task TestResetPullRequestActivity(string rule, string payloadFile, else { Assert.AreEqual(0, totalUpdates, $"The number of updates should have been 0 but was instead, {totalUpdates}"); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } @@ -97,24 +93,24 @@ public async Task TestReopenPullRequest(string rule, string payloadFile, RuleSta var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(prCommentPayload.Repository.Id, prCommentPayload.Issue.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the no-recent-activity removed - Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - if (commenterHasPermissionOrIsAuthor) { + // There should be two updates, an IssueUpdate with the State set to ItemState.Open and the no-recent-activity label removed + Assert.AreEqual(2, totalUpdates, $"If the commenter has permissions or is the author, the number of updates should have been 2 but was instead, {totalUpdates}"); + var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); // Verify the IssueUpdate is not null Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate."); // Verify the IssueUpdate contains the following changes: // State = ItemState.Open - Assert.AreEqual(issueUpdate.State, ItemState.Open, $"IssueUpdate's state should be ItemState.Open and was not."); + Assert.AreEqual(issueUpdate.State, ItemState.Open, $"IssueUpdate's state should be {ItemState.Open} and was not."); // Verify that NoRecentActivity was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove should contain {LabelConstants.NoRecentActivity} and does not."); } else { - // Verify the IssueUpdate is null - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should have produced an IssueUpdate when the commenter isn't the issue author and doesn't have collaborator permissions."); + // If the commenter isn't the author and doesn't have permissions then there should be 1 comment created + Assert.AreEqual(1, totalUpdates, $"If the commenter has permissions or is the author, there should be have been 1 update, a comment, but was instead {totalUpdates}"); // There should be a single comment created int numComments = mockGitHubEventClient.GetComments().Count; Assert.AreEqual(1, numComments, $"There should have been a single comment created but instead {numComments} were created."); @@ -123,7 +119,6 @@ public async Task TestReopenPullRequest(string rule, string payloadFile, RuleSta else { Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have produced any updates."); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestProcessingTests.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestProcessingTests.cs index fa27ad5619b..ccd5269e8a6 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestProcessingTests.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestProcessingTests.cs @@ -35,14 +35,17 @@ public class PullRequestProcessingTests : ProcessingTestBase /// Whether or not the PR creator has write or admin permissions [Category("static")] [NonParallelizable] - [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.On, true)] - [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.On, false)] - [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.Off, false)] - public async Task TestPullRequestTriage(string rule, string payloadFile, RuleState ruleState, bool hasWriteOrAdmin) + [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.On, true, true)] + [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.Off, false, false)] + [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.On, true, false)] + [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.On, false, true)] + [TestCase(RulesConstants.PullRequestTriage, "Tests.JsonEventPayloads/PullRequestTriage_pr_opened_no_labels.json", RuleState.On, false, false)] + public async Task TestPullRequestTriage(string rule, string payloadFile, RuleState ruleState, bool isMemberOfOrg, bool hasWriteOrAdmin) { var mockGitHubEventClient = new MockGitHubEventClient(OrgConstants.ProductHeaderName); mockGitHubEventClient.RulesConfiguration.Rules[rule] = ruleState; mockGitHubEventClient.UserHasPermissionsReturn = hasWriteOrAdmin; + mockGitHubEventClient.IsUserMemberOfOrgReturn = isMemberOfOrg; var rawJson = TestHelpers.GetTestEventPayload(payloadFile); var prEventPayload = PullRequestProcessing.DeserializePullRequest(rawJson, SimpleJsonSerializer); @@ -69,10 +72,10 @@ public async Task TestPullRequestTriage(string rule, string payloadFile, RuleSta // which means an issueUpdate will be created int expectedUpdates = 1; - if (hasWriteOrAdmin) + if (hasWriteOrAdmin || isMemberOfOrg) { // There should be one update, an IssueUpdate with the NoRecentActivity label removed - Assert.AreEqual(expectedUpdates, totalUpdates, $"The number of updates for a user having Write or Admin permission should have been {expectedUpdates} but was instead, {totalUpdates}"); + Assert.AreEqual(expectedUpdates, totalUpdates, $"The number of updates for a user having Write or Admin permission or being a member of Azure org should have been {expectedUpdates} but was instead, {totalUpdates}"); } // If the user doesn't have Write or Admin permissions then "customer-reported" and "Community Contribution" labels // will be added and a single comment will be created @@ -80,36 +83,33 @@ public async Task TestPullRequestTriage(string rule, string payloadFile, RuleSta { expectedUpdates++; // Along with the label updates, there should also be a comment added. - Assert.AreEqual(expectedUpdates, totalUpdates, $"The number of updates for a user without Write or Admin permission should have been {expectedUpdates} but was instead, {totalUpdates}"); + Assert.AreEqual(expectedUpdates, totalUpdates, $"The number of updates for a user without Write or Admin permission or being a member of Azure org should have been {expectedUpdates} but was instead, {totalUpdates}"); } - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with added labels."); // Regardless of permissions, all of the labels based on PR file paths should be added foreach (string label in prFilesAndLabels.Values.ToList()) { - Assert.True(issueUpdate.Labels.Contains(label), $"label {label} should have been added because of the file paths in the PR but was not."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(label), $"Labels to add should contain {label} which should have been added because of the file paths in the PR but was not."); } - if (hasWriteOrAdmin) + // If the user is not part of the Azure org AND they don't have write or admin collaborator permissions + // then customer-reported and community-contribution labels should have been added along with a comment + if (!isMemberOfOrg && !hasWriteOrAdmin) { - Assert.False(issueUpdate.Labels.Contains(LabelConstants.CustomerReported), $"User has write or admin permission, IssueUpdate should not contain {LabelConstants.CustomerReported}."); - Assert.False(issueUpdate.Labels.Contains(LabelConstants.CommunityContribution), $"User has write or admin permission, IssueUpdate should not contain {LabelConstants.CommunityContribution}."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.CustomerReported), $"User does not have write or admin permission, IssueUpdate should contain {LabelConstants.CustomerReported}."); + Assert.True(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.CommunityContribution), $"User does not have write or admin permission, IssueUpdate should contain {LabelConstants.CommunityContribution}."); + Assert.AreEqual(1, mockGitHubEventClient.GetComments().Count, "Without admin or write permission there should have been a comment added."); } else { - // Without Admin or Write permissions there should be two additional labels and a comment added - Assert.True(issueUpdate.Labels.Contains(LabelConstants.CustomerReported), $"User does not have write or admin permission, IssueUpdate should contain {LabelConstants.CustomerReported}."); - Assert.True(issueUpdate.Labels.Contains(LabelConstants.CommunityContribution), $"User does not have write or admin permission, IssueUpdate should contain {LabelConstants.CommunityContribution}."); - Assert.AreEqual(1, mockGitHubEventClient.GetComments().Count, "Without admin or write permission there should have been a comment added."); + Assert.False(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.CustomerReported), $"User has write or admin permission or is a member of Azure, IssueUpdate should not contain {LabelConstants.CustomerReported}."); + Assert.False(mockGitHubEventClient.GetLabelsToAdd().Contains(LabelConstants.CommunityContribution), $"User has write or admin permission or is a member of Azure, IssueUpdate should not contain {LabelConstants.CommunityContribution}."); + Assert.AreEqual(0, mockGitHubEventClient.GetComments().Count, "User has write or admin permission or is a member of Azure, there should not have been a comment added."); } - } else { Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have produced any updates."); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } @@ -146,19 +146,15 @@ public async Task TestResetPullRequestActivity(string rule, string payloadFile, var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(prEventPayload.Repository.Id, prEventPayload.PullRequest.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed + // There should be one update, the NoRecentActivity label removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NoRecentActivity} removed."); // Verify that NeedsAuthorFeedback was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove should contain {LabelConstants.NoRecentActivity} and does not."); } else { Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have produced any updates."); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestReviewProcessingTests.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestReviewProcessingTests.cs index 97d8309e693..9a99a7f391f 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestReviewProcessingTests.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Static/PullRequestReviewProcessingTests.cs @@ -41,19 +41,15 @@ public async Task TestResetPullRequestActivity(string rule, string payloadFile, var totalUpdates = await mockGitHubEventClient.ProcessPendingUpdates(prReviewEventPayload.Repository.Id, prReviewEventPayload.PullRequest.Number); if (RuleState.On == ruleState) { - // There should be one update, an IssueUpdate with the NoRecentActivity label removed + // There should be one update, the NoRecentActivity label removed Assert.AreEqual(1, totalUpdates, $"The number of updates should have been 1 but was instead, {totalUpdates}"); - // Retrieve the IssueUpdate and verify the expected changes - var issueUpdate = mockGitHubEventClient.GetIssueUpdate(); - Assert.IsNotNull(issueUpdate, $"{rule} is {ruleState} and should have produced an IssueUpdate with {LabelConstants.NoRecentActivity} removed."); // Verify that NeedsAuthorFeedback was removed - Assert.False(issueUpdate.Labels.Contains(LabelConstants.NoRecentActivity), $"IssueUpdate contains {LabelConstants.NoRecentActivity} label which should have been removed."); + Assert.True(mockGitHubEventClient.GetLabelsToRemove().Contains(LabelConstants.NoRecentActivity), $"Labels to remove should contain {LabelConstants.NoRecentActivity} and does not."); } else { Assert.AreEqual(0, totalUpdates, $"{rule} is {ruleState} and should not have produced any updates."); - Assert.IsNull(mockGitHubEventClient.GetIssueUpdate(), $"{rule} is {ruleState} and should not have produced an IssueUpdate."); } } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention_has_service_attention.json b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention_has_service_attention.json new file mode 100644 index 00000000000..450cb7bb62b --- /dev/null +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_CXP_attention_has_service_attention.json @@ -0,0 +1,284 @@ +{ + "action": "unlabeled", + "issue": { + "active_lock_reason": null, + "assignee": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + }, + "assignees": [ + { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + } + ], + "author_association": "OWNER", + "body": null, + "closed_at": null, + "comments": 0, + "comments_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/comments", + "created_at": "2023-01-27T17:01:30Z", + "events_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/events", + "html_url": "https://github.com/Azure/azure-sdk-fake/issues/14", + "id": 1560095682, + "labels": [ + { + "color": "d73a4a", + "default": true, + "description": "Something isn't working", + "id": 4273699693, + "name": "bug", + "node_id": "LA_kwDOHkcrQs7-u3tt", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/bug" + }, + { + "color": "ededed", + "default": false, + "description": null, + "id": 4976488245, + "name": "customer-reported", + "node_id": "LA_kwDOHkcrQs8AAAABKJ8vNQ", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/customer-reported" + }, + { + "color": "ededed", + "default": false, + "description": null, + "id": 4704569627, + "name": "needs-triage", + "node_id": "LA_kwDOHkcrQs8AAAABGGoJGw", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/needs-triage" + }, + { + "color": "9C7082", + "default": false, + "description": "fake label for testing", + "id": 5095712487, + "name": "FakeLabel1", + "node_id": "LA_kwDOHkcrQs8AAAABL7pm5w", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/FakeLabel1" + }, + { + "color": "F75BB2", + "default": false, + "description": "", + "id": 5095715984, + "name": "Service Attention", + "node_id": "LA_kwDOHkcrQs8AAAABL7p0kA", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/Service%20Attention" + } + ], + "labels_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/labels{/name}", + "locked": false, + "milestone": null, + "node_id": "I_kwDOHkcrQs5c_SvC", + "number": 14, + "performed_via_github_app": null, + "reactions": { + "+1": 0, + "-1": 0, + "confused": 0, + "eyes": 0, + "heart": 0, + "hooray": 0, + "laugh": 0, + "rocket": 0, + "total_count": 0, + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/reactions" + }, + "repository_url": "https://api.github.com/repos/Azure/azure-sdk-fake", + "state": "open", + "state_reason": null, + "timeline_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/timeline", + "title": "New test issue to generate event payloads", + "updated_at": "2023-01-30T16:36:13Z", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14", + "user": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + } + }, + "label": { + "color": "FDEB99", + "default": false, + "description": "", + "id": 5095753141, + "name": "CXP Attention", + "node_id": "LA_kwDOHkcrQs8AAAABL7sFtQ", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/CXP%20Attention" + }, + "repository": { + "allow_forking": true, + "archive_url": "https://api.github.com/repos/Azure/azure-sdk-fake/{archive_format}{/ref}", + "archived": false, + "assignees_url": "https://api.github.com/repos/Azure/azure-sdk-fake/assignees{/user}", + "blobs_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/Azure/azure-sdk-fake/branches{/branch}", + "clone_url": "https://github.com/Azure/azure-sdk-fake.git", + "collaborators_url": "https://api.github.com/repos/Azure/azure-sdk-fake/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/Azure/azure-sdk-fake/comments{/number}", + "commits_url": "https://api.github.com/repos/Azure/azure-sdk-fake/commits{/sha}", + "compare_url": "https://api.github.com/repos/Azure/azure-sdk-fake/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/Azure/azure-sdk-fake/contents/{+path}", + "contributors_url": "https://api.github.com/repos/Azure/azure-sdk-fake/contributors", + "created_at": "2022-06-27T16:19:29Z", + "default_branch": "main", + "deployments_url": "https://api.github.com/repos/Azure/azure-sdk-fake/deployments", + "description": "Tools repository leveraged by the Azure SDK team.", + "disabled": false, + "downloads_url": "https://api.github.com/repos/Azure/azure-sdk-fake/downloads", + "events_url": "https://api.github.com/repos/Azure/azure-sdk-fake/events", + "fork": true, + "forks": 0, + "forks_count": 0, + "forks_url": "https://api.github.com/repos/Azure/azure-sdk-fake/forks", + "full_name": "Azure/azure-sdk-fake", + "git_commits_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/tags{/sha}", + "git_url": "git://github.com/Azure/azure-sdk-fake.git", + "has_discussions": false, + "has_downloads": true, + "has_issues": true, + "has_pages": false, + "has_projects": true, + "has_wiki": true, + "homepage": null, + "hooks_url": "https://api.github.com/repos/Azure/azure-sdk-fake/hooks", + "html_url": "https://github.com/Azure/azure-sdk-fake", + "id": 507980610, + "is_template": false, + "issue_comment_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/events{/number}", + "issues_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues{/number}", + "keys_url": "https://api.github.com/repos/Azure/azure-sdk-fake/keys{/key_id}", + "labels_url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels{/name}", + "language": "C#", + "languages_url": "https://api.github.com/repos/Azure/azure-sdk-fake/languages", + "license": { + "key": "mit", + "name": "MIT License", + "node_id": "MDc6TGljZW5zZTEz", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit" + }, + "merges_url": "https://api.github.com/repos/Azure/azure-sdk-fake/merges", + "milestones_url": "https://api.github.com/repos/Azure/azure-sdk-fake/milestones{/number}", + "mirror_url": null, + "name": "azure-sdk-fake", + "node_id": "R_kgDOHkcrQg", + "notifications_url": "https://api.github.com/repos/Azure/azure-sdk-fake/notifications{?since,all,participating}", + "open_issues": 6, + "open_issues_count": 6, + "owner": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + }, + "private": false, + "pulls_url": "https://api.github.com/repos/Azure/azure-sdk-fake/pulls{/number}", + "pushed_at": "2023-01-27T16:33:00Z", + "releases_url": "https://api.github.com/repos/Azure/azure-sdk-fake/releases{/id}", + "size": 29098, + "ssh_url": "git@github.com:Azure/azure-sdk-fake.git", + "stargazers_count": 0, + "stargazers_url": "https://api.github.com/repos/Azure/azure-sdk-fake/stargazers", + "statuses_url": "https://api.github.com/repos/Azure/azure-sdk-fake/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/Azure/azure-sdk-fake/subscribers", + "subscription_url": "https://api.github.com/repos/Azure/azure-sdk-fake/subscription", + "svn_url": "https://github.com/Azure/azure-sdk-fake", + "tags_url": "https://api.github.com/repos/Azure/azure-sdk-fake/tags", + "teams_url": "https://api.github.com/repos/Azure/azure-sdk-fake/teams", + "topics": [], + "trees_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/trees{/sha}", + "updated_at": "2023-01-23T19:54:18Z", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake", + "visibility": "public", + "watchers": 0, + "watchers_count": 0, + "web_commit_signoff_required": false + }, + "sender": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + } +} diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention_has_CXP_attention.json b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention_has_CXP_attention.json new file mode 100644 index 00000000000..3e59398e358 --- /dev/null +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor.Tests/Tests.JsonEventPayloads/ManualTriageAfterExternalAssignment_issue_unlabeled_service_attention_has_CXP_attention.json @@ -0,0 +1,284 @@ +{ + "action": "unlabeled", + "issue": { + "active_lock_reason": null, + "assignee": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + }, + "assignees": [ + { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + } + ], + "author_association": "OWNER", + "body": null, + "closed_at": null, + "comments": 0, + "comments_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/comments", + "created_at": "2023-01-27T17:01:30Z", + "events_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/events", + "html_url": "https://github.com/Azure/azure-sdk-fake/issues/14", + "id": 1560095682, + "labels": [ + { + "color": "d73a4a", + "default": true, + "description": "Something isn't working", + "id": 4273699693, + "name": "bug", + "node_id": "LA_kwDOHkcrQs7-u3tt", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/bug" + }, + { + "color": "ededed", + "default": false, + "description": null, + "id": 4976488245, + "name": "customer-reported", + "node_id": "LA_kwDOHkcrQs8AAAABKJ8vNQ", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/customer-reported" + }, + { + "color": "ededed", + "default": false, + "description": null, + "id": 4704569627, + "name": "needs-triage", + "node_id": "LA_kwDOHkcrQs8AAAABGGoJGw", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/needs-triage" + }, + { + "color": "9C7082", + "default": false, + "description": "fake label for testing", + "id": 5095712487, + "name": "FakeLabel1", + "node_id": "LA_kwDOHkcrQs8AAAABL7pm5w", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/FakeLabel1" + }, + { + "color": "FDEB99", + "default": false, + "description": "", + "id": 5095753141, + "name": "CXP Attention", + "node_id": "LA_kwDOHkcrQs8AAAABL7sFtQ", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/CXP%20Attention" + } + ], + "labels_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/labels{/name}", + "locked": false, + "milestone": null, + "node_id": "I_kwDOHkcrQs5c_SvC", + "number": 14, + "performed_via_github_app": null, + "reactions": { + "+1": 0, + "-1": 0, + "confused": 0, + "eyes": 0, + "heart": 0, + "hooray": 0, + "laugh": 0, + "rocket": 0, + "total_count": 0, + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/reactions" + }, + "repository_url": "https://api.github.com/repos/Azure/azure-sdk-fake", + "state": "open", + "state_reason": null, + "timeline_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14/timeline", + "title": "New test issue to generate event payloads", + "updated_at": "2023-01-30T16:37:38Z", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/14", + "user": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + } + }, + "label": { + "color": "F75BB2", + "default": false, + "description": "", + "id": 5095715984, + "name": "Service Attention", + "node_id": "LA_kwDOHkcrQs8AAAABL7p0kA", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels/Service%20Attention" + }, + "repository": { + "allow_forking": true, + "archive_url": "https://api.github.com/repos/Azure/azure-sdk-fake/{archive_format}{/ref}", + "archived": false, + "assignees_url": "https://api.github.com/repos/Azure/azure-sdk-fake/assignees{/user}", + "blobs_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/blobs{/sha}", + "branches_url": "https://api.github.com/repos/Azure/azure-sdk-fake/branches{/branch}", + "clone_url": "https://github.com/Azure/azure-sdk-fake.git", + "collaborators_url": "https://api.github.com/repos/Azure/azure-sdk-fake/collaborators{/collaborator}", + "comments_url": "https://api.github.com/repos/Azure/azure-sdk-fake/comments{/number}", + "commits_url": "https://api.github.com/repos/Azure/azure-sdk-fake/commits{/sha}", + "compare_url": "https://api.github.com/repos/Azure/azure-sdk-fake/compare/{base}...{head}", + "contents_url": "https://api.github.com/repos/Azure/azure-sdk-fake/contents/{+path}", + "contributors_url": "https://api.github.com/repos/Azure/azure-sdk-fake/contributors", + "created_at": "2022-06-27T16:19:29Z", + "default_branch": "main", + "deployments_url": "https://api.github.com/repos/Azure/azure-sdk-fake/deployments", + "description": "Tools repository leveraged by the Azure SDK team.", + "disabled": false, + "downloads_url": "https://api.github.com/repos/Azure/azure-sdk-fake/downloads", + "events_url": "https://api.github.com/repos/Azure/azure-sdk-fake/events", + "fork": true, + "forks": 0, + "forks_count": 0, + "forks_url": "https://api.github.com/repos/Azure/azure-sdk-fake/forks", + "full_name": "Azure/azure-sdk-fake", + "git_commits_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/commits{/sha}", + "git_refs_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/refs{/sha}", + "git_tags_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/tags{/sha}", + "git_url": "git://github.com/Azure/azure-sdk-fake.git", + "has_discussions": false, + "has_downloads": true, + "has_issues": true, + "has_pages": false, + "has_projects": true, + "has_wiki": true, + "homepage": null, + "hooks_url": "https://api.github.com/repos/Azure/azure-sdk-fake/hooks", + "html_url": "https://github.com/Azure/azure-sdk-fake", + "id": 507980610, + "is_template": false, + "issue_comment_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/comments{/number}", + "issue_events_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues/events{/number}", + "issues_url": "https://api.github.com/repos/Azure/azure-sdk-fake/issues{/number}", + "keys_url": "https://api.github.com/repos/Azure/azure-sdk-fake/keys{/key_id}", + "labels_url": "https://api.github.com/repos/Azure/azure-sdk-fake/labels{/name}", + "language": "C#", + "languages_url": "https://api.github.com/repos/Azure/azure-sdk-fake/languages", + "license": { + "key": "mit", + "name": "MIT License", + "node_id": "MDc6TGljZW5zZTEz", + "spdx_id": "MIT", + "url": "https://api.github.com/licenses/mit" + }, + "merges_url": "https://api.github.com/repos/Azure/azure-sdk-fake/merges", + "milestones_url": "https://api.github.com/repos/Azure/azure-sdk-fake/milestones{/number}", + "mirror_url": null, + "name": "azure-sdk-fake", + "node_id": "R_kgDOHkcrQg", + "notifications_url": "https://api.github.com/repos/Azure/azure-sdk-fake/notifications{?since,all,participating}", + "open_issues": 6, + "open_issues_count": 6, + "owner": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + }, + "private": false, + "pulls_url": "https://api.github.com/repos/Azure/azure-sdk-fake/pulls{/number}", + "pushed_at": "2023-01-27T16:33:00Z", + "releases_url": "https://api.github.com/repos/Azure/azure-sdk-fake/releases{/id}", + "size": 29098, + "ssh_url": "git@github.com:Azure/azure-sdk-fake.git", + "stargazers_count": 0, + "stargazers_url": "https://api.github.com/repos/Azure/azure-sdk-fake/stargazers", + "statuses_url": "https://api.github.com/repos/Azure/azure-sdk-fake/statuses/{sha}", + "subscribers_url": "https://api.github.com/repos/Azure/azure-sdk-fake/subscribers", + "subscription_url": "https://api.github.com/repos/Azure/azure-sdk-fake/subscription", + "svn_url": "https://github.com/Azure/azure-sdk-fake", + "tags_url": "https://api.github.com/repos/Azure/azure-sdk-fake/tags", + "teams_url": "https://api.github.com/repos/Azure/azure-sdk-fake/teams", + "topics": [], + "trees_url": "https://api.github.com/repos/Azure/azure-sdk-fake/git/trees{/sha}", + "updated_at": "2023-01-23T19:54:18Z", + "url": "https://api.github.com/repos/Azure/azure-sdk-fake", + "visibility": "public", + "watchers": 0, + "watchers_count": 0, + "web_commit_signoff_required": false + }, + "sender": { + "avatar_url": "https://avatars.githubusercontent.com/u/13556087?v=4", + "events_url": "https://api.github.com/users/FakeUser1/events{/privacy}", + "followers_url": "https://api.github.com/users/FakeUser1/followers", + "following_url": "https://api.github.com/users/FakeUser1/following{/other_user}", + "gists_url": "https://api.github.com/users/FakeUser1/gists{/gist_id}", + "gravatar_id": "", + "html_url": "https://github.com/FakeUser1", + "id": 13556087, + "login": "FakeUser1", + "node_id": "MDQ6VXNlcjEzNTU2MDg3", + "organizations_url": "https://api.github.com/users/FakeUser1/orgs", + "received_events_url": "https://api.github.com/users/FakeUser1/received_events", + "repos_url": "https://api.github.com/users/FakeUser1/repos", + "site_admin": false, + "starred_url": "https://api.github.com/users/FakeUser1/starred{/owner}{/repo}", + "subscriptions_url": "https://api.github.com/users/FakeUser1/subscriptions", + "type": "User", + "url": "https://api.github.com/users/FakeUser1" + } +} diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueCommentProcessing.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueCommentProcessing.cs index b48d4611397..4e347d2cd4f 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueCommentProcessing.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueCommentProcessing.cs @@ -36,7 +36,6 @@ public static async Task ProcessIssueCommentEvent(GitHubEventClient gitHubEventC /// Resulting Action: /// Remove "needs-author-feedback" label /// Add "needs-team-attention" label - /// Create issue comment /// /// Authenticated GitHubEventClient /// issue_comment event payload @@ -50,11 +49,8 @@ public static void AuthorFeedback(GitHubEventClient gitHubEventClient, IssueComm LabelUtils.HasLabel(issueCommentPayload.Issue.Labels, LabelConstants.NeedsAuthorFeedback) && issueCommentPayload.Sender.Login == issueCommentPayload.Issue.User.Login) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueCommentPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.NeedsAuthorFeedback); - issueUpdate.AddLabel(LabelConstants.NeedsTeamAttention); - string issueComment = $"Hi @{issueCommentPayload.Sender.Login}. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue."; - gitHubEventClient.CreateComment(issueCommentPayload.Repository.Id, issueCommentPayload.Issue.Number, issueComment); + gitHubEventClient.RemoveLabel(LabelConstants.NeedsAuthorFeedback); + gitHubEventClient.AddLabel(LabelConstants.NeedsTeamAttention); } } } @@ -88,6 +84,7 @@ public static void ResetIssueActivity(GitHubEventClient gitHubEventClient, Issue /// Remove "no-recent-activity" label /// Remove "needs-author-feedback" label /// Add "needs-team-attention" label + /// Reopen the issue /// /// Authenticated GitHubEventClient /// issue_comment event payload @@ -106,11 +103,10 @@ public static void ReopenIssue(GitHubEventClient gitHubEventClient, IssueComment // but being that the issue is closed is part of the criteria, this will be set DateTime.UtcNow <= issueCommentPayload.Issue.ClosedAt.Value.UtcDateTime.AddDays(7)) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueCommentPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.NeedsAuthorFeedback); - issueUpdate.RemoveLabel(LabelConstants.NoRecentActivity); - issueUpdate.AddLabel(LabelConstants.NeedsTeamAttention); - issueUpdate.State = ItemState.Open; + gitHubEventClient.RemoveLabel(LabelConstants.NeedsAuthorFeedback); + gitHubEventClient.RemoveLabel(LabelConstants.NoRecentActivity); + gitHubEventClient.AddLabel(LabelConstants.NeedsTeamAttention); + gitHubEventClient.SetIssueState(issueCommentPayload.Issue, ItemState.Open); } } } @@ -185,10 +181,9 @@ public static async Task IssueAddressedCommands(GitHubEventClient gitHubEventCli if (issueCommentPayload.Sender.Login == issueCommentPayload.Issue.User.Login || hasAdminOrWritePermission) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueCommentPayload.Issue); - issueUpdate.State = ItemState.Open; - issueUpdate.RemoveLabel(LabelConstants.IssueAddressed); - issueUpdate.AddLabel(LabelConstants.NeedsTeamAttention); + gitHubEventClient.SetIssueState(issueCommentPayload.Issue, ItemState.Open); + gitHubEventClient.RemoveLabel(LabelConstants.IssueAddressed); + gitHubEventClient.AddLabel(LabelConstants.NeedsTeamAttention); } // else the user is not the original author AND they don't have admin or write permission else diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueProcessing.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueProcessing.cs index 0cf943cc1c5..f4587c0e31a 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueProcessing.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/IssueProcessing.cs @@ -50,11 +50,10 @@ public static async Task ProcessIssueEvent(GitHubEventClient gitHubEventClient, /// Resulting Actions: /// Evaluate the user that created the issue: /// IF creator is NOT an Azure SDK team owner - /// AND is NOT a member of the Azure organization - /// AND does NOT have write permission - /// AND does NOT have admin permission: - /// Add "customer-reported" label - /// Add "question" label + /// IF the user is NOT a member of the Azure Org + /// IF the user does not have Admin or Write Collaborator permission + /// Add "customer-reported" label + /// Add "question" label /// Query AI label service for label suggestions: /// IF labels were predicted: /// Assign returned labels to the issue @@ -88,18 +87,29 @@ public static async Task InitialIssueTriage(GitHubEventClient gitHubEventClient, // are, add them and then add needs-team-triage. The issue created to track these future updates // is https://github.com/Azure/azure-sdk-tools/issues/5743 List labelSuggestions = await gitHubEventClient.QueryAILabelService(issueEventPayload); - IssueUpdate issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); if (labelSuggestions.Count > 0 ) { foreach (string label in labelSuggestions) { - issueUpdate.AddLabel(label); + gitHubEventClient.AddLabel(label); } - issueUpdate.AddLabel(LabelConstants.NeedsTeamTriage); + gitHubEventClient.AddLabel(LabelConstants.NeedsTeamTriage); } else { - issueUpdate.AddLabel(LabelConstants.NeedsTriage); + gitHubEventClient.AddLabel(LabelConstants.NeedsTriage); + } + + // If the user is not a member of the Azure Org AND the user does not have write or admin collaborator permission + bool isMemberOfOrg = await gitHubEventClient.IsUserMemberOfOrg(OrgConstants.Azure, issueEventPayload.Issue.User.Login); + if (!isMemberOfOrg) + { + bool hasAdminOrWritePermission = await gitHubEventClient.DoesUserHaveAdminOrWritePermission(issueEventPayload.Repository.Id, issueEventPayload.Issue.User.Login); + if (!hasAdminOrWritePermission) + { + gitHubEventClient.AddLabel(LabelConstants.CustomerReported); + gitHubEventClient.AddLabel(LabelConstants.Question); + } } } } @@ -128,8 +138,7 @@ public static void ManualIssueTriage(GitHubEventClient gitHubEventClient, IssueE LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTriage) && !issueEventPayload.Label.Name.Equals(LabelConstants.NeedsTriage)) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.NeedsTriage); + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTriage); } } } @@ -216,6 +225,9 @@ public static void CXPAttention(GitHubEventClient gitHubEventClient, IssueEventG /// Conditions: Issue is open /// Has "customer-reported" label /// Label removed is "Service Attention" OR "CXP Attention" + /// Issue does not have "Service Attention" OR "CXP Attention" + /// (in other words if both labels are on the issue and one is removed, this + /// shouldn't process) /// Resulting Action: Add "needs-team-triage" label /// /// Authenticated GitHubEventClient @@ -230,10 +242,11 @@ public static void ManualTriageAfterExternalAssignment(GitHubEventClient gitHubE LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.CustomerReported) && (issueEventPayload.Label.Name.Equals(LabelConstants.CXPAttention) || issueEventPayload.Label.Name.Equals(LabelConstants.ServiceAttention)) && - !LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamTriage)) + !LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamTriage) && + !LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.CXPAttention) && + !LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.ServiceAttention)) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); - issueUpdate.AddLabel(LabelConstants.NeedsTeamTriage); + gitHubEventClient.AddLabel(LabelConstants.NeedsTeamTriage); } } } @@ -276,8 +289,7 @@ public static void Common_ResetIssueActivity(GitHubEventClient gitHubEventClient // If a user is a known GitHub bot, the user's AccountType will be Bot sender.Type != AccountType.Bot) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issue); - issueUpdate.RemoveLabel(LabelConstants.NoRecentActivity); + gitHubEventClient.RemoveLabel(LabelConstants.NoRecentActivity); } } } @@ -312,8 +324,7 @@ public static void RequireAttentionForNonMilestone(GitHubEventClient gitHubEvent !LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.IssueAddressed) && null == issueEventPayload.Issue.Milestone) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); - issueUpdate.AddLabel(LabelConstants.NeedsTeamAttention); + gitHubEventClient.AddLabel(LabelConstants.NeedsTeamAttention); } } } @@ -328,6 +339,10 @@ public static void RequireAttentionForNonMilestone(GitHubEventClient gitHubEvent /// Remove "needs-triage" label /// Remove "needs-team-triage" label /// Remove "needs-team-attention" label + /// Create the following comment + /// "Hi @{issueAuthor}. Thank you for opening this issue and giving us the opportunity to assist. To help our + /// team better understand your issue and the details of your scenario please provide a response to the question + /// asked above or the information requested above. This will help us more accurately address your issue." /// /// Authenticated GitHubEventClient /// IssueEventGitHubPayload deserialized from the json event payload @@ -338,17 +353,24 @@ public static void AuthorFeedbackNeeded(GitHubEventClient gitHubEventClient, Iss if (issueEventPayload.Action == ActionConstants.Labeled) { if (issueEventPayload.Issue.State == ItemState.Open && - issueEventPayload.Label.Name == LabelConstants.NeedsAuthorFeedback && - // Any of these labels will be removed if they exist on the Issue. If none exist then - // there's nothing to do. - (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTriage) || - LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamTriage) || - LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamAttention))) + issueEventPayload.Label.Name == LabelConstants.NeedsAuthorFeedback) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.NeedsTriage); - issueUpdate.RemoveLabel(LabelConstants.NeedsTeamTriage); - issueUpdate.RemoveLabel(LabelConstants.NeedsTeamAttention); + // Any of these labels will be removed if they exist on the Issue. If none exist then + // then the comment will be the only update + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTriage)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTriage); + } + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamTriage)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTeamTriage); + } + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamAttention)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTeamAttention); + } + string issueComment = $"Hi @{issueEventPayload.Issue.User.Login}. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue."; + gitHubEventClient.CreateComment(issueEventPayload.Repository.Id, issueEventPayload.Issue.Number, issueComment); } } } @@ -360,11 +382,11 @@ public static void AuthorFeedbackNeeded(GitHubEventClient gitHubEventClient, Iss /// Conditions: Issue is open /// Label added is "issue-addressed" /// Resulting Action: - /// Remove "needs-triage" label - /// Remove "needs-team-triage" label - /// Remove "needs-team-attention" label - /// Remove "needs-author-feedback" label - /// Remove "no-recent-activity" label + /// Remove "needs-triage" label if it exists on the issue + /// Remove "needs-team-triage" label if it exists on the issue + /// Remove "needs-team-attention" label if it exists on the issue + /// Remove "needs-author-feedback" label if it exists on the issue + /// Remove "no-recent-activity" label if it exists on the issue /// Add issue comment /// /// Authenticated GitHubEventClient @@ -378,20 +400,25 @@ public static void IssueAddressed(GitHubEventClient gitHubEventClient, IssueEven if (issueEventPayload.Issue.State == ItemState.Open && issueEventPayload.Label.Name == LabelConstants.IssueAddressed) { - // Don't bother creating the issue update unless at least one of the labels - // to be removed exists on the issue - if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTriage) || - LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamTriage) || - LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamAttention) || - LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsAuthorFeedback) || - LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NoRecentActivity)) + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTriage)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTriage); + } + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamTriage)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTeamTriage); + } + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsTeamAttention)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsTeamAttention); + } + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NeedsAuthorFeedback)) + { + gitHubEventClient.RemoveLabel(LabelConstants.NeedsAuthorFeedback); + } + if (LabelUtils.HasLabel(issueEventPayload.Issue.Labels, LabelConstants.NoRecentActivity)) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.NeedsTriage); - issueUpdate.RemoveLabel(LabelConstants.NeedsTeamTriage); - issueUpdate.RemoveLabel(LabelConstants.NeedsTeamAttention); - issueUpdate.RemoveLabel(LabelConstants.NeedsAuthorFeedback); - issueUpdate.RemoveLabel(LabelConstants.NoRecentActivity); + gitHubEventClient.RemoveLabel(LabelConstants.NoRecentActivity); } // The comment is always created string issueComment = $"Hi @{issueEventPayload.Issue.User.Login}. Thank you for opening this issue and giving us the opportunity to assist. We believe that this has been addressed. If you feel that further discussion is needed, please add a comment with the text \"/unresolve\" to remove the \"issue-addressed\" label and continue the conversation."; @@ -434,8 +461,7 @@ public static void IssueAddressedReset(GitHubEventClient gitHubEventClient, Issu issueEventPayload.Label.Name == LabelConstants.NeedsTriage || issueEventPayload.Label.Name == LabelConstants.NeedsTeamTriage) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(issueEventPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.IssueAddressed); + gitHubEventClient.RemoveLabel(LabelConstants.IssueAddressed); } } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestCommentProcessing.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestCommentProcessing.cs index 0c586336b41..8c2023274cb 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestCommentProcessing.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestCommentProcessing.cs @@ -80,8 +80,7 @@ public static async Task ResetPullRequestActivity(GitHubEventClient gitHubEventC if (removeLabel) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(prCommentPayload.Issue); - issueUpdate.RemoveLabel(LabelConstants.NoRecentActivity); + gitHubEventClient.RemoveLabel(LabelConstants.NoRecentActivity); } } } @@ -130,9 +129,8 @@ public static async Task ReopenPullRequest(GitHubEventClient gitHubEventClient, } if (reOpen) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(prCommentPayload.Issue); - issueUpdate.State = ItemState.Open; - issueUpdate.RemoveLabel(LabelConstants.NoRecentActivity); + gitHubEventClient.SetIssueState(prCommentPayload.Issue, ItemState.Open); + gitHubEventClient.RemoveLabel(LabelConstants.NoRecentActivity); } else { diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestProcessing.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestProcessing.cs index 38b9dec64cd..050c1415f18 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestProcessing.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/EventProcessing/PullRequestProcessing.cs @@ -53,23 +53,24 @@ public static async Task PullRequestTriage(GitHubEventClient gitHubEventClient, var prLabels = CodeOwnerUtils.GetPRAutoLabelsForFilePaths(prEventPayload.PullRequest.Labels, prFileList); if (prLabels.Count > 0) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(prEventPayload.PullRequest); foreach (var prLabel in prLabels) { - issueUpdate.AddLabel(prLabel); + gitHubEventClient.AddLabel(prLabel); } } - // The sender will only have Write or Admin permssion if they are a collaborator - bool hasAdminOrWritePermission = await gitHubEventClient.DoesUserHaveAdminOrWritePermission(prEventPayload.Repository.Id, prEventPayload.PullRequest.User.Login); - // If the user doesn't have Write or Admin permissions - if (!hasAdminOrWritePermission) + // If the user is not a member of the Azure Org AND the user does not have write or admin collaborator permission + bool isMemberOfOrg = await gitHubEventClient.IsUserMemberOfOrg(OrgConstants.Azure, prEventPayload.PullRequest.User.Login); + if (!isMemberOfOrg) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(prEventPayload.PullRequest); - issueUpdate.AddLabel(LabelConstants.CustomerReported); - issueUpdate.AddLabel(LabelConstants.CommunityContribution); - string prComment = $"Thank you for your contribution @{prEventPayload.PullRequest.User.Login}! We will review the pull request and get back to you soon."; - gitHubEventClient.CreateComment(prEventPayload.Repository.Id, prEventPayload.PullRequest.Number, prComment); + bool hasAdminOrWritePermission = await gitHubEventClient.DoesUserHaveAdminOrWritePermission(prEventPayload.Repository.Id, prEventPayload.PullRequest.User.Login); + if (!hasAdminOrWritePermission) + { + gitHubEventClient.AddLabel(LabelConstants.CustomerReported); + gitHubEventClient.AddLabel(LabelConstants.CommunityContribution); + string prComment = $"Thank you for your contribution @{prEventPayload.PullRequest.User.Login}! We will review the pull request and get back to you soon."; + gitHubEventClient.CreateComment(prEventPayload.Repository.Id, prEventPayload.PullRequest.Number, prComment); + } } } } @@ -156,8 +157,7 @@ public static void Common_ResetPullRequestActivity(GitHubEventClient gitHubEvent } if (removeLabel) { - var issueUpdate = gitHubEventClient.GetIssueUpdate(pullRequest); - issueUpdate.RemoveLabel(LabelConstants.NoRecentActivity); + gitHubEventClient.RemoveLabel(LabelConstants.NoRecentActivity); } } } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs index 01094a4fbb7..3294a8d5d91 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/GitHubEventClient.cs @@ -117,6 +117,8 @@ public GitHubIssueToUpdate(long repositoryId, int issueOrPRNumber, IssueUpdate i private RulesConfiguration _rulesConfiguration = null; // Protected instead of private so the mock class can access them protected IssueUpdate _issueUpdate = null; + protected List _labelsToAdd = new List(); + protected List _labelsToRemove = new List(); protected List _gitHubComments = new List(); protected List _gitHubReviewDismissals = new List(); // Locking issues is only done through scheduled event processing @@ -157,19 +159,43 @@ public virtual async Task ProcessPendingUpdates(long repositoryId, int issu // Process the issue update if (_issueUpdate != null) { + numUpdates++; await _gitHubClient.Issue.Update(repositoryId, issueOrPullRequestNumber, _issueUpdate); + } + + // Process the labels to add. They're all added as a single call and are additive, not replacement + if (_labelsToAdd.Count > 0) + { numUpdates++; + await _gitHubClient.Issue.Labels.AddToIssue(repositoryId, issueOrPullRequestNumber, _labelsToAdd.ToArray()); + } + + // Process the labels to remove + foreach (string labelToRemove in _labelsToRemove) + { + try + { + numUpdates++; + await _gitHubClient.Issue.Labels.RemoveFromIssue(repositoryId, issueOrPullRequestNumber, labelToRemove); + } + // Octokit's NotFoundException is what happens when someone tries to remove a label that's not + // on an issue. This could happen if it was removed while the action event was processing. + // In this case it can just be ignored + // https://docs.github.com/en/rest/issues/labels?apiVersion=2022-11-28#remove-a-label-from-an-issue + catch (NotFoundException) + { + } } // Process any comments foreach (var comment in _gitHubComments) { + numUpdates++; await _gitHubClient.Issue.Comment.Create(comment.RepositoryId, comment.IssueOrPullRequestNumber, comment.Comment); - numUpdates++; } // Process any PullRequest review dismissals @@ -177,29 +203,29 @@ await _gitHubClient.Issue.Comment.Create(comment.RepositoryId, { var prReview = new PullRequestReviewDismiss(); prReview.Message = dismissal.DismissalMessage; + numUpdates++; await _gitHubClient.PullRequest.Review.Dismiss(dismissal.RepositoryId, dismissal.PullRequestNumber, dismissal.ReviewId, prReview); - numUpdates++; } // Process any issue locks foreach (var issueToLock in _gitHubIssuesToLock) { + numUpdates++; await _gitHubClient.Issue.LockUnlock.Lock(issueToLock.RepositoryId, issueToLock.IssueNumber, issueToLock.LockReason); - numUpdates++; } // Process any Scheduled task IssueUpdates foreach (var issueToUpdate in _gitHubIssuesToUpdate) { + numUpdates++; await _gitHubClient.Issue.Update(issueToUpdate.RepositoryId, issueToUpdate.IssueOrPRNumber, issueToUpdate.IssueUpdate); - numUpdates++; } Console.WriteLine("Finished processing pending updates."); } @@ -239,6 +265,16 @@ public int ComputeNumberOfExpectedUpdates() Console.WriteLine("Common IssueUpdate from rules processing will be updated."); numUpdates++; } + if (_labelsToAdd.Count > 0) + { + Console.WriteLine($"There are {_labelsToAdd.Count} labels being added (1 call)"); + numUpdates++; + } + if (_labelsToRemove.Count > 0) + { + Console.WriteLine($"There are {_labelsToRemove.Count} labels being removed ({_labelsToRemove.Count} calls)"); + numUpdates += _labelsToRemove.Count; + } if (_gitHubComments.Count > 0) { Console.WriteLine($"Number of Comments to create {_gitHubComments.Count}"); @@ -335,6 +371,58 @@ public async Task GetRateLimits() return await _gitHubClient.RateLimit.GetRateLimits(); } + // JRS - Remove + public void SetIssueUpdate(IssueUpdate issueUpdate) + { + _issueUpdate = issueUpdate; + } + + /// + /// Store the label to add on the list of labels to add to an issue. This is only used by Actions + /// and not Scheduled events + /// + /// string, the label to add to the issue + public void AddLabel(string labelToAdd) + { + if (_labelsToRemove.Contains(labelToAdd)) + { + Console.WriteLine($"Label to add {labelToAdd} is currently on the remove list and will not be added."); + } + else + { + _labelsToAdd.Add(labelToAdd); + } + } + + /// + /// Store the label to remove on the list of labels to be removed from an issue. This is only used by Actions + /// and not Scheduled events + /// + /// string, the label to remove from the issue + public void RemoveLabel(string labelToRemove) + { + if (_labelsToAdd.Contains(labelToRemove)) + { + Console.WriteLine($"Label to remove {labelToRemove} is currently on the add list and will not be added"); + } + else + { + _labelsToRemove.Add(labelToRemove); + } + } + + public void SetIssueState(Issue issue, ItemState itemState) + { + var issueUpdate = GetIssueUpdate(issue); + issueUpdate.State = itemState; + } + + public void SetPullRequestState(PullRequest pullRequest, ItemState itemState) + { + var issueUpdate = GetIssueUpdate(pullRequest); + issueUpdate.State = itemState; + } + /// /// Overloaded convenience function that'll return the IssueUpdate. Actions all make changes to /// the same, shared, IssueUpdate because they're processing on the same event. For scheduled @@ -349,7 +437,19 @@ public IssueUpdate GetIssueUpdate(Issue issue, bool isProcessingAction = true) { if (null == _issueUpdate) { - _issueUpdate = issue.ToUpdate(); + // For Actions, the IssueUpdate should only be used to set the state. + // Everything else should be null so it doesn't touch those other fields + // except for the Milestone which, if null, would clear it out if one was + // set. That's the only field to pull from the payload. + _issueUpdate = new IssueUpdate + { + Milestone = issue.Milestone == null + ? new int?() + : issue.Milestone.Number, + State = null, + Body = null, + Title = null + }; } return _issueUpdate; } @@ -373,7 +473,20 @@ public IssueUpdate GetIssueUpdate(PullRequest pullRequest, bool isProcessingActi { if (null == _issueUpdate) { - _issueUpdate = CreateIssueUpdateForPR(pullRequest); + // For Actions, the IssueUpdate should only be used to set the state. + // Everything else should be null so it doesn't touch those other fields + // except for the Milestone which, if null, would clear it out if one was + // set. That's the only field to pull from the payload. + _issueUpdate = new IssueUpdate + { + Milestone = pullRequest.Milestone == null + ? new int?() + : pullRequest.Milestone.Number, + State = null, + Body = null, + Title = null + }; + } return _issueUpdate; } diff --git a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Utils/CodeOwnerUtils.cs b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Utils/CodeOwnerUtils.cs index 045981b997b..5bb3c2272a8 100644 --- a/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Utils/CodeOwnerUtils.cs +++ b/tools/github-event-processor/Azure.Sdk.Tools.GitHubEventProcessor/Utils/CodeOwnerUtils.cs @@ -13,7 +13,7 @@ namespace Azure.Sdk.Tools.GitHubEventProcessor.Utils /// public class CodeOwnerUtils { - private static readonly string CodeownersFileName = "event-processor.config"; + private static readonly string CodeownersFileName = "CODEOWNERS"; private static readonly string CodeownersSubDirectory = ".github"; static List _codeOwnerEntries = null; diff --git a/tools/github-event-processor/RULES.md b/tools/github-event-processor/RULES.md index 9f9a7277570..cc0fe5d8509 100644 --- a/tools/github-event-processor/RULES.md +++ b/tools/github-event-processor/RULES.md @@ -108,13 +108,10 @@ This is a stand-alone service providing a REST API which requires a service key ```text IF creator is NOT an Azure SDK team owner - AND is NOT a member of the Azure organization - AND is does NOT have a collaborator association - AND does NOT have write permission - AND does NOT have admin permission: - - - Add "customer-reported" label - - Add "question" label + IF the user is NOT a member of the Azure Org + IF the user does not have Admin or Write Collaborator permission + - Add "customer-reported" label to the issue + - Add "question" label to the issue ``` - Query AI label service for suggestions: @@ -126,6 +123,11 @@ This is a stand-alone service providing a REST API which requires a service key - Add "needs-team-triage" label to the issue ELSE - Add "needs-triage" label to the issue + + IF the user is NOT a member of the Azure Org + IF the user does not have Admin or Write Collaborator permission + - Add "customer-reported" label to the issue + - Add "question" label to the issue ``` - _This is what we'd like to get to. It requires changes to the CODEOWNERS file structure which have not been done yet_ @@ -218,6 +220,7 @@ This is a stand-alone service providing a REST API which requires a service key - Issue is open - Issue has "customer-reported" label - Label removed is "Service Attention" OR "CXP Attention" +- Issue does not have "Service Attention" OR "CXP Attention" ### Actions @@ -239,8 +242,6 @@ This is a stand-alone service providing a REST API which requires a service key - Remove "needs-author-feedback" label - Add "needs-team-attention" label -- Create the following comment - - "Hi @{issueAuthor}. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue." ## Reset issue activity @@ -346,6 +347,8 @@ OR - Remove "needs-triage" label - Remove "needs-team-triage" label - Remove "needs-team-attention" label +- Create the following comment + - "Hi @{issueAuthor}. Thank you for opening this issue and giving us the opportunity to assist. To help our team better understand your issue and the details of your scenario please provide a response to the question asked above or the information requested above. This will help us more accurately address your issue." ## Issue Addressed @@ -446,9 +449,8 @@ OR - Determine if this is a community contribution: ```text - IF the PR author does not have write permission - AND the PR author does not have write permission - AND the PR author does not have a collaborator association: + IF the user is NOT a member of the Azure Org + IF the user does not have Admin or Write Collaborator permission - Add "customer-reported" label - Add "Community Contribution" label - Create the following comment