From 6a3ed255d1139be0aa1d51853c8c151acab6dea6 Mon Sep 17 00:00:00 2001 From: Qasim Date: Sun, 21 Jun 2026 17:21:00 -0400 Subject: [PATCH] TW-5658: fix drafts list timestamps and add `drafts list --id` flag The Nylas v3 drafts API omits created_at/updated_at (or sends them as null) and carries the real timestamp in a `date` field, so the CLI rendered every draft's UPDATED column as "56 years ago" (Unix epoch). The full draft ID was also unreachable from the table without --json. - adapters/nylas/drafts: migrate draftResponse timestamps to domain.UnixTime; firstSetTime falls back to `date` when created_at/updated_at are unset (omitted -> zero time, null -> epoch). - cli/common/time: FormatTimeAgo renders "unknown" for an unset timestamp (zero value or exact Unix epoch); pre-1970 times still format. - cli/email/drafts: add `--id` flag to show full draft IDs (matches email list/folders/threads); ID column width and separator scale together; drafts show guards an unset UpdatedAt. - Tests across the changed packages; docs/COMMANDS.md updated. --- docs/COMMANDS.md | 1 + internal/adapters/nylas/drafts.go | 26 ++++++-- internal/adapters/nylas/drafts_unit_test.go | 67 +++++++++++++++++++-- internal/cli/common/time.go | 8 +++ internal/cli/common/time_test.go | 23 +++++++ internal/cli/email/drafts.go | 37 +++++++++--- internal/cli/email/email_advanced_test.go | 16 +++++ 7 files changed, 161 insertions(+), 17 deletions(-) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index b35b0f7..31ecf01 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -335,6 +335,7 @@ nylas email threads delete # Delete thread ```bash nylas email drafts list # List drafts +nylas email drafts list --id # Show full draft IDs nylas email drafts show # Show draft details nylas email drafts create --to EMAIL --subject S # Create draft nylas email drafts create --to EMAIL --subject S --signature-id SIG # Create draft with stored signature diff --git a/internal/adapters/nylas/drafts.go b/internal/adapters/nylas/drafts.go index 702b3fa..3962704 100644 --- a/internal/adapters/nylas/drafts.go +++ b/internal/adapters/nylas/drafts.go @@ -51,8 +51,12 @@ type draftResponse struct { ContentType string `json:"content_type"` Size int64 `json:"size"` } `json:"attachments"` - CreatedAt int64 `json:"created_at"` - UpdatedAt int64 `json:"updated_at"` + CreatedAt domain.UnixTime `json:"created_at"` + UpdatedAt domain.UnixTime `json:"updated_at"` + // Date is the draft's compose/last-saved time. The Nylas v3 drafts API + // returns created_at/updated_at as null and puts the real timestamp here, + // mirroring the message `date` field. + Date domain.UnixTime `json:"date"` } // GetDrafts retrieves drafts for a grant. @@ -400,7 +404,21 @@ func convertDraft(d draftResponse) domain.Draft { ReplyToMsgID: d.ReplyToMsgID, ThreadID: d.ThreadID, Attachments: util.Map(d.Attachments, convertAttachment), - CreatedAt: time.Unix(d.CreatedAt, 0), - UpdatedAt: time.Unix(d.UpdatedAt, 0), + // The Nylas v3 drafts API returns created_at/updated_at as null + // (decoded to the Unix epoch); fall back to the `date` field so drafts + // don't render as 1970. + CreatedAt: firstSetTime(d.CreatedAt, d.Date), + UpdatedAt: firstSetTime(d.UpdatedAt, d.Date), } } + +// firstSetTime returns primary when it is populated, otherwise fallback. The +// Nylas v3 drafts API leaves created_at/updated_at unset in two ways: omitted +// entirely (decodes to the zero time, year 1) or sent as null (decodes to the +// Unix epoch). Both count as unset, so we fall back to the `date` field. +func firstSetTime(primary, fallback domain.UnixTime) time.Time { + if !primary.IsZero() && primary.Unix() != 0 { + return primary.Time + } + return fallback.Time +} diff --git a/internal/adapters/nylas/drafts_unit_test.go b/internal/adapters/nylas/drafts_unit_test.go index e452015..b0f34ce 100644 --- a/internal/adapters/nylas/drafts_unit_test.go +++ b/internal/adapters/nylas/drafts_unit_test.go @@ -4,12 +4,20 @@ package nylas import ( + "encoding/json" "testing" "time" + "github.com/nylas/cli/internal/domain" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) +// ut builds a domain.UnixTime from Unix seconds for draftResponse fixtures. +func ut(sec int64) domain.UnixTime { + return domain.UnixTime{Time: time.Unix(sec, 0)} +} + func TestConvertDraft(t *testing.T) { now := time.Now().Unix() @@ -63,8 +71,8 @@ func TestConvertDraft(t *testing.T) { Size: 50000, }, }, - CreatedAt: now - 3600, - UpdatedAt: now, + CreatedAt: ut(now - 3600), + UpdatedAt: ut(now), } draft := convertDraft(apiDraft) @@ -120,8 +128,8 @@ func TestConvertDrafts(t *testing.T) { }{ {Name: "User1", Email: "user1@example.com"}, }, - CreatedAt: now, - UpdatedAt: now, + CreatedAt: ut(now), + UpdatedAt: ut(now), }, { ID: "draft-2", @@ -134,8 +142,8 @@ func TestConvertDrafts(t *testing.T) { }{ {Name: "User2", Email: "user2@example.com"}, }, - CreatedAt: now, - UpdatedAt: now, + CreatedAt: ut(now), + UpdatedAt: ut(now), }, } @@ -155,3 +163,50 @@ func TestConvertDrafts_Empty(t *testing.T) { assert.NotNil(t, drafts) assert.Len(t, drafts, 0) } + +// TestConvertDraft_DateFallback verifies that drafts use the `date` field for +// timestamps. The Nylas v3 drafts API carries the real timestamp in `date` and +// leaves created_at/updated_at unset — without this fallback the CLI renders +// every draft at the zero time ("292 years ago") or the Unix epoch. +// +// The two cases are decoded from raw JSON so the test exercises the real +// unmarshal path: created_at/updated_at omitted (decodes to the zero time) and +// sent as null (decodes to the Unix epoch). +func TestConvertDraft_DateFallback(t *testing.T) { + date := int64(1782059501) + + cases := map[string]string{ + "omitted": `{"id":"d","date":1782059501}`, + "null": `{"id":"d","created_at":null,"updated_at":null,"date":1782059501}`, + } + + for name, raw := range cases { + t.Run(name, func(t *testing.T) { + var d draftResponse + require.NoError(t, json.Unmarshal([]byte(raw), &d)) + + draft := convertDraft(d) + + assert.Equal(t, time.Unix(date, 0), draft.UpdatedAt, "UpdatedAt should fall back to date") + assert.Equal(t, time.Unix(date, 0), draft.CreatedAt, "CreatedAt should fall back to date") + }) + } +} + +// TestConvertDraft_ExplicitTimestampsWin verifies created_at/updated_at take +// precedence over date when the API does provide them. +func TestConvertDraft_ExplicitTimestampsWin(t *testing.T) { + date := int64(1782059501) + created := date - 3600 + updated := date - 60 + + draft := convertDraft(draftResponse{ + ID: "draft-explicit", + Date: ut(date), + CreatedAt: ut(created), + UpdatedAt: ut(updated), + }) + + assert.Equal(t, time.Unix(created, 0), draft.CreatedAt) + assert.Equal(t, time.Unix(updated, 0), draft.UpdatedAt) +} diff --git a/internal/cli/common/time.go b/internal/cli/common/time.go index ecbdf62..f0a971e 100644 --- a/internal/cli/common/time.go +++ b/internal/cli/common/time.go @@ -152,7 +152,15 @@ func ParseHumanTime(input string, opts ParseHumanTimeOpts) (time.Time, error) { } // FormatTimeAgo formats a time as a relative string (e.g., "2 hours ago"). +// An unset timestamp — the zero value (year 1) or the Unix epoch, which is how +// an omitted or null API timestamp typically decodes — renders as "unknown" +// rather than a nonsensical "56 years ago". A genuine pre-1970 timestamp +// (negative Unix seconds) is still formatted normally. func FormatTimeAgo(t time.Time) string { + if t.IsZero() || t.Unix() == 0 { + return "unknown" + } + now := time.Now() diff := now.Sub(t) diff --git a/internal/cli/common/time_test.go b/internal/cli/common/time_test.go index 64f1e47..e746349 100644 --- a/internal/cli/common/time_test.go +++ b/internal/cli/common/time_test.go @@ -2,6 +2,7 @@ package common import ( "errors" + "fmt" "testing" "time" ) @@ -136,6 +137,28 @@ func TestFormatTimeAgo(t *testing.T) { time: now.Add(-730 * 24 * time.Hour), expected: "2 years ago", }, + { + // Zero value (year 1) — an omitted API timestamp. Must not render + // as "292 years ago". + name: "zero time is unknown", + time: time.Time{}, + expected: "unknown", + }, + { + // Unix epoch — a null API timestamp. Must not render as + // "56 years ago". + name: "epoch is unknown", + time: time.Unix(0, 0), + expected: "unknown", + }, + { + // A genuine pre-1970 timestamp (negative Unix seconds) is real + // data, not "unset" — it must still format as a relative duration, + // not "unknown". Guards against an over-broad Unix() <= 0 check. + name: "pre-1970 timestamp is not unknown", + time: time.Unix(-100, 0), + expected: fmt.Sprintf("%d years ago", int(now.Sub(time.Unix(-100, 0)).Hours()/(24*365))), + }, } for _, tt := range tests { diff --git a/internal/cli/email/drafts.go b/internal/cli/email/drafts.go index d8975fb..0937cd2 100644 --- a/internal/cli/email/drafts.go +++ b/internal/cli/email/drafts.go @@ -36,6 +36,7 @@ API reference: https://developer.nylas.com/docs/reference/api/drafts/`, func newDraftsListCmd() *cobra.Command { var limit int + var showID bool cmd := &cobra.Command{ Use: "list [grant-id]", @@ -59,9 +60,17 @@ func newDraftsListCmd() *cobra.Command { return struct{}{}, nil } + // Widen the ID column when --id shows full IDs (otherwise the + // IDs are truncated for a compact table). + idWidth := 15 + if showID { + idWidth = 50 + } + fmt.Printf("Found %d drafts:\n\n", len(drafts)) - fmt.Printf("%-15s %-25s %-35s %s\n", "ID", "TO", "SUBJECT", "UPDATED") - fmt.Println("--------------------------------------------------------------------------------") + header := fmt.Sprintf("%-*s %-25s %-35s %s", idWidth, "ID", "TO", "SUBJECT", "UPDATED") + fmt.Println(header) + fmt.Println(strings.Repeat("-", len(header))) for _, d := range drafts { toStr := "" @@ -76,12 +85,18 @@ func newDraftsListCmd() *cobra.Command { } subj = common.Truncate(subj, 33) - // Show first 12 chars of ID - idShort := common.Truncate(d.ID, 15) - dateStr := common.FormatTimeAgo(d.UpdatedAt) - fmt.Printf("%-15s %-25s %-35s %s\n", idShort, toStr, subj, common.Dim.Sprint(dateStr)) + id := d.ID + if !showID { + id = common.Truncate(d.ID, 15) + } + fmt.Printf("%-*s %-25s %-35s %s\n", idWidth, id, toStr, subj, common.Dim.Sprint(dateStr)) + } + + if !showID { + fmt.Println() + _, _ = common.Dim.Printf("Use --id to see full draft IDs\n") } return struct{}{}, nil @@ -91,6 +106,7 @@ func newDraftsListCmd() *cobra.Command { } cmd.Flags().IntVarP(&limit, "limit", "l", 10, "Number of drafts to fetch") + cmd.Flags().BoolVar(&showID, "id", false, "Show full draft IDs") return cmd } @@ -304,7 +320,14 @@ func newDraftsShowCmd() *cobra.Command { if len(draft.Cc) > 0 { fmt.Printf("Cc: %s\n", common.FormatParticipants(draft.Cc)) } - fmt.Printf("Updated: %s\n", draft.UpdatedAt.Format(common.DisplayDateTime)) + // Mirror the list view: an unset timestamp (zero value or epoch, + // from the API omitting/nulling created_at/updated_at) shows as + // "unknown" rather than a nonsensical date. + updated := "unknown" + if !draft.UpdatedAt.IsZero() && draft.UpdatedAt.Unix() != 0 { + updated = draft.UpdatedAt.Format(common.DisplayDateTime) + } + fmt.Printf("Updated: %s\n", updated) // Show attachments if any if len(draft.Attachments) > 0 { diff --git a/internal/cli/email/email_advanced_test.go b/internal/cli/email/email_advanced_test.go index ac35c4c..f7f7c4d 100644 --- a/internal/cli/email/email_advanced_test.go +++ b/internal/cli/email/email_advanced_test.go @@ -7,6 +7,7 @@ import ( "github.com/nylas/cli/internal/cli/common" "github.com/nylas/cli/internal/domain" + "github.com/spf13/cobra" "github.com/stretchr/testify/assert" ) @@ -72,6 +73,21 @@ func TestDraftsCommand(t *testing.T) { assert.True(t, cmdMap[expected], "Missing expected subcommand: %s", expected) } }) + + t.Run("list_has_id_flag", func(t *testing.T) { + // The --id flag lets users retrieve full draft IDs (needed for + // show/send/delete) without resorting to --json. It matches the + // convention used by `email list`, `folders`, and `threads`. + var listCmd *cobra.Command + for _, sub := range cmd.Commands() { + if sub.Name() == "list" { + listCmd = sub + break + } + } + assert.NotNil(t, listCmd, "drafts list subcommand should exist") + assert.NotNil(t, listCmd.Flags().Lookup("id"), "drafts list should expose an --id flag") + }) } func TestHelperFunctions(t *testing.T) {