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

Commit 0e58e32

Browse files
grokysgrokys
committed
Don't save drafts for edits.
1 parent b6debc7 commit 0e58e32

File tree

5 files changed

+90
-43
lines changed

5 files changed

+90
-43
lines changed

‎src/GitHub.App/ViewModels/CommentThreadViewModel.cs‎

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,6 @@ public CommentThreadViewModel(
4949

5050
this.DraftStore = draftStore;
5151
this.timerScheduler = timerScheduler;
52-
53-
comments.ChangeTrackingEnabled = true;
54-
comments.ItemChanged.Subscribe(ItemChanged);
5552
}
5653

5754
/// <inheritdoc/>
@@ -74,6 +71,23 @@ public CommentThreadViewModel(
7471
/// <inheritdoc/>
7572
public abstract Task DeleteComment(int pullRequestId, int commentId);
7673

74+
/// <summary>
75+
/// Adds a placeholder comment that will allow the user to enter a reply, and wires up
76+
/// event listeners for saving drafts.
77+
/// </summary>
78+
/// <param name="placeholder">The placeholder comment view model.</param>
79+
/// <returns>An object which when disposed will remove the event listeners.</returns>
80+
protected IDisposable AddPlaceholder(ICommentViewModel placeholder)
81+
{
82+
Comments.Add(placeholder);
83+
84+
return placeholder.WhenAnyValue(
85+
x => x.EditState,
86+
x => x.Body,
87+
(state, body) => (state, body))
88+
.Subscribe(x => PlaceholderChanged(placeholder, x.state, x.body));
89+
}
90+
7791
/// <summary>
7892
/// Intializes a new instance of the <see cref="CommentThreadViewModel"/> class.
7993
/// </summary>
@@ -94,44 +108,48 @@ protected virtual CommentDraft BuildDraft(ICommentViewModel comment)
94108

95109
protected abstract (string key, string secondaryKey) GetDraftKeys(ICommentViewModel comment);
96110

97-
void ItemChanged(IReactivePropertyChangedEventArgs<ICommentViewModel> e)
111+
void PlaceholderChanged(ICommentViewModel placeholder, CommentEditState state, string body)
98112
{
99-
if (e.PropertyName == nameof(ICommentViewModel.Body) &&
100-
e.Sender.EditState == CommentEditState.Editing)
113+
if (state == CommentEditState.Editing)
101114
{
102-
if (!draftThrottles.TryGetValue(e.Sender, out var throttle))
115+
if (!draftThrottles.TryGetValue(placeholder, out var throttle))
103116
{
104117
var subject = new Subject<ICommentViewModel>();
105118
subject.Throttle(TimeSpan.FromSeconds(1), timerScheduler).Subscribe(UpdateDraft);
106-
draftThrottles.Add(e.Sender, subject);
119+
draftThrottles.Add(placeholder, subject);
107120
throttle = subject;
108121
}
109122

110-
throttle.OnNext(e.Sender);
123+
throttle.OnNext(placeholder);
111124
}
112-
else if (e.PropertyName == nameof(ICommentViewModel.EditState) &&
113-
e.Sender.EditState != CommentEditState.Editing)
125+
else if (state != CommentEditState.Editing)
114126
{
115-
if (draftThrottles.TryGetValue(e.Sender, out var throttle))
127+
if (draftThrottles.TryGetValue(placeholder, out var throttle))
116128
{
117129
throttle.OnCompleted();
118-
draftThrottles.Remove(e.Sender);
130+
draftThrottles.Remove(placeholder);
119131
}
132+
133+
var (key, secondaryKey) = GetDraftKeys(placeholder);
134+
DraftStore.DeleteDraft(key, secondaryKey).Forget();
120135
}
121136
}
122137

123138
void UpdateDraft(ICommentViewModel comment)
124139
{
125-
var draft = BuildDraft(comment);
126-
var (key, secondaryKey) = GetDraftKeys(comment);
127-
128-
if (draft != null)
129-
{
130-
DraftStore.UpdateDraft(key, secondaryKey, draft).Forget();
131-
}
132-
else
140+
if (comment.EditState == CommentEditState.Editing)
133141
{
134-
DraftStore.DeleteDraft(key, secondaryKey).Forget();
142+
var draft = BuildDraft(comment);
143+
var (key, secondaryKey) = GetDraftKeys(comment);
144+
145+
if (draft != null)
146+
{
147+
DraftStore.UpdateDraft(key, secondaryKey, draft).Forget();
148+
}
149+
else
150+
{
151+
DraftStore.DeleteDraft(key, secondaryKey).Forget();
152+
}
135153
}
136154
}
137155
}

‎src/GitHub.App/ViewModels/PullRequestReviewCommentThreadViewModel.cs‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ await vm.InitializeAsync(
107107
{
108108
var vm = factory.CreateViewModel<IPullRequestReviewCommentViewModel>();
109109
await vm.InitializeAsPlaceholderAsync(session, this, false).ConfigureAwait(true);
110-
Comments.Add(vm);
111110

112111
var (key, secondaryKey) = GetDraftKeys(vm);
113112
var draft = await DraftStore.GetDraft<PullRequestReviewCommentDraft>(key, secondaryKey).ConfigureAwait(true);
@@ -117,6 +116,8 @@ await vm.InitializeAsync(
117116
await vm.BeginEdit.Execute();
118117
vm.Body = draft.Body;
119118
}
119+
120+
AddPlaceholder(vm);
120121
}
121122
}
122123

@@ -140,7 +141,6 @@ public async Task InitializeNewAsync(
140141

141142
var vm = factory.CreateViewModel<IPullRequestReviewCommentViewModel>();
142143
await vm.InitializeAsPlaceholderAsync(session, this, isEditing).ConfigureAwait(false);
143-
Comments.Add(vm);
144144

145145
var (key, secondaryKey) = GetDraftKeys(vm);
146146
var draft = await DraftStore.GetDraft<PullRequestReviewCommentDraft>(key, secondaryKey).ConfigureAwait(true);
@@ -149,6 +149,8 @@ public async Task InitializeNewAsync(
149149
{
150150
vm.Body = draft.Body;
151151
}
152+
153+
AddPlaceholder(vm);
152154
}
153155

154156
public override async Task PostComment(string body)

‎src/GitHub.InlineReviews/ViewModels/InlineCommentPeekViewModel.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,16 +179,15 @@ async Task UpdateThread()
179179
var thread = file.InlineCommentThreads?.FirstOrDefault(x =>
180180
x.LineNumber == lineNumber &&
181181
((leftBuffer && x.DiffLineType == DiffChangeType.Delete) || (!leftBuffer && x.DiffLineType != DiffChangeType.Delete)));
182-
183-
Thread = factory.CreateViewModel<IPullRequestReviewCommentThreadViewModel>();
182+
var vm = factory.CreateViewModel<IPullRequestReviewCommentThreadViewModel>();
184183

185184
if (thread?.Comments.Count > 0)
186185
{
187-
await Thread.InitializeAsync(session, file, thread.Comments[0].Review, thread, true);
186+
await vm.InitializeAsync(session, file, thread.Comments[0].Review, thread, true);
188187
}
189188
else
190189
{
191-
await Thread.InitializeNewAsync(session, file, lineNumber, side, true);
190+
await vm.InitializeNewAsync(session, file, lineNumber, side, true);
192191
}
193192

194193
if (!string.IsNullOrWhiteSpace(placeholderBody))
@@ -201,6 +200,8 @@ async Task UpdateThread()
201200
placeholder.Body = placeholderBody;
202201
}
203202
}
203+
204+
Thread = vm;
204205
}
205206

206207
async Task SessionChanged(IPullRequestSession pullRequestSession)

‎test/GitHub.App.UnitTests/ViewModels/CommentThreadViewModelTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ public async Task AddPlaceholder(bool isEditing)
9191
{
9292
var c = new TestComment();
9393
await c.InitializeAsPlaceholderAsync(this, isEditing);
94-
Comments.Add(c);
94+
AddPlaceholder(c);
9595
}
9696

9797
public override Task DeleteComment(int pullRequestId, int commentId) => throw new NotImplementedException();

‎test/GitHub.App.UnitTests/ViewModels/PullRequestReviewCommentThreadViewModelTests.cs‎

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,45 @@ public async Task PostsCommentInReplyToCorrectComment()
7373
}
7474

7575
[Test]
76-
public async Task LoadsDraftForNewComment()
76+
public async Task LoadsDraftForNewThread()
7777
{
78-
var draftStore = Substitute.For<IMessageDraftStore>();
78+
using (TestUtils.WithScheduler(Scheduler.CurrentThread))
79+
{
80+
var draftStore = Substitute.For<IMessageDraftStore>();
7981

80-
draftStore.GetDraft<CommentDraft>(
81-
"pr-review-comment|https://github.com/owner/repo|47|file.cs", "10")
82-
.Returns(new CommentDraft
83-
{
84-
Body = "Draft comment.",
85-
});
82+
draftStore.GetDraft<PullRequestReviewCommentDraft>(
83+
"pr-review-comment|https://github.com/owner/repo|47|file.cs", "10")
84+
.Returns(new PullRequestReviewCommentDraft
85+
{
86+
Body = "Draft comment.",
87+
Side = DiffSide.Right,
88+
});
89+
90+
var target = await CreateTarget(draftStore: draftStore, newThread: true);
91+
92+
Assert.That(target.Comments[0].Body, Is.EqualTo("Draft comment."));
93+
}
94+
}
95+
96+
[Test]
97+
public async Task LoadsDraftForExistingThread()
98+
{
99+
using (TestUtils.WithScheduler(Scheduler.CurrentThread))
100+
{
101+
var draftStore = Substitute.For<IMessageDraftStore>();
86102

87-
var target = await CreateTarget(draftStore: draftStore, newThread: true);
103+
draftStore.GetDraft<PullRequestReviewCommentDraft>(
104+
"pr-review-comment|https://github.com/owner/repo|47|file.cs", "10")
105+
.Returns(new PullRequestReviewCommentDraft
106+
{
107+
Body = "Draft comment.",
108+
Side = DiffSide.Right,
109+
});
88110

89-
Assert.That(target.Comments[0].Body, Is.EqualTo("Draft comment."));
111+
var target = await CreateTarget(draftStore: draftStore);
112+
113+
Assert.That(target.Comments[0].Body, Is.EqualTo("Draft comment."));
114+
}
90115
}
91116

92117
async Task<PullRequestReviewCommentThreadViewModel> CreateTarget(
@@ -105,17 +130,18 @@ async Task<PullRequestReviewCommentThreadViewModel> CreateTarget(
105130
review = review ?? new PullRequestReviewModel();
106131
comments = comments ?? CreateComments();
107132

108-
var thread = Substitute.For<IInlineCommentThreadModel>();
109-
thread.Comments.Returns(comments.ToList());
110-
111133
var result = new PullRequestReviewCommentThreadViewModel(draftStore, factory);
112134

113135
if (newThread)
114136
{
115-
await result.InitializeNewAsync(session, file, 10, DiffSide.Left, true);
137+
await result.InitializeNewAsync(session, file, 10, DiffSide.Right, true);
116138
}
117139
else
118140
{
141+
var thread = Substitute.For<IInlineCommentThreadModel>();
142+
thread.Comments.Returns(comments.ToList());
143+
thread.LineNumber.Returns(10);
144+
119145
await result.InitializeAsync(session, file, review, thread, true);
120146
}
121147

0 commit comments

Comments
 (0)