diff --git a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs index df50a548c4..665918f114 100644 --- a/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs +++ b/src/GitHub.App/ViewModels/RepositoryPublishViewModel.cs @@ -151,23 +151,19 @@ ReactiveCommand InitializePublishRepositoryCommand() return ReactiveCommand.CreateAsyncObservable(canCreate, OnPublishRepository); } - private IObservable OnPublishRepository(object arg) + IObservable OnPublishRepository(object arg) { var newRepository = GatherRepositoryInfo(); var account = SelectedAccount; return repositoryPublishService.PublishRepository(newRepository, account, SelectedHost.ApiClient) .SelectUnit() - .Do(_ => vsServices.ShowMessage("Repository published successfully.")) .Catch(ex => { + log.Error(ex); if (!ex.IsCriticalException()) - { - log.Error(ex); - var error = new PublishRepositoryUserError(ex.Message); - vsServices.ShowError((error.ErrorMessage + Environment.NewLine + error.ErrorCauseOrResolution).TrimEnd()); - } - return Observable.Return(Unit.Default); + ex = new UnhandledUserErrorException(new PublishRepositoryUserError(ex.Message)); + return Observable.Throw(ex); }); } diff --git a/src/GitHub.Exports/UI/IView.cs b/src/GitHub.Exports/UI/IView.cs index 5c7697e755..3320088d03 100644 --- a/src/GitHub.Exports/UI/IView.cs +++ b/src/GitHub.Exports/UI/IView.cs @@ -8,5 +8,6 @@ public interface IView IObservable Done { get; } IObservable Cancel { get; } IObservable IsBusy { get; } + IObservable Error { get; } } } diff --git a/src/GitHub.UI.Reactive/Controls/SimpleViewUserControl.cs b/src/GitHub.UI.Reactive/Controls/SimpleViewUserControl.cs index 8bb5b79ee8..afc1977cb2 100644 --- a/src/GitHub.UI.Reactive/Controls/SimpleViewUserControl.cs +++ b/src/GitHub.UI.Reactive/Controls/SimpleViewUserControl.cs @@ -18,6 +18,7 @@ public class SimpleViewUserControl : UserControl, IDisposable, IActivatable { readonly Subject close = new Subject(); readonly Subject cancel = new Subject(); + readonly Subject error = new Subject(); readonly Subject isBusy = new Subject(); public SimpleViewUserControl() @@ -39,6 +40,8 @@ public SimpleViewUserControl() public IObservable Cancel { get { return cancel; } } + public IObservable Error { get { return error; } } + public IObservable IsBusy{ get { return isBusy; } } protected void NotifyDone() @@ -53,6 +56,12 @@ protected void NotifyCancel() cancel.OnCompleted(); } + protected void NotifyError(string message) + { + error.OnNext(message); + error.OnCompleted(); + } + protected void NotifyIsBusy(bool busy) { isBusy.OnNext(busy); @@ -66,6 +75,7 @@ protected virtual void Dispose(bool disposing) if (disposed) return; close.Dispose(); + error.Dispose(); disposed = true; } } diff --git a/src/GitHub.VisualStudio/TeamExplorer/Sync/GitHubPublishSection.cs b/src/GitHub.VisualStudio/TeamExplorer/Sync/GitHubPublishSection.cs index 18e429571f..e6df0dfc8b 100644 --- a/src/GitHub.VisualStudio/TeamExplorer/Sync/GitHubPublishSection.cs +++ b/src/GitHub.VisualStudio/TeamExplorer/Sync/GitHubPublishSection.cs @@ -8,11 +8,11 @@ using GitHub.Models; using GitHub.Services; using GitHub.Info; -using ReactiveUI; using System.Reactive.Linq; using GitHub.Extensions; using GitHub.Api; -using GitHub.VisualStudio.TeamExplorer; +using System.Reactive.Disposables; +using System.Windows.Controls; namespace GitHub.VisualStudio.TeamExplorer.Sync { @@ -24,8 +24,9 @@ public class GitHubPublishSection : TeamExplorerSectionBase, IGitHubInvitationSe readonly Lazy lazyBrowser; readonly IRepositoryHosts hosts; - IDisposable disposable; + readonly CompositeDisposable disposables = new CompositeDisposable(); bool loggedIn; + readonly UserControl view; [ImportingConstructor] public GitHubPublishSection(ISimpleApiClientFactory apiFactory, ITeamExplorerServiceHolder holder, @@ -45,12 +46,12 @@ public GitHubPublishSection(ISimpleApiClientFactory apiFactory, ITeamExplorerSer ShowGetStarted = false; IsVisible = false; IsExpanded = true; - var view = new GitHubInvitationContent(); + view = new GitHubInvitationContent(); SectionContent = view; view.DataContext = this; } - async void RTMSetup() + async void Setup() { if (ActiveRepo != null && ActiveRepoUri == null) { @@ -64,39 +65,23 @@ async void RTMSetup() IsVisible = false; } - async void PreRTMSetup() - { - if (ActiveRepo != null && ActiveRepoUri == null) - { - IsVisible = true; - loggedIn = await connectionManager.IsLoggedIn(hosts); - if (loggedIn) - ShowPublish(); - else - { - ShowGetStarted = true; - ShowSignup = true; - } - } - else - IsVisible = false; - } - public override void Initialize(object sender, SectionInitializeEventArgs e) { base.Initialize(sender, e); - RTMSetup(); + Setup(); } protected override void RepoChanged() { base.RepoChanged(); - RTMSetup(); + Setup(); } public async void Connect() { loggedIn = await connectionManager.IsLoggedIn(hosts); + // we run the login on a separate UI flow because the login + // dialog is a modal dialog while the publish dialog is inlined in Team Explorer if (loggedIn) ShowPublish(); else @@ -128,18 +113,52 @@ void StartFlow(UIControllerFlow controllerFlow) void ShowPublish() { + // set the loading indicator while we prep the form IsBusy = true; + var uiProvider = ServiceProvider.GetExportedValue(); var factory = uiProvider.GetService(); var uiflow = factory.UIControllerFactory.CreateExport(); - disposable = uiflow; + disposables.Add(uiflow); var ui = uiflow.Value; var creation = ui.SelectFlow(UIControllerFlow.Publish); + var busyTracker = new SerialDisposable(); creation.Subscribe(c => { SectionContent = c; c.DataContext = this; - ((IView)c).IsBusy.Subscribe(x => IsBusy = x); + + var v = (IView)c; + busyTracker.Disposable = v.IsBusy.Subscribe(x => IsBusy = x); + disposables.Add(v.Error.Subscribe(x => + { + var vsServices = ServiceProvider.GetExportedValue(); + vsServices.ShowError(x as string); + })); + }, + () => + { + var vsServices = ServiceProvider.GetExportedValue(); + vsServices.ShowMessage("Repository published successfully."); + + var v = SectionContent as IView; + // the IsPublishing flag takes way too long to fire, don't wait for it, we're done + if (IsBusy) + { + // we only want to dispose things when all the events are processed (IsPublishing is the last one) + busyTracker.Disposable = v.IsBusy.Subscribe(_ => + { + disposables.Clear(); + busyTracker.Dispose(); + }); + IsBusy = false; + } + else + { + busyTracker.Dispose(); + disposables.Clear(); + } + SectionContent = view; }); ui.Start(null); } @@ -151,8 +170,7 @@ protected override void Dispose(bool disposing) { if (!disposed) { - if (disposable != null) - disposable.Dispose(); + disposables.Dispose(); disposed = true; } } diff --git a/src/GitHub.VisualStudio/UI/Views/Controls/RepositoryPublishControl.xaml.cs b/src/GitHub.VisualStudio/UI/Views/Controls/RepositoryPublishControl.xaml.cs index 84249b2f60..74da35a998 100644 --- a/src/GitHub.VisualStudio/UI/Views/Controls/RepositoryPublishControl.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/Controls/RepositoryPublishControl.xaml.cs @@ -42,10 +42,17 @@ public RepositoryPublishControl() d(this.OneWayBind(ViewModel, vm => vm.IsPublishing, v => v.description.IsEnabled, x => x == false)); d(this.OneWayBind(ViewModel, vm => vm.IsPublishing, v => v.accountsComboBox.IsEnabled, x => x == false)); - ViewModel.PublishRepository.Subscribe(_ => NotifyDone()); + d(ViewModel.PublishRepository.Subscribe(_ => NotifyDone())); + d(ViewModel.PublishRepository.ThrownExceptions.Subscribe(ex => + { + var error = ex as UnhandledUserErrorException; + if (error == null) + NotifyError("Error publishing repository." + Environment.NewLine + ex.Message); + else + NotifyError((error.ReportedError.ErrorMessage + Environment.NewLine + error.ReportedError.ErrorCauseOrResolution).TrimEnd()); + })); - d(this.WhenAny(x => x.ViewModel.IsPublishing, x => x.Value) - .Subscribe(x => NotifyIsBusy(x))); + d(this.WhenAny(x => x.ViewModel.IsPublishing, x => x.Value).Subscribe(x => NotifyIsBusy(x))); nameText.Text = ViewModel.DefaultRepositoryName; }); diff --git a/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs b/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs index afc02776e0..979367b8bc 100644 --- a/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs +++ b/src/UnitTests/GitHub.App/ViewModels/RepositoryPublishViewModelTests.cs @@ -339,10 +339,10 @@ public async Task DisplaysSuccessMessageWhenCompletedWithoutError() vm.RepositoryName = "repo-name"; + var obs = Substitute.For>(); + vm.PublishRepository.ThrownExceptions.Subscribe(obs); await vm.PublishRepository.ExecuteAsync().Catch(Observable.Return(Unit.Default)); - - vsServices.Received().ShowMessage("Repository published successfully."); - vsServices.DidNotReceive().ShowError(Args.String); + obs.DidNotReceiveWithAnyArgs().OnNext(null); } [Fact] @@ -358,10 +358,11 @@ public async Task DisplaysRepositoryExistsErrorWithVisualStudioNotifications() var vm = Helpers.SetupConnectionsAndViewModel(hosts, repositoryPublishService, vsServices, cm); vm.RepositoryName = "repo-name"; + var obs = Substitute.For>(); + vm.PublishRepository.ThrownExceptions.Subscribe(obs); await vm.PublishRepository.ExecuteAsync().Catch(Observable.Return(Unit.Default)); - - vsServices.DidNotReceive().ShowMessage(Args.String); - vsServices.Received().ShowError("There is already a repository named 'repo-name' for the current account."); + obs.ReceivedWithAnyArgs().OnNext(null); + Assert.True(obs.ReceivedCalls().Any(x => ((UnhandledUserErrorException)x.GetArguments()[0]).Message == "There is already a repository named 'repo-name' for the current account.")); } [Fact] @@ -391,6 +392,7 @@ public async Task ClearsErrorsWhenSwitchingHosts() vm.RepositoryName = "repo-name"; + vm.PublishRepository.ThrownExceptions.Subscribe(); await vm.PublishRepository.ExecuteAsync().Catch(Observable.Return(Unit.Default)); vm.SelectedConnection = conns.First(x => x != vm.SelectedConnection); @@ -422,6 +424,7 @@ public async Task ClearsErrorsWhenSwitchingAccounts() vm.RepositoryName = "repo-name"; + vm.PublishRepository.ThrownExceptions.Subscribe(); await vm.PublishRepository.ExecuteAsync().Catch(Observable.Return(Unit.Default)); vm.SelectedAccount = accounts[1];