Skip to content

Commit 5cf40d7

Browse files
lauren-cihalauren-ciha
authored
Enable/disable sign in button (#201)
* Add a timeout on CompletOAuthAsync * Disable button: part 1 * Enable button on error * Enable button on error, cont. * Only enable button on error after sign in * Enable sign in button on sign out * Remove hanging test
1 parent f573cc3 commit 5cf40d7

File tree

7 files changed

+126
-16
lines changed

7 files changed

+126
-16
lines changed

‎GitHubExtension.Test/Controls/SignInFormTest.cs

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
5+
using GitHubExtension.DeveloperId;
6+
using Moq;
7+
using Octokit;
8+
9+
namespace GitHubExtension.Test.Controls;
10+
11+
[TestClass]
12+
public class SignInFormTest
13+
{
14+
[TestMethod]
15+
public async Task HandleOAuthRedirection_ShouldThrowInvalidOperationException_OnTimeout()
16+
{
17+
var mockGitHubClient = new Mock<IGitHubClient>();
18+
var mockOauthClient = new Mock<IOauthClient>();
19+
20+
mockGitHubClient.Setup(client => client.Oauth).Returns(mockOauthClient.Object);
21+
22+
mockOauthClient
23+
.Setup(oauth => oauth.CreateAccessToken(It.IsAny<OauthTokenRequest>()))
24+
.Returns(async () =>
25+
{
26+
await Task.Delay(TimeSpan.FromSeconds(10)); // Simulate delay longer than timeout
27+
return new OauthToken();
28+
});
29+
30+
var developerIdProvider = new DeveloperIdProvider();
31+
32+
mockGitHubClient.Setup(client => client.Oauth).Returns(mockOauthClient.Object);
33+
34+
mockOauthClient
35+
.Setup(oauth => oauth.CreateAccessToken(It.IsAny<OauthTokenRequest>()))
36+
.Returns(async () =>
37+
{
38+
await Task.Delay(TimeSpan.FromSeconds(10));
39+
return new OauthToken();
40+
});
41+
42+
var oAuthRequest = new OAuthRequest();
43+
44+
var authorizationResponse = new Uri("https://example.com/callback?code=valid_code");
45+
46+
await Assert.ThrowsExceptionAsync<InvalidOperationException>(async () =>
47+
{
48+
await Task.Run(() => developerIdProvider.HandleOauthRedirection(authorizationResponse));
49+
});
50+
}
51+
}

‎GitHubExtension/Controls/Forms/SignInForm.cs

+36-1
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@
22
// The Microsoft Corporation licenses this file to you under the MIT license.
33
// See the LICENSE file in the project root for more information.
44

5+
using System.Globalization;
56
using GitHubExtension.DeveloperId;
67
using GitHubExtension.Helpers;
78
using Microsoft.CommandPalette.Extensions;
89
using Microsoft.CommandPalette.Extensions.Toolkit;
9-
using Windows.Foundation;
1010

1111
namespace GitHubExtension.Controls.Forms;
1212

@@ -21,10 +21,43 @@ public partial class SignInForm : FormContent, IGitHubForm
2121
private readonly IDeveloperIdProvider _developerIdProvider;
2222
private readonly IResources _resources;
2323

24+
private bool _isButtonEnabled = true;
25+
26+
private string IsButtonEnabled =>
27+
_isButtonEnabled.ToString(CultureInfo.InvariantCulture).ToLower(CultureInfo.InvariantCulture);
28+
2429
public SignInForm(IDeveloperIdProvider developerIdProvider, IResources resources)
2530
{
2631
_resources = resources;
2732
_developerIdProvider = developerIdProvider;
33+
_developerIdProvider.OAuthRedirected += DeveloperIdProvider_OAuthRedirected;
34+
SignOutForm.SignOutAction += SignOutForm_SignOutAction;
35+
}
36+
37+
private void SignOutForm_SignOutAction(object? sender, SignInStatusChangedEventArgs e)
38+
{
39+
_isButtonEnabled = !e.IsSignedIn;
40+
}
41+
42+
private void DeveloperIdProvider_OAuthRedirected(object? sender, Exception? e)
43+
{
44+
if (e is not null)
45+
{
46+
SetButtonEnabled(true);
47+
LoadingStateChanged?.Invoke(this, false);
48+
SignInAction?.Invoke(this, new SignInStatusChangedEventArgs(false, e));
49+
FormSubmitted?.Invoke(this, new FormSubmitEventArgs(false, e));
50+
return;
51+
}
52+
53+
SetButtonEnabled(false);
54+
}
55+
56+
private void SetButtonEnabled(bool isEnabled)
57+
{
58+
_isButtonEnabled = isEnabled;
59+
TemplateJson = TemplateHelper.LoadTemplateJsonFromTemplateName("AuthTemplate", TemplateSubstitutions);
60+
OnPropertyChanged(nameof(TemplateJson));
2861
}
2962

3063
public Dictionary<string, string> TemplateSubstitutions => new()
@@ -33,6 +66,7 @@ public SignInForm(IDeveloperIdProvider developerIdProvider, IResources resources
3366
{ "{{AuthButtonTitle}}", _resources.GetResource("Forms_Sign_In") },
3467
{ "{{AuthIcon}}", $"data:image/png;base64,{GitHubIcon.GetBase64Icon("logo")}" },
3568
{ "{{AuthButtonTooltip}}", _resources.GetResource("Forms_Sign_In_Tooltip") },
69+
{ "{{ButtonIsEnabled}}", IsButtonEnabled },
3670
};
3771

3872
public override string TemplateJson => TemplateHelper.LoadTemplateJsonFromTemplateName("AuthTemplate", TemplateSubstitutions);
@@ -52,6 +86,7 @@ public override ICommandResult SubmitForm(string inputs, string data)
5286
catch (Exception ex)
5387
{
5488
LoadingStateChanged?.Invoke(this, false);
89+
SetButtonEnabled(true);
5590
SignInAction?.Invoke(this, new SignInStatusChangedEventArgs(false, ex));
5691
FormSubmitted?.Invoke(this, new FormSubmitEventArgs(false, ex));
5792
}

‎GitHubExtension/Controls/Forms/SignOutForm.cs

+1
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public SignOutForm(IDeveloperIdProvider developerIdProvider, IResources resource
3232
{ "{{AuthButtonTitle}}", _resources.GetResource("Forms_Sign_Out_Button_Title") },
3333
{ "{{AuthIcon}}", $"data:image/png;base64,{GitHubIcon.GetBase64Icon("logo")}" },
3434
{ "{{AuthButtonTooltip}}", _resources.GetResource("Forms_Sign_Out_Tooltip") },
35+
{ "{{ButtonIsEnabled}}", "true" },
3536
};
3637

3738
public override string TemplateJson => TemplateHelper.LoadTemplateJsonFromTemplateName("AuthTemplate", TemplateSubstitutions);

‎GitHubExtension/Controls/Pages/SignInPage.cs

+15-7
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1-
// Copyright (c) Microsoft Corporation
2-
// The Microsoft Corporation licenses this file to you under the MIT license.
3-
// See the LICENSE file in the project root for more information.
4-
1+
// Copyright (c) Microsoft Corporation
2+
// The Microsoft Corporation licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for more information.
4+
55
using GitHubExtension.Controls.Forms;
66
using GitHubExtension.Helpers;
77
using Microsoft.CommandPalette.Extensions;
88
using Microsoft.CommandPalette.Extensions.Toolkit;
9+
using Windows.Foundation;
910

1011
namespace GitHubExtension.Controls.Pages;
1112

@@ -24,12 +25,19 @@ public SignInPage(SignInForm signInForm, StatusMessage statusMessage, string suc
2425
_errorMessage = errorMessage;
2526

2627
// Wire up events using the helper
27-
FormEventHelper.WireFormEvents(_signInForm, this, _statusMessage, _successMessage, _errorMessage);
28+
FormEventHelper.WireFormEvents(_signInForm, this, _statusMessage, _successMessage, _errorMessage);
29+
30+
_signInForm.PropChanged += UpdatePage;
2831

2932
// Hide status message initially
3033
ExtensionHost.HideStatus(_statusMessage);
31-
}
32-
34+
}
35+
36+
private void UpdatePage(object sender, IPropChangedEventArgs args)
37+
{
38+
RaiseItemsChanged();
39+
}
40+
3341
public override IContent[] GetContent()
3442
{
3543
ExtensionHost.HideStatus(_statusMessage);

‎GitHubExtension/Controls/Templates/AuthTemplate.json

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
{
22
"type": "AdaptiveCard",
33
"$schema": "http://adaptivecards.io/schemas/adaptive-card.json",
4-
"version": "1.5",
4+
"version": "1.6",
55
"body": [
66
{
77
"type": "Container",
@@ -37,9 +37,10 @@
3737
"type": "ActionSet",
3838
"actions": [
3939
{
40-
"type": "Action.Submit",
4140
"title": "{{AuthButtonTitle}}",
42-
"tooltip": "{{AuthButtonTooltip}}"
41+
"tooltip": "{{AuthButtonTooltip}}",
42+
"type": "Action.Submit",
43+
"isEnabled": {{ButtonIsEnabled}}
4344
}
4445
]
4546
}

‎GitHubExtension/DeveloperId/DeveloperIdProvider.cs

+16-4
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@ private List<DeveloperId> DeveloperIds
3636
get; set;
3737
}
3838

39-
private readonly Lazy<CredentialVault> _credentialVault;
39+
private readonly Lazy<CredentialVault> _credentialVault;
40+
41+
public event EventHandler<Exception?>? OAuthRedirected;
4042

4143
// Private constructor for Singleton class.
4244
public DeveloperIdProvider()
@@ -147,7 +149,9 @@ public bool LogoutDeveloperId(IDeveloperId developerId)
147149

148150
public void HandleOauthRedirection(Uri authorizationResponse)
149151
{
150-
OAuthRequest? oAuthRequest = null;
152+
OAuthRequest? oAuthRequest = null;
153+
154+
OAuthRedirected?.Invoke(this, null);
151155

152156
lock (_oAuthRequestsLock)
153157
{
@@ -180,8 +184,16 @@ public void HandleOauthRedirection(Uri authorizationResponse)
180184
OAuthRequests.Remove(oAuthRequest);
181185
}
182186
}
183-
184-
oAuthRequest.CompleteOAuthAsync(authorizationResponse).Wait();
187+
188+
try
189+
{
190+
oAuthRequest.CompleteOAuthAsync(authorizationResponse).Wait();
191+
}
192+
catch (Exception ex)
193+
{
194+
_log.Error(ex, $"Error while completing OAuth request: ");
195+
OAuthRedirected?.Invoke(this, ex);
196+
}
185197
}
186198

187199
public IEnumerable<IDeveloperId> GetLoggedInDeveloperIdsInternal()

‎GitHubExtension/DeveloperId/IDeveloperIdProvider.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,7 @@ public interface IDeveloperIdProvider
1616

1717
bool LogoutDeveloperId(IDeveloperId developerId);
1818

19-
void HandleOauthRedirection(Uri authorizationResponse);
19+
void HandleOauthRedirection(Uri authorizationResponse);
20+
21+
public event EventHandler<Exception?>? OAuthRedirected;
2022
}

0 commit comments

Comments
 (0)