From d6c7a9832dfcefabdcd340ee8b3a164192d497a8 Mon Sep 17 00:00:00 2001 From: Amy Wah Date: Fri, 21 Aug 2026 17:16:03 -0400 Subject: [PATCH 1/6] feat: check requests to install an app Add a hidden `slack app requests` command behind the app-approval-status experiment that reports the most recent install approval request for the selected app on each team in the token's scope. Co-authored-by: Cursor --- cmd/app/app.go | 1 + cmd/app/requests.go | 196 ++++++++++++++++++++++++ cmd/app/requests_test.go | 238 ++++++++++++++++++++++++++++++ cmd/help/help.go | 4 +- docs/reference/experiments.md | 2 + internal/api/api_mock.go | 5 + internal/api/app.go | 105 +++++++++++++ internal/api/app_test.go | 102 +++++++++++++ internal/experiment/experiment.go | 5 + internal/slackerror/errors.go | 14 ++ 10 files changed, 670 insertions(+), 2 deletions(-) create mode 100644 cmd/app/requests.go create mode 100644 cmd/app/requests_test.go diff --git a/cmd/app/app.go b/cmd/app/app.go index 124f5d35..b8383082 100644 --- a/cmd/app/app.go +++ b/cmd/app/app.go @@ -52,6 +52,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { cmd.AddCommand(NewDeleteCommand(clients)) cmd.AddCommand(NewLinkCommand(clients)) cmd.AddCommand(NewListCommand(clients)) + cmd.AddCommand(NewRequestsCommand(clients)) cmd.AddCommand(NewSettingsCommand(clients)) cmd.AddCommand(NewUninstallCommand(clients)) cmd.AddCommand(NewUnlinkCommand(clients)) diff --git a/cmd/app/requests.go b/cmd/app/requests.go new file mode 100644 index 00000000..43217d66 --- /dev/null +++ b/cmd/app/requests.go @@ -0,0 +1,196 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "fmt" + "sort" + "strings" + "time" + + "github.com/opentracing/opentracing-go" + "github.com/slackapi/slack-cli/internal/api" + "github.com/slackapi/slack-cli/internal/cmdutil" + "github.com/slackapi/slack-cli/internal/experiment" + "github.com/slackapi/slack-cli/internal/prompts" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/style" + "github.com/spf13/cobra" +) + +// requestsTeamsLimit is the most teams the API searches in a single call +const requestsTeamsLimit = 50 + +// requestsTimeFormat displays the moment a request changed +const requestsTimeFormat = "2006-01-02 15:04:05 Z07:00" + +// Handle to a function used for testing +var requestsAppSelectPromptFunc = prompts.AppSelectPrompt + +// Flags + +type requestsCmdFlags struct { + teamIDs []string +} + +var requestsFlags requestsCmdFlags + +// NewRequestsCommand returns a new Cobra command +func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { + cmd := &cobra.Command{ + Use: "requests [flags]", + Aliases: []string{"approval-requests", "approvals"}, + Short: "Check requests to install the app", + Long: strings.Join([]string{ + "Check the status of your most recent request to have the app approved for", + "install.", + "", + "Requests are searched on the team of the authenticated account. An account of", + "a workspace that belongs to an organization also searches that organization,", + "while an account of an organization searches the organization alone.", + "", + "Other workspaces of an organization can be searched with the --team-ids flag.", + }, "\n"), + Hidden: true, + Example: style.ExampleCommandsf([]style.ExampleCommand{ + {Command: "app requests", Meaning: "Check requests to install an app"}, + {Command: "app requests --team-ids T0123456789,T9876543210", Meaning: "Check requests on certain teams of an organization"}, + }), + Args: cobra.NoArgs, + PreRunE: func(cmd *cobra.Command, args []string) error { + if !clients.Config.WithExperimentOn(experiment.AppApprovalStatus) { + return slackerror.New(slackerror.ErrExperimentRequired). + WithRemediation("Enable the %s experiment with %s", + style.Highlight(string(experiment.AppApprovalStatus)), + style.CommandText("--experiment app-approval-status"), + ) + } + if len(requestsFlags.teamIDs) > requestsTeamsLimit { + return slackerror.New(slackerror.ErrInvalidArguments). + WithMessage("The %s flag accepts at most %d teams", + style.CommandText("--team-ids"), + requestsTeamsLimit, + ) + } + clients.Config.SetFlags(cmd) + // Verify command is run in a project directory + return cmdutil.IsValidProjectDirectory(clients) + }, + RunE: func(cmd *cobra.Command, args []string) error { + return runRequestsCommand(cmd, clients) + }, + } + + cmd.Flags().StringSliceVar(&requestsFlags.teamIDs, "team-ids", nil, "also check these teams of an organization,\nwith a maximum of 50 teams") + + return cmd +} + +// runRequestsCommand will execute the requests command +func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error { + ctx := cmd.Context() + span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.app.requests") + defer span.Finish() + + selection, err := requestsAppSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAndUninstalledApps) + if err != nil { + return err + } + if selection.App.AppID == "" { + return slackerror.New(slackerror.ErrAppNotFound) + } + + result, err := clients.API().ListAppApprovalRequests(ctx, selection.Auth.Token, selection.App.AppID, requestsFlags.teamIDs) + if err != nil { + return err + } + + clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ + Emoji: "lock", + Text: "App Requests", + Secondary: FormatRequestsSuccess(result.Requests), + })) + return nil +} + +// FormatRequestsSuccess formats the install request of each team +func FormatRequestsSuccess(requests []api.AppsApprovalsRequest) (secondaryText []string) { + sort.Slice(requests, func(i, j int) bool { + return requests[i].TeamID < requests[j].TeamID + }) + field := func(label string, value string) string { + return fmt.Sprintf(style.Indent(style.Secondary("%-13s %s")), label+":", value) + } + for _, request := range requests { + secondaryText = append(secondaryText, fmt.Sprintf(style.Bold("%s:"), request.TeamID)) + secondaryText = append(secondaryText, field("Request ID", request.ID)) + secondaryText = append(secondaryText, field("Status", formatRequestStatus(request.Status))) + secondaryText = append(secondaryText, field("Requested", formatRequestTime(request.DateCreated))) + if request.DateResolved > 0 { + secondaryText = append(secondaryText, field("Resolved", formatRequestTime(request.DateResolved))) + } + if request.CancelledBy != "" { + secondaryText = append(secondaryText, field("Cancelled by", formatRequestCancelledBy(request.CancelledBy))) + } + if request.CanSelfApprove { + secondaryText = append(secondaryText, style.Indent(style.Secondary("You can install this app without approval. Please cancel the request."))) + } + } + if len(secondaryText) <= 0 { + secondaryText = append(secondaryText, "You have not requested to install this app") + } + return +} + +// formatRequestTime displays a Unix timestamp in the local timezone +func formatRequestTime(timestamp int64) string { + if timestamp <= 0 { + return "unknown" + } + return time.Unix(timestamp, 0).Format(requestsTimeFormat) +} + +// formatRequestCancelledBy names the kind of actor that cancelled a request. +// Every returned request was made by the authenticated account, so a request +// cancelled by a user was withdrawn by that same account. +func formatRequestCancelledBy(actor api.AppsApprovalsRequestCancelledBy) string { + switch actor { + case api.AppsApprovalsRequestCancelledByAdmin: + return "an admin" + case api.AppsApprovalsRequestCancelledBySystem: + return "the system" + case api.AppsApprovalsRequestCancelledByUser: + return "you" + default: + return string(actor) + } +} + +// formatRequestStatus styles a status by how much attention it deserves +func formatRequestStatus(status api.AppsApprovalsRequestStatus) string { + switch status { + case api.AppsApprovalsRequestStatusApproved: + return style.Green(string(status)) + case api.AppsApprovalsRequestStatusCancelled: + return style.Secondary(string(status)) + case api.AppsApprovalsRequestStatusDenied: + return style.Red(string(status)) + case api.AppsApprovalsRequestStatusPending: + return style.Yellow(string(status)) + default: + return string(status) + } +} diff --git a/cmd/app/requests_test.go b/cmd/app/requests_test.go new file mode 100644 index 00000000..2098e526 --- /dev/null +++ b/cmd/app/requests_test.go @@ -0,0 +1,238 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package app + +import ( + "context" + "fmt" + "strings" + "testing" + "time" + + "github.com/slackapi/slack-cli/internal/api" + "github.com/slackapi/slack-cli/internal/experiment" + "github.com/slackapi/slack-cli/internal/hooks" + "github.com/slackapi/slack-cli/internal/prompts" + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/test/testutil" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" +) + +// mockRequestCreated is the moment a mocked request was made +var mockRequestCreated = time.Date(2026, 8, 21, 15, 4, 5, 0, time.UTC).Unix() + +// mockRequestResolved is the moment a mocked request was reviewed +var mockRequestResolved = time.Date(2026, 8, 22, 9, 30, 0, 0, time.UTC).Unix() + +func TestRequestsCommand(t *testing.T) { + // enableRequests turns on the experiment that gates the command + enableRequests := func(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + cm.AddDefaultMocks() + cf.SDKConfig = hooks.NewSDKConfigMock() + cf.Config.ExperimentsFlag = []string{string(experiment.AppApprovalStatus)} + cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) + requestsAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { + return prompts.SelectedApp{ + App: types.App{AppID: "A1234", TeamID: "T1234", TeamDomain: "teamone"}, + Auth: types.SlackAuth{Token: "xoxp-example"}, + }, nil + } + } + + restoreRequests := func() { + requestsAppSelectPromptFunc = prompts.AppSelectPrompt + } + + testutil.TableTestCommand(t, testutil.CommandTests{ + "errors when the app-approval-status experiment is off": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + cm.AddDefaultMocks() + cf.SDKConfig = hooks.NewSDKConfigMock() + cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) + }, + ExpectedError: slackerror.New(slackerror.ErrExperimentRequired), + }, + "reports a request that awaits review": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + cm.API.On("ListAppApprovalRequests", mock.Anything, "xoxp-example", "A1234", []string(nil)). + Return(api.AppsApprovalsRequestsListResult{ + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + }, + }, nil) + }, + Teardown: restoreRequests, + ExpectedOutputs: []string{ + "App Requests", + "T1234", + "Request ID: Ar1234", + "Status: pending", + "Requested:", + }, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertCalled(t, "ListAppApprovalRequests", mock.Anything, "xoxp-example", "A1234", []string(nil)) + }, + }, + "explains that an app was never requested": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.AppsApprovalsRequestsListResult{Requests: []api.AppsApprovalsRequest{}}, nil) + }, + Teardown: restoreRequests, + ExpectedOutputs: []string{"You have not requested to install this app"}, + }, + "searches the teams of the provided team IDs": { + CmdArgs: []string{"--team-ids", "T1234,T5678"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.AppsApprovalsRequestsListResult{}, nil) + }, + Teardown: restoreRequests, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertCalled(t, "ListAppApprovalRequests", mock.Anything, "xoxp-example", "A1234", []string{"T1234", "T5678"}) + }, + }, + "errors when more than fifty teams are provided": { + CmdArgs: []string{"--team-ids", strings.Join(mockRequestTeamIDs(requestsTeamsLimit+1), ",")}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + }, + Teardown: restoreRequests, + ExpectedErrorStrings: []string{"--team-ids", "at most 50 teams"}, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "ListAppApprovalRequests") + }, + }, + "returns the error of a failed lookup": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.AppsApprovalsRequestsListResult{}, slackerror.New(slackerror.ErrFeatureNotEnabled)) + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrFeatureNotEnabled), + }, + }, func(cf *shared.ClientFactory) *cobra.Command { + return NewRequestsCommand(cf) + }) +} + +func TestRequestsFormat(t *testing.T) { + tests := map[string]struct { + Requests []api.AppsApprovalsRequest + Expected []string + Unexpected []string + }{ + "no request was made for the app": { + Requests: []api.AppsApprovalsRequest{}, + Expected: []string{"You have not requested to install this app"}, + }, + "an open request omits the resolved timestamp": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + }, + Expected: []string{ + "T1234", + "Request ID: Ar1234", + "Status: pending", + "Requested: 2026-08-21", + }, + Unexpected: []string{"Resolved:", "Cancelled by:", "without approval"}, + }, + "an approved request includes the resolved timestamp": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusApproved, DateCreated: mockRequestCreated, DateResolved: mockRequestResolved}, + }, + Expected: []string{ + "Status: approved", + "Requested: 2026-08-21", + "Resolved: 2026-08-22", + }, + }, + "a request cancelled by an admin of the team": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated, DateResolved: mockRequestResolved, CancelledBy: api.AppsApprovalsRequestCancelledByAdmin}, + }, + Expected: []string{ + "Status: cancelled", + "Cancelled by: an admin", + }, + }, + "a request withdrawn by the authenticated account": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated, DateResolved: mockRequestResolved, CancelledBy: api.AppsApprovalsRequestCancelledByUser}, + }, + Expected: []string{"Cancelled by: you"}, + }, + "a request cancelled without an actor of its own": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated, DateResolved: mockRequestResolved, CancelledBy: api.AppsApprovalsRequestCancelledBySystem}, + }, + Expected: []string{"Cancelled by: the system"}, + }, + "a denied request that the account can approve itself": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusDenied, CanSelfApprove: true, DateCreated: mockRequestCreated, DateResolved: mockRequestResolved}, + }, + Expected: []string{ + "Status: denied", + "You can install this app without approval. Please cancel the request.", + }, + }, + "requests are sorted by the team ID": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated}, + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusApproved, DateCreated: mockRequestCreated}, + }, + Expected: []string{ + "T1234", + "Status: approved", + "T5678", + "Status: cancelled", + }, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + formatted := strings.Join(FormatRequestsSuccess(tc.Requests), "\n") + previous := -1 + for _, value := range tc.Expected { + index := strings.Index(formatted, value) + assert.Greater(t, index, previous, "expected %q to follow the preceding values", value) + previous = index + } + for _, value := range tc.Unexpected { + assert.NotContains(t, formatted, value) + } + }) + } +} + +// mockRequestTeamIDs returns a count of unique team IDs +func mockRequestTeamIDs(count int) []string { + teamIDs := []string{} + for i := range count { + teamIDs = append(teamIDs, fmt.Sprintf("T%09d", i)) + } + return teamIDs +} diff --git a/cmd/help/help.go b/cmd/help/help.go index 967c9e0a..4027d32e 100644 --- a/cmd/help/help.go +++ b/cmd/help/help.go @@ -96,7 +96,7 @@ const charmHelpTemplate string = `{{.Long | ToDescription}} {{if eq .Name (GetProcessName)}}{{Header "Commands"}}{{range .Commands}}{{if and (.HasAvailableSubCommands) (not .Hidden)}} {{.Name | ToGroupName }}{{range .Commands}}{{if (not .Hidden)}} {{rpad .Name .NamePadding | ToCommandText}} {{.Short | ToDescription}}{{end}}{{end}}{{end}}{{end}}{{if and (.HasAvailableSubCommands) (not .Hidden)}}{{range .Commands}}{{if and (not .HasAvailableSubCommands) (not .Hidden)}}{{if not (IsAlias .Name $.Data.Aliases)}} - {{(rpad .Name .NamePadding) | ToGroupName }}{{.Short | ToDescription}}{{end}}{{end}}{{end}}{{end}}{{else}}{{Header "Subcommands"}}{{if and (.HasAvailableSubCommands) (not .Hidden)}}{{range .Commands}}{{if not .HasAvailableSubCommands}} + {{(rpad .Name .NamePadding) | ToGroupName }}{{.Short | ToDescription}}{{end}}{{end}}{{end}}{{end}}{{else}}{{Header "Subcommands"}}{{if and (.HasAvailableSubCommands) (not .Hidden)}}{{range .Commands}}{{if and (not .HasAvailableSubCommands) (not .Hidden)}} {{(rpad .Name .NamePadding) | ToCommandText }} {{.Short | ToDescription}}{{end}}{{end}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}} {{Header "Flags"}} @@ -139,7 +139,7 @@ const legacyHelpTemplate string = `{{.Long}} {{if eq .Name (GetProcessName)}}{{Header "Commands"}}{{range .Commands}}{{if and (.HasAvailableSubCommands) (not .Hidden)}} {{.Name | ToCommandText }}{{range .Commands}}{{if (not .Hidden)}} {{rpad .Name .NamePadding }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{if and (.HasAvailableSubCommands) (not .Hidden)}}{{range .Commands}}{{if and (not .HasAvailableSubCommands) (not .Hidden)}}{{if not (IsAlias .Name $.Data.Aliases)}} - {{(rpad .Name .NamePadding) | ToCommandText }}{{.Short}}{{end}}{{end}}{{end}}{{end}}{{else}}{{Header "Subcommands"}}{{if and (.HasAvailableSubCommands) (not .Hidden)}}{{range .Commands}}{{if not .HasAvailableSubCommands}} + {{(rpad .Name .NamePadding) | ToCommandText }}{{.Short}}{{end}}{{end}}{{end}}{{end}}{{else}}{{Header "Subcommands"}}{{if and (.HasAvailableSubCommands) (not .Hidden)}}{{range .Commands}}{{if and (not .HasAvailableSubCommands) (not .Hidden)}} {{(rpad .Name .NamePadding) | ToCommandText }} {{.Short}}{{end}}{{end}}{{end}}{{end}}{{end}}{{if .HasAvailableLocalFlags}} {{Header "Flags"}} diff --git a/docs/reference/experiments.md b/docs/reference/experiments.md index a5989207..edb38465 100644 --- a/docs/reference/experiments.md +++ b/docs/reference/experiments.md @@ -6,6 +6,7 @@ The Slack CLI has an experiment (`-e`) flag behind which we put features current The following is a list of currently available experiments. We'll remove experiments from this page if we decide they are no longer needed or once they are released, in which case we'll make an announcement about the feature's general availability in the [developer changelog](https://docs.slack.dev/changelog). +- `app-approval-status`: checks requests to install an app. - `lipgloss`: shows pretty styles. - `manifest-sync`: resolves conflicting app manifest values. @@ -13,6 +14,7 @@ The following is a list of currently available experiments. We'll remove experim Below is a list of updates related to experiments. +- **August 2026**: Added the `app-approval-status` experiment to check the status of requests to have an app approved for install. - **August 2026**: Concluded the `set-icon` experiment; The Slack CLI now offers full support for icon upload on all app types by default. - **July 2026**: Added the `manifest-sync` experiment to resolve changed app manifest values between a project and app settings. - **April 2026**: Concluded the `sandboxes` experiment with full support in the Slack CLI. Refer to the [`slack sandbox create`](/tools/slack-cli/reference/commands/slack_sandbox_create/), [`slack sandbox delete`](/tools/slack-cli/reference/commands/slack_sandbox_delete/), and [`slack sandbox list`](/tools/slack-cli/reference/commands/slack_sandbox_list/) commands for more details. diff --git a/internal/api/api_mock.go b/internal/api/api_mock.go index 3feb315f..7dbb330b 100644 --- a/internal/api/api_mock.go +++ b/internal/api/api_mock.go @@ -315,6 +315,11 @@ func (m *APIMock) CertifiedAppInstall(ctx context.Context, token string, certifi return args.Get(0).(CertifiedInstallResult), args.Error(1) } +func (m *APIMock) ListAppApprovalRequests(ctx context.Context, token string, appID string, requestedTeams []string) (AppsApprovalsRequestsListResult, error) { + args := m.Called(ctx, token, appID, requestedTeams) + return args.Get(0).(AppsApprovalsRequestsListResult), args.Error(1) +} + func (m *APIMock) RequestAppApproval(ctx context.Context, token string, appID string, teamID string, reason string, scopes string, outgoingDomains []string) (AppsApprovalsRequestsCreateResult, error) { args := m.Called(ctx, token, appID, teamID, reason, scopes, outgoingDomains) return args.Get(0).(AppsApprovalsRequestsCreateResult), args.Error(1) diff --git a/internal/api/app.go b/internal/api/app.go index a871dcd5..64885afc 100644 --- a/internal/api/app.go +++ b/internal/api/app.go @@ -45,6 +45,7 @@ const ( appStatusMethod = "apps.status" appApprovalRequestCreateMethod = "apps.approvals.requests.create" appApprovalRequestCancelMethod = "apps.approvals.requests.cancel" + appApprovalRequestListMethod = "apps.approvals.requests.list" ) // AppsClient is the interface for app-related API calls @@ -60,6 +61,7 @@ type AppsClient interface { Host() string Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) IconSet(ctx context.Context, fs afero.Fs, token, appID, iconFilePath string) (IconResult, error) + ListAppApprovalRequests(ctx context.Context, token string, appID string, requestedTeams []string) (AppsApprovalsRequestsListResult, error) RequestAppApproval(ctx context.Context, token string, appID string, teamID string, reason string, scopes string, outgoingDomains []string) (AppsApprovalsRequestsCreateResult, error) SetHost(host string) UninstallApp(ctx context.Context, token string, appID, teamID string) error @@ -587,6 +589,109 @@ type appsApprovalsRequestsCancelResponse struct { extendedBaseResponse } +// AppsApprovalsRequestStatus is where an app approval request stands. A pending +// request has not been resolved yet, while approved, denied, and cancelled +// requests are settled. The status is a property of the request alone, so it +// reads the same for every caller. +type AppsApprovalsRequestStatus string + +const ( + AppsApprovalsRequestStatusApproved AppsApprovalsRequestStatus = "approved" + AppsApprovalsRequestStatusCancelled AppsApprovalsRequestStatus = "cancelled" + AppsApprovalsRequestStatusDenied AppsApprovalsRequestStatus = "denied" + AppsApprovalsRequestStatusPending AppsApprovalsRequestStatus = "pending" +) + +// AppsApprovalsRequestCancelledBy is the kind of actor that cancelled an app +// approval request. A user is the requester withdrawing their own request, an +// admin is someone who resolves approvals on the team, and system is Slack +// cancelling the request with no actor of its own, which is what happens when +// the app named by the request is deleted. +type AppsApprovalsRequestCancelledBy string + +const ( + AppsApprovalsRequestCancelledByAdmin AppsApprovalsRequestCancelledBy = "admin" + AppsApprovalsRequestCancelledBySystem AppsApprovalsRequestCancelledBy = "system" + AppsApprovalsRequestCancelledByUser AppsApprovalsRequestCancelledBy = "user" +) + +// AppsApprovalsRequest is a single request by a user to have an app approved +// for install +type AppsApprovalsRequest struct { + // ID is the encoded ID of the request + ID string `json:"id"` + // TeamID is the team that resolves the request, which is an organization + // for requests routed to an organization and a workspace otherwise + TeamID string `json:"team_id"` + // Status is where the request stands + Status AppsApprovalsRequestStatus `json:"status"` + // CanSelfApprove is true if the account can install the app on the team + // without approval from anyone else + CanSelfApprove bool `json:"can_self_approve"` + // DateCreated is the Unix timestamp of when the request was created + DateCreated int64 `json:"date_created"` + // DateResolved is the Unix timestamp of when the request was resolved and + // is absent while the request remains open + DateResolved int64 `json:"date_resolved,omitempty"` + // CancelledBy is the kind of actor that cancelled the request and is absent + // unless the request was cancelled + CancelledBy AppsApprovalsRequestCancelledBy `json:"cancelled_by,omitempty"` +} + +type AppsApprovalsRequestsListResult struct { + // Requests contains the most recent request that the authenticated account + // made for an app on each of the searched teams + Requests []AppsApprovalsRequest `json:"requests"` +} + +type appsApprovalsRequestsListResponse struct { + extendedBaseResponse + AppsApprovalsRequestsListResult +} + +// ListAppApprovalRequests fetches the most recent request that the authenticated +// account made to install an app on each of the searched teams. +// +// The teams searched are the team of the token, the organization of that team +// if the token is scoped to a workspace of one, and any workspace of that +// organization named in requestedTeams. +func (c *Client) ListAppApprovalRequests(ctx context.Context, token string, appID string, requestedTeams []string) (AppsApprovalsRequestsListResult, error) { + var span opentracing.Span + span, ctx = opentracing.StartSpanFromContext(ctx, "apiclient.ListAppApprovalRequests") + defer span.Finish() + + args := struct { + AppID string `json:"app_id"` + RequestedTeams []string `json:"requested_teams,omitempty"` + }{ + appID, + requestedTeams, + } + + body, err := json.Marshal(args) + if err != nil { + return AppsApprovalsRequestsListResult{}, errInvalidArguments.WithRootCause(err) + } + + b, err := c.postJSON(ctx, appApprovalRequestListMethod, token, "", body) + if err != nil { + return AppsApprovalsRequestsListResult{}, errHTTPRequestFailed.WithRootCause(err) + } + + resp := appsApprovalsRequestsListResponse{} + err = goutils.JSONUnmarshal(b, &resp) + + if err != nil { + return AppsApprovalsRequestsListResult{}, errHTTPResponseInvalid.WithRootCause(err).AddAPIMethod(appApprovalRequestListMethod) + } + + if !resp.Ok { + return AppsApprovalsRequestsListResult{}, slackerror.NewAPIError(resp.Error, resp.Description, resp.Errors, appApprovalRequestListMethod) + } + + return resp.AppsApprovalsRequestsListResult, nil +} + // GenerateS3PresignedPost details to be saved type AppsConnectionsOpenResult struct { URL string `json:"url"` diff --git a/internal/api/app_test.go b/internal/api/app_test.go index 873f7b49..3884d788 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -154,6 +154,108 @@ func Test_Client_GetAppStatus(t *testing.T) { } } +func Test_Client_ListAppApprovalRequests(t *testing.T) { + tests := map[string]struct { + appID string + requestedTeams []string + expectedRequest string + httpResponseJSON string + expectedRequests []AppsApprovalsRequest + expectedError string + }{ + "omits the requested teams when none are named": { + appID: "A1234", + expectedRequest: `{"app_id":"A1234"}`, + httpResponseJSON: `{"ok":true,"requests":[]}`, + expectedRequests: []AppsApprovalsRequest{}, + }, + "includes the requested teams of an organization": { + appID: "A1234", + requestedTeams: []string{"T1234", "T5678"}, + expectedRequest: `{"app_id":"A1234","requested_teams":["T1234","T5678"]}`, + httpResponseJSON: `{"ok":true,"requests":[]}`, + expectedRequests: []AppsApprovalsRequest{}, + }, + "collects an open request that awaits review": { + appID: "A1234", + expectedRequest: `{"app_id":"A1234"}`, + httpResponseJSON: `{"ok":true,"requests":[{"id":"Ar1234","team_id":"T1234","status":"pending","can_self_approve":false,"date_created":1787000000}]}`, + expectedRequests: []AppsApprovalsRequest{ + { + ID: "Ar1234", + TeamID: "T1234", + Status: AppsApprovalsRequestStatusPending, + DateCreated: 1787000000, + }, + }, + }, + "collects a settled request of an organization and a workspace": { + appID: "A1234", + expectedRequest: `{"app_id":"A1234"}`, + httpResponseJSON: `{"ok":true,"requests":[{"id":"Ar1234","team_id":"E1234","status":"cancelled","can_self_approve":true,"date_created":1787000000,"date_resolved":1787060000,"cancelled_by":"admin"},{"id":"Ar5678","team_id":"T5678","status":"denied","can_self_approve":false,"date_created":1787000000,"date_resolved":1787060000}]}`, + expectedRequests: []AppsApprovalsRequest{ + { + ID: "Ar1234", + TeamID: "E1234", + Status: AppsApprovalsRequestStatusCancelled, + CanSelfApprove: true, + DateCreated: 1787000000, + DateResolved: 1787060000, + CancelledBy: AppsApprovalsRequestCancelledByAdmin, + }, + { + ID: "Ar5678", + TeamID: "T5678", + Status: AppsApprovalsRequestStatusDenied, + DateCreated: 1787000000, + DateResolved: 1787060000, + }, + }, + }, + "errors when the app is not found": { + appID: "A0000", + expectedRequest: `{"app_id":"A0000"}`, + httpResponseJSON: `{"ok":false,"error":"app_not_found"}`, + expectedError: "app_not_found", + }, + "errors when a team is outside of the organization": { + appID: "A1234", + requestedTeams: []string{"T0000"}, + expectedRequest: `{"app_id":"A1234","requested_teams":["T0000"]}`, + httpResponseJSON: `{"ok":false,"error":"restricted_action"}`, + expectedError: "restricted_action", + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + c, teardown := NewFakeClient(t, FakeClientParams{ + ExpectedMethod: appApprovalRequestListMethod, + ExpectedRequest: tc.expectedRequest, + Response: tc.httpResponseJSON, + }) + defer teardown() + + result, err := c.ListAppApprovalRequests(ctx, "token", tc.appID, tc.requestedTeams) + if tc.expectedError != "" { + require.Error(t, err) + require.Contains(t, err.Error(), tc.expectedError) + return + } + require.NoError(t, err) + require.Equal(t, tc.expectedRequests, result.Requests) + }) + } +} + +func Test_Client_ListAppApprovalRequests_CommonErrors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + verifyCommonErrorCases(t, appApprovalRequestListMethod, func(c *Client) error { + _, err := c.ListAppApprovalRequests(ctx, "token", "A1234", nil) + return err + }) +} + func TestClient_UpdateApp_OK(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ diff --git a/internal/experiment/experiment.go b/internal/experiment/experiment.go index c34894c1..d4ad75b6 100644 --- a/internal/experiment/experiment.go +++ b/internal/experiment/experiment.go @@ -30,6 +30,10 @@ type Experiment string // e.g. --experiment=first-toggle,second-toggle const ( + + // AppApprovalStatus experiment shows the requested install approval status of the app. + AppApprovalStatus Experiment = "app-approval-status" + // Lipgloss experiment shows pretty styles. Lipgloss Experiment = "lipgloss" @@ -43,6 +47,7 @@ const ( // AllExperiments is a list of all available experiments that can be enabled // Please also add here 👇 var AllExperiments = []Experiment{ + AppApprovalStatus, Lipgloss, ManifestSync, Placeholder, diff --git a/internal/slackerror/errors.go b/internal/slackerror/errors.go index e1b9404b..25099bbe 100644 --- a/internal/slackerror/errors.go +++ b/internal/slackerror/errors.go @@ -114,6 +114,7 @@ const ( ErrFailedExport = "failed_export" ErrFailedToGetUser = "failed_to_get_user" ErrFailedToSaveExtensionLogs = "failed_to_save_extension_logs" + ErrFeatureNotEnabled = "feature_not_enabled" ErrFeedbackNameInvalid = "feedback_name_invalid" ErrFeedbackNameRequired = "feedback_name_required" ErrFileRejected = "file_rejected" @@ -224,6 +225,7 @@ const ( ErrPublishedAppOnly = "published_app_only" ErrRatelimited = "ratelimited" ErrRequestIDOrAppIDIsRequired = "request_id_or_app_id_is_required" + ErrRestrictedAction = "restricted_action" ErrRestrictedPlanLevel = "restricted_plan_level" ErrRuntimeNotFound = "runtime_not_found" ErrRuntimeNotSupported = "runtime_not_supported" @@ -787,6 +789,12 @@ Otherwise start your app for local development with: %s`, Message: "Couldn't save the logs", }, + ErrFeatureNotEnabled: { + Code: ErrFeatureNotEnabled, + Message: "This feature is not enabled for the team", + Remediation: "Reach out to an admin for additional information", + }, + ErrFeedbackNameInvalid: { Code: ErrFeedbackNameInvalid, Message: "The name of the feedback is invalid", @@ -1382,6 +1390,12 @@ Otherwise start your app for local development with: %s`, Message: "Must include a request_id or app_id", }, + ErrRestrictedAction: { + Code: ErrRestrictedAction, + Message: "The requested action is not allowed for a specified team", + Remediation: "Check that each team belongs to the organization of the authenticated account", + }, + ErrRestrictedPlanLevel: { Code: ErrRestrictedPlanLevel, Message: "Your Slack plan does not have access to the requested feature", From 1e4be76aca0a4f14cd637145131bee092f965543 Mon Sep 17 00:00:00 2001 From: Amy Wah Date: Mon, 24 Aug 2026 11:00:04 -0400 Subject: [PATCH 2/6] test: cover error paths and formatting fallbacks of app requests Exercise the interrupted app selection and missing app ID branches of the command, plus the unknown timestamp, status, and cancellation actor fallbacks of the output. Co-authored-by: Cursor --- cmd/app/requests_test.go | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/cmd/app/requests_test.go b/cmd/app/requests_test.go index 2098e526..e77cfb7b 100644 --- a/cmd/app/requests_test.go +++ b/cmd/app/requests_test.go @@ -122,6 +122,32 @@ func TestRequestsCommand(t *testing.T) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") }, }, + "returns the error of an interrupted app selection": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + requestsAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { + return prompts.SelectedApp{}, slackerror.New(slackerror.ErrProcessInterrupted) + } + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrProcessInterrupted), + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "ListAppApprovalRequests") + }, + }, + "errors when the selected app is missing an ID": { + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + requestsAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { + return prompts.SelectedApp{Auth: types.SlackAuth{Token: "xoxp-example"}}, nil + } + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrAppNotFound), + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "ListAppApprovalRequests") + }, + }, "returns the error of a failed lookup": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { enableRequests(ctx, cm, cf) @@ -198,6 +224,25 @@ func TestRequestsFormat(t *testing.T) { "You can install this app without approval. Please cancel the request.", }, }, + "a request without a timestamp reports an unknown moment": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusPending}, + }, + Expected: []string{"Requested: unknown"}, + Unexpected: []string{"Resolved:"}, + }, + "an unrecognized status is reported without styles": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatus("escalated"), DateCreated: mockRequestCreated}, + }, + Expected: []string{"Status: escalated"}, + }, + "an unrecognized cancellation actor is reported as named": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated, CancelledBy: api.AppsApprovalsRequestCancelledBy("workflow")}, + }, + Expected: []string{"Cancelled by: workflow"}, + }, "requests are sorted by the team ID": { Requests: []api.AppsApprovalsRequest{ {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated}, From 8826cbf62e210a55ad74b44f3e4d8288a8aa0b9e Mon Sep 17 00:00:00 2001 From: Amy Wah Date: Mon, 24 Aug 2026 13:39:54 -0400 Subject: [PATCH 3/6] feat: check requests for an app named by ID without a project The app select prompt only offers apps saved to a project, so apps created elsewhere could not be checked. An app ID provided with the --app flag now skips both the project requirement and the project app list, gathering a token from the authenticated accounts instead. Co-authored-by: Cursor --- cmd/app/requests.go | 47 +++++++++++++++++++++++++--- cmd/app/requests_test.go | 67 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) diff --git a/cmd/app/requests.go b/cmd/app/requests.go index 43217d66..40037b8d 100644 --- a/cmd/app/requests.go +++ b/cmd/app/requests.go @@ -15,6 +15,7 @@ package app import ( + "context" "fmt" "sort" "strings" @@ -26,6 +27,7 @@ import ( "github.com/slackapi/slack-cli/internal/experiment" "github.com/slackapi/slack-cli/internal/prompts" "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" "github.com/spf13/cobra" @@ -40,6 +42,9 @@ const requestsTimeFormat = "2006-01-02 15:04:05 Z07:00" // Handle to a function used for testing var requestsAppSelectPromptFunc = prompts.AppSelectPrompt +// Handle to a function used for testing +var requestsTeamSelectPromptFunc = prompts.PromptTeamSlackAuth + // Flags type requestsCmdFlags struct { @@ -63,10 +68,14 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { "while an account of an organization searches the organization alone.", "", "Other workspaces of an organization can be searched with the --team-ids flag.", + "", + "Apps saved to a project are chosen with a prompt, but any app can be checked", + "by app ID with the --app flag, which does not require a project.", }, "\n"), Hidden: true, Example: style.ExampleCommandsf([]style.ExampleCommand{ {Command: "app requests", Meaning: "Check requests to install an app"}, + {Command: "app requests --app A0123456789", Meaning: "Check requests for an app outside a project"}, {Command: "app requests --team-ids T0123456789,T9876543210", Meaning: "Check requests on certain teams of an organization"}, }), Args: cobra.NoArgs, @@ -86,6 +95,10 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { ) } clients.Config.SetFlags(cmd) + // An app named by ID is checked without the apps of a project + if types.IsAppID(clients.Config.AppFlag) { + return nil + } // Verify command is run in a project directory return cmdutil.IsValidProjectDirectory(clients) }, @@ -105,15 +118,12 @@ func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.app.requests") defer span.Finish() - selection, err := requestsAppSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAndUninstalledApps) + appID, token, err := requestsAppSelection(ctx, clients) if err != nil { return err } - if selection.App.AppID == "" { - return slackerror.New(slackerror.ErrAppNotFound) - } - result, err := clients.API().ListAppApprovalRequests(ctx, selection.Auth.Token, selection.App.AppID, requestsFlags.teamIDs) + result, err := clients.API().ListAppApprovalRequests(ctx, token, appID, requestsFlags.teamIDs) if err != nil { return err } @@ -126,6 +136,33 @@ func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error return nil } +// requestsAppSelection decides the app to check and a token of the app team. +// +// An app named by ID with the app flag is checked without a project so that +// apps missing from a project can be checked too. The team of that app is +// gathered from the authenticated accounts instead of the project apps. +func requestsAppSelection(ctx context.Context, clients *shared.ClientFactory) (appID string, token string, err error) { + if types.IsAppID(clients.Config.AppFlag) { + auth, err := requestsTeamSelectPromptFunc(ctx, clients, "Select the team of the app", nil) + if err != nil { + return "", "", err + } + if auth == nil || auth.Token == "" { + return "", "", slackerror.New(slackerror.ErrCredentialsNotFound) + } + clients.Auth().SetSelectedAuth(ctx, *auth, clients.Config, clients.Os) + return clients.Config.AppFlag, auth.Token, nil + } + selection, err := requestsAppSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAndUninstalledApps) + if err != nil { + return "", "", err + } + if selection.App.AppID == "" { + return "", "", slackerror.New(slackerror.ErrAppNotFound) + } + return selection.App.AppID, selection.Auth.Token, nil +} + // FormatRequestsSuccess formats the install request of each team func FormatRequestsSuccess(requests []api.AppsApprovalsRequest) (secondaryText []string) { sort.Slice(requests, func(i, j int) bool { diff --git a/cmd/app/requests_test.go b/cmd/app/requests_test.go index e77cfb7b..36d1e487 100644 --- a/cmd/app/requests_test.go +++ b/cmd/app/requests_test.go @@ -57,6 +57,14 @@ func TestRequestsCommand(t *testing.T) { restoreRequests := func() { requestsAppSelectPromptFunc = prompts.AppSelectPrompt + requestsTeamSelectPromptFunc = prompts.PromptTeamSlackAuth + } + + // enableRequestsWithoutProject turns on the experiment outside of a project + enableRequestsWithoutProject := func(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + cm.AddDefaultMocks() + cf.Config.ExperimentsFlag = []string{string(experiment.AppApprovalStatus)} + cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) } testutil.TableTestCommand(t, testutil.CommandTests{ @@ -122,6 +130,65 @@ func TestRequestsCommand(t *testing.T) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") }, }, + "checks an app named by ID outside of a project": { + CmdArgs: []string{"--app", "A5678"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequestsWithoutProject(ctx, cm, cf) + requestsTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { + return &types.SlackAuth{Token: "xoxp-selected", TeamID: "T5678"}, nil + } + cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.AppsApprovalsRequestsListResult{ + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusApproved, DateCreated: mockRequestCreated}, + }, + }, nil) + }, + Teardown: restoreRequests, + ExpectedOutputs: []string{"Status: approved"}, + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertCalled(t, "ListAppApprovalRequests", mock.Anything, "xoxp-selected", "A5678", []string(nil)) + }, + }, + "returns the error of a failed team selection": { + CmdArgs: []string{"--app", "A5678"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequestsWithoutProject(ctx, cm, cf) + requestsTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { + return nil, slackerror.New(slackerror.ErrProcessInterrupted) + } + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrProcessInterrupted), + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "ListAppApprovalRequests") + }, + }, + "errors when the selected team has no token": { + CmdArgs: []string{"--app", "A5678"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequestsWithoutProject(ctx, cm, cf) + requestsTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { + return &types.SlackAuth{TeamID: "T5678"}, nil + } + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrCredentialsNotFound), + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "ListAppApprovalRequests") + }, + }, + "errors without a project when an app environment is used": { + CmdArgs: []string{"--app", "local"}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequestsWithoutProject(ctx, cm, cf) + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrInvalidAppDirectory), + ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { + cm.API.AssertNotCalled(t, "ListAppApprovalRequests") + }, + }, "returns the error of an interrupted app selection": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { enableRequests(ctx, cm, cf) From 4a96146c749a9834895f50bc487bf878be24c2b2 Mon Sep 17 00:00:00 2001 From: Amy Wah Date: Mon, 24 Aug 2026 14:43:14 -0400 Subject: [PATCH 4/6] add ability to pass in an appId --- cmd/app/requests.go | 35 ++++++++++++++++++++++------------- cmd/app/requests_test.go | 18 ++++++++++++++++-- 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/cmd/app/requests.go b/cmd/app/requests.go index 40037b8d..13aeb8b2 100644 --- a/cmd/app/requests.go +++ b/cmd/app/requests.go @@ -69,6 +69,9 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { "", "Other workspaces of an organization can be searched with the --team-ids flag.", "", + "Searches are made with the credentials of an authenticated account chosen", + "with the --team flag or a prompt.", + "", "Apps saved to a project are chosen with a prompt, but any app can be checked", "by app ID with the --app flag, which does not require a project.", }, "\n"), @@ -131,7 +134,7 @@ func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ Emoji: "lock", Text: "App Requests", - Secondary: FormatRequestsSuccess(result.Requests), + Secondary: FormatRequestsSuccess(appID, result.Requests), })) return nil } @@ -143,7 +146,7 @@ func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error // gathered from the authenticated accounts instead of the project apps. func requestsAppSelection(ctx context.Context, clients *shared.ClientFactory) (appID string, token string, err error) { if types.IsAppID(clients.Config.AppFlag) { - auth, err := requestsTeamSelectPromptFunc(ctx, clients, "Select the team of the app", nil) + auth, err := requestsTeamSelectPromptFunc(ctx, clients, "Select an account to search with", nil) if err != nil { return "", "", err } @@ -163,32 +166,38 @@ func requestsAppSelection(ctx context.Context, clients *shared.ClientFactory) (a return selection.App.AppID, selection.Auth.Token, nil } -// FormatRequestsSuccess formats the install request of each team -func FormatRequestsSuccess(requests []api.AppsApprovalsRequest) (secondaryText []string) { +// FormatRequestsSuccess formats the install request of each team for an app +func FormatRequestsSuccess(appID string, requests []api.AppsApprovalsRequest) (secondaryText []string) { sort.Slice(requests, func(i, j int) bool { return requests[i].TeamID < requests[j].TeamID }) field := func(label string, value string) string { return fmt.Sprintf(style.Indent(style.Secondary("%-13s %s")), label+":", value) } + if appID != "" { + secondaryText = append(secondaryText, fmt.Sprintf(style.Bold("%-13s %s"), "App ID:", appID)) + } + // Requests are gathered apart from the app to know when none were made + requestsText := []string{} for _, request := range requests { - secondaryText = append(secondaryText, fmt.Sprintf(style.Bold("%s:"), request.TeamID)) - secondaryText = append(secondaryText, field("Request ID", request.ID)) - secondaryText = append(secondaryText, field("Status", formatRequestStatus(request.Status))) - secondaryText = append(secondaryText, field("Requested", formatRequestTime(request.DateCreated))) + requestsText = append(requestsText, fmt.Sprintf(style.Bold("%s:"), request.TeamID)) + requestsText = append(requestsText, field("Request ID", request.ID)) + requestsText = append(requestsText, field("Status", formatRequestStatus(request.Status))) + requestsText = append(requestsText, field("Requested", formatRequestTime(request.DateCreated))) if request.DateResolved > 0 { - secondaryText = append(secondaryText, field("Resolved", formatRequestTime(request.DateResolved))) + requestsText = append(requestsText, field("Resolved", formatRequestTime(request.DateResolved))) } if request.CancelledBy != "" { - secondaryText = append(secondaryText, field("Cancelled by", formatRequestCancelledBy(request.CancelledBy))) + requestsText = append(requestsText, field("Cancelled by", formatRequestCancelledBy(request.CancelledBy))) } if request.CanSelfApprove { - secondaryText = append(secondaryText, style.Indent(style.Secondary("You can install this app without approval. Please cancel the request."))) + requestsText = append(requestsText, style.Indent(style.Secondary("You can install this app without approval. Please cancel the request."))) } } - if len(secondaryText) <= 0 { - secondaryText = append(secondaryText, "You have not requested to install this app") + if len(requestsText) <= 0 { + requestsText = append(requestsText, "You have not requested to install this app") } + secondaryText = append(secondaryText, requestsText...) return } diff --git a/cmd/app/requests_test.go b/cmd/app/requests_test.go index 36d1e487..de9dc2e3 100644 --- a/cmd/app/requests_test.go +++ b/cmd/app/requests_test.go @@ -89,6 +89,7 @@ func TestRequestsCommand(t *testing.T) { Teardown: restoreRequests, ExpectedOutputs: []string{ "App Requests", + "App ID: A1234", "T1234", "Request ID: Ar1234", "Status: pending", @@ -237,7 +238,20 @@ func TestRequestsFormat(t *testing.T) { }{ "no request was made for the app": { Requests: []api.AppsApprovalsRequest{}, - Expected: []string{"You have not requested to install this app"}, + Expected: []string{ + "App ID: A1234", + "You have not requested to install this app", + }, + }, + "the app is named before the requests of each team": { + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + }, + Expected: []string{ + "App ID: A1234", + "T1234", + "Request ID: Ar1234", + }, }, "an open request omits the resolved timestamp": { Requests: []api.AppsApprovalsRequest{ @@ -326,7 +340,7 @@ func TestRequestsFormat(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - formatted := strings.Join(FormatRequestsSuccess(tc.Requests), "\n") + formatted := strings.Join(FormatRequestsSuccess("A1234", tc.Requests), "\n") previous := -1 for _, value := range tc.Expected { index := strings.Index(formatted, value) From a82393a1923af0045a7cba646361c2a2339bec02 Mon Sep 17 00:00:00 2001 From: Amy Wah Date: Tue, 25 Aug 2026 11:17:21 -0400 Subject: [PATCH 5/6] address review comments --- cmd/app/requests.go | 31 +++++++-------- cmd/app/requests_test.go | 21 ++++++++--- internal/api/app_test.go | 33 +++++++++++++--- internal/prompts/team_select.go | 1 + internal/prompts/team_select_test.go | 56 ++++++++++++++++++++++++++++ 5 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 internal/prompts/team_select_test.go diff --git a/cmd/app/requests.go b/cmd/app/requests.go index 13aeb8b2..02f2201b 100644 --- a/cmd/app/requests.go +++ b/cmd/app/requests.go @@ -33,9 +33,6 @@ import ( "github.com/spf13/cobra" ) -// requestsTeamsLimit is the most teams the API searches in a single call -const requestsTeamsLimit = 50 - // requestsTimeFormat displays the moment a request changed const requestsTimeFormat = "2006-01-02 15:04:05 Z07:00" @@ -48,7 +45,7 @@ var requestsTeamSelectPromptFunc = prompts.PromptTeamSlackAuth // Flags type requestsCmdFlags struct { - teamIDs []string + workspaceIDs []string } var requestsFlags requestsCmdFlags @@ -67,7 +64,8 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { "a workspace that belongs to an organization also searches that organization,", "while an account of an organization searches the organization alone.", "", - "Other workspaces of an organization can be searched with the --team-ids flag.", + "Other workspaces of an organization can be searched with the --workspace-ids", + "flag.", "", "Searches are made with the credentials of an authenticated account chosen", "with the --team flag or a prompt.", @@ -79,7 +77,7 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { Example: style.ExampleCommandsf([]style.ExampleCommand{ {Command: "app requests", Meaning: "Check requests to install an app"}, {Command: "app requests --app A0123456789", Meaning: "Check requests for an app outside a project"}, - {Command: "app requests --team-ids T0123456789,T9876543210", Meaning: "Check requests on certain teams of an organization"}, + {Command: "app requests --workspace-ids T0123456789,T9876543210", Meaning: "Check requests on certain workspaces of an organization"}, }), Args: cobra.NoArgs, PreRunE: func(cmd *cobra.Command, args []string) error { @@ -90,27 +88,27 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { style.CommandText("--experiment app-approval-status"), ) } - if len(requestsFlags.teamIDs) > requestsTeamsLimit { - return slackerror.New(slackerror.ErrInvalidArguments). - WithMessage("The %s flag accepts at most %d teams", - style.CommandText("--team-ids"), - requestsTeamsLimit, - ) - } clients.Config.SetFlags(cmd) // An app named by ID is checked without the apps of a project if types.IsAppID(clients.Config.AppFlag) { return nil } // Verify command is run in a project directory - return cmdutil.IsValidProjectDirectory(clients) + if err := cmdutil.IsValidProjectDirectory(clients); err != nil { + invalid := slackerror.ToSlackError(err) + return invalid.WithRemediation("%s\n\nApps of other projects can be checked with %s", + invalid.Remediation, + style.CommandText("--app A0123456789"), + ) + } + return nil }, RunE: func(cmd *cobra.Command, args []string) error { return runRequestsCommand(cmd, clients) }, } - cmd.Flags().StringSliceVar(&requestsFlags.teamIDs, "team-ids", nil, "also check these teams of an organization,\nwith a maximum of 50 teams") + cmd.Flags().StringSliceVar(&requestsFlags.workspaceIDs, "workspace-ids", nil, "also check these workspaces of an organization,\nwith a maximum of 50 workspaces") return cmd } @@ -126,7 +124,7 @@ func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error return err } - result, err := clients.API().ListAppApprovalRequests(ctx, token, appID, requestsFlags.teamIDs) + result, err := clients.API().ListAppApprovalRequests(ctx, token, appID, requestsFlags.workspaceIDs) if err != nil { return err } @@ -153,7 +151,6 @@ func requestsAppSelection(ctx context.Context, clients *shared.ClientFactory) (a if auth == nil || auth.Token == "" { return "", "", slackerror.New(slackerror.ErrCredentialsNotFound) } - clients.Auth().SetSelectedAuth(ctx, *auth, clients.Config, clients.Os) return clients.Config.AppFlag, auth.Token, nil } selection, err := requestsAppSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAndUninstalledApps) diff --git a/cmd/app/requests_test.go b/cmd/app/requests_test.go index de9dc2e3..8650b154 100644 --- a/cmd/app/requests_test.go +++ b/cmd/app/requests_test.go @@ -108,8 +108,8 @@ func TestRequestsCommand(t *testing.T) { Teardown: restoreRequests, ExpectedOutputs: []string{"You have not requested to install this app"}, }, - "searches the teams of the provided team IDs": { - CmdArgs: []string{"--team-ids", "T1234,T5678"}, + "searches the workspaces of the provided workspace IDs": { + CmdArgs: []string{"--workspace-ids", "T1234,T5678"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { enableRequests(ctx, cm, cf) cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). @@ -120,17 +120,26 @@ func TestRequestsCommand(t *testing.T) { cm.API.AssertCalled(t, "ListAppApprovalRequests", mock.Anything, "xoxp-example", "A1234", []string{"T1234", "T5678"}) }, }, - "errors when more than fifty teams are provided": { - CmdArgs: []string{"--team-ids", strings.Join(mockRequestTeamIDs(requestsTeamsLimit+1), ",")}, + "suggests the app flag without a project directory": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) + enableRequestsWithoutProject(ctx, cm, cf) }, Teardown: restoreRequests, - ExpectedErrorStrings: []string{"--team-ids", "at most 50 teams"}, + ExpectedErrorStrings: []string{slackerror.ErrInvalidAppDirectory, "hooks.json", "--app A0123456789"}, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") }, }, + "returns the error of too many searched workspaces": { + CmdArgs: []string{"--workspace-ids", strings.Join(mockRequestTeamIDs(51), ",")}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequests(ctx, cm, cf) + cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.AppsApprovalsRequestsListResult{}, slackerror.New(slackerror.ErrInvalidArguments)) + }, + Teardown: restoreRequests, + ExpectedError: slackerror.New(slackerror.ErrInvalidArguments), + }, "checks an app named by ID outside of a project": { CmdArgs: []string{"--app", "A5678"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { diff --git a/internal/api/app_test.go b/internal/api/app_test.go index 3884d788..c1b4748f 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -161,7 +161,7 @@ func Test_Client_ListAppApprovalRequests(t *testing.T) { expectedRequest string httpResponseJSON string expectedRequests []AppsApprovalsRequest - expectedError string + expectedErrors []string }{ "omits the requested teams when none are named": { appID: "A1234", @@ -216,14 +216,35 @@ func Test_Client_ListAppApprovalRequests(t *testing.T) { appID: "A0000", expectedRequest: `{"app_id":"A0000"}`, httpResponseJSON: `{"ok":false,"error":"app_not_found"}`, - expectedError: "app_not_found", + expectedErrors: []string{slackerror.ErrAppNotFound, "The app was not found"}, }, "errors when a team is outside of the organization": { appID: "A1234", requestedTeams: []string{"T0000"}, expectedRequest: `{"app_id":"A1234","requested_teams":["T0000"]}`, httpResponseJSON: `{"ok":false,"error":"restricted_action"}`, - expectedError: "restricted_action", + expectedErrors: []string{ + slackerror.ErrRestrictedAction, + "The requested action is not allowed for a specified team", + "Check that each team belongs to the organization", + }, + }, + "errors when the team cannot check requests": { + appID: "A1234", + expectedRequest: `{"app_id":"A1234"}`, + httpResponseJSON: `{"ok":false,"error":"feature_not_enabled"}`, + expectedErrors: []string{ + slackerror.ErrFeatureNotEnabled, + "This feature is not enabled for the team", + "Reach out to an admin for additional information", + }, + }, + "errors when more than fifty workspaces are searched": { + appID: "A1234", + requestedTeams: []string{"T0000"}, + expectedRequest: `{"app_id":"A1234","requested_teams":["T0000"]}`, + httpResponseJSON: `{"ok":false,"error":"invalid_arguments"}`, + expectedErrors: []string{slackerror.ErrInvalidArguments}, }, } for name, tc := range tests { @@ -237,9 +258,11 @@ func Test_Client_ListAppApprovalRequests(t *testing.T) { defer teardown() result, err := c.ListAppApprovalRequests(ctx, "token", tc.appID, tc.requestedTeams) - if tc.expectedError != "" { + if len(tc.expectedErrors) > 0 { require.Error(t, err) - require.Contains(t, err.Error(), tc.expectedError) + for _, expected := range tc.expectedErrors { + require.Contains(t, err.Error(), expected) + } return } require.NoError(t, err) diff --git a/internal/prompts/team_select.go b/internal/prompts/team_select.go index 3542a655..d27f008a 100644 --- a/internal/prompts/team_select.go +++ b/internal/prompts/team_select.go @@ -40,6 +40,7 @@ func PromptTeamSlackAuth(ctx context.Context, clients *shared.ClientFactory, pro } if len(allAuths) == 1 { + clients.Auth().SetSelectedAuth(ctx, allAuths[0], clients.Config, clients.Os) return &allAuths[0], nil } diff --git a/internal/prompts/team_select_test.go b/internal/prompts/team_select_test.go new file mode 100644 index 00000000..9269daba --- /dev/null +++ b/internal/prompts/team_select_test.go @@ -0,0 +1,56 @@ +// Copyright 2022-2026 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package prompts + +import ( + "testing" + + "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" +) + +func TestPromptTeamSlackAuth(t *testing.T) { + tests := map[string]struct { + auths []types.SlackAuth + expectedAuth types.SlackAuth + }{ + "selects the only authenticated account without a prompt": { + auths: []types.SlackAuth{ + {Token: team1Token, TeamID: team1TeamID, TeamDomain: team1TeamDomain}, + }, + expectedAuth: types.SlackAuth{Token: team1Token, TeamID: team1TeamID, TeamDomain: team1TeamDomain}, + }, + } + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + clientsMock := shared.NewClientsMock() + clientsMock.Auth.On(Auths, mock.Anything).Return(tc.auths, nil) + clientsMock.AddDefaultMocks() + clients := shared.NewClientFactory(clientsMock.MockClientFactory()) + + auth, err := PromptTeamSlackAuth(ctx, clients, "Select a team", nil) + + require.NoError(t, err) + assert.Equal(t, tc.expectedAuth, *auth) + clientsMock.Auth.AssertCalled(t, "SetSelectedAuth", mock.Anything, tc.expectedAuth, clients.Config, clients.Os) + clientsMock.IO.AssertNotCalled(t, SelectPrompt) + }) + } +} From 7dce5c6068ec5ad89031573628f3b33bee9f8f28 Mon Sep 17 00:00:00 2001 From: Amy Wah Date: Wed, 26 Aug 2026 11:01:00 -0400 Subject: [PATCH 6/6] refactor: address review feedback on the app request command Rename the command to the singular "app request" with "requests" as the only alias, matching the CLI convention of a singular canonical name. Rename the API error codes to ErrAPIFeatureNotEnabled and ErrAPIRestrictedAction so it is clear they mirror responses of the API rather than errors raised by the CLI. Sort a copy of the requests while formatting so the slice of the caller keeps its order, and title the section "App Install Approval Requests" to spell out what is being listed. Co-authored-by: Cursor --- cmd/app/app.go | 2 +- cmd/app/{requests.go => request.go} | 106 +++++++----- cmd/app/{requests_test.go => request_test.go} | 155 +++++++++++------- internal/api/app_test.go | 4 +- internal/slackerror/errors.go | 28 ++-- 5 files changed, 174 insertions(+), 121 deletions(-) rename cmd/app/{requests.go => request.go} (63%) rename cmd/app/{requests_test.go => request_test.go} (73%) diff --git a/cmd/app/app.go b/cmd/app/app.go index b8383082..299388d8 100644 --- a/cmd/app/app.go +++ b/cmd/app/app.go @@ -52,7 +52,7 @@ func NewCommand(clients *shared.ClientFactory) *cobra.Command { cmd.AddCommand(NewDeleteCommand(clients)) cmd.AddCommand(NewLinkCommand(clients)) cmd.AddCommand(NewListCommand(clients)) - cmd.AddCommand(NewRequestsCommand(clients)) + cmd.AddCommand(NewRequestCommand(clients)) cmd.AddCommand(NewSettingsCommand(clients)) cmd.AddCommand(NewUninstallCommand(clients)) cmd.AddCommand(NewUnlinkCommand(clients)) diff --git a/cmd/app/requests.go b/cmd/app/request.go similarity index 63% rename from cmd/app/requests.go rename to cmd/app/request.go index 02f2201b..d4abb88d 100644 --- a/cmd/app/requests.go +++ b/cmd/app/request.go @@ -17,7 +17,7 @@ package app import ( "context" "fmt" - "sort" + "slices" "strings" "time" @@ -33,29 +33,28 @@ import ( "github.com/spf13/cobra" ) -// requestsTimeFormat displays the moment a request changed -const requestsTimeFormat = "2006-01-02 15:04:05 Z07:00" +// requestTimeFormat displays the moment a request changed +const requestTimeFormat = "2006-01-02 15:04:05 Z07:00" // Handle to a function used for testing -var requestsAppSelectPromptFunc = prompts.AppSelectPrompt +var requestAppSelectPromptFunc = prompts.AppSelectPrompt // Handle to a function used for testing -var requestsTeamSelectPromptFunc = prompts.PromptTeamSlackAuth +var requestTeamSelectPromptFunc = prompts.PromptTeamSlackAuth // Flags - -type requestsCmdFlags struct { +type requestCmdFlags struct { workspaceIDs []string } -var requestsFlags requestsCmdFlags +var requestFlags requestCmdFlags -// NewRequestsCommand returns a new Cobra command -func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { +// NewRequestCommand returns a new Cobra command +func NewRequestCommand(clients *shared.ClientFactory) *cobra.Command { cmd := &cobra.Command{ - Use: "requests [flags]", - Aliases: []string{"approval-requests", "approvals"}, - Short: "Check requests to install the app", + Use: "request [flags]", + Aliases: []string{"requests"}, + Short: "Check approval requests to install the app", Long: strings.Join([]string{ "Check the status of your most recent request to have the app approved for", "install.", @@ -75,9 +74,9 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { }, "\n"), Hidden: true, Example: style.ExampleCommandsf([]style.ExampleCommand{ - {Command: "app requests", Meaning: "Check requests to install an app"}, - {Command: "app requests --app A0123456789", Meaning: "Check requests for an app outside a project"}, - {Command: "app requests --workspace-ids T0123456789,T9876543210", Meaning: "Check requests on certain workspaces of an organization"}, + {Command: "app request", Meaning: "Check requests to install an app"}, + {Command: "app request --app A0123456789", Meaning: "Check requests for an app outside a project"}, + {Command: "app request --workspace-ids T0123456789,T9876543210", Meaning: "Check requests on certain workspaces of an organization"}, }), Args: cobra.NoArgs, PreRunE: func(cmd *cobra.Command, args []string) error { @@ -104,69 +103,82 @@ func NewRequestsCommand(clients *shared.ClientFactory) *cobra.Command { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return runRequestsCommand(cmd, clients) + return runRequestCommand(cmd, clients) }, } - cmd.Flags().StringSliceVar(&requestsFlags.workspaceIDs, "workspace-ids", nil, "also check these workspaces of an organization,\nwith a maximum of 50 workspaces") + cmd.Flags().StringSliceVar(&requestFlags.workspaceIDs, "workspace-ids", nil, "also check these workspaces of an organization,\nwith a maximum of 50 workspaces") return cmd } -// runRequestsCommand will execute the requests command -func runRequestsCommand(cmd *cobra.Command, clients *shared.ClientFactory) error { +// runRequestCommand will execute the request command +func runRequestCommand(cmd *cobra.Command, clients *shared.ClientFactory) error { ctx := cmd.Context() - span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.app.requests") + span, ctx := opentracing.StartSpanFromContext(ctx, "cmd.app.request") defer span.Finish() - appID, token, err := requestsAppSelection(ctx, clients) + appID, auth, err := requestAppSelection(ctx, clients) if err != nil { return err } - result, err := clients.API().ListAppApprovalRequests(ctx, token, appID, requestsFlags.workspaceIDs) + result, err := clients.API().ListAppApprovalRequests(ctx, auth.Token, appID, requestFlags.workspaceIDs) if err != nil { return err } clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{ Emoji: "lock", - Text: "App Requests", - Secondary: FormatRequestsSuccess(appID, result.Requests), + Text: "App Install Approval Requests", + Secondary: FormatRequestSuccess(appID, requestTeamNames(auth), result.Requests), })) return nil } -// requestsAppSelection decides the app to check and a token of the app team. +// requestTeamNames collects the names of searched teams that are known. +// +// Requests are returned with team IDs alone, so only the team of the +// authenticated account is named. Other teams of an organization are not +// looked up to avoid another API call. +func requestTeamNames(auth types.SlackAuth) map[string]string { + if auth.TeamID == "" || auth.TeamDomain == "" { + return nil + } + return map[string]string{auth.TeamID: auth.TeamDomain} +} + +// requestAppSelection decides the app to check and the account to search with. // // An app named by ID with the app flag is checked without a project so that // apps missing from a project can be checked too. The team of that app is // gathered from the authenticated accounts instead of the project apps. -func requestsAppSelection(ctx context.Context, clients *shared.ClientFactory) (appID string, token string, err error) { +func requestAppSelection(ctx context.Context, clients *shared.ClientFactory) (appID string, auth types.SlackAuth, err error) { if types.IsAppID(clients.Config.AppFlag) { - auth, err := requestsTeamSelectPromptFunc(ctx, clients, "Select an account to search with", nil) + selected, err := requestTeamSelectPromptFunc(ctx, clients, "Select an account to search with", nil) if err != nil { - return "", "", err + return "", types.SlackAuth{}, err } - if auth == nil || auth.Token == "" { - return "", "", slackerror.New(slackerror.ErrCredentialsNotFound) + if selected == nil || selected.Token == "" { + return "", types.SlackAuth{}, slackerror.New(slackerror.ErrCredentialsNotFound) } - return clients.Config.AppFlag, auth.Token, nil + return clients.Config.AppFlag, *selected, nil } - selection, err := requestsAppSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAndUninstalledApps) + selection, err := requestAppSelectPromptFunc(ctx, clients, prompts.ShowAllEnvironments, prompts.ShowInstalledAndUninstalledApps) if err != nil { - return "", "", err + return "", types.SlackAuth{}, err } if selection.App.AppID == "" { - return "", "", slackerror.New(slackerror.ErrAppNotFound) + return "", types.SlackAuth{}, slackerror.New(slackerror.ErrAppNotFound) } - return selection.App.AppID, selection.Auth.Token, nil + return selection.App.AppID, selection.Auth, nil } -// FormatRequestsSuccess formats the install request of each team for an app -func FormatRequestsSuccess(appID string, requests []api.AppsApprovalsRequest) (secondaryText []string) { - sort.Slice(requests, func(i, j int) bool { - return requests[i].TeamID < requests[j].TeamID +// FormatRequestSuccess formats the install request of each team for an app. +// Teams found in teamNames are titled by name while others are titled by ID. +func FormatRequestSuccess(appID string, teamNames map[string]string, requests []api.AppsApprovalsRequest) (secondaryText []string) { + sorted := slices.SortedFunc(slices.Values(requests), func(a api.AppsApprovalsRequest, b api.AppsApprovalsRequest) int { + return strings.Compare(a.TeamID, b.TeamID) }) field := func(label string, value string) string { return fmt.Sprintf(style.Indent(style.Secondary("%-13s %s")), label+":", value) @@ -176,8 +188,8 @@ func FormatRequestsSuccess(appID string, requests []api.AppsApprovalsRequest) (s } // Requests are gathered apart from the app to know when none were made requestsText := []string{} - for _, request := range requests { - requestsText = append(requestsText, fmt.Sprintf(style.Bold("%s:"), request.TeamID)) + for _, request := range sorted { + requestsText = append(requestsText, fmt.Sprintf(style.Bold("%s:"), formatRequestTeam(teamNames, request.TeamID))) requestsText = append(requestsText, field("Request ID", request.ID)) requestsText = append(requestsText, field("Status", formatRequestStatus(request.Status))) requestsText = append(requestsText, field("Requested", formatRequestTime(request.DateCreated))) @@ -198,12 +210,20 @@ func FormatRequestsSuccess(appID string, requests []api.AppsApprovalsRequest) (s return } +// formatRequestTeam titles a team by name and ID when the name is known +func formatRequestTeam(teamNames map[string]string, teamID string) string { + if name, ok := teamNames[teamID]; ok { + return fmt.Sprintf("%s (%s)", name, teamID) + } + return teamID +} + // formatRequestTime displays a Unix timestamp in the local timezone func formatRequestTime(timestamp int64) string { if timestamp <= 0 { return "unknown" } - return time.Unix(timestamp, 0).Format(requestsTimeFormat) + return time.Unix(timestamp, 0).Format(requestTimeFormat) } // formatRequestCancelledBy names the kind of actor that cancelled a request. diff --git a/cmd/app/requests_test.go b/cmd/app/request_test.go similarity index 73% rename from cmd/app/requests_test.go rename to cmd/app/request_test.go index 8650b154..d57d6a33 100644 --- a/cmd/app/requests_test.go +++ b/cmd/app/request_test.go @@ -40,28 +40,28 @@ var mockRequestCreated = time.Date(2026, 8, 21, 15, 4, 5, 0, time.UTC).Unix() // mockRequestResolved is the moment a mocked request was reviewed var mockRequestResolved = time.Date(2026, 8, 22, 9, 30, 0, 0, time.UTC).Unix() -func TestRequestsCommand(t *testing.T) { - // enableRequests turns on the experiment that gates the command - enableRequests := func(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { +func TestRequestCommand(t *testing.T) { + // enableRequest turns on the experiment that gates the command + enableRequest := func(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { cm.AddDefaultMocks() cf.SDKConfig = hooks.NewSDKConfigMock() cf.Config.ExperimentsFlag = []string{string(experiment.AppApprovalStatus)} cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) - requestsAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { + requestAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { return prompts.SelectedApp{ App: types.App{AppID: "A1234", TeamID: "T1234", TeamDomain: "teamone"}, - Auth: types.SlackAuth{Token: "xoxp-example"}, + Auth: types.SlackAuth{Token: "xoxp-example", TeamID: "T1234", TeamDomain: "teamone"}, }, nil } } - restoreRequests := func() { - requestsAppSelectPromptFunc = prompts.AppSelectPrompt - requestsTeamSelectPromptFunc = prompts.PromptTeamSlackAuth + restoreRequest := func() { + requestAppSelectPromptFunc = prompts.AppSelectPrompt + requestTeamSelectPromptFunc = prompts.PromptTeamSlackAuth } - // enableRequestsWithoutProject turns on the experiment outside of a project - enableRequestsWithoutProject := func(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + // enableRequestWithoutProject turns on the experiment outside of a project + enableRequestWithoutProject := func(ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { cm.AddDefaultMocks() cf.Config.ExperimentsFlag = []string{string(experiment.AppApprovalStatus)} cf.Config.LoadExperiments(ctx, cf.IO.PrintDebug) @@ -78,7 +78,7 @@ func TestRequestsCommand(t *testing.T) { }, "reports a request that awaits review": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) + enableRequest(ctx, cm, cf) cm.API.On("ListAppApprovalRequests", mock.Anything, "xoxp-example", "A1234", []string(nil)). Return(api.AppsApprovalsRequestsListResult{ Requests: []api.AppsApprovalsRequest{ @@ -86,11 +86,11 @@ func TestRequestsCommand(t *testing.T) { }, }, nil) }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedOutputs: []string{ - "App Requests", + "App Install Approval Requests", "App ID: A1234", - "T1234", + "teamone (T1234):", "Request ID: Ar1234", "Status: pending", "Requested:", @@ -101,61 +101,67 @@ func TestRequestsCommand(t *testing.T) { }, "explains that an app was never requested": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) + enableRequest(ctx, cm, cf) cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.AppsApprovalsRequestsListResult{Requests: []api.AppsApprovalsRequest{}}, nil) }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedOutputs: []string{"You have not requested to install this app"}, }, "searches the workspaces of the provided workspace IDs": { CmdArgs: []string{"--workspace-ids", "T1234,T5678"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) + enableRequest(ctx, cm, cf) cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.AppsApprovalsRequestsListResult{}, nil) }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertCalled(t, "ListAppApprovalRequests", mock.Anything, "xoxp-example", "A1234", []string{"T1234", "T5678"}) }, }, + "returns the error of too many searched workspaces": { + CmdArgs: []string{"--workspace-ids", strings.Join(mockRequestWorkspaceIDs(51), ",")}, + Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { + enableRequest(ctx, cm, cf) + cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). + Return(api.AppsApprovalsRequestsListResult{}, slackerror.New(slackerror.ErrInvalidArguments)) + }, + Teardown: restoreRequest, + ExpectedError: slackerror.New(slackerror.ErrInvalidArguments), + }, "suggests the app flag without a project directory": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequestsWithoutProject(ctx, cm, cf) + enableRequestWithoutProject(ctx, cm, cf) }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedErrorStrings: []string{slackerror.ErrInvalidAppDirectory, "hooks.json", "--app A0123456789"}, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") }, }, - "returns the error of too many searched workspaces": { - CmdArgs: []string{"--workspace-ids", strings.Join(mockRequestTeamIDs(51), ",")}, - Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) - cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(api.AppsApprovalsRequestsListResult{}, slackerror.New(slackerror.ErrInvalidArguments)) - }, - Teardown: restoreRequests, - ExpectedError: slackerror.New(slackerror.ErrInvalidArguments), - }, "checks an app named by ID outside of a project": { CmdArgs: []string{"--app", "A5678"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequestsWithoutProject(ctx, cm, cf) - requestsTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { - return &types.SlackAuth{Token: "xoxp-selected", TeamID: "T5678"}, nil + enableRequestWithoutProject(ctx, cm, cf) + requestTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { + return &types.SlackAuth{Token: "xoxp-selected", TeamID: "T5678", TeamDomain: "teamtwo"}, nil } cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). Return(api.AppsApprovalsRequestsListResult{ Requests: []api.AppsApprovalsRequest{ {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusApproved, DateCreated: mockRequestCreated}, + {ID: "Ar9012", TeamID: "T9012", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, }, }, nil) }, - Teardown: restoreRequests, - ExpectedOutputs: []string{"Status: approved"}, + Teardown: restoreRequest, + ExpectedOutputs: []string{ + "teamtwo (T5678):", + "Status: approved", + "T9012:", + "Status: pending", + }, ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertCalled(t, "ListAppApprovalRequests", mock.Anything, "xoxp-selected", "A5678", []string(nil)) }, @@ -163,12 +169,12 @@ func TestRequestsCommand(t *testing.T) { "returns the error of a failed team selection": { CmdArgs: []string{"--app", "A5678"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequestsWithoutProject(ctx, cm, cf) - requestsTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { + enableRequestWithoutProject(ctx, cm, cf) + requestTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { return nil, slackerror.New(slackerror.ErrProcessInterrupted) } }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedError: slackerror.New(slackerror.ErrProcessInterrupted), ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") @@ -177,12 +183,12 @@ func TestRequestsCommand(t *testing.T) { "errors when the selected team has no token": { CmdArgs: []string{"--app", "A5678"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequestsWithoutProject(ctx, cm, cf) - requestsTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { + enableRequestWithoutProject(ctx, cm, cf) + requestTeamSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, promptText string, promptConfig *prompts.PromptTeamSlackAuthConfig) (*types.SlackAuth, error) { return &types.SlackAuth{TeamID: "T5678"}, nil } }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedError: slackerror.New(slackerror.ErrCredentialsNotFound), ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") @@ -191,9 +197,9 @@ func TestRequestsCommand(t *testing.T) { "errors without a project when an app environment is used": { CmdArgs: []string{"--app", "local"}, Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequestsWithoutProject(ctx, cm, cf) + enableRequestWithoutProject(ctx, cm, cf) }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedError: slackerror.New(slackerror.ErrInvalidAppDirectory), ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") @@ -201,12 +207,12 @@ func TestRequestsCommand(t *testing.T) { }, "returns the error of an interrupted app selection": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) - requestsAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { + enableRequest(ctx, cm, cf) + requestAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { return prompts.SelectedApp{}, slackerror.New(slackerror.ErrProcessInterrupted) } }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedError: slackerror.New(slackerror.ErrProcessInterrupted), ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") @@ -214,12 +220,12 @@ func TestRequestsCommand(t *testing.T) { }, "errors when the selected app is missing an ID": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) - requestsAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { + enableRequest(ctx, cm, cf) + requestAppSelectPromptFunc = func(ctx context.Context, clients *shared.ClientFactory, environment prompts.AppEnvironmentType, status prompts.AppInstallStatus, opts ...prompts.AppSelectOption) (prompts.SelectedApp, error) { return prompts.SelectedApp{Auth: types.SlackAuth{Token: "xoxp-example"}}, nil } }, - Teardown: restoreRequests, + Teardown: restoreRequest, ExpectedError: slackerror.New(slackerror.ErrAppNotFound), ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) { cm.API.AssertNotCalled(t, "ListAppApprovalRequests") @@ -227,20 +233,21 @@ func TestRequestsCommand(t *testing.T) { }, "returns the error of a failed lookup": { Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) { - enableRequests(ctx, cm, cf) + enableRequest(ctx, cm, cf) cm.API.On("ListAppApprovalRequests", mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(api.AppsApprovalsRequestsListResult{}, slackerror.New(slackerror.ErrFeatureNotEnabled)) + Return(api.AppsApprovalsRequestsListResult{}, slackerror.New(slackerror.ErrAPIFeatureNotEnabled)) }, - Teardown: restoreRequests, - ExpectedError: slackerror.New(slackerror.ErrFeatureNotEnabled), + Teardown: restoreRequest, + ExpectedError: slackerror.New(slackerror.ErrAPIFeatureNotEnabled), }, }, func(cf *shared.ClientFactory) *cobra.Command { - return NewRequestsCommand(cf) + return NewRequestCommand(cf) }) } -func TestRequestsFormat(t *testing.T) { +func TestRequestFormat(t *testing.T) { tests := map[string]struct { + TeamNames map[string]string Requests []api.AppsApprovalsRequest Expected []string Unexpected []string @@ -333,6 +340,17 @@ func TestRequestsFormat(t *testing.T) { }, Expected: []string{"Cancelled by: workflow"}, }, + "a searched team is titled by name when it is known": { + TeamNames: map[string]string{"T1234": "teamone"}, + Requests: []api.AppsApprovalsRequest{ + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + }, + Expected: []string{ + "teamone (T1234):", + "T5678:", + }, + }, "requests are sorted by the team ID": { Requests: []api.AppsApprovalsRequest{ {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusCancelled, DateCreated: mockRequestCreated}, @@ -349,7 +367,7 @@ func TestRequestsFormat(t *testing.T) { for name, tc := range tests { t.Run(name, func(t *testing.T) { - formatted := strings.Join(FormatRequestsSuccess("A1234", tc.Requests), "\n") + formatted := strings.Join(FormatRequestSuccess("A1234", tc.TeamNames, tc.Requests), "\n") previous := -1 for _, value := range tc.Expected { index := strings.Index(formatted, value) @@ -363,11 +381,26 @@ func TestRequestsFormat(t *testing.T) { } } -// mockRequestTeamIDs returns a count of unique team IDs -func mockRequestTeamIDs(count int) []string { - teamIDs := []string{} +// TestRequestFormatOrdering checks that the requests of the caller are left in +// the order they were provided while output remains sorted by team +func TestRequestFormatOrdering(t *testing.T) { + requests := []api.AppsApprovalsRequest{ + {ID: "Ar5678", TeamID: "T5678", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + {ID: "Ar1234", TeamID: "T1234", Status: api.AppsApprovalsRequestStatusPending, DateCreated: mockRequestCreated}, + } + + formatted := strings.Join(FormatRequestSuccess("A1234", nil, requests), "\n") + + assert.Less(t, strings.Index(formatted, "Ar1234"), strings.Index(formatted, "Ar5678")) + assert.Equal(t, "Ar5678", requests[0].ID, "expected the provided requests to remain unsorted") + assert.Equal(t, "Ar1234", requests[1].ID, "expected the provided requests to remain unsorted") +} + +// mockRequestWorkspaceIDs returns a count of unique workspace IDs +func mockRequestWorkspaceIDs(count int) []string { + workspaceIDs := []string{} for i := range count { - teamIDs = append(teamIDs, fmt.Sprintf("T%09d", i)) + workspaceIDs = append(workspaceIDs, fmt.Sprintf("T%09d", i)) } - return teamIDs + return workspaceIDs } diff --git a/internal/api/app_test.go b/internal/api/app_test.go index c1b4748f..5414ce36 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -224,7 +224,7 @@ func Test_Client_ListAppApprovalRequests(t *testing.T) { expectedRequest: `{"app_id":"A1234","requested_teams":["T0000"]}`, httpResponseJSON: `{"ok":false,"error":"restricted_action"}`, expectedErrors: []string{ - slackerror.ErrRestrictedAction, + slackerror.ErrAPIRestrictedAction, "The requested action is not allowed for a specified team", "Check that each team belongs to the organization", }, @@ -234,7 +234,7 @@ func Test_Client_ListAppApprovalRequests(t *testing.T) { expectedRequest: `{"app_id":"A1234"}`, httpResponseJSON: `{"ok":false,"error":"feature_not_enabled"}`, expectedErrors: []string{ - slackerror.ErrFeatureNotEnabled, + slackerror.ErrAPIFeatureNotEnabled, "This feature is not enabled for the team", "Reach out to an admin for additional information", }, diff --git a/internal/slackerror/errors.go b/internal/slackerror/errors.go index 25099bbe..98e4ab1b 100644 --- a/internal/slackerror/errors.go +++ b/internal/slackerror/errors.go @@ -27,6 +27,8 @@ const ( ErrAddAppToProject = "add_app_to_project_error" ErrAlreadyLoggedOut = "already_logged_out" ErrAlreadyResolved = "already_resolved" + ErrAPIFeatureNotEnabled = "feature_not_enabled" + ErrAPIRestrictedAction = "restricted_action" ErrAppAdd = "app_add_error" ErrAppApprovalRequestDenied = "app_approval_request_denied" ErrAppApprovalRequestEligible = "app_approval_request_eligible" @@ -114,7 +116,6 @@ const ( ErrFailedExport = "failed_export" ErrFailedToGetUser = "failed_to_get_user" ErrFailedToSaveExtensionLogs = "failed_to_save_extension_logs" - ErrFeatureNotEnabled = "feature_not_enabled" ErrFeedbackNameInvalid = "feedback_name_invalid" ErrFeedbackNameRequired = "feedback_name_required" ErrFileRejected = "file_rejected" @@ -225,7 +226,6 @@ const ( ErrPublishedAppOnly = "published_app_only" ErrRatelimited = "ratelimited" ErrRequestIDOrAppIDIsRequired = "request_id_or_app_id_is_required" - ErrRestrictedAction = "restricted_action" ErrRestrictedPlanLevel = "restricted_plan_level" ErrRuntimeNotFound = "runtime_not_found" ErrRuntimeNotSupported = "runtime_not_supported" @@ -310,6 +310,18 @@ var ErrorCodeMap = map[string]Error{ Message: "The app already has a resolution and cannot be requested", }, + ErrAPIFeatureNotEnabled: { + Code: ErrAPIFeatureNotEnabled, + Message: "This feature is not enabled for the team", + Remediation: "Reach out to an admin for additional information", + }, + + ErrAPIRestrictedAction: { + Code: ErrAPIRestrictedAction, + Message: "The requested action is not allowed for a specified team", + Remediation: "Check that each team belongs to the organization of the authenticated account", + }, + ErrAppAdd: { Code: ErrAppAdd, Message: "Couldn't create a new app", @@ -789,12 +801,6 @@ Otherwise start your app for local development with: %s`, Message: "Couldn't save the logs", }, - ErrFeatureNotEnabled: { - Code: ErrFeatureNotEnabled, - Message: "This feature is not enabled for the team", - Remediation: "Reach out to an admin for additional information", - }, - ErrFeedbackNameInvalid: { Code: ErrFeedbackNameInvalid, Message: "The name of the feedback is invalid", @@ -1390,12 +1396,6 @@ Otherwise start your app for local development with: %s`, Message: "Must include a request_id or app_id", }, - ErrRestrictedAction: { - Code: ErrRestrictedAction, - Message: "The requested action is not allowed for a specified team", - Remediation: "Check that each team belongs to the organization of the authenticated account", - }, - ErrRestrictedPlanLevel: { Code: ErrRestrictedPlanLevel, Message: "Your Slack plan does not have access to the requested feature",