Adds line to template for custom role annotation #31
Conversation
|
Hi @melissamahoney-mongodb, I've been thinking about creating a PR to do something like this for a while now, can you please take a look and provide input on whether you think this is a good addition to the template? |
|
Marked as a draft until I figure out how I can lessen the cyclomatic complexity, since it was already at the max before adding this line. |
|
|
||
| func requiredRole(buf *bytes.Buffer, cmd *cobra.Command) { | ||
| if cmd.Annotations["requiredRole"] != "" { | ||
| buf.WriteString("\nTo use this command, the requesting user or API key must have the " + cmd.Annotations["requiredRole"] + " role.\n") |
There was a problem hiding this comment.
Reading the rest of the long description, everything is framed in the second person. Should we do that here as well?
| buf.WriteString("\nTo use this command, the requesting user or API key must have the " + cmd.Annotations["requiredRole"] + " role.\n") | |
| buf.WriteString("\nTo use this command, you must have the " + cmd.Annotations["requiredRole"] + " role.\n") |
There was a problem hiding this comment.
@melissamahoney-mongodb My concern with this change is that they can be authenticating either using their own user login (via atlas auth login) or with their API key. So there could conceivably be a scenario where their personal user login doesn't have the right permissions, but the API key does (or vice versa), and saying "you" doesn't capture that and might confuse people. What do you think of the following instead?
| buf.WriteString("\nTo use this command, the requesting user or API key must have the " + cmd.Annotations["requiredRole"] + " role.\n") | |
| buf.WriteString("\nTo use this command, you must authenticate with a user account or an API key that has the " + cmd.Annotations["requiredRole"] + " role.\n") |
It's a little wordy, but I think it clears that up while using the second person.
There was a problem hiding this comment.
Yeah I like it!
| ) | ||
|
|
||
| func requiredRole(buf *bytes.Buffer, cmd *cobra.Command) { | ||
| if cmd.Annotations["requiredRole"] != "" { |
There was a problem hiding this comment.
no strong opinion if this is a good idea or not, if this works for docs good for me just a reminder that any metadata added to annotations doesn't reflect in the command help (atlas--help)
There was a problem hiding this comment.
Oooh good point... I wonder if we could display this inside the long description? Or concatenate them somehow? so that it does show up in the command help. No problem with this merged as-is, but could be a follow-on if we can make it work
|
@gssbzn @melissamahoney-mongodb Looks like I don't have merge capabilities on this repo, but since this LGTM for both of you then please merge when you have a chance @gssbzn |
Proposed changes
We have info on the roles required for all commands in the OpenAPI spec. When I started the MCLI backfill, I planned to explore ways we might programmatically pull this info in rather than adding it manually.
After some research, it seems that we do need to add the roles manually, but I think that the best way to do this is by standardizing the language at the cobra2snooty template level, adding a new custom annotation for requiredRole to each command, and having the info display for all commands with the annotation after the Long description. This also aligns with a new ask we have this quarter to make sure that all pages for the UI include info on required roles to perform actions.
If we choose to merge this PR, I can add all of the requiredRoles annotations for Atlas commands.
I tested this locally and results display as expected for commands with a requiredRole annotation and don't display for commands without it. See the highlighted line on the attached screenshot for how it displays on an autogenerated docs page.
Jira ticket:
N/A
Checklist
make fmtand formatted my code