Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

waypoint user modify command that will not modify a user returns help. #2444

Merged
merged 6 commits into from
Oct 12, 2021

Conversation

izaaklauer
Copy link
Contributor

Previous behavior:

Running modify with no flags returned success. This was confusing for people who expected this to return help text.

$ waypoint user modify
User modification successful.

Running modify with flags to specify (but not modify) a user also returned user modification successful, even though no real modifications were performed.

$ waypoint user modify -username=foo
User modification successful.

As of this change, waypoint user modify without a flag that will do a modification (i.e. -new-username or -display-name) will return an error and help text, similar to other commands like project inspect:

$ ./waypoint user modify
! At least one user modification flag must be specified...
  
  Usage: waypoint user modify [options]
  
    Modify details about a user.
  
    Some details such as username and display name may be updated.
    Use "waypoint user inspect" to see the current attributes for a user.
  
  Global Options:
  
    -app=<string>
        App to target. Certain commands require a single app target for Waypoint
        configurations with multiple apps. If you have a single app, then this
        can be ignored.
  
    -plain
        Plain output: no colors, no animation. The default is false.
  
    -workspace=<string>
        Workspace to operate in.
  
  Command Options:
  
    -display-name=<string>
        The display name for a user. If this is set, this is used in some places
        in the CLI and UI. This does not have to be unique.
  
    -new-username=<string>
        Set a new username for this user. This must be unique to the server.
  
    -username=<string>
        The user to modify. This defaults to the currently logged in user.
$ ./waypoint user modify -username=bar
! At least one user modification flag must be specified...
  
  Usage: waypoint user modify [options]
  
    Modify details about a user.
  
    Some details such as username and display name may be updated.
    Use "waypoint user inspect" to see the current attributes for a user.
  
  Global Options:
  
    -app=<string>
        App to target. Certain commands require a single app target for Waypoint
        configurations with multiple apps. If you have a single app, then this
        can be ignored.
  
    -plain
        Plain output: no colors, no animation. The default is false.
  
    -workspace=<string>
        Workspace to operate in.
  
  Command Options:
  
    -display-name=<string>
        The display name for a user. If this is set, this is used in some places
        in the CLI and UI. This does not have to be unique.
  
    -new-username=<string>
        Set a new username for this user. This must be unique to the server.
  
    -username=<string>
        The user to modify. This defaults to the currently logged in user.

@github-actions github-actions bot added the core label Oct 7, 2021
@izaaklauer izaaklauer requested a review from a team October 7, 2021 19:27
Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

LGTM just nitpick feedback on the error display

internal/cli/user_modify.go Outdated Show resolved Hide resolved
@izaaklauer izaaklauer marked this pull request as ready for review October 8, 2021 12:30
@izaaklauer
Copy link
Contributor Author

In a related ux nitpick, I was really surprised to find that -username selects a user, but -display-name modifies a user. I was expecting to be able to use -display-name to find a user (makes sense that I can't - display names aren't globally unique).

It feels to me like the username should be an argument, and -display-name should be -new-display-name.

@izaaklauer izaaklauer requested review from a team and koesbong October 8, 2021 12:37
@krantzinator
Copy link
Contributor

In a related ux nitpick, I was really surprised to find that -username selects a user, but -display-name modifies a user. I was expecting to be able to use -display-name to find a user (makes sense that I can't - display names aren't globally unique).

It feels to me like the username should be an argument, and -display-name should be -new-display-name.

That does sound really confusing. I support this proposal.

Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

🎉

@@ -0,0 +1,3 @@
```release-note:improvement
cli/user-modify: Return help on malformed command
Copy link
Contributor

Choose a reason for hiding this comment

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

I think our prefix is usually just cli: and specify the "user modify" in the text

Copy link
Member

@briancain briancain left a comment

Choose a reason for hiding this comment

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

The go tests might be failing from a flake, rerunning them before merge would be good! 👍🏻

@izaaklauer izaaklauer merged commit 8049c13 into main Oct 12, 2021
@izaaklauer izaaklauer deleted the user-modify-empty branch October 12, 2021 13:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants