fix: handle nested scope objects in ListScopes API response#51
Conversation
The DevLake GET /scopes API returns scopes wrapped in a 'scope' key:
{ count: N, scopes: [{ scope: { githubId: ..., fullName: ... } }] }
The previous ScopeListEntry assumed flat fields (scopeId, scopeName)
which don't exist. Fixed to:
- Added ScopeListWrapper type for the nesting
- ScopeListEntry now captures githubId (int, GitHub) and id (string, Copilot)
- listConnectionScopes resolves the ID field based on plugin type
Found during live testing against a real DevLake instance with 231 GitHub
scopes and 1 Copilot scope.
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in the scope listing functionality where the DevLake API returns scopes in a nested structure that wasn't properly parsed. The previous implementation expected flat scopeId/scopeName fields, but the actual API wraps each scope in a scope key with different field names (githubId/id, name, fullName).
Changes:
- Introduced
ScopeListWrapperto unwrap the nestedscopeobject from API responses - Updated
ScopeListEntryfields to match actual API structure (GithubID, ID, Name, FullName) - Added scope ID resolution logic to handle plugin-specific ID field differences (GitHub's int
githubIdvs Copilot's stringid)
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/devlake/types.go | Added ScopeListWrapper type and updated ScopeListEntry/ScopeListResponse to match the nested API response structure |
| cmd/configure_projects.go | Added unwrapping logic and plugin-specific scope ID/name resolution when processing scopes |
| // Resolve scope ID: GitHub uses githubId (int), Copilot uses id (string) | ||
| scopeID := s.ID | ||
| if c.plugin == "github" && s.GithubID > 0 { | ||
| scopeID = fmt.Sprintf("%d", s.GithubID) | ||
| } |
There was a problem hiding this comment.
If both s.GithubID is 0 (or negative) and s.ID is empty, scopeID will be an empty string. This could lead to issues when creating BlueprintScope entries with empty IDs. Consider adding validation to ensure at least one ID field is populated, or log a warning when this occurs.
| var bpScopes []devlake.BlueprintScope | ||
| var repos []string | ||
| for _, s := range resp.Scopes { | ||
| for _, w := range resp.Scopes { |
There was a problem hiding this comment.
The ListScopes API call in client.go uses pageSize=100&page=1, which may not retrieve all scopes if more than 100 exist. The PR description mentions testing with 231 GitHub scopes. Consider verifying that all scopes were successfully retrieved, and if not, implement pagination logic or use a larger page size. The resp.Count field could be checked to warn if scopes were truncated.
Bug found during live testing: the DevLake
GET /scopesAPI returns scopes wrapped in a nestedscopekey, not flatscopeId/scopeNamefields.Before:
ScopeListEntryexpected{ scopeId, scopeName }- parsed as empty.After:
ScopeListWrapper.Scopehandles{ scope: { githubId, id, fullName } }correctly.Tested against a real DevLake instance with 231 GitHub scopes and 1 Copilot scope.