Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix up & touch up user facing messages #468

Merged
merged 1 commit into from
Jul 24, 2024
Merged

fix up & touch up user facing messages #468

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/D2L.Bmx/Aws/AwsCredsCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ AwsCredentials credentials
&& o.Org == org
)
// Prune older (closer to expiry) credentials for the same role
.GroupBy( o => $"{o.AccountName}.${o.RoleName}", ( _, entries ) =>
.GroupBy( o => (o.AccountName, o.RoleName), ( _, entries ) =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Somewhat unrelated - this isn't a user-facing message, but it's also a string-related issue.

  • Value tuples are preferred when we want to use multiple values as a single key. Converting them to a single string is unnecessary and has more overhead.
  • Even if we do want to use a string here, it should've been 
    -$"{o.AccountName}.${o.RoleName}"
+$"{o.AccountName}.{o.RoleName}"
    A common mistake when using C# interpolated strings.

entries.OrderBy( o => o.Credentials.Expiration )
)
.Select( o => o.Last() );
Expand Down
10 changes: 7 additions & 3 deletions src/D2L.Bmx/BmxConfigProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ internal interface IBmxConfigProvider {
void SaveConfiguration( BmxConfig config );
}

internal class BmxConfigProvider( FileIniDataParser parser ) : IBmxConfigProvider {
internal class BmxConfigProvider(
FileIniDataParser parser,
IConsoleWriter consoleWriter
) : IBmxConfigProvider {

public BmxConfig GetConfiguration() {
// Main config is at ~/.bmx/config
string configFileName = BmxPaths.CONFIG_FILE_NAME;
Expand All @@ -18,7 +22,7 @@ public BmxConfig GetConfiguration() {
var tempdata = parser.ReadFile( configFileName );
data.Merge( tempdata );
} catch( Exception ) {
Console.Error.Write( $"WARNING: Failed to load the global config file {configFileName}." );
consoleWriter.WriteWarning( $"WARNING: Failed to load the global config file {configFileName}." );
Comment on lines -21 to +25
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to reproduce such a failure scenario so it's never manifested, but Console.Error.Write (without a newline) would've never worked nicely.

}
}
// If set, we recursively look up from CWD (all the way to root) for additional bmx config files (labelled as .bmx)
Expand All @@ -29,7 +33,7 @@ public BmxConfig GetConfiguration() {
var tempdata = parser.ReadFile( projectConfigInfo.FullName );
data.Merge( tempdata );
} catch( Exception ) {
Console.Error.Write( $"WARNING: Failed to load the local config file {projectConfigInfo.FullName}." );
consoleWriter.WriteWarning( $"WARNING: Failed to load the local config file {projectConfigInfo.FullName}." );
}
}

Expand Down
7 changes: 4 additions & 3 deletions src/D2L.Bmx/Okta/OktaApi.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,10 @@ await resp.Content.ReadAsStreamAsync(),
}

string org = _organization ?? "unknown";
throw new BmxException(
$"Okta authentication for user '{username}' in org '{org}'"
+ "failed. Check if org, user, and password is correct" );
throw new BmxException( $"""
Okta authentication for user '{username}' in org '{org}' failed.
Check if org, user, and password is correct.
""" );
Comment on lines -91 to +94
Copy link
Member Author

@cfbao cfbao Jul 24, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one was missing a space before "failed", and the usage of period was inconsistent:
image

Easier (and clearer, I think) to use a raw string literal with multiple lines instead.
image

}

async Task IOktaApi.IssueMfaChallengeAsync( string stateToken, string factorId ) {
Expand Down
8 changes: 4 additions & 4 deletions src/D2L.Bmx/OktaAuthenticator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ mfaFactor is OktaMfaQuestionFactor // Security question factor is a static value
if( File.Exists( BmxPaths.CONFIG_FILE_NAME ) ) {
CacheOktaSession( user, org, sessionResp.Id, sessionResp.ExpiresAt );
} else {
consoleWriter.WriteWarning(
"No config file found. Your Okta session will not be cached. " +
"Consider running `bmx configure` if you own this machine."
);
consoleWriter.WriteWarning( """
No config file found. Your Okta session will not be cached.
Consider running `bmx configure` if you own this machine.
""" );
}
return new AuthenticatedOktaApi( Org: org, User: user, Api: oktaApi );
}
Expand Down
11 changes: 6 additions & 5 deletions src/D2L.Bmx/Program.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,13 @@
userOption,
};
loginCommand.SetHandler( ( InvocationContext context ) => {
var config = new BmxConfigProvider( new FileIniDataParser() ).GetConfiguration();
var consoleWriter = new ConsoleWriter();
var config = new BmxConfigProvider( new FileIniDataParser(), consoleWriter ).GetConfiguration();
var handler = new LoginHandler( new OktaAuthenticator(
new OktaApi(),
new OktaSessionStorage(),
new ConsolePrompter(),
new ConsoleWriter(),
consoleWriter,
config
) );
return handler.HandleAsync(
Expand Down Expand Up @@ -67,7 +68,7 @@

configureCommand.SetHandler( ( InvocationContext context ) => {
var handler = new ConfigureHandler(
new BmxConfigProvider( new FileIniDataParser() ),
new BmxConfigProvider( new FileIniDataParser(), new ConsoleWriter() ),
new ConsolePrompter() );
handler.Handle(
org: context.ParseResult.GetValueForOption( orgOption ),
Expand Down Expand Up @@ -123,7 +124,7 @@
printCommand.SetHandler( ( InvocationContext context ) => {
var consolePrompter = new ConsolePrompter();
var consoleWriter = new ConsoleWriter();
var config = new BmxConfigProvider( new FileIniDataParser() ).GetConfiguration();
var config = new BmxConfigProvider( new FileIniDataParser(), consoleWriter ).GetConfiguration();
var handler = new PrintHandler(
new OktaAuthenticator(
new OktaApi(),
Expand Down Expand Up @@ -177,7 +178,7 @@
writeCommand.SetHandler( ( InvocationContext context ) => {
var consolePrompter = new ConsolePrompter();
var consoleWriter = new ConsoleWriter();
var config = new BmxConfigProvider( new FileIniDataParser() ).GetConfiguration();
var config = new BmxConfigProvider( new FileIniDataParser(), consoleWriter ).GetConfiguration();
var handler = new WriteHandler(
new OktaAuthenticator(
new OktaApi(),
Expand Down
Loading