diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index 90ac1d1b77..5e17d636ee 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -30,6 +30,7 @@ using static Octokit.GraphQL.Variable; using CheckConclusionState = GitHub.Models.CheckConclusionState; using CheckStatusState = GitHub.Models.CheckStatusState; +using PullRequestState = GitHub.Models.PullRequestState; using StatusState = GitHub.Models.StatusState; namespace GitHub.Services @@ -44,8 +45,8 @@ public class PullRequestService : IssueishService, IPullRequestService, IStaticR static readonly Regex InvalidBranchCharsRegex = new Regex(@"[^0-9A-Za-z\-]", RegexOptions.ECMAScript); static readonly Regex BranchCapture = new Regex(@"branch\.(?.+)\.ghfvs-pr", RegexOptions.ECMAScript); static ICompiledQuery> readAssignableUsers; - static ICompiledQuery> readPullRequests; - static ICompiledQuery> readPullRequestsEnterprise; + static ICompiledQuery> readPullRequests; + static ICompiledQuery> readPullRequestsEnterprise; static readonly string[] TemplatePaths = new[] { @@ -84,73 +85,72 @@ public PullRequestService( this.tempFileMappings = new Dictionary(StringComparer.OrdinalIgnoreCase); } - public async Task> ReadPullRequests( - HostAddress address, + public async Task> ReadPullRequests(HostAddress address, string owner, string name, string after, - Models.PullRequestState[] states) + PullRequestState[] states, string author) { + string filter = null; + ICompiledQuery> query; + - ICompiledQuery> query; if (address.IsGitHubDotCom()) { if (readPullRequests == null) { - readPullRequests = new Query() - .Repository(owner: Var(nameof(owner)), name: Var(nameof(name))) - .PullRequests( - first: 100, - after: Var(nameof(after)), - orderBy: new IssueOrder { Direction = OrderDirection.Desc, Field = IssueOrderField.CreatedAt }, - states: Var(nameof(states))) - .Select(page => new Page - { - EndCursor = page.PageInfo.EndCursor, - HasNextPage = page.PageInfo.HasNextPage, - TotalCount = page.TotalCount, - Items = page.Nodes.Select(pr => new ListItemAdapter - { - Id = pr.Id.Value, - 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 - { - CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10) - .Select(run => new CheckRunSummaryModel - { - Conclusion = run.Conclusion.FromGraphQl(), - Status = run.Status.FromGraphQl() - }).ToList(), - }).ToList(), - Statuses = commit.Commit.Status - .Select(context => - context.Contexts.Select(statusContext => new StatusSummaryModel - { - State = statusContext.State.FromGraphQl(), - }).ToList() - ).SingleOrDefault() - }).ToList().FirstOrDefault(), - Author = new ActorModel - { - Login = pr.Author.Login, - AvatarUrl = pr.Author.AvatarUrl(null), - }, - CommentCount = pr.Comments(0, null, null, null).TotalCount, - Number = pr.Number, - Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new ReviewAdapter - { - Body = review.Body, - CommentCount = review.Comments(null, null, null, null).TotalCount, - }).ToList(), - State = pr.State.FromGraphQl(), - Title = pr.Title, - UpdatedAt = pr.UpdatedAt, - }).ToList(), - }).Compile(); + readPullRequests = new Query().Search(Var(nameof(filter)), first: 100, after: Var(nameof(after)), type: SearchType.Issue) + .Select(page => new Page + { + EndCursor = page.PageInfo.EndCursor, + HasNextPage = page.PageInfo.HasNextPage, + TotalCount = page.IssueCount, + Items = page.Nodes.Select(issueOrPr => new SearchItemAdapter + { + Result = issueOrPr.Switch(when => when.PullRequest(pr => new ListItemAdapter + { + Id = pr.Id.Value, + 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 + { + CheckRuns = suite.CheckRuns(null, null, null, null, null).AllPages(10) + .Select(run => new CheckRunSummaryModel + { + Conclusion = run.Conclusion.FromGraphQl(), + Status = run.Status.FromGraphQl() + }).ToList(), + }).ToList(), + Statuses = commit.Commit.Status + .Select(context => + context.Contexts.Select(statusContext => new StatusSummaryModel + { + State = statusContext.State.FromGraphQl(), + }).ToList() + ).SingleOrDefault() + }).ToList().FirstOrDefault(), + Author = new ActorModel + { + Login = pr.Author.Login, + AvatarUrl = pr.Author.AvatarUrl(null), + }, + CommentCount = pr.Comments(0, null, null, null).TotalCount, + Number = pr.Number, + Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new ReviewAdapter + { + Body = review.Body, + CommentCount = review.Comments(null, null, null, null).TotalCount, + }).ToList(), + State = pr.State.FromGraphQl(), + Title = pr.Title, + UpdatedAt = pr.UpdatedAt, + })) + }).ToList() + }) + .Compile(); } query = readPullRequests; @@ -159,138 +159,169 @@ public async Task> ReadPullRequests( { if (readPullRequestsEnterprise == null) { - readPullRequestsEnterprise = new Query() - .Repository(owner: Var(nameof(owner)), name: Var(nameof(name))) - .PullRequests( - first: 100, - after: Var(nameof(after)), - orderBy: new IssueOrder { Direction = OrderDirection.Desc, Field = IssueOrderField.CreatedAt }, - states: Var(nameof(states))) - .Select(page => new Page - { - EndCursor = page.PageInfo.EndCursor, - HasNextPage = page.PageInfo.HasNextPage, - TotalCount = page.TotalCount, - Items = page.Nodes.Select(pr => new ListItemAdapter - { - Id = pr.Id.Value, - LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit => - new LastCommitSummaryAdapter - { - Statuses = commit.Commit.Status.Select(context => - context == null - ? null - : context.Contexts - .Select(statusContext => new StatusSummaryModel - { - State = statusContext.State.FromGraphQl() - }).ToList() - ).SingleOrDefault() - }).ToList().FirstOrDefault(), - Author = new ActorModel - { - Login = pr.Author.Login, - AvatarUrl = pr.Author.AvatarUrl(null), - }, - CommentCount = pr.Comments(0, null, null, null).TotalCount, - Number = pr.Number, - Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new ReviewAdapter - { - Body = review.Body, - CommentCount = review.Comments(null, null, null, null).TotalCount, - }).ToList(), - State = pr.State.FromGraphQl(), - Title = pr.Title, - UpdatedAt = pr.UpdatedAt, - }).ToList(), - }).Compile(); + readPullRequestsEnterprise = new Query().Search(Var(nameof(filter)), first: 100, after: Var(nameof(after)), type: SearchType.Issue) + .Select(page => new Page + { + EndCursor = page.PageInfo.EndCursor, + HasNextPage = page.PageInfo.HasNextPage, + TotalCount = page.IssueCount, + Items = page.Nodes.Select(issueOrPr => new SearchItemAdapter() + { + Result = issueOrPr.Switch(when => + when.PullRequest(pr => new ListItemAdapter + { + Id = pr.Id.Value, + LastCommit = pr.Commits(null, null, 1, null).Nodes.Select(commit => + new LastCommitSummaryAdapter + { + Statuses = commit.Commit.Status.Select(context => + context == null + ? null + : context.Contexts + .Select(statusContext => new StatusSummaryModel + { + State = statusContext.State.FromGraphQl() + }).ToList() + ).SingleOrDefault() + }).ToList().FirstOrDefault(), + Author = new ActorModel + { + Login = pr.Author.Login, + AvatarUrl = pr.Author.AvatarUrl(null), + }, + CommentCount = pr.Comments(0, null, null, null).TotalCount, + Number = pr.Number, + Reviews = pr.Reviews(null, null, null, null, null, null).AllPages().Select(review => new ReviewAdapter + { + Body = review.Body, + CommentCount = review.Comments(null, null, null, null).TotalCount, + }).ToList(), + State = pr.State.FromGraphQl(), + Title = pr.Title, + UpdatedAt = pr.UpdatedAt, + })) + }).ToList() + }) + .Compile(); } query = readPullRequestsEnterprise; } + filter = $"type:pr repo:{owner}/{name}"; + if (states.Any()) + { + var filterState = states.First(); + switch (filterState) + { + case PullRequestState.Open: + filter += " is:open"; + break; + case PullRequestState.Closed: + filter += " is:unmerged is:closed"; + break; + case PullRequestState.Merged: + filter += " is:merged is:closed"; + break; + + default: + throw new ArgumentOutOfRangeException(nameof(filterState)); + } + } + var graphql = await graphqlFactory.CreateConnection(address); var vars = new Dictionary { - { nameof(owner), owner }, - { nameof(name), name }, + { nameof(filter), filter }, { nameof(after), after }, - { nameof(states), states.Select(x => (Octokit.GraphQL.Model.PullRequestState)x).ToList() }, }; var result = await graphql.Run(query, vars); foreach (var item in result.Items.Cast()) { - item.CommentCount += item.Reviews.Sum(x => x.Count); - item.Reviews = null; + ConvertItemAdapterToListItem(item); + } - var checkRuns = item.LastCommit?.CheckSuites?.SelectMany(model => model.CheckRuns).ToArray(); - var statuses = item.LastCommit?.Statuses; + return new Page() + { + EndCursor = result.EndCursor, + HasNextPage = result.HasNextPage, + TotalCount = result.TotalCount, + Items = new List(result.Items.Select(model => ConvertItemAdapterToListItem(model.Result))) + }; + } - var totalCount = 0; - var pendingCount = 0; - var successCount = 0; - var errorCount = 0; + static PullRequestListItemModel ConvertItemAdapterToListItem(ListItemAdapter item) + { + item.CommentCount += item.Reviews.Sum(x => x.Count); + item.Reviews = null; - if (checkRuns != null) - { - totalCount += checkRuns.Length; - - pendingCount += checkRuns.Count(model => model.Status != CheckStatusState.Completed); - - successCount += checkRuns.Count(model => model.Status == CheckStatusState.Completed && - model.Conclusion.HasValue && - (model.Conclusion == CheckConclusionState.Success || - model.Conclusion == CheckConclusionState.Neutral)); - errorCount += checkRuns.Count(model => model.Status == CheckStatusState.Completed && - model.Conclusion.HasValue && - !(model.Conclusion == CheckConclusionState.Success || - model.Conclusion == CheckConclusionState.Neutral)); - } + var checkRuns = item.LastCommit?.CheckSuites?.SelectMany(model => model.CheckRuns).ToArray(); + var statuses = item.LastCommit?.Statuses; - if (statuses != null) - { - totalCount += statuses.Count; + var totalCount = 0; + var pendingCount = 0; + var successCount = 0; + var errorCount = 0; - pendingCount += statuses.Count(model => - model.State == StatusState.Pending || model.State == StatusState.Expected); + if (checkRuns != null) + { + totalCount += checkRuns.Length; + + pendingCount += checkRuns.Count(model => model.Status != CheckStatusState.Completed); + + successCount += checkRuns.Count(model => model.Status == CheckStatusState.Completed && + model.Conclusion.HasValue && + (model.Conclusion == CheckConclusionState.Success || + model.Conclusion == CheckConclusionState.Neutral)); + errorCount += checkRuns.Count(model => model.Status == CheckStatusState.Completed && + model.Conclusion.HasValue && + !(model.Conclusion == CheckConclusionState.Success || + model.Conclusion == CheckConclusionState.Neutral)); + } - successCount += statuses.Count(model => model.State == StatusState.Success); + if (statuses != null) + { + totalCount += statuses.Count; - errorCount += statuses.Count(model => - model.State == StatusState.Error || model.State == StatusState.Failure); - } + pendingCount += statuses.Count(model => + model.State == StatusState.Pending || model.State == StatusState.Expected); - item.ChecksPendingCount = pendingCount; - item.ChecksSuccessCount = successCount; - item.ChecksErrorCount = errorCount; + successCount += statuses.Count(model => model.State == StatusState.Success); - if (totalCount == 0) - { - item.ChecksSummary = PullRequestChecksSummaryState.None; - } - else if (totalCount == pendingCount) - { - item.ChecksSummary = PullRequestChecksSummaryState.Pending; - } - else if (totalCount == successCount) - { - item.ChecksSummary = PullRequestChecksSummaryState.Success; - } - else if (totalCount == errorCount) - { - item.ChecksSummary = PullRequestChecksSummaryState.Failure; - } - else - { - item.ChecksSummary = PullRequestChecksSummaryState.Mixed; - } + errorCount += statuses.Count(model => + model.State == StatusState.Error || model.State == StatusState.Failure); + } - item.LastCommit = null; + item.ChecksPendingCount = pendingCount; + item.ChecksSuccessCount = successCount; + item.ChecksErrorCount = errorCount; + + if (totalCount == 0) + { + item.ChecksSummary = PullRequestChecksSummaryState.None; } + else if (totalCount == pendingCount) + { + item.ChecksSummary = PullRequestChecksSummaryState.Pending; + } + else if (totalCount == successCount) + { + item.ChecksSummary = PullRequestChecksSummaryState.Success; + } + else if (totalCount == errorCount) + { + item.ChecksSummary = PullRequestChecksSummaryState.Failure; + } + else + { + item.ChecksSummary = PullRequestChecksSummaryState.Mixed; + } + + item.LastCommit = null; - return result; + return item; } public async Task> ReadAssignableUsers( @@ -1065,6 +1096,11 @@ static string CanonicalizeLocalFilePath(string localPath) return Path.GetFullPath(localPath); } + class SearchItemAdapter + { + public ListItemAdapter Result { get; set; } + } + class ListItemAdapter : PullRequestListItemModel { public IList Reviews { get; set; } diff --git a/src/GitHub.App/ViewModels/GitHubPane/IssueListViewModelBase.cs b/src/GitHub.App/ViewModels/GitHubPane/IssueListViewModelBase.cs index 9525991fd0..9b52815ad4 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/IssueListViewModelBase.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/IssueListViewModelBase.cs @@ -286,13 +286,6 @@ bool FilterItem(object o) } } - if (result && AuthorFilter.Selected != null) - { - result = item.Author.Login.Equals( - AuthorFilter.Selected.Login, - StringComparison.CurrentCultureIgnoreCase); - } - return result; } diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestListViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestListViewModel.cs index d81467e938..2d5e285595 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestListViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestListViewModel.cs @@ -141,12 +141,15 @@ protected override async Task> LoadPage(string af break; } + var authorLogin = owner.AuthorFilter.Selected?.Login; + var result = await owner.service.ReadPullRequests( HostAddress.Create(owner.RemoteRepository.CloneUrl), owner.RemoteRepository.Owner, owner.RemoteRepository.Name, after, - states).ConfigureAwait(false); + states, + authorLogin).ConfigureAwait(false); return result; } } diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs index 9513f0a177..64ad2ede3f 100644 --- a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs +++ b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs @@ -19,13 +19,14 @@ public interface IPullRequestService : IIssueishService /// The repository name. /// The end cursor of the previous page, or null for the first page. /// The pull request states to filter by + /// /// A page of pull request item models. - Task> ReadPullRequests( - HostAddress address, + Task> ReadPullRequests(HostAddress address, string owner, string name, string after, - PullRequestState[] states); + PullRequestState[] states, + string author); /// /// Reads a page of users that can be assigned to pull requests. diff --git a/src/GitHub.Exports/Models/PullRequestListItemModel.cs b/src/GitHub.Exports/Models/PullRequestListItemModel.cs index fa3a71fb9a..66906d99a6 100644 --- a/src/GitHub.Exports/Models/PullRequestListItemModel.cs +++ b/src/GitHub.Exports/Models/PullRequestListItemModel.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; namespace GitHub.Models { diff --git a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/IssueListViewModelBaseTests.cs b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/IssueListViewModelBaseTests.cs index 2b274342c4..400cf3dd8d 100644 --- a/test/GitHub.App.UnitTests/ViewModels/GitHubPane/IssueListViewModelBaseTests.cs +++ b/test/GitHub.App.UnitTests/ViewModels/GitHubPane/IssueListViewModelBaseTests.cs @@ -118,7 +118,7 @@ protected static IPullRequestSessionManager CreateSessionManager(PullRequestDeta protected static IPullRequestService CreatePullRequestService(int itemCount = 10) { var result = Substitute.For(); - result.ReadPullRequests(null, null, null, null, null).ReturnsForAnyArgs( + result.ReadPullRequests(null, null, null, null, null, null).ReturnsForAnyArgs( new Page { Items = Enumerable.Range(0, itemCount).Select(x => new PullRequestListItemModel