diff --git a/src/GitHub.App/GitHub.App.csproj b/src/GitHub.App/GitHub.App.csproj index 3e9bff0d12..46cd1945a0 100644 --- a/src/GitHub.App/GitHub.App.csproj +++ b/src/GitHub.App/GitHub.App.csproj @@ -148,6 +148,10 @@ ..\..\packages\Microsoft.VisualStudio.Utilities.14.3.25407\lib\net45\Microsoft.VisualStudio.Utilities.dll True + + ..\..\packages\Microsoft.VisualStudio.Utilities.14.3.25407\lib\net45\Microsoft.VisualStudio.Utilities.dll + True + @@ -236,6 +240,7 @@ + diff --git a/src/GitHub.App/Models/Account.cs b/src/GitHub.App/Models/Account.cs index b2b07462aa..050871f6fa 100644 --- a/src/GitHub.App/Models/Account.cs +++ b/src/GitHub.App/Models/Account.cs @@ -88,7 +88,7 @@ public BitmapSource Avatar set { avatar = value; this.RaisePropertyChanged(); } } -#region Equality things + #region Equality things public void CopyFrom(IAccount other) { if (!Equals(other)) diff --git a/src/GitHub.App/Resources.Designer.cs b/src/GitHub.App/Resources.Designer.cs index d658cc788d..5851e6a658 100644 --- a/src/GitHub.App/Resources.Designer.cs +++ b/src/GitHub.App/Resources.Designer.cs @@ -69,6 +69,15 @@ internal static string AddedFileStatus { } } + /// + /// Looks up a localized string similar to Approved. + /// + internal static string Approved { + get { + return ResourceManager.GetString("Approved", resourceCulture); + } + } + /// /// Looks up a localized string similar to Select a containing folder for your new repository.. /// @@ -78,6 +87,15 @@ internal static string BrowseForDirectory { } } + /// + /// Looks up a localized string similar to Changes Requested. + /// + internal static string ChangesRequested { + get { + return ResourceManager.GetString("ChangesRequested", resourceCulture); + } + } + /// /// Looks up a localized string similar to Clone a {0} Repository. /// @@ -87,6 +105,15 @@ internal static string CloneTitle { } } + /// + /// Looks up a localized string similar to Commented. + /// + internal static string Commented { + get { + return ResourceManager.GetString("Commented", resourceCulture); + } + } + /// /// Looks up a localized string similar to Could not connect to github.com. /// @@ -180,6 +207,15 @@ internal static string Fork { } } + /// + /// Looks up a localized string similar to InProgress. + /// + internal static string InProgress { + get { + return ResourceManager.GetString("InProgress", resourceCulture); + } + } + /// /// Looks up a localized string similar to [invalid]. /// diff --git a/src/GitHub.App/Resources.resx b/src/GitHub.App/Resources.resx index 7fa2be0265..153a52be18 100644 --- a/src/GitHub.App/Resources.resx +++ b/src/GitHub.App/Resources.resx @@ -294,4 +294,16 @@ Please install Git for Windows from: https://git-scm.com/download/win + + Approved + + + Changes Requested + + + Commented + + + InProgress + \ No newline at end of file diff --git a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs index 63f163f42a..387bfeda66 100644 --- a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs @@ -67,6 +67,34 @@ public PullRequestDetailViewModelDesigner() modelsDir.Files.Add(oldBranchModel); gitHubDir.Directories.Add(modelsDir); + Reviews = new[] + { + new PullRequestReviewSummaryViewModel + { + Id = 2, + User = new AccountDesigner { Login = "grokys", IsUser = true }, + State = PullRequestReviewState.Pending, + FileCommentCount = 0, + }, + new PullRequestReviewSummaryViewModel + { + Id = 1, + User = new AccountDesigner { Login = "jcansdale", IsUser = true }, + State = PullRequestReviewState.Approved, + FileCommentCount = 5, + }, + new PullRequestReviewSummaryViewModel + { + Id = 2, + User = new AccountDesigner { Login = "shana", IsUser = true }, + State = PullRequestReviewState.ChangesRequested, + FileCommentCount = 5, + }, + new PullRequestReviewSummaryViewModel + { + }, + }; + Files = new PullRequestFilesViewModelDesigner(); } @@ -81,6 +109,7 @@ public PullRequestDetailViewModelDesigner() public bool IsCheckedOut { get; } public bool IsFromFork { get; } public string Body { get; } + public IReadOnlyList Reviews { get; } public IPullRequestFilesViewModel Files { get; set; } public IPullRequestCheckoutState CheckoutState { get; set; } public IPullRequestUpdateState UpdateState { get; set; } @@ -92,6 +121,7 @@ public PullRequestDetailViewModelDesigner() public ReactiveCommand Pull { get; } public ReactiveCommand Push { get; } public ReactiveCommand OpenOnGitHub { get; } + public ReactiveCommand ShowReview { get; } public Task InitializeAsync(ILocalRepositoryModel localRepository, IConnection connection, string owner, string repo, int number) => Task.CompletedTask; diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs index f75616cc16..145493c5ee 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModel.cs @@ -42,6 +42,7 @@ public sealed class PullRequestDetailViewModel : PanePageViewModelBase, IPullReq string targetBranchDisplayName; int commentCount; string body; + IReadOnlyList reviews; IPullRequestCheckoutState checkoutState; IPullRequestUpdateState updateState; string operationError; @@ -117,6 +118,7 @@ public PullRequestDetailViewModel( SubscribeOperationError(SyncSubmodules); OpenOnGitHub = ReactiveCommand.Create(); + ShowReview = ReactiveCommand.Create().OnExecuteCompleted(DoShowReview); } /// @@ -244,6 +246,15 @@ public string OperationError private set { this.RaiseAndSetIfChanged(ref operationError, value); } } + /// + /// Gets the latest pull request review for each user. + /// + public IReadOnlyList Reviews + { + get { return reviews; } + private set { this.RaiseAndSetIfChanged(ref reviews, value); } + } + /// /// Gets the pull request's changed files. /// @@ -283,6 +294,11 @@ public Uri WebUrl /// public ReactiveCommand OpenOnGitHub { get; } + /// + /// Gets a command that navigates to a pull request review. + /// + public ReactiveCommand ShowReview { get; } + /// /// Initializes the view model. /// @@ -353,6 +369,8 @@ public async Task Load(IPullRequestModel pullRequest) TargetBranchDisplayName = GetBranchDisplayName(IsFromFork, pullRequest.Base?.Label); CommentCount = pullRequest.Comments.Count + pullRequest.ReviewComments.Count; Body = !string.IsNullOrWhiteSpace(pullRequest.Body) ? pullRequest.Body : Resources.NoDescriptionProvidedMarkdown; + Reviews = PullRequestReviewSummaryViewModel.BuildByUser(Session.User, pullRequest).ToList(); + await Files.InitializeAsync(Session); var localBranches = await pullRequestsService.GetLocalBranches(LocalRepository, pullRequest).ToList(); @@ -616,6 +634,11 @@ async Task DoSyncSubmodules(object unused) } } + void DoShowReview(object item) + { + // TODO + } + class CheckoutCommandState : IPullRequestCheckoutState { public CheckoutCommandState(string caption, string disabledMessage) diff --git a/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewSummaryViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewSummaryViewModel.cs new file mode 100644 index 0000000000..b321b35bbe --- /dev/null +++ b/src/GitHub.App/ViewModels/GitHubPane/PullRequestReviewSummaryViewModel.cs @@ -0,0 +1,122 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using GitHub.App; +using GitHub.Models; + +namespace GitHub.ViewModels.GitHubPane +{ + /// + /// Displays a short overview of a pull request review in the . + /// + public class PullRequestReviewSummaryViewModel : IPullRequestReviewSummaryViewModel + { + /// + public long Id { get; set; } + + /// + public IAccount User { get; set; } + + /// + public PullRequestReviewState State { get; set; } + + /// + public string StateDisplay => ToString(State); + + /// + public int FileCommentCount { get; set; } + + /// + /// Builds a collection of s by user. + /// + /// The current user. + /// The pull request model. + /// + /// This method builds a list similar to that found in the "Reviewers" section at the top- + /// right of the Pull Request page on GitHub. + /// + public static IEnumerable BuildByUser( + IAccount currentUser, + IPullRequestModel pullRequest) + { + var existing = new Dictionary(); + + foreach (var review in pullRequest.Reviews.OrderBy(x => x.Id)) + { + if (review.State == PullRequestReviewState.Pending && review.User.Login != currentUser.Login) + continue; + + PullRequestReviewSummaryViewModel previous; + existing.TryGetValue(review.User.Login, out previous); + + var previousPriority = ToPriority(previous); + var reviewPriority = ToPriority(review.State); + + if (reviewPriority >= previousPriority) + { + var count = pullRequest.ReviewComments + .Where(x => x.PullRequestReviewId == review.Id) + .Count(); + existing[review.User.Login] = new PullRequestReviewSummaryViewModel + { + Id = review.Id, + User = review.User, + State = review.State, + FileCommentCount = count + }; + } + } + + var result = existing.Values.OrderBy(x => x.User).AsEnumerable(); + + if (!result.Any(x => x.State == PullRequestReviewState.Pending)) + { + var newReview = new PullRequestReviewSummaryViewModel + { + State = PullRequestReviewState.Pending, + User = currentUser, + }; + result = result.Concat(new[] { newReview }); + } + + return result; + } + + static int ToPriority(PullRequestReviewSummaryViewModel review) + { + return review != null ? ToPriority(review.State) : 0; + } + + static int ToPriority(PullRequestReviewState state) + { + switch (state) + { + case PullRequestReviewState.Approved: + case PullRequestReviewState.ChangesRequested: + return 1; + case PullRequestReviewState.Pending: + return 2; + default: + return 0; + } + } + + static string ToString(PullRequestReviewState state) + { + switch (state) + { + case PullRequestReviewState.Approved: + return Resources.Approved; + case PullRequestReviewState.ChangesRequested: + return Resources.ChangesRequested; + case PullRequestReviewState.Commented: + case PullRequestReviewState.Dismissed: + return Resources.Commented; + case PullRequestReviewState.Pending: + return Resources.InProgress; + default: + throw new NotSupportedException(); + } + } + } +} diff --git a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj index f653adf5dc..07a1464237 100644 --- a/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj +++ b/src/GitHub.Exports.Reactive/GitHub.Exports.Reactive.csproj @@ -197,6 +197,7 @@ + diff --git a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs index 6d6a4262ac..fa091605d7 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestDetailViewModel.cs @@ -125,6 +125,11 @@ public interface IPullRequestDetailViewModel : IPanePageViewModel, IOpenInBrowse /// string Body { get; } + /// + /// Gets the latest pull request review for each user. + /// + IReadOnlyList Reviews { get; } + /// /// Gets the pull request's changed files. /// @@ -165,6 +170,11 @@ public interface IPullRequestDetailViewModel : IPanePageViewModel, IOpenInBrowse /// ReactiveCommand OpenOnGitHub { get; } + /// + /// Gets a command that navigates to a pull request review. + /// + ReactiveCommand ShowReview { get; } + /// /// Initializes the view model. /// diff --git a/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestReviewSummaryViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestReviewSummaryViewModel.cs new file mode 100644 index 0000000000..178a1d8d0f --- /dev/null +++ b/src/GitHub.Exports.Reactive/ViewModels/GitHubPane/IPullRequestReviewSummaryViewModel.cs @@ -0,0 +1,35 @@ +using GitHub.Models; + +namespace GitHub.ViewModels.GitHubPane +{ + /// + /// Displays a short overview of a pull request review in the . + /// + public interface IPullRequestReviewSummaryViewModel + { + /// + /// Gets the ID of the pull request review. + /// + long Id { get; set; } + + /// + /// Gets the user who submitted the review. + /// + IAccount User { get; set; } + + /// + /// Gets the state of the review. + /// + PullRequestReviewState State { get; set; } + + /// + /// Gets a string representing the state of the review. + /// + string StateDisplay { get; } + + /// + /// Gets the number of file comments in the review. + /// + int FileCommentCount { get; set; } + } +} \ No newline at end of file diff --git a/src/GitHub.UI/Converters/NotEqualsToVisibilityConverter.cs b/src/GitHub.UI/Converters/NotEqualsToVisibilityConverter.cs new file mode 100644 index 0000000000..42d186302b --- /dev/null +++ b/src/GitHub.UI/Converters/NotEqualsToVisibilityConverter.cs @@ -0,0 +1,30 @@ +using System; +using System.Globalization; +using System.Windows; +using System.Windows.Data; +using System.Windows.Markup; + +namespace GitHub.UI +{ + public class NotEqualsToVisibilityConverter : MarkupExtension, IValueConverter + { + readonly string collapsedValue; + + public NotEqualsToVisibilityConverter(string collapsedValue) + { + this.collapsedValue = collapsedValue; + } + + public object Convert(object value, Type targetType, object parameter, CultureInfo culture) + { + return value?.ToString() != collapsedValue ? Visibility.Visible : Visibility.Collapsed; + } + + public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture) + { + throw new NotImplementedException(); + } + + public override object ProvideValue(IServiceProvider serviceProvider) => this; + } +} diff --git a/src/GitHub.UI/GitHub.UI.csproj b/src/GitHub.UI/GitHub.UI.csproj index ed3bbb281e..6fcc20e64a 100644 --- a/src/GitHub.UI/GitHub.UI.csproj +++ b/src/GitHub.UI/GitHub.UI.csproj @@ -92,6 +92,7 @@ + diff --git a/src/GitHub.VisualStudio.UI/Resources.Designer.cs b/src/GitHub.VisualStudio.UI/Resources.Designer.cs index c69eb5a5c3..c1461c477f 100644 --- a/src/GitHub.VisualStudio.UI/Resources.Designer.cs +++ b/src/GitHub.VisualStudio.UI/Resources.Designer.cs @@ -60,6 +60,15 @@ internal Resources() { } } + /// + /// Looks up a localized string similar to Add your review. + /// + public static string AddYourReview { + get { + return ResourceManager.GetString("AddYourReview", resourceCulture); + } + } + /// /// Looks up a localized string similar to Invalid authentication code. /// @@ -150,6 +159,15 @@ public static string CompareFileAsDefaultAction { } } + /// + /// Looks up a localized string similar to Continue your review. + /// + public static string ContinueYourReview { + get { + return ResourceManager.GetString("ContinueYourReview", resourceCulture); + } + } + /// /// Looks up a localized string similar to Could not connect to github.com. /// @@ -753,6 +771,15 @@ public static string resendCodeButtonToolTip { } } + /// + /// Looks up a localized string similar to Reviewers. + /// + public static string Reviewers { + get { + return ResourceManager.GetString("Reviewers", resourceCulture); + } + } + /// /// Looks up a localized string similar to Sign in.... /// diff --git a/src/GitHub.VisualStudio.UI/Resources.resx b/src/GitHub.VisualStudio.UI/Resources.resx index 9f7b318db5..6bc1e1e436 100644 --- a/src/GitHub.VisualStudio.UI/Resources.resx +++ b/src/GitHub.VisualStudio.UI/Resources.resx @@ -401,4 +401,13 @@ Token + + Continue your review + + + Add your review + + + Reviewers + \ No newline at end of file diff --git a/src/GitHub.VisualStudio.UI/Styles/ThemeBlue.xaml b/src/GitHub.VisualStudio.UI/Styles/ThemeBlue.xaml index f2a8b1f284..215464137f 100644 --- a/src/GitHub.VisualStudio.UI/Styles/ThemeBlue.xaml +++ b/src/GitHub.VisualStudio.UI/Styles/ThemeBlue.xaml @@ -63,5 +63,6 @@ + \ No newline at end of file diff --git a/src/GitHub.VisualStudio.UI/Styles/ThemeDark.xaml b/src/GitHub.VisualStudio.UI/Styles/ThemeDark.xaml index 2a2dd5f175..47ec2ac1f9 100644 --- a/src/GitHub.VisualStudio.UI/Styles/ThemeDark.xaml +++ b/src/GitHub.VisualStudio.UI/Styles/ThemeDark.xaml @@ -63,5 +63,6 @@ + \ No newline at end of file diff --git a/src/GitHub.VisualStudio.UI/Styles/ThemeLight.xaml b/src/GitHub.VisualStudio.UI/Styles/ThemeLight.xaml index cbaa1fa55d..65d7a28d90 100644 --- a/src/GitHub.VisualStudio.UI/Styles/ThemeLight.xaml +++ b/src/GitHub.VisualStudio.UI/Styles/ThemeLight.xaml @@ -63,5 +63,6 @@ + \ No newline at end of file diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index acf8349a34..ac1602410c 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -382,6 +382,9 @@ GitHubPaneView.xaml + + PullRequestReviewSummaryView.xaml + PullRequestFilesView.xaml @@ -537,6 +540,10 @@ Designer MSBuild:Compile + + MSBuild:Compile + Designer + Designer MSBuild:Compile diff --git a/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestDetailView.xaml b/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestDetailView.xaml index 00976c6ee0..85419b6c70 100644 --- a/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestDetailView.xaml +++ b/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestDetailView.xaml @@ -20,11 +20,6 @@ - @@ -80,7 +75,7 @@ - + @@ -88,17 +83,49 @@ + + + + + - + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -116,83 +143,94 @@ - - - - - - - - + + - - - - + + + - + - - - - + + + + + - - - - - - + + + + + + + + + + + + - + + + View on GitHub + - + + Content="{Binding CheckoutState.Caption}" + VerticalAlignment="Center" + TextTrimming="CharacterEllipsis" + Visibility="{Binding CheckoutState, Converter={ghfvs:NullToVisibilityConverter}}" + ToolTip="{Binding CheckoutState.ToolTip}" + ToolTipService.ShowOnDisabled="True"/> - + + Command="{Binding Pull}" + Margin="4,0" + ToolTip="{Binding UpdateState.PullToolTip}" + ToolTipService.ShowOnDisabled="True" + VerticalAlignment="Center"/> + Command="{Binding Push}" + Margin="4,0" + ToolTip="{Binding UpdateState.PushToolTip}" + ToolTipService.ShowOnDisabled="True" + VerticalAlignment="Center"/> + Visibility="{Binding UpdateState.SyncSubmodulesEnabled, FallbackValue=Collapsed, Converter={ghfvs:BooleanToVisibilityConverter}}" /> + Visibility="{Binding UpdateState.SyncSubmodulesEnabled, FallbackValue=Collapsed, Converter={ghfvs:BooleanToVisibilityConverter}}" /> + Command="{Binding SyncSubmodules}" + Margin="4 0" + ToolTip="{Binding UpdateState.SyncSubmodulesToolTip}" + Visibility="{Binding UpdateState.SyncSubmodulesEnabled, FallbackValue=Collapsed, Converter={ghfvs:BooleanToVisibilityConverter}}" /> - + - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + - - + + + + + + - - - View conversation on GitHub - + + + + + + + + + + + + + + + + + + + + Margin="0 8 10 0"> - + \ No newline at end of file diff --git a/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestReviewSummaryView.xaml b/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestReviewSummaryView.xaml new file mode 100644 index 0000000000..f3805f77fb --- /dev/null +++ b/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestReviewSummaryView.xaml @@ -0,0 +1,87 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestReviewSummaryView.xaml.cs b/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestReviewSummaryView.xaml.cs new file mode 100644 index 0000000000..a2c397b589 --- /dev/null +++ b/src/GitHub.VisualStudio/Views/GitHubPane/PullRequestReviewSummaryView.xaml.cs @@ -0,0 +1,12 @@ +using System.Windows.Controls; + +namespace GitHub.VisualStudio.Views.GitHubPane +{ + public partial class PullRequestReviewSummaryView : UserControl + { + public PullRequestReviewSummaryView() + { + InitializeComponent(); + } + } +} diff --git a/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs b/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs index a344f1499a..b3c216944e 100644 --- a/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs +++ b/test/UnitTests/GitHub.App/ViewModels/GitHubPane/PullRequestDetailViewModelTests.cs @@ -1,6 +1,8 @@ using System; +using System.Collections.Generic; using System.ComponentModel; using System.IO; +using System.Linq; using System.Reactive; using System.Reactive.Linq; using System.Threading.Tasks; @@ -15,7 +17,7 @@ namespace UnitTests.GitHub.App.ViewModels.GitHubPane { - public class PullRequestDetailViewModelTests : TestBaseClass + public class PullRequestDetailViewModelTests { static readonly Uri Uri = new Uri("http://foo"); @@ -26,19 +28,19 @@ public async Task ShouldUsePlaceholderBodyIfNoneExists() { var target = CreateTarget(); - await target.Load(CreatePullRequest(body: string.Empty)); + await target.Load(CreatePullRequestModel(body: string.Empty)); Assert.That("*No description provided.*", Is.EqualTo(target.Body)); } } - public class TheHeadProperty + public class TheHeadProperty : TestBaseClass { [Test] public async Task ShouldAcceptNullHead() { var target = CreateTarget(); - var model = CreatePullRequest(); + var model = CreatePullRequestModel(); // PullRequest.Head can be null for example if a user deletes the repository after creating the PR. model.Head = null; @@ -49,7 +51,112 @@ public async Task ShouldAcceptNullHead() } } - public class TheCheckoutCommand + public class TheReviewsProperty : TestBaseClass + { + [Test] + public async Task ShouldShowLatestAcceptedOrChangesRequestedReview() + { + var target = CreateTarget(); + var model = CreatePullRequestModel( + CreatePullRequestReviewModel(1, "grokys", PullRequestReviewState.ChangesRequested), + CreatePullRequestReviewModel(2, "shana", PullRequestReviewState.ChangesRequested), + CreatePullRequestReviewModel(3, "grokys", PullRequestReviewState.Approved), + CreatePullRequestReviewModel(4, "grokys", PullRequestReviewState.Commented)); + + await target.Load(model); + + Assert.That(target.Reviews, Has.Count.EqualTo(3)); + Assert.That(target.Reviews[0].User.Login, Is.EqualTo("grokys")); + Assert.That(target.Reviews[1].User.Login, Is.EqualTo("shana")); + Assert.That(target.Reviews[2].User.Login, Is.EqualTo("grokys")); + Assert.That(target.Reviews[0].Id, Is.EqualTo(3)); + Assert.That(target.Reviews[1].Id, Is.EqualTo(2)); + Assert.That(target.Reviews[2].Id, Is.EqualTo(0)); + } + + [Test] + public async Task ShouldShowLatestCommentedReviewIfNothingElsePresent() + { + var target = CreateTarget(); + var model = CreatePullRequestModel( + CreatePullRequestReviewModel(1, "shana", PullRequestReviewState.Commented), + CreatePullRequestReviewModel(2, "shana", PullRequestReviewState.Commented)); + + await target.Load(model); + + Assert.That(target.Reviews, Has.Count.EqualTo(2)); + Assert.That(target.Reviews[0].User.Login, Is.EqualTo("shana")); + Assert.That(target.Reviews[1].User.Login, Is.EqualTo("grokys")); + Assert.That(target.Reviews[0].Id, Is.EqualTo(2)); + } + + [Test] + public async Task ShouldNotShowStartNewReviewWhenHasPendingReview() + { + var target = CreateTarget(); + var model = CreatePullRequestModel( + CreatePullRequestReviewModel(1, "grokys", PullRequestReviewState.Pending)); + + await target.Load(model); + + Assert.That(target.Reviews, Has.Count.EqualTo(1)); + Assert.That(target.Reviews[0].User.Login, Is.EqualTo("grokys")); + Assert.That(target.Reviews[0].Id, Is.EqualTo(1)); + } + + [Test] + public async Task ShouldShowPendingReviewOverApproved() + { + var target = CreateTarget(); + var model = CreatePullRequestModel( + CreatePullRequestReviewModel(1, "grokys", PullRequestReviewState.Approved), + CreatePullRequestReviewModel(2, "grokys", PullRequestReviewState.Pending)); + + await target.Load(model); + + Assert.That(target.Reviews, Has.Count.EqualTo(1)); + Assert.That(target.Reviews[0].User.Login, Is.EqualTo("grokys")); + Assert.That(target.Reviews[0].Id, Is.EqualTo(2)); + } + + [Test] + public async Task ShouldNotShowPendingReviewForOtherUser() + { + var target = CreateTarget(); + var model = CreatePullRequestModel( + CreatePullRequestReviewModel(1, "shana", PullRequestReviewState.Pending)); + + await target.Load(model); + + Assert.That(target.Reviews, Has.Count.EqualTo(1)); + Assert.That(target.Reviews[0].User.Login, Is.EqualTo("grokys")); + Assert.That(target.Reviews[0].Id, Is.EqualTo(0)); + } + + static PullRequestModel CreatePullRequestModel( + params IPullRequestReviewModel[] reviews) + { + return PullRequestDetailViewModelTests.CreatePullRequestModel(reviews: reviews); + } + + static PullRequestReviewModel CreatePullRequestReviewModel( + long id, + string login, + PullRequestReviewState state) + { + var account = Substitute.For(); + account.Login.Returns(login); + + return new PullRequestReviewModel + { + Id = id, + User = account, + State = state, + }; + } + } + + public class TheCheckoutCommand : TestBaseClass { [Test] public async Task CheckedOutAndUpToDate() @@ -58,7 +165,7 @@ public async Task CheckedOutAndUpToDate() currentBranch: "pr/123", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Checkout.CanExecute(null)); Assert.That(target.CheckoutState, Is.Null); @@ -71,7 +178,7 @@ public async Task NotCheckedOut() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Checkout.CanExecute(null)); Assert.True(target.CheckoutState.IsEnabled); @@ -86,7 +193,7 @@ public async Task NotCheckedOutWithWorkingDirectoryDirty() existingPrBranch: "pr/123", dirty: true); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Checkout.CanExecute(null)); Assert.That("Cannot checkout as your working directory has uncommitted changes.", Is.EqualTo(target.CheckoutState.ToolTip)); @@ -99,7 +206,7 @@ public async Task CheckoutExistingLocalBranch() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest(number: 123)); + await target.Load(CreatePullRequestModel(number: 123)); Assert.True(target.Checkout.CanExecute(null)); Assert.That("Checkout pr/123", Is.EqualTo(target.CheckoutState.Caption)); @@ -111,7 +218,7 @@ public async Task CheckoutNonExistingLocalBranch() var target = CreateTarget( currentBranch: "master"); - await target.Load(CreatePullRequest(number: 123)); + await target.Load(CreatePullRequestModel(number: 123)); Assert.True(target.Checkout.CanExecute(null)); Assert.That("Checkout to pr/123", Is.EqualTo(target.CheckoutState.Caption)); @@ -123,7 +230,7 @@ public async Task UpdatesOperationErrorWithExceptionMessage() var target = CreateTarget( currentBranch: "master", existingPrBranch: "pr/123"); - var pr = CreatePullRequest(); + var pr = CreatePullRequestModel(); pr.Head = new GitReferenceModel("source", null, "sha", (string)null); @@ -140,7 +247,7 @@ public async Task SetsOperationErrorOnCheckoutFailure() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Checkout.CanExecute(null)); @@ -156,7 +263,7 @@ public async Task ClearsOperationErrorOnCheckoutSuccess() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Checkout.CanExecute(null)); Assert.ThrowsAsync(async () => await target.Checkout.ExecuteAsyncTask()); @@ -173,7 +280,7 @@ public async Task ClearsOperationErrorOnCheckoutRefresh() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Checkout.CanExecute(null)); Assert.ThrowsAsync(async () => await target.Checkout.ExecuteAsyncTask()); @@ -184,7 +291,7 @@ public async Task ClearsOperationErrorOnCheckoutRefresh() } } - public class ThePullCommand + public class ThePullCommand : TestBaseClass { [Test] public async Task NotCheckedOut() @@ -193,7 +300,7 @@ public async Task NotCheckedOut() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Pull.CanExecute(null)); Assert.That(target.UpdateState, Is.Null); @@ -206,7 +313,7 @@ public async Task CheckedOutAndUpToDate() currentBranch: "pr/123", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Pull.CanExecute(null)); Assert.That(0, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -222,7 +329,7 @@ public async Task CheckedOutAndBehind() existingPrBranch: "pr/123", behindBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Pull.CanExecute(null)); Assert.That(0, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -239,7 +346,7 @@ public async Task CheckedOutAndAheadAndBehind() aheadBy: 3, behindBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Pull.CanExecute(null)); Assert.That(3, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -256,7 +363,7 @@ public async Task CheckedOutAndBehindFork() prFromFork: true, behindBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Pull.CanExecute(null)); Assert.That(0, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -271,14 +378,14 @@ public async Task UpdatesOperationErrorWithExceptionMessage() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.ThrowsAsync(() => target.Pull.ExecuteAsyncTask(null)); Assert.That("Pull threw", Is.EqualTo(target.OperationError)); } } - public class ThePushCommand + public class ThePushCommand : TestBaseClass { [Test] public async Task NotCheckedOut() @@ -287,7 +394,7 @@ public async Task NotCheckedOut() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Push.CanExecute(null)); Assert.That(target.UpdateState, Is.Null); @@ -300,7 +407,7 @@ public async Task CheckedOutAndUpToDate() currentBranch: "pr/123", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Push.CanExecute(null)); Assert.That(0, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -316,7 +423,7 @@ public async Task CheckedOutAndAhead() existingPrBranch: "pr/123", aheadBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Push.CanExecute(null)); Assert.That(2, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -332,7 +439,7 @@ public async Task CheckedOutAndBehind() existingPrBranch: "pr/123", behindBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Push.CanExecute(null)); Assert.That(0, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -349,7 +456,7 @@ public async Task CheckedOutAndAheadAndBehind() aheadBy: 3, behindBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.False(target.Push.CanExecute(null)); Assert.That(3, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -366,7 +473,7 @@ public async Task CheckedOutAndAheadOfFork() prFromFork: true, aheadBy: 2); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.True(target.Push.CanExecute(null)); Assert.That(2, Is.EqualTo(target.UpdateState.CommitsAhead)); @@ -381,7 +488,7 @@ public async Task UpdatesOperationErrorWithExceptionMessage() currentBranch: "master", existingPrBranch: "pr/123"); - await target.Load(CreatePullRequest()); + await target.Load(CreatePullRequestModel()); Assert.ThrowsAsync(() => target.Push.ExecuteAsyncTask(null)); Assert.That("Push threw", Is.EqualTo(target.OperationError)); @@ -454,9 +561,19 @@ static Tuple CreateTargetAndSer pullRequestService.CalculateHistoryDivergence(repository, Arg.Any()) .Returns(Observable.Return(divergence)); + if (sessionManager == null) + { + var currentSession = Substitute.For(); + currentSession.User.Login.Returns("grokys"); + + sessionManager = Substitute.For(); + sessionManager.CurrentSession.Returns(currentSession); + sessionManager.GetSession(null).ReturnsForAnyArgs(currentSession); + } + var vm = new PullRequestDetailViewModel( pullRequestService, - sessionManager ?? Substitute.For(), + sessionManager, Substitute.For(), Substitute.For(), Substitute.For(), @@ -467,16 +584,22 @@ static Tuple CreateTargetAndSer return Tuple.Create(vm, pullRequestService); } - static PullRequestModel CreatePullRequest(int number = 1, string body = "PR Body") + static PullRequestModel CreatePullRequestModel( + int number = 1, + string body = "PR Body", + IEnumerable reviews = null) { var author = Substitute.For(); + reviews = reviews ?? new IPullRequestReviewModel[0]; + return new PullRequestModel(number, "PR 1", author, DateTimeOffset.Now) { State = PullRequestStateEnum.Open, Body = string.Empty, Head = new GitReferenceModel("source", "foo:baz", "sha", "https://github.com/foo/bar.git"), Base = new GitReferenceModel("dest", "foo:bar", "sha", "https://github.com/foo/bar.git"), + Reviews = reviews.ToList(), }; }