From ae6ad7c0e7ea56c5c0bae0a528208f09a16163cb Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 11 Jun 2018 20:20:49 +0100 Subject: [PATCH 1/9] Infer PR associated with current branch Search PRs in current and parent repo for one that targets the current branch name and owner. --- .../Services/IPullRequestSessionService.cs | 7 +++++ .../Services/PullRequestSessionManager.cs | 13 ++++++++ .../Services/PullRequestSessionService.cs | 31 +++++++++++++++++++ 3 files changed, 51 insertions(+) diff --git a/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs index d816271980..d634b7e174 100644 --- a/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs @@ -135,6 +135,13 @@ Task ExtractFileFromGit( /// Task ReadFileAsync(string path); + /// + /// Infer the PR that is most likely to be associated with the current branch. + /// + /// The local repository. + /// A pull request qualifed by its target owner and number. + Task> InferPullRequestForCurrentBranch(ILocalRepositoryModel repository); + /// /// Reads a for a specified pull request. /// diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index 56109be303..9d29390b02 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -203,6 +203,19 @@ async Task StatusChanged() var session = CurrentSession; var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync(); + if(pr == null) + { + try + { + // If the branch hasn't been tagged, try inferring the associated PR + pr = await sessionService.InferPullRequestForCurrentBranch(repository); + } + catch(Exception e) + { + log.Error(e, nameof(sessionService.InferPullRequestForCurrentBranch)); + } + } + if (pr != null) { var changePR = diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs index 0e0eb3eb39..0958f3fa91 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs @@ -266,6 +266,37 @@ public async Task ReadFileAsync(string path) return null; } + /// + public async Task> InferPullRequestForCurrentBranch(ILocalRepositoryModel localRepository) + { + var address = HostAddress.Create(localRepository.CloneUrl.Host); + var graphql = await graphqlFactory.CreateConnection(address); + + var localOwner = localRepository.Owner; + var headRefName = localRepository.CurrentBranch.Name; + + var query = + from x in new Query().Repository(localOwner, localRepository.Name) + select Enumerable.Concat( + (from a in x.PullRequests(100, null, null, null, null, null, headRefName, null, null).Nodes select new PullRequestOwnerInfo { BaseRepositoryOwner = a.Repository.Owner.Login, Number = a.Number, HeadRepositoryOwner = a.HeadRepositoryOwner.Login, HeadRefName = a.HeadRefName }).ToList(), + x.Parent != null ? (from b in x.Parent.PullRequests(100, null, null, null, null, null, headRefName, null, null).Nodes select new PullRequestOwnerInfo { BaseRepositoryOwner = b.Repository.Owner.Login, Number = b.Number, HeadRepositoryOwner = b.HeadRepositoryOwner.Login, HeadRefName = b.HeadRefName }).ToList() : new List() + ); + + var results = await graphql.Run(query); + + var match = results.Where(x => x.HeadRepositoryOwner == localOwner && x.HeadRefName == headRefName).FirstOrDefault(); + + return match != null ? new Tuple(match.BaseRepositoryOwner, match.Number) : null; + } + + public class PullRequestOwnerInfo + { + public string BaseRepositoryOwner { get; set; } + public int Number { get; set; } + public string HeadRepositoryOwner { get; set; } + public string HeadRefName { get; set; } + } + public virtual async Task ReadPullRequestDetail(HostAddress address, string owner, string name, int number) { if (readPullRequest == null) From a2e393720aa60befd3f7e12c2e53cca94b4014d8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 8 Jun 2018 11:42:44 +0100 Subject: [PATCH 2/9] Find associated PR using remote tracking branch --- src/GitHub.Exports/Models/BranchModel.cs | 17 +++++++++-- src/GitHub.Exports/Models/IBranch.cs | 5 +++- .../Services/IPullRequestSessionService.cs | 7 +++-- .../Services/PullRequestSessionManager.cs | 14 ++++++--- .../Services/PullRequestSessionService.cs | 29 +++++++++---------- 5 files changed, 46 insertions(+), 26 deletions(-) diff --git a/src/GitHub.Exports/Models/BranchModel.cs b/src/GitHub.Exports/Models/BranchModel.cs index 9a02091871..2d390752f2 100644 --- a/src/GitHub.Exports/Models/BranchModel.cs +++ b/src/GitHub.Exports/Models/BranchModel.cs @@ -1,5 +1,6 @@ using System; using System.Globalization; +using GitHub.Primitives; using GitHub.Services; namespace GitHub.Models @@ -34,19 +35,29 @@ public BranchModel(LibGit2Sharp.Branch branch, IRepositoryModel repo, IGitServic #pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. Repository = branch.IsRemote ? new LocalRepositoryModel(branch.Remote.Url, gitService) : repo; #pragma warning restore 0618 - IsTracking = branch.IsTracking; Sha = branch.Tip?.Sha; - TrackedSha = branch.TrackedBranch?.Tip?.Sha; Id = String.Format(CultureInfo.InvariantCulture, "{0}/{1}", Repository.Owner, Name); + + if(IsTracking = branch.IsTracking) + { + var trackedBranch = branch.TrackedBranch; + TrackedSha = trackedBranch.Tip?.Sha; +#pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. + TrackedRemoteUrl = new UriString(trackedBranch.Remote.Url); +#pragma warning restore 0618 + TrackedRemoteCanonicalName = trackedBranch.UpstreamBranchCanonicalName; + } } public string Id { get; private set; } public string Name { get; private set; } public IRepositoryModel Repository { get; private set; } - public bool IsTracking { get; private set; } public string DisplayName { get; set; } public string Sha { get; private set; } + public bool IsTracking { get; private set; } public string TrackedSha { get; private set; } + public UriString TrackedRemoteUrl { get; private set; } + public string TrackedRemoteCanonicalName { get; private set; } #region Equality things public void CopyFrom(IBranch other) diff --git a/src/GitHub.Exports/Models/IBranch.cs b/src/GitHub.Exports/Models/IBranch.cs index 223172e8fe..625be66d12 100644 --- a/src/GitHub.Exports/Models/IBranch.cs +++ b/src/GitHub.Exports/Models/IBranch.cs @@ -1,4 +1,5 @@ using System; +using GitHub.Primitives; using GitHub.Collections; namespace GitHub.Models @@ -9,9 +10,11 @@ public interface IBranch : ICopyable, string Id { get; } string Name { get; } IRepositoryModel Repository { get; } - bool IsTracking { get; } string DisplayName { get; set; } string Sha { get; } + bool IsTracking { get; } string TrackedSha { get; } + UriString TrackedRemoteUrl { get; } + string TrackedRemoteCanonicalName { get; } } } diff --git a/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs index d634b7e174..7e95dcc735 100644 --- a/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/IPullRequestSessionService.cs @@ -136,11 +136,12 @@ Task ExtractFileFromGit( Task ReadFileAsync(string path); /// - /// Infer the PR that is most likely to be associated with the current branch. + /// Find the first PR (if any) associated with a ref. /// - /// The local repository. + /// The repository the ref is from. + /// The fully qualified ref name. /// A pull request qualifed by its target owner and number. - Task> InferPullRequestForCurrentBranch(ILocalRepositoryModel repository); + Task> FindPullRequestForRef(UriString repositoryUrl, string refQualifiedName); /// /// Reads a for a specified pull request. diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index 9d29390b02..3e3b1eff99 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -202,17 +202,23 @@ async Task StatusChanged() { var session = CurrentSession; - var pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync(); + + Tuple pr = null; + + // HACK: Don't use tagged branch info when testing. + // pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync(); + if(pr == null) { try { // If the branch hasn't been tagged, try inferring the associated PR - pr = await sessionService.InferPullRequestForCurrentBranch(repository); + var branch = repository.CurrentBranch; + pr = await sessionService.FindPullRequestForRef(branch.TrackedRemoteUrl, branch.TrackedRemoteCanonicalName); } - catch(Exception e) + catch (Exception e) { - log.Error(e, nameof(sessionService.InferPullRequestForCurrentBranch)); + log.Error(e, nameof(sessionService.FindPullRequestForRef)); } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs index 0958f3fa91..4deadb4ad8 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs @@ -267,25 +267,26 @@ public async Task ReadFileAsync(string path) } /// - public async Task> InferPullRequestForCurrentBranch(ILocalRepositoryModel localRepository) + public async Task> FindPullRequestForRef(UriString repositoryUrl, string refQualifiedName) { - var address = HostAddress.Create(localRepository.CloneUrl.Host); + var address = HostAddress.Create(repositoryUrl.Host); var graphql = await graphqlFactory.CreateConnection(address); - var localOwner = localRepository.Owner; - var headRefName = localRepository.CurrentBranch.Name; - var query = - from x in new Query().Repository(localOwner, localRepository.Name) - select Enumerable.Concat( - (from a in x.PullRequests(100, null, null, null, null, null, headRefName, null, null).Nodes select new PullRequestOwnerInfo { BaseRepositoryOwner = a.Repository.Owner.Login, Number = a.Number, HeadRepositoryOwner = a.HeadRepositoryOwner.Login, HeadRefName = a.HeadRefName }).ToList(), - x.Parent != null ? (from b in x.Parent.PullRequests(100, null, null, null, null, null, headRefName, null, null).Nodes select new PullRequestOwnerInfo { BaseRepositoryOwner = b.Repository.Owner.Login, Number = b.Number, HeadRepositoryOwner = b.HeadRepositoryOwner.Login, HeadRefName = b.HeadRefName }).ToList() : new List() - ); + from x in new Query() + .Repository(repositoryUrl.Owner, repositoryUrl.RepositoryName) + .Ref(refQualifiedName) + .AssociatedPullRequests(first: 100) + .Nodes + select + new PullRequestOwnerInfo + { + BaseRepositoryOwner = x.Repository.Owner.Login, + Number = x.Number + }; var results = await graphql.Run(query); - - var match = results.Where(x => x.HeadRepositoryOwner == localOwner && x.HeadRefName == headRefName).FirstOrDefault(); - + var match = results.FirstOrDefault(); return match != null ? new Tuple(match.BaseRepositoryOwner, match.Number) : null; } @@ -293,8 +294,6 @@ public class PullRequestOwnerInfo { public string BaseRepositoryOwner { get; set; } public int Number { get; set; } - public string HeadRepositoryOwner { get; set; } - public string HeadRefName { get; set; } } public virtual async Task ReadPullRequestDetail(HostAddress address, string owner, string name, int number) From 329b56c7ce7453eefbc303a12aef8ca18c367d83 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 8 Jun 2018 12:59:46 +0100 Subject: [PATCH 3/9] Remove redundant branch.IsRemote check BranchModel is always constructed using a local branch so IsRemote is never true (it is passed repo.Head not repo.Remote.Head). --- src/GitHub.Exports/Models/BranchModel.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/GitHub.Exports/Models/BranchModel.cs b/src/GitHub.Exports/Models/BranchModel.cs index 2d390752f2..98cebd178d 100644 --- a/src/GitHub.Exports/Models/BranchModel.cs +++ b/src/GitHub.Exports/Models/BranchModel.cs @@ -32,9 +32,7 @@ public BranchModel(LibGit2Sharp.Branch branch, IRepositoryModel repo, IGitServic Extensions.Guard.ArgumentNotNull(branch, nameof(branch)); Extensions.Guard.ArgumentNotNull(repo, nameof(repo)); Name = DisplayName = branch.FriendlyName; -#pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. - Repository = branch.IsRemote ? new LocalRepositoryModel(branch.Remote.Url, gitService) : repo; -#pragma warning restore 0618 + Repository = repo; Sha = branch.Tip?.Sha; Id = String.Format(CultureInfo.InvariantCulture, "{0}/{1}", Repository.Owner, Name); From c865da8f7c67b7701ba66573bda0102735e98445 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 8 Jun 2018 14:13:02 +0100 Subject: [PATCH 4/9] Extract LocalBranchModel from BranchModel LocalBranchModel represents the local branch of a LocalRepositoryModel. --- src/GitHub.Exports/GitHub.Exports.csproj | 1 + src/GitHub.Exports/Models/BranchModel.cs | 32 ++++--------------- src/GitHub.Exports/Models/LocalBranchModel.cs | 25 +++++++++++++++ .../Models/LocalRepositoryModel.cs | 2 +- 4 files changed, 33 insertions(+), 27 deletions(-) create mode 100644 src/GitHub.Exports/Models/LocalBranchModel.cs diff --git a/src/GitHub.Exports/GitHub.Exports.csproj b/src/GitHub.Exports/GitHub.Exports.csproj index b127580afb..b53828bac7 100644 --- a/src/GitHub.Exports/GitHub.Exports.csproj +++ b/src/GitHub.Exports/GitHub.Exports.csproj @@ -164,6 +164,7 @@ + diff --git a/src/GitHub.Exports/Models/BranchModel.cs b/src/GitHub.Exports/Models/BranchModel.cs index 98cebd178d..cff5f0c054 100644 --- a/src/GitHub.Exports/Models/BranchModel.cs +++ b/src/GitHub.Exports/Models/BranchModel.cs @@ -1,7 +1,6 @@ using System; using System.Globalization; using GitHub.Primitives; -using GitHub.Services; namespace GitHub.Models { @@ -27,35 +26,16 @@ public BranchModel(Octokit.Branch branch, IRepositoryModel repo) Id = String.Format(CultureInfo.InvariantCulture, "{0}/{1}", Repository.Owner, Name); } - public BranchModel(LibGit2Sharp.Branch branch, IRepositoryModel repo, IGitService gitService) - { - Extensions.Guard.ArgumentNotNull(branch, nameof(branch)); - Extensions.Guard.ArgumentNotNull(repo, nameof(repo)); - Name = DisplayName = branch.FriendlyName; - Repository = repo; - Sha = branch.Tip?.Sha; - Id = String.Format(CultureInfo.InvariantCulture, "{0}/{1}", Repository.Owner, Name); - - if(IsTracking = branch.IsTracking) - { - var trackedBranch = branch.TrackedBranch; - TrackedSha = trackedBranch.Tip?.Sha; -#pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. - TrackedRemoteUrl = new UriString(trackedBranch.Remote.Url); -#pragma warning restore 0618 - TrackedRemoteCanonicalName = trackedBranch.UpstreamBranchCanonicalName; - } - } - public string Id { get; private set; } public string Name { get; private set; } public IRepositoryModel Repository { get; private set; } public string DisplayName { get; set; } - public string Sha { get; private set; } - public bool IsTracking { get; private set; } - public string TrackedSha { get; private set; } - public UriString TrackedRemoteUrl { get; private set; } - public string TrackedRemoteCanonicalName { get; private set; } + + public string Sha { get; protected set; } + public bool IsTracking { get; protected set; } + public string TrackedSha { get; protected set; } + public UriString TrackedRemoteUrl { get; protected set; } + public string TrackedRemoteCanonicalName { get; protected set; } #region Equality things public void CopyFrom(IBranch other) diff --git a/src/GitHub.Exports/Models/LocalBranchModel.cs b/src/GitHub.Exports/Models/LocalBranchModel.cs new file mode 100644 index 0000000000..2a054df467 --- /dev/null +++ b/src/GitHub.Exports/Models/LocalBranchModel.cs @@ -0,0 +1,25 @@ +using GitHub.Services; +using GitHub.Primitives; +using LibGit2Sharp; + +namespace GitHub.Models +{ + public class LocalBranchModel : BranchModel, IBranch + { + public LocalBranchModel(Branch branch, IRepositoryModel repo, IGitService gitService) + : base(branch?.FriendlyName, repo) + { + Sha = branch.Tip?.Sha; + + if(IsTracking = branch.IsTracking) + { + var trackedBranch = branch.TrackedBranch; + TrackedSha = trackedBranch.Tip?.Sha; +#pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. + TrackedRemoteUrl = new UriString(trackedBranch.Remote.Url); +#pragma warning restore 0618 + TrackedRemoteCanonicalName = trackedBranch.UpstreamBranchCanonicalName; + } + } + } +} diff --git a/src/GitHub.Exports/Models/LocalRepositoryModel.cs b/src/GitHub.Exports/Models/LocalRepositoryModel.cs index fe07ef5ab8..379274c26c 100644 --- a/src/GitHub.Exports/Models/LocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/LocalRepositoryModel.cs @@ -183,7 +183,7 @@ public IBranch CurrentBranch // BranchModel doesn't keep a reference to Repository using (var repo = gitService.GetRepository(LocalPath)) { - return new BranchModel(repo?.Head, this, gitService); + return new LocalBranchModel(repo?.Head, this, gitService); } } } From bac12273840077121d7231fa94c98ceb51554f22 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 8 Jun 2018 14:40:34 +0100 Subject: [PATCH 5/9] Stop using obsolete .Network.Remotes property --- src/GitHub.Exports/Models/LocalBranchModel.cs | 9 ++++----- src/GitHub.Exports/Models/LocalRepositoryModel.cs | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/GitHub.Exports/Models/LocalBranchModel.cs b/src/GitHub.Exports/Models/LocalBranchModel.cs index 2a054df467..ebd3d66514 100644 --- a/src/GitHub.Exports/Models/LocalBranchModel.cs +++ b/src/GitHub.Exports/Models/LocalBranchModel.cs @@ -6,18 +6,17 @@ namespace GitHub.Models { public class LocalBranchModel : BranchModel, IBranch { - public LocalBranchModel(Branch branch, IRepositoryModel repo, IGitService gitService) + public LocalBranchModel(IRepository repository, Branch branch, IRepositoryModel repo, IGitService gitService) : base(branch?.FriendlyName, repo) { Sha = branch.Tip?.Sha; - if(IsTracking = branch.IsTracking) + if (IsTracking = branch.IsTracking) { var trackedBranch = branch.TrackedBranch; TrackedSha = trackedBranch.Tip?.Sha; -#pragma warning disable 0618 // TODO: Replace `Branch.Remote` with `Repository.Network.Remotes[branch.RemoteName]`. - TrackedRemoteUrl = new UriString(trackedBranch.Remote.Url); -#pragma warning restore 0618 + var trackedRemote = repository.Network.Remotes[trackedBranch.RemoteName]; + TrackedRemoteUrl = trackedRemote != null ? new UriString(trackedRemote.Url) : null; TrackedRemoteCanonicalName = trackedBranch.UpstreamBranchCanonicalName; } } diff --git a/src/GitHub.Exports/Models/LocalRepositoryModel.cs b/src/GitHub.Exports/Models/LocalRepositoryModel.cs index 379274c26c..52491a101c 100644 --- a/src/GitHub.Exports/Models/LocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/LocalRepositoryModel.cs @@ -183,7 +183,7 @@ public IBranch CurrentBranch // BranchModel doesn't keep a reference to Repository using (var repo = gitService.GetRepository(LocalPath)) { - return new LocalBranchModel(repo?.Head, this, gitService); + return new LocalBranchModel(repo, repo?.Head, this, gitService); } } } From b450991ade1cac65e29cb5a88a15518fb53960e5 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 8 Jun 2018 15:49:53 +0100 Subject: [PATCH 6/9] Extract ILocalBranch from IBranch ILocalBranch includes extra information that is available when a branch is from a local repository. --- .../SampleData/LocalRepositoryModelDesigner.cs | 2 +- src/GitHub.Exports/GitHub.Exports.csproj | 1 + src/GitHub.Exports/Models/BranchModel.cs | 7 ------- src/GitHub.Exports/Models/IBranch.cs | 6 ------ src/GitHub.Exports/Models/ILocalBranch.cs | 13 +++++++++++++ src/GitHub.Exports/Models/ILocalRepositoryModel.cs | 2 +- src/GitHub.Exports/Models/LocalBranchModel.cs | 8 +++++++- src/GitHub.Exports/Models/LocalRepositoryModel.cs | 2 +- .../GitHub.App/Services/TeamExplorerContextTests.cs | 2 +- .../Dialog/PullRequestCreationViewModelTests.cs | 12 +++++++++++- .../GitHubPane/PullRequestDetailViewModelTests.cs | 12 +++++++++++- 11 files changed, 47 insertions(+), 20 deletions(-) create mode 100644 src/GitHub.Exports/Models/ILocalBranch.cs diff --git a/src/GitHub.App/SampleData/LocalRepositoryModelDesigner.cs b/src/GitHub.App/SampleData/LocalRepositoryModelDesigner.cs index dbcac88908..de78b9fdcd 100644 --- a/src/GitHub.App/SampleData/LocalRepositoryModelDesigner.cs +++ b/src/GitHub.App/SampleData/LocalRepositoryModelDesigner.cs @@ -11,7 +11,7 @@ namespace GitHub.App.SampleData public class LocalRepositoryModelDesigner : ILocalRepositoryModel { public UriString CloneUrl { get; set; } - public IBranch CurrentBranch { get; set; } + public ILocalBranch CurrentBranch { get; set; } public Octicon Icon { get; set; } public string LocalPath { get; set; } public string Name { get; set; } diff --git a/src/GitHub.Exports/GitHub.Exports.csproj b/src/GitHub.Exports/GitHub.Exports.csproj index b53828bac7..a862cf174f 100644 --- a/src/GitHub.Exports/GitHub.Exports.csproj +++ b/src/GitHub.Exports/GitHub.Exports.csproj @@ -164,6 +164,7 @@ + diff --git a/src/GitHub.Exports/Models/BranchModel.cs b/src/GitHub.Exports/Models/BranchModel.cs index cff5f0c054..14a61ad4fa 100644 --- a/src/GitHub.Exports/Models/BranchModel.cs +++ b/src/GitHub.Exports/Models/BranchModel.cs @@ -31,12 +31,6 @@ public BranchModel(Octokit.Branch branch, IRepositoryModel repo) public IRepositoryModel Repository { get; private set; } public string DisplayName { get; set; } - public string Sha { get; protected set; } - public bool IsTracking { get; protected set; } - public string TrackedSha { get; protected set; } - public UriString TrackedRemoteUrl { get; protected set; } - public string TrackedRemoteCanonicalName { get; protected set; } - #region Equality things public void CopyFrom(IBranch other) { @@ -46,7 +40,6 @@ public void CopyFrom(IBranch other) Name = other.Name; Repository = other.Repository; DisplayName = other.DisplayName; - IsTracking = other.IsTracking; } public override bool Equals(object obj) diff --git a/src/GitHub.Exports/Models/IBranch.cs b/src/GitHub.Exports/Models/IBranch.cs index 625be66d12..4589d288e0 100644 --- a/src/GitHub.Exports/Models/IBranch.cs +++ b/src/GitHub.Exports/Models/IBranch.cs @@ -1,5 +1,4 @@ using System; -using GitHub.Primitives; using GitHub.Collections; namespace GitHub.Models @@ -11,10 +10,5 @@ public interface IBranch : ICopyable, string Name { get; } IRepositoryModel Repository { get; } string DisplayName { get; set; } - string Sha { get; } - bool IsTracking { get; } - string TrackedSha { get; } - UriString TrackedRemoteUrl { get; } - string TrackedRemoteCanonicalName { get; } } } diff --git a/src/GitHub.Exports/Models/ILocalBranch.cs b/src/GitHub.Exports/Models/ILocalBranch.cs new file mode 100644 index 0000000000..1b41da67f9 --- /dev/null +++ b/src/GitHub.Exports/Models/ILocalBranch.cs @@ -0,0 +1,13 @@ +using GitHub.Primitives; + +namespace GitHub.Models +{ + public interface ILocalBranch : IBranch + { + string Sha { get; } + bool IsTracking { get; } + string TrackedSha { get; } + UriString TrackedRemoteUrl { get; } + string TrackedRemoteCanonicalName { get; } + } +} diff --git a/src/GitHub.Exports/Models/ILocalRepositoryModel.cs b/src/GitHub.Exports/Models/ILocalRepositoryModel.cs index 0dc69f52ed..2207082f5f 100644 --- a/src/GitHub.Exports/Models/ILocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/ILocalRepositoryModel.cs @@ -18,7 +18,7 @@ public interface ILocalRepositoryModel : IRepositoryModel, INotifyPropertyChange /// /// Gets the current branch. /// - IBranch CurrentBranch { get; } + ILocalBranch CurrentBranch { get; } /// /// Updates the url information based on the local path diff --git a/src/GitHub.Exports/Models/LocalBranchModel.cs b/src/GitHub.Exports/Models/LocalBranchModel.cs index ebd3d66514..7f64d4788e 100644 --- a/src/GitHub.Exports/Models/LocalBranchModel.cs +++ b/src/GitHub.Exports/Models/LocalBranchModel.cs @@ -4,7 +4,7 @@ namespace GitHub.Models { - public class LocalBranchModel : BranchModel, IBranch + public class LocalBranchModel : BranchModel, ILocalBranch { public LocalBranchModel(IRepository repository, Branch branch, IRepositoryModel repo, IGitService gitService) : base(branch?.FriendlyName, repo) @@ -20,5 +20,11 @@ public LocalBranchModel(IRepository repository, Branch branch, IRepositoryModel TrackedRemoteCanonicalName = trackedBranch.UpstreamBranchCanonicalName; } } + + public string Sha { get; } + public bool IsTracking { get; } + public string TrackedSha { get; } + public UriString TrackedRemoteUrl { get; } + public string TrackedRemoteCanonicalName { get; } } } diff --git a/src/GitHub.Exports/Models/LocalRepositoryModel.cs b/src/GitHub.Exports/Models/LocalRepositoryModel.cs index 52491a101c..7d9b436e50 100644 --- a/src/GitHub.Exports/Models/LocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/LocalRepositoryModel.cs @@ -176,7 +176,7 @@ public string HeadSha /// /// Gets the current branch of the repository. /// - public IBranch CurrentBranch + public ILocalBranch CurrentBranch { get { diff --git a/test/UnitTests/GitHub.App/Services/TeamExplorerContextTests.cs b/test/UnitTests/GitHub.App/Services/TeamExplorerContextTests.cs index e055bde196..7140408169 100644 --- a/test/UnitTests/GitHub.App/Services/TeamExplorerContextTests.cs +++ b/test/UnitTests/GitHub.App/Services/TeamExplorerContextTests.cs @@ -227,7 +227,7 @@ static ILocalRepositoryModel CreateRepositoryModel(string path, string branchNam { var repo = Substitute.For(); repo.LocalPath.Returns(path); - var currentBranch = Substitute.For(); + var currentBranch = Substitute.For(); currentBranch.Name.Returns(branchName); currentBranch.Sha.Returns(headSha); currentBranch.TrackedSha.Returns(trackedSha); diff --git a/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs b/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs index 497515b37a..a12e28e30d 100644 --- a/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs +++ b/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs @@ -91,7 +91,7 @@ static TestData PrepareTestData( if (repoIsFork) githubRepoParent = CreateRepository(targetRepoOwner, repoName, id: 1); var githubRepo = CreateRepository(sourceRepoOwner, repoName, id: 2, parent: githubRepoParent); - var sourceBranch = new BranchModel(sourceBranchName, activeRepo); + var sourceBranch = CreateLocalBranch(sourceBranchName, activeRepo); var sourceRepo = new RemoteRepositoryModel(githubRepo); var targetRepo = targetRepoOwner == sourceRepoOwner ? sourceRepo : sourceRepo.Parent; var targetBranch = targetBranchName != targetRepo.DefaultBranch.Name ? new BranchModel(targetBranchName, targetRepo) : targetRepo.DefaultBranch; @@ -209,4 +209,14 @@ public void TemplateIsUsedIfPresent() Assert.That("Test PR template", Is.EqualTo(vm.Description)); } + + static ILocalBranch CreateLocalBranch(string sourceBranchName, ILocalRepositoryModel activeRepo) + { + var branch = Substitute.For(); + branch.DisplayName.Returns(sourceBranchName); + branch.Name.Returns(sourceBranchName); + branch.Id.Returns(activeRepo.Name + "/" + sourceBranchName); + branch.Repository.Returns(activeRepo); + return branch; + } } diff --git a/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs b/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs index c459f733c2..c0417fb4bd 100644 --- a/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs +++ b/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs @@ -526,7 +526,7 @@ static Tuple CreateTargetAndSer IPullRequestSessionManager sessionManager = null) { var repository = Substitute.For(); - var currentBranchModel = new BranchModel(currentBranch, repository); + var currentBranchModel = CreateLocalBranch(currentBranch, repository); repository.CurrentBranch.Returns(currentBranchModel); repository.CloneUrl.Returns(new UriString(Uri.ToString())); repository.LocalPath.Returns(@"C:\projects\ThisRepo"); @@ -613,6 +613,16 @@ static PullRequestDetailModel CreatePullRequestModel( }; } + static ILocalBranch CreateLocalBranch(string sourceBranchName, ILocalRepositoryModel activeRepo) + { + var branch = Substitute.For(); + branch.DisplayName.Returns(sourceBranchName); + branch.Name.Returns(sourceBranchName); + branch.Id.Returns(activeRepo.Name + "/" + sourceBranchName); + branch.Repository.Returns(activeRepo); + return branch; + } + static IObservable Throws(string message) { Func, Action> f = _ => { throw new FileNotFoundException(message); }; From 712b3dc8a94916a47b7f7cecb1475cc82313a393 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 8 Jun 2018 16:04:03 +0100 Subject: [PATCH 7/9] LocalBranchModel no longer depends on IGitService --- src/GitHub.Exports/Models/LocalBranchModel.cs | 2 +- src/GitHub.Exports/Models/LocalRepositoryModel.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/GitHub.Exports/Models/LocalBranchModel.cs b/src/GitHub.Exports/Models/LocalBranchModel.cs index 7f64d4788e..2cf1bc7c96 100644 --- a/src/GitHub.Exports/Models/LocalBranchModel.cs +++ b/src/GitHub.Exports/Models/LocalBranchModel.cs @@ -6,7 +6,7 @@ namespace GitHub.Models { public class LocalBranchModel : BranchModel, ILocalBranch { - public LocalBranchModel(IRepository repository, Branch branch, IRepositoryModel repo, IGitService gitService) + public LocalBranchModel(IRepository repository, Branch branch, IRepositoryModel repo) : base(branch?.FriendlyName, repo) { Sha = branch.Tip?.Sha; diff --git a/src/GitHub.Exports/Models/LocalRepositoryModel.cs b/src/GitHub.Exports/Models/LocalRepositoryModel.cs index 7d9b436e50..bef56952c4 100644 --- a/src/GitHub.Exports/Models/LocalRepositoryModel.cs +++ b/src/GitHub.Exports/Models/LocalRepositoryModel.cs @@ -183,7 +183,7 @@ public ILocalBranch CurrentBranch // BranchModel doesn't keep a reference to Repository using (var repo = gitService.GetRepository(LocalPath)) { - return new LocalBranchModel(repo, repo?.Head, this, gitService); + return new LocalBranchModel(repo, repo?.Head, this); } } } From ceea6fbff46fdeae33d28a6a04b336cf6241feca Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 11 Jun 2018 10:52:20 +0100 Subject: [PATCH 8/9] Fix failing tests that use ILocalBranch --- .../Services/PullRequestSessionManager.cs | 6 +++--- .../Dialog/PullRequestCreationViewModelTests.cs | 14 +++++++------- .../GitHubPane/PullRequestDetailViewModelTests.cs | 14 +++++++------- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs index 3e3b1eff99..9a68e2c7c4 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionManager.cs @@ -205,10 +205,10 @@ async Task StatusChanged() Tuple pr = null; - // HACK: Don't use tagged branch info when testing. - // pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync(); + // Use tagged branch info when available + pr = await service.GetPullRequestForCurrentBranch(repository).FirstOrDefaultAsync(); - if(pr == null) + if (pr == null) { try { diff --git a/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs b/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs index a12e28e30d..4b9523ccab 100644 --- a/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs +++ b/test/UnitTests/GitHub.App/ViewModels/Dialog/PullRequestCreationViewModelTests.cs @@ -210,13 +210,13 @@ public void TemplateIsUsedIfPresent() Assert.That("Test PR template", Is.EqualTo(vm.Description)); } - static ILocalBranch CreateLocalBranch(string sourceBranchName, ILocalRepositoryModel activeRepo) + static ILocalBranch CreateLocalBranch(string sourceBranchName, ILocalRepositoryModel parentRepo) { - var branch = Substitute.For(); - branch.DisplayName.Returns(sourceBranchName); - branch.Name.Returns(sourceBranchName); - branch.Id.Returns(activeRepo.Name + "/" + sourceBranchName); - branch.Repository.Returns(activeRepo); - return branch; + var repository = Substitute.For(); + var branch = Substitute.For(); + branch.FriendlyName.Returns(sourceBranchName); + branch.IsTracking.Returns(false); + var localBranch = new LocalBranchModel(repository, branch, parentRepo); + return localBranch; } } diff --git a/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs b/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs index c0417fb4bd..b767336c11 100644 --- a/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs +++ b/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs @@ -613,14 +613,14 @@ static PullRequestDetailModel CreatePullRequestModel( }; } - static ILocalBranch CreateLocalBranch(string sourceBranchName, ILocalRepositoryModel activeRepo) + static ILocalBranch CreateLocalBranch(string sourceBranchName, ILocalRepositoryModel parentRepo) { - var branch = Substitute.For(); - branch.DisplayName.Returns(sourceBranchName); - branch.Name.Returns(sourceBranchName); - branch.Id.Returns(activeRepo.Name + "/" + sourceBranchName); - branch.Repository.Returns(activeRepo); - return branch; + var repository = Substitute.For(); + var branch = Substitute.For(); + branch.FriendlyName.Returns(sourceBranchName); + branch.IsTracking.Returns(false); + var localBranch = new LocalBranchModel(repository, branch, parentRepo); + return localBranch; } static IObservable Throws(string message) From c0bf42c44c385072634c2c63833092d8f816a50b Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 11 Jun 2018 20:12:08 +0100 Subject: [PATCH 9/9] Find PRs that target parent or owner repo We're only interested in PRs that target the parent (upstream) or owner (current) repo. --- .../Services/PullRequestSessionService.cs | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs index 4deadb4ad8..8f3147659a 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestSessionService.cs @@ -273,21 +273,38 @@ public async Task> FindPullRequestForRef(UriString repository var graphql = await graphqlFactory.CreateConnection(address); var query = - from x in new Query() - .Repository(repositoryUrl.Owner, repositoryUrl.RepositoryName) - .Ref(refQualifiedName) - .AssociatedPullRequests(first: 100) - .Nodes - select - new PullRequestOwnerInfo - { - BaseRepositoryOwner = x.Repository.Owner.Login, - Number = x.Number - }; - + from r in new Query().Repository(repositoryUrl.Owner, repositoryUrl.RepositoryName) + select new + { + Owner = r.Owner.Login, + Parent = r.Parent != null ? r.Parent.Owner.Login : null, + PullRequests = + (from x in r.Ref(refQualifiedName) + .AssociatedPullRequests(100, null, null, null, null, null, null, null, null).Nodes + select new PullRequestOwnerInfo + { + BaseRepositoryOwner = x.Repository.Owner.Login, + Number = x.Number + }).ToList() + }; + var results = await graphql.Run(query); - var match = results.FirstOrDefault(); - return match != null ? new Tuple(match.BaseRepositoryOwner, match.Number) : null; + + // Find pull request to the parent repo if any + var pullRequestToParent = results.PullRequests.Where(x => results.Parent != null && x.BaseRepositoryOwner == results.Parent).FirstOrDefault(); + if(pullRequestToParent != null) + { + return new Tuple(pullRequestToParent.BaseRepositoryOwner, pullRequestToParent.Number); + } + + // Find pull request to the owner repo if any + var pullRequestToOwner = results.PullRequests.Where(x => x.BaseRepositoryOwner == results.Owner).FirstOrDefault(); + if (pullRequestToOwner != null) + { + return new Tuple(pullRequestToOwner.BaseRepositoryOwner, pullRequestToOwner.Number); + } + + return null; } public class PullRequestOwnerInfo