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

Commit d57692b

Browse files
committed
Don't retain placeholder when starting a review.
Starting a review by replying to an existing comment was causing the submitted comment's text to be copied into the placeholder after submission. Add a unit test to check for this and fix it.
1 parent b95d370 commit d57692b

File tree

6 files changed

+72
-7
lines changed

6 files changed

+72
-7
lines changed

src/GitHub.InlineReviews/SampleData/CommentViewModelDesigner.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ public CommentViewModelDesigner()
2323
public string ErrorMessage { get; set; }
2424
public CommentEditState EditState { get; set; }
2525
public bool IsReadOnly { get; set; }
26+
public bool IsSubmitting { get; set; }
2627
public ICommentThreadViewModel Thread { get; }
2728
public DateTimeOffset UpdatedAt => DateTime.Now.Subtract(TimeSpan.FromDays(3));
2829
public IAccount User { get; set; }

src/GitHub.InlineReviews/ViewModels/CommentViewModel.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ public class CommentViewModel : ReactiveObject, ICommentViewModel
2020
string body;
2121
string errorMessage;
2222
bool isReadOnly;
23+
bool isSubmitting;
2324
CommentEditState state;
2425
DateTimeOffset updatedAt;
2526
string undoBody;
@@ -127,6 +128,7 @@ async Task DoCommitEdit(object unused)
127128
try
128129
{
129130
ErrorMessage = null;
131+
IsSubmitting = true;
130132

131133
var model = await Thread.PostComment.ExecuteAsyncTask(Body);
132134
Id = model.Id;
@@ -140,6 +142,10 @@ async Task DoCommitEdit(object unused)
140142
ErrorMessage = message;
141143
log.Error(e, "Error posting comment");
142144
}
145+
finally
146+
{
147+
IsSubmitting = false;
148+
}
143149
}
144150

145151
/// <inheritdoc/>
@@ -176,6 +182,13 @@ public bool IsReadOnly
176182
set { this.RaiseAndSetIfChanged(ref isReadOnly, value); }
177183
}
178184

185+
/// <inheritdoc/>
186+
public bool IsSubmitting
187+
{
188+
get { return isSubmitting; }
189+
protected set { this.RaiseAndSetIfChanged(ref isSubmitting, value); }
190+
}
191+
179192
/// <inheritdoc/>
180193
public DateTimeOffset UpdatedAt
181194
{

src/GitHub.InlineReviews/ViewModels/ICommentViewModel.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ public interface ICommentViewModel : IViewModel
4848
/// </summary>
4949
bool IsReadOnly { get; set; }
5050

51+
/// <summary>
52+
/// Gets a value indicating whether the comment is currently in the process of being
53+
/// submitted.
54+
/// </summary>
55+
bool IsSubmitting { get; }
56+
5157
/// <summary>
5258
/// Gets the modified date of the comment.
5359
/// </summary>

src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ async void LinesChanged(IReadOnlyList<Tuple<int, DiffSide>> lines)
154154

155155
async Task UpdateThread()
156156
{
157-
var placeholderBody = await GetPlaceholderBodyToPreserve();
157+
var placeholderBody = GetPlaceholderBodyToPreserve();
158158

159159
Thread = null;
160160
threadSubscription?.Dispose();
@@ -208,14 +208,13 @@ async Task SessionChanged(IPullRequestSession pullRequestSession)
208208
}
209209
}
210210

211-
async Task<string> GetPlaceholderBodyToPreserve()
211+
string GetPlaceholderBodyToPreserve()
212212
{
213213
var lastComment = Thread?.Comments.LastOrDefault();
214214

215215
if (lastComment?.EditState == CommentEditState.Editing)
216216
{
217-
var executing = await lastComment.CommitEdit.IsExecuting.FirstAsync();
218-
if (!executing) return lastComment.Body;
217+
if (!lastComment.IsSubmitting) return lastComment.Body;
219218
}
220219

221220
return null;

src/GitHub.InlineReviews/ViewModels/PullRequestReviewCommentViewModel.cs

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,17 @@ public static CommentViewModel CreatePlaceholder(
120120

121121
async Task DoStartReview(object unused)
122122
{
123-
await session.StartReview();
124-
await CommitEdit.ExecuteAsync(null);
123+
IsSubmitting = true;
124+
125+
try
126+
{
127+
await session.StartReview();
128+
await CommitEdit.ExecuteAsync(null);
129+
}
130+
finally
131+
{
132+
IsSubmitting = false;
133+
}
125134
}
126135
}
127136
}

test/GitHub.InlineReviews.UnitTests/ViewModels/InlineCommentPeekViewModelTests.cs

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ public async Task RetainsCommentBeingEditedWhenSessionRefreshed()
195195
}
196196

197197
[Test]
198-
public async Task DoesntRetainSubmittedCommentInPlaceholderAfterPost()
198+
public async Task CommittingEditDoesntRetainSubmittedCommentInPlaceholderAfterPost()
199199
{
200200
var sessionManager = CreateSessionManager();
201201
var peekSession = CreatePeekSession();
@@ -231,6 +231,43 @@ public async Task DoesntRetainSubmittedCommentInPlaceholderAfterPost()
231231
Assert.That(string.Empty, Is.EqualTo(placeholder.Body));
232232
}
233233

234+
[Test]
235+
public async Task StartingReviewDoesntRetainSubmittedCommentInPlaceholderAfterPost()
236+
{
237+
var sessionManager = CreateSessionManager();
238+
var peekSession = CreatePeekSession();
239+
var target = new InlineCommentPeekViewModel(
240+
CreatePeekService(lineNumber: 10),
241+
peekSession,
242+
sessionManager,
243+
Substitute.For<INextInlineCommentCommand>(),
244+
Substitute.For<IPreviousInlineCommentCommand>());
245+
246+
await target.Initialize();
247+
248+
Assert.That(2, Is.EqualTo(target.Thread.Comments.Count));
249+
250+
sessionManager.CurrentSession.StartReview()
251+
.ReturnsForAnyArgs(async x =>
252+
{
253+
var file = await sessionManager.GetLiveFile(
254+
RelativePath,
255+
peekSession.TextView,
256+
peekSession.TextView.TextBuffer);
257+
RaiseLinesChanged(file, Tuple.Create(10, DiffSide.Right));
258+
return Substitute.For<IPullRequestReviewModel>();
259+
});
260+
261+
var placeholder = (IPullRequestReviewCommentViewModel)target.Thread.Comments.Last();
262+
placeholder.BeginEdit.Execute(null);
263+
placeholder.Body = "Comment being edited";
264+
placeholder.StartReview.Execute(null);
265+
266+
placeholder = (IPullRequestReviewCommentViewModel)target.Thread.Comments.Last();
267+
Assert.That(CommentEditState.Placeholder, Is.EqualTo(placeholder.EditState));
268+
Assert.That(string.Empty, Is.EqualTo(placeholder.Body));
269+
}
270+
234271
void AddCommentToExistingThread(IPullRequestSessionFile file)
235272
{
236273
var newThreads = file.InlineCommentThreads.ToList();

0 commit comments

Comments
 (0)