diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a54a783..0ba9100 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,7 @@ jobs: uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: '1.26' + check-latest: true - name: Download dependencies run: go mod download @@ -106,6 +107,7 @@ jobs: uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: '1.26' + check-latest: true - name: Run golangci-lint uses: golangci/golangci-lint-action@9fae48acfc02a90574d7c304a1758ef9895495fa # v7 diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a4127d7..7bd7ef4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -64,6 +64,7 @@ jobs: uses: actions/setup-go@40f1582b2485089dde7abd97c1529aa768e1baff # v5 with: go-version: '1.26' + check-latest: true - name: Run tests run: go test ./... -short diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index c477a38..20a9c34 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -908,12 +908,11 @@ nylas scheduler pages create # Create booking page nylas scheduler pages update # Update page nylas scheduler pages delete # Delete page -# Bookings -nylas scheduler bookings list # List bookings -nylas scheduler bookings show # Show booking details -nylas scheduler bookings confirm # Confirm booking -nylas scheduler bookings reschedule # Reschedule booking -nylas scheduler bookings cancel # Cancel booking +# Bookings (--configuration-id required: booking endpoints use a Scheduler session token, not the API key) +nylas scheduler bookings show --configuration-id # Show booking details +nylas scheduler bookings confirm --configuration-id --salt # Confirm booking +nylas scheduler bookings reschedule --configuration-id # Reschedule booking +nylas scheduler bookings cancel --configuration-id # Cancel booking # Configurations nylas scheduler configurations list # List configurations @@ -966,11 +965,11 @@ nylas admin connectors update # Update connector nylas admin connectors delete # Delete connector # Credentials -nylas admin credentials list # List credentials -nylas admin credentials show # Show credential -nylas admin credentials create # Create credential -nylas admin credentials update # Update credential -nylas admin credentials delete # Delete credential +nylas admin credentials list [connector] # List credentials (provider auto-detected) +nylas admin credentials show --connector # Show credential +nylas admin credentials create --connector # Create credential +nylas admin credentials update --connector # Update credential +nylas admin credentials delete --connector # Delete credential # Grants nylas admin grants list # List all grants diff --git a/docs/RPC.md b/docs/RPC.md index 419c613..2a1a8c6 100644 --- a/docs/RPC.md +++ b/docs/RPC.md @@ -172,7 +172,8 @@ Standard JSON-RPC 2.0 codes: Conventions: - `grant_id` is **optional** on per-grant methods — it falls back to the server's default grant. - App-level methods (admin, scheduler configs, etc.) take **no** grant. + App-level methods (admin, etc.) take **no** grant. Scheduler configs are **per-grant** (the + Nylas v3 configuration endpoints are grant-scoped). - Required ids return `-32602` when missing. - Create/update params **embed the corresponding `domain.*Request` struct** at the top level — i.e. the request fields sit alongside `grant_id`/ids (see `internal/domain` for exact fields). @@ -277,9 +278,15 @@ Conventions: (all per-grant; `notetaker_id` required where applicable). ### Scheduler -- Configs: `scheduler.config.list` / `.get` / `.create` / `.update` / `.delete` -- Sessions: `scheduler.session.create` / `.get` -- Bookings: `scheduler.booking.get` / `.confirm` / `.reschedule` / `.cancel` (`{ cancelled }`) +- Configs (per-grant): `scheduler.config.list` / `.get` / `.create` / `.update` / `.delete` +- Sessions: `scheduler.session.create` (requires `configuration_id` or `slug`; `time_to_live` in + minutes, max 30, legacy alias `ttl`) / `.get` +- Bookings (all require `configuration_id` — booking endpoints authenticate with a Scheduler + session token minted from the configuration): + `scheduler.booking.get` / `.confirm` (`salt` + `status` required; `cancellation_reason`, legacy + alias `reason`, applies when declining) / `.reschedule` (returns the booking; adds `warning` + when the reschedule was applied but the record could not be read back) / `.cancel` + (`{ cancelled }`; `cancellation_reason`, legacy alias `reason`) - Group events (per-grant): `scheduler.groupEvent.list` (requires `config_id`, `calendar_id`, `start_time`, `end_time`) / `.create` / `.update` / `.delete` / `.import` diff --git a/docs/commands/admin.md b/docs/commands/admin.md index a337d81..f060ff3 100644 --- a/docs/commands/admin.md +++ b/docs/commands/admin.md @@ -186,58 +186,60 @@ Settings: Manage authentication credentials for connectors. +The connector provider (e.g. `google`) is auto-detected when the application +has exactly one connector. Pass it explicitly — positionally for `list`, or +with `--connector` for show/create/update/delete — when there are several. + ```bash -# List credentials +# List credentials (connector auto-detected, or pass the provider) nylas admin credentials list -nylas admin creds list # Alias -nylas admin credentials list --json +nylas admin creds list google # Alias, explicit provider +nylas admin credentials list google --json # Show credential details nylas admin credentials show -nylas admin cred show --json +nylas admin cred show --connector google --json -# Create credential -nylas admin credentials create --connector-id \ +# Create a connector credential. --client-id/--client-secret are the PROVIDER's +# OAuth app credentials (e.g. your own Google Cloud / Azure app), NOT your Nylas +# app's — Nylas uses them to broker auth through that provider application. +nylas admin credentials create --connector google \ --name "Production Credentials" \ - --credential-type oauth - -# Create credential with data -nylas admin cred create --connector-id \ - --name "Service Account" \ - --credential-type service_account \ - --credential-data '{"private_key":"..."}' + --type connector \ + --client-id "" \ + --client-secret "" # Update credential -nylas admin credentials update --name "Updated Name" +nylas admin credentials update --connector google --name "Updated Name" # Delete credential -nylas admin credentials delete +nylas admin credentials delete --connector google nylas admin cred delete --yes ``` **Example: List credentials** ```bash -$ nylas admin credentials list +$ nylas admin credentials list google Found 2 credential(s): -NAME ID CONNECTOR TYPE -Production OAuth cred_oauth_123 conn_google_123 oauth -Service Account cred_sa_456 conn_google_123 service_account +NAME ID CREATED AT +Production Creds cred_conn_123 Dec 1, 2024 10:00 AM +Service Account cred_sa_456 Dec 3, 2024 9:15 AM ``` +> The Nylas v3 credential object returns only `id`, `name`, `created_at`, and +> `updated_at`; `credential_type`/`credential_data` are create-time inputs and are +> never echoed back. + **Example: Show credential details** ```bash -$ nylas admin credentials show cred_oauth_123 - -Credential: Production OAuth - ID: cred_oauth_123 - Connector ID: conn_google_123 - Name: Production OAuth - Type: oauth +$ nylas admin credentials show cred_conn_123 --connector google -Created: Dec 1, 2024 10:00 AM -Updated: Dec 15, 2024 2:30 PM +Credential: Production Creds + ID: cred_conn_123 + Created At: Dec 1, 2024 10:00 AM + Updated At: Dec 15, 2024 2:30 PM ``` ### Grants diff --git a/docs/commands/scheduler.md b/docs/commands/scheduler.md index a7ad9bd..daa912f 100644 --- a/docs/commands/scheduler.md +++ b/docs/commands/scheduler.md @@ -14,11 +14,17 @@ Nylas Scheduler enables you to create customizable booking workflows for schedul Manage scheduling configurations (meeting types): +> Configurations are grant-scoped (`/v3/grants/{grant_id}/scheduling/configurations`). +> The grant is taken from your default grant, or pass it as an optional trailing +> `[grant-id]` positional. Leading positionals like `` are not mistaken +> for a grant. + ```bash -# List all scheduler configurations +# List all scheduler configurations (uses default grant) nylas scheduler configurations list nylas scheduler configs list # Alias nylas scheduler configurations list --json +nylas scheduler configurations list # Explicit grant # Show configuration details nylas scheduler configurations show @@ -119,8 +125,9 @@ When both `--file` and flags are provided, flags take precedence over file value Create temporary booking sessions for configurations: ```bash -# Create a session for a configuration -nylas scheduler sessions create +# Create a session for a configuration (TTL is in minutes, max 30) +nylas scheduler sessions create --config-id +nylas scheduler sessions create --config-id --ttl 10 # Show session details nylas scheduler sessions show @@ -136,23 +143,31 @@ nylas scheduler sessions show Manage scheduled appointments: ```bash -# List all bookings -nylas scheduler bookings list -nylas scheduler bookings list --json +# Booking commands are authorized by a Scheduler session token that the CLI +# mints from the booking's configuration, so --configuration-id is required on +# every booking command (the API key is not accepted on booking endpoints). # Show booking details -nylas scheduler bookings show +nylas scheduler bookings show --configuration-id -# Confirm a booking -nylas scheduler bookings confirm +# Confirm a booking. --salt is required and comes from the booking reference +# (in the organizer confirmation link, the cancel/reschedule URL, or a Scheduler +# webhook). It cannot be looked up from the booking ID. +nylas scheduler bookings confirm --configuration-id --salt # Reschedule a booking nylas scheduler bookings reschedule \\ - --start-time "2024-03-20T10:00:00Z" + --configuration-id \\ + --start-time 1710930600 --end-time 1710934200 +# If the reschedule is applied but the booking cannot be read back afterwards, +# the command still succeeds and prints a warning on stderr (in --json mode the +# output additionally carries a "warning" field); re-run `bookings show` to +# verify the booking's current server-side record. # Cancel a booking -nylas scheduler bookings cancel +nylas scheduler bookings cancel --configuration-id nylas scheduler bookings cancel \\ + --configuration-id \\ --reason "Meeting no longer needed" ``` @@ -210,12 +225,12 @@ nylas scheduler pages create \\ # 3. Share the booking URL with prospects # URL format: https://schedule.nylas.com/product-demo -# 4. View bookings -nylas scheduler bookings list +# 4. Show a booking (booking IDs arrive via Scheduler webhooks or confirmation links) +nylas scheduler bookings show --configuration-id -# 5. Manage bookings -nylas scheduler bookings confirm -nylas scheduler bookings reschedule --start-time "..." +# 5. Manage bookings (--configuration-id is required on booking commands) +nylas scheduler bookings confirm --configuration-id --salt +nylas scheduler bookings reschedule --configuration-id --start-time --end-time ``` **Note:** Some scheduler features may not be available in all Nylas API versions or require specific subscription tiers. diff --git a/go.mod b/go.mod index b313b6d..7c390f5 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/nylas/cli go 1.26.0 -toolchain go1.26.3 +toolchain go1.26.5 require ( charm.land/huh/v2 v2.0.3 diff --git a/internal/adapters/nylas/admin.go b/internal/adapters/nylas/admin.go index bc5de13..822676d 100644 --- a/internal/adapters/nylas/admin.go +++ b/internal/adapters/nylas/admin.go @@ -313,24 +313,54 @@ func (c *HTTPClient) ListCredentials(ctx context.Context, connectorID string) ([ return nil, err } - queryURL := fmt.Sprintf("%s/v3/connectors/%s/credentials", c.baseURL, url.PathEscape(connectorID)) + // The v3 creds list is offset-paginated (default page size 10), so follow + // the pages until a short page rather than silently truncating. + baseURL := fmt.Sprintf("%s/v3/connectors/%s/creds", c.baseURL, url.PathEscape(connectorID)) + const pageSize = 200 + // Safety ceiling (mirrors ListAllGrants' maxGrantPages) so a misbehaving + // endpoint that never returns a short page can't loop forever. + const maxCredentialPages = 1000 + + // Non-nil so an empty result marshals to `[]`, not `null`. + all := make([]domain.ConnectorCredential, 0) + offset := 0 + for range maxCredentialPages { + queryURL := NewQueryBuilder().AddInt("limit", pageSize).AddInt("offset", offset).BuildURL(baseURL) - var result struct { - Data []domain.ConnectorCredential `json:"data"` - } - if err := c.doGet(ctx, queryURL, &result); err != nil { - return nil, err + var result struct { + Data []domain.ConnectorCredential `json:"data"` + Limit int `json:"limit"` + } + if err := c.doGet(ctx, queryURL, &result); err != nil { + return nil, err + } + all = append(all, result.Data...) + + // The API echoes the effective page size; a page shorter than that (or + // empty) is the last one. Falling back to the requested size handles a + // response that omits `limit`. + effective := result.Limit + if effective <= 0 { + effective = pageSize + } + if len(result.Data) < effective { + return all, nil + } + offset += len(result.Data) } - return result.Data, nil + return nil, fmt.Errorf("failed to paginate credentials: exceeded max page count (%d)", maxCredentialPages) } -// GetCredential retrieves a specific credential. -func (c *HTTPClient) GetCredential(ctx context.Context, credentialID string) (*domain.ConnectorCredential, error) { +// GetCredential retrieves a specific credential for a connector. +func (c *HTTPClient) GetCredential(ctx context.Context, connectorID, credentialID string) (*domain.ConnectorCredential, error) { + if err := validateRequired("connector ID", connectorID); err != nil { + return nil, err + } if err := validateRequired("credential ID", credentialID); err != nil { return nil, err } - queryURL := fmt.Sprintf("%s/v3/credentials/%s", c.baseURL, url.PathEscape(credentialID)) + queryURL := fmt.Sprintf("%s/v3/connectors/%s/creds/%s", c.baseURL, url.PathEscape(connectorID), url.PathEscape(credentialID)) var result struct { Data domain.ConnectorCredential `json:"data"` @@ -347,7 +377,7 @@ func (c *HTTPClient) CreateCredential(ctx context.Context, connectorID string, r return nil, err } - queryURL := fmt.Sprintf("%s/v3/connectors/%s/credentials", c.baseURL, url.PathEscape(connectorID)) + queryURL := fmt.Sprintf("%s/v3/connectors/%s/creds", c.baseURL, url.PathEscape(connectorID)) resp, err := c.doJSONRequest(ctx, "POST", queryURL, req) if err != nil { @@ -363,13 +393,16 @@ func (c *HTTPClient) CreateCredential(ctx context.Context, connectorID string, r return &result.Data, nil } -// UpdateCredential updates an existing credential. -func (c *HTTPClient) UpdateCredential(ctx context.Context, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) { +// UpdateCredential updates an existing credential for a connector. +func (c *HTTPClient) UpdateCredential(ctx context.Context, connectorID, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) { + if err := validateRequired("connector ID", connectorID); err != nil { + return nil, err + } if err := validateRequired("credential ID", credentialID); err != nil { return nil, err } - queryURL := fmt.Sprintf("%s/v3/credentials/%s", c.baseURL, url.PathEscape(credentialID)) + queryURL := fmt.Sprintf("%s/v3/connectors/%s/creds/%s", c.baseURL, url.PathEscape(connectorID), url.PathEscape(credentialID)) resp, err := c.doJSONRequest(ctx, "PATCH", queryURL, req) if err != nil { @@ -385,12 +418,15 @@ func (c *HTTPClient) UpdateCredential(ctx context.Context, credentialID string, return &result.Data, nil } -// DeleteCredential deletes a credential. -func (c *HTTPClient) DeleteCredential(ctx context.Context, credentialID string) error { +// DeleteCredential deletes a credential for a connector. +func (c *HTTPClient) DeleteCredential(ctx context.Context, connectorID, credentialID string) error { + if err := validateRequired("connector ID", connectorID); err != nil { + return err + } if err := validateRequired("credential ID", credentialID); err != nil { return err } - queryURL := fmt.Sprintf("%s/v3/credentials/%s", c.baseURL, url.PathEscape(credentialID)) + queryURL := fmt.Sprintf("%s/v3/connectors/%s/creds/%s", c.baseURL, url.PathEscape(connectorID), url.PathEscape(credentialID)) return c.doDelete(ctx, queryURL) } diff --git a/internal/adapters/nylas/admin_connectors_credentials_test.go b/internal/adapters/nylas/admin_connectors_credentials_test.go index 3be05af..7ee7a32 100644 --- a/internal/adapters/nylas/admin_connectors_credentials_test.go +++ b/internal/adapters/nylas/admin_connectors_credentials_test.go @@ -218,21 +218,13 @@ func TestHTTPClient_DeleteConnector_EmptyID(t *testing.T) { func TestHTTPClient_ListCredentials(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/connectors/conn-123/credentials", r.URL.Path) + assert.Equal(t, "/v3/connectors/conn-123/creds", r.URL.Path) assert.Equal(t, "GET", r.Method) response := map[string]any{ "data": []map[string]any{ - { - "id": "cred-1", - "name": "OAuth Credential", - "credential_type": "oauth", - }, - { - "id": "cred-2", - "name": "Service Account", - "credential_type": "service_account", - }, + {"id": "cred-1", "name": "First Credential"}, + {"id": "cred-2", "name": "Second Credential"}, }, } w.Header().Set("Content-Type", "application/json") @@ -250,7 +242,61 @@ func TestHTTPClient_ListCredentials(t *testing.T) { require.NoError(t, err) assert.Len(t, credentials, 2) assert.Equal(t, "cred-1", credentials[0].ID) - assert.Equal(t, "oauth", credentials[0].CredentialType) + assert.Equal(t, "First Credential", credentials[0].Name) +} + +func TestHTTPClient_ListCredentials_Paginates(t *testing.T) { + // A full first page (len == limit) must trigger a follow-up page; a short + // second page ends it. Guards against silently truncating at the default 10. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/v3/connectors/conn-123/creds", r.URL.Path) + limit := r.URL.Query().Get("limit") + require.NotEmpty(t, limit, "list must send a limit") + w.Header().Set("Content-Type", "application/json") + // Page 1 omits offset (defaults to 0); page 2 sends offset=200. + if off := r.URL.Query().Get("offset"); off == "" || off == "0" { + full := make([]map[string]any, 200) + for i := range full { + full[i] = map[string]any{"id": "cred", "name": "c"} + } + _ = json.NewEncoder(w).Encode(map[string]any{"data": full, "limit": 200}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": []map[string]any{{"id": "cred-last", "name": "last"}}, + "limit": 200, + }) + })) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + credentials, err := client.ListCredentials(context.Background(), "conn-123") + + require.NoError(t, err) + assert.Len(t, credentials, 201, "all pages must be aggregated") + assert.Equal(t, "cred-last", credentials[200].ID) +} + +func TestHTTPClient_ListCredentials_EmptyReturnsNonNilSlice(t *testing.T) { + // An empty result must be a non-nil slice so JSON output is `[]`, not `null`. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"data": []any{}}) + })) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + creds, err := client.ListCredentials(context.Background(), "conn-123") + + require.NoError(t, err) + assert.NotNil(t, creds, "empty result must be a non-nil slice (marshals to [] not null)") + assert.Empty(t, creds) } func TestHTTPClient_ListCredentials_EmptyConnectorID(t *testing.T) { @@ -267,14 +313,13 @@ func TestHTTPClient_ListCredentials_EmptyConnectorID(t *testing.T) { func TestHTTPClient_GetCredential(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/credentials/cred-456", r.URL.Path) + assert.Equal(t, "/v3/connectors/conn-123/creds/cred-456", r.URL.Path) assert.Equal(t, "GET", r.Method) response := map[string]any{ "data": map[string]any{ - "id": "cred-456", - "name": "Test Credential", - "credential_type": "oauth", + "id": "cred-456", + "name": "Test Credential", }, } w.Header().Set("Content-Type", "application/json") @@ -287,28 +332,28 @@ func TestHTTPClient_GetCredential(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - credential, err := client.GetCredential(ctx, "cred-456") + credential, err := client.GetCredential(ctx, "conn-123", "cred-456") require.NoError(t, err) assert.Equal(t, "cred-456", credential.ID) - assert.Equal(t, "oauth", credential.CredentialType) + assert.Equal(t, "Test Credential", credential.Name) } func TestHTTPClient_CreateCredential(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/connectors/conn-123/credentials", r.URL.Path) + assert.Equal(t, "/v3/connectors/conn-123/creds", r.URL.Path) assert.Equal(t, "POST", r.Method) var body map[string]any _ = json.NewDecoder(r.Body).Decode(&body) assert.Equal(t, "New Credential", body["name"]) - assert.Equal(t, "oauth", body["credential_type"]) + // credential_type is a request-only field (not echoed in the response). + assert.Equal(t, "connector", body["credential_type"]) response := map[string]any{ "data": map[string]any{ - "id": "cred-new", - "name": "New Credential", - "credential_type": "oauth", + "id": "cred-new", + "name": "New Credential", }, } w.Header().Set("Content-Type", "application/json") @@ -323,18 +368,18 @@ func TestHTTPClient_CreateCredential(t *testing.T) { ctx := context.Background() req := &domain.CreateCredentialRequest{ Name: "New Credential", - CredentialType: "oauth", + CredentialType: "connector", } credential, err := client.CreateCredential(ctx, "conn-123", req) require.NoError(t, err) assert.Equal(t, "cred-new", credential.ID) - assert.Equal(t, "oauth", credential.CredentialType) + assert.Equal(t, "New Credential", credential.Name) } func TestHTTPClient_UpdateCredential(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/credentials/cred-789", r.URL.Path) + assert.Equal(t, "/v3/connectors/conn-123/creds/cred-789", r.URL.Path) assert.Equal(t, "PATCH", r.Method) var body map[string]any @@ -361,7 +406,7 @@ func TestHTTPClient_UpdateCredential(t *testing.T) { req := &domain.UpdateCredentialRequest{ Name: &name, } - credential, err := client.UpdateCredential(ctx, "cred-789", req) + credential, err := client.UpdateCredential(ctx, "conn-123", "cred-789", req) require.NoError(t, err) assert.Equal(t, "cred-789", credential.ID) @@ -369,7 +414,7 @@ func TestHTTPClient_UpdateCredential(t *testing.T) { func TestHTTPClient_DeleteCredential(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/credentials/cred-delete", r.URL.Path) + assert.Equal(t, "/v3/connectors/conn-123/creds/cred-delete", r.URL.Path) assert.Equal(t, "DELETE", r.Method) w.WriteHeader(http.StatusOK) @@ -381,7 +426,7 @@ func TestHTTPClient_DeleteCredential(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - err := client.DeleteCredential(ctx, "cred-delete") + err := client.DeleteCredential(ctx, "conn-123", "cred-delete") require.NoError(t, err) } diff --git a/internal/adapters/nylas/admin_grants_test.go b/internal/adapters/nylas/admin_grants_test.go index e0bcdcb..06d4c6e 100644 --- a/internal/adapters/nylas/admin_grants_test.go +++ b/internal/adapters/nylas/admin_grants_test.go @@ -294,7 +294,7 @@ func TestMockClient_AdminOperations(t *testing.T) { require.NoError(t, err) assert.NotEmpty(t, credentials) - credential, err := mock.GetCredential(ctx, "cred-456") + credential, err := mock.GetCredential(ctx, "conn-123", "cred-456") require.NoError(t, err) assert.Equal(t, "cred-456", credential.ID) @@ -305,11 +305,11 @@ func TestMockClient_AdminOperations(t *testing.T) { credName := "Updated Cred" updateCredReq := &domain.UpdateCredentialRequest{Name: &credName} - updatedCred, err := mock.UpdateCredential(ctx, "cred-789", updateCredReq) + updatedCred, err := mock.UpdateCredential(ctx, "conn-123", "cred-789", updateCredReq) require.NoError(t, err) assert.Equal(t, "Updated Cred", updatedCred.Name) - err = mock.DeleteCredential(ctx, "cred-delete") + err = mock.DeleteCredential(ctx, "conn-123", "cred-delete") require.NoError(t, err) // Grant tests diff --git a/internal/adapters/nylas/client.go b/internal/adapters/nylas/client.go index e3b0cc0..dba0e1d 100644 --- a/internal/adapters/nylas/client.go +++ b/internal/adapters/nylas/client.go @@ -128,6 +128,17 @@ func (c *HTTPClient) setAuthHeader(req *http.Request) { } } +// setAuth sets the authorization header, preferring an explicit bearer token +// (e.g. a Scheduler session token) over the application API key. Scheduler +// booking endpoints are authorized by a session token, not the API key. +func (c *HTTPClient) setAuth(req *http.Request, token string) { + if token != "" { + req.Header.Set("Authorization", "Bearer "+token) + return + } + c.setAuthHeader(req) +} + // parseError parses an error response from the API. // Uses streaming decoder with size limit to avoid large allocations. func (c *HTTPClient) parseError(resp *http.Response) error { @@ -435,15 +446,19 @@ func (c *HTTPClient) doJSONRequestInternal( withAuth bool, acceptedStatuses ...int, ) (*http.Response, error) { - return c.doJSONRequestInternalWithRetry(ctx, method, url, body, withAuth, true, acceptedStatuses...) + return c.doJSONRequestInternalWithRetry(ctx, method, url, body, withAuth, true, "", acceptedStatuses...) } +// doJSONRequestInternalWithRetry performs a JSON request. When withAuth is true, +// authToken (if non-empty) is used as the bearer token in place of the API key; +// otherwise the API key is used. func (c *HTTPClient) doJSONRequestInternalWithRetry( ctx context.Context, method, url string, body any, withAuth bool, retry bool, + authToken string, acceptedStatuses ...int, ) (*http.Response, error) { // Default accepted statuses @@ -472,7 +487,7 @@ func (c *HTTPClient) doJSONRequestInternalWithRetry( req.Header.Set("Content-Type", "application/json") } if withAuth { - c.setAuthHeader(req) + c.setAuth(req, authToken) } // Execute request with the configured retry policy. @@ -531,7 +546,18 @@ func (c *HTTPClient) doJSONRequestNoRetry( body any, acceptedStatuses ...int, ) (*http.Response, error) { - return c.doJSONRequestInternalWithRetry(ctx, method, url, body, true, false, acceptedStatuses...) + return c.doJSONRequestInternalWithRetry(ctx, method, url, body, true, false, "", acceptedStatuses...) +} + +// doJSONRequestWithToken performs a JSON request authenticated with an explicit +// bearer token (a Scheduler session token) instead of the API key. +func (c *HTTPClient) doJSONRequestWithToken( + ctx context.Context, + method, url, token string, + body any, + acceptedStatuses ...int, +) (*http.Response, error) { + return c.doJSONRequestInternalWithRetry(ctx, method, url, body, true, true, token, acceptedStatuses...) } // decodeJSONResponse decodes a JSON response body into the provided struct. @@ -563,7 +589,7 @@ func (c *HTTPClient) doJSONRequestNoAuth( body any, acceptedStatuses ...int, ) (*http.Response, error) { - return c.doJSONRequestInternalWithRetry(ctx, method, url, body, false, true, acceptedStatuses...) + return c.doJSONRequestInternalWithRetry(ctx, method, url, body, false, true, "", acceptedStatuses...) } // validateRequired validates that a required field is not empty. diff --git a/internal/adapters/nylas/client_helpers.go b/internal/adapters/nylas/client_helpers.go index c762add..c85090d 100644 --- a/internal/adapters/nylas/client_helpers.go +++ b/internal/adapters/nylas/client_helpers.go @@ -94,11 +94,17 @@ func (c *HTTPClient) doGet(ctx context.Context, url string, result any) error { // return nil, err // } func (c *HTTPClient) doGetWithNotFound(ctx context.Context, url string, result any, notFoundErr error) error { + return c.doGetWithNotFoundAuth(ctx, url, "", result, notFoundErr) +} + +// doGetWithNotFoundAuth is doGetWithNotFound with an explicit bearer token +// (e.g. a Scheduler session token); an empty token falls back to the API key. +func (c *HTTPClient) doGetWithNotFoundAuth(ctx context.Context, url, token string, result any, notFoundErr error) error { req, err := http.NewRequestWithContext(ctx, "GET", url, nil) if err != nil { return err } - c.setAuthHeader(req) + c.setAuth(req, token) resp, err := c.doRequest(ctx, req) if err != nil { diff --git a/internal/adapters/nylas/demo_admin.go b/internal/adapters/nylas/demo_admin.go index 856d59b..80d416c 100644 --- a/internal/adapters/nylas/demo_admin.go +++ b/internal/adapters/nylas/demo_admin.go @@ -10,14 +10,14 @@ import ( // Scheduler Demo Implementations -func (d *DemoClient) ListSchedulerConfigurations(ctx context.Context) ([]domain.SchedulerConfiguration, error) { +func (d *DemoClient) ListSchedulerConfigurations(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) { return []domain.SchedulerConfiguration{ {ID: "config-demo-1", Name: "30 Minute Meeting", Slug: "30min-demo"}, {ID: "config-demo-2", Name: "1 Hour Meeting", Slug: "1hour-demo"}, }, nil } -func (d *DemoClient) GetSchedulerConfiguration(ctx context.Context, configID string) (*domain.SchedulerConfiguration, error) { +func (d *DemoClient) GetSchedulerConfiguration(ctx context.Context, grantID, configID string) (*domain.SchedulerConfiguration, error) { return &domain.SchedulerConfiguration{ ID: configID, Name: "30 Minute Meeting", @@ -25,7 +25,7 @@ func (d *DemoClient) GetSchedulerConfiguration(ctx context.Context, configID str }, nil } -func (d *DemoClient) CreateSchedulerConfiguration(ctx context.Context, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { +func (d *DemoClient) CreateSchedulerConfiguration(ctx context.Context, grantID string, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { return &domain.SchedulerConfiguration{ ID: "config-demo-new", Name: req.Name, @@ -33,7 +33,7 @@ func (d *DemoClient) CreateSchedulerConfiguration(ctx context.Context, req *doma }, nil } -func (d *DemoClient) UpdateSchedulerConfiguration(ctx context.Context, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { +func (d *DemoClient) UpdateSchedulerConfiguration(ctx context.Context, grantID, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { name := "Updated Configuration" if req.Name != nil { name = *req.Name @@ -44,7 +44,7 @@ func (d *DemoClient) UpdateSchedulerConfiguration(ctx context.Context, configID }, nil } -func (d *DemoClient) DeleteSchedulerConfiguration(ctx context.Context, configID string) error { +func (d *DemoClient) DeleteSchedulerConfiguration(ctx context.Context, grantID, configID string) error { return nil } @@ -64,7 +64,7 @@ func (d *DemoClient) GetSchedulerSession(ctx context.Context, sessionID string) }, nil } -func (d *DemoClient) GetBooking(ctx context.Context, bookingID string) (*domain.Booking, error) { +func (d *DemoClient) GetBooking(ctx context.Context, configurationID, bookingID string) (*domain.Booking, error) { return &domain.Booking{ BookingID: bookingID, Title: "Demo Meeting", @@ -72,14 +72,14 @@ func (d *DemoClient) GetBooking(ctx context.Context, bookingID string) (*domain. }, nil } -func (d *DemoClient) ConfirmBooking(ctx context.Context, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { +func (d *DemoClient) ConfirmBooking(ctx context.Context, configurationID, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { return &domain.Booking{ BookingID: bookingID, Status: req.Status, }, nil } -func (d *DemoClient) RescheduleBooking(ctx context.Context, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { +func (d *DemoClient) RescheduleBooking(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { return &domain.Booking{ BookingID: bookingID, Status: "confirmed", @@ -88,7 +88,7 @@ func (d *DemoClient) RescheduleBooking(ctx context.Context, bookingID string, re }, nil } -func (d *DemoClient) CancelBooking(ctx context.Context, bookingID string, reason string) error { +func (d *DemoClient) CancelBooking(ctx context.Context, configurationID, bookingID string, reason string) error { return nil } @@ -293,38 +293,35 @@ func (d *DemoClient) DeleteWorkspace(ctx context.Context, workspaceID string) er func (d *DemoClient) ListCredentials(ctx context.Context, connectorID string) ([]domain.ConnectorCredential, error) { return []domain.ConnectorCredential{ - {ID: "cred-demo-1", Name: "OAuth Demo Credential", CredentialType: "oauth"}, + {ID: "cred-demo-1", Name: "Connector Demo Credential"}, }, nil } -func (d *DemoClient) GetCredential(ctx context.Context, credentialID string) (*domain.ConnectorCredential, error) { +func (d *DemoClient) GetCredential(ctx context.Context, connectorID, credentialID string) (*domain.ConnectorCredential, error) { return &domain.ConnectorCredential{ - ID: credentialID, - Name: "OAuth Demo Credential", - CredentialType: "oauth", + ID: credentialID, + Name: "Connector Demo Credential", }, nil } func (d *DemoClient) CreateCredential(ctx context.Context, connectorID string, req *domain.CreateCredentialRequest) (*domain.ConnectorCredential, error) { return &domain.ConnectorCredential{ - ID: "cred-demo-new", - Name: req.Name, - CredentialType: req.CredentialType, + ID: "cred-demo-new", + Name: req.Name, }, nil } -func (d *DemoClient) UpdateCredential(ctx context.Context, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) { +func (d *DemoClient) UpdateCredential(ctx context.Context, connectorID, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) { name := "Updated Credential" if req.Name != nil { name = *req.Name } return &domain.ConnectorCredential{ - ID: credentialID, - Name: name, - CredentialType: "oauth", + ID: credentialID, + Name: name, }, nil } -func (d *DemoClient) DeleteCredential(ctx context.Context, credentialID string) error { +func (d *DemoClient) DeleteCredential(ctx context.Context, connectorID, credentialID string) error { return nil } diff --git a/internal/adapters/nylas/mock_admin.go b/internal/adapters/nylas/mock_admin.go index 2ca94c5..a22c031 100644 --- a/internal/adapters/nylas/mock_admin.go +++ b/internal/adapters/nylas/mock_admin.go @@ -174,38 +174,35 @@ func (m *MockClient) DeleteWorkspace(ctx context.Context, workspaceID string) er func (m *MockClient) ListCredentials(ctx context.Context, connectorID string) ([]domain.ConnectorCredential, error) { return []domain.ConnectorCredential{ - {ID: "cred-1", Name: "OAuth Credential", CredentialType: "oauth"}, + {ID: "cred-1", Name: "Connector Credential"}, }, nil } -func (m *MockClient) GetCredential(ctx context.Context, credentialID string) (*domain.ConnectorCredential, error) { +func (m *MockClient) GetCredential(ctx context.Context, connectorID, credentialID string) (*domain.ConnectorCredential, error) { return &domain.ConnectorCredential{ - ID: credentialID, - Name: "OAuth Credential", - CredentialType: "oauth", + ID: credentialID, + Name: "Connector Credential", }, nil } func (m *MockClient) CreateCredential(ctx context.Context, connectorID string, req *domain.CreateCredentialRequest) (*domain.ConnectorCredential, error) { return &domain.ConnectorCredential{ - ID: "new-cred", - Name: req.Name, - CredentialType: req.CredentialType, + ID: "new-cred", + Name: req.Name, }, nil } -func (m *MockClient) UpdateCredential(ctx context.Context, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) { +func (m *MockClient) UpdateCredential(ctx context.Context, connectorID, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) { name := "Updated Credential" if req.Name != nil { name = *req.Name } return &domain.ConnectorCredential{ - ID: credentialID, - Name: name, - CredentialType: "oauth", + ID: credentialID, + Name: name, }, nil } -func (m *MockClient) DeleteCredential(ctx context.Context, credentialID string) error { +func (m *MockClient) DeleteCredential(ctx context.Context, connectorID, credentialID string) error { return nil } diff --git a/internal/adapters/nylas/mock_scheduler.go b/internal/adapters/nylas/mock_scheduler.go index 51b205c..4997c83 100644 --- a/internal/adapters/nylas/mock_scheduler.go +++ b/internal/adapters/nylas/mock_scheduler.go @@ -8,14 +8,14 @@ import ( "github.com/nylas/cli/internal/domain" ) -func (m *MockClient) ListSchedulerConfigurations(ctx context.Context) ([]domain.SchedulerConfiguration, error) { +func (m *MockClient) ListSchedulerConfigurations(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) { return []domain.SchedulerConfiguration{ {ID: "config-1", Name: "30 Minute Meeting", Slug: "30min"}, {ID: "config-2", Name: "1 Hour Meeting", Slug: "1hour"}, }, nil } -func (m *MockClient) GetSchedulerConfiguration(ctx context.Context, configID string) (*domain.SchedulerConfiguration, error) { +func (m *MockClient) GetSchedulerConfiguration(ctx context.Context, grantID, configID string) (*domain.SchedulerConfiguration, error) { return &domain.SchedulerConfiguration{ ID: configID, Name: "30 Minute Meeting", @@ -23,7 +23,7 @@ func (m *MockClient) GetSchedulerConfiguration(ctx context.Context, configID str }, nil } -func (m *MockClient) CreateSchedulerConfiguration(ctx context.Context, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { +func (m *MockClient) CreateSchedulerConfiguration(ctx context.Context, grantID string, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { return &domain.SchedulerConfiguration{ ID: "new-config", Name: req.Name, @@ -31,7 +31,7 @@ func (m *MockClient) CreateSchedulerConfiguration(ctx context.Context, req *doma }, nil } -func (m *MockClient) UpdateSchedulerConfiguration(ctx context.Context, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { +func (m *MockClient) UpdateSchedulerConfiguration(ctx context.Context, grantID, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { name := "Updated Configuration" if req.Name != nil { name = *req.Name @@ -42,7 +42,7 @@ func (m *MockClient) UpdateSchedulerConfiguration(ctx context.Context, configID }, nil } -func (m *MockClient) DeleteSchedulerConfiguration(ctx context.Context, configID string) error { +func (m *MockClient) DeleteSchedulerConfiguration(ctx context.Context, grantID, configID string) error { return nil } @@ -62,7 +62,7 @@ func (m *MockClient) GetSchedulerSession(ctx context.Context, sessionID string) }, nil } -func (m *MockClient) GetBooking(ctx context.Context, bookingID string) (*domain.Booking, error) { +func (m *MockClient) GetBooking(ctx context.Context, configurationID, bookingID string) (*domain.Booking, error) { return &domain.Booking{ BookingID: bookingID, Title: "Meeting with John", @@ -70,14 +70,14 @@ func (m *MockClient) GetBooking(ctx context.Context, bookingID string) (*domain. }, nil } -func (m *MockClient) ConfirmBooking(ctx context.Context, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { +func (m *MockClient) ConfirmBooking(ctx context.Context, configurationID, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { return &domain.Booking{ BookingID: bookingID, Status: "confirmed", }, nil } -func (m *MockClient) RescheduleBooking(ctx context.Context, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { +func (m *MockClient) RescheduleBooking(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { return &domain.Booking{ BookingID: bookingID, Status: "confirmed", @@ -86,7 +86,7 @@ func (m *MockClient) RescheduleBooking(ctx context.Context, bookingID string, re }, nil } -func (m *MockClient) CancelBooking(ctx context.Context, bookingID string, reason string) error { +func (m *MockClient) CancelBooking(ctx context.Context, configurationID, bookingID string, reason string) error { return nil } diff --git a/internal/adapters/nylas/scheduler.go b/internal/adapters/nylas/scheduler.go index 18db2be..c6512eb 100644 --- a/internal/adapters/nylas/scheduler.go +++ b/internal/adapters/nylas/scheduler.go @@ -2,35 +2,62 @@ package nylas import ( "context" + "errors" "fmt" "net/http" "net/url" + "time" "github.com/nylas/cli/internal/domain" ) // Scheduler Configurations -// ListSchedulerConfigurations retrieves all scheduler configurations. -func (c *HTTPClient) ListSchedulerConfigurations(ctx context.Context) ([]domain.SchedulerConfiguration, error) { - queryURL := fmt.Sprintf("%s/v3/scheduling/configurations", c.baseURL) - - var result struct { - Data []domain.SchedulerConfiguration `json:"data"` - } - if err := c.doGet(ctx, queryURL, &result); err != nil { +// ListSchedulerConfigurations retrieves all scheduler configurations for a grant. +func (c *HTTPClient) ListSchedulerConfigurations(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) { + if err := validateRequired("grant ID", grantID); err != nil { return nil, err } - return result.Data, nil + + // The configurations endpoint is cursor-paginated (limit + page_token / + // next_cursor). Follow the cursor so accounts with more than the default page + // of configurations aren't silently truncated. + baseURL := fmt.Sprintf("%s/v3/grants/%s/scheduling/configurations", c.baseURL, url.PathEscape(grantID)) + const maxConfigPages = 1000 + + // Non-nil so an empty result marshals to `[]`, not `null`. + all := make([]domain.SchedulerConfiguration, 0) + pageToken := "" + for range maxConfigPages { + queryURL := NewQueryBuilder().AddInt("limit", 200).Add("page_token", pageToken).BuildURL(baseURL) + + var result struct { + Data []domain.SchedulerConfiguration `json:"data"` + NextCursor string `json:"next_cursor,omitempty"` + } + if err := c.doGet(ctx, queryURL, &result); err != nil { + return nil, err + } + all = append(all, result.Data...) + + if result.NextCursor == "" { + return all, nil + } + pageToken = result.NextCursor + } + return nil, fmt.Errorf("failed to paginate scheduler configurations: exceeded max page count (%d)", maxConfigPages) } -// GetSchedulerConfiguration retrieves a specific scheduler configuration. -func (c *HTTPClient) GetSchedulerConfiguration(ctx context.Context, configID string) (*domain.SchedulerConfiguration, error) { +// GetSchedulerConfiguration retrieves a specific scheduler configuration for a grant. +func (c *HTTPClient) GetSchedulerConfiguration(ctx context.Context, grantID, configID string) (*domain.SchedulerConfiguration, error) { + if err := validateRequired("grant ID", grantID); err != nil { + return nil, err + } if err := validateRequired("configuration ID", configID); err != nil { return nil, err } - queryURL := fmt.Sprintf("%s/v3/scheduling/configurations/%s", c.baseURL, url.PathEscape(configID)) + queryURL := fmt.Sprintf("%s/v3/grants/%s/scheduling/configurations/%s", c.baseURL, url.PathEscape(grantID), url.PathEscape(configID)) var result struct { Data domain.SchedulerConfiguration `json:"data"` @@ -41,9 +68,13 @@ func (c *HTTPClient) GetSchedulerConfiguration(ctx context.Context, configID str return &result.Data, nil } -// CreateSchedulerConfiguration creates a new scheduler configuration. -func (c *HTTPClient) CreateSchedulerConfiguration(ctx context.Context, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { - queryURL := fmt.Sprintf("%s/v3/scheduling/configurations", c.baseURL) +// CreateSchedulerConfiguration creates a new scheduler configuration for a grant. +func (c *HTTPClient) CreateSchedulerConfiguration(ctx context.Context, grantID string, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { + if err := validateRequired("grant ID", grantID); err != nil { + return nil, err + } + + queryURL := fmt.Sprintf("%s/v3/grants/%s/scheduling/configurations", c.baseURL, url.PathEscape(grantID)) resp, err := c.doJSONRequest(ctx, "POST", queryURL, req) if err != nil { @@ -59,13 +90,16 @@ func (c *HTTPClient) CreateSchedulerConfiguration(ctx context.Context, req *doma return &result.Data, nil } -// UpdateSchedulerConfiguration updates an existing scheduler configuration. -func (c *HTTPClient) UpdateSchedulerConfiguration(ctx context.Context, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { +// UpdateSchedulerConfiguration updates an existing scheduler configuration for a grant. +func (c *HTTPClient) UpdateSchedulerConfiguration(ctx context.Context, grantID, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) { + if err := validateRequired("grant ID", grantID); err != nil { + return nil, err + } if err := validateRequired("configuration ID", configID); err != nil { return nil, err } - queryURL := fmt.Sprintf("%s/v3/scheduling/configurations/%s", c.baseURL, url.PathEscape(configID)) + queryURL := fmt.Sprintf("%s/v3/grants/%s/scheduling/configurations/%s", c.baseURL, url.PathEscape(grantID), url.PathEscape(configID)) resp, err := c.doJSONRequest(ctx, "PUT", queryURL, req) if err != nil { @@ -81,12 +115,15 @@ func (c *HTTPClient) UpdateSchedulerConfiguration(ctx context.Context, configID return &result.Data, nil } -// DeleteSchedulerConfiguration deletes a scheduler configuration. -func (c *HTTPClient) DeleteSchedulerConfiguration(ctx context.Context, configID string) error { +// DeleteSchedulerConfiguration deletes a scheduler configuration for a grant. +func (c *HTTPClient) DeleteSchedulerConfiguration(ctx context.Context, grantID, configID string) error { + if err := validateRequired("grant ID", grantID); err != nil { + return err + } if err := validateRequired("configuration ID", configID); err != nil { return err } - queryURL := fmt.Sprintf("%s/v3/scheduling/configurations/%s", c.baseURL, url.PathEscape(configID)) + queryURL := fmt.Sprintf("%s/v3/grants/%s/scheduling/configurations/%s", c.baseURL, url.PathEscape(grantID), url.PathEscape(configID)) return c.doDelete(ctx, queryURL) } @@ -94,6 +131,10 @@ func (c *HTTPClient) DeleteSchedulerConfiguration(ctx context.Context, configID // CreateSchedulerSession creates a new scheduler session. func (c *HTTPClient) CreateSchedulerSession(ctx context.Context, req *domain.CreateSchedulerSessionRequest) (*domain.SchedulerSession, error) { + // The v3 spec requires configuration_id or slug, and caps time_to_live. + if err := req.Validate(); err != nil { + return nil, err + } queryURL := fmt.Sprintf("%s/v3/scheduling/sessions", c.baseURL) resp, err := c.doJSONRequest(ctx, "POST", queryURL, req) @@ -129,36 +170,88 @@ func (c *HTTPClient) GetSchedulerSession(ctx context.Context, sessionID string) // Bookings -// GetBooking retrieves a specific booking. -func (c *HTTPClient) GetBooking(ctx context.Context, bookingID string) (*domain.Booking, error) { - if err := validateRequired("booking ID", bookingID); err != nil { - return nil, err +// bookingSessionToken mints a short-lived Scheduler session for a configuration +// and returns its token. Booking endpoints (/v3/scheduling/bookings/{id}) are +// authorized by a SCHEDULER_SESSION_TOKEN, not the application API key, so every +// booking operation exchanges a configuration ID for a session token first. +// +// ponytail: one fresh session per booking call, no caching — these are one-shot +// CLI/RPC operations. Cache per configuration if a batch caller ever needs it. +func (c *HTTPClient) bookingSessionToken(ctx context.Context, configurationID string) (string, error) { + if err := validateRequired("configuration ID", configurationID); err != nil { + return "", err + } + session, err := c.CreateSchedulerSession(ctx, &domain.CreateSchedulerSessionRequest{ + ConfigurationID: configurationID, + TimeToLive: 5, // minutes; enough for a single booking call + }) + if err != nil { + return "", fmt.Errorf("create scheduler session: %w", err) + } + if session.SessionID == "" { + return "", fmt.Errorf("scheduler session response missing session_id") } + return session.SessionID, nil +} +// getBookingWithToken reads a booking using an already-minted session token. +// Shared by GetBooking (mints then reads) and RescheduleBooking (reuses its +// token to read the updated booking back, since PATCH returns no booking body). +func (c *HTTPClient) getBookingWithToken(ctx context.Context, token, bookingID string) (*domain.Booking, error) { queryURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s", c.baseURL, url.PathEscape(bookingID)) var result struct { Data domain.Booking `json:"data"` } - if err := c.doGetWithNotFound(ctx, queryURL, &result, domain.ErrBookingNotFound); err != nil { + if err := c.doGetWithNotFoundAuth(ctx, queryURL, token, &result, domain.ErrBookingNotFound); err != nil { return nil, err } return &result.Data, nil } +// GetBooking retrieves a specific booking. +func (c *HTTPClient) GetBooking(ctx context.Context, configurationID, bookingID string) (*domain.Booking, error) { + if err := validateRequired("booking ID", bookingID); err != nil { + return nil, err + } + token, err := c.bookingSessionToken(ctx, configurationID) + if err != nil { + return nil, err + } + return c.getBookingWithToken(ctx, token, bookingID) +} + // ConfirmBooking confirms a booking. -func (c *HTTPClient) ConfirmBooking(ctx context.Context, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { +func (c *HTTPClient) ConfirmBooking(ctx context.Context, configurationID, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { if err := validateRequired("booking ID", bookingID); err != nil { return nil, err } + if req == nil { + return nil, fmt.Errorf("confirm booking request is required") + } + // The Nylas v3 spec requires salt and a confirmed/cancelled status. + if err := req.Validate(); err != nil { + return nil, err + } + token, err := c.bookingSessionToken(ctx, configurationID) + if err != nil { + return nil, err + } queryURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s", c.baseURL, url.PathEscape(bookingID)) - resp, err := c.doJSONRequest(ctx, "PUT", queryURL, req) + resp, err := c.doJSONRequestWithToken(ctx, "PUT", queryURL, token, req) if err != nil { return nil, err } + // Declining a pending booking (status "cancelled") returns the no-data + // delete envelope rather than a booking body, so don't try to decode one. + if req.Status == "cancelled" { + _ = resp.Body.Close() + return &domain.Booking{BookingID: bookingID, Status: "cancelled"}, nil + } + var result struct { Data domain.Booking `json:"data"` } @@ -169,37 +262,66 @@ func (c *HTTPClient) ConfirmBooking(ctx context.Context, bookingID string, req * } // RescheduleBooking reschedules a booking. -func (c *HTTPClient) RescheduleBooking(ctx context.Context, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { +func (c *HTTPClient) RescheduleBooking(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { if err := validateRequired("booking ID", bookingID); err != nil { return nil, err } - - queryURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s/reschedule", c.baseURL, url.PathEscape(bookingID)) - - resp, err := c.doJSONRequest(ctx, "PATCH", queryURL, req) + if req == nil { + return nil, fmt.Errorf("reschedule booking request is required") + } + token, err := c.bookingSessionToken(ctx, configurationID) if err != nil { return nil, err } - var result struct { - Data domain.Booking `json:"data"` - } - if err := c.decodeJSONResponse(resp, &result); err != nil { + // Nylas v3 reschedules via PATCH on the bare booking resource (no + // /reschedule suffix); the new start/end times are in the request body. + queryURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s", c.baseURL, url.PathEscape(bookingID)) + + // The PATCH response carries only a request_id (no booking body). Read the + // updated booking back for the full record; only a 404 (read racing ahead of + // propagation) may fall back to the requested times — any other failure is + // surfaced so a possibly-diverged server state is never reported as success. + resp, err := c.doJSONRequestWithToken(ctx, "PATCH", queryURL, token, req) + if err != nil { return nil, err } - return &result.Data, nil + _ = resp.Body.Close() + + start, end := time.Unix(req.StartTime, 0), time.Unix(req.EndTime, 0) + booking, err := c.getBookingWithToken(ctx, token, bookingID) + if err != nil { + // Single construction site for the partial-success record: callers on + // the ErrBookingReadBackFailed path reuse this booking as-is. + fallback := &domain.Booking{BookingID: bookingID, StartTime: start, EndTime: end} + if errors.Is(err, domain.ErrBookingNotFound) { + return fallback, nil + } + return fallback, fmt.Errorf("%w: %w", domain.ErrBookingReadBackFailed, err) + } + // The Nylas booking data model carries no start/end times (they exist only + // on the update request), so reflect the just-applied times onto the + // read-back record instead of leaving them zero. + booking.StartTime, booking.EndTime = start, end + return booking, nil } // CancelBooking cancels a booking. -func (c *HTTPClient) CancelBooking(ctx context.Context, bookingID string, reason string) error { +func (c *HTTPClient) CancelBooking(ctx context.Context, configurationID, bookingID string, reason string) error { if err := validateRequired("booking ID", bookingID); err != nil { return err } + token, err := c.bookingSessionToken(ctx, configurationID) + if err != nil { + return err + } - baseURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s", c.baseURL, url.PathEscape(bookingID)) - queryURL := NewQueryBuilder().Add("reason", reason).BuildURL(baseURL) + queryURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s", c.baseURL, url.PathEscape(bookingID)) + body := struct { + CancellationReason string `json:"cancellation_reason,omitempty"` + }{CancellationReason: reason} - resp, err := c.doJSONRequest(ctx, "DELETE", queryURL, nil, http.StatusOK, http.StatusNoContent) + resp, err := c.doJSONRequestWithToken(ctx, "DELETE", queryURL, token, body, http.StatusOK, http.StatusNoContent) if err != nil { return err } diff --git a/internal/adapters/nylas/scheduler_bookings_test.go b/internal/adapters/nylas/scheduler_bookings_test.go index d59dca9..dfb977c 100644 --- a/internal/adapters/nylas/scheduler_bookings_test.go +++ b/internal/adapters/nylas/scheduler_bookings_test.go @@ -79,8 +79,33 @@ func TestHTTPClient_GetSchedulerSession(t *testing.T) { // Booking Tests +// bookingTestServer answers the session-mint POST /v3/scheduling/sessions that +// precedes every booking call (returning sessionID), then delegates all other +// requests to bookingHandler. It asserts the booking request is authorized by +// the minted session token, not the application API key — the auth model the +// Nylas v3 booking endpoints actually require. +func bookingTestServer(t *testing.T, sessionID string, bookingHandler http.HandlerFunc) *httptest.Server { + t.Helper() + return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/v3/scheduling/sessions" { + assert.Equal(t, http.MethodPost, r.Method) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.NotEmpty(t, body["configuration_id"], "session mint must send configuration_id") + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{"session_id": sessionID}, + }) + return + } + assert.Equal(t, "Bearer "+sessionID, r.Header.Get("Authorization"), + "booking requests must use the session token, not the API key") + bookingHandler(w, r) + })) +} + func TestHTTPClient_GetBooking(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/v3/scheduling/bookings/booking-123", r.URL.Path) assert.Equal(t, "GET", r.Method) @@ -94,7 +119,7 @@ func TestHTTPClient_GetBooking(t *testing.T) { w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode( // Test helper, encode error not actionable response) - })) + }) defer server.Close() client := nylas.NewHTTPClient() @@ -102,7 +127,7 @@ func TestHTTPClient_GetBooking(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - booking, err := client.GetBooking(ctx, "booking-123") + booking, err := client.GetBooking(ctx, "config-1", "booking-123") require.NoError(t, err) assert.Equal(t, "booking-123", booking.BookingID) @@ -113,10 +138,21 @@ func TestHTTPClient_GetBooking(t *testing.T) { // Note: Bookings are created through the scheduler session flow, not directly func TestHTTPClient_ConfirmBooking(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/v3/scheduling/bookings/booking-123", r.URL.Path) assert.Equal(t, "PUT", r.Method) + // The spec requires salt + status in the body; cancellation_reason is + // optional. Verify they are sent (regression for the missing salt/ + // mislabelled reason bug). + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, "s4lt", body["salt"]) + assert.Equal(t, "confirmed", body["status"]) + assert.Equal(t, "changed my mind", body["cancellation_reason"]) + _, hasReason := body["reason"] + assert.False(t, hasReason, "must not send the non-spec 'reason' field") + response := map[string]any{ "data": map[string]any{ "booking_id": "booking-123", @@ -126,7 +162,7 @@ func TestHTTPClient_ConfirmBooking(t *testing.T) { w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode( // Test helper, encode error not actionable response) - })) + }) defer server.Close() client := nylas.NewHTTPClient() @@ -134,54 +170,232 @@ func TestHTTPClient_ConfirmBooking(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - req := &domain.ConfirmBookingRequest{} - booking, err := client.ConfirmBooking(ctx, "booking-123", req) + req := &domain.ConfirmBookingRequest{ + Salt: "s4lt", + Status: "confirmed", + CancellationReason: "changed my mind", + } + booking, err := client.ConfirmBooking(ctx, "config-1", "booking-123", req) require.NoError(t, err) assert.Equal(t, "booking-123", booking.BookingID) assert.Equal(t, "confirmed", booking.Status) } -func TestHTTPClient_RescheduleBooking(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/scheduling/bookings/booking-456/reschedule", r.URL.Path) - assert.Equal(t, "PATCH", r.Method) - - response := map[string]any{ - "data": map[string]any{ - "booking_id": "booking-456", - "status": "confirmed", - }, - } +func TestHTTPClient_ConfirmBooking_CancelledReturnsNoDataEnvelope(t *testing.T) { + // Declining a pending booking (status "cancelled") returns the no-data delete + // envelope, not a booking body; the adapter must not try to decode a booking. + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, http.MethodPut, r.Method) w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode( // Test helper, encode error not actionable - response) + _ = json.NewEncoder(w).Encode(map[string]any{"request_id": "req-1"}) // no "data" + }) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + booking, err := client.ConfirmBooking(context.Background(), "config-1", "booking-9", + &domain.ConfirmBookingRequest{Salt: "s4lt", Status: "cancelled"}) + + require.NoError(t, err) + assert.Equal(t, "booking-9", booking.BookingID) + assert.Equal(t, "cancelled", booking.Status) +} + +func TestHTTPClient_ConfirmBooking_Validation(t *testing.T) { + // The adapter is the API boundary; it must reject spec-invalid confirm + // requests (nil, missing salt, bad status) before hitting the network, + // regardless of what the CLI/RPC callers validate. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("unexpected request to %s — invalid confirm should not reach the API", r.URL.Path) })) defer server.Close() client := nylas.NewHTTPClient() client.SetCredentials("client-id", "secret", "api-key") client.SetBaseURL(server.URL) + ctx := context.Background() + + tests := []struct { + name string + req *domain.ConfirmBookingRequest + }{ + {name: "nil request", req: nil}, + {name: "missing salt", req: &domain.ConfirmBookingRequest{Status: "confirmed"}}, + {name: "empty status", req: &domain.ConfirmBookingRequest{Salt: "s4lt"}}, + {name: "invalid status", req: &domain.ConfirmBookingRequest{Salt: "s4lt", Status: "maybe"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := client.ConfirmBooking(ctx, "config-1", "booking-1", tt.req) + require.Error(t, err) + }) + } +} + +func TestHTTPClient_CreateSchedulerSession_RejectsInvalidRequest(t *testing.T) { + // The adapter must reject spec-invalid session requests before any HTTP + // call: nil, missing configuration_id/slug, and out-of-range TTL. The + // substring assertions pin the failure to Validate() — a transport error + // from the unreachable base URL must not satisfy the test. + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL("http://127.0.0.1:0") // any request would fail loudly ctx := context.Background() - req := &domain.RescheduleBookingRequest{ - StartTime: 1704067200, - EndTime: 1704070800, + for name, tc := range map[string]struct { + req *domain.CreateSchedulerSessionRequest + wantErr string + }{ + "nil request": {req: nil, wantErr: "session request is required"}, + "no config or slug": {req: &domain.CreateSchedulerSessionRequest{TimeToLive: 10}, wantErr: "configuration_id or slug is required"}, + "ttl above cap": {req: &domain.CreateSchedulerSessionRequest{ConfigurationID: "config-1", TimeToLive: 31}, wantErr: "time_to_live"}, + } { + t.Run(name, func(t *testing.T) { + _, err := client.CreateSchedulerSession(ctx, tc.req) + require.Error(t, err) + assert.Contains(t, err.Error(), tc.wantErr) + }) } - booking, err := client.RescheduleBooking(ctx, "booking-456", req) +} + +func TestHTTPClient_CancelBooking_EmptyReasonOmitsField(t *testing.T) { + // With an empty reason the cancellation_reason field must be omitted from + // the body (omitempty), matching the spec where it is optional. + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + _, present := body["cancellation_reason"] + assert.False(t, present, "cancellation_reason must be absent when reason is empty") + w.WriteHeader(http.StatusNoContent) + }) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + require.NoError(t, client.CancelBooking(context.Background(), "config-1", "booking-1", "")) +} + +func TestHTTPClient_RescheduleBooking(t *testing.T) { + // PATCH returns only a request_id; the adapter reads the booking back for the + // full record (status/title/etc.). + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/v3/scheduling/bookings/booking-456", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + switch r.Method { + case http.MethodPatch: + _ = json.NewEncoder(w).Encode(map[string]any{"request_id": "req-1"}) + case http.MethodGet: + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": map[string]any{"booking_id": "booking-456", "status": "confirmed", "title": "Interview"}, + }) + default: + t.Errorf("unexpected method %s on reschedule", r.Method) + } + }) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + ctx := context.Background() + req := &domain.RescheduleBookingRequest{StartTime: 1704067200, EndTime: 1704070800} + booking, err := client.RescheduleBooking(ctx, "config-1", "booking-456", req) require.NoError(t, err) assert.Equal(t, "booking-456", booking.BookingID) + // Full record comes from the read-back... + assert.Equal(t, "confirmed", booking.Status) + assert.Equal(t, "Interview", booking.Title) + // ...but the booking model has no times, so they come from the request. + assert.Equal(t, int64(1704067200), booking.StartTime.Unix()) + assert.Equal(t, int64(1704070800), booking.EndTime.Unix()) +} + +func TestHTTPClient_RescheduleBooking_FallsBackWhenReadBackNotFound(t *testing.T) { + // If the read-back GET races ahead of propagation (404), a successful + // reschedule must still succeed, reflecting the requested times. + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodPatch: + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"request_id": "req-1"}) + case http.MethodGet: + w.WriteHeader(http.StatusNotFound) + default: + t.Errorf("unexpected method %s", r.Method) + } + }) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + req := &domain.RescheduleBookingRequest{StartTime: 1704067200, EndTime: 1704070800} + booking, err := client.RescheduleBooking(context.Background(), "config-1", "booking-456", req) + + require.NoError(t, err) + assert.Equal(t, "booking-456", booking.BookingID) + assert.Equal(t, int64(1704067200), booking.StartTime.Unix()) + assert.Equal(t, int64(1704070800), booking.EndTime.Unix()) +} + +func TestHTTPClient_RescheduleBooking_SurfacesReadBackError(t *testing.T) { + // Only a propagation 404 may silently fall back to the requested times; any + // other read-back failure (auth, 5xx, rate limit) must surface the typed + // partial success so a possibly-diverged server state is never reported as + // a clean, verified result. 403 keeps the test out of the 5xx retry loop. + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodPatch: + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"request_id": "req-1"}) + case http.MethodGet: + w.WriteHeader(http.StatusForbidden) + default: + t.Errorf("unexpected method %s", r.Method) + } + }) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + req := &domain.RescheduleBookingRequest{StartTime: 1704067200, EndTime: 1704070800} + booking, err := client.RescheduleBooking(context.Background(), "config-1", "booking-456", req) + + require.Error(t, err) + // The typed sentinel lets the CLI/RPC boundaries report the reschedule as + // applied while surfacing the verification failure... + assert.ErrorIs(t, err, domain.ErrBookingReadBackFailed) + // ...using the partial record the port contract promises alongside it. + require.NotNil(t, booking) + assert.Equal(t, "booking-456", booking.BookingID) + assert.Equal(t, int64(1704067200), booking.StartTime.Unix()) + assert.Equal(t, int64(1704070800), booking.EndTime.Unix()) } func TestHTTPClient_CancelBooking(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/v3/scheduling/bookings/booking-cancel", r.URL.Path) assert.Equal(t, "DELETE", r.Method) - assert.Equal(t, "cancelled", r.URL.Query().Get("reason")) + // The spec carries the reason in the JSON body as cancellation_reason, + // not as a query param (regression for the dropped-reason bug). + assert.Empty(t, r.URL.Query().Get("reason")) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, "user cancelled", body["cancellation_reason"]) w.WriteHeader(http.StatusNoContent) - })) + }) defer server.Close() client := nylas.NewHTTPClient() @@ -189,7 +403,7 @@ func TestHTTPClient_CancelBooking(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - err := client.CancelBooking(ctx, "booking-cancel", "cancelled") + err := client.CancelBooking(ctx, "config-1", "booking-cancel", "user cancelled") require.NoError(t, err) } @@ -205,18 +419,18 @@ func TestHTTPClient_RescheduleBooking_URLEscaping(t *testing.T) { { name: "simple ID", bookingID: "booking-123", - expectedPath: "/v3/scheduling/bookings/booking-123/reschedule", + expectedPath: "/v3/scheduling/bookings/booking-123", }, { name: "ID with slash", bookingID: "booking/123", - expectedPath: "/v3/scheduling/bookings/booking%2F123/reschedule", + expectedPath: "/v3/scheduling/bookings/booking%2F123", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { // Check the path - RawPath only set for escaped chars, Path always has decoded value if r.URL.RawPath != "" { assert.Equal(t, tt.expectedPath, r.URL.RawPath) @@ -232,7 +446,7 @@ func TestHTTPClient_RescheduleBooking_URLEscaping(t *testing.T) { } w.Header().Set("Content-Type", "application/json") _ = json.NewEncoder(w).Encode(response) - })) + }) defer server.Close() client := nylas.NewHTTPClient() @@ -244,7 +458,7 @@ func TestHTTPClient_RescheduleBooking_URLEscaping(t *testing.T) { StartTime: 1704067200, EndTime: 1704070800, } - _, err := client.RescheduleBooking(ctx, tt.bookingID, req) + _, err := client.RescheduleBooking(ctx, "config-1", tt.bookingID, req) require.NoError(t, err) }) } @@ -252,54 +466,46 @@ func TestHTTPClient_RescheduleBooking_URLEscaping(t *testing.T) { func TestHTTPClient_CancelBooking_URLEscaping(t *testing.T) { tests := []struct { - name string - bookingID string - reason string - expectedPath string - expectedQuery string + name string + bookingID string + reason string + expectedPath string }{ { - name: "simple values", - bookingID: "booking-123", - reason: "cancelled", - expectedPath: "/v3/scheduling/bookings/booking-123", - expectedQuery: "reason=cancelled", - }, - { - name: "reason with spaces", - bookingID: "booking-123", - reason: "user requested cancellation", - expectedPath: "/v3/scheduling/bookings/booking-123", - expectedQuery: "reason=user+requested+cancellation", + name: "simple values", + bookingID: "booking-123", + reason: "cancelled", + expectedPath: "/v3/scheduling/bookings/booking-123", }, { - name: "reason with special chars", - bookingID: "booking-123", - reason: "conflict & reschedule", - expectedPath: "/v3/scheduling/bookings/booking-123", - expectedQuery: "reason=conflict+%26+reschedule", + name: "reason with special chars", + bookingID: "booking-123", + reason: "conflict & reschedule", + expectedPath: "/v3/scheduling/bookings/booking-123", }, { - name: "ID with slash", - bookingID: "booking/123", - reason: "test", - expectedPath: "/v3/scheduling/bookings/booking%2F123", - expectedQuery: "reason=test", + name: "ID with slash", + bookingID: "booking/123", + reason: "test", + expectedPath: "/v3/scheduling/bookings/booking%2F123", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + server := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { // Check path escaping (use RawPath for escaped, Path for unescaped) if tt.expectedPath != r.URL.Path { assert.Equal(t, tt.expectedPath, r.URL.RawPath) } - // Check query escaping - assert.Equal(t, tt.expectedQuery, r.URL.RawQuery) + // The reason travels in the JSON body, not the query string. + assert.Empty(t, r.URL.RawQuery) + var body map[string]any + require.NoError(t, json.NewDecoder(r.Body).Decode(&body)) + assert.Equal(t, tt.reason, body["cancellation_reason"]) w.WriteHeader(http.StatusNoContent) - })) + }) defer server.Close() client := nylas.NewHTTPClient() @@ -307,7 +513,7 @@ func TestHTTPClient_CancelBooking_URLEscaping(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - err := client.CancelBooking(ctx, tt.bookingID, tt.reason) + err := client.CancelBooking(ctx, "config-1", tt.bookingID, tt.reason) require.NoError(t, err) }) } diff --git a/internal/adapters/nylas/scheduler_config_test.go b/internal/adapters/nylas/scheduler_config_test.go index 1a711ef..7a4beab 100644 --- a/internal/adapters/nylas/scheduler_config_test.go +++ b/internal/adapters/nylas/scheduler_config_test.go @@ -17,7 +17,7 @@ import ( func TestHTTPClient_ListSchedulerConfigurations(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/scheduling/configurations", r.URL.Path) + assert.Equal(t, "/v3/grants/grant-123/scheduling/configurations", r.URL.Path) assert.Equal(t, "GET", r.Method) response := map[string]any{ @@ -45,7 +45,7 @@ func TestHTTPClient_ListSchedulerConfigurations(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - configs, err := client.ListSchedulerConfigurations(ctx) + configs, err := client.ListSchedulerConfigurations(ctx, "grant-123") require.NoError(t, err) assert.Len(t, configs, 2) @@ -54,9 +54,59 @@ func TestHTTPClient_ListSchedulerConfigurations(t *testing.T) { assert.Equal(t, "30min", configs[0].Slug) } +func TestHTTPClient_ListSchedulerConfigurations_EmptyReturnsNonNilSlice(t *testing.T) { + // An empty result must be a non-nil slice so JSON output is `[]`, not `null`. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(map[string]any{"data": []any{}}) + })) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + configs, err := client.ListSchedulerConfigurations(context.Background(), "grant-123") + + require.NoError(t, err) + assert.NotNil(t, configs, "empty result must be a non-nil slice (marshals to [] not null)") + assert.Empty(t, configs) +} + +func TestHTTPClient_ListSchedulerConfigurations_FollowsCursor(t *testing.T) { + // The endpoint is cursor-paginated; the adapter must follow next_cursor and + // aggregate rather than truncating at the first page. + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + assert.Equal(t, "/v3/grants/grant-123/scheduling/configurations", r.URL.Path) + w.Header().Set("Content-Type", "application/json") + if r.URL.Query().Get("page_token") == "" { + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": []map[string]any{{"id": "config-1"}}, + "next_cursor": "cursor-2", + }) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{ + "data": []map[string]any{{"id": "config-2"}}, + }) + })) + defer server.Close() + + client := nylas.NewHTTPClient() + client.SetCredentials("client-id", "secret", "api-key") + client.SetBaseURL(server.URL) + + configs, err := client.ListSchedulerConfigurations(context.Background(), "grant-123") + + require.NoError(t, err) + require.Len(t, configs, 2) + assert.Equal(t, "config-1", configs[0].ID) + assert.Equal(t, "config-2", configs[1].ID) +} + func TestHTTPClient_GetSchedulerConfiguration(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/scheduling/configurations/config-123", r.URL.Path) + assert.Equal(t, "/v3/grants/grant-123/scheduling/configurations/config-123", r.URL.Path) assert.Equal(t, "GET", r.Method) response := map[string]any{ @@ -84,7 +134,7 @@ func TestHTTPClient_GetSchedulerConfiguration(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - config, err := client.GetSchedulerConfiguration(ctx, "config-123") + config, err := client.GetSchedulerConfiguration(ctx, "grant-123", "config-123") require.NoError(t, err) assert.Equal(t, "config-123", config.ID) @@ -95,7 +145,7 @@ func TestHTTPClient_GetSchedulerConfiguration(t *testing.T) { func TestHTTPClient_CreateSchedulerConfiguration(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/scheduling/configurations", r.URL.Path) + assert.Equal(t, "/v3/grants/grant-123/scheduling/configurations", r.URL.Path) assert.Equal(t, "POST", r.Method) assert.Equal(t, "application/json", r.Header.Get("Content-Type")) @@ -125,7 +175,7 @@ func TestHTTPClient_CreateSchedulerConfiguration(t *testing.T) { req := &domain.CreateSchedulerConfigurationRequest{ Name: "New Meeting Type", } - config, err := client.CreateSchedulerConfiguration(ctx, req) + config, err := client.CreateSchedulerConfiguration(ctx, "grant-123", req) require.NoError(t, err) assert.Equal(t, "config-new", config.ID) @@ -134,7 +184,7 @@ func TestHTTPClient_CreateSchedulerConfiguration(t *testing.T) { func TestHTTPClient_UpdateSchedulerConfiguration(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/scheduling/configurations/config-456", r.URL.Path) + assert.Equal(t, "/v3/grants/grant-123/scheduling/configurations/config-456", r.URL.Path) assert.Equal(t, "PUT", r.Method) var body map[string]any @@ -162,7 +212,7 @@ func TestHTTPClient_UpdateSchedulerConfiguration(t *testing.T) { req := &domain.UpdateSchedulerConfigurationRequest{ Name: strPtr("Updated Meeting"), } - config, err := client.UpdateSchedulerConfiguration(ctx, "config-456", req) + config, err := client.UpdateSchedulerConfiguration(ctx, "grant-123", "config-456", req) require.NoError(t, err) assert.Equal(t, "config-456", config.ID) @@ -171,7 +221,7 @@ func TestHTTPClient_UpdateSchedulerConfiguration(t *testing.T) { func TestHTTPClient_DeleteSchedulerConfiguration(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - assert.Equal(t, "/v3/scheduling/configurations/config-delete", r.URL.Path) + assert.Equal(t, "/v3/grants/grant-123/scheduling/configurations/config-delete", r.URL.Path) assert.Equal(t, "DELETE", r.Method) w.WriteHeader(http.StatusOK) @@ -183,7 +233,7 @@ func TestHTTPClient_DeleteSchedulerConfiguration(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - err := client.DeleteSchedulerConfiguration(ctx, "config-delete") + err := client.DeleteSchedulerConfiguration(ctx, "grant-123", "config-delete") require.NoError(t, err) } diff --git a/internal/adapters/nylas/scheduler_mock_test.go b/internal/adapters/nylas/scheduler_mock_test.go index aa77036..860102b 100644 --- a/internal/adapters/nylas/scheduler_mock_test.go +++ b/internal/adapters/nylas/scheduler_mock_test.go @@ -15,29 +15,29 @@ func TestMockClient_SchedulerOperations(t *testing.T) { mock := nylas.NewMockClient() // Test ListSchedulerConfigurations - configs, err := mock.ListSchedulerConfigurations(ctx) + configs, err := mock.ListSchedulerConfigurations(ctx, "grant-123") require.NoError(t, err) assert.NotEmpty(t, configs) // Test GetSchedulerConfiguration - config, err := mock.GetSchedulerConfiguration(ctx, "config-123") + config, err := mock.GetSchedulerConfiguration(ctx, "grant-123", "config-123") require.NoError(t, err) assert.Equal(t, "config-123", config.ID) // Test CreateSchedulerConfiguration createReq := &domain.CreateSchedulerConfigurationRequest{Name: "Test Config"} - created, err := mock.CreateSchedulerConfiguration(ctx, createReq) + created, err := mock.CreateSchedulerConfiguration(ctx, "grant-123", createReq) require.NoError(t, err) assert.NotEmpty(t, created.ID) // Test UpdateSchedulerConfiguration updateReq := &domain.UpdateSchedulerConfigurationRequest{Name: strPtr("Updated")} - updated, err := mock.UpdateSchedulerConfiguration(ctx, "config-456", updateReq) + updated, err := mock.UpdateSchedulerConfiguration(ctx, "grant-123", "config-456", updateReq) require.NoError(t, err) assert.Equal(t, "Updated", updated.Name) // Test DeleteSchedulerConfiguration - err = mock.DeleteSchedulerConfiguration(ctx, "config-789") + err = mock.DeleteSchedulerConfiguration(ctx, "grant-123", "config-789") require.NoError(t, err) // Test CreateSchedulerSession @@ -52,13 +52,13 @@ func TestMockClient_SchedulerOperations(t *testing.T) { assert.Equal(t, "session-123", getSession.SessionID) // Test GetBooking - booking, err := mock.GetBooking(ctx, "booking-123") + booking, err := mock.GetBooking(ctx, "config-1", "booking-123") require.NoError(t, err) assert.Equal(t, "booking-123", booking.BookingID) // Test ConfirmBooking confirmReq := &domain.ConfirmBookingRequest{} - confirmed, err := mock.ConfirmBooking(ctx, "booking-123", confirmReq) + confirmed, err := mock.ConfirmBooking(ctx, "config-1", "booking-123", confirmReq) require.NoError(t, err) assert.Equal(t, "confirmed", confirmed.Status) @@ -67,12 +67,12 @@ func TestMockClient_SchedulerOperations(t *testing.T) { StartTime: 1704067200, EndTime: 1704070800, } - rescheduled, err := mock.RescheduleBooking(ctx, "booking-456", rescheduleReq) + rescheduled, err := mock.RescheduleBooking(ctx, "config-1", "booking-456", rescheduleReq) require.NoError(t, err) assert.NotEmpty(t, rescheduled.BookingID) // Test CancelBooking - err = mock.CancelBooking(ctx, "booking-789", "User cancelled") + err = mock.CancelBooking(ctx, "config-1", "booking-789", "User cancelled") require.NoError(t, err) } @@ -84,29 +84,29 @@ func TestDemoClient_SchedulerOperations(t *testing.T) { demo := nylas.NewDemoClient() // Test ListSchedulerConfigurations - configs, err := demo.ListSchedulerConfigurations(ctx) + configs, err := demo.ListSchedulerConfigurations(ctx, "grant-123") require.NoError(t, err) assert.NotEmpty(t, configs) // Test GetSchedulerConfiguration - config, err := demo.GetSchedulerConfiguration(ctx, "demo-config") + config, err := demo.GetSchedulerConfiguration(ctx, "grant-123", "demo-config") require.NoError(t, err) assert.NotEmpty(t, config.ID) // Test CreateSchedulerConfiguration createReq := &domain.CreateSchedulerConfigurationRequest{Name: "Demo Config"} - created, err := demo.CreateSchedulerConfiguration(ctx, createReq) + created, err := demo.CreateSchedulerConfiguration(ctx, "grant-123", createReq) require.NoError(t, err) assert.Equal(t, "Demo Config", created.Name) // Test UpdateSchedulerConfiguration updateReq := &domain.UpdateSchedulerConfigurationRequest{Name: strPtr("Demo Updated")} - updated, err := demo.UpdateSchedulerConfiguration(ctx, "demo-config", updateReq) + updated, err := demo.UpdateSchedulerConfiguration(ctx, "grant-123", "demo-config", updateReq) require.NoError(t, err) assert.Equal(t, "Demo Updated", updated.Name) // Test DeleteSchedulerConfiguration - err = demo.DeleteSchedulerConfiguration(ctx, "demo-config") + err = demo.DeleteSchedulerConfiguration(ctx, "grant-123", "demo-config") require.NoError(t, err) // Test sessions, bookings, and pages diff --git a/internal/adapters/rpcserver/handlers_admin.go b/internal/adapters/rpcserver/handlers_admin.go index 4981f1b..a28a729 100644 --- a/internal/adapters/rpcserver/handlers_admin.go +++ b/internal/adapters/rpcserver/handlers_admin.go @@ -3,7 +3,9 @@ package rpcserver import ( "context" "encoding/json" + "errors" "fmt" + "strings" "github.com/nylas/cli/internal/domain" "github.com/nylas/cli/internal/ports" @@ -81,6 +83,7 @@ type credentialListResult struct { } type credentialGetParams struct { + ConnectorID string `json:"connector_id"` CredentialID string `json:"credential_id"` } @@ -90,11 +93,13 @@ type credentialCreateParams struct { } type credentialUpdateParams struct { + ConnectorID string `json:"connector_id"` CredentialID string `json:"credential_id"` domain.UpdateCredentialRequest } type credentialDeleteParams struct { + ConnectorID string `json:"connector_id"` CredentialID string `json:"credential_id"` } @@ -132,6 +137,49 @@ type grantsListAllResult struct { Grants []domain.Grant `json:"grants"` } +// resolveConnectorID returns the explicit connector provider (rejecting +// deprecated ones), or auto-detects it when the application has exactly one +// connector — mirroring the CLI's `credentials` commands. Credentials live under +// /v3/connectors/{provider}/creds, so a provider is always required. The +// resolution policy is shared via domain.ResolveConnectorProvider so this RPC +// surface and the CLI cannot diverge; this wrapper only supplies the connector +// list and maps errors to RPC errors. +func resolveConnectorID(ctx context.Context, client ports.AdminClient, explicit string) (string, error) { + connectors, listErr := client.ListConnectors(ctx) + + provider, err := domain.ResolveConnectorProvider(connectors, explicit) + if err == nil { + return provider, nil + } + + // Discovery failed and nothing was named to fall back on: surface the real + // listing error rather than a misleading "no connectors found". + if listErr != nil && explicit == "" { + return "", fmt.Errorf("resolve connector: %w", listErr) + } + // Same masking guard as the CLI: a discovery failure must not surface as a + // misleading "unknown connector" when an explicit legacy ID couldn't be + // mapped only because the connector list was unavailable. + if listErr != nil && errors.Is(err, domain.ErrUnknownConnector) { + return "", fmt.Errorf("resolve connector: %w", listErr) + } + + var multi *domain.MultipleConnectorsError + switch { + case errors.As(err, &multi): + return "", NewRPCError(InvalidParams, + fmt.Sprintf("connector_id required: multiple connectors (%s)", strings.Join(multi.Providers, ", ")), nil) + case errors.Is(err, domain.ErrDeprecatedConnector): + return "", NewRPCError(InvalidParams, + fmt.Sprintf("connector provider %q is no longer supported", explicit), nil) + case errors.Is(err, domain.ErrUnknownConnector): + return "", NewRPCError(InvalidParams, + fmt.Sprintf("unknown connector provider %q", explicit), nil) + default: // domain.ErrNoConnectors + return "", NewRPCError(InvalidParams, "connector_id required: no connectors found", nil) + } +} + func RegisterAdminHandlers(d *Dispatcher, client ports.AdminClient) { d.Register("admin.app.list", func(ctx context.Context, params json.RawMessage) (any, error) { apps, err := client.ListApplications(ctx) @@ -342,11 +390,12 @@ func RegisterAdminHandlers(d *Dispatcher, client ports.AdminClient) { if err := decodeParams(params, &p); err != nil { return nil, err } - if p.ConnectorID == "" { - return nil, NewRPCError(InvalidParams, "connector_id required", nil) + connectorID, err := resolveConnectorID(ctx, client, p.ConnectorID) + if err != nil { + return nil, err } - credentials, err := client.ListCredentials(ctx, p.ConnectorID) + credentials, err := client.ListCredentials(ctx, connectorID) if err != nil { return nil, fmt.Errorf("admin.credential.list: %w", err) } @@ -361,8 +410,12 @@ func RegisterAdminHandlers(d *Dispatcher, client ports.AdminClient) { if p.CredentialID == "" { return nil, NewRPCError(InvalidParams, "credential_id required", nil) } + connectorID, err := resolveConnectorID(ctx, client, p.ConnectorID) + if err != nil { + return nil, err + } - credential, err := client.GetCredential(ctx, p.CredentialID) + credential, err := client.GetCredential(ctx, connectorID, p.CredentialID) if err != nil { return nil, fmt.Errorf("admin.credential.get: %w", err) } @@ -374,11 +427,12 @@ func RegisterAdminHandlers(d *Dispatcher, client ports.AdminClient) { if err := decodeParams(params, &p); err != nil { return nil, err } - if p.ConnectorID == "" { - return nil, NewRPCError(InvalidParams, "connector_id required", nil) + connectorID, err := resolveConnectorID(ctx, client, p.ConnectorID) + if err != nil { + return nil, err } - credential, err := client.CreateCredential(ctx, p.ConnectorID, &p.CreateCredentialRequest) + credential, err := client.CreateCredential(ctx, connectorID, &p.CreateCredentialRequest) if err != nil { return nil, fmt.Errorf("admin.credential.create: %w", err) } @@ -393,8 +447,12 @@ func RegisterAdminHandlers(d *Dispatcher, client ports.AdminClient) { if p.CredentialID == "" { return nil, NewRPCError(InvalidParams, "credential_id required", nil) } + connectorID, err := resolveConnectorID(ctx, client, p.ConnectorID) + if err != nil { + return nil, err + } - credential, err := client.UpdateCredential(ctx, p.CredentialID, &p.UpdateCredentialRequest) + credential, err := client.UpdateCredential(ctx, connectorID, p.CredentialID, &p.UpdateCredentialRequest) if err != nil { return nil, fmt.Errorf("admin.credential.update: %w", err) } @@ -409,8 +467,12 @@ func RegisterAdminHandlers(d *Dispatcher, client ports.AdminClient) { if p.CredentialID == "" { return nil, NewRPCError(InvalidParams, "credential_id required", nil) } + connectorID, err := resolveConnectorID(ctx, client, p.ConnectorID) + if err != nil { + return nil, err + } - if err := client.DeleteCredential(ctx, p.CredentialID); err != nil { + if err := client.DeleteCredential(ctx, connectorID, p.CredentialID); err != nil { return nil, fmt.Errorf("admin.credential.delete: %w", err) } return deletedResult{Deleted: true}, nil diff --git a/internal/adapters/rpcserver/handlers_admin_test.go b/internal/adapters/rpcserver/handlers_admin_test.go index bc937aa..c8a0392 100644 --- a/internal/adapters/rpcserver/handlers_admin_test.go +++ b/internal/adapters/rpcserver/handlers_admin_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "strings" "testing" "github.com/nylas/cli/internal/domain" @@ -16,8 +17,9 @@ type fakeAdminClient struct { listApplications func(context.Context) ([]domain.Application, error) createCallbackURI func(context.Context, *domain.CreateCallbackURIRequest) (*domain.CallbackURI, error) updateConnector func(context.Context, string, *domain.UpdateConnectorRequest) (*domain.Connector, error) + listConnectors func(context.Context) ([]domain.Connector, error) listCredentials func(context.Context, string) ([]domain.ConnectorCredential, error) - getCredential func(context.Context, string) (*domain.ConnectorCredential, error) + getCredential func(context.Context, string, string) (*domain.ConnectorCredential, error) getWorkspace func(context.Context, string) (*domain.Workspace, error) assignWorkspaceGrants func(context.Context, string, *domain.WorkspaceAssignRequest) (*domain.WorkspaceAssignResult, error) listAllGrants func(context.Context, *domain.GrantsQueryParams) ([]domain.Grant, error) @@ -45,6 +47,13 @@ func (f *fakeAdminClient) UpdateConnector(ctx context.Context, connectorID strin return f.updateConnector(ctx, connectorID, req) } +func (f *fakeAdminClient) ListConnectors(ctx context.Context) ([]domain.Connector, error) { + if f.listConnectors == nil { + return nil, errors.New("unexpected ListConnectors") + } + return f.listConnectors(ctx) +} + func (f *fakeAdminClient) ListCredentials(ctx context.Context, connectorID string) ([]domain.ConnectorCredential, error) { if f.listCredentials == nil { return nil, errors.New("unexpected ListCredentials") @@ -52,11 +61,11 @@ func (f *fakeAdminClient) ListCredentials(ctx context.Context, connectorID strin return f.listCredentials(ctx, connectorID) } -func (f *fakeAdminClient) GetCredential(ctx context.Context, credentialID string) (*domain.ConnectorCredential, error) { +func (f *fakeAdminClient) GetCredential(ctx context.Context, connectorID, credentialID string) (*domain.ConnectorCredential, error) { if f.getCredential == nil { return nil, errors.New("unexpected GetCredential") } - return f.getCredential(ctx, credentialID) + return f.getCredential(ctx, connectorID, credentialID) } func (f *fakeAdminClient) GetWorkspace(ctx context.Context, workspaceID string) (*domain.Workspace, error) { @@ -277,15 +286,18 @@ func TestRegisterAdminHandlers(t *testing.T) { }, }, { - name: "admin.credential.list returns credentials", + name: "admin.credential.list maps legacy connector_id to provider", method: "admin.credential.list", params: `{"connector_id":"connector-1"}`, client: &fakeAdminClient{ + listConnectors: func(ctx context.Context) ([]domain.Connector, error) { + return []domain.Connector{{ID: "connector-1", Provider: "google"}}, nil + }, listCredentials: func(ctx context.Context, connectorID string) ([]domain.ConnectorCredential, error) { - if connectorID != "connector-1" { - t.Fatalf("connectorID = %q, want connector-1", connectorID) + if connectorID != "google" { + t.Fatalf("connectorID = %q, want provider google", connectorID) } - return []domain.ConnectorCredential{{ID: "credential-1", Name: "OAuth", CredentialType: "oauth"}}, nil + return []domain.ConnectorCredential{{ID: "credential-1", Name: "Google Credential"}}, nil }, }, assert: func(t *testing.T, resp rpcTestResponse) { @@ -301,9 +313,61 @@ func TestRegisterAdminHandlers(t *testing.T) { }, }, { - name: "admin.credential.list missing connector_id returns invalid params", + name: "admin.credential.list missing connector_id with no unique connector returns invalid params", + method: "admin.credential.list", + params: `{}`, + client: &fakeAdminClient{ + listConnectors: func(ctx context.Context) ([]domain.Connector, error) { + // Zero (or multiple) connectors → cannot auto-detect. + return nil, nil + }, + }, + assert: func(t *testing.T, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + }, + }, + { + name: "admin.credential.list auto-detects the sole connector when connector_id omitted", + method: "admin.credential.list", + params: `{}`, + client: &fakeAdminClient{ + listConnectors: func(ctx context.Context) ([]domain.Connector, error) { + return []domain.Connector{{ID: "connector-1", Provider: "google"}}, nil + }, + listCredentials: func(ctx context.Context, connectorID string) ([]domain.ConnectorCredential, error) { + if connectorID != "google" { + t.Fatalf("connectorID = %q, want auto-detected provider google", connectorID) + } + return []domain.ConnectorCredential{{ID: "credential-1"}}, nil + }, + }, + assert: func(t *testing.T, resp rpcTestResponse) { + requireNoRPCError(t, resp) + }, + }, + { + name: "admin.credential.list omitted connector_id with multiple connectors names them", method: "admin.credential.list", params: `{}`, + client: &fakeAdminClient{ + listConnectors: func(ctx context.Context) ([]domain.Connector, error) { + return []domain.Connector{ + {ID: "c1", Provider: "google"}, + {ID: "c2", Provider: "microsoft"}, + }, nil + }, + }, + assert: func(t *testing.T, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if !strings.Contains(resp.Error.Message, "google") || !strings.Contains(resp.Error.Message, "microsoft") { + t.Fatalf("error %q should name the candidate providers google and microsoft", resp.Error.Message) + } + }, + }, + { + name: "admin.credential.get missing credential_id returns invalid params", + method: "admin.credential.get", + params: `{"connector_id":"connector-1"}`, client: &fakeAdminClient{}, assert: func(t *testing.T, resp rpcTestResponse) { requireRPCErrorCode(t, resp, InvalidParams) @@ -312,9 +376,9 @@ func TestRegisterAdminHandlers(t *testing.T) { { name: "admin.credential.get client error maps to internal error", method: "admin.credential.get", - params: `{"credential_id":"credential-1"}`, + params: `{"connector_id":"google","credential_id":"credential-1"}`, client: &fakeAdminClient{ - getCredential: func(ctx context.Context, credentialID string) (*domain.ConnectorCredential, error) { + getCredential: func(ctx context.Context, connectorID, credentialID string) (*domain.ConnectorCredential, error) { return nil, clientErr }, }, diff --git a/internal/adapters/rpcserver/handlers_scheduler.go b/internal/adapters/rpcserver/handlers_scheduler.go index fe8d4b1..07638f7 100644 --- a/internal/adapters/rpcserver/handlers_scheduler.go +++ b/internal/adapters/rpcserver/handlers_scheduler.go @@ -3,6 +3,7 @@ package rpcserver import ( "context" "encoding/json" + "errors" "fmt" "github.com/nylas/cli/internal/domain" @@ -13,50 +14,89 @@ type schedulerConfigListResult struct { Configurations []domain.SchedulerConfiguration `json:"configurations"` } +type schedulerConfigListParams struct { + GrantID string `json:"grant_id,omitempty"` +} + type schedulerConfigGetParams struct { + GrantID string `json:"grant_id,omitempty"` ConfigID string `json:"config_id"` } type schedulerConfigCreateParams struct { + GrantID string `json:"grant_id,omitempty"` domain.CreateSchedulerConfigurationRequest } type schedulerConfigUpdateParams struct { + GrantID string `json:"grant_id,omitempty"` ConfigID string `json:"config_id"` domain.UpdateSchedulerConfigurationRequest } type schedulerSessionCreateParams struct { domain.CreateSchedulerSessionRequest + // TTL is the legacy field name for time_to_live; accepted so older + // clients keep working. + TTL int `json:"ttl,omitempty"` } type schedulerSessionGetParams struct { SessionID string `json:"session_id"` } +// Booking endpoints authenticate with a Scheduler session token minted from the +// configuration, so every booking RPC requires configuration_id. type schedulerBookingGetParams struct { - BookingID string `json:"booking_id"` + ConfigurationID string `json:"configuration_id"` + BookingID string `json:"booking_id"` } type schedulerBookingConfirmParams struct { - BookingID string `json:"booking_id"` + ConfigurationID string `json:"configuration_id"` + BookingID string `json:"booking_id"` domain.ConfirmBookingRequest + // Reason is the legacy field name for cancellation_reason; accepted so + // older clients keep working (mirrors scheduler.booking.cancel). + Reason string `json:"reason,omitempty"` } type schedulerBookingRescheduleParams struct { - BookingID string `json:"booking_id"` + ConfigurationID string `json:"configuration_id"` + BookingID string `json:"booking_id"` domain.RescheduleBookingRequest } type schedulerBookingCancelParams struct { - BookingID string `json:"booking_id"` - Reason string `json:"reason,omitempty"` + ConfigurationID string `json:"configuration_id"` + BookingID string `json:"booking_id"` + CancellationReason string `json:"cancellation_reason,omitempty"` + // Reason is the legacy field name; accepted so older clients keep working. + Reason string `json:"reason,omitempty"` +} + +// cancellationReason returns the cancellation reason, preferring the spec field +// over the legacy "reason" alias. +func (p schedulerBookingCancelParams) cancellationReason() string { + if p.CancellationReason != "" { + return p.CancellationReason + } + return p.Reason } type schedulerBookingCancelResult struct { Cancelled bool `json:"cancelled"` } +// schedulerBookingRescheduleResult is a booking plus an optional warning set +// when the reschedule was applied but the record could not be read back +// (domain.ErrBookingReadBackFailed) — a partial success must stay +// distinguishable from a verified one. +type schedulerBookingRescheduleResult struct { + domain.Booking + Warning string `json:"warning,omitempty"` +} + type schedulerGroupEventListParams struct { GrantID string `json:"grant_id,omitempty"` ConfigID string `json:"config_id"` @@ -95,12 +135,16 @@ type schedulerGroupEventResult struct { func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defaultGrant string) { d.Register("scheduler.config.list", func(ctx context.Context, params json.RawMessage) (any, error) { - var p struct{} + var p schedulerConfigListParams if err := decodeParams(params, &p); err != nil { return nil, err } + grantID, err := resolveGrant(p.GrantID, defaultGrant) + if err != nil { + return nil, err + } - configurations, err := client.ListSchedulerConfigurations(ctx) + configurations, err := client.ListSchedulerConfigurations(ctx, grantID) if err != nil { return nil, fmt.Errorf("scheduler.config.list: %w", err) } @@ -115,8 +159,12 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.ConfigID == "" { return nil, NewRPCError(InvalidParams, "config_id required", nil) } + grantID, err := resolveGrant(p.GrantID, defaultGrant) + if err != nil { + return nil, err + } - config, err := client.GetSchedulerConfiguration(ctx, p.ConfigID) + config, err := client.GetSchedulerConfiguration(ctx, grantID, p.ConfigID) if err != nil { return nil, fmt.Errorf("scheduler.config.get: %w", err) } @@ -128,8 +176,12 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if err := decodeParams(params, &p); err != nil { return nil, err } + grantID, err := resolveGrant(p.GrantID, defaultGrant) + if err != nil { + return nil, err + } - config, err := client.CreateSchedulerConfiguration(ctx, &p.CreateSchedulerConfigurationRequest) + config, err := client.CreateSchedulerConfiguration(ctx, grantID, &p.CreateSchedulerConfigurationRequest) if err != nil { return nil, fmt.Errorf("scheduler.config.create: %w", err) } @@ -144,8 +196,12 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.ConfigID == "" { return nil, NewRPCError(InvalidParams, "config_id required", nil) } + grantID, err := resolveGrant(p.GrantID, defaultGrant) + if err != nil { + return nil, err + } - config, err := client.UpdateSchedulerConfiguration(ctx, p.ConfigID, &p.UpdateSchedulerConfigurationRequest) + config, err := client.UpdateSchedulerConfiguration(ctx, grantID, p.ConfigID, &p.UpdateSchedulerConfigurationRequest) if err != nil { return nil, fmt.Errorf("scheduler.config.update: %w", err) } @@ -160,8 +216,12 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.ConfigID == "" { return nil, NewRPCError(InvalidParams, "config_id required", nil) } + grantID, err := resolveGrant(p.GrantID, defaultGrant) + if err != nil { + return nil, err + } - if err := client.DeleteSchedulerConfiguration(ctx, p.ConfigID); err != nil { + if err := client.DeleteSchedulerConfiguration(ctx, grantID, p.ConfigID); err != nil { return nil, fmt.Errorf("scheduler.config.delete: %w", err) } return deletedResult{Deleted: true}, nil @@ -172,6 +232,13 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if err := decodeParams(params, &p); err != nil { return nil, err } + if p.TimeToLive == 0 { + p.TimeToLive = p.TTL + } + // The v3 spec requires configuration_id or slug, and caps time_to_live. + if err := p.Validate(); err != nil { + return nil, NewRPCError(InvalidParams, err.Error(), nil) + } session, err := client.CreateSchedulerSession(ctx, &p.CreateSchedulerSessionRequest) if err != nil { @@ -204,8 +271,11 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.BookingID == "" { return nil, NewRPCError(InvalidParams, "booking_id required", nil) } + if p.ConfigurationID == "" { + return nil, NewRPCError(InvalidParams, "configuration_id required", nil) + } - booking, err := client.GetBooking(ctx, p.BookingID) + booking, err := client.GetBooking(ctx, p.ConfigurationID, p.BookingID) if err != nil { return nil, fmt.Errorf("scheduler.booking.get: %w", err) } @@ -220,8 +290,20 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.BookingID == "" { return nil, NewRPCError(InvalidParams, "booking_id required", nil) } + if p.ConfigurationID == "" { + return nil, NewRPCError(InvalidParams, "configuration_id required", nil) + } + // Only a decline carries a reason; a stray legacy "reason" on a + // confirmation was ignored before the alias existed and must stay so. + if p.CancellationReason == "" && p.Status == "cancelled" { + p.CancellationReason = p.Reason + } + // The Nylas v3 spec requires salt + status on the confirm payload. + if err := p.Validate(); err != nil { + return nil, NewRPCError(InvalidParams, err.Error(), nil) + } - booking, err := client.ConfirmBooking(ctx, p.BookingID, &p.ConfirmBookingRequest) + booking, err := client.ConfirmBooking(ctx, p.ConfigurationID, p.BookingID, &p.ConfirmBookingRequest) if err != nil { return nil, fmt.Errorf("scheduler.booking.confirm: %w", err) } @@ -236,12 +318,28 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.BookingID == "" { return nil, NewRPCError(InvalidParams, "booking_id required", nil) } + if p.ConfigurationID == "" { + return nil, NewRPCError(InvalidParams, "configuration_id required", nil) + } + // Mirror the CLI's guard: reschedule requires valid new start/end times. + if p.StartTime == 0 || p.EndTime == 0 { + return nil, NewRPCError(InvalidParams, "start_time and end_time are required", nil) + } + if p.EndTime <= p.StartTime { + return nil, NewRPCError(InvalidParams, "end_time must be after start_time", nil) + } - booking, err := client.RescheduleBooking(ctx, p.BookingID, &p.RescheduleBookingRequest) + booking, err := client.RescheduleBooking(ctx, p.ConfigurationID, p.BookingID, &p.RescheduleBookingRequest) if err != nil { + // Typed partial success: the reschedule was applied but the record + // could not be read back — return the port's partial booking with + // an explicit warning instead of failing an applied change. + if errors.Is(err, domain.ErrBookingReadBackFailed) && booking != nil { + return schedulerBookingRescheduleResult{Booking: *booking, Warning: err.Error()}, nil + } return nil, fmt.Errorf("scheduler.booking.reschedule: %w", err) } - return booking, nil + return schedulerBookingRescheduleResult{Booking: *booking}, nil }) d.Register("scheduler.booking.cancel", func(ctx context.Context, params json.RawMessage) (any, error) { @@ -252,8 +350,11 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa if p.BookingID == "" { return nil, NewRPCError(InvalidParams, "booking_id required", nil) } + if p.ConfigurationID == "" { + return nil, NewRPCError(InvalidParams, "configuration_id required", nil) + } - if err := client.CancelBooking(ctx, p.BookingID, p.Reason); err != nil { + if err := client.CancelBooking(ctx, p.ConfigurationID, p.BookingID, p.cancellationReason()); err != nil { return nil, fmt.Errorf("scheduler.booking.cancel: %w", err) } return schedulerBookingCancelResult{Cancelled: true}, nil diff --git a/internal/adapters/rpcserver/handlers_scheduler_test.go b/internal/adapters/rpcserver/handlers_scheduler_test.go index dbf34d0..06452b9 100644 --- a/internal/adapters/rpcserver/handlers_scheduler_test.go +++ b/internal/adapters/rpcserver/handlers_scheduler_test.go @@ -2,8 +2,11 @@ package rpcserver import ( "context" + "encoding/json" "errors" + "fmt" "testing" + "time" "github.com/nylas/cli/internal/domain" "github.com/nylas/cli/internal/ports" @@ -12,34 +15,70 @@ import ( type fakeSchedulerClient struct { ports.SchedulerClient - listSchedulerConfigurations func(context.Context) ([]domain.SchedulerConfiguration, error) - getSchedulerConfiguration func(context.Context, string) (*domain.SchedulerConfiguration, error) + listSchedulerConfigurations func(context.Context, string) ([]domain.SchedulerConfiguration, error) + getSchedulerConfiguration func(context.Context, string, string) (*domain.SchedulerConfiguration, error) getSchedulerSession func(context.Context, string) (*domain.SchedulerSession, error) - getBooking func(context.Context, string) (*domain.Booking, error) + getBooking func(context.Context, string, string) (*domain.Booking, error) listGroupEvents func(context.Context, string, string, string, int64, int64) ([]domain.GroupEvent, error) - configID string - sessionID string - bookingID string - grantID string - calendarID string - startTime int64 - endTime int64 + configID string + configurationID string + sessionID string + bookingID string + grantID string + calendarID string + startTime int64 + endTime int64 + cancelReason string + confirmReq *domain.ConfirmBookingRequest + sessionReq *domain.CreateSchedulerSessionRequest + + rescheduleBooking func(context.Context, string, string, *domain.RescheduleBookingRequest) (*domain.Booking, error) +} + +func (f *fakeSchedulerClient) RescheduleBooking(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { + f.configurationID = configurationID + f.bookingID = bookingID + if f.rescheduleBooking == nil { + return nil, errors.New("unexpected RescheduleBooking") + } + return f.rescheduleBooking(ctx, configurationID, bookingID, req) +} + +func (f *fakeSchedulerClient) CancelBooking(ctx context.Context, configurationID, bookingID, reason string) error { + f.configurationID = configurationID + f.bookingID = bookingID + f.cancelReason = reason + return nil +} + +func (f *fakeSchedulerClient) ConfirmBooking(ctx context.Context, configurationID, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) { + f.configurationID = configurationID + f.bookingID = bookingID + f.confirmReq = req + return &domain.Booking{BookingID: bookingID}, nil } -func (f *fakeSchedulerClient) ListSchedulerConfigurations(ctx context.Context) ([]domain.SchedulerConfiguration, error) { +func (f *fakeSchedulerClient) CreateSchedulerSession(ctx context.Context, req *domain.CreateSchedulerSessionRequest) (*domain.SchedulerSession, error) { + f.sessionReq = req + return &domain.SchedulerSession{SessionID: "session-1"}, nil +} + +func (f *fakeSchedulerClient) ListSchedulerConfigurations(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) { + f.grantID = grantID if f.listSchedulerConfigurations == nil { return nil, errors.New("unexpected ListSchedulerConfigurations") } - return f.listSchedulerConfigurations(ctx) + return f.listSchedulerConfigurations(ctx, grantID) } -func (f *fakeSchedulerClient) GetSchedulerConfiguration(ctx context.Context, configID string) (*domain.SchedulerConfiguration, error) { +func (f *fakeSchedulerClient) GetSchedulerConfiguration(ctx context.Context, grantID, configID string) (*domain.SchedulerConfiguration, error) { + f.grantID = grantID f.configID = configID if f.getSchedulerConfiguration == nil { return nil, errors.New("unexpected GetSchedulerConfiguration") } - return f.getSchedulerConfiguration(ctx, configID) + return f.getSchedulerConfiguration(ctx, grantID, configID) } func (f *fakeSchedulerClient) GetSchedulerSession(ctx context.Context, sessionID string) (*domain.SchedulerSession, error) { @@ -50,12 +89,13 @@ func (f *fakeSchedulerClient) GetSchedulerSession(ctx context.Context, sessionID return f.getSchedulerSession(ctx, sessionID) } -func (f *fakeSchedulerClient) GetBooking(ctx context.Context, bookingID string) (*domain.Booking, error) { +func (f *fakeSchedulerClient) GetBooking(ctx context.Context, configurationID, bookingID string) (*domain.Booking, error) { + f.configurationID = configurationID f.bookingID = bookingID if f.getBooking == nil { return nil, errors.New("unexpected GetBooking") } - return f.getBooking(ctx, bookingID) + return f.getBooking(ctx, configurationID, bookingID) } func (f *fakeSchedulerClient) ListGroupEvents(ctx context.Context, grantID, configID, calendarID string, startTime, endTime int64) ([]domain.GroupEvent, error) { @@ -82,11 +122,11 @@ func TestRegisterSchedulerHandlers(t *testing.T) { assert func(*testing.T, *fakeSchedulerClient, rpcTestResponse) }{ { - name: "scheduler.config.list returns configurations", + name: "scheduler.config.list forwards grant and returns configurations", method: "scheduler.config.list", - params: `{}`, + params: `{"grant_id":"grant-1"}`, client: &fakeSchedulerClient{ - listSchedulerConfigurations: func(ctx context.Context) ([]domain.SchedulerConfiguration, error) { + listSchedulerConfigurations: func(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) { return []domain.SchedulerConfiguration{{ID: "config-1", Name: "Intro"}}, nil }, }, @@ -98,6 +138,38 @@ func TestRegisterSchedulerHandlers(t *testing.T) { if len(result.Configurations) != 1 || result.Configurations[0].ID != "config-1" { t.Fatalf("configurations = %+v, want config-1", result.Configurations) } + if client.grantID != "grant-1" { + t.Fatalf("grantID = %q, want grant-1", client.grantID) + } + }, + }, + { + name: "scheduler.config.list uses default grant when omitted", + method: "scheduler.config.list", + params: `{}`, + defaultGrant: "default-grant", + client: &fakeSchedulerClient{ + listSchedulerConfigurations: func(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) { + return nil, nil + }, + }, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.grantID != "default-grant" { + t.Fatalf("grantID = %q, want default-grant", client.grantID) + } + }, + }, + { + name: "scheduler.config.list requires a grant", + method: "scheduler.config.list", + params: `{}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if client.grantID != "" { + t.Fatalf("ListSchedulerConfigurations called with grant %q, want no call", client.grantID) + } }, }, { @@ -113,16 +185,19 @@ func TestRegisterSchedulerHandlers(t *testing.T) { }, }, { - name: "scheduler.config.get client error maps to internal error", + name: "scheduler.config.get forwards grant and maps client error", method: "scheduler.config.get", - params: `{"config_id":"config-1"}`, + params: `{"grant_id":"grant-1","config_id":"config-1"}`, client: &fakeSchedulerClient{ - getSchedulerConfiguration: func(ctx context.Context, configID string) (*domain.SchedulerConfiguration, error) { + getSchedulerConfiguration: func(ctx context.Context, grantID, configID string) (*domain.SchedulerConfiguration, error) { return nil, clientErr }, }, assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { requireRPCErrorCode(t, resp, InternalError) + if client.grantID != "grant-1" { + t.Fatalf("grantID = %q, want grant-1", client.grantID) + } if client.configID != "config-1" { t.Fatalf("configID = %q, want config-1", client.configID) } @@ -165,14 +240,17 @@ func TestRegisterSchedulerHandlers(t *testing.T) { { name: "scheduler.booking.get returns booking", method: "scheduler.booking.get", - params: `{"booking_id":"booking-1"}`, + params: `{"configuration_id":"config-1","booking_id":"booking-1"}`, client: &fakeSchedulerClient{ - getBooking: func(ctx context.Context, bookingID string) (*domain.Booking, error) { + getBooking: func(ctx context.Context, configurationID, bookingID string) (*domain.Booking, error) { return &domain.Booking{BookingID: bookingID, Title: "Intro", Status: "confirmed"}, nil }, }, assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { requireNoRPCError(t, resp) + if client.configurationID != "config-1" { + t.Fatalf("configurationID = %q, want config-1", client.configurationID) + } if client.bookingID != "booking-1" { t.Fatalf("bookingID = %q, want booking-1", client.bookingID) } @@ -196,6 +274,252 @@ func TestRegisterSchedulerHandlers(t *testing.T) { } }, }, + { + // Booking endpoints authenticate with a session token minted from the + // configuration, so configuration_id is required before any API call. + name: "scheduler.booking.get requires configuration_id", + method: "scheduler.booking.get", + params: `{"booking_id":"booking-1"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if client.configurationID != "" { + t.Fatalf("GetBooking called with configuration %q, want no call", client.configurationID) + } + }, + }, + { + name: "scheduler.booking.confirm requires salt", + method: "scheduler.booking.confirm", + params: `{"configuration_id":"config-1","booking_id":"booking-1","status":"confirmed"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if client.bookingID != "" { + t.Fatalf("ConfirmBooking called with booking %q, want no call", client.bookingID) + } + }, + }, + { + name: "scheduler.booking.confirm requires valid status", + method: "scheduler.booking.confirm", + params: `{"configuration_id":"config-1","booking_id":"booking-1","salt":"s4lt","status":"bogus"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if client.bookingID != "" { + t.Fatalf("ConfirmBooking called with booking %q, want no call", client.bookingID) + } + }, + }, + { + // Declining via confirm carries the reason too; the legacy "reason" + // alias must keep working just like scheduler.booking.cancel's. + name: "scheduler.booking.confirm accepts legacy reason field", + method: "scheduler.booking.confirm", + params: `{"configuration_id":"config-1","booking_id":"booking-1","salt":"s4lt","status":"cancelled","reason":"organizer conflict"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.confirmReq == nil || client.confirmReq.CancellationReason != "organizer conflict" { + t.Fatalf("confirmReq = %+v, want legacy reason forwarded as cancellation_reason", client.confirmReq) + } + }, + }, + { + name: "scheduler.booking.confirm prefers cancellation_reason over legacy reason", + method: "scheduler.booking.confirm", + params: `{"configuration_id":"config-1","booking_id":"booking-1","salt":"s4lt","status":"cancelled","cancellation_reason":"spec","reason":"legacy"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.confirmReq == nil || client.confirmReq.CancellationReason != "spec" { + t.Fatalf("confirmReq = %+v, want cancellation_reason to win", client.confirmReq) + } + }, + }, + { + // The session TTL param was renamed ttl -> time_to_live to match the + // Nylas v3 spec; older RPC clients still sending "ttl" must not have + // their TTL silently dropped. + name: "scheduler.session.create accepts legacy ttl field", + method: "scheduler.session.create", + params: `{"configuration_id":"config-1","ttl":25}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.sessionReq == nil || client.sessionReq.TimeToLive != 25 { + t.Fatalf("sessionReq = %+v, want legacy ttl forwarded as time_to_live", client.sessionReq) + } + }, + }, + { + // Trust boundary: malformed session params must be rejected before + // they reach the Nylas API (spec: configuration_id OR slug required). + name: "scheduler.session.create requires configuration_id or slug", + method: "scheduler.session.create", + params: `{"time_to_live":10}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if client.sessionReq != nil { + t.Fatalf("CreateSchedulerSession called with %+v, want no call", client.sessionReq) + } + }, + }, + { + name: "scheduler.session.create accepts slug without configuration_id", + method: "scheduler.session.create", + params: `{"slug":"my-page"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.sessionReq == nil || client.sessionReq.Slug != "my-page" { + t.Fatalf("sessionReq = %+v, want slug forwarded", client.sessionReq) + } + }, + }, + { + // The spec caps time_to_live at 30 minutes. + name: "scheduler.session.create rejects ttl above the 30-minute cap", + method: "scheduler.session.create", + params: `{"configuration_id":"config-1","time_to_live":31}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + if client.sessionReq != nil { + t.Fatalf("CreateSchedulerSession called with %+v, want no call", client.sessionReq) + } + }, + }, + { + name: "scheduler.session.create prefers time_to_live over legacy ttl", + method: "scheduler.session.create", + params: `{"configuration_id":"config-1","time_to_live":10,"ttl":25}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.sessionReq == nil || client.sessionReq.TimeToLive != 10 { + t.Fatalf("sessionReq = %+v, want time_to_live to win", client.sessionReq) + } + }, + }, + { + name: "scheduler.booking.cancel accepts legacy reason field", + method: "scheduler.booking.cancel", + params: `{"configuration_id":"config-1","booking_id":"booking-1","reason":"customer no-show"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.cancelReason != "customer no-show" { + t.Fatalf("cancelReason = %q, want the legacy reason field to be forwarded", client.cancelReason) + } + }, + }, + { + name: "scheduler.booking.cancel prefers cancellation_reason over legacy reason", + method: "scheduler.booking.cancel", + params: `{"configuration_id":"config-1","booking_id":"booking-1","cancellation_reason":"spec","reason":"legacy"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.cancelReason != "spec" { + t.Fatalf("cancelReason = %q, want cancellation_reason to win", client.cancelReason) + } + }, + }, + { + // A stray legacy "reason" on a status:"confirmed" payload was ignored + // before the alias existed; it must not start flowing to the API as a + // cancellation_reason on a confirmation. + name: "scheduler.booking.confirm ignores legacy reason unless cancelling", + method: "scheduler.booking.confirm", + params: `{"configuration_id":"config-1","booking_id":"booking-1","salt":"s4lt","status":"confirmed","reason":"stray note"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + if client.confirmReq == nil || client.confirmReq.CancellationReason != "" { + t.Fatalf("confirmReq = %+v, want stray legacy reason dropped on confirm", client.confirmReq) + } + }, + }, + { + // The reschedule PATCH applied but the read-back could not verify the + // record: a typed partial success, not a failure — the client must get + // the port's partial booking plus an explicit warning, so it stays + // distinguishable from a verified success. + name: "scheduler.booking.reschedule returns booking and warning when read-back fails", + method: "scheduler.booking.reschedule", + params: `{"configuration_id":"config-1","booking_id":"booking-1","start_time":1704067200,"end_time":1704070800}`, + client: &fakeSchedulerClient{ + rescheduleBooking: func(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { + return &domain.Booking{BookingID: bookingID, StartTime: time.Unix(req.StartTime, 0), EndTime: time.Unix(req.EndTime, 0)}, + fmt.Errorf("%w: transient failure", domain.ErrBookingReadBackFailed) + }, + }, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + + var result schedulerBookingRescheduleResult + unmarshalResult(t, resp, &result) + if result.BookingID != "booking-1" { + t.Fatalf("booking = %+v, want booking-1 returned as partial success", result.Booking) + } + if result.StartTime.Unix() != 1704067200 || result.EndTime.Unix() != 1704070800 { + t.Fatalf("times = %v %v, want the applied request times", result.StartTime, result.EndTime) + } + if result.Warning == "" { + t.Fatal("Warning is empty, want the read-back failure surfaced") + } + var raw map[string]json.RawMessage + if err := json.Unmarshal(resp.Result, &raw); err != nil { + t.Fatalf("unmarshal raw result: %v", err) + } + if _, ok := raw["warning"]; !ok { + t.Fatal("warning key absent from JSON, want it present on partial success") + } + }, + }, + { + // A fully verified reschedule must NOT carry a warning. + name: "scheduler.booking.reschedule omits warning on verified success", + method: "scheduler.booking.reschedule", + params: `{"configuration_id":"config-1","booking_id":"booking-1","start_time":1704067200,"end_time":1704070800}`, + client: &fakeSchedulerClient{ + rescheduleBooking: func(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) { + return &domain.Booking{BookingID: bookingID, Status: "confirmed"}, nil + }, + }, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireNoRPCError(t, resp) + + var result schedulerBookingRescheduleResult + unmarshalResult(t, resp, &result) + if result.BookingID != "booking-1" || result.Status != "confirmed" { + t.Fatalf("booking = %+v, want the read-back record", result.Booking) + } + // Assert on the raw JSON: omitempty must drop the key entirely, an + // empty-string decode cannot distinguish absent from "". + var raw map[string]json.RawMessage + if err := json.Unmarshal(resp.Result, &raw); err != nil { + t.Fatalf("unmarshal raw result: %v", err) + } + if _, ok := raw["warning"]; ok { + t.Fatalf("warning key present in JSON (%s), want it omitted on verified success", raw["warning"]) + } + }, + }, + { + // Mirror the CLI guard: reschedule must reject zero/invalid times + // before minting a session or calling the API. + name: "scheduler.booking.reschedule requires valid times", + method: "scheduler.booking.reschedule", + params: `{"configuration_id":"config-1","booking_id":"booking-1"}`, + client: &fakeSchedulerClient{}, + assert: func(t *testing.T, client *fakeSchedulerClient, resp rpcTestResponse) { + requireRPCErrorCode(t, resp, InvalidParams) + }, + }, { name: "scheduler.groupEvent.list forwards grant and returns events", method: "scheduler.groupEvent.list", diff --git a/internal/cli/admin/credentials.go b/internal/cli/admin/credentials.go index 9c90106..5696853 100644 --- a/internal/cli/admin/credentials.go +++ b/internal/cli/admin/credentials.go @@ -3,7 +3,9 @@ package admin import ( "context" "encoding/json" + "errors" "fmt" + "strings" "github.com/nylas/cli/internal/cli/common" "github.com/nylas/cli/internal/domain" @@ -11,6 +13,53 @@ import ( "github.com/spf13/cobra" ) +// resolveConnectorID returns the explicit connector provider when given +// (rejecting deprecated ones), otherwise auto-detects it when the application +// has exactly one connector. Connector credentials are keyed by provider (e.g. +// "google") in the /v3/connectors/{provider}/creds path, so a provider is always +// required. The resolution policy lives in domain.ResolveConnectorProvider so the +// CLI and RPC surfaces cannot diverge; this wrapper only supplies the connector +// list and maps errors to CLI user errors. +func resolveConnectorID(ctx context.Context, client ports.NylasClient, explicit string) (string, error) { + connectors, listErr := client.ListConnectors(ctx) + + provider, err := domain.ResolveConnectorProvider(connectors, explicit) + if err == nil { + return provider, nil + } + + // Discovery failed: surface the real listing error rather than a misleading + // "no connectors found" (empty input) or "unknown connector" (an explicit + // legacy ID we simply couldn't map because the list was unavailable). + if listErr != nil && (explicit == "" || errors.Is(err, domain.ErrUnknownConnector)) { + return "", fmt.Errorf("resolve connector: %w", listErr) + } + + var multi *domain.MultipleConnectorsError + switch { + case errors.As(err, &multi): + return "", common.NewUserError( + fmt.Sprintf("multiple connectors found (%s); specify which one", strings.Join(multi.Providers, ", ")), + "Pass the provider with --connector (e.g. --connector google)", + ) + case errors.Is(err, domain.ErrDeprecatedConnector): + return "", common.NewUserError( + fmt.Sprintf("connector provider %q is no longer supported", explicit), + "Choose a supported connector (e.g. --connector google)", + ) + case errors.Is(err, domain.ErrUnknownConnector): + return "", common.NewUserError( + fmt.Sprintf("unknown connector provider %q", explicit), + "Use a supported provider (google, microsoft, imap, icloud, yahoo, ews, virtual-calendar, zoom, nylas)", + ) + default: // domain.ErrNoConnectors + return "", common.NewUserError( + "no connectors found", + "Create a connector first, or pass the provider with --connector (e.g. --connector google)", + ) + } +} + func newCredentialsCmd() *cobra.Command { cmd := &cobra.Command{ Use: "credentials", @@ -32,14 +81,21 @@ API reference: https://developer.nylas.com/docs/reference/api/connector-credenti func newCredentialListCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "list ", + Use: "list [connector]", Aliases: []string{"ls"}, Short: "List credentials for a connector", - Long: "List all authentication credentials for a specific connector.", - Args: cobra.ExactArgs(1), + Long: "List all authentication credentials for a connector. The connector\nprovider is auto-detected when the application has exactly one connector.", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - connectorID := args[0] + explicit := "" + if len(args) > 0 { + explicit = args[0] + } _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { + connectorID, err := resolveConnectorID(ctx, client, explicit) + if err != nil { + return struct{}{}, err + } credentials, err := client.ListCredentials(ctx, connectorID) if err != nil { return struct{}{}, common.WrapListError("credentials", err) @@ -56,9 +112,9 @@ func newCredentialListCmd() *cobra.Command { fmt.Printf("Found %d credential(s):\n\n", len(credentials)) - table := common.NewTable("NAME", "ID", "TYPE", "CREATED AT") + table := common.NewTable("NAME", "ID", "CREATED AT") for _, cred := range credentials { - table.AddRow(common.Cyan.Sprint(cred.Name), cred.ID, common.Green.Sprint(cred.CredentialType), formatUnixTime(cred.CreatedAt)) + table.AddRow(common.Cyan.Sprint(cred.Name), cred.ID, formatUnixTime(cred.CreatedAt)) } table.Render() @@ -72,15 +128,20 @@ func newCredentialListCmd() *cobra.Command { } func newCredentialShowCmd() *cobra.Command { + var connector string cmd := &cobra.Command{ Use: "show ", Short: "Show credential details", - Long: "Show detailed information about a specific credential.", + Long: "Show detailed information about a specific credential. The connector\nprovider is auto-detected when the application has exactly one connector.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { credentialID := args[0] _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { - credential, err := client.GetCredential(ctx, credentialID) + connectorID, err := resolveConnectorID(ctx, client, connector) + if err != nil { + return struct{}{}, err + } + credential, err := client.GetCredential(ctx, connectorID, credentialID) if err != nil { return struct{}{}, common.WrapGetError("credential", err) } @@ -91,8 +152,6 @@ func newCredentialShowCmd() *cobra.Command { _, _ = common.Bold.Printf("Credential: %s\n", credential.Name) fmt.Printf(" ID: %s\n", common.Cyan.Sprint(credential.ID)) - fmt.Printf(" Connector ID: %s\n", credential.ConnectorID) - fmt.Printf(" Type: %s\n", common.Green.Sprint(credential.CredentialType)) fmt.Printf(" Created At: %s\n", formatUnixTime(credential.CreatedAt)) fmt.Printf(" Updated At: %s\n", formatUnixTime(credential.UpdatedAt)) @@ -102,6 +161,8 @@ func newCredentialShowCmd() *cobra.Command { }, } + cmd.Flags().StringVar(&connector, "connector", "", "Connector provider (e.g. google); auto-detected if only one connector exists") + return cmd } @@ -117,9 +178,31 @@ func newCredentialCreateCmd() *cobra.Command { cmd := &cobra.Command{ Use: "create", Short: "Create a credential", - Long: "Create a new authentication credential for a connector.", + Long: `Create a connector credential from a provider OAuth application. + +--client-id and --client-secret are the PROVIDER's OAuth app credentials (e.g. +your own Google Cloud project or Azure app) — NOT your Nylas application's. Nylas +uses them to broker authentication through that provider app (for example, to +authenticate enterprise customers who own their provider application, or to run +your own app alongside the Nylas Shared GCP App).`, RunE: func(cmd *cobra.Command, args []string) error { + // The v3 API also accepts serviceaccount/adminconsent, but those + // flows need credential_data this command doesn't collect — reject + // unsupported types here instead of sending a payload the API 400s. + // Direct comparison, not ValidateOneOf: that helper treats an + // explicitly empty value as OK, which would slip past this guard. + if credentialType != "connector" { + return common.NewUserError( + fmt.Sprintf("invalid type: %q", credentialType), + "This command only supports --type connector; serviceaccount/adminconsent need credential data it doesn't collect", + ) + } _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { + connectorProvider, rerr := resolveConnectorID(ctx, client, connectorID) + if rerr != nil { + return struct{}{}, rerr + } + req := &domain.CreateCredentialRequest{ Name: name, CredentialType: credentialType, @@ -133,14 +216,13 @@ func newCredentialCreateCmd() *cobra.Command { } } - credential, err := client.CreateCredential(ctx, connectorID, req) + credential, err := client.CreateCredential(ctx, connectorProvider, req) if err != nil { return struct{}{}, common.WrapCreateError("credential", err) } _, _ = common.Green.Printf("Created credential: %s\n", credential.Name) fmt.Printf(" ID: %s\n", common.Cyan.Sprint(credential.ID)) - fmt.Printf(" Type: %s\n", credential.CredentialType) return struct{}{}, nil }) @@ -148,37 +230,61 @@ func newCredentialCreateCmd() *cobra.Command { }, } - cmd.Flags().StringVar(&connectorID, "connector-id", "", "Connector ID (required)") + cmd.Flags().StringVar(&connectorID, "connector", "", "Connector provider (e.g. google); auto-detected if only one connector exists") + cmd.Flags().StringVar(&connectorID, "connector-id", "", "") + _ = cmd.Flags().MarkDeprecated("connector-id", "use --connector") + // Both bind the same target; forbid passing both so one doesn't silently win. + cmd.MarkFlagsMutuallyExclusive("connector", "connector-id") cmd.Flags().StringVar(&name, "name", "", "Credential name (required)") - cmd.Flags().StringVar(&credentialType, "type", "", "Credential type (oauth, service_account, connector) (required)") - cmd.Flags().StringVar(&clientID, "client-id", "", "OAuth client ID") - cmd.Flags().StringVar(&clientSecret, "client-secret", "", "OAuth client secret") + // Valid v3 credential_type values are connector, serviceaccount, and + // adminconsent. This command builds credential_data only from + // --client-id/--client-secret, which is the `connector` override flow; + // serviceaccount/adminconsent need credential_data this command doesn't + // collect yet, so only `connector` is advertised. + // + // --client-id/--client-secret are the PROVIDER's OAuth app credentials (e.g. + // a Google Cloud / Azure app), NOT your Nylas application's — they populate + // credential_data so Nylas can broker auth through that provider app. + cmd.Flags().StringVar(&credentialType, "type", "", "Credential type: connector (required)") + cmd.Flags().StringVar(&clientID, "client-id", "", "Provider OAuth app client ID (e.g. your Google Cloud / Azure app), required") + cmd.Flags().StringVar(&clientSecret, "client-secret", "", "Provider OAuth app client secret, required") - _ = cmd.MarkFlagRequired("connector-id") _ = cmd.MarkFlagRequired("name") _ = cmd.MarkFlagRequired("type") + // The v3 create request requires credential_data; this command builds it from + // the client ID/secret, so both are required for the connector flow. + _ = cmd.MarkFlagRequired("client-id") + _ = cmd.MarkFlagRequired("client-secret") return cmd } func newCredentialUpdateCmd() *cobra.Command { - var name string + var ( + name string + connector string + ) cmd := &cobra.Command{ Use: "update ", Short: "Update a credential", - Long: "Update an existing credential.", + Long: "Update an existing credential. The connector provider is auto-detected\nwhen the application has exactly one connector.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { credentialID := args[0] _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { + connectorID, err := resolveConnectorID(ctx, client, connector) + if err != nil { + return struct{}{}, err + } + req := &domain.UpdateCredentialRequest{} if name != "" { req.Name = &name } - credential, err := client.UpdateCredential(ctx, credentialID, req) + credential, err := client.UpdateCredential(ctx, connectorID, credentialID, req) if err != nil { return struct{}{}, common.WrapUpdateError("credential", err) } @@ -192,29 +298,38 @@ func newCredentialUpdateCmd() *cobra.Command { } cmd.Flags().StringVar(&name, "name", "", "Credential name") + cmd.Flags().StringVar(&connector, "connector", "", "Connector provider (e.g. google); auto-detected if only one connector exists") return cmd } func newCredentialDeleteCmd() *cobra.Command { - var yes bool + var ( + yes bool + connector string + ) cmd := &cobra.Command{ Use: "delete ", Short: "Delete a credential", - Long: "Delete a credential permanently.", + Long: "Delete a credential permanently. The connector provider is auto-detected\nwhen the application has exactly one connector.", Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + credentialID := args[0] if !yes { - if !common.Confirm(fmt.Sprintf("Are you sure you want to delete credential %s?", args[0]), false) { + if !common.Confirm(fmt.Sprintf("Are you sure you want to delete credential %s?", credentialID), false) { fmt.Println("Cancelled.") return nil } } - credentialID := args[0] _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { - if err := client.DeleteCredential(ctx, credentialID); err != nil { + connectorID, err := resolveConnectorID(ctx, client, connector) + if err != nil { + return struct{}{}, err + } + + if err := client.DeleteCredential(ctx, connectorID, credentialID); err != nil { return struct{}{}, common.WrapDeleteError("credential", err) } @@ -227,6 +342,7 @@ func newCredentialDeleteCmd() *cobra.Command { } cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip confirmation prompt") + cmd.Flags().StringVar(&connector, "connector", "", "Connector provider (e.g. google); auto-detected if only one connector exists") return cmd } diff --git a/internal/cli/admin/credentials_test.go b/internal/cli/admin/credentials_test.go new file mode 100644 index 0000000..01f78fc --- /dev/null +++ b/internal/cli/admin/credentials_test.go @@ -0,0 +1,171 @@ +package admin + +import ( + "context" + "errors" + "testing" + + "github.com/nylas/cli/internal/domain" + "github.com/nylas/cli/internal/ports" + "github.com/spf13/cobra" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// fakeConnectorClient implements just enough of ports.NylasClient to exercise +// resolveConnectorID; the embedded interface panics if anything else is called. +type fakeConnectorClient struct { + ports.NylasClient + + connectors []domain.Connector + err error + calls int +} + +func (f *fakeConnectorClient) ListConnectors(ctx context.Context) ([]domain.Connector, error) { + f.calls++ + return f.connectors, f.err +} + +func TestResolveConnectorID(t *testing.T) { + t.Run("explicit provider is preserved", func(t *testing.T) { + client := &fakeConnectorClient{connectors: []domain.Connector{{ID: "connector-1", Provider: "microsoft"}}} + got, err := resolveConnectorID(context.Background(), client, "microsoft") + require.NoError(t, err) + assert.Equal(t, "microsoft", got) + assert.Equal(t, 1, client.calls) + }) + + t.Run("legacy connector ID maps to provider", func(t *testing.T) { + client := &fakeConnectorClient{connectors: []domain.Connector{{ID: "connector-1", Provider: "google"}}} + got, err := resolveConnectorID(context.Background(), client, "connector-1") + require.NoError(t, err) + assert.Equal(t, "google", got) + assert.Equal(t, 1, client.calls) + }) + + t.Run("explicit provider survives connector discovery failure", func(t *testing.T) { + client := &fakeConnectorClient{err: errors.New("boom")} + got, err := resolveConnectorID(context.Background(), client, "google") + require.NoError(t, err) + assert.Equal(t, "google", got) + }) + + t.Run("explicit deprecated provider is rejected", func(t *testing.T) { + // The explicit path must not leak a deprecated provider that auto-detect + // and listings hide; otherwise --connector inbox would hit the API. + client := &fakeConnectorClient{connectors: []domain.Connector{{Provider: "google"}}} + _, err := resolveConnectorID(context.Background(), client, "inbox") + require.Error(t, err) + assert.Contains(t, err.Error(), "no longer supported") + }) + + t.Run("sole connector is auto-detected by provider", func(t *testing.T) { + client := &fakeConnectorClient{connectors: []domain.Connector{{Provider: "google"}}} + got, err := resolveConnectorID(context.Background(), client, "") + require.NoError(t, err) + assert.Equal(t, "google", got) + }) + + t.Run("deprecated connectors are ignored when auto-detecting", func(t *testing.T) { + // A single visible connector alongside a hidden "inbox" connector must + // still auto-detect the visible one, not error as ambiguous. + client := &fakeConnectorClient{connectors: []domain.Connector{ + {Provider: "inbox"}, + {Provider: "google"}, + }} + got, err := resolveConnectorID(context.Background(), client, "") + require.NoError(t, err) + assert.Equal(t, "google", got) + }) + + t.Run("no connectors is a user error", func(t *testing.T) { + client := &fakeConnectorClient{connectors: nil} + _, err := resolveConnectorID(context.Background(), client, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "no connectors") + }) + + t.Run("multiple connectors require an explicit choice", func(t *testing.T) { + client := &fakeConnectorClient{connectors: []domain.Connector{ + {Provider: "google"}, + {Provider: "microsoft"}, + }} + _, err := resolveConnectorID(context.Background(), client, "") + require.Error(t, err) + assert.Contains(t, err.Error(), "google") + assert.Contains(t, err.Error(), "microsoft") + }) + + t.Run("ListConnectors failure is surfaced", func(t *testing.T) { + client := &fakeConnectorClient{err: errors.New("boom")} + _, err := resolveConnectorID(context.Background(), client, "") + require.Error(t, err) + }) + + t.Run("legacy ID during discovery failure surfaces the real error, not 'unknown provider'", func(t *testing.T) { + // A UUID-like legacy ID can't be validated while discovery is down; the + // user must see the discovery failure, not a misleading "unknown provider". + client := &fakeConnectorClient{err: errors.New("boom")} + _, err := resolveConnectorID(context.Background(), client, "conn-legacy-uuid") + require.Error(t, err) + assert.Contains(t, err.Error(), "boom") + assert.NotContains(t, err.Error(), "unknown connector provider") + }) +} + +func TestCredentialCommands_Structure(t *testing.T) { + // The connector is optional everywhere (auto-detected); the credential ID is + // the required positional for show/update/delete. + list := newCredentialListCmd() + assert.Equal(t, "list [connector]", list.Use) + + show := newCredentialShowCmd() + assert.Equal(t, "show ", show.Use) + assert.NotNil(t, show.Flags().Lookup("connector"), "show must expose --connector") + + update := newCredentialUpdateCmd() + assert.Equal(t, "update ", update.Use) + assert.NotNil(t, update.Flags().Lookup("connector")) + + del := newCredentialDeleteCmd() + assert.Equal(t, "delete ", del.Use) + assert.NotNil(t, del.Flags().Lookup("connector")) + + create := newCredentialCreateCmd() + assert.NotNil(t, create.Flags().Lookup("connector"), "create must expose --connector") + // --connector-id is kept as a deprecated alias for backward compatibility. + legacy := create.Flags().Lookup("connector-id") + require.NotNil(t, legacy) + assert.NotEmpty(t, legacy.Deprecated, "--connector-id must be marked deprecated") + + // The v3 create request requires credential_data, which the command builds + // from the client ID/secret, so both must be required flags. + for _, name := range []string{"name", "type", "client-id", "client-secret"} { + f := create.Flags().Lookup(name) + require.NotNil(t, f, "create must expose --%s", name) + assert.Contains(t, f.Annotations[cobra.BashCompOneRequiredFlag], "true", "--%s must be required", name) + } +} + +func TestCredentialCreateRejectsUnsupportedType(t *testing.T) { + // The v3 API accepts connector/serviceaccount/adminconsent, but this + // command only builds connector-shaped credential_data — any other --type + // must fail fast with a clear message instead of an opaque provider 400. + // "" covers --type "" — cobra's required-flag check is satisfied by an + // explicitly set empty value, so the guard must reject it too. + for _, unsupported := range []string{"oauth", "serviceaccount", "adminconsent", "bogus", ""} { + t.Run(unsupported, func(t *testing.T) { + cmd := newCredentialCreateCmd() + require.NoError(t, cmd.Flags().Set("name", "test")) + require.NoError(t, cmd.Flags().Set("type", unsupported)) + require.NoError(t, cmd.Flags().Set("client-id", "cid")) + require.NoError(t, cmd.Flags().Set("client-secret", "secret")) + + err := cmd.RunE(cmd, nil) + require.Error(t, err) + // Validation must fire before any client/API work. + assert.Contains(t, err.Error(), "invalid type") + }) + } +} diff --git a/internal/cli/calendar/ai_schedule.go b/internal/cli/calendar/ai_schedule.go index 4453e7e..44e5f83 100644 --- a/internal/cli/calendar/ai_schedule.go +++ b/internal/cli/calendar/ai_schedule.go @@ -22,6 +22,7 @@ func newAIScheduleCmd() *cobra.Command { privacyMode bool autoConfirm bool userTimezone string + grantFlag string ) cmd := &cobra.Command{ @@ -89,7 +90,10 @@ Examples: // Create AI router router := ai.NewRouter(cfg.AI) - _, err = common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + // Resolve the grant from --grant (or the stored default). The + // positional args are the natural-language query, not a grant ID, + // so they must not be passed to grant resolution. + _, err = common.WithClient(scheduleGrantArgs(grantFlag), func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { // Create AI scheduler scheduler := ai.NewAIScheduler(router, client, selectedProvider) @@ -123,21 +127,15 @@ Examples: fmt.Println("\n🔒 Privacy: All processing done locally, no data sent to cloud.") } - // Handle confirmation + // Handle confirmation. With --yes the first option is created + // without prompting; otherwise the user picks. + choice := "" if !autoConfirm && len(response.Options) > 0 { fmt.Print("\nCreate meeting with option #1? [y/N/2/3]: ") - var choice string - _, _ = fmt.Scanln(&choice) // User input, validation handled below - - choice = strings.ToLower(strings.TrimSpace(choice)) - if choice == "y" || choice == "yes" { - // Create with first option - return struct{}{}, createMeetingFromOption(cmd, response.Options[0], grantID, client) - } else if choice == "2" && len(response.Options) > 1 { - return struct{}{}, createMeetingFromOption(cmd, response.Options[1], grantID, client) - } else if choice == "3" && len(response.Options) > 2 { - return struct{}{}, createMeetingFromOption(cmd, response.Options[2], grantID, client) - } + _, _ = fmt.Scanln(&choice) // User input, validated by selectScheduleOption + } + if idx := selectScheduleOption(autoConfirm, len(response.Options), choice); idx >= 0 { + return struct{}{}, createMeetingFromOption(cmd, response.Options[idx], grantID, client) } return struct{}{}, nil @@ -151,10 +149,47 @@ Examples: cmd.Flags().BoolVar(&privacyMode, "privacy", false, "Use local LLM (Ollama) for privacy") cmd.Flags().BoolVar(&autoConfirm, "yes", false, "Automatically create the first suggested option") cmd.Flags().StringVar(&userTimezone, "timezone", "", "Your timezone (auto-detected if not specified)") + cmd.Flags().StringVarP(&grantFlag, "grant", "g", "", "Grant ID (uses default if not specified)") return cmd } +// scheduleGrantArgs builds the grant-resolution args for the AI scheduler. +// The positional args hold the natural-language query, so the grant comes only +// from --grant; an empty result makes common.WithClient fall back to the +// stored default grant. +func scheduleGrantArgs(grantFlag string) []string { + if grantFlag != "" { + return []string{grantFlag} + } + return nil +} + +// selectScheduleOption returns the index of the option to create, or -1 for +// none. With autoConfirm (--yes) the first option is always chosen; otherwise +// the user's choice ("y"/"yes"/"2"/"3") selects an option when it exists. +func selectScheduleOption(autoConfirm bool, optionCount int, choice string) int { + if optionCount == 0 { + return -1 + } + if autoConfirm { + return 0 + } + switch strings.ToLower(strings.TrimSpace(choice)) { + case "y", "yes": + return 0 + case "2": + if optionCount > 1 { + return 1 + } + case "3": + if optionCount > 2 { + return 2 + } + } + return -1 +} + // displayScheduleOptions displays the AI-suggested meeting options. func displayScheduleOptions(response *ai.ScheduleResponse, _ string) error { if len(response.Options) == 0 { diff --git a/internal/cli/calendar/ai_schedule_test.go b/internal/cli/calendar/ai_schedule_test.go index fc0cee4..6cc1f59 100644 --- a/internal/cli/calendar/ai_schedule_test.go +++ b/internal/cli/calendar/ai_schedule_test.go @@ -59,3 +59,53 @@ func TestCreateMeetingFromOptionCreatesRealEvent(t *testing.T) { t.Fatalf("participants = %d, want 2", len(created.Participants)) } } + +func TestScheduleGrantArgs(t *testing.T) { + t.Parallel() + + // The NL query must never be used as the grant. With no --grant the + // result is empty (so WithClient uses the default grant); with --grant the + // flag value is the only grant arg. + if got := scheduleGrantArgs(""); got != nil { + t.Fatalf("scheduleGrantArgs(\"\") = %v, want nil", got) + } + got := scheduleGrantArgs("grant-abc") + if len(got) != 1 || got[0] != "grant-abc" { + t.Fatalf("scheduleGrantArgs(\"grant-abc\") = %v, want [grant-abc]", got) + } +} + +func TestSelectScheduleOption(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + autoConfirm bool + optionCount int + choice string + want int + }{ + // --yes must create the first option, not silently skip creation. + {name: "autoConfirm picks first", autoConfirm: true, optionCount: 3, want: 0}, + {name: "autoConfirm with no options", autoConfirm: true, optionCount: 0, want: -1}, + {name: "no options never creates", optionCount: 0, choice: "y", want: -1}, + {name: "yes picks first", optionCount: 3, choice: "y", want: 0}, + {name: "YES uppercase picks first", optionCount: 3, choice: "YES", want: 0}, + {name: "empty declines", optionCount: 3, choice: "", want: -1}, + {name: "n declines", optionCount: 3, choice: "n", want: -1}, + {name: "2 picks second", optionCount: 3, choice: "2", want: 1}, + {name: "3 picks third", optionCount: 3, choice: "3", want: 2}, + {name: "2 out of range declines", optionCount: 1, choice: "2", want: -1}, + {name: "3 out of range declines", optionCount: 2, choice: "3", want: -1}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := selectScheduleOption(tt.autoConfirm, tt.optionCount, tt.choice); got != tt.want { + t.Fatalf("selectScheduleOption(%v, %d, %q) = %d, want %d", + tt.autoConfirm, tt.optionCount, tt.choice, got, tt.want) + } + }) + } +} diff --git a/internal/cli/calendar/recurring.go b/internal/cli/calendar/recurring.go index 5080ca8..214072a 100644 --- a/internal/cli/calendar/recurring.go +++ b/internal/cli/calendar/recurring.go @@ -32,6 +32,26 @@ API reference: https://developer.nylas.com/docs/v3/calendar/recurring-events/`, return cmd } +// recurringGrantArgs resolves the grant-resolution args for recurring commands. +// The positional [grant-id] and --grant flag are both accepted; two different +// grants at once is ambiguous and rejected rather than silently picking one +// (the wrong pick would mutate another account's events). +func recurringGrantArgs(positional []string, grantFlag string) ([]string, error) { + if len(positional) > 0 && grantFlag != "" && positional[0] != grantFlag { + // Compared as raw strings: an email alias and its grant ID for the + // same account read as a conflict too — erring loud beats guessing + // and mutating another account's events. + return nil, common.NewUserError( + fmt.Sprintf("conflicting grants: positional %q and --grant %q", positional[0], grantFlag), + "Pass the grant either as the positional argument or via --grant, not both (if they identify the same account, drop one)", + ) + } + if len(positional) == 0 && grantFlag != "" { + return []string{grantFlag}, nil + } + return positional, nil +} + // newRecurringListCmd creates the list recurring event instances command. func newRecurringListCmd() *cobra.Command { var ( @@ -58,7 +78,10 @@ The master event ID is the ID of the parent recurring event.`, Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { masterEventID := args[0] - grantArgs := args[1:] + grantArgs, gerr := recurringGrantArgs(args[1:], grantID) + if gerr != nil { + return gerr + } if calendarID == "" { return common.NewUserError("calendar ID is required", "Use --calendar to specify the calendar") @@ -141,7 +164,10 @@ This creates an exception for that particular instance.`, Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { instanceID := args[0] - grantArgs := args[1:] + grantArgs, gerr := recurringGrantArgs(args[1:], grantID) + if gerr != nil { + return gerr + } if calendarID == "" { return common.NewUserError("calendar ID is required", "Use --calendar to specify the calendar") @@ -237,7 +263,10 @@ This adds an exception to the recurrence rule.`, Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { instanceID := args[0] - grantArgs := args[1:] + grantArgs, gerr := recurringGrantArgs(args[1:], grantID) + if gerr != nil { + return gerr + } if calendarID == "" { return common.NewUserError("calendar ID is required", "Use --calendar to specify the calendar") diff --git a/internal/cli/calendar/recurring_test.go b/internal/cli/calendar/recurring_test.go new file mode 100644 index 0000000..0f5697c --- /dev/null +++ b/internal/cli/calendar/recurring_test.go @@ -0,0 +1,40 @@ +package calendar + +import ( + "reflect" + "testing" +) + +func TestRecurringGrantArgs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + positional []string + grantFlag string + want []string + wantErr bool + }{ + // --grant must be honored when no positional grant is given; two + // DIFFERENT grants at once is ambiguous and must be rejected rather + // than silently picking one (wrong pick = mutating another account). + {name: "flag used when no positional", positional: nil, grantFlag: "grant-flag", want: []string{"grant-flag"}}, + {name: "no positional, no flag falls back to default", positional: nil, grantFlag: "", want: nil}, + {name: "positional used with no flag", positional: []string{"grant-pos"}, grantFlag: "", want: []string{"grant-pos"}}, + {name: "same grant in both is accepted", positional: []string{"grant-1"}, grantFlag: "grant-1", want: []string{"grant-1"}}, + {name: "conflicting grants are rejected", positional: []string{"grant-pos"}, grantFlag: "grant-flag", wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + got, err := recurringGrantArgs(tt.positional, tt.grantFlag) + if (err != nil) != tt.wantErr { + t.Fatalf("recurringGrantArgs(%v, %q) error = %v, wantErr %v", tt.positional, tt.grantFlag, err, tt.wantErr) + } + if !tt.wantErr && !reflect.DeepEqual(got, tt.want) { + t.Fatalf("recurringGrantArgs(%v, %q) = %v, want %v", tt.positional, tt.grantFlag, got, tt.want) + } + }) + } +} diff --git a/internal/cli/common/connectors.go b/internal/cli/common/connectors.go index a0d2333..9b19efb 100644 --- a/internal/cli/common/connectors.go +++ b/internal/cli/common/connectors.go @@ -1,41 +1,26 @@ package common import ( - "strings" - "github.com/nylas/cli/internal/domain" ) -const deprecatedConnectorProviderInbox = "inbox" - // IsDeprecatedConnectorProvider reports whether the CLI should hide or reject a -// connector provider that is no longer supported. +// connector provider that is no longer supported. The provider-deprecation +// knowledge lives in the domain layer so adapters can share it. func IsDeprecatedConnectorProvider(provider string) bool { - return strings.EqualFold(strings.TrimSpace(provider), deprecatedConnectorProviderInbox) + return domain.IsDeprecatedConnectorProvider(provider) } // FilterVisibleConnectors removes deprecated connector providers from CLI-facing // listings while leaving the backend API surface unchanged. func FilterVisibleConnectors(connectors []domain.Connector) []domain.Connector { - if len(connectors) == 0 { - return connectors - } - - filtered := make([]domain.Connector, 0, len(connectors)) - for _, connector := range connectors { - if IsDeprecatedConnectorProvider(connector.Provider) { - continue - } - filtered = append(filtered, connector) - } - - return filtered + return domain.FilterVisibleConnectors(connectors) } // ValidateSupportedConnectorProvider rejects connector providers that are no // longer supported by the CLI. func ValidateSupportedConnectorProvider(provider string) error { - if !IsDeprecatedConnectorProvider(provider) { + if !domain.IsDeprecatedConnectorProvider(provider) { return nil } diff --git a/internal/cli/common/format.go b/internal/cli/common/format.go index 0f9ba17..61c38ba 100644 --- a/internal/cli/common/format.go +++ b/internal/cli/common/format.go @@ -150,6 +150,15 @@ func PrintWarning(format string, args ...any) { _, _ = Yellow.Printf("⚠ "+format+"\n", args...) } +// PrintWarningStderr prints a warning message to stderr, keeping stdout clean +// for structured output (e.g. --json). +func PrintWarningStderr(format string, args ...any) { + if IsQuiet() { + return + } + _, _ = Yellow.Fprintf(os.Stderr, "⚠ "+format+"\n", args...) +} + // PrintInfo prints an info message. func PrintInfo(format string, args ...any) { if IsQuiet() { diff --git a/internal/cli/demo/scheduler_main.go b/internal/cli/demo/scheduler_main.go index 4c40150..d21c839 100644 --- a/internal/cli/demo/scheduler_main.go +++ b/internal/cli/demo/scheduler_main.go @@ -59,7 +59,7 @@ func newDemoConfigListCmd() *cobra.Command { client := nylas.NewDemoClient() ctx := context.Background() - configs, err := client.ListSchedulerConfigurations(ctx) + configs, err := client.ListSchedulerConfigurations(ctx, "demo-grant") if err != nil { return common.WrapListError("scheduler configs", err) } diff --git a/internal/cli/email/drafts.go b/internal/cli/email/drafts.go index 0937cd2..b5b2def 100644 --- a/internal/cli/email/drafts.go +++ b/internal/cli/email/drafts.go @@ -139,16 +139,7 @@ func newDraftsCreateCmd() *cobra.Command { subject = strings.TrimSpace(subject) fmt.Println("Body (end with a line containing only '.'):") - var bodyLines []string - for { - line, _ := reader.ReadString('\n') - line = strings.TrimSuffix(line, "\n") - if line == "." { - break - } - bodyLines = append(bodyLines, line) - } - body = strings.Join(bodyLines, "\n") + body = readBodyLines(reader) } _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { diff --git a/internal/cli/email/reply.go b/internal/cli/email/reply.go index 8b54863..7e00263 100644 --- a/internal/cli/email/reply.go +++ b/internal/cli/email/reply.go @@ -230,7 +230,13 @@ func promptReplyBody() string { // readReplyBody reads a multi-line body terminated by a line containing only // "." or by EOF, so a closed/piped stdin cannot loop forever. func readReplyBody(r io.Reader) string { - reader := bufio.NewReader(r) + return readBodyLines(bufio.NewReader(r)) +} + +// readBodyLines reads a multi-line body from reader, terminated by a line +// containing only "." or by EOF, so a closed/piped stdin cannot loop forever. +// It returns the collected lines joined by "\n". +func readBodyLines(reader *bufio.Reader) string { var lines []string for { line, err := reader.ReadString('\n') diff --git a/internal/cli/email/send.go b/internal/cli/email/send.go index b724626..07464f6 100644 --- a/internal/cli/email/send.go +++ b/internal/cli/email/send.go @@ -147,16 +147,7 @@ Supports hosted templates: if templateOpts.TemplateID == "" && body == "" { fmt.Println("Body (end with a line containing only '.'):") - var bodyLines []string - for { - line, _ := reader.ReadString('\n') - line = strings.TrimSuffix(line, "\n") - if line == "." { - break - } - bodyLines = append(bodyLines, line) - } - body = strings.Join(bodyLines, "\n") + body = readBodyLines(reader) } } diff --git a/internal/cli/email/templates_create.go b/internal/cli/email/templates_create.go index 2c49f60..a58501a 100644 --- a/internal/cli/email/templates_create.go +++ b/internal/cli/email/templates_create.go @@ -63,16 +63,7 @@ Use --interactive for a guided creation experience.`, if body == "" { fmt.Println("Body (end with a line containing only '.'):") - var bodyLines []string - for { - line, _ := reader.ReadString('\n') - line = strings.TrimSuffix(line, "\n") - if line == "." { - break - } - bodyLines = append(bodyLines, line) - } - body = strings.Join(bodyLines, "\n") + body = readBodyLines(reader) } if category == "" { diff --git a/internal/cli/integration/admin_test.go b/internal/cli/integration/admin_test.go index 2a24fac..057ddfd 100644 --- a/internal/cli/integration/admin_test.go +++ b/internal/cli/integration/admin_test.go @@ -203,7 +203,7 @@ func TestCLI_AdminCredentialsList(t *testing.T) { t.Skip("No valid connector provider found") } - stdout, stderr, err := runCLI("admin", "credentials", "list", "--connector-id", connectorProvider) + stdout, stderr, err := runCLI("admin", "credentials", "list", connectorProvider) skipIfProviderNotSupported(t, stderr) // Skip if credentials endpoint isn't available (404 - endpoint may not exist for all providers) @@ -246,7 +246,7 @@ func TestCLI_AdminCredentialsListJSON(t *testing.T) { t.Skip("No valid connector provider found") } - stdout, stderr, err := runCLI("admin", "credentials", "list", "--connector-id", connectorProvider, "--json") + stdout, stderr, err := runCLI("admin", "credentials", "list", connectorProvider, "--json") skipIfProviderNotSupported(t, stderr) // Skip if credentials endpoint isn't available (404 - endpoint may not exist for all providers) diff --git a/internal/cli/integration/group_events_test.go b/internal/cli/integration/group_events_test.go index b55fa5b..0c235e7 100644 --- a/internal/cli/integration/group_events_test.go +++ b/internal/cli/integration/group_events_test.go @@ -46,7 +46,7 @@ func TestGroupEvents_Integration(t *testing.T) { defer cancel() acquireRateLimit(t) - configs, err := client.ListSchedulerConfigurations(ctx) + configs, err := client.ListSchedulerConfigurations(ctx, testGrantID) if err != nil { if isUnavailableErr(err) { t.Skipf("scheduler not available for this account: %v", err) diff --git a/internal/cli/integration/scheduler_basic_test.go b/internal/cli/integration/scheduler_basic_test.go index 6d2c2c4..a5f952f 100644 --- a/internal/cli/integration/scheduler_basic_test.go +++ b/internal/cli/integration/scheduler_basic_test.go @@ -152,6 +152,11 @@ func TestCLI_SchedulerBookingsShowHelp(t *testing.T) { if !strings.Contains(stdout, "--json") { t.Errorf("Expected '--json' flag in help, got: %s", stdout) } + // Booking endpoints authenticate with a session token minted from the + // configuration, so the flag must be surfaced in help. + if !strings.Contains(stdout, "--configuration-id") { + t.Errorf("Expected '--configuration-id' flag in help, got: %s", stdout) + } t.Logf("scheduler bookings show --help output:\n%s", stdout) } @@ -171,6 +176,13 @@ func TestCLI_SchedulerBookingsConfirmHelp(t *testing.T) { if !strings.Contains(stdout, "booking-id") && !strings.Contains(stdout, "") { t.Errorf("Expected booking-id in help, got: %s", stdout) } + if !strings.Contains(stdout, "--configuration-id") { + t.Errorf("Expected '--configuration-id' flag in help, got: %s", stdout) + } + // The salt from the booking reference is required by the v3 confirm spec. + if !strings.Contains(stdout, "--salt") { + t.Errorf("Expected '--salt' flag in help, got: %s", stdout) + } t.Logf("scheduler bookings confirm --help output:\n%s", stdout) } @@ -190,6 +202,9 @@ func TestCLI_SchedulerBookingsRescheduleHelp(t *testing.T) { if !strings.Contains(stdout, "booking-id") && !strings.Contains(stdout, "") { t.Errorf("Expected booking-id in help, got: %s", stdout) } + if !strings.Contains(stdout, "--configuration-id") { + t.Errorf("Expected '--configuration-id' flag in help, got: %s", stdout) + } t.Logf("scheduler bookings reschedule --help output:\n%s", stdout) } @@ -217,6 +232,9 @@ func TestCLI_SchedulerBookingsCancelHelp(t *testing.T) { if !strings.Contains(stdout, "--reason") { t.Errorf("Expected '--reason' flag in help, got: %s", stdout) } + if !strings.Contains(stdout, "--configuration-id") { + t.Errorf("Expected '--configuration-id' flag in help, got: %s", stdout) + } t.Logf("scheduler bookings cancel --help output:\n%s", stdout) } @@ -237,10 +255,10 @@ func TestCLI_SchedulerBookingsLifecycle(t *testing.T) { "Manual testing:\n" + " (1) Create a scheduler configuration via Dashboard\n" + " (2) Create a booking via the public booking page or sessions API\n" + - " (3) Get booking ID from 'scheduler bookings list'\n" + - " (4) Test show command: nylas scheduler bookings show \n" + - " (5) Test show JSON: nylas scheduler bookings show --json\n" + - " (6) Test confirm (if pending): nylas scheduler bookings confirm \n" + - " (7) Test reschedule: nylas scheduler bookings reschedule \n" + - " (8) Test cancel: nylas scheduler bookings cancel --yes --reason 'Testing'\n") + " (3) Get the booking ID from a Scheduler webhook or the confirmation link\n" + + " (4) Test show command: nylas scheduler bookings show --configuration-id \n" + + " (5) Test show JSON: nylas scheduler bookings show --configuration-id --json\n" + + " (6) Test confirm (if pending): nylas scheduler bookings confirm --configuration-id --salt \n" + + " (7) Test reschedule: nylas scheduler bookings reschedule --configuration-id \n" + + " (8) Test cancel: nylas scheduler bookings cancel --configuration-id --yes --reason 'Testing'\n") } diff --git a/internal/cli/scheduler/bookings.go b/internal/cli/scheduler/bookings.go index 8228bc0..1ec777c 100644 --- a/internal/cli/scheduler/bookings.go +++ b/internal/cli/scheduler/bookings.go @@ -3,6 +3,7 @@ package scheduler import ( "context" "encoding/json" + "errors" "fmt" "time" @@ -31,15 +32,19 @@ API reference: https://developer.nylas.com/docs/reference/api/bookings/`, } func newBookingShowCmd() *cobra.Command { + var configurationID string cmd := &cobra.Command{ Use: "show ", Short: "Show booking details", - Long: "Show detailed information about a specific booking.", - Args: cobra.ExactArgs(1), + Long: `Show detailed information about a specific booking. + +Booking endpoints are authorized by a Scheduler session token minted from the +configuration, so --configuration-id is required.`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { bookingID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - booking, err := client.GetBooking(ctx, bookingID) + _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { + booking, err := client.GetBooking(ctx, configurationID, bookingID) if err != nil { return struct{}{}, common.WrapGetError("booking", err) } @@ -83,28 +88,55 @@ func newBookingShowCmd() *cobra.Command { }, } + addConfigurationIDFlag(cmd, &configurationID) + return cmd } +// addConfigurationIDFlag registers the required --configuration-id flag shared by +// all booking commands. Booking endpoints authenticate with a Scheduler session +// token minted from the configuration, so the configuration ID is mandatory. +func addConfigurationIDFlag(cmd *cobra.Command, target *string) { + cmd.Flags().StringVar(target, "configuration-id", "", "Scheduler configuration ID that owns the booking (required)") + _ = cmd.MarkFlagRequired("configuration-id") +} + func newBookingConfirmCmd() *cobra.Command { var ( - reason string + configurationID string + salt string + reason string // deprecated: the v3 confirm payload has no reason field ) cmd := &cobra.Command{ Use: "confirm ", Short: "Confirm a booking", - Long: "Confirm a pending booking.", - Args: cobra.ExactArgs(1), + Long: `Confirm a pending booking. + +Confirming a booking requires the --salt from that booking's reference. Nylas +does not expose the salt through any read API, so it cannot be looked up from +the booking ID alone; you must take it from the booking reference, which appears +in the organizer confirmation link, the cancel/reschedule page URL, or a +Scheduler webhook payload.`, + Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { bookingID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + if salt == "" { + return common.NewUserErrorWithSuggestions( + "the --salt is required to confirm a booking", + "Find it in the booking reference (it is not retrievable from the booking ID).", + "The reference appears in the organizer confirmation link and the cancel/reschedule page URL.", + "You can also read it from a Scheduler webhook payload.", + "Then pass it: nylas scheduler bookings confirm --salt ", + ) + } + _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { req := &domain.ConfirmBookingRequest{ + Salt: salt, Status: "confirmed", - Reason: reason, } - booking, err := client.ConfirmBooking(ctx, bookingID, req) + booking, err := client.ConfirmBooking(ctx, configurationID, bookingID, req) if err != nil { return struct{}{}, common.WrapUpdateError("booking", err) } @@ -122,17 +154,24 @@ func newBookingConfirmCmd() *cobra.Command { }, } - cmd.Flags().StringVar(&reason, "reason", "", "Reason for confirmation") + addConfigurationIDFlag(cmd, &configurationID) + cmd.Flags().StringVar(&salt, "salt", "", "Salt from the booking reference, required (found in the confirmation link, cancel/reschedule URL, or a Scheduler webhook)") + // --reason was removed from the v3 confirm payload; keep it as a deprecated + // no-op so existing scripts degrade gracefully instead of hitting cobra's + // "unknown flag" error on upgrade. + cmd.Flags().StringVar(&reason, "reason", "", "") + _ = cmd.Flags().MarkDeprecated("reason", "confirm no longer takes a reason; the flag is ignored") return cmd } func newBookingRescheduleCmd() *cobra.Command { var ( - startTime int64 - endTime int64 - timezone string - reason string + configurationID string + startTime int64 + endTime int64 + timezone string + reason string ) cmd := &cobra.Command{ @@ -141,11 +180,8 @@ func newBookingRescheduleCmd() *cobra.Command { Long: `Reschedule an existing booking to a new time. You must provide the new start and end times as Unix timestamps.`, - Example: ` # Reschedule to a new time - nylas scheduler bookings reschedule abc123 --start-time 1704067200 --end-time 1704070800 - - # Reschedule with timezone - nylas scheduler bookings reschedule abc123 --start-time 1704067200 --end-time 1704070800 --timezone "America/New_York"`, + Example: ` # Reschedule to a new time (Unix timestamps) + nylas scheduler bookings reschedule abc123 --configuration-id cfg123 --start-time 1704067200 --end-time 1704070800`, Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { if startTime == 0 || endTime == 0 { @@ -157,21 +193,22 @@ You must provide the new start and end times as Unix timestamps.`, } bookingID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { req := &domain.RescheduleBookingRequest{ StartTime: startTime, EndTime: endTime, - Timezone: timezone, - Reason: reason, } - booking, err := client.RescheduleBooking(ctx, bookingID, req) + booking, warning, err := resolveRescheduleResult(client.RescheduleBooking(ctx, configurationID, bookingID, req)) if err != nil { return struct{}{}, common.WrapUpdateError("booking", err) } + if warning != "" { + common.PrintWarningStderr("%s", warning) + } if common.IsJSON(cmd) { - return struct{}{}, common.PrintJSON(booking) + return struct{}{}, common.PrintJSON(rescheduleJSONPayload(booking, warning)) } _, _ = common.Green.Printf("✓ Rescheduled booking: %s\n", booking.BookingID) @@ -184,18 +221,52 @@ You must provide the new start and end times as Unix timestamps.`, }, } + addConfigurationIDFlag(cmd, &configurationID) cmd.Flags().Int64Var(&startTime, "start-time", 0, "New start time (Unix timestamp, required)") cmd.Flags().Int64Var(&endTime, "end-time", 0, "New end time (Unix timestamp, required)") - cmd.Flags().StringVar(&timezone, "timezone", "", "Timezone for the booking (e.g., America/New_York)") - cmd.Flags().StringVar(&reason, "reason", "", "Reason for rescheduling") + // The v3 reschedule payload (booking_update) has only start/end times. + // --timezone and --reason are kept as deprecated no-ops so existing scripts + // don't break on upgrade. + cmd.Flags().StringVar(&timezone, "timezone", "", "") + cmd.Flags().StringVar(&reason, "reason", "", "") + _ = cmd.Flags().MarkDeprecated("timezone", "reschedule does not accept a timezone; the flag is ignored") + _ = cmd.Flags().MarkDeprecated("reason", "reschedule does not accept a reason; the flag is ignored") return cmd } +// rescheduleJSONPayload mirrors the RPC result shape so --json consumers can +// distinguish a verified reschedule from an applied-but-unverified one: the +// booking gains a "warning" field on the partial-success path. +func rescheduleJSONPayload(booking *domain.Booking, warning string) any { + if warning == "" { + return booking + } + return struct { + domain.Booking + Warning string `json:"warning"` + }{Booking: *booking, Warning: warning} +} + +// resolveRescheduleResult maps the port's typed partial success onto the CLI +// outcome: on domain.ErrBookingReadBackFailed the reschedule was applied, so +// the partial booking is reported as success with a warning instead of failing +// an operation that took effect. +func resolveRescheduleResult(booking *domain.Booking, err error) (*domain.Booking, string, error) { + if err == nil { + return booking, "", nil + } + if errors.Is(err, domain.ErrBookingReadBackFailed) && booking != nil { + return booking, fmt.Sprintf("%v; the booking's current server-side record is unverified", err), nil + } + return nil, "", err +} + func newBookingCancelCmd() *cobra.Command { var ( - reason string - yes bool + configurationID string + reason string + yes bool ) cmd := &cobra.Command{ @@ -212,8 +283,8 @@ func newBookingCancelCmd() *cobra.Command { } bookingID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - if err := client.CancelBooking(ctx, bookingID, reason); err != nil { + _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { + if err := client.CancelBooking(ctx, configurationID, bookingID, reason); err != nil { return struct{}{}, common.WrapCancelError("booking", err) } @@ -225,6 +296,7 @@ func newBookingCancelCmd() *cobra.Command { }, } + addConfigurationIDFlag(cmd, &configurationID) cmd.Flags().StringVar(&reason, "reason", "", "Cancellation reason") cmd.Flags().BoolVarP(&yes, "yes", "y", false, "Skip confirmation prompt") diff --git a/internal/cli/scheduler/configurations.go b/internal/cli/scheduler/configurations.go index af4d470..2024cd8 100644 --- a/internal/cli/scheduler/configurations.go +++ b/internal/cli/scheduler/configurations.go @@ -11,6 +11,12 @@ import ( "github.com/spf13/cobra" ) +// Scheduler configuration endpoints are grant-scoped +// (/v3/grants/{grant_id}/scheduling/configurations), so these commands resolve a +// grant via WithClient. The grant is taken from an optional trailing [grant-id] +// positional (or the default grant); leading positionals like +// are sliced off before grant resolution so they are never mistaken +// for a grant. func newConfigurationsCmd() *cobra.Command { cmd := &cobra.Command{ Use: "configurations", @@ -32,13 +38,14 @@ API reference: https://developer.nylas.com/docs/reference/api/configurations/`, func newConfigListCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "list", + Use: "list [grant-id]", Aliases: []string{"ls"}, Short: "List scheduler configurations", Long: "List all scheduler configurations (meeting types).", + Args: cobra.MaximumNArgs(1), RunE: func(cmd *cobra.Command, args []string) error { _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - configs, err := client.ListSchedulerConfigurations(ctx) + configs, err := client.ListSchedulerConfigurations(ctx, grantID) if err != nil { return struct{}{}, common.WrapListError("configurations", err) } @@ -72,14 +79,14 @@ func newConfigListCmd() *cobra.Command { func newConfigShowCmd() *cobra.Command { cmd := &cobra.Command{ - Use: "show ", + Use: "show [grant-id]", Short: "Show scheduler configuration details", Long: "Show detailed information about a specific scheduler configuration.", - Args: cobra.ExactArgs(1), + Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { configID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - config, err := client.GetSchedulerConfiguration(ctx, configID) + _, err := common.WithClient(args[1:], func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + config, err := client.GetSchedulerConfiguration(ctx, grantID, configID) if err != nil { return struct{}{}, common.WrapGetError("configuration", err) } @@ -111,7 +118,8 @@ func newConfigCreateCmd() *cobra.Command { flags := &configFlags{} cmd := &cobra.Command{ - Use: "create", + Use: "create [grant-id]", + Args: cobra.MaximumNArgs(1), Short: "Create a scheduler configuration", Long: `Create a new scheduler configuration (meeting type). @@ -158,7 +166,7 @@ When both are provided, flags override file values.`, } _, err = common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - config, err := client.CreateSchedulerConfiguration(ctx, req) + config, err := client.CreateSchedulerConfiguration(ctx, grantID, req) if err != nil { return struct{}{}, common.WrapCreateError("configuration", err) } @@ -201,7 +209,7 @@ func newConfigUpdateCmd() *cobra.Command { flags := &configFlags{} cmd := &cobra.Command{ - Use: "update ", + Use: "update [grant-id]", Short: "Update a scheduler configuration", Long: `Update an existing scheduler configuration. @@ -218,7 +226,7 @@ When both are provided, flags override file values.`, # File as base, override specific values nylas scheduler configs update abc123 --file update.json --duration 45`, - Args: cobra.ExactArgs(1), + Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { if err := validateConfigFlags(flags); err != nil { return err @@ -233,8 +241,8 @@ When both are provided, flags override file values.`, return err } - _, err = common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - config, err := client.UpdateSchedulerConfiguration(ctx, configID, req) + _, err = common.WithClient(args[1:], func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + config, err := client.UpdateSchedulerConfiguration(ctx, grantID, configID, req) if err != nil { return struct{}{}, common.WrapUpdateError("configuration", err) } @@ -265,10 +273,10 @@ func newConfigDeleteCmd() *cobra.Command { var yes bool cmd := &cobra.Command{ - Use: "delete ", + Use: "delete [grant-id]", Short: "Delete a scheduler configuration", Long: "Delete a scheduler configuration permanently.", - Args: cobra.ExactArgs(1), + Args: cobra.RangeArgs(1, 2), RunE: func(cmd *cobra.Command, args []string) error { if !yes { if !common.Confirm(fmt.Sprintf("Are you sure you want to delete configuration %s?", args[0]), false) { @@ -278,8 +286,8 @@ func newConfigDeleteCmd() *cobra.Command { } configID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { - if err := client.DeleteSchedulerConfiguration(ctx, configID); err != nil { + _, err := common.WithClient(args[1:], func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + if err := client.DeleteSchedulerConfiguration(ctx, grantID, configID); err != nil { return struct{}{}, common.WrapDeleteError("configuration", err) } diff --git a/internal/cli/scheduler/scheduler_test.go b/internal/cli/scheduler/scheduler_test.go index f5862e7..6ad2d9a 100644 --- a/internal/cli/scheduler/scheduler_test.go +++ b/internal/cli/scheduler/scheduler_test.go @@ -1,9 +1,16 @@ package scheduler import ( + "encoding/json" + "errors" + "fmt" "testing" + "github.com/nylas/cli/internal/cli/testutil" + "github.com/nylas/cli/internal/domain" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewSchedulerCmd(t *testing.T) { @@ -75,7 +82,7 @@ func TestConfigListCmd(t *testing.T) { cmd := newConfigListCmd() t.Run("command_name", func(t *testing.T) { - assert.Equal(t, "list", cmd.Use) + assert.Equal(t, "list [grant-id]", cmd.Use) }) t.Run("has_ls_alias", func(t *testing.T) { @@ -88,7 +95,7 @@ func TestConfigShowCmd(t *testing.T) { cmd := newConfigShowCmd() t.Run("command_name", func(t *testing.T) { - assert.Equal(t, "show ", cmd.Use) + assert.Equal(t, "show [grant-id]", cmd.Use) }) } @@ -97,7 +104,7 @@ func TestConfigCreateCmd(t *testing.T) { cmd := newConfigCreateCmd() t.Run("command_name", func(t *testing.T) { - assert.Equal(t, "create", cmd.Use) + assert.Equal(t, "create [grant-id]", cmd.Use) }) t.Run("has_base_flags", func(t *testing.T) { @@ -151,7 +158,7 @@ func TestConfigUpdateCmd(t *testing.T) { cmd := newConfigUpdateCmd() t.Run("command_name", func(t *testing.T) { - assert.Equal(t, "update ", cmd.Use) + assert.Equal(t, "update [grant-id]", cmd.Use) }) t.Run("has_base_flags", func(t *testing.T) { @@ -198,7 +205,7 @@ func TestConfigDeleteCmd(t *testing.T) { cmd := newConfigDeleteCmd() t.Run("command_name", func(t *testing.T) { - assert.Equal(t, "delete ", cmd.Use) + assert.Equal(t, "delete [grant-id]", cmd.Use) }) t.Run("has_yes_flag", func(t *testing.T) { @@ -306,6 +313,20 @@ func TestBookingShowCmd(t *testing.T) { } +// Booking endpoints authenticate with a session token minted from the +// configuration, so every booking command must require --configuration-id. +func TestBookingCommands_RequireConfigurationID(t *testing.T) { + for _, newCmd := range []func() *cobra.Command{ + newBookingShowCmd, newBookingConfirmCmd, newBookingRescheduleCmd, newBookingCancelCmd, + } { + cmd := newCmd() + flag := cmd.Flags().Lookup("configuration-id") + require.NotNil(t, flag, "%s must expose --configuration-id", cmd.Name()) + annotations := flag.Annotations[cobra.BashCompOneRequiredFlag] + assert.Contains(t, annotations, "true", "%s must mark --configuration-id required", cmd.Name()) + } +} + func TestBookingConfirmCmd(t *testing.T) { cmd := newBookingConfirmCmd() @@ -316,6 +337,35 @@ func TestBookingConfirmCmd(t *testing.T) { t.Run("requires_exactly_one_arg", func(t *testing.T) { assert.NotNil(t, cmd.Args) }) + + // Salt is a server-issued token that cannot be looked up from the booking + // ID, so a missing --salt must fail locally with guidance on where to find + // it — never reach the API. + t.Run("missing_salt_returns_actionable_error", func(t *testing.T) { + _, _, err := testutil.ExecuteCommand(newBookingConfirmCmd(), "booking-1", "--configuration-id", "config-1") + require.Error(t, err) + // The custom message (not cobra's generic "required flag" error) + // confirms the salt guard fired locally before any API call. + assert.Contains(t, err.Error(), "salt is required to confirm a booking") + }) + + // --reason was dropped from the v3 confirm payload. It is kept as a + // deprecated no-op so existing `confirm --reason ...` scripts degrade + // gracefully instead of hitting cobra's "unknown flag" error on upgrade. + t.Run("reason_flag_is_deprecated_not_removed", func(t *testing.T) { + flag := cmd.Flags().Lookup("reason") + require.NotNil(t, flag, "--reason must remain as a compatibility shim") + assert.NotEmpty(t, flag.Deprecated, "--reason must be marked deprecated") + }) + + // Passing the deprecated --reason must not crash with "unknown flag"; it + // still falls through to the salt guard (reason alone can't confirm). + t.Run("deprecated_reason_flag_is_accepted", func(t *testing.T) { + _, _, err := testutil.ExecuteCommand(newBookingConfirmCmd(), "booking-1", "--configuration-id", "config-1", "--reason", "obsolete") + require.Error(t, err) + assert.NotContains(t, err.Error(), "unknown flag") + assert.Contains(t, err.Error(), "salt is required to confirm a booking") + }) } func TestBookingRescheduleCmd(t *testing.T) { @@ -400,7 +450,7 @@ func TestBookingRescheduleCmd_Validation(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cmd := newBookingRescheduleCmd() - cmd.SetArgs([]string{"test-booking-id", "--start-time", tt.startTime, "--end-time", tt.endTime}) + cmd.SetArgs([]string{"test-booking-id", "--configuration-id", "config-1", "--start-time", tt.startTime, "--end-time", tt.endTime}) err := cmd.Execute() @@ -429,3 +479,65 @@ func TestBookingCancelCmd(t *testing.T) { assert.NotNil(t, flag) }) } + +func TestResolveRescheduleResult(t *testing.T) { + applied := &domain.Booking{BookingID: "booking-1"} + + t.Run("verified success passes through without warning", func(t *testing.T) { + booking, warning, err := resolveRescheduleResult(applied, nil) + require.NoError(t, err) + assert.Equal(t, applied, booking) + assert.Empty(t, warning) + }) + + t.Run("read-back failure becomes success with warning", func(t *testing.T) { + // The reschedule was applied; failing the command would make scripts + // retry or alert on a change that took effect. + sentinelErr := fmt.Errorf("%w: transient failure", domain.ErrBookingReadBackFailed) + booking, warning, err := resolveRescheduleResult(applied, sentinelErr) + require.NoError(t, err) + assert.Equal(t, applied, booking) + assert.Contains(t, warning, "unverified") + }) + + t.Run("other errors stay errors", func(t *testing.T) { + booking, warning, err := resolveRescheduleResult(nil, errors.New("PATCH failed")) + require.Error(t, err) + assert.Nil(t, booking) + assert.Empty(t, warning) + }) + + t.Run("sentinel without booking stays an error", func(t *testing.T) { + // Defensive: an implementer violating the port contract must not cause + // a nil-booking success. + sentinelErr := fmt.Errorf("%w: transient failure", domain.ErrBookingReadBackFailed) + booking, warning, err := resolveRescheduleResult(nil, sentinelErr) + require.Error(t, err) + assert.Nil(t, booking) + assert.Empty(t, warning) + }) +} + +func TestRescheduleJSONPayload(t *testing.T) { + booking := &domain.Booking{BookingID: "booking-1"} + + t.Run("verified success omits warning key", func(t *testing.T) { + data, err := json.Marshal(rescheduleJSONPayload(booking, "")) + require.NoError(t, err) + var raw map[string]json.RawMessage + require.NoError(t, json.Unmarshal(data, &raw)) + _, ok := raw["warning"] + assert.False(t, ok, "warning key must be absent on verified success") + }) + + t.Run("partial success carries warning key", func(t *testing.T) { + // --json consumers must be able to tell an unverified reschedule from a + // verified one without parsing stderr. + data, err := json.Marshal(rescheduleJSONPayload(booking, "record unverified")) + require.NoError(t, err) + var raw map[string]json.RawMessage + require.NoError(t, json.Unmarshal(data, &raw)) + assert.Contains(t, raw, "warning") + assert.Contains(t, raw, "booking_id") + }) +} diff --git a/internal/cli/scheduler/sessions.go b/internal/cli/scheduler/sessions.go index f820cd6..23fc16b 100644 --- a/internal/cli/scheduler/sessions.go +++ b/internal/cli/scheduler/sessions.go @@ -38,7 +38,7 @@ func newSessionCreateCmd() *cobra.Command { Short: "Create a scheduler session", Long: "Create a new scheduler session for a configuration.", RunE: func(cmd *cobra.Command, args []string) error { - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { req := &domain.CreateSchedulerSessionRequest{ ConfigurationID: configID, TimeToLive: ttl, @@ -64,7 +64,7 @@ func newSessionCreateCmd() *cobra.Command { } cmd.Flags().StringVar(&configID, "config-id", "", "Configuration ID (required)") - cmd.Flags().IntVar(&ttl, "ttl", 30, "Time to live in minutes") + cmd.Flags().IntVar(&ttl, "ttl", 30, "Time to live in minutes, max 30") _ = cmd.MarkFlagRequired("config-id") @@ -79,7 +79,7 @@ func newSessionShowCmd() *cobra.Command { Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { sessionID := args[0] - _, err := common.WithClient(args, func(ctx context.Context, client ports.NylasClient, grantID string) (struct{}, error) { + _, err := common.WithClientNoGrant(func(ctx context.Context, client ports.NylasClient) (struct{}, error) { session, err := client.GetSchedulerSession(ctx, sessionID) if err != nil { return struct{}{}, common.WrapGetError("session", err) diff --git a/internal/domain/admin.go b/internal/domain/admin.go index bdbe985..f0f15ee 100644 --- a/internal/domain/admin.go +++ b/internal/domain/admin.go @@ -1,5 +1,147 @@ package domain +import ( + "errors" + "strings" +) + +// deprecatedConnectorProviderInbox is the provider the CLI and API surface no +// longer support. +const deprecatedConnectorProviderInbox = "inbox" + +// IsDeprecatedConnectorProvider reports whether a connector provider is no +// longer supported and should be hidden or rejected. +func IsDeprecatedConnectorProvider(provider string) bool { + return strings.EqualFold(strings.TrimSpace(provider), deprecatedConnectorProviderInbox) +} + +// supportedConnectorProviders is the Nylas v3 provider allow-list (the {provider} +// path-segment enum). Used to reject a typo'd/unknown explicit connector value +// before it is sent as a bogus path segment. +var supportedConnectorProviders = map[string]bool{ + "google": true, "microsoft": true, "imap": true, "icloud": true, + "yahoo": true, "ews": true, "virtual-calendar": true, "zoom": true, "nylas": true, +} + +// IsSupportedConnectorProvider reports whether a value is a recognized Nylas v3 +// connector provider name. +func IsSupportedConnectorProvider(provider string) bool { + return supportedConnectorProviders[strings.ToLower(strings.TrimSpace(provider))] +} + +// FilterVisibleConnectors removes deprecated connector providers from a list +// while leaving the backend API surface unchanged. +func FilterVisibleConnectors(connectors []Connector) []Connector { + if len(connectors) == 0 { + return connectors + } + + filtered := make([]Connector, 0, len(connectors)) + for _, connector := range connectors { + if IsDeprecatedConnectorProvider(connector.Provider) { + continue + } + filtered = append(filtered, connector) + } + + return filtered +} + +// VisibleConnectorProviders returns the providers of all non-deprecated +// connectors, in order. Auto-detecting a sole connector is len()==1; a longer +// list is the "which one?" disambiguation set. Shared by the CLI and RPC +// credential surfaces so they resolve the same connector. +func VisibleConnectorProviders(connectors []Connector) []string { + visible := FilterVisibleConnectors(connectors) + providers := make([]string, len(visible)) + for i, c := range visible { + providers[i] = c.Provider + } + return providers +} + +// ConnectorProviderForIdentifier maps either a provider name or a legacy +// connector ID to the provider required by provider-scoped connector APIs. +func ConnectorProviderForIdentifier(connectors []Connector, identifier string) (string, bool) { + for _, connector := range connectors { + if strings.EqualFold(identifier, connector.Provider) || identifier == connector.ID { + return connector.Provider, true + } + } + return "", false +} + +// ErrNoConnectors is returned by ResolveSoleConnectorProvider when there are no +// visible connectors to auto-detect. +var ErrNoConnectors = errors.New("no connectors found") + +// MultipleConnectorsError is returned by ResolveSoleConnectorProvider when +// auto-detection is ambiguous because more than one visible connector exists. +// Providers lists the candidates so callers can name them. +type MultipleConnectorsError struct{ Providers []string } + +func (e *MultipleConnectorsError) Error() string { + return "multiple connectors found: " + strings.Join(e.Providers, ", ") +} + +// ResolveSoleConnectorProvider returns the provider of the single visible +// connector, or ErrNoConnectors / *MultipleConnectorsError. It is the single +// source of truth for the auto-detect decision shared by the CLI and RPC +// credential commands; each caller maps the error to its own surface's format. +func ResolveSoleConnectorProvider(connectors []Connector) (string, error) { + providers := VisibleConnectorProviders(connectors) + switch len(providers) { + case 1: + return providers[0], nil + case 0: + return "", ErrNoConnectors + default: + return "", &MultipleConnectorsError{Providers: providers} + } +} + +// ErrDeprecatedConnector is returned by ResolveConnectorProvider when the caller +// explicitly names a provider that is no longer supported. +var ErrDeprecatedConnector = errors.New("connector provider is no longer supported") + +// ErrUnknownConnector is returned by ResolveConnectorProvider when an explicit +// value matches no known connector and is not a recognized provider name. +var ErrUnknownConnector = errors.New("unknown connector provider") + +// ResolveConnectorProvider is the single decision shared by the CLI and RPC +// credential surfaces: it maps an explicit provider/legacy connector ID to a +// provider, or auto-detects the sole visible connector when explicit is empty. +// Callers pass whatever ListConnectors returned (nil/empty is fine — an explicit +// value still resolves by passthrough) and map the returned error to their own +// surface's format. Keeping the policy here stops the two surfaces from silently +// diverging. +func ResolveConnectorProvider(connectors []Connector, explicit string) (string, error) { + if explicit != "" { + if IsDeprecatedConnectorProvider(explicit) { + return "", ErrDeprecatedConnector + } + if provider, ok := ConnectorProviderForIdentifier(connectors, explicit); ok { + // A legacy connector ID can map to a deprecated provider even though + // the ID itself is not the literal "inbox" string, so re-check the + // resolved provider to close that bypass. + if IsDeprecatedConnectorProvider(provider) { + return "", ErrDeprecatedConnector + } + return provider, nil + } + // Not a known connector (by name or ID). Accept it only if it is a + // recognized provider name — this keeps `--connector google` working when + // no connector exists yet or discovery is unavailable, while rejecting a + // typo instead of sending it as a bogus /connectors/{value}/creds segment. + // Return the normalized form so casing/whitespace can't leak into the path. + if IsSupportedConnectorProvider(explicit) { + return strings.ToLower(strings.TrimSpace(explicit)), nil + } + return "", ErrUnknownConnector + } + return ResolveSoleConnectorProvider(connectors) +} + // Application represents a Nylas application type Application struct { ID string `json:"id,omitempty"` @@ -103,15 +245,14 @@ type UpdateConnectorRequest struct { Scopes []string `json:"scopes,omitempty"` } -// ConnectorCredential represents authentication credentials for a connector +// ConnectorCredential is the Nylas v3 credential response object. Per the spec +// (CredentialObject) the API returns only these fields; credential_type and +// credential_data are request-only inputs and are never echoed back. type ConnectorCredential struct { - ID string `json:"id,omitempty"` - Name string `json:"name"` - ConnectorID string `json:"connector_id,omitempty"` - CredentialType string `json:"credential_type"` // "oauth", "service_account", "connector" - CredentialData map[string]any `json:"credential_data,omitempty"` - CreatedAt *UnixTime `json:"created_at,omitempty"` - UpdatedAt *UnixTime `json:"updated_at,omitempty"` + ID string `json:"id,omitempty"` + Name string `json:"name"` + CreatedAt *UnixTime `json:"created_at,omitempty"` + UpdatedAt *UnixTime `json:"updated_at,omitempty"` } // CreateCredentialRequest represents a request to create a credential diff --git a/internal/domain/admin_test.go b/internal/domain/admin_test.go new file mode 100644 index 0000000..5818876 --- /dev/null +++ b/internal/domain/admin_test.go @@ -0,0 +1,83 @@ +package domain + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestResolveConnectorProvider pins the resolution policy shared by the CLI and +// RPC credential surfaces: explicit values win (legacy IDs map to providers, +// deprecated ones are rejected, unknown ones pass through), and an empty value +// auto-detects the sole visible connector. +func TestResolveConnectorProvider(t *testing.T) { + connectors := []Connector{ + {ID: "conn-1", Provider: "google"}, + {Provider: "inbox"}, // deprecated, hidden from auto-detect + } + + t.Run("explicit provider is returned", func(t *testing.T) { + got, err := ResolveConnectorProvider(connectors, "google") + require.NoError(t, err) + assert.Equal(t, "google", got) + }) + + t.Run("legacy connector ID maps to provider", func(t *testing.T) { + got, err := ResolveConnectorProvider(connectors, "conn-1") + require.NoError(t, err) + assert.Equal(t, "google", got) + }) + + t.Run("explicit deprecated provider is rejected", func(t *testing.T) { + _, err := ResolveConnectorProvider(connectors, "inbox") + require.ErrorIs(t, err, ErrDeprecatedConnector) + }) + + t.Run("legacy ID mapping to a deprecated provider is rejected", func(t *testing.T) { + // The bypass: the identifier is an ID (not the literal "inbox"), so it + // slips the by-name check but resolves to the deprecated inbox provider. + withID := []Connector{{ID: "inbox-conn-9", Provider: "inbox"}} + _, err := ResolveConnectorProvider(withID, "inbox-conn-9") + require.ErrorIs(t, err, ErrDeprecatedConnector) + }) + + t.Run("supported provider name passes through even with no connectors", func(t *testing.T) { + got, err := ResolveConnectorProvider(nil, "microsoft") + require.NoError(t, err) + assert.Equal(t, "microsoft", got) + }) + + t.Run("unknown/typo'd explicit value is rejected, not sent as a path segment", func(t *testing.T) { + _, err := ResolveConnectorProvider(connectors, "googel") + require.ErrorIs(t, err, ErrUnknownConnector) + }) + + t.Run("supported provider is normalized (casing/whitespace) so it can't leak into the path", func(t *testing.T) { + for _, in := range []string{"Google", " google ", "GOOGLE"} { + got, err := ResolveConnectorProvider(nil, in) + require.NoError(t, err, in) + assert.Equal(t, "google", got, in) + } + }) + + t.Run("empty explicit auto-detects the sole visible connector", func(t *testing.T) { + got, err := ResolveConnectorProvider(connectors, "") + require.NoError(t, err) + assert.Equal(t, "google", got) + }) + + t.Run("empty explicit with no connectors returns ErrNoConnectors", func(t *testing.T) { + _, err := ResolveConnectorProvider(nil, "") + require.ErrorIs(t, err, ErrNoConnectors) + }) + + t.Run("empty explicit with multiple visible connectors is ambiguous", func(t *testing.T) { + multi := []Connector{{Provider: "google"}, {Provider: "microsoft"}} + _, err := ResolveConnectorProvider(multi, "") + var mErr *MultipleConnectorsError + require.True(t, errors.As(err, &mErr)) + assert.ElementsMatch(t, []string{"google", "microsoft"}, mErr.Providers) + }) +} diff --git a/internal/domain/errors.go b/internal/domain/errors.go index a572c0f..04cc3c4 100644 --- a/internal/domain/errors.go +++ b/internal/domain/errors.go @@ -80,6 +80,11 @@ var ( ErrBookingNotFound = errors.New("booking not found") ErrSessionNotFound = errors.New("session not found") ErrConfigurationNotFound = errors.New("configuration not found") + // ErrBookingReadBackFailed is a typed partial success: the reschedule PATCH + // was applied, but reading the booking back failed, so the server-side + // record could not be verified. Callers should report the change as applied + // while surfacing the verification failure. + ErrBookingReadBackFailed = errors.New("booking was rescheduled but reading it back failed") ) // APIError carries structured details from an HTTP error response while still diff --git a/internal/domain/scheduler.go b/internal/domain/scheduler.go index 1989c49..424151c 100644 --- a/internal/domain/scheduler.go +++ b/internal/domain/scheduler.go @@ -1,6 +1,10 @@ package domain -import "time" +import ( + "errors" + "fmt" + "time" +) // SchedulerConfiguration represents a scheduling configuration (meeting type) type SchedulerConfiguration struct { @@ -138,11 +142,28 @@ type SchedulerSession struct { // CreateSchedulerSessionRequest represents a request to create a scheduler session type CreateSchedulerSessionRequest struct { ConfigurationID string `json:"configuration_id"` - TimeToLive int `json:"ttl,omitempty"` // Session TTL in minutes + TimeToLive int `json:"time_to_live,omitempty"` // Session TTL in minutes (max 30) Slug string `json:"slug,omitempty"` AdditionalFields map[string]any `json:"additional_fields,omitempty"` } +// Validate checks the fields the Nylas v3 spec requires on session creation: +// the configuration is identified by configuration_id OR slug, and +// time_to_live is capped at 30 minutes (0 means unset; the API defaults to 5). +// It is the single source of truth shared by the adapter and RPC entrypoints. +func (r *CreateSchedulerSessionRequest) Validate() error { + if r == nil { + return errors.New("session request is required") + } + if r.ConfigurationID == "" && r.Slug == "" { + return errors.New("configuration_id or slug is required") + } + if r.TimeToLive < 0 || r.TimeToLive > 30 { + return fmt.Errorf("time_to_live must be between 0 and 30 minutes (0 uses the server default), got %d", r.TimeToLive) + } + return nil +} + // Booking represents a scheduled booking type Booking struct { BookingID string `json:"booking_id"` @@ -163,17 +184,32 @@ type Booking struct { UpdatedAt time.Time `json:"updated_at,omitempty"` } -// ConfirmBookingRequest represents a request to confirm a booking +// ConfirmBookingRequest represents a request to confirm or cancel a pending +// booking. Per the Nylas v3 spec, salt and status are required; the salt is +// extracted from the booking reference in the organizer confirmation link. type ConfirmBookingRequest struct { - Status string `json:"status"` // "confirmed" or "cancelled" - Reason string `json:"reason,omitempty"` - AdditionalData map[string]any `json:"additional_data,omitempty"` -} - -// RescheduleBookingRequest represents a request to reschedule a booking + Salt string `json:"salt"` + Status string `json:"status"` // "confirmed" or "cancelled" + CancellationReason string `json:"cancellation_reason,omitempty"` +} + +// Validate checks the fields the Nylas v3 spec requires on a confirm/cancel +// request: a salt and a "confirmed" or "cancelled" status. It is the single +// source of truth shared by the adapter and RPC entrypoints. +func (r *ConfirmBookingRequest) Validate() error { + if r.Salt == "" { + return errors.New("salt is required") + } + if r.Status != "confirmed" && r.Status != "cancelled" { + return fmt.Errorf("status must be 'confirmed' or 'cancelled', got %q", r.Status) + } + return nil +} + +// RescheduleBookingRequest is the Nylas v3 reschedule payload. Per the spec +// (booking_update.yaml) it carries only the new start and end times; timezone +// and reason are not part of the request model. type RescheduleBookingRequest struct { - StartTime int64 `json:"start_time"` // Unix timestamp for new start time - EndTime int64 `json:"end_time"` // Unix timestamp for new end time - Timezone string `json:"timezone,omitempty"` // Timezone for the booking (e.g., "America/New_York") - Reason string `json:"reason,omitempty"` // Reason for rescheduling + StartTime int64 `json:"start_time"` // Unix timestamp for new start time + EndTime int64 `json:"end_time"` // Unix timestamp for new end time } diff --git a/internal/domain/scheduler_settings_test.go b/internal/domain/scheduler_settings_test.go index a487cf8..22cd079 100644 --- a/internal/domain/scheduler_settings_test.go +++ b/internal/domain/scheduler_settings_test.go @@ -192,8 +192,8 @@ func TestConfirmBookingRequest_Creation(t *testing.T) { { name: "cancel booking with reason", req: ConfirmBookingRequest{ - Status: "cancelled", - Reason: "Schedule conflict", + Status: "cancelled", + CancellationReason: "Schedule conflict", }, }, } @@ -216,8 +216,6 @@ func TestRescheduleBookingRequest_Creation(t *testing.T) { req := RescheduleBookingRequest{ StartTime: now.Add(48 * time.Hour).Unix(), EndTime: now.Add(49 * time.Hour).Unix(), - Timezone: "America/Chicago", - Reason: "Guest requested different time", } if req.StartTime == 0 { @@ -226,9 +224,6 @@ func TestRescheduleBookingRequest_Creation(t *testing.T) { if req.EndTime == 0 { t.Error("RescheduleBookingRequest.EndTime should not be zero") } - if req.Timezone != "America/Chicago" { - t.Errorf("RescheduleBookingRequest.Timezone = %q, want %q", req.Timezone, "America/Chicago") - } } // ============================================================================= @@ -290,3 +285,35 @@ func TestUpdateSchedulerConfigurationRequest_Creation(t *testing.T) { t.Errorf("AvailabilityRules.DurationMinutes = %d, want 45", req.Availability.DurationMinutes) } } + +// ============================================================================= +// CreateSchedulerSessionRequest Validation Tests +// ============================================================================= + +func TestCreateSchedulerSessionRequest_Validate(t *testing.T) { + tests := []struct { + name string + req *CreateSchedulerSessionRequest + wantErr bool + }{ + // Per the v3 spec the configuration is identified by configuration_id + // OR slug, and time_to_live is capped at 30 minutes. + {name: "configuration_id only", req: &CreateSchedulerSessionRequest{ConfigurationID: "config-1"}, wantErr: false}, + {name: "slug only", req: &CreateSchedulerSessionRequest{Slug: "my-page"}, wantErr: false}, + {name: "ttl at max", req: &CreateSchedulerSessionRequest{ConfigurationID: "config-1", TimeToLive: 30}, wantErr: false}, + {name: "ttl unset defaults server-side", req: &CreateSchedulerSessionRequest{ConfigurationID: "config-1", TimeToLive: 0}, wantErr: false}, + {name: "nil request", req: nil, wantErr: true}, + {name: "missing configuration_id and slug", req: &CreateSchedulerSessionRequest{TimeToLive: 10}, wantErr: true}, + {name: "ttl above max", req: &CreateSchedulerSessionRequest{ConfigurationID: "config-1", TimeToLive: 31}, wantErr: true}, + {name: "negative ttl", req: &CreateSchedulerSessionRequest{ConfigurationID: "config-1", TimeToLive: -1}, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.req.Validate() + if (err != nil) != tt.wantErr { + t.Fatalf("Validate() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} diff --git a/internal/ports/admin.go b/internal/ports/admin.go index db3b0b4..30a57f5 100644 --- a/internal/ports/admin.go +++ b/internal/ports/admin.go @@ -87,17 +87,17 @@ type AdminClient interface { // ListCredentials retrieves all credentials for a connector. ListCredentials(ctx context.Context, connectorID string) ([]domain.ConnectorCredential, error) - // GetCredential retrieves a specific credential. - GetCredential(ctx context.Context, credentialID string) (*domain.ConnectorCredential, error) + // GetCredential retrieves a specific credential for a connector. + GetCredential(ctx context.Context, connectorID, credentialID string) (*domain.ConnectorCredential, error) // CreateCredential creates a new credential. CreateCredential(ctx context.Context, connectorID string, req *domain.CreateCredentialRequest) (*domain.ConnectorCredential, error) - // UpdateCredential updates an existing credential. - UpdateCredential(ctx context.Context, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) + // UpdateCredential updates an existing credential for a connector. + UpdateCredential(ctx context.Context, connectorID, credentialID string, req *domain.UpdateCredentialRequest) (*domain.ConnectorCredential, error) - // DeleteCredential deletes a credential. - DeleteCredential(ctx context.Context, credentialID string) error + // DeleteCredential deletes a credential for a connector. + DeleteCredential(ctx context.Context, connectorID, credentialID string) error // ================================ // ADMIN GRANT OPERATIONS diff --git a/internal/ports/scheduler.go b/internal/ports/scheduler.go index 29cec6f..cdd8111 100644 --- a/internal/ports/scheduler.go +++ b/internal/ports/scheduler.go @@ -12,20 +12,23 @@ type SchedulerClient interface { // CONFIGURATION OPERATIONS // ================================ - // ListSchedulerConfigurations retrieves all scheduler configurations. - ListSchedulerConfigurations(ctx context.Context) ([]domain.SchedulerConfiguration, error) + // Configuration operations are grant-scoped + // (/v3/grants/{grant_id}/scheduling/configurations). - // GetSchedulerConfiguration retrieves a specific scheduler configuration. - GetSchedulerConfiguration(ctx context.Context, configID string) (*domain.SchedulerConfiguration, error) + // ListSchedulerConfigurations retrieves all scheduler configurations for a grant. + ListSchedulerConfigurations(ctx context.Context, grantID string) ([]domain.SchedulerConfiguration, error) - // CreateSchedulerConfiguration creates a new scheduler configuration. - CreateSchedulerConfiguration(ctx context.Context, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) + // GetSchedulerConfiguration retrieves a specific scheduler configuration for a grant. + GetSchedulerConfiguration(ctx context.Context, grantID, configID string) (*domain.SchedulerConfiguration, error) - // UpdateSchedulerConfiguration updates an existing scheduler configuration. - UpdateSchedulerConfiguration(ctx context.Context, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) + // CreateSchedulerConfiguration creates a new scheduler configuration for a grant. + CreateSchedulerConfiguration(ctx context.Context, grantID string, req *domain.CreateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) - // DeleteSchedulerConfiguration deletes a scheduler configuration. - DeleteSchedulerConfiguration(ctx context.Context, configID string) error + // UpdateSchedulerConfiguration updates an existing scheduler configuration for a grant. + UpdateSchedulerConfiguration(ctx context.Context, grantID, configID string, req *domain.UpdateSchedulerConfigurationRequest) (*domain.SchedulerConfiguration, error) + + // DeleteSchedulerConfiguration deletes a scheduler configuration for a grant. + DeleteSchedulerConfiguration(ctx context.Context, grantID, configID string) error // ================================ // SESSION OPERATIONS @@ -41,17 +44,24 @@ type SchedulerClient interface { // BOOKING OPERATIONS // ================================ + // Booking operations authenticate with a Scheduler session token minted from + // the configuration ID (booking endpoints reject the application API key), so + // each takes the booking's configurationID. + // GetBooking retrieves a specific booking. - GetBooking(ctx context.Context, bookingID string) (*domain.Booking, error) + GetBooking(ctx context.Context, configurationID, bookingID string) (*domain.Booking, error) // ConfirmBooking confirms a booking. - ConfirmBooking(ctx context.Context, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) + ConfirmBooking(ctx context.Context, configurationID, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) - // RescheduleBooking reschedules an existing booking. - RescheduleBooking(ctx context.Context, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) + // RescheduleBooking reschedules an existing booking. If the returned error + // wraps domain.ErrBookingReadBackFailed the reschedule was applied but the + // record could not be verified; the returned booking is then a non-nil + // partial record carrying the applied times. + RescheduleBooking(ctx context.Context, configurationID, bookingID string, req *domain.RescheduleBookingRequest) (*domain.Booking, error) // CancelBooking cancels a booking. - CancelBooking(ctx context.Context, bookingID string, reason string) error + CancelBooking(ctx context.Context, configurationID, bookingID string, reason string) error // ================================ // GROUP EVENT OPERATIONS diff --git a/internal/ui/server_exec.go b/internal/ui/server_exec.go index 42a5caa..84a72f1 100644 --- a/internal/ui/server_exec.go +++ b/internal/ui/server_exec.go @@ -165,7 +165,6 @@ var allowedCommands = map[string]bool{ "scheduler sessions create": true, "scheduler sessions delete": true, // Scheduler bookings subcommands - "scheduler bookings list": true, "scheduler bookings show": true, "scheduler bookings create": true, "scheduler bookings confirm": true, diff --git a/internal/ui/static/js/commands-admin.js b/internal/ui/static/js/commands-admin.js index 06b60e6..089b638 100644 --- a/internal/ui/static/js/commands-admin.js +++ b/internal/ui/static/js/commands-admin.js @@ -57,8 +57,8 @@ const adminCommandSections = [ { title: 'Credentials', commands: { - 'credentials-list': { title: 'List', cmd: 'admin credentials list', desc: 'List credentials' }, - 'credentials-show': { title: 'Show', cmd: 'admin credentials show', desc: 'Show credential details', param: { name: 'credential-id', placeholder: 'Enter credential ID...' } } + 'credentials-list': { title: 'List', cmd: 'admin credentials list', desc: 'List credentials (connector auto-detected if only one)', param: { name: 'connector', placeholder: 'Connector provider, optional (e.g. google)...' } }, + 'credentials-show': { title: 'Show', cmd: 'admin credentials show', desc: 'Show credential details', param: { name: 'credential-id', placeholder: 'Enter credential ID...' }, flags: [{ name: 'connector', label: 'Connector', type: 'text', placeholder: 'Provider, optional (e.g. google)' }] } } }, { diff --git a/internal/ui/static/js/commands-scheduler.js b/internal/ui/static/js/commands-scheduler.js index 991368e..d407587 100644 --- a/internal/ui/static/js/commands-scheduler.js +++ b/internal/ui/static/js/commands-scheduler.js @@ -57,10 +57,9 @@ const schedulerCommandSections = [ { title: 'Bookings', commands: { - 'booking-list': { title: 'List', cmd: 'scheduler bookings list', desc: 'List scheduler bookings' }, - 'booking-show': { title: 'Show', cmd: 'scheduler bookings show', desc: 'Show booking details', param: { name: 'booking-id', placeholder: 'Enter booking ID...' } }, - 'booking-confirm': { title: 'Confirm', cmd: 'scheduler bookings confirm', desc: 'Confirm a booking', param: { name: 'booking-id', placeholder: 'Enter booking ID...' } }, - 'booking-cancel': { title: 'Cancel', cmd: 'scheduler bookings cancel', desc: 'Cancel a booking', param: { name: 'booking-id', placeholder: 'Enter booking ID...' } } + 'booking-show': { title: 'Show', cmd: 'scheduler bookings show', desc: 'Show booking details', flags: [{ name: 'configuration-id', label: 'Configuration ID', type: 'text', placeholder: 'Enter configuration ID...', required: true }], param: { name: 'booking-id', placeholder: 'Enter booking ID...' } }, + 'booking-confirm': { title: 'Confirm', cmd: 'scheduler bookings confirm', desc: 'Confirm a booking', flags: [{ name: 'configuration-id', label: 'Configuration ID', type: 'text', placeholder: 'Enter configuration ID...', required: true }, { name: 'salt', label: 'Salt (from confirmation link)', type: 'text', placeholder: 'Enter salt...', required: true }], param: { name: 'booking-id', placeholder: 'Enter booking ID...' } }, + 'booking-cancel': { title: 'Cancel', cmd: 'scheduler bookings cancel', desc: 'Cancel a booking', flags: [{ name: 'configuration-id', label: 'Configuration ID', type: 'text', placeholder: 'Enter configuration ID...', required: true }], param: { name: 'booking-id', placeholder: 'Enter booking ID...' } } } } ]; diff --git a/internal/ui/templates.go b/internal/ui/templates.go index 25357a0..320bacd 100644 --- a/internal/ui/templates.go +++ b/internal/ui/templates.go @@ -179,7 +179,6 @@ func GetDefaultCommands() Commands { {Key: "config-show", Title: "Show Config", Cmd: "scheduler configurations show", Desc: "Show configuration details", ParamName: "config-id", Placeholder: "Enter configuration ID..."}, {Key: "session-create", Title: "Create Session", Cmd: "scheduler sessions create", Desc: "Create a scheduling session"}, {Key: "session-show", Title: "Show Session", Cmd: "scheduler sessions show", Desc: "Show session details", ParamName: "session-id", Placeholder: "Enter session ID..."}, - {Key: "booking-list", Title: "List Bookings", Cmd: "scheduler bookings list", Desc: "List scheduler bookings"}, {Key: "booking-show", Title: "Show Booking", Cmd: "scheduler bookings show", Desc: "Show booking details", ParamName: "booking-id", Placeholder: "Enter booking ID..."}, {Key: "page-list", Title: "List Pages", Cmd: "scheduler pages list", Desc: "List scheduler pages"}, {Key: "page-create", Title: "Create Page", Cmd: "scheduler pages create", Desc: "Create a scheduler page"},