From 6a3767e66b85d3df53eed2c5151449f7152806b8 Mon Sep 17 00:00:00 2001 From: Qasim Date: Thu, 9 Jul 2026 23:54:44 -0400 Subject: [PATCH 1/2] TW-5785: fix Nylas v3 request shapes + add Scheduler session-token auth Corrects CLI request shapes that hit wrong Nylas v3 endpoints or sent invalid payloads, and adds the Scheduler session-token auth flow that booking commands require. Verified against the v3 spec; make ci-full green. Admin / credentials: - Fix connector credential URL: /connectors/{provider}/credentials -> /creds - Connector-scope credential ops; auto-detect the sole connector, validate an explicit provider against the v3 allow-list, reject the deprecated `inbox` provider (by name and by legacy connector ID) - Correct credential --type to the spec enum (connector); require the provider OAuth app --client-id/--client-secret (spec-required credential_data) - Trim ConnectorCredential to the fields the v3 CredentialObject returns (id/name/created_at/updated_at) - Offset-paginate credential lists so they no longer truncate at the default 10 - Deduplicate connector resolution into domain.ResolveConnectorProvider Scheduler: - Booking endpoints authenticate with a session token minted from a now-required --configuration-id (the API key is not accepted on these endpoints); reworked GetBooking/ConfirmBooking/RescheduleBooking/CancelBooking + session-token bearer plumbing in the HTTP client - Fix reschedule URL (bare PATCH /scheduling/bookings/{id}); read the booking back with the requested times overlaid (the booking model carries no times) - Confirm requires salt+status; handle the cancelled no-data envelope - Cancel sends cancellation_reason in the DELETE body - Grant-scope configuration endpoints; cursor-paginate the configuration list - Fix session TTL tag (ttl -> time_to_live) - Deprecated no-op shims: confirm --reason, reschedule --timezone/--reason JSON-RPC handlers, mocks/demo adapters, unit/integration tests, and command docs updated to match. --- docs/COMMANDS.md | 20 +- docs/commands/admin.md | 58 ++-- docs/commands/scheduler.md | 32 +- internal/adapters/nylas/admin.go | 70 +++-- .../admin_connectors_credentials_test.go | 103 +++++-- internal/adapters/nylas/admin_grants_test.go | 6 +- internal/adapters/nylas/client.go | 34 ++- internal/adapters/nylas/client_helpers.go | 8 +- internal/adapters/nylas/demo_admin.go | 41 ++- internal/adapters/nylas/mock_admin.go | 23 +- internal/adapters/nylas/mock_scheduler.go | 18 +- internal/adapters/nylas/scheduler.go | 192 +++++++++--- .../adapters/nylas/scheduler_bookings_test.go | 274 +++++++++++++----- .../adapters/nylas/scheduler_config_test.go | 70 ++++- .../adapters/nylas/scheduler_mock_test.go | 28 +- internal/adapters/rpcserver/handlers_admin.go | 80 ++++- .../adapters/rpcserver/handlers_admin_test.go | 84 +++++- .../adapters/rpcserver/handlers_scheduler.go | 97 ++++++- .../rpcserver/handlers_scheduler_test.go | 171 +++++++++-- internal/cli/admin/credentials.go | 157 ++++++++-- internal/cli/admin/credentials_test.go | 149 ++++++++++ internal/cli/calendar/ai_schedule.go | 63 +++- internal/cli/calendar/ai_schedule_test.go | 50 ++++ internal/cli/calendar/recurring.go | 16 +- internal/cli/calendar/recurring_test.go | 33 +++ internal/cli/common/connectors.go | 25 +- internal/cli/demo/scheduler_main.go | 2 +- internal/cli/email/drafts.go | 11 +- internal/cli/email/reply.go | 8 +- internal/cli/email/send.go | 11 +- internal/cli/email/templates_create.go | 11 +- internal/cli/integration/admin_test.go | 4 +- internal/cli/integration/group_events_test.go | 2 +- internal/cli/scheduler/bookings.go | 101 +++++-- internal/cli/scheduler/configurations.go | 40 ++- internal/cli/scheduler/scheduler_test.go | 58 +++- internal/cli/scheduler/sessions.go | 4 +- internal/domain/admin.go | 157 +++++++++- internal/domain/admin_test.go | 83 ++++++ internal/domain/scheduler.go | 45 ++- internal/domain/scheduler_settings_test.go | 9 +- internal/ports/admin.go | 12 +- internal/ports/scheduler.go | 35 ++- internal/ui/static/js/commands-admin.js | 4 +- internal/ui/static/js/commands-scheduler.js | 6 +- 45 files changed, 1967 insertions(+), 538 deletions(-) create mode 100644 internal/cli/admin/credentials_test.go create mode 100644 internal/cli/calendar/recurring_test.go create mode 100644 internal/domain/admin_test.go diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index c477a38..fd8d96c 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -908,12 +908,12 @@ nylas scheduler pages create # Create booking page nylas scheduler pages update # Update page nylas scheduler pages delete # Delete page -# Bookings +# Bookings (--configuration-id required: booking endpoints use a Scheduler session token, not the API key) 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 +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 +966,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/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..eb4a021 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 @@ -140,19 +146,27 @@ Manage scheduled appointments: 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 # Cancel a booking -nylas scheduler bookings cancel +nylas scheduler bookings cancel --configuration-id nylas scheduler bookings cancel \\ + --configuration-id \\ --reason "Meeting no longer needed" ``` @@ -213,9 +227,9 @@ nylas scheduler pages create \\ # 4. View bookings nylas scheduler bookings list -# 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/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..07ea28e 100644 --- a/internal/adapters/nylas/scheduler.go +++ b/internal/adapters/nylas/scheduler.go @@ -5,32 +5,58 @@ import ( "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 +67,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 +89,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 +114,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) } @@ -129,36 +165,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 +257,59 @@ 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 } + if req == nil { + return nil, fmt.Errorf("reschedule booking request is required") + } + token, err := c.bookingSessionToken(ctx, configurationID) + if err != nil { + return nil, err + } - queryURL := fmt.Sprintf("%s/v3/scheduling/bookings/%s/reschedule", c.baseURL, url.PathEscape(bookingID)) + // 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)) - resp, err := c.doJSONRequest(ctx, "PATCH", queryURL, req) + // The PATCH response carries only a request_id (no booking body). Read the + // updated booking back for the full record; if that read races ahead of + // propagation (404) or otherwise fails, fall back to reflecting the requested + // times so a successful reschedule never surfaces as an error or blank record. + resp, err := c.doJSONRequestWithToken(ctx, "PATCH", queryURL, token, req) if err != nil { return nil, err } + _ = resp.Body.Close() - var result struct { - Data domain.Booking `json:"data"` + start, end := time.Unix(req.StartTime, 0), time.Unix(req.EndTime, 0) + if booking, err := c.getBookingWithToken(ctx, token, bookingID); err == nil { + // 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 } - if err := c.decodeJSONResponse(resp, &result); err != nil { - return nil, err - } - return &result.Data, nil + return &domain.Booking{BookingID: bookingID, StartTime: start, EndTime: end}, 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..7870206 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,29 +170,108 @@ 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) { +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(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) { - assert.Equal(t, "/v3/scheduling/bookings/booking-456/reschedule", r.URL.Path) - assert.Equal(t, "PATCH", r.Method) + t.Errorf("unexpected request to %s — invalid confirm should not reach the API", r.URL.Path) + })) + defer server.Close() - response := map[string]any{ - "data": map[string]any{ - "booking_id": "booking-456", - "status": "confirmed", - }, - } + 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_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") - _ = json.NewEncoder(w).Encode( // Test helper, encode error not actionable - response) - })) + 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() @@ -164,24 +279,61 @@ func TestHTTPClient_RescheduleBooking(t *testing.T) { client.SetBaseURL(server.URL) ctx := context.Background() - req := &domain.RescheduleBookingRequest{ - StartTime: 1704067200, - EndTime: 1704070800, - } - booking, err := client.RescheduleBooking(ctx, "booking-456", req) + 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_FallsBackWhenReadBackFails(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_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 +341,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 +357,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 +384,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 +396,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 +404,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 +451,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..ac76d16 100644 --- a/internal/adapters/rpcserver/handlers_scheduler.go +++ b/internal/adapters/rpcserver/handlers_scheduler.go @@ -13,15 +13,22 @@ 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 } @@ -34,23 +41,40 @@ 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 } 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 { @@ -95,12 +119,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 +143,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 +160,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 +180,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 +200,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 @@ -204,8 +248,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 +267,15 @@ 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) + } + // 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,8 +290,18 @@ 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 { return nil, fmt.Errorf("scheduler.booking.reschedule: %w", err) } @@ -252,8 +316,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..b669994 100644 --- a/internal/adapters/rpcserver/handlers_scheduler_test.go +++ b/internal/adapters/rpcserver/handlers_scheduler_test.go @@ -12,34 +12,45 @@ 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 } -func (f *fakeSchedulerClient) ListSchedulerConfigurations(ctx context.Context) ([]domain.SchedulerConfiguration, error) { +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) 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 +61,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 +94,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 +110,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 +157,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 +212,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 +246,79 @@ 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) + } + }, + }, + { + 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) + } + }, + }, + { + // 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..13d7fa1 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,20 @@ 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 { _, 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 +205,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 +219,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 +287,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 +331,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..03246bc --- /dev/null +++ b/internal/cli/admin/credentials_test.go @@ -0,0 +1,149 @@ +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) + } +} 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..bc22e2e 100644 --- a/internal/cli/calendar/recurring.go +++ b/internal/cli/calendar/recurring.go @@ -32,6 +32,16 @@ API reference: https://developer.nylas.com/docs/v3/calendar/recurring-events/`, return cmd } +// recurringGrantArgs resolves the grant-resolution args for recurring commands. +// An explicit positional grant wins; otherwise the --grant flag is used as a +// fallback. +func recurringGrantArgs(positional []string, grantFlag string) []string { + if len(positional) == 0 && grantFlag != "" { + return []string{grantFlag} + } + return positional +} + // newRecurringListCmd creates the list recurring event instances command. func newRecurringListCmd() *cobra.Command { var ( @@ -58,7 +68,7 @@ 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 := recurringGrantArgs(args[1:], grantID) if calendarID == "" { return common.NewUserError("calendar ID is required", "Use --calendar to specify the calendar") @@ -141,7 +151,7 @@ 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 := recurringGrantArgs(args[1:], grantID) if calendarID == "" { return common.NewUserError("calendar ID is required", "Use --calendar to specify the calendar") @@ -237,7 +247,7 @@ 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 := recurringGrantArgs(args[1:], grantID) 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..0d7f27d --- /dev/null +++ b/internal/cli/calendar/recurring_test.go @@ -0,0 +1,33 @@ +package calendar + +import ( + "reflect" + "testing" +) + +func TestRecurringGrantArgs(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + positional []string + grantFlag string + want []string + }{ + // --grant must be honored when no positional grant is given, and the + // positional must win when both are set. + {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 wins over flag", positional: []string{"grant-pos"}, grantFlag: "grant-flag", want: []string{"grant-pos"}}, + {name: "positional used with no flag", positional: []string{"grant-pos"}, grantFlag: "", want: []string{"grant-pos"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + if got := recurringGrantArgs(tt.positional, tt.grantFlag); !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/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/scheduler/bookings.go b/internal/cli/scheduler/bookings.go index 8228bc0..3585206 100644 --- a/internal/cli/scheduler/bookings.go +++ b/internal/cli/scheduler/bookings.go @@ -31,15 +31,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 +87,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 +153,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 +179,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,15 +192,13 @@ 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, err := client.RescheduleBooking(ctx, configurationID, bookingID, req) if err != nil { return struct{}{}, common.WrapUpdateError("booking", err) } @@ -184,18 +217,25 @@ 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 } func newBookingCancelCmd() *cobra.Command { var ( - reason string - yes bool + configurationID string + reason string + yes bool ) cmd := &cobra.Command{ @@ -212,8 +252,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 +265,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..c01be64 100644 --- a/internal/cli/scheduler/scheduler_test.go +++ b/internal/cli/scheduler/scheduler_test.go @@ -3,7 +3,10 @@ package scheduler import ( "testing" + "github.com/nylas/cli/internal/cli/testutil" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNewSchedulerCmd(t *testing.T) { @@ -75,7 +78,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 +91,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 +100,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 +154,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 +201,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 +309,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 +333,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 +446,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() diff --git a/internal/cli/scheduler/sessions.go b/internal/cli/scheduler/sessions.go index f820cd6..e4359db 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, @@ -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/scheduler.go b/internal/domain/scheduler.go index 1989c49..9a693b7 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,7 +142,7 @@ 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"` } @@ -163,17 +167,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..a56087f 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") - } } // ============================================================================= 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..290cc8f 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,21 @@ 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(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/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..daa97cf 100644 --- a/internal/ui/static/js/commands-scheduler.js +++ b/internal/ui/static/js/commands-scheduler.js @@ -58,9 +58,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...' } } } } ]; From 052985999efb13274dc6841f43ee0fa982d18dc4 Mon Sep 17 00:00:00 2001 From: Qasim Date: Fri, 10 Jul 2026 07:28:47 -0400 Subject: [PATCH 2/2] fix --- .github/workflows/ci.yml | 2 + .github/workflows/release.yml | 1 + docs/COMMANDS.md | 1 - docs/RPC.md | 15 +- docs/commands/scheduler.md | 17 +- go.mod | 2 +- internal/adapters/nylas/scheduler.go | 34 ++- .../adapters/nylas/scheduler_bookings_test.go | 64 +++++- .../adapters/rpcserver/handlers_scheduler.go | 36 +++- .../rpcserver/handlers_scheduler_test.go | 201 ++++++++++++++++++ internal/cli/admin/credentials.go | 11 + internal/cli/admin/credentials_test.go | 22 ++ internal/cli/calendar/recurring.go | 35 ++- internal/cli/calendar/recurring_test.go | 15 +- internal/cli/common/format.go | 9 + .../cli/integration/scheduler_basic_test.go | 30 ++- internal/cli/scheduler/bookings.go | 35 ++- internal/cli/scheduler/scheduler_test.go | 66 ++++++ internal/cli/scheduler/sessions.go | 2 +- internal/domain/errors.go | 5 + internal/domain/scheduler.go | 17 ++ internal/domain/scheduler_settings_test.go | 32 +++ internal/ports/scheduler.go | 5 +- internal/ui/server_exec.go | 1 - internal/ui/static/js/commands-scheduler.js | 1 - internal/ui/templates.go | 1 - 26 files changed, 608 insertions(+), 52 deletions(-) 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 fd8d96c..20a9c34 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -909,7 +909,6 @@ nylas scheduler pages update # Update page nylas scheduler pages delete # Delete page # Bookings (--configuration-id required: booking endpoints use a Scheduler session token, not the API key) -nylas scheduler bookings list # List bookings 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 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/scheduler.md b/docs/commands/scheduler.md index eb4a021..daa912f 100644 --- a/docs/commands/scheduler.md +++ b/docs/commands/scheduler.md @@ -125,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 @@ -142,10 +143,6 @@ 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). @@ -162,6 +159,10 @@ nylas scheduler bookings confirm --configuration-id --s nylas scheduler bookings reschedule \\ --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 --configuration-id @@ -224,8 +225,8 @@ 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 (--configuration-id is required on booking commands) nylas scheduler bookings confirm --configuration-id --salt 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/scheduler.go b/internal/adapters/nylas/scheduler.go index 07ea28e..c6512eb 100644 --- a/internal/adapters/nylas/scheduler.go +++ b/internal/adapters/nylas/scheduler.go @@ -2,6 +2,7 @@ package nylas import ( "context" + "errors" "fmt" "net/http" "net/url" @@ -130,6 +131,10 @@ func (c *HTTPClient) DeleteSchedulerConfiguration(ctx context.Context, grantID, // 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) @@ -274,9 +279,9 @@ func (c *HTTPClient) RescheduleBooking(ctx context.Context, configurationID, boo 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; if that read races ahead of - // propagation (404) or otherwise fails, fall back to reflecting the requested - // times so a successful reschedule never surfaces as an error or blank record. + // 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 @@ -284,14 +289,21 @@ func (c *HTTPClient) RescheduleBooking(ctx context.Context, configurationID, boo _ = resp.Body.Close() start, end := time.Unix(req.StartTime, 0), time.Unix(req.EndTime, 0) - if booking, err := c.getBookingWithToken(ctx, token, bookingID); err == nil { - // 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 - } - return &domain.Booking{BookingID: bookingID, StartTime: start, EndTime: end}, nil + 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. diff --git a/internal/adapters/nylas/scheduler_bookings_test.go b/internal/adapters/nylas/scheduler_bookings_test.go index 7870206..dfb977c 100644 --- a/internal/adapters/nylas/scheduler_bookings_test.go +++ b/internal/adapters/nylas/scheduler_bookings_test.go @@ -236,6 +236,32 @@ func TestHTTPClient_ConfirmBooking_Validation(t *testing.T) { } } +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() + 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) + }) + } +} + 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. @@ -292,7 +318,7 @@ func TestHTTPClient_RescheduleBooking(t *testing.T) { assert.Equal(t, int64(1704070800), booking.EndTime.Unix()) } -func TestHTTPClient_RescheduleBooking_FallsBackWhenReadBackFails(t *testing.T) { +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) { @@ -321,6 +347,42 @@ func TestHTTPClient_RescheduleBooking_FallsBackWhenReadBackFails(t *testing.T) { 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 := bookingTestServer(t, "session-abc", func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "/v3/scheduling/bookings/booking-cancel", r.URL.Path) diff --git a/internal/adapters/rpcserver/handlers_scheduler.go b/internal/adapters/rpcserver/handlers_scheduler.go index ac76d16..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" @@ -35,6 +36,9 @@ type schedulerConfigUpdateParams struct { 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 { @@ -52,6 +56,9 @@ type schedulerBookingConfirmParams struct { 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 { @@ -81,6 +88,15 @@ 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"` @@ -216,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 { @@ -270,6 +293,11 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa 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) @@ -303,9 +331,15 @@ func RegisterSchedulerHandlers(d *Dispatcher, client ports.SchedulerClient, defa 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) { diff --git a/internal/adapters/rpcserver/handlers_scheduler_test.go b/internal/adapters/rpcserver/handlers_scheduler_test.go index b669994..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" @@ -27,6 +30,19 @@ type fakeSchedulerClient struct { 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 { @@ -36,6 +52,18 @@ func (f *fakeSchedulerClient) CancelBooking(ctx context.Context, configurationID 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) 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 { @@ -284,6 +312,98 @@ func TestRegisterSchedulerHandlers(t *testing.T) { } }, }, + { + // 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", @@ -308,6 +428,87 @@ func TestRegisterSchedulerHandlers(t *testing.T) { } }, }, + { + // 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. diff --git a/internal/cli/admin/credentials.go b/internal/cli/admin/credentials.go index 13d7fa1..5696853 100644 --- a/internal/cli/admin/credentials.go +++ b/internal/cli/admin/credentials.go @@ -186,6 +186,17 @@ 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 { diff --git a/internal/cli/admin/credentials_test.go b/internal/cli/admin/credentials_test.go index 03246bc..01f78fc 100644 --- a/internal/cli/admin/credentials_test.go +++ b/internal/cli/admin/credentials_test.go @@ -147,3 +147,25 @@ func TestCredentialCommands_Structure(t *testing.T) { 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/recurring.go b/internal/cli/calendar/recurring.go index bc22e2e..214072a 100644 --- a/internal/cli/calendar/recurring.go +++ b/internal/cli/calendar/recurring.go @@ -33,13 +33,23 @@ API reference: https://developer.nylas.com/docs/v3/calendar/recurring-events/`, } // recurringGrantArgs resolves the grant-resolution args for recurring commands. -// An explicit positional grant wins; otherwise the --grant flag is used as a -// fallback. -func recurringGrantArgs(positional []string, grantFlag string) []string { +// 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} + return []string{grantFlag}, nil } - return positional + return positional, nil } // newRecurringListCmd creates the list recurring event instances command. @@ -68,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 := recurringGrantArgs(args[1:], grantID) + 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") @@ -151,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 := recurringGrantArgs(args[1:], grantID) + 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") @@ -247,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 := recurringGrantArgs(args[1:], grantID) + 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 index 0d7f27d..0f5697c 100644 --- a/internal/cli/calendar/recurring_test.go +++ b/internal/cli/calendar/recurring_test.go @@ -13,19 +13,26 @@ func TestRecurringGrantArgs(t *testing.T) { positional []string grantFlag string want []string + wantErr bool }{ - // --grant must be honored when no positional grant is given, and the - // positional must win when both are set. + // --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 wins over flag", positional: []string{"grant-pos"}, grantFlag: "grant-flag", want: []string{"grant-pos"}}, {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() - if got := recurringGrantArgs(tt.positional, tt.grantFlag); !reflect.DeepEqual(got, tt.want) { + 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/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/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 3585206..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" @@ -198,13 +199,16 @@ You must provide the new start and end times as Unix timestamps.`, EndTime: endTime, } - booking, err := client.RescheduleBooking(ctx, configurationID, 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) @@ -231,6 +235,33 @@ You must provide the new start and end times as Unix timestamps.`, 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 ( configurationID string diff --git a/internal/cli/scheduler/scheduler_test.go b/internal/cli/scheduler/scheduler_test.go index c01be64..6ad2d9a 100644 --- a/internal/cli/scheduler/scheduler_test.go +++ b/internal/cli/scheduler/scheduler_test.go @@ -1,9 +1,13 @@ 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" @@ -475,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 e4359db..23fc16b 100644 --- a/internal/cli/scheduler/sessions.go +++ b/internal/cli/scheduler/sessions.go @@ -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") 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 9a693b7..424151c 100644 --- a/internal/domain/scheduler.go +++ b/internal/domain/scheduler.go @@ -147,6 +147,23 @@ type CreateSchedulerSessionRequest struct { 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"` diff --git a/internal/domain/scheduler_settings_test.go b/internal/domain/scheduler_settings_test.go index a56087f..22cd079 100644 --- a/internal/domain/scheduler_settings_test.go +++ b/internal/domain/scheduler_settings_test.go @@ -285,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/scheduler.go b/internal/ports/scheduler.go index 290cc8f..cdd8111 100644 --- a/internal/ports/scheduler.go +++ b/internal/ports/scheduler.go @@ -54,7 +54,10 @@ type SchedulerClient interface { // ConfirmBooking confirms a booking. ConfirmBooking(ctx context.Context, configurationID, bookingID string, req *domain.ConfirmBookingRequest) (*domain.Booking, error) - // RescheduleBooking reschedules an existing booking. + // 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. 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-scheduler.js b/internal/ui/static/js/commands-scheduler.js index daa97cf..d407587 100644 --- a/internal/ui/static/js/commands-scheduler.js +++ b/internal/ui/static/js/commands-scheduler.js @@ -57,7 +57,6 @@ 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', 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"},