Skip to content

Absorb additive activity-timeline event fields (bc3 #11629) #424

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jeremy wants to merge 1 commit into
base: tool-visible-to-clients
Choose a base branch
from
Open

Absorb additive activity-timeline event fields (bc3 #11629) #424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions conformance/tests/live-my-surface.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,17 @@
[
{
"mode": "live",
"name": "GetProgressReport account feed validates against the timeline schema",
"description": "Exercises the account activity feed and validates each timeline event against the schema, including the additive kind/avatars_sample/data/attachments fields. Live-dormant: validates statically until credentials are provisioned.",
"operation": "GetProgressReport",
"method": "GET",
"path": "/reports/progress.json",
"tags": ["live", "read-only"],
"liveAssertions": [
{ "type": "liveCallSucceeds" },
{ "type": "liveSchemaValidate" }
]
},
{
"mode": "live",
"name": "ListProjects returns a list and validates against schema",
Expand Down
170 changes: 158 additions & 12 deletions go/pkg/basecamp/timeline.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,25 +8,94 @@ import (
"time"

"github.com/basecamp/basecamp-sdk/go/pkg/generated"
"github.com/basecamp/basecamp-sdk/go/pkg/types"
)

// DefaultTimelineLimit is the default number of timeline events to return when no limit is specified.
const DefaultTimelineLimit = 100

// TimelineEvent represents an activity event in the timeline.
type TimelineEvent struct {
ID int64 `json:"id"`
CreatedAt time.Time `json:"created_at"`
Kind string `json:"kind"`
ParentRecordingID int64 `json:"parent_recording_id"`
URL string `json:"url"`
AppURL string `json:"app_url"`
Creator *Person `json:"creator,omitempty"`
Action string `json:"action"`
Target string `json:"target"`
Title string `json:"title"`
SummaryExcerpt string `json:"summary_excerpt"`
Bucket *Bucket `json:"bucket,omitempty"`
ID int64 `json:"id"`
CreatedAt time.Time `json:"created_at"`
// Kind is an open, non-exhaustive vocabulary (BC3 adds new kinds over time);
// treat unrecognized values as valid. Common values include message_created,
// todo_created, todo_completed, upload_created, schedule_entry_created,
// chat_transcript_rollup, dock_created, and project_access_changed.
Kind string `json:"kind"`
ParentRecordingID int64 `json:"parent_recording_id"`
URL string `json:"url"`
AppURL string `json:"app_url"`
Creator *Person `json:"creator,omitempty"`
Action string `json:"action"`
Target string `json:"target"`
Title string `json:"title"`
SummaryExcerpt string `json:"summary_excerpt"`
// AvatarsSample holds avatar URLs of participants, populated for
// chat_transcript_rollup events and empty otherwise.
AvatarsSample []string `json:"avatars_sample,omitempty"`
Bucket *Bucket `json:"bucket,omitempty"`
// Data carries schedule-entry timing, present only for schedule_entry_created
// and schedule_entry_rescheduled events.
Data *TimelineEventData `json:"data,omitempty"`
// Attachments holds files attached to the event's recording. It is
// heterogeneous: an upload-kind recording contributes its full Upload shape,
// other recordings contribute rich-text attachment/blob partials. Each
// element is an optional-field superset; only the fields of the variant it
// represents are populated.
Attachments []TimelineAttachment `json:"attachments,omitempty"`
}

// TimelineEventData carries schedule-entry timing for schedule_entry_* events.
// StartsAt and EndsAt are date-or-timestamp (types.FlexibleTime): a full ISO
// 8601 timestamp for timed entries, or a bare date when AllDay is true.
type TimelineEventData struct {
AllDay bool `json:"all_day"`
StartsAt types.FlexibleTime `json:"starts_at"`
EndsAt types.FlexibleTime `json:"ends_at"`
}

// TimelineAttachment is a single timeline-event attachment: an optional-field
// superset over two wire variants — a full Upload recording (upload-kind
// recordings) and a rich-text attachment/blob partial (all other recordings).
// Only the fields of the variant an instance represents are populated.
type TimelineAttachment struct {
ID int64 `json:"id,omitempty"`

// Shared by both variants.
ContentType string `json:"content_type,omitempty"`
ByteSize int64 `json:"byte_size,omitempty"`
Filename string `json:"filename,omitempty"`
DownloadURL string `json:"download_url,omitempty"`
// Width and Height are null for non-image blobs and may be float-spelled
// (1024.0) on the wire; narrowed to *int32 here (nil when absent/null).
Width *int32 `json:"width,omitempty"`
Height *int32 `json:"height,omitempty"`

// Upload-recording variant. CreatedAt/UpdatedAt/VisibleToClients are
// pointers so an absent field (this instance is the attachment variant)
// stays nil and round-trips as omitted rather than a fabricated zero
// timestamp or a dropped explicit false.
Type string `json:"type,omitempty"`
Title string `json:"title,omitempty"`
Status string `json:"status,omitempty"`
CreatedAt *time.Time `json:"created_at,omitempty"`
UpdatedAt *time.Time `json:"updated_at,omitempty"`
RecordingURL string `json:"url,omitempty"`
AppURL string `json:"app_url,omitempty"`
AppDownloadURL string `json:"app_download_url,omitempty"`
VisibleToClients *bool `json:"visible_to_clients,omitempty"`

// Rich-text attachment/blob variant. Previewable is a pointer for the same
// presence-faithful reason as VisibleToClients.
AttachableSGID string `json:"attachable_sgid,omitempty"`
SGID string `json:"sgid,omitempty"`
StatusURL string `json:"status_url,omitempty"`
Caption string `json:"caption,omitempty"`
Key string `json:"key,omitempty"`
Previewable *bool `json:"previewable,omitempty"`
PreviewURL string `json:"preview_url,omitempty"`
ThumbnailURL string `json:"thumbnail_url,omitempty"`
}

// TimelineListOptions specifies options for listing timeline events.
Expand Down Expand Up @@ -399,5 +468,82 @@ func timelineEventFromGenerated(ge generated.TimelineEvent) TimelineEvent {
}
}

if ge.AvatarsSample != nil {
e.AvatarsSample = append([]string(nil), ge.AvatarsSample...)
}

// data is present only on schedule_entry_* events. Detect a populated
// payload via the timing fields so an absent object stays nil.
if !ge.Data.StartsAt.IsZero() || !ge.Data.EndsAt.IsZero() || ge.Data.AllDay {
e.Data = &TimelineEventData{
AllDay: ge.Data.AllDay,
StartsAt: ge.Data.StartsAt,
EndsAt: ge.Data.EndsAt,
}
}

if ge.Attachments != nil {
e.Attachments = make([]TimelineAttachment, 0, len(ge.Attachments))
for _, ga := range ge.Attachments {
e.Attachments = append(e.Attachments, timelineAttachmentFromGenerated(ga))
}
}

return e
}

// UnmarshalJSON routes decoding through the generated TimelineAttachment so the
// public struct handles the float-encoded integers (1024.0) and null dimensions
// the BC3 API emits for width/height. Mirrors RichTextAttachment.UnmarshalJSON.
// Because TimelineEvent has no custom UnmarshalJSON, its Attachments elements
// invoke this automatically on direct decode.
func (a *TimelineAttachment) UnmarshalJSON(data []byte) error {
var ga generated.TimelineAttachment
if err := json.Unmarshal(data, &ga); err != nil {
return err
}
*a = timelineAttachmentFromGenerated(ga)
return nil
}

// timelineAttachmentFromGenerated converts a generated TimelineAttachment (the
// optional-field superset) to the clean public type. Width and Height are
// optional/nullable *types.FlexInt in the generated type; a nil pointer leaves
// the public *int32 nil, and a present value is narrowed to int32.
func timelineAttachmentFromGenerated(ga generated.TimelineAttachment) TimelineAttachment {
a := TimelineAttachment{
ContentType: ga.ContentType,
ByteSize: ga.ByteSize,
Filename: ga.Filename,
DownloadURL: ga.DownloadUrl,
Type: ga.Type,
Title: ga.Title,
Status: ga.Status,
CreatedAt: ga.CreatedAt,
UpdatedAt: ga.UpdatedAt,
RecordingURL: ga.Url,
AppURL: ga.AppUrl,
AppDownloadURL: ga.AppDownloadUrl,
VisibleToClients: ga.VisibleToClients,
AttachableSGID: ga.AttachableSgid,
SGID: ga.Sgid,
StatusURL: ga.StatusUrl,
Caption: ga.Caption,
Key: ga.Key,
Previewable: ga.Previewable,
PreviewURL: ga.PreviewUrl,
ThumbnailURL: ga.ThumbnailUrl,
}
if ga.Id != nil {
a.ID = *ga.Id
}
if ga.Width != nil {
w := int32(*ga.Width)
a.Width = &w
}
if ga.Height != nil {
h := int32(*ga.Height)
a.Height = &h
}
return a
}
179 changes: 179 additions & 0 deletions go/pkg/basecamp/timeline_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,185 @@ func TestTimelineEvent_Unmarshal(t *testing.T) {
}
}

// TestTimelineEvent_AdditiveFields exercises runtime decode of the additive
// fields: a non-empty avatars_sample, the schedule-entry data payload with a
// date-only (all_day) starts_at/ends_at, and BOTH heterogeneous attachment
// variants — a full Upload recording and a rich-text attachment/blob partial —
// in a single non-empty array. Empty arrays / shape-only tests do not prove

@cubic-dev-ai cubic-dev-ai Bot Jul 25, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The additive-field test does not actually decode both attachment variants from one heterogeneous attachments array: the upload and blob cases are in separate event arrays. Combining both objects under one event would make the test match the claimed coverage and catch slice-level polymorphic regressions.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At go/pkg/basecamp/timeline_test.go, line 89:

<comment>The additive-field test does not actually decode both attachment variants from one heterogeneous `attachments` array: the upload and blob cases are in separate event arrays. Combining both objects under one event would make the test match the claimed coverage and catch slice-level polymorphic regressions.</comment>

<file context>
@@ -82,6 +82,145 @@ func TestTimelineEvent_Unmarshal(t *testing.T) {
+// fields: a non-empty avatars_sample, the schedule-entry data payload with a
+// date-only (all_day) starts_at/ends_at, and BOTH heterogeneous attachment
+// variants — a full Upload recording and a rich-text attachment/blob partial —
+// in a single non-empty array. Empty arrays / shape-only tests do not prove
+// polymorphic decode, so both variants carry real per-variant fields here.
+func TestTimelineEvent_AdditiveFields(t *testing.T) {
</file context>

// polymorphic decode, so both variants carry real per-variant fields here.
func TestTimelineEvent_AdditiveFields(t *testing.T) {
data := `[
{
"id": 1,
"created_at": "2024-03-15T10:30:00Z",
"kind": "chat_transcript_rollup",
"avatars_sample": [
"https://3.basecampapi.com/1/people/aaa/avatar",
"https://3.basecampapi.com/1/people/bbb/avatar"
]
},
{
"id": 2,
"created_at": "2024-03-15T10:31:00Z",
"kind": "schedule_entry_created",
"avatars_sample": [],
"data": {
"all_day": true,
"starts_at": "2025-10-30",
"ends_at": "2025-10-30"
}
},
{
"id": 3,
"created_at": "2024-03-15T10:32:00Z",
"kind": "upload_created",
"avatars_sample": [],
"attachments": [
{
"id": 900,
"type": "Upload",
"status": "active",
"visible_to_clients": false,
"title": "Diagram",
"filename": "diagram.png",
"content_type": "image/png",
"byte_size": 20480,
"width": 1024.0,
"height": 768.0,
"url": "https://3.basecampapi.com/1/buckets/2/uploads/900.json",
"app_url": "https://3.basecamp.com/1/buckets/2/uploads/900",
"download_url": "https://3.basecampapi.com/1/buckets/2/uploads/900/download/diagram.png",
"app_download_url": "https://3.basecamp.com/1/buckets/2/uploads/900/download"
}
]
},
{
"id": 4,
"created_at": "2024-03-15T10:33:00Z",
"kind": "comment_created",
"avatars_sample": [],
"attachments": [
{
"id": 500,
"attachable_sgid": "sgid-attachable-500",
"sgid": "sgid-500",
"status_url": "https://3.basecampapi.com/1/attachments/sgid-500/status.json",
"caption": "See attached",
"filename": "notes.pdf",
"content_type": "application/pdf",
"byte_size": 4096,
"key": "blobkey500",
"width": null,
"height": null,
"previewable": true,
"download_url": "https://3.basecampapi.com/1/blobs/blobkey500/download/notes.pdf",
"preview_url": "https://3.basecampapi.com/1/blobs/blobkey500/previews/full",
"thumbnail_url": "https://3.basecampapi.com/1/blobs/blobkey500/previews/card"
}
]
}
]`

var events []TimelineEvent
if err := json.Unmarshal([]byte(data), &events); err != nil {
t.Fatalf("failed to unmarshal timeline events: %v", err)
}
if len(events) != 4 {
t.Fatalf("expected 4 events, got %d", len(events))
}

// avatars_sample non-empty
if got := events[0].AvatarsSample; len(got) != 2 || got[0] == "" {
t.Errorf("expected 2 avatar URLs, got %v", got)
}

// data with date-only all-day timing (FlexibleTime accepts date-only)
ev := events[1]
if ev.Data == nil {
t.Fatal("expected data on schedule_entry_created event")
}
if !ev.Data.AllDay {
t.Error("expected all_day true")
}
wantDate := time.Date(2025, 10, 30, 0, 0, 0, 0, time.UTC)
if !ev.Data.StartsAt.Equal(wantDate) {
t.Errorf("expected StartsAt %v, got %v", wantDate, ev.Data.StartsAt)
}
if !ev.Data.EndsAt.Equal(wantDate) {
t.Errorf("expected EndsAt %v, got %v", wantDate, ev.Data.EndsAt)
}

// Upload-recording attachment variant
up := events[2].Attachments
if len(up) != 1 {
t.Fatalf("expected 1 upload attachment, got %d", len(up))
}
if up[0].Type != "Upload" || up[0].Filename != "diagram.png" || up[0].AppDownloadURL == "" {
t.Errorf("upload variant fields not decoded: %+v", up[0])
}
if up[0].Width == nil || *up[0].Width != 1024 {
t.Errorf("expected float-spelled width 1024, got %v", up[0].Width)
}
if up[0].AttachableSGID != "" {
t.Errorf("upload variant should not carry attachable_sgid, got %q", up[0].AttachableSGID)
}

// Rich-text attachment/blob variant
att := events[3].Attachments
if len(att) != 1 {
t.Fatalf("expected 1 blob attachment, got %d", len(att))
}
if att[0].AttachableSGID != "sgid-attachable-500" || att[0].Caption != "See attached" || att[0].Key != "blobkey500" {
t.Errorf("attachment variant fields not decoded: %+v", att[0])
}
if att[0].Previewable == nil || !*att[0].Previewable || att[0].ThumbnailURL == "" {
t.Errorf("attachment variant preview fields not decoded: %+v", att[0])
}
if att[0].Width != nil || att[0].Height != nil {
t.Errorf("expected nil width/height for non-image blob, got w=%v h=%v", att[0].Width, att[0].Height)
}
// Presence-faithful: the attachment variant carries no upload timestamps or
// visibility, so those pointers stay nil (and re-marshal omits them).
if att[0].CreatedAt != nil || att[0].VisibleToClients != nil {
t.Errorf("expected nil upload-variant fields on attachment, got created_at=%v visible=%v", att[0].CreatedAt, att[0].VisibleToClients)
}
}

// TestTimelineAttachment_PresenceFaithfulRoundTrip verifies that re-marshaling a
// decoded attachment-variant superset omits the absent upload-variant fields
// (no fabricated zero timestamp, no dropped explicit false) — the reason the
// optional timestamps/booleans are pointers.
func TestTimelineAttachment_PresenceFaithfulRoundTrip(t *testing.T) {
// Attachment variant: explicit previewable:false, no created_at/updated_at,
// no visible_to_clients.
src := `{"id":500,"attachable_sgid":"sgid-500","filename":"notes.pdf","previewable":false}`
var a TimelineAttachment
if err := json.Unmarshal([]byte(src), &a); err != nil {
t.Fatalf("unmarshal: %v", err)
}
if a.Previewable == nil || *a.Previewable != false {
t.Fatalf("expected explicit previewable=false preserved, got %v", a.Previewable)
}
out, err := json.Marshal(a)
if err != nil {
t.Fatalf("marshal: %v", err)
}
var round map[string]any
if err := json.Unmarshal(out, &round); err != nil {
t.Fatalf("re-unmarshal: %v", err)
}
// Absent upload-variant fields must NOT be fabricated on re-marshal.
for _, k := range []string{"created_at", "updated_at", "visible_to_clients"} {
if _, present := round[k]; present {
t.Errorf("re-marshal fabricated absent field %q: %s", k, out)
}
}
// Explicit false must survive the round trip (not dropped by omitempty).
if v, ok := round["previewable"]; !ok || v != false {
t.Errorf("re-marshal dropped explicit previewable=false: %s", out)
}
}

// wrappedPaginationHandler serves wrapped {person, events} responses with Link headers.
type wrappedPaginationHandler struct {
pageSize int
Expand Down
Loading
Loading