feat: add toggle flag step #111
Conversation
|
This pull request has been linked to Shortcut Story #238066: add toggle flag page to setup wizard. |
k3llymariee
changed the title
| envKey := viper.GetString(cliflags.EnvironmentFlag) | ||
| patch = flags.BuildToggleFlagPatch(envKey, cmd.CalledAs() == "toggle-on") |
There was a problem hiding this comment.
refactored this to play better with where we call this in the quickstart
| @@ -89,3 +89,83 @@ func TestUpdate(t *testing.T) { | |||
| assert.EqualError(t, err, "base-uri is invalid"+errorHelp) | |||
| }) | |||
| } | |||
|
|
|||
| func TestToggle(t *testing.T) { | |||
There was a problem hiding this comment.
added tests to make sure i didn't break anything
| @@ -23,13 +28,14 @@ type ContainerModel struct { | |||
| accessToken string | |||
| baseUri string | |||
| currentModel tea.Model | |||
| sdkKind string | |||
There was a problem hiding this comment.
like flagKey storing anything we might need in a subsequent step on the container model
| accessToken := viper.GetString(cliflags.AccessTokenFlag) | ||
| baseUri := viper.GetString(cliflags.BaseURIFlag) |
There was a problem hiding this comment.
removed this since we have this at the container level/should set these values on the createFlagModel
k3llymariee
changed the title
| @@ -24,6 +24,25 @@ func sendErr(err error) tea.Cmd { | |||
| } | |||
| } | |||
|
|
|||
| type toggledFlagMsg struct{} | |||
|
|
|||
| func sendToggleFlagMsg(client flags.Client, accessToken, baseUri, flagKey string, enabled bool) tea.Cmd { | |||
There was a problem hiding this comment.
Does it make more sense to name this after the message it sends, so in this case it would be sendToggledFlagMsg (past tense)? I could see it implicitly sending a toggleFlagMsg and returning the results, either a toggledFlagMsg or errMsg, but I could also see that as confusing because there isn't a specific toggleFlagMsg type.
There was a problem hiding this comment.
ya I did actually get a bit tripped on this looking for the the actual msg type it specified when I picked it back up again, but it is also Doing The Thing here
internal/quickstart/toggle_flag.go
Outdated
| accessToken string | ||
| baseUri string | ||
| client flags.Client | ||
| enabled bool // whether the flag has ever been toggled on |
There was a problem hiding this comment.
Could you rename this something like flagWasToggled since I think that enabled means the flag is currently toggled on and not that it was changed at all.
Adds the remaining functionality for the toggle flag steps. Error handling will follow in a separate PR.