From 406c5d41f0ad68c68768f52b781587606555d211 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Wed, 31 Oct 2018 09:07:02 -0400 Subject: [PATCH 1/8] Allow functionality to set ProtectedBranches --- src/GitHub.App/Services/PullRequestService.cs | 8 ++++ src/GitHub.App/Services/RepositoryService.cs | 40 +++++++++++++++++-- src/GitHub.Exports/Models/ProtectedBranch.cs | 8 ++++ .../Services/IRepositoryService.cs | 4 ++ 4 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 src/GitHub.Exports/Models/ProtectedBranch.cs diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index 9b8015cb43..697e87810f 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -55,6 +55,7 @@ public class PullRequestService : IPullRequestService readonly IGitService gitService; readonly IVSGitExt gitExt; readonly IGraphQLClientFactory graphqlFactory; + readonly IRepositoryService repositoryService; readonly IOperatingSystem os; readonly IUsageTracker usageTracker; @@ -64,6 +65,7 @@ public PullRequestService( IGitService gitService, IVSGitExt gitExt, IGraphQLClientFactory graphqlFactory, + IRepositoryService repositoryService, IOperatingSystem os, IUsageTracker usageTracker) { @@ -71,6 +73,7 @@ public PullRequestService( this.gitService = gitService; this.gitExt = gitExt; this.graphqlFactory = graphqlFactory; + this.repositoryService = repositoryService; this.os = os; this.usageTracker = usageTracker; } @@ -198,6 +201,11 @@ public async Task> ReadPullRequests( query = readPullRequestsEnterprise; } + var protectedBranches = await repositoryService.GetProtectedBranches(address, owner, name) + .ConfigureAwait(false); + + var protectedBranchDictionary = protectedBranches.ToDictionary(branch => branch.Name); + var graphql = await graphqlFactory.CreateConnection(address); var vars = new Dictionary { diff --git a/src/GitHub.App/Services/RepositoryService.cs b/src/GitHub.App/Services/RepositoryService.cs index 36b729fe3e..1c8bf2c7af 100644 --- a/src/GitHub.App/Services/RepositoryService.cs +++ b/src/GitHub.App/Services/RepositoryService.cs @@ -1,9 +1,11 @@ using System; using System.Collections.Generic; using System.ComponentModel.Composition; +using System.Linq; using System.Threading.Tasks; using GitHub.Api; using GitHub.Extensions; +using GitHub.Models; using GitHub.Primitives; using Octokit.GraphQL; using static Octokit.GraphQL.Variable; @@ -12,9 +14,10 @@ namespace GitHub.Services { [Export(typeof(IRepositoryService))] [PartCreationPolicy(CreationPolicy.Shared)] - public class RepositoryService : IRepositoryService + public class RepositoryService: IRepositoryService { static ICompiledQuery> readParentOwnerLogin; + static ICompiledQuery> queryProtectedBranches; readonly IGraphQLClientFactory graphqlFactory; [ImportingConstructor] @@ -45,9 +48,40 @@ public RepositoryService(IGraphQLClientFactory graphqlFactory) { nameof(name), name }, }; - var graphql = await graphqlFactory.CreateConnection(address); - var result = await graphql.Run(readParentOwnerLogin, vars); + var graphql = await graphqlFactory.CreateConnection(address).ConfigureAwait(false); + var result = await graphql.Run(readParentOwnerLogin, vars).ConfigureAwait(false); return result != null ? (result.Item1, result.Item2) : ((string, string)?)null; } + + public async Task> GetProtectedBranches(HostAddress address, string owner, string name) + { + Guard.ArgumentNotNull(address, nameof(address)); + Guard.ArgumentNotEmptyString(owner, nameof(owner)); + Guard.ArgumentNotEmptyString(name, nameof(name)); + + if (queryProtectedBranches == null) + { + queryProtectedBranches = new Query() + .Repository(Var(nameof(owner)), Var(nameof(name))) + .Select(r => + r.ProtectedBranches(null, null, null, null) + .AllPages() + .Select(branch => new ProtectedBranch + { + Name = branch.Name, + RequiredStatusCheckContexts = branch.RequiredStatusCheckContexts.ToArray() + }) + ).Compile(); + } + + var vars = new Dictionary + { + { nameof(owner), owner }, + { nameof(name), name }, + }; + + var graphql = await graphqlFactory.CreateConnection(address).ConfigureAwait(false); + return await graphql.Run(queryProtectedBranches, vars).ConfigureAwait(false); + } } } diff --git a/src/GitHub.Exports/Models/ProtectedBranch.cs b/src/GitHub.Exports/Models/ProtectedBranch.cs new file mode 100644 index 0000000000..019bc9ff2b --- /dev/null +++ b/src/GitHub.Exports/Models/ProtectedBranch.cs @@ -0,0 +1,8 @@ +namespace GitHub.Models +{ + public class ProtectedBranch + { + public string Name { get; set; } + public string[] RequiredStatusCheckContexts { get; set; } + } +} \ No newline at end of file diff --git a/src/GitHub.Exports/Services/IRepositoryService.cs b/src/GitHub.Exports/Services/IRepositoryService.cs index c71f492e29..88e720fb28 100644 --- a/src/GitHub.Exports/Services/IRepositoryService.cs +++ b/src/GitHub.Exports/Services/IRepositoryService.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using System.Threading.Tasks; +using GitHub.Models; using GitHub.Primitives; namespace GitHub.Services @@ -17,5 +19,7 @@ public interface IRepositoryService /// otherwise null. /// Task<(string owner, string name)?> FindParent(HostAddress address, string owner, string name); + + Task> GetProtectedBranches(HostAddress address, string owner, string name); } } From 0b979b380095c16255abcc053ffe9c5794a07351 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 2 Nov 2018 08:51:11 -0400 Subject: [PATCH 2/8] GraphQL fix --- src/GitHub.App/Services/RepositoryService.cs | 8 ++++---- src/GitHub.Exports/Services/IRepositoryService.cs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/GitHub.App/Services/RepositoryService.cs b/src/GitHub.App/Services/RepositoryService.cs index 474e100af9..d4d183ea30 100644 --- a/src/GitHub.App/Services/RepositoryService.cs +++ b/src/GitHub.App/Services/RepositoryService.cs @@ -17,7 +17,7 @@ namespace GitHub.Services public class RepositoryService: IRepositoryService { static ICompiledQuery> readParentOwnerLogin; - static ICompiledQuery> queryProtectedBranches; + static ICompiledQuery> queryProtectedBranches; readonly IGraphQLClientFactory graphqlFactory; [ImportingConstructor] @@ -53,7 +53,7 @@ public RepositoryService(IGraphQLClientFactory graphqlFactory) return result != null ? (result.Item1, result.Item2) : ((string, string)?)null; } - public async Task> GetProtectedBranches(HostAddress address, string owner, string name) + public async Task> GetProtectedBranches(HostAddress address, string owner, string name) { Guard.ArgumentNotNull(address, nameof(address)); Guard.ArgumentNotEmptyString(owner, nameof(owner)); @@ -62,7 +62,7 @@ public async Task> GetProtectedBranches(HostAddress if (queryProtectedBranches == null) { queryProtectedBranches = new Query() - .Repository(Var(nameof(owner)), Var(nameof(name))) + .Repository(Var(nameof(name)), Var(nameof(owner))) .Select(r => r.ProtectedBranches(null, null, null, null) .AllPages() @@ -70,7 +70,7 @@ public async Task> GetProtectedBranches(HostAddress { Name = branch.Name, RequiredStatusCheckContexts = branch.RequiredStatusCheckContexts.ToArray() - }) + }).ToList() ).Compile(); } diff --git a/src/GitHub.Exports/Services/IRepositoryService.cs b/src/GitHub.Exports/Services/IRepositoryService.cs index 88e720fb28..975996bc69 100644 --- a/src/GitHub.Exports/Services/IRepositoryService.cs +++ b/src/GitHub.Exports/Services/IRepositoryService.cs @@ -20,6 +20,6 @@ public interface IRepositoryService /// Task<(string owner, string name)?> FindParent(HostAddress address, string owner, string name); - Task> GetProtectedBranches(HostAddress address, string owner, string name); + Task> GetProtectedBranches(HostAddress address, string owner, string name); } } From e17b44b350da6e52450b97931704f548ec5db326 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 2 Nov 2018 11:37:36 -0400 Subject: [PATCH 3/8] Adding a second check for differing non-required statuses --- .../PullRequestListItemViewModelDesigner.cs | 2 +- src/GitHub.App/Services/PullRequestService.cs | 199 ++++++++++++++---- .../PullRequestListItemViewModel.cs | 2 +- .../Collections/TrackingCollection.cs | 4 +- .../Services/IGitClient.cs | 6 +- .../Services/IPullRequestService.cs | 4 +- .../Services/IRepositoryCloneService.cs | 4 +- .../GitHubPane/IPullRequestDetailViewModel.cs | 2 +- .../IPullRequestListItemViewModel.cs | 2 +- .../Models/IPullRequestModel.cs | 10 + .../Models/PullRequestDetailModel.cs | 2 +- .../Models/PullRequestListItemModel.cs | 12 +- .../IEnterpriseCapabilitiesService.cs | 2 +- .../GitHubPane/PullRequestListItemView.xaml | 4 + .../Services/PullRequestServiceTests.cs | 6 + .../PullRequestCreationViewModelTests.cs | 8 +- 16 files changed, 205 insertions(+), 64 deletions(-) diff --git a/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs index 500089fb84..aba518d750 100644 --- a/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs @@ -17,6 +17,6 @@ public class PullRequestListItemViewModelDesigner : ViewModelBase, IPullRequestL public int Number { get; set; } public string Title { get; set; } public DateTimeOffset UpdatedAt { get; set; } - public PullRequestChecksState Checks { get; set; } + public PullRequestChecksSummaryState Checks { get; set; } } } diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index b460f7fca9..e7c8f6091f 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -26,6 +26,7 @@ using static Octokit.GraphQL.Variable; using CheckConclusionState = GitHub.Models.CheckConclusionState; using CheckStatusState = GitHub.Models.CheckStatusState; +using ProtectedBranch = GitHub.Models.ProtectedBranch; using StatusState = GitHub.Models.StatusState; namespace GitHub.Services @@ -107,12 +108,14 @@ public async Task> ReadPullRequests( Items = page.Nodes.Select(pr => new ListItemAdapter { Id = pr.Id.Value, + BaseRefName = pr.BaseRefName, LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit => new LastCommitSummaryAdapter { CheckSuites = commit.Commit.CheckSuites(null, null, null, null, null).AllPages(10) .Select(suite => new CheckSuiteSummaryModel { + ApplicationName = suite.App != null ? suite.App.Name : "Private Application", CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10) .Select(run => new CheckRunSummaryModel { @@ -124,6 +127,7 @@ public async Task> ReadPullRequests( .Select(context => context.Contexts.Select(statusContext => new StatusSummaryModel { + Context = statusContext.Context, State = statusContext.State.FromGraphQl(), }).ToList() ).SingleOrDefault() @@ -168,6 +172,7 @@ public async Task> ReadPullRequests( Items = page.Nodes.Select(pr => new ListItemAdapter { Id = pr.Id.Value, + BaseRefName = pr.BaseRefName, LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit => new LastCommitSummaryAdapter { @@ -204,7 +209,7 @@ public async Task> ReadPullRequests( var protectedBranches = await repositoryService.GetProtectedBranches(address, owner, name) .ConfigureAwait(false); - var protectedBranchDictionary = protectedBranches.ToDictionary(branch => branch.Name); + var protectedBranchesContextsDictionary = protectedBranches.ToDictionary(branch => branch.Name, branch => new HashSet(branch.RequiredStatusCheckContexts)); var graphql = await graphqlFactory.CreateConnection(address); var vars = new Dictionary @@ -219,74 +224,172 @@ public async Task> ReadPullRequests( foreach (var item in result.Items.Cast()) { + HashSet protectedBranchContexts; + protectedBranchesContextsDictionary.TryGetValue(item.BaseRefName, out protectedBranchContexts); + item.CommentCount += item.Reviews.Sum(x => x.Count); item.Reviews = null; - var checkRuns = item.LastCommit?.CheckSuites?.SelectMany(model => model.CheckRuns).ToArray(); + if (protectedBranchContexts != null) + { + var requiredCheckRuns = item.LastCommit?.CheckSuites? + .Where(model => protectedBranchContexts.Contains(model.ApplicationName)) + .SelectMany(checkSuite => checkSuite.CheckRuns) + .ToArray(); - var hasCheckRuns = checkRuns?.Any() ?? false; - var hasStatuses = item.LastCommit?.Statuses?.Any() ?? false; + var requiredStatues = item.LastCommit?.Statuses + .Where(model => protectedBranchContexts.Contains(model.Context)) + .ToArray(); - if (!hasCheckRuns && !hasStatuses) - { - item.Checks = PullRequestChecksState.None; + item.RequiredChecks = CalculatePullRequestCheckState(requiredCheckRuns, requiredStatues); } else { - var checksHasFailure = false; - var checksHasCompleteSuccess = true; - - if (hasCheckRuns) - { - checksHasFailure = checkRuns - .Any(model => model.Conclusion.HasValue - && (model.Conclusion.Value == CheckConclusionState.Failure - || model.Conclusion.Value == CheckConclusionState.ActionRequired)); + item.RequiredChecks = PullRequestChecksState.None; + } - if (!checksHasFailure) - { - checksHasCompleteSuccess = checkRuns - .All(model => model.Conclusion.HasValue - && (model.Conclusion.Value == CheckConclusionState.Success - || model.Conclusion.Value == CheckConclusionState.Neutral)); - } - } + var checkRuns = item.LastCommit?.CheckSuites? + .Where(model => protectedBranchContexts == null || !protectedBranchContexts.Contains(model.ApplicationName)) + .SelectMany(checkSuite => checkSuite.CheckRuns) + .ToArray(); - var statusHasFailure = false; - var statusHasCompleteSuccess = true; + var statuses = item.LastCommit?.Statuses + .Where(model => protectedBranchContexts == null || !protectedBranchContexts.Contains(model.Context)) + .ToArray(); - if (!checksHasFailure && hasStatuses) - { - statusHasFailure = item.LastCommit - .Statuses - .Any(status => status.State == StatusState.Failure - || status.State == StatusState.Error); + item.OtherChecks = CalculatePullRequestCheckState(checkRuns, statuses); - if (!statusHasFailure) - { - statusHasCompleteSuccess = - item.LastCommit.Statuses.All(status => status.State == StatusState.Success); - } - } + switch (item.RequiredChecks) + { + case PullRequestChecksState.None: + item.Checks = PullRequestChecksSummaryState.None; + break; + case PullRequestChecksState.Pending: + item.Checks = PullRequestChecksSummaryState.Pending; + break; + case PullRequestChecksState.Success: + item.Checks = PullRequestChecksSummaryState.Success; + break; + case PullRequestChecksState.Failure: + item.Checks = PullRequestChecksSummaryState.Failure; + break; + default: + throw new ArgumentOutOfRangeException(); + } - if (checksHasFailure || statusHasFailure) + if (item.RequiredChecks != item.OtherChecks) + { + switch (item.RequiredChecks) { - item.Checks = PullRequestChecksState.Failure; + case PullRequestChecksState.None: + switch (item.OtherChecks) + { + case PullRequestChecksState.None: + item.Checks = PullRequestChecksSummaryState.None; + break; + case PullRequestChecksState.Pending: + item.Checks = PullRequestChecksSummaryState.Pending; + break; + case PullRequestChecksState.Success: + item.Checks = PullRequestChecksSummaryState.Success; + break; + case PullRequestChecksState.Failure: + item.Checks = PullRequestChecksSummaryState.Failure; + break; + default: + throw new ArgumentOutOfRangeException(); + } + break; + case PullRequestChecksState.Pending: + item.Checks = PullRequestChecksSummaryState.Pending; + break; + case PullRequestChecksState.Success: + switch (item.OtherChecks) + { + case PullRequestChecksState.Pending: + item.Checks = PullRequestChecksSummaryState.SuccessWithPending; + break; + case PullRequestChecksState.Failure: + item.Checks = PullRequestChecksSummaryState.SuccessWithFailure; + break; + } + break; + case PullRequestChecksState.Failure: + item.Checks = PullRequestChecksSummaryState.Failure; + break; + default: + throw new ArgumentOutOfRangeException(); } - else if (statusHasCompleteSuccess && checksHasCompleteSuccess) + } + + item.LastCommit = null; + } + + return result; + } + + static PullRequestChecksState CalculatePullRequestCheckState(IList checkRuns, IList statusSummaryModels) + { + var hasCheckRuns = checkRuns?.Any() ?? false; + var hasStatuses = statusSummaryModels?.Any() ?? false; + + PullRequestChecksState resultState; + if (!hasCheckRuns && !hasStatuses) + { + resultState = PullRequestChecksState.None; + } + else + { + var checksHasFailure = false; + var checksHasCompleteSuccess = true; + + if (hasCheckRuns) + { + checksHasFailure = checkRuns + .Any(model => model.Conclusion.HasValue + && (model.Conclusion.Value == CheckConclusionState.Failure + || model.Conclusion.Value == CheckConclusionState.ActionRequired)); + + if (!checksHasFailure) { - item.Checks = PullRequestChecksState.Success; + checksHasCompleteSuccess = checkRuns + .All(model => model.Conclusion.HasValue + && (model.Conclusion.Value == CheckConclusionState.Success + || model.Conclusion.Value == CheckConclusionState.Neutral)); } - else + } + + var statusHasFailure = false; + var statusHasCompleteSuccess = true; + + if (!checksHasFailure && hasStatuses) + { + statusHasFailure = statusSummaryModels + .Any(status => status.State == StatusState.Failure + || status.State == StatusState.Error); + + if (!statusHasFailure) { - item.Checks = PullRequestChecksState.Pending; + statusHasCompleteSuccess = + statusSummaryModels.All(status => status.State == StatusState.Success); } } - item.LastCommit = null; + if (checksHasFailure || statusHasFailure) + { + resultState = PullRequestChecksState.Failure; + } + else if (statusHasCompleteSuccess && checksHasCompleteSuccess) + { + resultState = PullRequestChecksState.Success; + } + else + { + resultState = PullRequestChecksState.Pending; + } } - return result; + return resultState; } public async Task> ReadAssignableUsers( @@ -1017,6 +1120,8 @@ class ListItemAdapter : PullRequestListItemModel public IList Reviews { get; set; } public LastCommitSummaryAdapter LastCommit { get; set; } + + public string BaseRefName { get; set; } } class ReviewAdapter @@ -1036,6 +1141,7 @@ class LastCommitSummaryAdapter class CheckSuiteSummaryModel { public List CheckRuns { get; set; } + public string ApplicationName { get; set; } } class CheckRunSummaryModel @@ -1047,6 +1153,7 @@ class CheckRunSummaryModel class StatusSummaryModel { public StatusState State { get; set; } + public string Context { get; set; } } } } diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs index b60cf0e70b..f4f0a8693b 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs @@ -33,7 +33,7 @@ public PullRequestListItemViewModel(PullRequestListItemModel model) public IActorViewModel Author { get; } /// - public PullRequestChecksState Checks { get; } + public PullRequestChecksSummaryState Checks { get; } /// public int CommentCount { get; } diff --git a/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs b/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs index 0c724bf9f0..4e6350229d 100644 --- a/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs +++ b/src/GitHub.Exports.Reactive/Collections/TrackingCollection.cs @@ -700,7 +700,7 @@ ActionData FilteredInsert(ActionData data) } /// - /// Checks if the object being moved affects the filtered list in any way and update + /// OtherChecks if the object being moved affects the filtered list in any way and update /// the list accordingly /// /// @@ -756,7 +756,7 @@ ActionData FilteredMove(ActionData data) } /// - /// Checks if the object being moved affects the filtered list in any way and update + /// OtherChecks if the object being moved affects the filtered list in any way and update /// the list accordingly /// /// diff --git a/src/GitHub.Exports.Reactive/Services/IGitClient.cs b/src/GitHub.Exports.Reactive/Services/IGitClient.cs index 81fbbafebe..ef02f0c0cf 100644 --- a/src/GitHub.Exports.Reactive/Services/IGitClient.cs +++ b/src/GitHub.Exports.Reactive/Services/IGitClient.cs @@ -57,7 +57,7 @@ public interface IGitClient Task Fetch(IRepository repository, UriString remoteUri, params string[] refspecs); /// - /// Checks out a branch. + /// OtherChecks out a branch. /// /// The repository to carry out the checkout on /// The name of the branch @@ -178,7 +178,7 @@ public interface IGitClient Task ExtractFileBinary(IRepository repository, string commitSha, string fileName); /// - /// Checks whether the latest commit of a file in the repository has the specified file + /// OtherChecks whether the latest commit of a file in the repository has the specified file /// contents. /// /// The repository. @@ -203,7 +203,7 @@ public interface IGitClient Task GetPullRequestMergeBase(IRepository repo, UriString targetCloneUrl, string baseSha, string headSha, string baseRef, int pullNumber); /// - /// Checks whether the current head is pushed to its remote tracking branch. + /// OtherChecks whether the current head is pushed to its remote tracking branch. /// /// The repository. /// diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs index 48837ce2d6..54617c97cc 100644 --- a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs +++ b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs @@ -47,7 +47,7 @@ IObservable CreatePullRequest(IModelService modelService, string title, string body); /// - /// Checks whether the working directory for the specified repository is in a clean state. + /// OtherChecks whether the working directory for the specified repository is in a clean state. /// /// The repository. /// @@ -61,7 +61,7 @@ IObservable CreatePullRequest(IModelService modelService, IObservable CountSubmodulesToSync(ILocalRepositoryModel repository); /// - /// Checks out a pull request to a local branch. + /// OtherChecks out a pull request to a local branch. /// /// The repository. /// The pull request details. diff --git a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs index a4810bf742..a05e4c6005 100644 --- a/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs +++ b/src/GitHub.Exports.Reactive/Services/IRepositoryCloneService.cs @@ -48,7 +48,7 @@ Task CloneOrOpenRepository( object progress = null); /// - /// Checks whether the specified destination directory already exists. + /// OtherChecks whether the specified destination directory already exists. /// /// The destination path. /// @@ -57,7 +57,7 @@ Task CloneOrOpenRepository( bool DestinationDirectoryExists(string path); /// - /// Checks whether the specified destination file already exists. + /// OtherChecks whether the specified destination file already exists. /// /// The destination file. /// diff --git a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs index 37e87ed78b..4fe726aa16 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs @@ -176,7 +176,7 @@ public interface IPullRequestDetailViewModel : IPanePageViewModel, IOpenInBrowse ReactiveCommand ShowReview { get; } /// - /// Gets the latest pull request Checks & Statuses + /// Gets the latest pull request OtherChecks & Statuses /// IReadOnlyList Checks { get; } diff --git a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs index f8fa89c351..0f135924b5 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs @@ -32,6 +32,6 @@ public interface IPullRequestListItemViewModel : IIssueListItemViewModelBase /// /// Gets the pull request checks and statuses summary /// - PullRequestChecksState Checks { get; } + PullRequestChecksSummaryState Checks { get; } } } diff --git a/src/GitHub.Exports/Models/IPullRequestModel.cs b/src/GitHub.Exports/Models/IPullRequestModel.cs index 4d78de08ab..ae8b8f148d 100644 --- a/src/GitHub.Exports/Models/IPullRequestModel.cs +++ b/src/GitHub.Exports/Models/IPullRequestModel.cs @@ -21,6 +21,16 @@ public enum PullRequestChecksState Failure } + public enum PullRequestChecksSummaryState + { + None, + Pending, + Success, + SuccessWithFailure, + SuccessWithPending, + Failure + } + public interface IPullRequestModel : ICopyable, IEquatable, IComparable { diff --git a/src/GitHub.Exports/Models/PullRequestDetailModel.cs b/src/GitHub.Exports/Models/PullRequestDetailModel.cs index 975c6511d2..6c765b01a1 100644 --- a/src/GitHub.Exports/Models/PullRequestDetailModel.cs +++ b/src/GitHub.Exports/Models/PullRequestDetailModel.cs @@ -93,7 +93,7 @@ public class PullRequestDetailModel public IReadOnlyList Threads { get; set; } /// - /// Gets or sets a collection of pull request Checks Suites + /// Gets or sets a collection of pull request OtherChecks Suites /// public IReadOnlyList CheckSuites { get; set; } diff --git a/src/GitHub.Exports/Models/PullRequestListItemModel.cs b/src/GitHub.Exports/Models/PullRequestListItemModel.cs index f6a9a7fe55..18a104899f 100644 --- a/src/GitHub.Exports/Models/PullRequestListItemModel.cs +++ b/src/GitHub.Exports/Models/PullRequestListItemModel.cs @@ -40,7 +40,17 @@ public class PullRequestListItemModel /// /// Gets the pull request checks and statuses summary /// - public PullRequestChecksState Checks { get; set; } + public PullRequestChecksState RequiredChecks { get; set; } + + /// + /// Gets the pull request checks and statuses summary + /// + public PullRequestChecksState OtherChecks { get; set; } + + /// + /// Gets the pull request checks and statuses summary + /// + public PullRequestChecksSummaryState Checks { get; set; } /// /// Gets or sets the date/time at which the pull request was last updated. diff --git a/src/GitHub.Exports/Services/IEnterpriseCapabilitiesService.cs b/src/GitHub.Exports/Services/IEnterpriseCapabilitiesService.cs index 875916c722..1e091977bf 100644 --- a/src/GitHub.Exports/Services/IEnterpriseCapabilitiesService.cs +++ b/src/GitHub.Exports/Services/IEnterpriseCapabilitiesService.cs @@ -33,7 +33,7 @@ public interface IEnterpriseCapabilitiesService Task Probe(Uri enterpriseBaseUrl); /// - /// Checks the login methods supported by an enterprise instance. + /// OtherChecks the login methods supported by an enterprise instance. /// /// The URL to test. /// The supported login methods. diff --git a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml index 4d7e261552..19120e288f 100644 --- a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml +++ b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml @@ -101,6 +101,10 @@ + + + + diff --git a/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs b/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs index c8e437cb8d..05c13e6424 100644 --- a/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs @@ -576,6 +576,7 @@ static PullRequestService CreatePullRequestService(Repository repo) gitService, Substitute.For(), Substitute.For(), + Substitute.For(), serviceProvider.GetOperatingSystem(), Substitute.For()); return service; @@ -685,6 +686,7 @@ public void CreatePullRequestAllArgsMandatory() serviceProvider.GetGitService(), Substitute.For(), Substitute.For(), + Substitute.For(), serviceProvider.GetOperatingSystem(), Substitute.For()); @@ -894,6 +896,7 @@ public async Task ShouldReturnCorrectDefaultLocalBranchNameForPullRequestsWithNo MockGitService(), Substitute.For(), Substitute.For(), + Substitute.For(), Substitute.For(), Substitute.For()); @@ -1046,6 +1049,7 @@ static PullRequestService CreateTarget( IGitService gitService = null, IVSGitExt gitExt = null, IGraphQLClientFactory graphqlFactory = null, + IRepositoryService repositoryService = null, IOperatingSystem os = null, IUsageTracker usageTracker = null) { @@ -1055,12 +1059,14 @@ static PullRequestService CreateTarget( graphqlFactory = graphqlFactory ?? Substitute.For(); os = os ?? Substitute.For(); usageTracker = usageTracker ?? Substitute.For(); + repositoryService = repositoryService ?? Substitute.For(); return new PullRequestService( gitClient, gitService, gitExt, graphqlFactory, + repositoryService, os, usageTracker); } diff --git a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs index 5b21260e0a..cd7fa8b8ed 100644 --- a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/PullRequestCreationViewModelTests.cs @@ -128,7 +128,9 @@ static TestData PrepareTestData( public async Task TargetBranchDisplayNameIncludesRepoOwnerWhenForkAsync() { var data = PrepareTestData("octokit.net", "shana", "master", "octokit", "master", "origin", true, true); - var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For(), Substitute.For(), data.ServiceProvider.GetOperatingSystem(), Substitute.For()); + var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For(), + Substitute.For(), Substitute.For(), + data.ServiceProvider.GetOperatingSystem(), Substitute.For()); prservice.GetPullRequestTemplate(data.ActiveRepo).Returns(Observable.Empty()); var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, Substitute.For()); await vm.InitializeAsync(data.ActiveRepo, data.Connection); @@ -164,7 +166,9 @@ public async Task CreatingPRsAsync( var targetBranch = data.TargetBranch; var ms = data.ModelService; - var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For(), Substitute.For(), data.ServiceProvider.GetOperatingSystem(), Substitute.For()); + var prservice = new PullRequestService(data.GitClient, data.GitService, Substitute.For(), + Substitute.For(), Substitute.For(), + data.ServiceProvider.GetOperatingSystem(), Substitute.For()); var vm = new PullRequestCreationViewModel(data.GetModelServiceFactory(), prservice, data.NotificationService, Substitute.For()); await vm.InitializeAsync(data.ActiveRepo, data.Connection); From bc9284290a6bc59ecead7d550e38c3c4e8ce6740 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 2 Nov 2018 12:51:14 -0400 Subject: [PATCH 4/8] Adding a required tag to PullRequestCheckViewModel --- .../PullRequestCheckViewModelDesigner.cs | 2 ++ src/GitHub.App/Services/PullRequestService.cs | 8 ++--- src/GitHub.App/Services/RepositoryService.cs | 7 +++++ .../GitHubPane/PullRequestCheckViewModel.cs | 4 +++ .../GitHubPane/IPullRequestCheckViewModel.cs | 2 ++ src/GitHub.Exports/Models/CheckRunModel.cs | 2 ++ src/GitHub.Exports/Models/StatusModel.cs | 2 ++ .../Services/IRepositoryService.cs | 2 ++ .../Services/PullRequestSessionService.cs | 30 ++++++++++++++++--- .../GitHubPane/PullRequestCheckView.xaml | 12 +++++--- 10 files changed, 59 insertions(+), 12 deletions(-) diff --git a/src/GitHub.App/SampleData/PullRequestCheckViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestCheckViewModelDesigner.cs index 3011e581c1..086e842644 100644 --- a/src/GitHub.App/SampleData/PullRequestCheckViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestCheckViewModelDesigner.cs @@ -9,6 +9,8 @@ namespace GitHub.SampleData { public sealed class PullRequestCheckViewModelDesigner : ViewModelBase, IPullRequestCheckViewModel { + public bool IsRequired { get; } = true; + public string Title { get; set; } = "continuous-integration/appveyor/pr"; public string Description { get; set; } = "AppVeyor build failed"; diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index e7c8f6091f..240be1daf8 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -115,10 +115,10 @@ public async Task> ReadPullRequests( CheckSuites = commit.Commit.CheckSuites(null, null, null, null, null).AllPages(10) .Select(suite => new CheckSuiteSummaryModel { - ApplicationName = suite.App != null ? suite.App.Name : "Private Application", CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10) .Select(run => new CheckRunSummaryModel { + Name = run.Name, Conclusion = run.Conclusion.FromGraphQl(), Status = run.Status.FromGraphQl() }).ToList() @@ -233,8 +233,8 @@ public async Task> ReadPullRequests( if (protectedBranchContexts != null) { var requiredCheckRuns = item.LastCommit?.CheckSuites? - .Where(model => protectedBranchContexts.Contains(model.ApplicationName)) .SelectMany(checkSuite => checkSuite.CheckRuns) + .Where(model => protectedBranchContexts.Contains(model.Name)) .ToArray(); var requiredStatues = item.LastCommit?.Statuses @@ -249,8 +249,8 @@ public async Task> ReadPullRequests( } var checkRuns = item.LastCommit?.CheckSuites? - .Where(model => protectedBranchContexts == null || !protectedBranchContexts.Contains(model.ApplicationName)) .SelectMany(checkSuite => checkSuite.CheckRuns) + .Where(model => protectedBranchContexts == null || !protectedBranchContexts.Contains(model.Name)) .ToArray(); var statuses = item.LastCommit?.Statuses @@ -1141,11 +1141,11 @@ class LastCommitSummaryAdapter class CheckSuiteSummaryModel { public List CheckRuns { get; set; } - public string ApplicationName { get; set; } } class CheckRunSummaryModel { + public string Name { get; set; } public CheckConclusionState? Conclusion { get; set; } public CheckStatusState Status { get; set; } } diff --git a/src/GitHub.App/Services/RepositoryService.cs b/src/GitHub.App/Services/RepositoryService.cs index d4d183ea30..2212445de4 100644 --- a/src/GitHub.App/Services/RepositoryService.cs +++ b/src/GitHub.App/Services/RepositoryService.cs @@ -83,5 +83,12 @@ public async Task> GetProtectedBranches(HostAddress addre var graphql = await graphqlFactory.CreateConnection(address).ConfigureAwait(false); return await graphql.Run(queryProtectedBranches, vars).ConfigureAwait(false); } + + public async Task GetProtectedBranch(HostAddress address, string owner, string name, string branchName) + { + Guard.ArgumentNotNull(branchName, nameof(branchName)); + var protectedBranches = await GetProtectedBranches(address, owner, name).ConfigureAwait(false); + return protectedBranches.FirstOrDefault(branch => branch.Name.Equals(branchName, StringComparison.InvariantCultureIgnoreCase)); + } } } diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs index f95bd95fd1..75b162e626 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestCheckViewModel.cs @@ -45,6 +45,7 @@ public static IEnumerable Build(IViewViewModelFactor var pullRequestCheckViewModel = (PullRequestCheckViewModel) viewViewModelFactory.CreateViewModel(); pullRequestCheckViewModel.CheckType = PullRequestCheckType.StatusApi; + pullRequestCheckViewModel.IsRequired = model.IsRequired; pullRequestCheckViewModel.Title = model.Context; pullRequestCheckViewModel.Description = model.Description; pullRequestCheckViewModel.Status = checkStatus; @@ -91,6 +92,7 @@ public static IEnumerable Build(IViewViewModelFactor var pullRequestCheckViewModel = (PullRequestCheckViewModel)viewViewModelFactory.CreateViewModel(); pullRequestCheckViewModel.CheckType = PullRequestCheckType.ChecksApi; + pullRequestCheckViewModel.IsRequired = model.IsRequired; pullRequestCheckViewModel.Title = model.Name; pullRequestCheckViewModel.Description = model.Summary; pullRequestCheckViewModel.Status = checkStatus; @@ -124,6 +126,8 @@ private void DoOpenDetailsUrl() usageTracker.IncrementCounter(expression).Forget(); } + public bool IsRequired { get; private set; } + public string Title { get; private set; } public string Description { get; private set; } diff --git a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestCheckViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestCheckViewModel.cs index 1a9d658a39..c54365c0a1 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestCheckViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestCheckViewModel.cs @@ -11,6 +11,8 @@ namespace GitHub.ViewModels.GitHubPane /// public interface IPullRequestCheckViewModel: IViewModel { + bool IsRequired { get; } + /// /// The title of the Status/Check /// diff --git a/src/GitHub.Exports/Models/CheckRunModel.cs b/src/GitHub.Exports/Models/CheckRunModel.cs index a25335c8bb..e1c78b0426 100644 --- a/src/GitHub.Exports/Models/CheckRunModel.cs +++ b/src/GitHub.Exports/Models/CheckRunModel.cs @@ -33,5 +33,7 @@ public class CheckRunModel /// The summary of a Check Run. /// public string Summary { get; set; } + + public bool IsRequired { get; set; } } } \ No newline at end of file diff --git a/src/GitHub.Exports/Models/StatusModel.cs b/src/GitHub.Exports/Models/StatusModel.cs index 3b0832caa0..a911bfa7e0 100644 --- a/src/GitHub.Exports/Models/StatusModel.cs +++ b/src/GitHub.Exports/Models/StatusModel.cs @@ -24,5 +24,7 @@ public class StatusModel /// The descritption for the Status /// public string Description { get; set; } + + public bool IsRequired { get; set; } } } \ No newline at end of file diff --git a/src/GitHub.Exports/Services/IRepositoryService.cs b/src/GitHub.Exports/Services/IRepositoryService.cs index 975996bc69..c0e8ea4fd7 100644 --- a/src/GitHub.Exports/Services/IRepositoryService.cs +++ b/src/GitHub.Exports/Services/IRepositoryService.cs @@ -21,5 +21,7 @@ public interface IRepositoryService Task<(string owner, string name)?> FindParent(HostAddress address, string owner, string name); Task> GetProtectedBranches(HostAddress address, string owner, string name); + + Task GetProtectedBranch(HostAddress address, string owner, string name, string branchName); } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs index d7db28e6f5..8e41d6d57f 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs @@ -57,6 +57,7 @@ public class PullRequestSessionService : IPullRequestSessionService readonly IDiffService diffService; readonly IApiClientFactory apiClientFactory; readonly IGraphQLClientFactory graphqlFactory; + readonly IRepositoryService repositoryService; readonly IUsageTracker usageTracker; readonly IDictionary, string> mergeBaseCache; @@ -67,6 +68,7 @@ public PullRequestSessionService( IDiffService diffService, IApiClientFactory apiClientFactory, IGraphQLClientFactory graphqlFactory, + IRepositoryService repositoryService, IUsageTracker usageTracker) { this.gitService = gitService; @@ -74,6 +76,7 @@ public PullRequestSessionService( this.diffService = diffService; this.apiClientFactory = apiClientFactory; this.graphqlFactory = graphqlFactory; + this.repositoryService = repositoryService; this.usageTracker = usageTracker; mergeBaseCache = new Dictionary, string>(); @@ -351,9 +354,12 @@ public virtual async Task ReadPullRequestDetail(HostAddr var connection = await graphqlFactory.CreateConnection(address); var result = await connection.Run(readPullRequest, vars); + var protectedBranches = await repositoryService.GetProtectedBranch(address, owner, name, result.BaseRefName); + var protectedContexts = protectedBranches != null ? new HashSet(protectedBranches.RequiredStatusCheckContexts) : null; + var apiClient = await apiClientFactory.Create(address); var files = await apiClient.GetPullRequestFiles(owner, name, number).ToList(); - var lastCommitModel = await GetPullRequestLastCommitAdapter(address, owner, name, number); + var lastCommitModel = await GetPullRequestLastCommitAdapter(address, owner, name, number, protectedContexts); result.Statuses = lastCommitModel.Statuses; result.CheckSuites = lastCommitModel.CheckSuites; @@ -752,7 +758,8 @@ Task GetRepository(ILocalRepositoryModel repository) return Task.Factory.StartNew(() => gitService.GetRepository(repository.LocalPath)); } - async Task GetPullRequestLastCommitAdapter(HostAddress address, string owner, string name, int number) + async Task GetPullRequestLastCommitAdapter(HostAddress address, string owner, string name, + int number, HashSet protectedContexts) { ICompiledQuery> query; if (address.IsGitHubDotCom()) @@ -827,8 +834,23 @@ async Task GetPullRequestLastCommitAdapter(HostAddress addres }; var connection = await graphqlFactory.CreateConnection(address); - var result = await connection.Run(query, vars); - return result.First(); + var results = await connection.Run(query, vars); + var result = results.First(); + + foreach (var resultCheckSuite in result.CheckSuites) + { + foreach (var checkRunModel in resultCheckSuite.CheckRuns) + { + checkRunModel.IsRequired = protectedContexts?.Contains(checkRunModel.Name) ?? false; + } + } + + foreach (var resultStatus in result.Statuses) + { + resultStatus.IsRequired = protectedContexts?.Contains(resultStatus.Context) ?? false; + } + + return result; } static void BuildPullRequestThreads(PullRequestDetailModel model) diff --git a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml index d04daeeda1..89017fb747 100644 --- a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml +++ b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml @@ -45,10 +45,14 @@ - + + + + From 5d0c825659f2fd39a1419da8f26ae8b4033ff7fd Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Fri, 2 Nov 2018 15:05:06 -0400 Subject: [PATCH 5/8] Fix compilation error --- .../Services/PullRequestSessionServiceTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionServiceTests.cs b/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionServiceTests.cs index 0ab2b674f1..51559f5f8f 100644 --- a/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionServiceTests.cs +++ b/test/GitHub.InlineReviews.UnitTests/Services/PullRequestSessionServiceTests.cs @@ -296,6 +296,7 @@ static PullRequestSessionService CreateTarget(IDiffService diffService) diffService, Substitute.For(), Substitute.For(), + Substitute.For(), Substitute.For()); } From f25c4881e33708d5af60c76dbcfb41d9dcfc866a Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 13 Dec 2018 10:54:45 -0500 Subject: [PATCH 6/8] Reverting functionality to process protected branches in the pull request list view --- .../PullRequestListItemViewModelDesigner.cs | 2 +- src/GitHub.App/Services/PullRequestService.cs | 205 ++++-------------- .../PullRequestListItemViewModel.cs | 2 +- .../IPullRequestListItemViewModel.cs | 2 +- .../Models/PullRequestListItemModel.cs | 12 +- .../GitHubPane/PullRequestListItemView.xaml | 4 - .../Services/PullRequestServiceTests.cs | 6 - 7 files changed, 49 insertions(+), 184 deletions(-) diff --git a/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs index aba518d750..500089fb84 100644 --- a/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestListItemViewModelDesigner.cs @@ -17,6 +17,6 @@ public class PullRequestListItemViewModelDesigner : ViewModelBase, IPullRequestL public int Number { get; set; } public string Title { get; set; } public DateTimeOffset UpdatedAt { get; set; } - public PullRequestChecksSummaryState Checks { get; set; } + public PullRequestChecksState Checks { get; set; } } } diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index 6b19ff81f5..abfc1feac1 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -26,7 +26,6 @@ using static Octokit.GraphQL.Variable; using CheckConclusionState = GitHub.Models.CheckConclusionState; using CheckStatusState = GitHub.Models.CheckStatusState; -using ProtectedBranch = GitHub.Models.ProtectedBranch; using StatusState = GitHub.Models.StatusState; namespace GitHub.Services @@ -56,7 +55,6 @@ public class PullRequestService : IPullRequestService readonly IGitService gitService; readonly IVSGitExt gitExt; readonly IGraphQLClientFactory graphqlFactory; - readonly IRepositoryService repositoryService; readonly IOperatingSystem os; readonly IUsageTracker usageTracker; @@ -66,7 +64,6 @@ public PullRequestService( IGitService gitService, IVSGitExt gitExt, IGraphQLClientFactory graphqlFactory, - IRepositoryService repositoryService, IOperatingSystem os, IUsageTracker usageTracker) { @@ -74,7 +71,6 @@ public PullRequestService( this.gitService = gitService; this.gitExt = gitExt; this.graphqlFactory = graphqlFactory; - this.repositoryService = repositoryService; this.os = os; this.usageTracker = usageTracker; } @@ -108,7 +104,6 @@ public async Task> ReadPullRequests( Items = page.Nodes.Select(pr => new ListItemAdapter { Id = pr.Id.Value, - BaseRefName = pr.BaseRefName, LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit => new LastCommitSummaryAdapter { @@ -118,7 +113,6 @@ public async Task> ReadPullRequests( CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10) .Select(run => new CheckRunSummaryModel { - Name = run.Name, Conclusion = run.Conclusion.FromGraphQl(), Status = run.Status.FromGraphQl() }).ToList(), @@ -127,7 +121,6 @@ public async Task> ReadPullRequests( .Select(context => context.Contexts.Select(statusContext => new StatusSummaryModel { - Context = statusContext.Context, State = statusContext.State.FromGraphQl(), }).ToList() ).SingleOrDefault() @@ -172,7 +165,6 @@ public async Task> ReadPullRequests( Items = page.Nodes.Select(pr => new ListItemAdapter { Id = pr.Id.Value, - BaseRefName = pr.BaseRefName, LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit => new LastCommitSummaryAdapter { @@ -208,11 +200,6 @@ public async Task> ReadPullRequests( query = readPullRequestsEnterprise; } - var protectedBranches = await repositoryService.GetProtectedBranches(address, owner, name) - .ConfigureAwait(false); - - var protectedBranchesContextsDictionary = protectedBranches.ToDictionary(branch => branch.Name, branch => new HashSet(branch.RequiredStatusCheckContexts)); - var graphql = await graphqlFactory.CreateConnection(address); var vars = new Dictionary { @@ -226,172 +213,74 @@ public async Task> ReadPullRequests( foreach (var item in result.Items.Cast()) { - HashSet protectedBranchContexts; - protectedBranchesContextsDictionary.TryGetValue(item.BaseRefName, out protectedBranchContexts); - item.CommentCount += item.Reviews.Sum(x => x.Count); item.Reviews = null; - if (protectedBranchContexts != null) - { - var requiredCheckRuns = item.LastCommit?.CheckSuites? - .SelectMany(checkSuite => checkSuite.CheckRuns) - .Where(model => protectedBranchContexts.Contains(model.Name)) - .ToArray(); + var checkRuns = item.LastCommit?.CheckSuites?.SelectMany(model => model.CheckRuns).ToArray(); - var requiredStatues = item.LastCommit?.Statuses - .Where(model => protectedBranchContexts.Contains(model.Context)) - .ToArray(); + var hasCheckRuns = checkRuns?.Any() ?? false; + var hasStatuses = item.LastCommit?.Statuses?.Any() ?? false; - item.RequiredChecks = CalculatePullRequestCheckState(requiredCheckRuns, requiredStatues); - } - else + if (!hasCheckRuns && !hasStatuses) { - item.RequiredChecks = PullRequestChecksState.None; + item.Checks = PullRequestChecksState.None; } - - var checkRuns = item.LastCommit?.CheckSuites? - .SelectMany(checkSuite => checkSuite.CheckRuns) - .Where(model => protectedBranchContexts == null || !protectedBranchContexts.Contains(model.Name)) - .ToArray(); - - var statuses = item.LastCommit?.Statuses - .Where(model => protectedBranchContexts == null || !protectedBranchContexts.Contains(model.Context)) - .ToArray(); - - item.OtherChecks = CalculatePullRequestCheckState(checkRuns, statuses); - - switch (item.RequiredChecks) + else { - case PullRequestChecksState.None: - item.Checks = PullRequestChecksSummaryState.None; - break; - case PullRequestChecksState.Pending: - item.Checks = PullRequestChecksSummaryState.Pending; - break; - case PullRequestChecksState.Success: - item.Checks = PullRequestChecksSummaryState.Success; - break; - case PullRequestChecksState.Failure: - item.Checks = PullRequestChecksSummaryState.Failure; - break; - default: - throw new ArgumentOutOfRangeException(); - } + var checksHasFailure = false; + var checksHasCompleteSuccess = true; - if (item.RequiredChecks != item.OtherChecks) - { - switch (item.RequiredChecks) + if (hasCheckRuns) { - case PullRequestChecksState.None: - switch (item.OtherChecks) - { - case PullRequestChecksState.None: - item.Checks = PullRequestChecksSummaryState.None; - break; - case PullRequestChecksState.Pending: - item.Checks = PullRequestChecksSummaryState.Pending; - break; - case PullRequestChecksState.Success: - item.Checks = PullRequestChecksSummaryState.Success; - break; - case PullRequestChecksState.Failure: - item.Checks = PullRequestChecksSummaryState.Failure; - break; - default: - throw new ArgumentOutOfRangeException(); - } - break; - case PullRequestChecksState.Pending: - item.Checks = PullRequestChecksSummaryState.Pending; - break; - case PullRequestChecksState.Success: - switch (item.OtherChecks) - { - case PullRequestChecksState.Pending: - item.Checks = PullRequestChecksSummaryState.SuccessWithPending; - break; - case PullRequestChecksState.Failure: - item.Checks = PullRequestChecksSummaryState.SuccessWithFailure; - break; - } - break; - case PullRequestChecksState.Failure: - item.Checks = PullRequestChecksSummaryState.Failure; - break; - default: - throw new ArgumentOutOfRangeException(); - } - } - - item.LastCommit = null; - } + checksHasFailure = checkRuns + .Any(model => model.Conclusion.HasValue + && (model.Conclusion.Value == CheckConclusionState.Failure + || model.Conclusion.Value == CheckConclusionState.ActionRequired)); - return result; - } + if (!checksHasFailure) + { + checksHasCompleteSuccess = checkRuns + .All(model => model.Conclusion.HasValue + && (model.Conclusion.Value == CheckConclusionState.Success + || model.Conclusion.Value == CheckConclusionState.Neutral)); + } + } - static PullRequestChecksState CalculatePullRequestCheckState(IList checkRuns, IList statusSummaryModels) - { - var hasCheckRuns = checkRuns?.Any() ?? false; - var hasStatuses = statusSummaryModels?.Any() ?? false; + var statusHasFailure = false; + var statusHasCompleteSuccess = true; - PullRequestChecksState resultState; - if (!hasCheckRuns && !hasStatuses) - { - resultState = PullRequestChecksState.None; - } - else - { - var checksHasFailure = false; - var checksHasCompleteSuccess = true; + if (!checksHasFailure && hasStatuses) + { + statusHasFailure = item.LastCommit + .Statuses + .Any(status => status.State == StatusState.Failure + || status.State == StatusState.Error); - if (hasCheckRuns) - { - checksHasFailure = checkRuns - .Any(model => model.Conclusion.HasValue - && (model.Conclusion.Value == CheckConclusionState.Failure - || model.Conclusion.Value == CheckConclusionState.ActionRequired)); + if (!statusHasFailure) + { + statusHasCompleteSuccess = + item.LastCommit.Statuses.All(status => status.State == StatusState.Success); + } + } - if (!checksHasFailure) + if (checksHasFailure || statusHasFailure) { - checksHasCompleteSuccess = checkRuns - .All(model => model.Conclusion.HasValue - && (model.Conclusion.Value == CheckConclusionState.Success - || model.Conclusion.Value == CheckConclusionState.Neutral)); + item.Checks = PullRequestChecksState.Failure; } - } - - var statusHasFailure = false; - var statusHasCompleteSuccess = true; - - if (!checksHasFailure && hasStatuses) - { - statusHasFailure = statusSummaryModels - .Any(status => status.State == StatusState.Failure - || status.State == StatusState.Error); - - if (!statusHasFailure) + else if (statusHasCompleteSuccess && checksHasCompleteSuccess) { - statusHasCompleteSuccess = - statusSummaryModels.All(status => status.State == StatusState.Success); + item.Checks = PullRequestChecksState.Success; + } + else + { + item.Checks = PullRequestChecksState.Pending; } } - if (checksHasFailure || statusHasFailure) - { - resultState = PullRequestChecksState.Failure; - } - else if (statusHasCompleteSuccess && checksHasCompleteSuccess) - { - resultState = PullRequestChecksState.Success; - } - else - { - resultState = PullRequestChecksState.Pending; - } + item.LastCommit = null; } - return resultState; + return result; } public async Task> ReadAssignableUsers( @@ -1122,8 +1011,6 @@ class ListItemAdapter : PullRequestListItemModel public IList Reviews { get; set; } public LastCommitSummaryAdapter LastCommit { get; set; } - - public string BaseRefName { get; set; } } class ReviewAdapter @@ -1147,7 +1034,6 @@ class CheckSuiteSummaryModel class CheckRunSummaryModel { - public string Name { get; set; } public CheckConclusionState? Conclusion { get; set; } public CheckStatusState Status { get; set; } } @@ -1155,7 +1041,6 @@ class CheckRunSummaryModel class StatusSummaryModel { public StatusState State { get; set; } - public string Context { get; set; } } } } diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs index f4f0a8693b..b60cf0e70b 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestListItemViewModel.cs @@ -33,7 +33,7 @@ public PullRequestListItemViewModel(PullRequestListItemModel model) public IActorViewModel Author { get; } /// - public PullRequestChecksSummaryState Checks { get; } + public PullRequestChecksState Checks { get; } /// public int CommentCount { get; } diff --git a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs index 0f135924b5..f8fa89c351 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestListItemViewModel.cs @@ -32,6 +32,6 @@ public interface IPullRequestListItemViewModel : IIssueListItemViewModelBase /// /// Gets the pull request checks and statuses summary /// - PullRequestChecksSummaryState Checks { get; } + PullRequestChecksState Checks { get; } } } diff --git a/src/GitHub.Exports/Models/PullRequestListItemModel.cs b/src/GitHub.Exports/Models/PullRequestListItemModel.cs index 18a104899f..f6a9a7fe55 100644 --- a/src/GitHub.Exports/Models/PullRequestListItemModel.cs +++ b/src/GitHub.Exports/Models/PullRequestListItemModel.cs @@ -40,17 +40,7 @@ public class PullRequestListItemModel /// /// Gets the pull request checks and statuses summary /// - public PullRequestChecksState RequiredChecks { get; set; } - - /// - /// Gets the pull request checks and statuses summary - /// - public PullRequestChecksState OtherChecks { get; set; } - - /// - /// Gets the pull request checks and statuses summary - /// - public PullRequestChecksSummaryState Checks { get; set; } + public PullRequestChecksState Checks { get; set; } /// /// Gets or sets the date/time at which the pull request was last updated. diff --git a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml index 19120e288f..4d7e261552 100644 --- a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml +++ b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestListItemView.xaml @@ -101,10 +101,6 @@ - - - - diff --git a/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs b/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs index 6bc72c00f3..6fedcf1526 100644 --- a/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs +++ b/test/GitHub.App.UnitTests/Services/PullRequestServiceTests.cs @@ -576,7 +576,6 @@ static PullRequestService CreatePullRequestService(Repository repo) gitService, Substitute.For(), Substitute.For(), - Substitute.For(), serviceProvider.GetOperatingSystem(), Substitute.For()); return service; @@ -690,7 +689,6 @@ public void CreatePullRequestAllArgsMandatory() serviceProvider.GetGitService(), Substitute.For(), Substitute.For(), - Substitute.For(), serviceProvider.GetOperatingSystem(), Substitute.For()); @@ -904,7 +902,6 @@ public async Task ShouldReturnCorrectDefaultLocalBranchNameForPullRequestsWithNo MockGitService(), Substitute.For(), Substitute.For(), - Substitute.For(), Substitute.For(), Substitute.For()); @@ -1062,7 +1059,6 @@ static PullRequestService CreateTarget( IGitService gitService = null, IVSGitExt gitExt = null, IGraphQLClientFactory graphqlFactory = null, - IRepositoryService repositoryService = null, IOperatingSystem os = null, IUsageTracker usageTracker = null) { @@ -1072,14 +1068,12 @@ static PullRequestService CreateTarget( graphqlFactory = graphqlFactory ?? Substitute.For(); os = os ?? Substitute.For(); usageTracker = usageTracker ?? Substitute.For(); - repositoryService = repositoryService ?? Substitute.For(); return new PullRequestService( gitClient, gitService, gitExt, graphqlFactory, - repositoryService, os, usageTracker); } From 1cf3c800201ba4d7c273d8e9c51508629bc513a6 Mon Sep 17 00:00:00 2001 From: Stanley Goldman Date: Thu, 13 Dec 2018 10:59:46 -0500 Subject: [PATCH 7/8] Repositioning --- .../Views/GitHubPane/PullRequestCheckView.xaml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml index 8e11b97711..caf2380d7f 100644 --- a/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml +++ b/src/GitHub.VisualStudio.UI/Views/GitHubPane/PullRequestCheckView.xaml @@ -28,7 +28,6 @@ - @@ -48,13 +47,13 @@ - + -