Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Pull Request conversation view #2017

Merged
merged 73 commits into from
Feb 21, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
38cae06
Show document pane for PR conversation.
grokys Oct 26, 2018
50b04ac
Display PR details in document pane.
grokys Oct 26, 2018
cfb1dd3
Initial implementation of PR conversation view.
grokys Oct 26, 2018
02bc4ee
Display sample data in PR conversation page.
grokys Oct 30, 2018
102e337
Load PR timeline from GraphQL.
grokys Oct 30, 2018
424439d
Show commit in TE when OID clicked.
grokys Oct 31, 2018
f976412
Display PR title as pane caption.
grokys Nov 1, 2018
abeb1b8
Display an "Add comment" box at the bottom of a PR.
grokys Nov 1, 2018
b724525
Change close issue caption when comment has body.
grokys Nov 1, 2018
d296739
Implement posting PR comments.
grokys Nov 5, 2018
0b91c87
Disable comment view while submitting.
grokys Nov 5, 2018
1200c4c
Jiggled the comment thread hierachy a little.
grokys Nov 5, 2018
057c4e4
Merge branch 'master' into feature/pr-conversation
grokys Nov 5, 2018
fe66df5
Fix up comment view models after merge.
grokys Nov 5, 2018
a8d4da6
Correctly add new comments.
grokys Nov 5, 2018
d246ad2
Make PR conversation tool windows multi-instance.
grokys Nov 5, 2018
6165f30
Fix UI for closed PRs.
grokys Nov 5, 2018
996aac3
Display commit count.
grokys Nov 5, 2018
6da33e6
Implement closing/reopening a PR.
grokys Nov 8, 2018
c3f9778
Merge branch 'master' into feature/pr-conversation
grokys Nov 9, 2018
1308c0e
Implemented edit/delete of comments.
grokys Nov 14, 2018
6e88e35
Append "commented" after author name
donokuda Nov 16, 2018
8697cb7
Tweak header spacing
donokuda Nov 16, 2018
624b5be
Make it boxy
donokuda Nov 16, 2018
d7d5969
Space it out a bit
donokuda Nov 16, 2018
d12b441
More indenting and layout madness
donokuda Nov 16, 2018
a03c85f
Lighten up the commit octicon
donokuda Nov 16, 2018
aeb7c45
Indent comment body
donokuda Nov 16, 2018
b508e6a
Nudge things around
donokuda Nov 19, 2018
39d0b5f
Standardizing all the spacing
donokuda Nov 19, 2018
b317753
Adjust top margin
donokuda Nov 19, 2018
7ebcfb9
Adjust vertical margins for reply prompt box
donokuda Nov 19, 2018
815ee14
nudge left margin
donokuda Nov 19, 2018
7535c64
Make things better for dark theme
donokuda Nov 20, 2018
1d57ce8
Update separator colors
donokuda Nov 20, 2018
8340942
Merge pull request #2070 from github/donokuda/polish-conversation-view
grokys Nov 22, 2018
7b6a032
Finish off display of commit list w/caption.
grokys Nov 22, 2018
5308404
Theming
donokuda Dec 11, 2018
01676fe
Merge branch 'master' into feature/pr-conversation
grokys Jan 17, 2019
ed0b3a4
Start styling the top comment
donokuda Jan 25, 2019
448ccac
Octicon in a circle yaaaaaa
donokuda Jan 25, 2019
3ca7340
Tweak layout for commit list
donokuda Jan 25, 2019
2ae522c
Decrease the size of the avatar
donokuda Jan 25, 2019
4edf726
Adjust padding
donokuda Jan 26, 2019
6ac2d95
lol at least i tried
donokuda Jan 26, 2019
8f327c2
Octicon timeline theming
donokuda Feb 5, 2019
dc8a865
Let's not be too bold, now
donokuda Feb 5, 2019
5be7e2f
Merge branch 'master' into feature/pr-conversation
grokys Feb 5, 2019
72fb28b
Merge branch 'feature/pr-conversation' into donokuda/pr-polish
grokys Feb 5, 2019
e307b36
Fake it
donokuda Feb 5, 2019
d3caa17
Remove the description section
donokuda Feb 5, 2019
abb4661
Refactored comment views.
grokys Feb 6, 2019
a28d75a
Move around conversation button
donokuda Feb 6, 2019
b1b49a3
Record # of PR conversation views opened.
grokys Feb 7, 2019
e10d6fb
Always leave a note
donokuda Feb 7, 2019
84dd7ff
Hook up "View conversation" in PR details pane.
grokys Feb 8, 2019
08bfdc9
Merge branch 'donokuda/pr-pane' into feature/pr-conversation
grokys Feb 8, 2019
d8cf315
Limit to 100 repos per org (fix bad merge)
jcansdale Feb 12, 2019
d05a0ce
Stub out comment count and view on github links
donokuda Feb 13, 2019
8e42868
Add comment octicon and leave some notes around
donokuda Feb 13, 2019
4bdfa13
Add back in the description section
donokuda Feb 13, 2019
c8579c8
Bump spacing
donokuda Feb 13, 2019
5958982
Collapse sections
donokuda Feb 13, 2019
5702cb3
Hook up "View on GitHub" link
jcansdale Feb 19, 2019
e504484
Show number of comments on OpenConversation button
jcansdale Feb 19, 2019
ae6935a
Don't collapse Reviewers or Checks sections
jcansdale Feb 19, 2019
17f3f93
Set the pull request WebUrl and OpenOnGitHub
jcansdale Feb 19, 2019
3f43625
Allow pull request title to wrap
jcansdale Feb 19, 2019
abce76d
Merge pull request #2229 from github/donokuda/moving-some-cheese
jcansdale Feb 20, 2019
a8df4e6
Merge branch 'master' into feature/pr-conversation
jcansdale Feb 20, 2019
fd9eab5
Disabling automatic focus
StanleyGoldman Feb 20, 2019
f144895
Removing left over event handler
StanleyGoldman Feb 20, 2019
89f33ac
Shouldn't need the nuget package for octokit.graphql anymore
StanleyGoldman Feb 21, 2019
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/GitHub.App/Models/PullRequestModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ public string Title
}
}

PullRequestStateEnum status;
public PullRequestStateEnum State
PullRequestState status;
public PullRequestState State
{
get { return status; }
set
Expand All @@ -126,8 +126,8 @@ public PullRequestStateEnum State
}

// TODO: Remove these property once maintainer workflow has been merged to master.
public bool IsOpen => State == PullRequestStateEnum.Open;
public bool Merged => State == PullRequestStateEnum.Merged;
public bool IsOpen => State == PullRequestState.Open;
public bool Merged => State == PullRequestState.Merged;

int commentCount;
public int CommentCount
Expand Down
2 changes: 2 additions & 0 deletions src/GitHub.App/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.SampleData")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.SampleData.Dialog.Clone")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.SampleData.Documents")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.Dialog")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.Dialog.Clone")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.Documents")]
[assembly: XmlnsDefinition("https://github.com/github/VisualStudio", "GitHub.ViewModels.GitHubPane")]
11 changes: 10 additions & 1 deletion src/GitHub.App/SampleData/CommentViewModelDesigner.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
using System;
using System.Diagnostics.CodeAnalysis;
using System.Reactive;
using System.Threading.Tasks;
using GitHub.Models;
using GitHub.ViewModels;
using ReactiveUI;

Expand All @@ -22,7 +24,9 @@ public CommentViewModelDesigner()
public CommentEditState EditState { get; set; }
public bool IsReadOnly { get; set; }
public bool IsSubmitting { get; set; }
public bool CanCancel { get; } = true;
public bool CanDelete { get; } = true;
public string CommitCaption { get; set; } = "Comment";
public ICommentThreadViewModel Thread { get; }
public DateTimeOffset CreatedAt => DateTime.Now.Subtract(TimeSpan.FromDays(3));
public IActorViewModel Author { get; set; }
Expand All @@ -31,7 +35,12 @@ public CommentViewModelDesigner()
public ReactiveCommand<Unit, Unit> BeginEdit { get; }
public ReactiveCommand<Unit, Unit> CancelEdit { get; }
public ReactiveCommand<Unit, Unit> CommitEdit { get; }
public ReactiveCommand<Unit, Unit> OpenOnGitHub { get; }
public ReactiveCommand<Unit, Unit> OpenOnGitHub { get; } = ReactiveCommand.Create(() => { });
public ReactiveCommand<Unit, Unit> Delete { get; }

public Task InitializeAsync(ICommentThreadViewModel thread, ActorModel currentUser, CommentModel comment, CommentEditState state)
{
return Task.CompletedTask;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
using System;
using System.Threading.Tasks;
using GitHub.Models;
using GitHub.ViewModels;
using GitHub.ViewModels.Documents;
using ReactiveUI;

namespace GitHub.SampleData.Documents
{
public class IssueishCommentThreadViewModelDesigner : ViewModelBase, IIssueishCommentThreadViewModel
{
public IActorViewModel CurrentUser { get; } = new ActorViewModelDesigner("grokys");
public Task InitializeAsync(ActorModel currentUser, IssueishDetailModel model, bool addPlaceholder) => Task.CompletedTask;
public Task DeleteComment(ICommentViewModel comment) => Task.CompletedTask;
public Task EditComment(ICommentViewModel comment) => Task.CompletedTask;
public Task PostComment(ICommentViewModel comment) => Task.CompletedTask;
public Task CloseOrReopen(ICommentViewModel comment) => Task.CompletedTask;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
using System;
using System.Collections.Generic;
using System.Reactive;
using System.Threading.Tasks;
using GitHub.Models;
using GitHub.ViewModels;
using GitHub.ViewModels.Documents;
using ReactiveUI;

namespace GitHub.SampleData.Documents
{
public class PullRequestPageViewModelDesigner : ViewModelBase, IPullRequestPageViewModel
{
public PullRequestPageViewModelDesigner()
{
Body = @"Save drafts of inline comments, PR reviews and PRs.

> Note: This feature required a refactoring of the comment view models because they now need async initialization and to be available from GitHub.App. This part of the PR has been submitted separately as #1993 to ease review. The two PRs can alternatively be reviewed as one if that's more convenient.

As described in #1905, it is easy to lose a comment that you're working on if you close the diff view accidentally. This PR saves drafts of comments as they are being written to an SQLite database.

In addition to saving drafts of inline comments, it also saves comments to PR reviews and PRs themselves.

The comments are written to an SQLite database directly instead of going through Akavache because in the case of inline reviews, there can be many drafts in progress on a separate file. When a diff is opened we need to look for any comments present on that file and show the most recent. That use-case didn't fit well with Akavache (being a pure key/value store).

## Testing

### Inline Comments

- Open a PR
- Open the diff of a file
- Start adding a comment
- Close the comment by closing the peek view, or the document tab
- Reopen the diff
- You should see the comment displayed in edit mode with the draft of the comment you were previously writing

### PR reviews

- Open a PR
- Click ""Add your review""
- Start adding a review
- Click the ""Back"" button and navigate to a different PR
- Click the ""Back"" button and navigate to the original PR
- Click ""Add your review""
- You should see the the draft of the review you were previously writing

### PRs

-Click ""Create new"" at the top of the PR list
- Start adding a PR title/ description
- Close VS
- Restart VS and click ""Create new"" again
- You should see the the draft of the PR you were previously writing

Depends on #1993
Fixes #1905";
Timeline = new IViewModel[]
{
new CommitListViewModel(
new CommitSummaryViewModel(new CommitModel
{
Author = new ActorModel { Login = "grokys" },
AbbreviatedOid = "c7c7d25",
MessageHeadline = "Refactor comment view models."
}),
new CommitSummaryViewModel(new CommitModel
{
Author = new ActorModel { Login = "shana" },
AbbreviatedOid = "04e6a90",
MessageHeadline = "Refactor comment view models.",
})),
new CommentViewModelDesigner
{
Author = new ActorViewModelDesigner("meaghanlewis"),
Body = @"This is looking great! Really enjoying using this feature so far.

When leaving an inline comment, the comment posts successfully and then a new comment is drafted with the same text.",
},
new CommentViewModelDesigner
{
Author = new ActorViewModelDesigner("grokys"),
Body = @"Oops, sorry about that @meaghanlewis - I was sure I tested those things, but must have got messed up again at some point. Should be fixed now.",
},
};
}

public string Id { get; set; }
public PullRequestState State { get; set; } = PullRequestState.Open;
public IReadOnlyList<IViewModel> Timeline { get; }
public string SourceBranchDisplayName { get; set; } = "feature/save-drafts";
public string TargetBranchDisplayName { get; set; } = "master";
public IActorViewModel Author { get; set; } = new ActorViewModelDesigner("grokys");
public int CommitCount { get; set; } = 2;
public string Body { get; set; }
public int Number { get; set; } = 1994;
public LocalRepositoryModel LocalRepository { get; }
public RemoteRepositoryModel Repository { get; set; }
public string Title { get; set; } = "Save drafts of comments";
public Uri WebUrl { get; set; }
public ReactiveCommand<Unit, Unit> OpenOnGitHub { get; }
public ReactiveCommand<string, Unit> ShowCommit { get; }


public Task InitializeAsync(RemoteRepositoryModel repository, LocalRepositoryModel localRepository, ActorModel currentUser, PullRequestDetailModel model)
{
throw new NotImplementedException();
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ public PullRequestDetailViewModelDesigner()
public ReactiveCommand<Unit, Unit> Pull { get; }
public ReactiveCommand<Unit, Unit> Push { get; }
public ReactiveCommand<Unit, Unit> SyncSubmodules { get; }
public ReactiveCommand<Unit, Unit> OpenConversation { get; }
public ReactiveCommand<Unit, Unit> OpenOnGitHub { get; }
public ReactiveCommand<IPullRequestReviewSummaryViewModel, Unit> ShowReview { get; }
public ReactiveCommand<IPullRequestCheckViewModel, Unit> ShowAnnotations { get; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public PullRequestListViewModelDesigner()
public IReadOnlyList<string> States { get; }
public Uri WebUrl => null;
public ReactiveCommand<Unit, Unit> CreatePullRequest { get; }
public ReactiveCommand<IPullRequestListItemViewModel, Unit> OpenConversation { get; }
public ReactiveCommand<IIssueListItemViewModelBase, Unit> OpenItem { get; }
public ReactiveCommand<IPullRequestListItemViewModel, IPullRequestListItemViewModel> OpenItemInBrowser { get; }

Expand Down
14 changes: 7 additions & 7 deletions src/GitHub.App/Services/FromGraphQlExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ public static class FromGraphQlExtensions
}
}

public static PullRequestStateEnum FromGraphQl(this PullRequestState value)
public static Models.PullRequestState FromGraphQl(this Octokit.GraphQL.Model.PullRequestState value)
{
switch (value)
{
case PullRequestState.Open:
return PullRequestStateEnum.Open;
case PullRequestState.Closed:
return PullRequestStateEnum.Closed;
case PullRequestState.Merged:
return PullRequestStateEnum.Merged;
case Octokit.GraphQL.Model.PullRequestState.Open:
return Models.PullRequestState.Open;
case Octokit.GraphQL.Model.PullRequestState.Closed:
return Models.PullRequestState.Closed;
case Octokit.GraphQL.Model.PullRequestState.Merged:
return Models.PullRequestState.Merged;
default:
throw new ArgumentOutOfRangeException(nameof(value), value, null);
}
Expand Down
5 changes: 5 additions & 0 deletions src/GitHub.App/Services/GitClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,11 @@ public Task Checkout(IRepository repository, string branchName)
});
}

public async Task<bool> CommitExists(IRepository repository, string sha)
{
return await Task.Run(() => repository.Lookup<Commit>(sha) != null).ConfigureAwait(false);
}

public Task CreateBranch(IRepository repository, string branchName)
{
Guard.ArgumentNotNull(repository, nameof(repository));
Expand Down
113 changes: 113 additions & 0 deletions src/GitHub.App/Services/IssueishService.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
using System;
using System.Collections.Generic;
using System.Threading.Tasks;
using GitHub.Api;
using GitHub.Factories;
using GitHub.Models;
using GitHub.Primitives;
using Octokit;
using Octokit.GraphQL;
using Octokit.GraphQL.Model;
using static Octokit.GraphQL.Variable;

namespace GitHub.Services
{
/// <summary>
/// Base class for issue and pull request services.
/// </summary>
public abstract class IssueishService : IIssueishService
{
static ICompiledQuery<CommentModel> postComment;
readonly IApiClientFactory apiClientFactory;
readonly IGraphQLClientFactory graphqlFactory;

/// <summary>
/// Initializes a new instance of the <see cref="IssueishService"/> class.
/// </summary>
/// <param name="apiClientFactory">The API client factory.</param>
/// <param name="graphqlFactory">The GraphQL client factory.</param>
public IssueishService(
IApiClientFactory apiClientFactory,
IGraphQLClientFactory graphqlFactory)
{
this.apiClientFactory = apiClientFactory;
this.graphqlFactory = graphqlFactory;
}

/// <inheritdoc/>
public async Task CloseIssueish(HostAddress address, string owner, string repository, int number)
{
var client = await apiClientFactory.CreateGitHubClient(address).ConfigureAwait(false);
var update = new IssueUpdate { State = ItemState.Closed };
await client.Issue.Update(owner, repository, number, update).ConfigureAwait(false);
}

/// <inheritdoc/>
public async Task ReopenIssueish(HostAddress address, string owner, string repository, int number)
{
var client = await apiClientFactory.CreateGitHubClient(address).ConfigureAwait(false);
var update = new IssueUpdate { State = ItemState.Open };
await client.Issue.Update(owner, repository, number, update).ConfigureAwait(false);
}

/// <inheritdoc/>
public async Task<CommentModel> PostComment(HostAddress address, string issueishId, string body)
{
var input = new AddCommentInput
{
Body = body,
SubjectId = new ID(issueishId),
};

if (postComment == null)
{
postComment = new Mutation()
.AddComment(Var(nameof(input)))
.CommentEdge
.Node
.Select(comment => new CommentModel
{
Author = new ActorModel
{
Login = comment.Author.Login,
AvatarUrl = comment.Author.AvatarUrl(null),
},
Body = comment.Body,
CreatedAt = comment.CreatedAt,
DatabaseId = comment.DatabaseId.Value,
Id = comment.Id.Value,
Url = comment.Url,
}).Compile();
}

var vars = new Dictionary<string, object>
{
{ nameof(input), input },
};

var graphql = await graphqlFactory.CreateConnection(address).ConfigureAwait(false);
return await graphql.Run(postComment, vars).ConfigureAwait(false);
}

public async Task DeleteComment(
HostAddress address,
string owner,
string repository,
int commentId)
{
var client = await apiClientFactory.CreateGitHubClient(address).ConfigureAwait(false);
await client.Issue.Comment.Delete(owner, repository, commentId).ConfigureAwait(false);
}

public async Task EditComment(
HostAddress address,
string owner,
string repository,
int commentId,
string body)
{
var client = await apiClientFactory.CreateGitHubClient(address).ConfigureAwait(false);
await client.Issue.Comment.Update(owner, repository, commentId, body).ConfigureAwait(false);
}
}
}
Loading