Merge pull request #1021 from github/fixes/863-fork-pull-requests

Allow selection of fork/parent pull requests.

Author: grokys

GitHubVS.sln

@@ -231,6 +231,7 @@ Global
 		{E4ED0537-D1D9-44B6-9212-3096D7C3F7A1}.Release|x86.ActiveCfg = Release|Any CPU
 		{E4ED0537-D1D9-44B6-9212-3096D7C3F7A1}.Release|x86.Build.0 = Release|Any CPU
 		{B1F5C227-456F-437D-BD5F-4C11B7A8D1A0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{B1F5C227-456F-437D-BD5F-4C11B7A8D1A0}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{B1F5C227-456F-437D-BD5F-4C11B7A8D1A0}.Debug|x86.ActiveCfg = Debug|Any CPU
 		{B1F5C227-456F-437D-BD5F-4C11B7A8D1A0}.Debug|x86.Build.0 = Debug|Any CPU
 		{B1F5C227-456F-437D-BD5F-4C11B7A8D1A0}.Publish|Any CPU.ActiveCfg = Release|Any CPU

src/GitHub.App/GitHub.App.csproj

@@ -134,6 +134,7 @@
   <ItemGroup>
     <Compile Include="Models\IssueCommentModel.cs" />
     <Compile Include="Models\PullRequestReviewCommentModel.cs" />
+    <Compile Include="Models\PullRequestDetailArgument.cs" />
     <Compile Include="ViewModels\ViewModelBase.cs" />
     <None Include="..\..\script\Key.snk" Condition="$(Buildtype) == 'Internal'">
       <Link>Key.snk</Link>

src/GitHub.App/Models/PullRequestDetailArgument.cs

@@ -0,0 +1,21 @@
+using System;
+using GitHub.ViewModels;
+
+namespace GitHub.Models
+{
+    /// <summary>
+    /// Passes arguments to a <see cref="PullRequestDetailViewModel"/>
+    /// </summary>
+    public class PullRequestDetailArgument
+    {
+        /// <summary>
+        /// Gets or sets the repository containing the pull request.
+        /// </summary>
+        public IRemoteRepositoryModel Repository { get; set; }
+
+        /// <summary>
+        /// Gets or sets the number of the pull request.
+        /// </summary>
+        public int Number { get; set; }
+    }
+}

src/GitHub.App/Models/RemoteRepositoryModel.cs

@@ -20,7 +20,8 @@ public class RemoteRepositoryModel : RepositoryModel, IRemoteRepositoryModel,
         /// <param name="isPrivate">Whether the repository is private.</param>
         /// <param name="isFork">Whether the repository is a fork.</param>
         /// <param name="ownerAccount">The repository owner account.</param>
-        public RemoteRepositoryModel(long id, string name, UriString cloneUrl, bool isPrivate, bool isFork, IAccount ownerAccount)
+        /// <param name="parent">The parent repository if this repository is a fork.</param>
+        public RemoteRepositoryModel(long id, string name, UriString cloneUrl, bool isPrivate, bool isFork,  IAccount ownerAccount, IRemoteRepositoryModel parent)
             : base(name, cloneUrl)
         {
             Guard.ArgumentNotEmptyString(name, nameof(name));
@@ -33,6 +34,7 @@ public RemoteRepositoryModel(long id, string name, UriString cloneUrl, bool isPr
             // this is an assumption, we'd have to load the repo information from octokit to know for sure
             // probably not worth it for this ctor
             DefaultBranch = new BranchModel("master", this);
+            Parent = parent;
         }
 
         /// <summary>

src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs

@@ -73,7 +73,8 @@ public PullRequestDetailViewModelDesigner()
 
         public IPullRequestModel Model { get; }
         public IPullRequestSession Session { get; }
-        public ILocalRepositoryModel Repository { get; }
+        public ILocalRepositoryModel LocalRepository { get; }
+        public IRemoteRepositoryModel RemoteRepository { get; }
         public string SourceBranchDisplayName { get; set; }
         public string TargetBranchDisplayName { get; set; }
         public int CommentCount { get; set; }

src/GitHub.App/SampleData/PullRequestListViewModelDesigner.cs

@@ -55,6 +55,9 @@ public PullRequestListViewModelDesigner()
             SelectedAuthor = Authors.ElementAt(1);
         }
 
+        public IReadOnlyList<IRemoteRepositoryModel> Repositories { get; }
+        public IRemoteRepositoryModel SelectedRepository { get; set; }
+
         public ITrackingCollection<IPullRequestModel> PullRequests { get; set; }
         public IPullRequestModel SelectedPullRequest { get; set; }
 
@@ -63,6 +66,8 @@ public PullRequestListViewModelDesigner()
 
         public ObservableCollection<IAccount> Authors { get; set; }
         public IAccount SelectedAuthor { get; set; }
+        public bool RepositoryIsFork { get; set; } = true;
+        public bool ShowPullRequestsForFork { get; set; }
 
         public ObservableCollection<IAccount> Assignees { get; set; }
         public IAccount SelectedAssignee { get; set; }

src/GitHub.App/SampleData/SampleViewModels.cs

@@ -321,7 +321,7 @@ public static IRemoteRepositoryModel Create(string name = null, string owner = n
         {
             name = name ?? "octocat";
             owner = owner ?? "github";
-            return new RemoteRepositoryModel(0, name, new UriString("http://github.com/" + name + "/" + owner), false, false, new AccountDesigner() { Login = owner });
+            return new RemoteRepositoryModel(0, name, new UriString("http://github.com/" + name + "/" + owner), false, false, new AccountDesigner() { Login = owner }, null);
         }
     }
 

src/GitHub.App/Services/GitClient.cs

@@ -15,6 +15,8 @@ namespace GitHub.Services
     [PartCreationPolicy(CreationPolicy.Shared)]
     public class GitClient : IGitClient
     {
+        const string defaultOriginName = "origin";
+
         static readonly Logger log = LogManager.GetCurrentClassLogger();
         readonly PullOptions pullOptions;
         readonly PushOptions pushOptions;
@@ -370,37 +372,43 @@ public Task<bool> IsModified(IRepository repository, string path, byte[] content
             });
         }
 
-        public async Task<string> GetPullRequestMergeBase(IRepository repo, string remoteName, string baseSha, string headSha, string baseRef, int pullNumber)
+        public async Task<string> GetPullRequestMergeBase(IRepository repo,
+            UriString baseCloneUrl, UriString headCloneUrl, string baseSha, string headSha, string baseRef, string headRef)
         {
             Guard.ArgumentNotNull(repo, nameof(repo));
-            Guard.ArgumentNotEmptyString(remoteName, nameof(remoteName));
+            Guard.ArgumentNotNull(baseCloneUrl, nameof(baseCloneUrl));
+            Guard.ArgumentNotNull(headCloneUrl, nameof(headCloneUrl));
             Guard.ArgumentNotEmptyString(baseRef, nameof(baseRef));
 
-            var mergeBase = GetMergeBase(repo, baseSha, headSha);
-            if (mergeBase == null)
+            var baseCommit = repo.Lookup<Commit>(baseSha);
+            if (baseCommit == null)
             {
-                var pullHeadRef = $"refs/pull/{pullNumber}/head";
-                await Fetch(repo, remoteName, baseRef, pullHeadRef);
-
-                mergeBase = GetMergeBase(repo, baseSha, headSha);
+                await Fetch(repo, baseCloneUrl, baseRef);
+                baseCommit = repo.Lookup<Commit>(baseSha);
+                if (baseCommit == null)
+                {
+                    return null;
+                }
             }
 
-            return mergeBase;
-        }
-
-        static string GetMergeBase(IRepository repo, string a, string b)
-        {
-            Guard.ArgumentNotNull(repo, nameof(repo));
+            var headCommit = repo.Lookup<Commit>(headSha);
+            if (headCommit == null)
+            {
+                await Fetch(repo, headCloneUrl, headRef);
+                headCommit = repo.Lookup<Commit>(headSha);
+                if (headCommit == null)
+                {
+                    return null;
+                }
+            }
 
-            var aCommit = repo.Lookup<Commit>(a);
-            var bCommit = repo.Lookup<Commit>(b);
-            if (aCommit == null || bCommit == null)
+            var mergeBaseCommit = repo.ObjectDatabase.FindMergeBase(baseCommit, headCommit);
+            if(mergeBaseCommit == null)
             {
                 return null;
             }
 
-            var baseCommit = repo.ObjectDatabase.FindMergeBase(aCommit, bCommit);
-            return baseCommit?.Sha;
+            return mergeBaseCommit.Sha;
         }
 
         public Task<bool> IsHeadPushed(IRepository repo)
@@ -422,6 +430,26 @@ public Task<bool> IsHeadPushed(IRepository repo)
             });
         }
 
+        public Task Fetch(IRepository repo, UriString cloneUrl, params string[] refspecs)
+        {
+            var httpsUrl = UriString.ToUriString(cloneUrl.ToRepositoryUrl());
+            if (repo.Network.Remotes[defaultOriginName]?.Url == httpsUrl)
+            {
+                return Fetch(repo, defaultOriginName, refspecs);
+            }
+
+            var tempRemoteName = cloneUrl.Owner + "-" + Guid.NewGuid();
+            repo.Network.Remotes.Add(tempRemoteName, httpsUrl);
+            try
+            {
+                return Fetch(repo, tempRemoteName, refspecs);
+            }
+            finally
+            {
+                repo.Network.Remotes.Remove(tempRemoteName);
+            }
+        }
+
         static bool IsCanonical(string s)
         {
             Guard.ArgumentNotEmptyString(s, nameof(s));

src/GitHub.App/Services/ModelService.cs

@@ -153,7 +153,7 @@ IObservable<AccountCacheItem> GetUserFromCache()
         /// <param name="repo"></param>
         /// <param name="collection"></param>
         /// <returns></returns>
-        public ITrackingCollection<IPullRequestModel> GetPullRequests(ILocalRepositoryModel repo,
+        public ITrackingCollection<IPullRequestModel> GetPullRequests(IRepositoryModel repo,
             ITrackingCollection<IPullRequestModel> collection)
         {
             // Since the api to list pull requests returns all the data for each pr, cache each pr in its own entry
@@ -163,7 +163,7 @@ public ITrackingCollection<IPullRequestModel> GetPullRequests(ILocalRepositoryMo
             // and replaces it instead of appending, so items get refreshed in-place as they come in.
 
             var keyobs = GetUserFromCache()
-                .Select(user => string.Format(CultureInfo.InvariantCulture, "{0}|{1}:{2}", CacheIndex.PRPrefix, user.Login, repo.Name));
+                .Select(user => string.Format(CultureInfo.InvariantCulture, "{0}|{1}:{2}", CacheIndex.PRPrefix, repo.Owner, repo.Name));
 
             var source = Observable.Defer(() => keyobs
                 .SelectMany(key =>
@@ -188,16 +188,16 @@ public ITrackingCollection<IPullRequestModel> GetPullRequests(ILocalRepositoryMo
             return collection;
         }
 
-        public IObservable<IPullRequestModel> GetPullRequest(ILocalRepositoryModel repo, int number)
+        public IObservable<IPullRequestModel> GetPullRequest(string owner, string name, int number)
         {
             return Observable.Defer(() =>
             {
                 return hostCache.GetAndRefreshObject(PRPrefix + '|' + number, () =>
                         Observable.CombineLatest(
-                            apiClient.GetPullRequest(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName, number),
-                            apiClient.GetPullRequestFiles(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName, number).ToList(),
-                            apiClient.GetIssueComments(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName, number).ToList(),
-                            apiClient.GetPullRequestReviewComments(repo.CloneUrl.Owner, repo.CloneUrl.RepositoryName, number).ToList(),
+                            apiClient.GetPullRequest(owner, name, number),
+                            apiClient.GetPullRequestFiles(owner, name, number).ToList(),
+                            apiClient.GetIssueComments(owner, name, number).ToList(),
+                            apiClient.GetPullRequestReviewComments(owner, name, number).ToList(),
                             (pr, files, comments, reviewComments) => new
                             {
                                 PullRequest = pr,
@@ -216,6 +216,19 @@ public IObservable<IPullRequestModel> GetPullRequest(ILocalRepositoryModel repo,
             });
         }
 
+        public IObservable<IRemoteRepositoryModel> GetRepository(string owner, string repo)
+        {
+            var keyobs = GetUserFromCache()
+                .Select(user => string.Format(CultureInfo.InvariantCulture, "{0}|{1}", CacheIndex.RepoPrefix, user.Login));
+
+            return Observable.Defer(() => keyobs
+                .SelectMany(key =>
+                    hostCache.GetAndFetchLatest(
+                        key,
+                        () => apiClient.GetRepository(owner, repo).Select(RepositoryCacheItem.Create))
+                    .Select(Create)));
+        }
+
         public ITrackingCollection<IRemoteRepositoryModel> GetRepositories(ITrackingCollection<IRemoteRepositoryModel> collection)
         {
             var keyobs = GetUserFromCache()
@@ -390,7 +403,8 @@ IRemoteRepositoryModel Create(RepositoryCacheItem item)
                 new UriString(item.CloneUrl),
                 item.Private,
                 item.Fork,
-                Create(item.Owner))
+                Create(item.Owner),
+                item.Parent != null ? Create(item.Parent) : null)
             {
                 CreatedAt = item.CreatedAt,
                 UpdatedAt = item.UpdatedAt
@@ -504,6 +518,7 @@ public RepositoryCacheItem(Repository apiRepository)
                 CreatedAt = apiRepository.CreatedAt;
                 UpdatedAt = apiRepository.UpdatedAt;
                 Timestamp = apiRepository.UpdatedAt;
+                Parent = apiRepository.Parent != null ? new RepositoryCacheItem(apiRepository.Parent) : null;
             }
 
             public long Id { get; set; }
@@ -515,6 +530,7 @@ public RepositoryCacheItem(Repository apiRepository)
             public bool Fork { get; set; }
             public DateTimeOffset CreatedAt { get; set; }
             public DateTimeOffset UpdatedAt { get; set; }
+            public RepositoryCacheItem Parent { get; set; }
         }
 
         public class PullRequestCacheItem : CacheItem