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

Commit ebdf9e9

Browse files
committed
Check for dirty repository state.
When checking out/updating PR branch as that will block the checkout.
1 parent 6e57be4 commit ebdf9e9

File tree

8 files changed

+219
-41
lines changed

8 files changed

+219
-41
lines changed

src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public PullRequestDetailViewModelDesigner()
6969
public CheckoutMode CheckoutMode { get; set; }
7070
public string CheckoutError { get; set; }
7171
public int CommitsBehind { get; set; }
72+
public string CheckoutDisabledMessage { get; set; }
7273

7374
public ReactiveCommand<Unit> Checkout { get; }
7475
public ReactiveCommand<object> OpenOnGitHub { get; }

src/GitHub.App/Services/PullRequestService.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ public IObservable<string> GetPullRequestTemplate(ILocalRepositoryModel reposito
8383
});
8484
}
8585

86+
public IObservable<bool> CleanForCheckout(ILocalRepositoryModel repository)
87+
{
88+
var repo = gitService.GetRepository(repository.LocalPath);
89+
return Observable.Return(!repo.RetrieveStatus().IsDirty);
90+
}
91+
8692
public IObservable<Unit> FetchAndCheckout(ILocalRepositoryModel repository, int pullRequestNumber, string localBranchName)
8793
{
8894
return DoFetchAndCheckout(repository, pullRequestNumber, localBranchName).ToObservable();

src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ public class PullRequestDetailViewModel : BaseViewModel, IPullRequestDetailViewM
3333
{
3434
readonly IRepositoryHost repositoryHost;
3535
readonly ILocalRepositoryModel repository;
36-
readonly IGitService gitService;
3736
readonly IPullRequestService pullRequestsService;
3837
readonly IAvatarProvider avatarProvider;
3938
PullRequestState state;
@@ -47,29 +46,26 @@ public class PullRequestDetailViewModel : BaseViewModel, IPullRequestDetailViewM
4746
int changeCount;
4847
ChangedFilesView changedFilesView;
4948
OpenChangedFileAction openChangedFileAction;
50-
bool uncommittedChanges;
5149
CheckoutMode checkoutMode;
5250
string checkoutError;
5351
int commitsBehind;
52+
string checkoutDisabledMessage;
5453

5554
/// <summary>
5655
/// Initializes a new instance of the <see cref="PullRequestDetailViewModel"/> class.
5756
/// </summary>
5857
/// <param name="connectionRepositoryHostMap">The connection repository host map.</param>
5958
/// <param name="teservice">The team explorer service.</param>
60-
/// <param name="gitService">The git service.</param>
6159
/// <param name="pullRequestsService">The pull requests service.</param>
6260
/// <param name="avatarProvider">The avatar provider.</param>
6361
[ImportingConstructor]
6462
PullRequestDetailViewModel(
6563
IConnectionRepositoryHostMap connectionRepositoryHostMap,
6664
ITeamExplorerServiceHolder teservice,
67-
IGitService gitService,
6865
IPullRequestService pullRequestsService,
6966
IAvatarProvider avatarProvider)
7067
: this(connectionRepositoryHostMap.CurrentRepositoryHost,
7168
teservice.ActiveRepo,
72-
gitService,
7369
pullRequestsService,
7470
avatarProvider)
7571
{
@@ -80,25 +76,24 @@ public class PullRequestDetailViewModel : BaseViewModel, IPullRequestDetailViewM
8076
/// </summary>
8177
/// <param name="repositoryHost">The repository host.</param>
8278
/// <param name="teservice">The team explorer service.</param>
83-
/// <param name="gitService">The git service.</param>
8479
/// <param name="pullRequestsService">The pull requests service.</param>
8580
/// <param name="avatarProvider">The avatar provider.</param>
8681
public PullRequestDetailViewModel(
8782
IRepositoryHost repositoryHost,
8883
ILocalRepositoryModel repository,
89-
IGitService gitService,
9084
IPullRequestService pullRequestsService,
9185
IAvatarProvider avatarProvider)
9286
{
9387
this.repositoryHost = repositoryHost;
9488
this.repository = repository;
95-
this.gitService = gitService;
9689
this.pullRequestsService = pullRequestsService;
9790
this.avatarProvider = avatarProvider;
9891

99-
Checkout = ReactiveCommand.CreateAsyncObservable(
100-
this.WhenAnyValue(x => x.UncommittedChanges, x => !x),
101-
DoCheckout);
92+
var canCheckout = this.WhenAnyValue(
93+
x => x.CheckoutMode,
94+
x => x.CheckoutDisabledMessage,
95+
(mode, disabled) => mode != CheckoutMode.UpToDate && disabled == null);
96+
Checkout = ReactiveCommand.CreateAsyncObservable(canCheckout, DoCheckout);
10297

10398
OpenOnGitHub = ReactiveCommand.Create();
10499

@@ -216,15 +211,6 @@ public OpenChangedFileAction OpenChangedFileAction
216211
set { this.RaiseAndSetIfChanged(ref openChangedFileAction, value); }
217212
}
218213

219-
/// <summary>
220-
/// Gets a value indicating whether there are uncommitted changes blocking a checkout.
221-
/// </summary>
222-
public bool UncommittedChanges
223-
{
224-
get { return uncommittedChanges; }
225-
private set { this.RaiseAndSetIfChanged(ref uncommittedChanges, value); }
226-
}
227-
228214
/// <summary>
229215
/// Gets the checkout mode for the pull request.
230216
/// </summary>
@@ -253,6 +239,15 @@ public int CommitsBehind
253239
private set { this.RaiseAndSetIfChanged(ref commitsBehind, value); }
254240
}
255241

242+
/// <summary>
243+
/// Gets a message indicating the why the <see cref="Checkout"/> command is disabled.
244+
/// </summary>
245+
public string CheckoutDisabledMessage
246+
{
247+
get { return checkoutDisabledMessage; }
248+
private set { this.RaiseAndSetIfChanged(ref checkoutDisabledMessage, value); }
249+
}
250+
256251
/// <summary>
257252
/// Gets the changed files as a tree.
258253
/// </summary>
@@ -334,14 +329,6 @@ public async Task Load(PullRequest pullRequest, IList<PullRequestFile> files)
334329
ChangedFilesTree.Add(change);
335330
}
336331

337-
var repo = gitService.GetRepository(repository.LocalPath);
338-
UncommittedChanges = repo.RetrieveStatus().IsDirty;
339-
340-
if (UncommittedChanges)
341-
{
342-
CheckoutError = "Cannot check out: you have uncommitted changes";
343-
}
344-
345332
var localBranches = await pullRequestsService.GetLocalBranches(repository, Number).ToList();
346333

347334
if (localBranches.Contains(repository.CurrentBranch))
@@ -371,6 +358,12 @@ public async Task Load(PullRequest pullRequest, IList<PullRequestFile> files)
371358
CheckoutMode = CheckoutMode.Fetch;
372359
}
373360

361+
var clean = await pullRequestsService.CleanForCheckout(repository);
362+
363+
CheckoutDisabledMessage = (!clean && CheckoutMode != CheckoutMode.UpToDate) ?
364+
$"Cannot {GetCheckoutModeDescription(CheckoutMode)} as your working directory has uncommitted changes." :
365+
null;
366+
374367
IsBusy = false;
375368
}
376369

@@ -411,6 +404,22 @@ static IPullRequestDirectoryViewModel CreateChangedFilesTree(IEnumerable<IPullRe
411404
return dirs[string.Empty];
412405
}
413406

407+
static string GetCheckoutModeDescription(CheckoutMode checkoutMode)
408+
{
409+
switch (checkoutMode)
410+
{
411+
case CheckoutMode.NeedsPull:
412+
return "update branch";
413+
case CheckoutMode.Switch:
414+
return "switch branches";
415+
case CheckoutMode.Fetch:
416+
return "checkout pull request";
417+
default:
418+
Debug.Fail("Invalid CheckoutMode in GetCheckoutModeDescription");
419+
return null;
420+
}
421+
}
422+
414423
static PullRequestDirectoryViewModel GetDirectory(string path, Dictionary<string, PullRequestDirectoryViewModel> dirs)
415424
{
416425
PullRequestDirectoryViewModel dir;

src/GitHub.Exports.Reactive/Services/IPullRequestService.cs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,13 @@ IObservable<IPullRequestModel> CreatePullRequest(IRepositoryHost host,
1212
IBranch sourceBranch, IBranch targetBranch,
1313
string title, string body);
1414

15+
/// <summary>
16+
/// Checks whether the specified repository is in a clean state that will allow a checkout.
17+
/// </summary>
18+
/// <param name="repository">The repository.</param>
19+
/// <returns></returns>
20+
IObservable<bool> CleanForCheckout(ILocalRepositoryModel repository);
21+
1522
/// <summary>
1623
/// Fetches a pull request to a local branch and checks out the branch.
1724
/// </summary>

src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,11 @@ public interface IPullRequestDetailViewModel : IViewModel, IHasBusy
149149
/// </summary>
150150
int CommitsBehind { get; }
151151

152+
/// <summary>
153+
/// Gets a message indicating the why the <see cref="Checkout"/> command is disabled.
154+
/// </summary>
155+
string CheckoutDisabledMessage { get; }
156+
152157
/// <summary>
153158
/// Gets a command that checks out the pull request locally.
154159
/// </summary>

src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
<d:DesignProperties.DataContext>
2020
<Binding>
2121
<Binding.Source>
22-
<sampleData:PullRequestDetailViewModelDesigner CheckoutMode="NeedsPull" CommitsBehind="2"/>
22+
<sampleData:PullRequestDetailViewModelDesigner CheckoutMode="NeedsPull"
23+
CommitsBehind="2"
24+
CheckoutDisabledMessage="Cannot update branch as your working directory has uncommitted changes."/>
2325
</Binding.Source>
2426
</Binding>
2527
</d:DesignProperties.DataContext>
@@ -219,6 +221,21 @@
219221
</TextBlock.Style>
220222
</TextBlock>
221223

224+
<!-- Checkout disabled message -->
225+
<TextBlock Margin="0 4" TextWrapping="Wrap">
226+
<ui:OcticonImage Icon="alert" Foreground="Orange" Margin="0 0 0 -4"/>
227+
<Run Text="{Binding CheckoutDisabledMessage}"/>
228+
<TextBlock.Style>
229+
<Style TargetType="TextBlock">
230+
<Style.Triggers>
231+
<DataTrigger Binding="{Binding CheckoutDisabledMessage}" Value="{x:Null}">
232+
<Setter Property="Visibility" Value="Collapsed" />
233+
</DataTrigger>
234+
</Style.Triggers>
235+
</Style>
236+
</TextBlock.Style>
237+
</TextBlock>
238+
222239
<!-- Checkout error message -->
223240
<TextBlock Foreground="Red" Margin="0 4" Text="{Binding CheckoutError}" TextWrapping="Wrap">
224241
<TextBlock.Style>

src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public void DefaultLocalBranchNameShouldNotClashWithExistingBranchNames()
8282
Assert.Equal("pr/123-foo1-3", result);
8383
}
8484

85-
private static IGitService MockGitService()
85+
static IGitService MockGitService()
8686
{
8787
var repository = Substitute.For<IRepository>();
8888
var branches = MockBranches("pr/123-foo1", "pr/123-foo1-2");
@@ -94,7 +94,7 @@ private static IGitService MockGitService()
9494
}
9595
}
9696

97-
private static BranchCollection MockBranches(params string[] names)
97+
static BranchCollection MockBranches(params string[] names)
9898
{
9999
var result = Substitute.For<BranchCollection>();
100100

0 commit comments

Comments
 (0)