-
Notifications
You must be signed in to change notification settings - Fork 278
Improve "TEAMS" Dashboard Tab: Optimize Metrics Retrieval Based on GitHub Enterprise Type #272
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.
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: liguori <[email protected]>
Co-authored-by: liguori <[email protected]>
Co-authored-by: liguori <[email protected]>
…d of REST API Co-authored-by: liguori <[email protected]>
…rprises Co-authored-by: liguori <[email protected]>
Add GitHub Enterprise type support for TEAMS tab functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for distinguishing between "Full GitHub Enterprises" and "Copilot Business Only" enterprises to optimize team retrieval and metrics access. The implementation adds a new enterpriseType
configuration option that affects how teams are fetched - using GraphQL to enumerate organizations for full enterprises versus using enterprise-level APIs for copilot-only enterprises.
Key changes include:
- Added
enterpriseType
configuration with valuesfull
orcopilot-only
(defaulting tocopilot-only
for backward compatibility) - Enhanced team retrieval logic to use GraphQL for full enterprises and existing enterprise APIs for copilot-only
- Updated API URL construction to handle different enterprise types appropriately
Reviewed Changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
tests/enterprise-type.spec.ts |
Comprehensive test suite covering enterprise type handling, serialization, and API URL generation |
server/api/teams.ts |
Enhanced teams API with GraphQL support for full enterprises and organization enumeration |
nuxt.config.ts |
Added default enterpriseType configuration |
app/model/Options.ts |
Extended Options class with enterprise type support and API URL logic |
README.md |
Updated documentation explaining enterprise type differences |
DEPLOYMENT.md |
Added deployment examples for both enterprise types |
.env |
Added environment variable configuration for enterprise type |
logger.info(`Fetching organizations for full enterprise ${options.githubEnt} using GraphQL`) | ||
|
||
const graphqlQuery = { | ||
query: `query { enterprise(slug: "${options.githubEnt}") { organizations(first: 100) { nodes { login name url } } } }` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Direct string interpolation in GraphQL query creates a potential injection vulnerability. The enterprise slug should be properly escaped or parameterized to prevent GraphQL injection attacks.
query: `query { enterprise(slug: "${options.githubEnt}") { organizations(first: 100) { nodes { login name url } } } }` | |
query: `query($slug: String!) { enterprise(slug: $slug) { organizations(first: 100) { nodes { login name url } } } }`, | |
variables: { slug: options.githubEnt } |
Copilot uses AI. Check for mistakes.
const graphqlQuery = { | ||
query: `query { enterprise(slug: "${options.githubEnt}") { organizations(first: 100) { nodes { login name url } } } }` | ||
} | ||
|
||
const graphqlResponse = await $fetch<GraphQLResponse>('https://api.github.com/graphql', { | ||
method: 'POST', | ||
headers: event.context.headers, | ||
body: JSON.stringify(graphqlQuery) | ||
}) | ||
|
||
const data = res._data as GitHubTeam[] | ||
for (const t of data) { | ||
const name: string = t.name | ||
const slug: string = t.slug | ||
const description: string = t.description || '' | ||
if (name && slug) allTeams.push({ name, slug, description }) | ||
|
||
const organizations = graphqlResponse.data.enterprise.organizations.nodes | ||
logger.info(`Found ${organizations.length} organizations in enterprise`) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded limit of 100 organizations could be problematic for large enterprises. Consider implementing pagination or making this configurable to handle enterprises with more than 100 organizations.
const graphqlQuery = { | |
query: `query { enterprise(slug: "${options.githubEnt}") { organizations(first: 100) { nodes { login name url } } } }` | |
} | |
const graphqlResponse = await $fetch<GraphQLResponse>('https://api.github.com/graphql', { | |
method: 'POST', | |
headers: event.context.headers, | |
body: JSON.stringify(graphqlQuery) | |
}) | |
const data = res._data as GitHubTeam[] | |
for (const t of data) { | |
const name: string = t.name | |
const slug: string = t.slug | |
const description: string = t.description || '' | |
if (name && slug) allTeams.push({ name, slug, description }) | |
const organizations = graphqlResponse.data.enterprise.organizations.nodes | |
logger.info(`Found ${organizations.length} organizations in enterprise`) | |
// Paginate through all organizations in the enterprise | |
let organizations: GraphQLOrganization[] = []; | |
let hasNextPage = true; | |
let endCursor: string | null = null; | |
while (hasNextPage) { | |
const graphqlQuery = { | |
query: `query { | |
enterprise(slug: "${options.githubEnt}") { | |
organizations(first: 100${endCursor ? `, after: "${endCursor}"` : ''}) { | |
nodes { login name url } | |
pageInfo { hasNextPage endCursor } | |
} | |
} | |
}` | |
}; | |
const graphqlResponse = await $fetch<any>('https://api.github.com/graphql', { | |
method: 'POST', | |
headers: event.context.headers, | |
body: JSON.stringify(graphqlQuery) | |
}); | |
const orgs = graphqlResponse.data.enterprise.organizations.nodes; | |
organizations = organizations.concat(orgs); | |
const pageInfo = graphqlResponse.data.enterprise.organizations.pageInfo; | |
hasNextPage = pageInfo.hasNextPage; | |
endCursor = pageInfo.endCursor; | |
} | |
logger.info(`Found ${organizations.length} organizations in enterprise`); |
Copilot uses AI. Check for mistakes.
const teamParts = this.githubTeam.split(' - '); | ||
if (teamParts.length >= 2) { | ||
const orgName = teamParts[0]; | ||
const teamName = teamParts[1]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The team slug parsing logic uses teamParts.length >= 2
but only extracts the first two parts. If a team name contains ' - ', this could lead to incorrect parsing. Consider using a more robust parsing approach like split(' - ', 2)
or joining remaining parts for the team name.
const teamParts = this.githubTeam.split(' - '); | |
if (teamParts.length >= 2) { | |
const orgName = teamParts[0]; | |
const teamName = teamParts[1]; | |
const separator = ' - '; | |
const sepIndex = this.githubTeam.indexOf(separator); | |
if (sepIndex > 0) { | |
const orgName = this.githubTeam.substring(0, sepIndex); | |
const teamName = this.githubTeam.substring(sepIndex + separator.length); |
Copilot uses AI. Check for mistakes.
This pull request adds support for distinguishing between "Full GitHub Enterprises" and "Copilot Business Only" enterprises, enabling more accurate team retrieval and metrics for each enterprise type. The change introduces a new
enterpriseType
option throughout the configuration, API, and model layers, updates documentation, and provides comprehensive tests for the new logic.Enterprise Type Support and API Logic:
NUXT_PUBLIC_ENTERPRISE_TYPE
environment variable and propagated its usage throughout the codebase to distinguish betweenfull
andcopilot-only
enterprise types, affecting how teams and metrics are retrieved. (.env
,nuxt.config.ts
, [1] [2]Options
model to includeenterpriseType
, with serialization, deserialization, and merging logic, and modified API URL construction to support both enterprise types for team and enterprise scopes. (app/model/Options.ts
, [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14]Backend Team Retrieval Logic:
server/api/teams.ts
, [1] [2] [3] [4]Documentation Updates:
DEPLOYMENT.md
, [1];README.md
, [2]Testing:
Options
enterprise type handling, including serialization, merging, API URL logic, and error cases. (tests/enterprise-type.spec.ts
, tests/enterprise-type.spec.tsR1-R122)@karpikpl as discussed this is a first implementation created with the help of Coding Agent and tested by myself in a Codespaces, should start to address #271, please verify it.
NOTE: A new scope is required for the GitHub Token (read:org)