From d7103402a16e643933f9a4710b0b77ed3e3bf35e Mon Sep 17 00:00:00 2001 From: Omer C <639682+omercnet@users.noreply.github.com> Date: Mon, 29 Dec 2025 13:43:24 +0200 Subject: [PATCH 1/2] feat: add --org flag to filter contributions by organization Add support for generating skylines scoped to a specific GitHub organization. This is useful for teams who want to visualize contributions to their org's repos specifically, rather than a user's entire GitHub activity. Implementation details: - New --org flag accepts organization login name - Uses contributionsByRepository GraphQL queries (commits, PRs, issues, reviews) - Queries each quarter separately to work around GitHub's 100 repo limit - Filters results client-side by repository owner - Aggregates into the same contribution grid format for STL generation Example usage: gh skyline --user employee --org mycompany --year 2024 --- cmd/root.go | 4 +- cmd/skyline/skyline.go | 94 +++++++++++- cmd/skyline/skyline_test.go | 235 +++++++++++++++++++++++++++++- internal/github/client.go | 99 +++++++++++++ internal/github/client_test.go | 69 +++++++++ internal/testutil/mocks/github.go | 82 ++++++++++- internal/types/types.go | 59 ++++++++ 7 files changed, 629 insertions(+), 13 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 38d2b97..8045adf 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -21,6 +21,7 @@ import ( var ( yearRange string user string + org string // filter contributions to specific organization full bool debug bool web bool @@ -74,6 +75,7 @@ func initFlags() { flags.BoolVarP(&web, "web", "w", false, "Open GitHub profile (authenticated or specified user).") flags.BoolVarP(&artOnly, "art-only", "a", false, "Generate only ASCII preview") flags.StringVarP(&output, "output", "o", "", "Output file path (optional)") + flags.StringVar(&org, "org", "", "Filter contributions to a specific organization") } // executeRootCmd is the main execution function for the root command. @@ -105,7 +107,7 @@ func handleSkylineCommand(_ *cobra.Command, _ []string) error { return fmt.Errorf("invalid year range: %v", err) } - return skyline.GenerateSkyline(startYear, endYear, user, full, output, artOnly) + return skyline.GenerateSkyline(startYear, endYear, user, org, full, output, artOnly) } // Browser interface matches browser.Browser functionality. diff --git a/cmd/skyline/skyline.go b/cmd/skyline/skyline.go index de62e5e..1101977 100644 --- a/cmd/skyline/skyline.go +++ b/cmd/skyline/skyline.go @@ -21,10 +21,11 @@ type GitHubClientInterface interface { GetAuthenticatedUser() (string, error) GetUserJoinYear(username string) (int, error) FetchContributions(username string, year int) (*types.ContributionsResponse, error) + FetchOrgContributions(username string, org string, year int) (*types.OrgContributionsResponse, error) } // GenerateSkyline creates a 3D model with ASCII art preview of GitHub contributions for the specified year range, or "full lifetime" of the user -func GenerateSkyline(startYear, endYear int, targetUser string, full bool, output string, artOnly bool) error { +func GenerateSkyline(startYear, endYear int, targetUser string, org string, full bool, output string, artOnly bool) error { log := logger.GetLogger() client, err := github.InitializeGitHubClient() @@ -54,7 +55,13 @@ func GenerateSkyline(startYear, endYear int, targetUser string, full bool, outpu var allContributions [][][]types.ContributionDay for year := startYear; year <= endYear; year++ { - contributions, err := fetchContributionData(client, targetUser, year) + var contributions [][]types.ContributionDay + var err error + if org != "" { + contributions, err = fetchOrgContributionData(client, targetUser, org, year) + } else { + contributions, err = fetchContributionData(client, targetUser, year) + } if err != nil { return err } @@ -111,7 +118,6 @@ func fetchContributionData(client *github.Client, username string, year int) ([] return nil, fmt.Errorf("failed to fetch contributions: %w", err) } - // Convert weeks data to 2D array for STL generation weeks := response.User.ContributionsCollection.ContributionCalendar.Weeks contributionGrid := make([][]types.ContributionDay, len(weeks)) for i, week := range weeks { @@ -120,3 +126,85 @@ func fetchContributionData(client *github.Client, username string, year int) ([] return contributionGrid, nil } + +// fetchOrgContributionData retrieves contributions filtered to a specific organization. +func fetchOrgContributionData(client *github.Client, username string, org string, year int) ([][]types.ContributionDay, error) { + response, err := client.FetchOrgContributions(username, org, year) + if err != nil { + return nil, fmt.Errorf("failed to fetch org contributions: %w", err) + } + + dailyCounts := make(map[string]int) + + for _, repo := range response.User.ContributionsCollection.CommitContributionsByRepository { + if strings.EqualFold(repo.Repository.Owner.Login, org) { + for _, node := range repo.Contributions.Nodes { + date := node.OccurredAt[:10] + dailyCounts[date]++ + } + } + } + for _, repo := range response.User.ContributionsCollection.IssueContributionsByRepository { + if strings.EqualFold(repo.Repository.Owner.Login, org) { + for _, node := range repo.Contributions.Nodes { + date := node.OccurredAt[:10] + dailyCounts[date]++ + } + } + } + for _, repo := range response.User.ContributionsCollection.PullRequestContributionsByRepository { + if strings.EqualFold(repo.Repository.Owner.Login, org) { + for _, node := range repo.Contributions.Nodes { + date := node.OccurredAt[:10] + dailyCounts[date]++ + } + } + } + for _, repo := range response.User.ContributionsCollection.PullRequestReviewContributionsByRepository { + if strings.EqualFold(repo.Repository.Owner.Login, org) { + for _, node := range repo.Contributions.Nodes { + date := node.OccurredAt[:10] + dailyCounts[date]++ + } + } + } + + return buildContributionGrid(year, dailyCounts), nil +} + +func buildContributionGrid(year int, dailyCounts map[string]int) [][]types.ContributionDay { + startDate := time.Date(year, 1, 1, 0, 0, 0, 0, time.UTC) + endDate := time.Date(year, 12, 31, 0, 0, 0, 0, time.UTC) + + for startDate.Weekday() != time.Sunday { + startDate = startDate.AddDate(0, 0, -1) + } + + var weeks [][]types.ContributionDay + var currentWeek []types.ContributionDay + + for d := startDate; !d.After(endDate) || len(currentWeek) > 0; d = d.AddDate(0, 0, 1) { + dateStr := d.Format("2006-01-02") + count := dailyCounts[dateStr] + + if d.Year() == year || (d.Year() == year-1 && d.After(startDate.AddDate(0, 0, -1))) { + currentWeek = append(currentWeek, types.ContributionDay{ + Date: dateStr, + ContributionCount: count, + }) + } + + if d.Weekday() == time.Saturday || d.Equal(endDate) { + if len(currentWeek) > 0 { + weeks = append(weeks, currentWeek) + currentWeek = nil + } + } + + if d.After(endDate) && d.Weekday() == time.Saturday { + break + } + } + + return weeks +} diff --git a/cmd/skyline/skyline_test.go b/cmd/skyline/skyline_test.go index 05970a5..7bb22eb 100644 --- a/cmd/skyline/skyline_test.go +++ b/cmd/skyline/skyline_test.go @@ -6,6 +6,7 @@ import ( "github.com/github/gh-skyline/internal/github" "github.com/github/gh-skyline/internal/testutil/fixtures" "github.com/github/gh-skyline/internal/testutil/mocks" + "github.com/github/gh-skyline/internal/types" ) func TestGenerateSkyline(t *testing.T) { @@ -67,15 +68,245 @@ func TestGenerateSkyline(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Create a closure that returns our mock client github.InitializeGitHubClient = func() (*github.Client, error) { return github.NewClient(tt.mockClient), nil } - err := GenerateSkyline(tt.startYear, tt.endYear, tt.targetUser, tt.full, "", false) + err := GenerateSkyline(tt.startYear, tt.endYear, tt.targetUser, "", tt.full, "", false) if (err != nil) != tt.wantErr { t.Errorf("GenerateSkyline() error = %v, wantErr %v", err, tt.wantErr) } }) } } + +func TestGenerateSkylineWithOrg(t *testing.T) { + originalInit := github.InitializeGitHubClient + defer func() { + github.InitializeGitHubClient = originalInit + }() + + tests := []struct { + name string + startYear int + endYear int + targetUser string + org string + mockClient *mocks.MockGitHubClient + wantErr bool + }{ + { + name: "org filter single year", + startYear: 2024, + endYear: 2024, + targetUser: "testuser", + org: "testorg", + mockClient: &mocks.MockGitHubClient{ + Username: "testuser", + JoinYear: 2020, + }, + wantErr: false, + }, + { + name: "org filter year range", + startYear: 2023, + endYear: 2024, + targetUser: "testuser", + org: "testorg", + mockClient: &mocks.MockGitHubClient{ + Username: "testuser", + JoinYear: 2020, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + github.InitializeGitHubClient = func() (*github.Client, error) { + return github.NewClient(tt.mockClient), nil + } + + err := GenerateSkyline(tt.startYear, tt.endYear, tt.targetUser, tt.org, false, "", true) + if (err != nil) != tt.wantErr { + t.Errorf("GenerateSkyline() error = %v, wantErr %v", err, tt.wantErr) + } + }) + } +} + +func TestBuildContributionGrid(t *testing.T) { + tests := []struct { + name string + year int + dailyCounts map[string]int + wantWeeks int + }{ + { + name: "empty contributions", + year: 2024, + dailyCounts: map[string]int{}, + wantWeeks: 53, + }, + { + name: "single day contribution", + year: 2024, + dailyCounts: map[string]int{ + "2024-06-15": 5, + }, + wantWeeks: 53, + }, + { + name: "multiple days", + year: 2024, + dailyCounts: map[string]int{ + "2024-01-15": 3, + "2024-06-15": 5, + "2024-12-25": 1, + }, + wantWeeks: 53, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + grid := buildContributionGrid(tt.year, tt.dailyCounts) + + if len(grid) < 52 || len(grid) > 54 { + t.Errorf("expected ~53 weeks, got %d", len(grid)) + } + + for date, expectedCount := range tt.dailyCounts { + found := false + for _, week := range grid { + for _, day := range week { + if day.Date == date { + found = true + if day.ContributionCount != expectedCount { + t.Errorf("date %s: expected count %d, got %d", date, expectedCount, day.ContributionCount) + } + } + } + } + if !found { + t.Errorf("date %s not found in grid", date) + } + } + }) + } +} + +func TestFetchOrgContributionDataFiltering(t *testing.T) { + originalInit := github.InitializeGitHubClient + defer func() { + github.InitializeGitHubClient = originalInit + }() + + mockOrgData := &types.OrgContributionsResponse{} + mockOrgData.User.Login = "testuser" + mockOrgData.User.ContributionsCollection.CommitContributionsByRepository = []struct { + Repository struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Contributions struct { + TotalCount int `json:"totalCount"` + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + } `json:"contributions"` + }{ + { + Repository: struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + }{ + Name: "included-repo", + Owner: struct { + Login string `json:"login"` + }{Login: "targetorg"}, + }, + Contributions: struct { + TotalCount int `json:"totalCount"` + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + }{ + TotalCount: 2, + Nodes: []struct { + OccurredAt string `json:"occurredAt"` + }{ + {OccurredAt: "2024-03-15T10:00:00Z"}, + {OccurredAt: "2024-03-15T14:00:00Z"}, + }, + }, + }, + { + Repository: struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + }{ + Name: "excluded-repo", + Owner: struct { + Login string `json:"login"` + }{Login: "otherorg"}, + }, + Contributions: struct { + TotalCount int `json:"totalCount"` + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + }{ + TotalCount: 5, + Nodes: []struct { + OccurredAt string `json:"occurredAt"` + }{ + {OccurredAt: "2024-03-16T10:00:00Z"}, + }, + }, + }, + } + + mockClient := &mocks.MockGitHubClient{ + Username: "testuser", + MockOrgData: mockOrgData, + } + + github.InitializeGitHubClient = func() (*github.Client, error) { + return github.NewClient(mockClient), nil + } + + client, _ := github.InitializeGitHubClient() + grid, err := fetchOrgContributionData(client, "testuser", "targetorg", 2024) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + march15Count := 0 + march16Count := 0 + for _, week := range grid { + for _, day := range week { + if day.Date == "2024-03-15" { + march15Count = day.ContributionCount + } + if day.Date == "2024-03-16" { + march16Count = day.ContributionCount + } + } + } + + // With quarterly queries, the mock returns data 4 times (once per quarter), so counts are 4x + // The key test is that targetorg contributions are included and otherorg contributions are filtered + if march15Count == 0 { + t.Errorf("expected contributions on 2024-03-15 (targetorg), got %d", march15Count) + } + if march16Count != 0 { + t.Errorf("expected 0 contributions on 2024-03-16 (otherorg should be filtered), got %d", march16Count) + } +} diff --git a/internal/github/client.go b/internal/github/client.go index 8694f44..1881b46 100644 --- a/internal/github/client.go +++ b/internal/github/client.go @@ -107,6 +107,105 @@ func (c *Client) FetchContributions(username string, year int) (*types.Contribut return &response, nil } +// FetchOrgContributions retrieves contribution data filtered by organization for a given username and year. +// +// GitHub's GraphQL API limits contributionsByRepository queries to 100 repositories per request. +// To work around this, we query each quarter separately and merge the results. This allows us to +// capture contributions across up to 400 repos per year (100 per quarter), which covers virtually +// all real-world usage patterns. The quarterly approach also reduces the chance of hitting the +// per-repo contribution limit (100 nodes) since contributions are spread across shorter time ranges. +func (c *Client) FetchOrgContributions(username string, org string, year int) (*types.OrgContributionsResponse, error) { + if username == "" { + return nil, errors.New(errors.ValidationError, "username cannot be empty", nil) + } + if org == "" { + return nil, errors.New(errors.ValidationError, "org cannot be empty", nil) + } + if year < 2008 { + return nil, errors.New(errors.ValidationError, "year cannot be before GitHub's launch (2008)", nil) + } + + // Query each quarter separately to work around the 100 repo limit per query. + // Each query can return up to 100 unique repos, so quarterly queries give us up to 400 repos/year. + quarters := []struct { + start string + end string + }{ + {fmt.Sprintf("%d-01-01T00:00:00Z", year), fmt.Sprintf("%d-03-31T23:59:59Z", year)}, + {fmt.Sprintf("%d-04-01T00:00:00Z", year), fmt.Sprintf("%d-06-30T23:59:59Z", year)}, + {fmt.Sprintf("%d-07-01T00:00:00Z", year), fmt.Sprintf("%d-09-30T23:59:59Z", year)}, + {fmt.Sprintf("%d-10-01T00:00:00Z", year), fmt.Sprintf("%d-12-31T23:59:59Z", year)}, + } + + query := ` + query ContributionsByRepo($username: String!, $from: DateTime!, $to: DateTime!) { + user(login: $username) { + login + contributionsCollection(from: $from, to: $to) { + commitContributionsByRepository(maxRepositories: 100) { + repository { + name + owner { login } + } + contributions(first: 100) { + totalCount + nodes { occurredAt } + } + } + issueContributionsByRepository(maxRepositories: 100) { + repository { owner { login } } + contributions(first: 100) { nodes { occurredAt } } + } + pullRequestContributionsByRepository(maxRepositories: 100) { + repository { owner { login } } + contributions(first: 100) { nodes { occurredAt } } + } + pullRequestReviewContributionsByRepository(maxRepositories: 100) { + repository { owner { login } } + contributions(first: 100) { nodes { occurredAt } } + } + } + } + }` + + var merged types.OrgContributionsResponse + + for _, q := range quarters { + variables := map[string]interface{}{ + "username": username, + "from": q.start, + "to": q.end, + } + + var response types.OrgContributionsResponse + err := c.api.Do(query, variables, &response) + if err != nil { + return nil, errors.New(errors.NetworkError, "failed to fetch org contributions", err) + } + + if response.User.Login == "" { + return nil, errors.New(errors.ValidationError, "received empty username from GitHub API", nil) + } + + // Merge quarterly results into the combined response + merged.User.Login = response.User.Login + merged.User.ContributionsCollection.CommitContributionsByRepository = append( + merged.User.ContributionsCollection.CommitContributionsByRepository, + response.User.ContributionsCollection.CommitContributionsByRepository...) + merged.User.ContributionsCollection.IssueContributionsByRepository = append( + merged.User.ContributionsCollection.IssueContributionsByRepository, + response.User.ContributionsCollection.IssueContributionsByRepository...) + merged.User.ContributionsCollection.PullRequestContributionsByRepository = append( + merged.User.ContributionsCollection.PullRequestContributionsByRepository, + response.User.ContributionsCollection.PullRequestContributionsByRepository...) + merged.User.ContributionsCollection.PullRequestReviewContributionsByRepository = append( + merged.User.ContributionsCollection.PullRequestReviewContributionsByRepository, + response.User.ContributionsCollection.PullRequestReviewContributionsByRepository...) + } + + return &merged, nil +} + // GetUserJoinYear fetches the year a user joined GitHub using the GitHub API. func (c *Client) GetUserJoinYear(username string) (int, error) { if username == "" { diff --git a/internal/github/client_test.go b/internal/github/client_test.go index c83ede9..eb0dd97 100644 --- a/internal/github/client_test.go +++ b/internal/github/client_test.go @@ -100,6 +100,75 @@ func TestGetUserJoinYear(t *testing.T) { } } +func TestFetchOrgContributions(t *testing.T) { + tests := []struct { + name string + username string + org string + year int + mockError error + expectedError bool + }{ + { + name: "successful response", + username: "testuser", + org: "testorg", + year: 2024, + expectedError: false, + }, + { + name: "empty username", + username: "", + org: "testorg", + year: 2024, + expectedError: true, + }, + { + name: "empty org", + username: "testuser", + org: "", + year: 2024, + expectedError: true, + }, + { + name: "invalid year", + username: "testuser", + org: "testorg", + year: 2007, + expectedError: true, + }, + { + name: "network error", + username: "testuser", + org: "testorg", + year: 2024, + mockError: errors.New(errors.NetworkError, "network error", nil), + expectedError: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := NewClient(&mocks.MockGitHubClient{ + Username: tt.username, + Err: tt.mockError, + }) + + resp, err := client.FetchOrgContributions(tt.username, tt.org, tt.year) + if (err != nil) != tt.expectedError { + t.Errorf("expected error: %v, got: %v", tt.expectedError, err) + } + if !tt.expectedError { + if resp == nil { + t.Error("expected response but got nil") + } else if resp.User.Login != tt.username { + t.Errorf("expected user %s, got %s", tt.username, resp.User.Login) + } + } + }) + } +} + func TestFetchContributions(t *testing.T) { mockContributions := &types.ContributionsResponse{ User: struct { diff --git a/internal/testutil/mocks/github.go b/internal/testutil/mocks/github.go index 8d6f906..67058d1 100644 --- a/internal/testutil/mocks/github.go +++ b/internal/testutil/mocks/github.go @@ -11,11 +11,12 @@ import ( // MockGitHubClient implements both GitHubClientInterface and APIClient interfaces type MockGitHubClient struct { - Username string - JoinYear int - MockData *types.ContributionsResponse - Response interface{} // Generic response field for testing - Err error // Error to return if needed + Username string + JoinYear int + MockData *types.ContributionsResponse + MockOrgData *types.OrgContributionsResponse + Response interface{} + Err error } // GetAuthenticatedUser implements GitHubClientInterface @@ -45,10 +46,72 @@ func (m *MockGitHubClient) FetchContributions(username string, year int) (*types if m.Err != nil { return nil, m.Err } - // Always return generated mock data with valid contributions return fixtures.GenerateContributionsResponse(username, year), nil } +// FetchOrgContributions implements GitHubClientInterface +func (m *MockGitHubClient) FetchOrgContributions(username string, _ string, _ int) (*types.OrgContributionsResponse, error) { + if m.Err != nil { + return nil, m.Err + } + if m.MockOrgData != nil { + return m.MockOrgData, nil + } + return GenerateOrgContributionsResponse(username, "testorg"), nil +} + +// GenerateOrgContributionsResponse creates mock org contribution data for testing +func GenerateOrgContributionsResponse(username, org string) *types.OrgContributionsResponse { + resp := &types.OrgContributionsResponse{} + resp.User.Login = username + + resp.User.ContributionsCollection.CommitContributionsByRepository = []struct { + Repository struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Contributions struct { + TotalCount int `json:"totalCount"` + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + } `json:"contributions"` + }{ + { + Repository: struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + }{ + Name: "test-repo", + Owner: struct { + Login string `json:"login"` + }{Login: org}, + }, + Contributions: struct { + TotalCount int `json:"totalCount"` + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + }{ + TotalCount: 3, + Nodes: []struct { + OccurredAt string `json:"occurredAt"` + }{ + {OccurredAt: "2024-01-15T10:00:00Z"}, + {OccurredAt: "2024-01-15T14:00:00Z"}, + {OccurredAt: "2024-02-20T09:00:00Z"}, + }, + }, + }, + } + + return resp +} + // Do implements APIClient func (m *MockGitHubClient) Do(_ string, _ map[string]interface{}, response interface{}) error { if m.Err != nil { @@ -71,9 +134,14 @@ func (m *MockGitHubClient) Do(_ string, _ map[string]interface{}, response inter v.User.CreatedAt = time.Date(m.JoinYear, 1, 1, 0, 0, 0, 0, time.UTC) } case *types.ContributionsResponse: - // Always use generated mock data instead of empty response mockResp := fixtures.GenerateContributionsResponse(m.Username, time.Now().Year()) *v = *mockResp + case *types.OrgContributionsResponse: + if m.MockOrgData != nil { + *v = *m.MockOrgData + } else { + *v = *GenerateOrgContributionsResponse(m.Username, "testorg") + } } return nil } diff --git a/internal/types/types.go b/internal/types/types.go index 909c3e4..2b8d419 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -51,6 +51,65 @@ type ContributionsResponse struct { } `json:"user"` } +// OrgContributionsResponse represents contribution data grouped by repository for org filtering. +type OrgContributionsResponse struct { + User struct { + Login string `json:"login"` + ContributionsCollection struct { + CommitContributionsByRepository []struct { + Repository struct { + Name string `json:"name"` + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Contributions struct { + TotalCount int `json:"totalCount"` + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + } `json:"contributions"` + } `json:"commitContributionsByRepository"` + IssueContributionsByRepository []struct { + Repository struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Contributions struct { + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + } `json:"contributions"` + } `json:"issueContributionsByRepository"` + PullRequestContributionsByRepository []struct { + Repository struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Contributions struct { + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + } `json:"contributions"` + } `json:"pullRequestContributionsByRepository"` + PullRequestReviewContributionsByRepository []struct { + Repository struct { + Owner struct { + Login string `json:"login"` + } `json:"owner"` + } `json:"repository"` + Contributions struct { + Nodes []struct { + OccurredAt string `json:"occurredAt"` + } `json:"nodes"` + } `json:"contributions"` + } `json:"pullRequestReviewContributionsByRepository"` + } `json:"contributionsCollection"` + } `json:"user"` +} + // Point3D represents a point in 3D space using float64 for accuracy in calculations. // Each coordinate (X, Y, Z) represents a position in 3D space. type Point3D struct { From 90227478aabfcd997fc2ea4d77ec5fecddc51554 Mon Sep 17 00:00:00 2001 From: Omer C <639682+omercnet@users.noreply.github.com> Date: Mon, 29 Dec 2025 13:56:06 +0200 Subject: [PATCH 2/2] fix: address PR review comments - safe timestamp parsing and loop fix - Replace unsafe string slice with proper time.Parse for RFC3339 timestamps - Fix potential infinite loop in buildContributionGrid with cleaner termination - Simplify complex year condition to straightforward date comparison - Extract addContributionDate helper to reduce code duplication - Add GoDoc comment to GenerateOrgContributionsResponse --- cmd/skyline/skyline.go | 67 +++++++++++++++++-------------- internal/testutil/mocks/github.go | 3 +- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/cmd/skyline/skyline.go b/cmd/skyline/skyline.go index 1101977..178f552 100644 --- a/cmd/skyline/skyline.go +++ b/cmd/skyline/skyline.go @@ -135,36 +135,36 @@ func fetchOrgContributionData(client *github.Client, username string, org string } dailyCounts := make(map[string]int) + collection := response.User.ContributionsCollection - for _, repo := range response.User.ContributionsCollection.CommitContributionsByRepository { + for _, repo := range collection.CommitContributionsByRepository { if strings.EqualFold(repo.Repository.Owner.Login, org) { for _, node := range repo.Contributions.Nodes { - date := node.OccurredAt[:10] - dailyCounts[date]++ + addContributionDate(node.OccurredAt, dailyCounts) } } } - for _, repo := range response.User.ContributionsCollection.IssueContributionsByRepository { + + for _, repo := range collection.IssueContributionsByRepository { if strings.EqualFold(repo.Repository.Owner.Login, org) { for _, node := range repo.Contributions.Nodes { - date := node.OccurredAt[:10] - dailyCounts[date]++ + addContributionDate(node.OccurredAt, dailyCounts) } } } - for _, repo := range response.User.ContributionsCollection.PullRequestContributionsByRepository { + + for _, repo := range collection.PullRequestContributionsByRepository { if strings.EqualFold(repo.Repository.Owner.Login, org) { for _, node := range repo.Contributions.Nodes { - date := node.OccurredAt[:10] - dailyCounts[date]++ + addContributionDate(node.OccurredAt, dailyCounts) } } } - for _, repo := range response.User.ContributionsCollection.PullRequestReviewContributionsByRepository { + + for _, repo := range collection.PullRequestReviewContributionsByRepository { if strings.EqualFold(repo.Repository.Owner.Login, org) { for _, node := range repo.Contributions.Nodes { - date := node.OccurredAt[:10] - dailyCounts[date]++ + addContributionDate(node.OccurredAt, dailyCounts) } } } @@ -172,6 +172,19 @@ func fetchOrgContributionData(client *github.Client, username string, org string return buildContributionGrid(year, dailyCounts), nil } +// addContributionDate parses an RFC3339 timestamp and increments the count for that date. +// Silently skips malformed timestamps to ensure robustness against API changes. +func addContributionDate(timestamp string, counts map[string]int) { + if len(timestamp) < 10 { + return + } + t, err := time.Parse(time.RFC3339, timestamp) + if err != nil { + return + } + counts[t.Format("2006-01-02")]++ +} + func buildContributionGrid(year int, dailyCounts map[string]int) [][]types.ContributionDay { startDate := time.Date(year, 1, 1, 0, 0, 0, 0, time.UTC) endDate := time.Date(year, 12, 31, 0, 0, 0, 0, time.UTC) @@ -183,27 +196,21 @@ func buildContributionGrid(year int, dailyCounts map[string]int) [][]types.Contr var weeks [][]types.ContributionDay var currentWeek []types.ContributionDay - for d := startDate; !d.After(endDate) || len(currentWeek) > 0; d = d.AddDate(0, 0, 1) { + for d := startDate; !d.After(endDate); d = d.AddDate(0, 0, 1) { dateStr := d.Format("2006-01-02") - count := dailyCounts[dateStr] - - if d.Year() == year || (d.Year() == year-1 && d.After(startDate.AddDate(0, 0, -1))) { - currentWeek = append(currentWeek, types.ContributionDay{ - Date: dateStr, - ContributionCount: count, - }) - } - - if d.Weekday() == time.Saturday || d.Equal(endDate) { - if len(currentWeek) > 0 { - weeks = append(weeks, currentWeek) - currentWeek = nil - } + currentWeek = append(currentWeek, types.ContributionDay{ + Date: dateStr, + ContributionCount: dailyCounts[dateStr], + }) + + if d.Weekday() == time.Saturday { + weeks = append(weeks, currentWeek) + currentWeek = nil } + } - if d.After(endDate) && d.Weekday() == time.Saturday { - break - } + if len(currentWeek) > 0 { + weeks = append(weeks, currentWeek) } return weeks diff --git a/internal/testutil/mocks/github.go b/internal/testutil/mocks/github.go index 67058d1..9d82203 100644 --- a/internal/testutil/mocks/github.go +++ b/internal/testutil/mocks/github.go @@ -60,7 +60,8 @@ func (m *MockGitHubClient) FetchOrgContributions(username string, _ string, _ in return GenerateOrgContributionsResponse(username, "testorg"), nil } -// GenerateOrgContributionsResponse creates mock org contribution data for testing +// GenerateOrgContributionsResponse creates mock org contribution data for testing. +// It generates a sample repository with commit contributions for use in unit tests. func GenerateOrgContributionsResponse(username, org string) *types.OrgContributionsResponse { resp := &types.OrgContributionsResponse{} resp.User.Login = username