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

Commit 0cef013

Browse files
committed
Make sure an IConnection is always registered
Fix a bug where if we're logged in but don't pass a connection to Start, no connection object would be passed along to whatever viewmodel we're loading, which is a Bad Thing(tm). If we're logged in but no connection object is passed in, we should retrieve the first available logged-in connection from the connections list and pass that one in, so that viewmodels *always* get a connection.
1 parent 82529e6 commit 0cef013

File tree

2 files changed

+52
-9
lines changed

2 files changed

+52
-9
lines changed

src/GitHub.App/Controllers/UIController.cs

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
using ReactiveUI;
1919
using Stateless;
2020
using System.Collections.Specialized;
21+
using System.Linq;
2122

2223
namespace GitHub.Controllers
2324
{
@@ -226,7 +227,11 @@ public void Start([AllowNull] IConnection connection)
226227
{
227228
if (connection != null)
228229
{
229-
uiProvider.AddService(typeof(IConnection), connection);
230+
if (currentFlow != UIControllerFlow.Authentication)
231+
uiProvider.AddService(connection);
232+
else // sanity check: it makes zero sense to pass a connection in when calling the auth flow
233+
Debug.Assert(false, "Calling the auth flow with a connection makes no sense!");
234+
230235
connection.Login()
231236
.Select(c => hosts.LookupHost(connection.HostAddress))
232237
.Do(host =>
@@ -253,13 +258,21 @@ public void Start([AllowNull] IConnection connection)
253258
.IsLoggedIn(hosts)
254259
.Do(loggedin =>
255260
{
256-
if (!loggedin && currentFlow != UIControllerFlow.Authentication)
261+
if (currentFlow != UIControllerFlow.Authentication)
257262
{
258-
connectionAdded = (s, e) => {
259-
if (e.Action == NotifyCollectionChangedAction.Add)
260-
uiProvider.AddService(typeof(IConnection), e.NewItems[0]);
261-
};
262-
connectionManager.Connections.CollectionChanged += connectionAdded;
263+
if (loggedin) // register the first available connection so the viewmodel can use it
264+
uiProvider.AddService(connectionManager.Connections.First(c => hosts.LookupHost(c.HostAddress).IsLoggedIn));
265+
else
266+
{
267+
// a connection will be added to the list when auth is done, register it so the next
268+
// viewmodel can use it
269+
connectionAdded = (s, e) =>
270+
{
271+
if (e.Action == NotifyCollectionChangedAction.Add)
272+
uiProvider.AddService(typeof(IConnection), e.NewItems[0]);
273+
};
274+
connectionManager.Connections.CollectionChanged += connectionAdded;
275+
}
263276
}
264277

265278
machine.Configure(UIViewType.None)

src/UnitTests/GitHub.App/Controllers/UIControllerTests.cs

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
using ReactiveUI;
1414
using System.Collections.Generic;
1515
using GitHub.Authentication;
16+
using System.Collections.ObjectModel;
1617

1718
public class UIControllerTests
1819
{
@@ -55,8 +56,7 @@ public void ShowingCloneDialogWithoutBeingLoggedInShowsLoginDialog()
5556
var loginView = factory.GetView(GitHub.Exports.UIViewType.Login);
5657
loginView.Value.Cancel.Returns(Observable.Empty<object>());
5758
var cm = provider.GetConnectionManager();
58-
var cons = new System.Collections.ObjectModel.ObservableCollection<IConnection>();
59-
cm.Connections.Returns(cons);
59+
cm.Connections.Returns(new ObservableCollection<IConnection>());
6060

6161
using (var uiController = new UIController((IUIProvider)provider, hosts, factory, cm, LazySubstitute.For<ITwoFactorChallengeHandler>()))
6262
{
@@ -82,6 +82,7 @@ public void ShowingCloneDialogWhenLoggedInShowsCloneDialog()
8282
var connection = provider.GetConnection();
8383
connection.Login().Returns(Observable.Return(connection));
8484
var cm = provider.GetConnectionManager();
85+
cm.Connections.Returns(new ObservableCollection<IConnection> { connection });
8586
var host = hosts.GitHubHost;
8687
hosts.LookupHost(connection.HostAddress).Returns(host);
8788
host.IsLoggedIn.Returns(true);
@@ -99,5 +100,34 @@ public void ShowingCloneDialogWhenLoggedInShowsCloneDialog()
99100
uiController.Start(connection);
100101
}
101102
}
103+
104+
[STAFact]
105+
public void CloneDialogLoggedInWithoutConnection()
106+
{
107+
var provider = Substitutes.GetFullyMockedServiceProvider();
108+
var hosts = provider.GetRepositoryHosts();
109+
var factory = SetupFactory(provider);
110+
var connection = provider.GetConnection();
111+
connection.Login().Returns(Observable.Return(connection));
112+
var cm = provider.GetConnectionManager();
113+
cm.Connections.Returns(new ObservableCollection<IConnection> { connection });
114+
var host = hosts.GitHubHost;
115+
hosts.LookupHost(connection.HostAddress).Returns(host);
116+
host.IsLoggedIn.Returns(true);
117+
118+
using (var uiController = new UIController((IUIProvider)provider, hosts, factory, cm, LazySubstitute.For<ITwoFactorChallengeHandler>()))
119+
{
120+
var list = new List<IView>();
121+
uiController.SelectFlow(UIControllerFlow.Clone)
122+
.Subscribe(uc => list.Add(uc as IView),
123+
() =>
124+
{
125+
Assert.Equal(1, list.Count);
126+
Assert.IsAssignableFrom<IViewFor<IRepositoryCloneViewModel>>(list[0]);
127+
((IUIProvider)provider).Received().AddService(connection);
128+
});
129+
uiController.Start(null);
130+
}
131+
}
102132
}
103133
}

0 commit comments

Comments
 (0)