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

Commit 82a2ccd

Browse files
jcansdalejcansdale
authored
Merge pull request #1994 from github/feature/save-drafts
Save drafts of comments
2 parents adb4119 + f93fdf4 commit 82a2ccd

37 files changed

+7232
-335
lines changed

‎codecov.yml

+1
Original file line numberDiff line numberDiff line change
@@ -32,3 +32,4 @@ ignore:
3232
- "*.xaml"
3333
- "*.xaml.cs"
3434
- "**/SampleData/*"
35+
- "src/GitHub.App/sqlite-net/*"

‎src/GitHub.App/Models/Drafts/CommentDraft.cs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using GitHub.ViewModels;
2+
3+
namespace GitHub.Models.Drafts
4+
{
5+
/// <summary>
6+
/// Stores a draft for a <see cref="CommentViewModel"/>
7+
/// </summary>
8+
public class CommentDraft
9+
{
10+
/// <summary>
11+
/// Gets or sets the draft comment body.
12+
/// </summary>
13+
public string Body { get; set; }
14+
}
15+
}

‎src/GitHub.App/Models/Drafts/PullRequestDraft.cs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
using GitHub.ViewModels.GitHubPane;
2+
3+
namespace GitHub.Models.Drafts
4+
{
5+
/// <summary>
6+
/// Stores a draft for a <see cref="PullRequestCreationViewModel"/>.
7+
/// </summary>
8+
public class PullRequestDraft
9+
{
10+
/// <summary>
11+
/// Gets or sets the draft pull request title.
12+
/// </summary>
13+
public string Title { get; set; }
14+
15+
/// <summary>
16+
/// Gets or sets the draft pull request body.
17+
/// </summary>
18+
public string Body { get; set; }
19+
}
20+
}

‎src/GitHub.App/Models/Drafts/PullRequestReviewCommentDraft.cs

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
using System;
2+
using GitHub.ViewModels;
3+
4+
namespace GitHub.Models.Drafts
5+
{
6+
/// <summary>
7+
/// Stores a draft for a <see cref="PullRequestReviewCommentViewModel"/>
8+
/// </summary>
9+
public class PullRequestReviewCommentDraft : CommentDraft
10+
{
11+
/// <summary>
12+
/// Gets or sets the side of the diff that the draft comment was left on.
13+
/// </summary>
14+
public DiffSide Side { get; set; }
15+
16+
/// <summary>
17+
/// Gets or sets the time that the draft was last updated.
18+
/// </summary>
19+
public DateTimeOffset UpdatedAt { get; set; }
20+
}
21+
}

‎src/GitHub.App/Models/Drafts/PullRequestReviewDraft.cs

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
using GitHub.ViewModels.GitHubPane;
2+
3+
namespace GitHub.Models.Drafts
4+
{
5+
/// <summary>
6+
/// Stores a draft for a <see cref="PullRequestReviewAuthoringViewModel"/>.
7+
/// </summary>
8+
public class PullRequestReviewDraft
9+
{
10+
/// <summary>
11+
/// Gets or sets the draft review body.
12+
/// </summary>
13+
public string Body { get; set; }
14+
}
15+
}

‎src/GitHub.App/SampleData/CommentThreadViewModelDesigner.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ public class CommentThreadViewModelDesigner : ViewModelBase, ICommentThreadViewM
1414
public IActorViewModel CurrentUser { get; set; }
1515
= new ActorViewModel { Login = "shana" };
1616

17-
public Task DeleteComment(int pullRequestId, int commentId) => Task.CompletedTask;
18-
public Task EditComment(string id, string body) => Task.CompletedTask;
19-
public Task PostComment(string body) => Task.CompletedTask;
17+
public Task DeleteComment(ICommentViewModel comment) => Task.CompletedTask;
18+
public Task EditComment(ICommentViewModel comment) => Task.CompletedTask;
19+
public Task PostComment(ICommentViewModel comment) => Task.CompletedTask;
2020
}
2121
}

‎src/GitHub.InlineReviews/Services/InlineCommentPeekService.cs renamed to ‎src/GitHub.App/Services/InlineCommentPeekService.cs

+16-44
Original file line numberDiff line numberDiff line change
@@ -2,30 +2,25 @@
22
using System.ComponentModel.Composition;
33
using System.Linq;
44
using System.Reactive.Linq;
5-
using System.Threading.Tasks;
6-
using GitHub.Api;
75
using GitHub.Extensions;
8-
using GitHub.Factories;
9-
using GitHub.InlineReviews.Peek;
10-
using GitHub.InlineReviews.Tags;
116
using GitHub.Models;
12-
using GitHub.Primitives;
13-
using GitHub.Services;
7+
using GitHub.ViewModels;
148
using Microsoft.VisualStudio.Language.Intellisense;
159
using Microsoft.VisualStudio.Text;
1610
using Microsoft.VisualStudio.Text.Differencing;
1711
using Microsoft.VisualStudio.Text.Editor;
1812
using Microsoft.VisualStudio.Text.Outlining;
1913
using Microsoft.VisualStudio.Text.Projection;
2014

21-
namespace GitHub.InlineReviews.Services
15+
namespace GitHub.Services
2216
{
2317
/// <summary>
2418
/// Shows inline comments in a peek view.
2519
/// </summary>
2620
[Export(typeof(IInlineCommentPeekService))]
2721
class InlineCommentPeekService : IInlineCommentPeekService
2822
{
23+
const string relationship = "GitHubCodeReview";
2924
readonly IOutliningManagerService outliningService;
3025
readonly IPeekBroker peekBroker;
3126
readonly IUsageTracker usageTracker;
@@ -90,69 +85,46 @@ public void Hide(ITextView textView)
9085
}
9186

9287
/// <inheritdoc/>
93-
public ITrackingPoint Show(ITextView textView, AddInlineCommentTag tag)
88+
public ITrackingPoint Show(ITextView textView, DiffSide side, int lineNumber)
9489
{
95-
Guard.ArgumentNotNull(tag, nameof(tag));
96-
97-
var lineAndtrackingPoint = GetLineAndTrackingPoint(textView, tag);
90+
var lineAndtrackingPoint = GetLineAndTrackingPoint(textView, side, lineNumber);
9891
var line = lineAndtrackingPoint.Item1;
9992
var trackingPoint = lineAndtrackingPoint.Item2;
10093
var options = new PeekSessionCreationOptions(
10194
textView,
102-
InlineCommentPeekRelationship.Instance.Name,
95+
relationship,
10396
trackingPoint,
10497
defaultHeight: 0);
10598

10699
ExpandCollapsedRegions(textView, line.Extent);
107100

108101
var session = peekBroker.TriggerPeekSession(options);
109-
var item = session.PeekableItems.OfType<InlineCommentPeekableItem>().FirstOrDefault();
110-
item?.ViewModel.Close.Take(1).Subscribe(_ => session.Dismiss());
111-
112-
return trackingPoint;
113-
}
114-
115-
/// <inheritdoc/>
116-
public ITrackingPoint Show(ITextView textView, ShowInlineCommentTag tag)
117-
{
118-
Guard.ArgumentNotNull(textView, nameof(textView));
119-
Guard.ArgumentNotNull(tag, nameof(tag));
120-
121-
var lineAndtrackingPoint = GetLineAndTrackingPoint(textView, tag);
122-
var line = lineAndtrackingPoint.Item1;
123-
var trackingPoint = lineAndtrackingPoint.Item2;
124-
var options = new PeekSessionCreationOptions(
125-
textView,
126-
InlineCommentPeekRelationship.Instance.Name,
127-
trackingPoint,
128-
defaultHeight: 0);
129-
130-
ExpandCollapsedRegions(textView, line.Extent);
131-
132-
var session = peekBroker.TriggerPeekSession(options);
133-
var item = session.PeekableItems.OfType<InlineCommentPeekableItem>().FirstOrDefault();
134-
item?.ViewModel.Close.Take(1).Subscribe(_ => session.Dismiss());
135-
102+
var item = session.PeekableItems.OfType<IClosable>().FirstOrDefault();
103+
item?.Closed.Take(1).Subscribe(_ => session.Dismiss());
104+
136105
return trackingPoint;
137106
}
138107

139-
Tuple<ITextSnapshotLine, ITrackingPoint> GetLineAndTrackingPoint(ITextView textView, InlineCommentTag tag)
108+
Tuple<ITextSnapshotLine, ITrackingPoint> GetLineAndTrackingPoint(
109+
ITextView textView,
110+
DiffSide side,
111+
int lineNumber)
140112
{
141113
var diffModel = (textView as IWpfTextView)?.TextViewModel as IDifferenceTextViewModel;
142114
var snapshot = textView.TextSnapshot;
143115

144116
if (diffModel?.ViewType == DifferenceViewType.InlineView)
145117
{
146-
snapshot = tag.DiffChangeType == DiffChangeType.Delete ?
118+
snapshot = side == DiffSide.Left ?
147119
diffModel.Viewer.DifferenceBuffer.LeftBuffer.CurrentSnapshot :
148120
diffModel.Viewer.DifferenceBuffer.RightBuffer.CurrentSnapshot;
149121
}
150122

151-
var line = snapshot.GetLineFromLineNumber(tag.LineNumber);
123+
var line = snapshot.GetLineFromLineNumber(lineNumber);
152124
var trackingPoint = snapshot.CreateTrackingPoint(line.Start.Position, PointTrackingMode.Positive);
153125

154126
ExpandCollapsedRegions(textView, line.Extent);
155-
peekBroker.TriggerPeekSession(textView, trackingPoint, InlineCommentPeekRelationship.Instance.Name);
127+
peekBroker.TriggerPeekSession(textView, trackingPoint, relationship);
156128

157129
usageTracker.IncrementCounter(x => x.NumberOfPRReviewDiffViewInlineCommentOpen).Forget();
158130

‎src/GitHub.App/Services/PullRequestEditorService.cs

+50-3
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,12 @@
55
using System.Linq;
66
using System.Reactive.Linq;
77
using System.Threading.Tasks;
8+
using EnvDTE;
89
using GitHub.Commands;
910
using GitHub.Extensions;
1011
using GitHub.Models;
12+
using GitHub.Models.Drafts;
13+
using GitHub.ViewModels;
1114
using GitHub.VisualStudio;
1215
using Microsoft.VisualStudio;
1316
using Microsoft.VisualStudio.Editor;
@@ -18,7 +21,6 @@
1821
using Microsoft.VisualStudio.Text.Editor;
1922
using Microsoft.VisualStudio.Text.Projection;
2023
using Microsoft.VisualStudio.TextManager.Interop;
21-
using EnvDTE;
2224
using Task = System.Threading.Tasks.Task;
2325

2426
namespace GitHub.Services
@@ -39,6 +41,8 @@ public class PullRequestEditorService : IPullRequestEditorService
3941
readonly IStatusBarNotificationService statusBar;
4042
readonly IGoToSolutionOrPullRequestFileCommand goToSolutionOrPullRequestFileCommand;
4143
readonly IEditorOptionsFactoryService editorOptionsFactoryService;
44+
readonly IMessageDraftStore draftStore;
45+
readonly IInlineCommentPeekService peekService;
4246
readonly IUsageTracker usageTracker;
4347

4448
[ImportingConstructor]
@@ -49,6 +53,8 @@ public PullRequestEditorService(
4953
IStatusBarNotificationService statusBar,
5054
IGoToSolutionOrPullRequestFileCommand goToSolutionOrPullRequestFileCommand,
5155
IEditorOptionsFactoryService editorOptionsFactoryService,
56+
IMessageDraftStore draftStore,
57+
IInlineCommentPeekService peekService,
5258
IUsageTracker usageTracker)
5359
{
5460
Guard.ArgumentNotNull(serviceProvider, nameof(serviceProvider));
@@ -58,13 +64,17 @@ public PullRequestEditorService(
5864
Guard.ArgumentNotNull(goToSolutionOrPullRequestFileCommand, nameof(goToSolutionOrPullRequestFileCommand));
5965
Guard.ArgumentNotNull(goToSolutionOrPullRequestFileCommand, nameof(editorOptionsFactoryService));
6066
Guard.ArgumentNotNull(usageTracker, nameof(usageTracker));
67+
Guard.ArgumentNotNull(peekService, nameof(peekService));
68+
Guard.ArgumentNotNull(draftStore, nameof(draftStore));
6169

6270
this.serviceProvider = serviceProvider;
6371
this.pullRequestService = pullRequestService;
6472
this.vsEditorAdaptersFactory = vsEditorAdaptersFactory;
6573
this.statusBar = statusBar;
6674
this.goToSolutionOrPullRequestFileCommand = goToSolutionOrPullRequestFileCommand;
6775
this.editorOptionsFactoryService = editorOptionsFactoryService;
76+
this.draftStore = draftStore;
77+
this.peekService = peekService;
6878
this.usageTracker = usageTracker;
6979
}
7080

@@ -129,7 +139,7 @@ public async Task<ITextView> OpenFile(
129139
}
130140

131141
/// <inheritdoc/>
132-
public async Task<IDifferenceViewer> OpenDiff(IPullRequestSession session, string relativePath, string headSha, bool scrollToFirstDiff)
142+
public async Task<IDifferenceViewer> OpenDiff(IPullRequestSession session, string relativePath, string headSha, bool scrollToFirstDraftOrDiff)
133143
{
134144
Guard.ArgumentNotNull(session, nameof(session));
135145
Guard.ArgumentNotEmptyString(relativePath, nameof(relativePath));
@@ -168,12 +178,37 @@ await pullRequestService.ExtractToTempFile(
168178
var caption = $"Diff - {Path.GetFileName(file.RelativePath)}";
169179
var options = __VSDIFFSERVICEOPTIONS.VSDIFFOPT_DetectBinaryFiles |
170180
__VSDIFFSERVICEOPTIONS.VSDIFFOPT_LeftFileIsTemporary;
181+
var openThread = (line: -1, side: DiffSide.Left);
182+
var scrollToFirstDiff = false;
171183

172184
if (!workingDirectory)
173185
{
174186
options |= __VSDIFFSERVICEOPTIONS.VSDIFFOPT_RightFileIsTemporary;
175187
}
176188

189+
if (scrollToFirstDraftOrDiff)
190+
{
191+
var (key, _) = PullRequestReviewCommentThreadViewModel.GetDraftKeys(
192+
session.LocalRepository.CloneUrl.WithOwner(session.RepositoryOwner),
193+
session.PullRequest.Number,
194+
relativePath,
195+
0);
196+
var drafts = (await draftStore.GetDrafts<PullRequestReviewCommentDraft>(key)
197+
.ConfigureAwait(true))
198+
.OrderByDescending(x => x.data.UpdatedAt)
199+
.ToList();
200+
201+
if (drafts.Count > 0 && int.TryParse(drafts[0].secondaryKey, out var line))
202+
{
203+
openThread = (line, drafts[0].data.Side);
204+
scrollToFirstDiff = false;
205+
}
206+
else
207+
{
208+
scrollToFirstDiff = true;
209+
}
210+
}
211+
177212
IVsWindowFrame frame;
178213
using (OpenWithOption(DifferenceViewerOptions.ScrollToFirstDiffName, scrollToFirstDiff))
179214
using (OpenInProvisionalTab())
@@ -228,6 +263,18 @@ await pullRequestService.ExtractToTempFile(
228263
else
229264
await usageTracker.IncrementCounter(x => x.NumberOfPRDetailsViewChanges);
230265

266+
if (openThread.line != -1)
267+
{
268+
var view = diffViewer.ViewMode == DifferenceViewMode.Inline ?
269+
diffViewer.InlineView :
270+
openThread.side == DiffSide.Left ? diffViewer.LeftView : diffViewer.RightView;
271+
272+
// HACK: We need to wait here for the view to initialize or the peek session won't appear.
273+
// There must be a better way of doing this.
274+
await Task.Delay(1500).ConfigureAwait(true);
275+
peekService.Show(view, openThread.side, openThread.line);
276+
}
277+
231278
return diffViewer;
232279
}
233280
catch (Exception e)
@@ -247,7 +294,7 @@ public async Task<IDifferenceViewer> OpenDiff(
247294
Guard.ArgumentNotEmptyString(relativePath, nameof(relativePath));
248295
Guard.ArgumentNotNull(thread, nameof(thread));
249296

250-
var diffViewer = await OpenDiff(session, relativePath, thread.CommitSha, scrollToFirstDiff: false);
297+
var diffViewer = await OpenDiff(session, relativePath, thread.CommitSha, scrollToFirstDraftOrDiff: false);
251298

252299
var param = (object)new InlineCommentNavigationParams
253300
{

0 commit comments

Comments
 (0)